>-----Original Message----- >From: Yang, Libin >Sent: Tuesday, May 7, 2019 4:15 PM >To: 'Pierre-Louis Bossart' <pierre-louis.bossart@xxxxxxxxxxxxxxx>; 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 > >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. Typo: " If RESUME_INFO flag is not set " should be "If RESUME_INFO flag is set" Regards, Libin > >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
