Skip to content

Commit

Permalink
Second/Alternative change to correct for 5.5+ change in trigger order
Browse files Browse the repository at this point in the history 
The idea is the same as the previous attempt: calls ac101_trigger() just
before set_clock(1).

respeaker/seeed-voicecard#290
    6mics / linear 4 mics: fails to record against v5.10 kernel
raspberrypi/linux#4279
    [regression] alsa system call blocks on record between 5.4.83 and 5.5.19

In v5.5,

commit 4378f1fbe924054a09ff0d4e39e1a581b9245252
Author: Peter Ujfalusi <[email protected]>
Date:   Fri Sep 27 10:16:46 2019 +0300

    ASoC: soc-pcm: Use different sequence for start/stop trigger

    On stream stop currently we stop the DMA first followed by the CPU DAI.
    This can cause underflow (playback) or overflow (capture) on the DAI side
    as the DMA is no longer feeding data while the DAI is still active.
    It can be observed easily if the DAI side does not have FIFO (or it is
    disabled) to survive the time while the DMA is stopped, but still can
    happen on relatively slow CPUs when relatively high sampling rate is used:
    the FIFO is drained between the time the DMA is stopped and the DAI is
    stopped.

    It can only fixed by using different sequence within trigger for 'stop' and
    'start':
    case SNDRV_PCM_TRIGGER_START:
    case SNDRV_PCM_TRIGGER_RESUME:
    case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
            Trigger order: dai_link, DMA, CPU DAI then the codec

    case SNDRV_PCM_TRIGGER_STOP:
    case SNDRV_PCM_TRIGGER_SUSPEND:
    case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
            Trigger order: codec, CPU DAI, DMA then dai_link

    Signed-off-by: Peter Ujfalusi <[email protected]>
    Reviewed-by: Pierre-Louis Bossart <[email protected]>
    Link: https://lore.kernel.org/r/[email protected]
    Signed-off-by: Mark Brown <[email protected]>
  • Loading branch information
@HinTak
HinTak committed Apr 27, 2021
1 parent 5793cf6 commit 19067f3
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 19 deletions.
2 changes: 1 addition & 1 deletion ac101.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1237,7 +1237,7 @@ int ac101_audio_startup(struct snd_pcm_substream *substream,
}

#if _MASTER_MULTI_CODEC == _MASTER_AC101
static int ac101_set_clock(int y_start_n_stop) {
static int ac101_set_clock(int y_start_n_stop, struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) {
int r;

if (y_start_n_stop) {
Expand Down
16 changes: 7 additions & 9 deletions ac108.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -991,14 +991,17 @@ static int ac108_set_fmt(struct snd_soc_dai *dai, unsigned int fmt) {
/*
* due to miss channels order in cpu_dai, we meed defer the clock starting.
*/
static int ac108_set_clock(int y_start_n_stop) {
static int ac108_set_clock(int y_start_n_stop, struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai) {
u8 reg;
int ret = 0;

dev_dbg(ac10x->codec->dev, "%s() L%d cmd:%d\n", __func__, __LINE__, y_start_n_stop);

/* spin_lock move to machine trigger */

if (y_start_n_stop && ac10x->i2c101 && _MASTER_MULTI_CODEC == _MASTER_AC101) {
ac101_trigger(substream, cmd, dai);
}
if (y_start_n_stop && ac10x->sysclk_en == 0) {
/* enable lrck clock */
ac10x_read(I2S_CTRL, &reg, ac10x->i2cmap[_MASTER_INDEX]);
Expand Down Expand Up @@ -1058,13 +1061,6 @@ static int ac108_trigger(struct snd_pcm_substream *substream, int cmd,
snd_pcm_stream_str(substream),
cmd);

if (ac10x->i2c101 && _MASTER_MULTI_CODEC == _MASTER_AC101) {
ac101_trigger(substream, cmd, dai);
if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
goto __ret;
}
}

switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
Expand All @@ -1083,12 +1079,14 @@ static int ac108_trigger(struct snd_pcm_substream *substream, int cmd,
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
if (ac10x->i2c101 && _MASTER_MULTI_CODEC == _MASTER_AC101) {
ac101_trigger(substream, cmd, dai);
}
break;
default:
ret = -EINVAL;
}

__ret:
dev_dbg(dai->dev, "%s() stream=%s cmd=%d; finished %d\n",
__FUNCTION__,
snd_pcm_stream_str(substream),
Expand Down
2 changes: 1 addition & 1 deletion ac10x.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ void ac101_shutdown(struct i2c_client *i2c);
int ac101_remove(struct i2c_client *i2c);

/* seeed voice card export */
int seeed_voice_card_register_set_clock(int stream, int (*set_clock)(int));
int seeed_voice_card_register_set_clock(int stream, int (*set_clock)(int, struct snd_pcm_substream *, int, struct snd_soc_dai *));

int ac10x_fill_regcache(struct device* dev, struct regmap* map);

Expand Down
16 changes: 8 additions & 8 deletions seeed-voicecard.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,9 +163,9 @@ static int seeed_voice_card_hw_params(struct snd_pcm_substream *substream,
}

#define _SET_CLOCK_CNT 2
static int (* _set_clock[_SET_CLOCK_CNT])(int y_start_n_stop);
static int (* _set_clock[_SET_CLOCK_CNT])(int y_start_n_stop, struct snd_pcm_substream *substream, int cmd, struct snd_soc_dai *dai);

int seeed_voice_card_register_set_clock(int stream, int (*set_clock)(int)) {
int seeed_voice_card_register_set_clock(int stream, int (*set_clock)(int, struct snd_pcm_substream *, int, struct snd_soc_dai *)) {
if (! _set_clock[stream]) {
_set_clock[stream] = set_clock;
}
Expand All @@ -182,10 +182,10 @@ static void work_cb_codec_clk(struct work_struct *work)
int r = 0;

if (_set_clock[SNDRV_PCM_STREAM_CAPTURE]) {
r = r || _set_clock[SNDRV_PCM_STREAM_CAPTURE](0);
r = r || _set_clock[SNDRV_PCM_STREAM_CAPTURE](0, NULL, 0, NULL); /* not using 2nd to 4th arg if 1st == 0 */
}
if (_set_clock[SNDRV_PCM_STREAM_PLAYBACK]) {
r = r || _set_clock[SNDRV_PCM_STREAM_PLAYBACK](0);
r = r || _set_clock[SNDRV_PCM_STREAM_PLAYBACK](0, NULL, 0, NULL); /* not using 2nd to 4th arg if 1st == 0 */
}

if (r && priv->try_stop++ < TRY_STOP_MAX) {
Expand Down Expand Up @@ -217,8 +217,8 @@ static int seeed_voice_card_trigger(struct snd_pcm_substream *substream, int cmd
/* I know it will degrades performance, but I have no choice */
spin_lock_irqsave(&priv->lock, flags);
#endif
if (_set_clock[SNDRV_PCM_STREAM_CAPTURE]) _set_clock[SNDRV_PCM_STREAM_CAPTURE](1);
if (_set_clock[SNDRV_PCM_STREAM_PLAYBACK]) _set_clock[SNDRV_PCM_STREAM_PLAYBACK](1);
if (_set_clock[SNDRV_PCM_STREAM_CAPTURE]) _set_clock[SNDRV_PCM_STREAM_CAPTURE](1, substream, cmd, dai);
if (_set_clock[SNDRV_PCM_STREAM_PLAYBACK]) _set_clock[SNDRV_PCM_STREAM_PLAYBACK](1, substream, cmd, dai);
#if CONFIG_AC10X_TRIG_LOCK
spin_unlock_irqrestore(&priv->lock, flags);
#endif
Expand All @@ -238,8 +238,8 @@ static int seeed_voice_card_trigger(struct snd_pcm_substream *substream, int cmd
if (0 != schedule_work(&priv->work_codec_clk)) {
}
} else {
if (_set_clock[SNDRV_PCM_STREAM_CAPTURE]) _set_clock[SNDRV_PCM_STREAM_CAPTURE](0);
if (_set_clock[SNDRV_PCM_STREAM_PLAYBACK]) _set_clock[SNDRV_PCM_STREAM_PLAYBACK](0);
if (_set_clock[SNDRV_PCM_STREAM_CAPTURE]) _set_clock[SNDRV_PCM_STREAM_CAPTURE](0, NULL, 0, NULL); /* not using 2nd to 4th arg if 1st == 0 */
if (_set_clock[SNDRV_PCM_STREAM_PLAYBACK]) _set_clock[SNDRV_PCM_STREAM_PLAYBACK](0, NULL, 0, NULL); /* not using 2nd to 4th arg if 1st == 0 */
}
break;
default:
Expand Down

0 comments on commit 19067f3

Please sign in to comment.