From 19067f3333d7759bdf626e62bc09a7deae4627a1 Mon Sep 17 00:00:00 2001 From: Hin-Tak Leung Date: Tue, 27 Apr 2021 01:35:13 +0100 Subject: [PATCH] Second/Alternative change to correct for 5.5+ change in trigger order The idea is the same as the previous attempt: calls ac101_trigger() just before set_clock(1). https://github.com/respeaker/seeed-voicecard/issues/290 6mics / linear 4 mics: fails to record against v5.10 kernel https://github.com/raspberrypi/linux/issues/4279 [regression] alsa system call blocks on record between 5.4.83 and 5.5.19 In v5.5, commit 4378f1fbe924054a09ff0d4e39e1a581b9245252 Author: Peter Ujfalusi 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 Reviewed-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20190927071646.22319-1-peter.ujfalusi@ti.com Signed-off-by: Mark Brown --- ac101.c | 2 +- ac108.c | 16 +++++++--------- ac10x.h | 2 +- seeed-voicecard.c | 16 ++++++++-------- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/ac101.c b/ac101.c index 8e19c53..23837a7 100644 --- a/ac101.c +++ b/ac101.c @@ -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) { diff --git a/ac108.c b/ac108.c index 4075eee..d5dd12d 100644 --- a/ac108.c +++ b/ac108.c @@ -991,7 +991,7 @@ 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; @@ -999,6 +999,9 @@ static int ac108_set_clock(int 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, ®, ac10x->i2cmap[_MASTER_INDEX]); @@ -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: @@ -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), diff --git a/ac10x.h b/ac10x.h index 9b83807..a41bacf 100644 --- a/ac10x.h +++ b/ac10x.h @@ -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); diff --git a/seeed-voicecard.c b/seeed-voicecard.c index 9f5f7e7..b90af93 100644 --- a/seeed-voicecard.c +++ b/seeed-voicecard.c @@ -163,9 +163,9 @@ err: } #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; } @@ -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) { @@ -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 @@ -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: