* [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
@ 2019-05-05 6:37 libin.yang
2019-05-06 15:07 ` Pierre-Louis Bossart
0 siblings, 1 reply; 4+ messages in thread
From: libin.yang @ 2019-05-05 6:37 UTC (permalink / raw)
To: alsa-devel, broonie
Cc: Libin Yang, ranjani.sridharan, pierre-louis.bossart, rander.wang
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",
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
2019-05-05 6:37 [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume libin.yang
@ 2019-05-06 15:07 ` Pierre-Louis Bossart
2019-05-07 8:15 ` Yang, Libin
2019-05-07 8:18 ` Yang, Libin
0 siblings, 2 replies; 4+ messages in thread
From: Pierre-Louis Bossart @ 2019-05-06 15:07 UTC (permalink / raw)
To: libin.yang, alsa-devel, broonie; +Cc: ranjani.sridharan, rander.wang
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",
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
2019-05-06 15:07 ` Pierre-Louis Bossart
@ 2019-05-07 8:15 ` Yang, Libin
2019-05-07 8:18 ` Yang, Libin
1 sibling, 0 replies; 4+ messages in thread
From: Yang, Libin @ 2019-05-07 8:15 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel, broonie
Cc: Sridharan, Ranjani, Wang, Rander
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",
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
2019-05-06 15:07 ` Pierre-Louis Bossart
2019-05-07 8:15 ` Yang, Libin
@ 2019-05-07 8:18 ` Yang, Libin
1 sibling, 0 replies; 4+ messages in thread
From: Yang, Libin @ 2019-05-07 8:18 UTC (permalink / raw)
To: Pierre-Louis Bossart, alsa-devel, broonie
Cc: Sridharan, Ranjani, Wang, Rander
>-----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",
>>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-07 8:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-05 6:37 [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume libin.yang
2019-05-06 15:07 ` Pierre-Louis Bossart
2019-05-07 8:15 ` Yang, Libin
2019-05-07 8:18 ` Yang, Libin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.