[alsa-devel] [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
From: Libin Yang libin.yang@intel.com
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.
Signed-off-by: Libin Yang libin.yang@intel.com --- 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",
On 5/5/19 1:37 AM, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
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 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.
[1] https://github.com/thesofproject/linux/pull/868
Signed-off-by: Libin Yang libin.yang@intel.com
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",
Hi Pierre,
-----Original Message----- From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com] Sent: Monday, May 6, 2019 11:08 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; broonie@kernel.org Cc: Sridharan, Ranjani ranjani.sridharan@intel.com; Wang, Rander rander.wang@intel.com Subject: Re: [alsa-devel] [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
On 5/5/19 1:37 AM, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
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@intel.com
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",
-----Original Message----- From: Yang, Libin Sent: Tuesday, May 7, 2019 4:15 PM To: 'Pierre-Louis Bossart' pierre-louis.bossart@linux.intel.com; alsa- devel@alsa-project.org; broonie@kernel.org Cc: Sridharan, Ranjani ranjani.sridharan@intel.com; Wang, Rander rander.wang@intel.com Subject: RE: [alsa-devel] [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@linux.intel.com] Sent: Monday, May 6, 2019 11:08 PM To: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; broonie@kernel.org Cc: Sridharan, Ranjani ranjani.sridharan@intel.com; Wang, Rander rander.wang@intel.com Subject: Re: [alsa-devel] [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
On 5/5/19 1:37 AM, libin.yang@intel.com wrote:
From: Libin Yang libin.yang@intel.com
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@intel.com
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",
participants (3)
-
libin.yang＠intel.com -
Pierre-Louis Bossart -
Yang, Libin