On Tue, 01 Dec 2020 17:15:15 +0100, Ranjani Sridharan wrote: > > > > > > > I was able to reproduce this error with Mark's for-next branch on a > > > CHT device w/ rt5640, and git bisect points to this commit: > > > > > > a27b421f1d04b201c474a15ee1591919c81fb413 is the first bad commit > > > commit a27b421f1d04b201c474a15ee1591919c81fb413 > > > Author: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx> > > > Date: Tue Nov 17 13:50:01 2020 -0800 > > > > > > ASoC: pcm: call snd_soc_dapm_stream_stop() in soc_pcm_hw_clean > > > > > > Currently, the SND_SOC_DAPM_STREAM_START event is sent during > > > pcm_prepare() but the SND_SOC_DAPM_STREAM_STOP event is > > > sent only in dpcm_fe_dai_shutdown() after soc_pcm_close(). > > > This results in an imbalance between when the DAPM widgets > > > receive the PRE/POST_PMU/PMD events. So call > > > snd_soc_dapm_stream_stop() in soc_pcm_hw_clean() before the > > > snd_soc_pcm_component_hw_free() to keep the stream_stop DAPM > > > event balanced with the stream_start event in soc_pm_prepare(). > > > > > > Also, in order to prevent duplicate DAPM stream events, > > > remove the call for DAPM STREAM_START event in > > > dpcm_fe_dai_prepare() > > > and the call for DAPM STREAM_STOP event in > > > dpcm_fe_dai_shutdown(). > > > > > > Reviewed-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx> > > > Reviewed-by: Pierre-Louis Bossart > > > <pierre-louis.bossart@xxxxxxxxxxxxxxx> > > > Signed-off-by: Ranjani Sridharan < > > > ranjani.sridharan@xxxxxxxxxxxxxxx> > > > Link: > > > https://lore.kernel.org/r/20201117215001.163107-1-ranjani.sridharan@xxxxxxxxxxxxxxx > > > Signed-off-by: Mark Brown <broonie@xxxxxxxxxx> > > > > > > sound/soc/soc-pcm.c | 10 +++------- > > > 1 file changed, 3 insertions(+), 7 deletions(-) > > > > > > > > > I am not sure why this break the Atom/SST driver, this was reviewed > > > and seemed legit - even required IIRC to deal with topology > > > pipelines > > > initialized on-demand. Reverting this patch restores functionality. > > > I > > > would guess it's the DAPM_STREAM_START that's now missing (or in > > > the > > > 'wrong' location) and causing issues? > > > > Indeed the DAPM_START_STREAM call completely disappeared after the > > patch, which looks very wrong. This has to be revisited before 5.11 > > merge. > > Hi Pierre/Takashi, > > The DAPM_STREAM_START event is still there in soc_pcm_prepare() and > this patch only removed the duplicate call in dpcm_fe_dai_prepare(). Ah, thanks, I see now. But note that the PCM prepare callback may be called multiple times in row; i.e. it's not always paired with hw_clean (that is via either hw_params error path or hw_free). So if the balance really matters, we need another type of checks, not relying on the call pattern. Takashi
