Hi Pierre, >-----Original Message----- >From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@xxxxxxxxxxxxxxx] >Sent: Monday, May 6, 2019 11:08 PM >To: Yang, Libin <libin.yang@xxxxxxxxx>; alsa-devel@xxxxxxxxxxxxxxxx; >broonie@xxxxxxxxxx >Cc: Sridharan, Ranjani <ranjani.sridharan@xxxxxxxxx>; Wang, Rander ><rander.wang@xxxxxxxxx> >Subject: Re: [PATCH] ASoC: soc-pcm: BE dai needs prepare when >pause release after resume > >On 5/5/19 1:37 AM, libin.yang@xxxxxxxxx wrote: >> From: Libin Yang <libin.yang@xxxxxxxxx> >> >> If playback/capture is paused and system enters S3, after system >> returns from suspend, BE dai needs to call prepare() callback when >> playback/capture is released from pause. > >Libin, this patch was discussed at length on GitHub [1] and the commit >message does not convey any of the information we looked into. It's not very >helpful to send such patches to a larger audience without explaining context Thanks for pointing it out. I will add more context in the patch description. >and goals. I personally still have no idea of the state machine and if all >solutions need this or if this is only needed in the case where the >RESUME_INFO flag is not set. Let me describe the bug for the reviewers who doesn't know the details. The patch is trying to fix the below bug: Playback -> pause -> suspend -> resume -> pause release. When pause release, playback should continue to play. However we found playback will quit abnormally. This is because we should restore the registers after S3. Currently RESUME_INFO flag is not set, so alsa will call prepare() callback. But as you can see, dpcm_be_dai_prepare() will block calling prepare() because it lacks of this patch. This means prepare() of BE will not be called, which is wrong. So this patch adds the judgement to let the driver call the prepare() if the state is SND_SOC_DPCM_STATE_PAUSED. And the state is SND_SOC_DPCM_STATE_PAUSED because it is paused. Although S3 happens before pause release, but the state is still in SND_SOC_DPCM_STATE_PAUSED based on the code of dpcm_be_dai_trigger(). If RESUME_INFO flag is not set, prepare() will not be called by alsa after S3. So the patch is not necessary. Regards, Libin > >[1] https://github.com/thesofproject/linux/pull/868 > > >> >> Signed-off-by: Libin Yang <libin.yang@xxxxxxxxx> >> --- >> sound/soc/soc-pcm.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index >> d2aa560..0888995 100644 >> --- a/sound/soc/soc-pcm.c >> +++ b/sound/soc/soc-pcm.c >> @@ -2471,7 +2471,8 @@ int dpcm_be_dai_prepare(struct >> snd_soc_pcm_runtime *fe, int stream) >> >> if ((be->dpcm[stream].state != >SND_SOC_DPCM_STATE_HW_PARAMS) && >> (be->dpcm[stream].state != SND_SOC_DPCM_STATE_STOP) >&& >> - (be->dpcm[stream].state != >SND_SOC_DPCM_STATE_SUSPEND)) >> + (be->dpcm[stream].state != >SND_SOC_DPCM_STATE_SUSPEND) && >> + (be->dpcm[stream].state != >SND_SOC_DPCM_STATE_PAUSED)) >> continue; >> >> dev_dbg(be->dev, "ASoC: prepare BE %s\n", >> _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
