[alsa-devel] [PATCH V2] 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 if RESUME_INFO flag is not set.
Currently, the dpcm_be_dai_prepare() function will block calling prepare() if the pcm is in SND_SOC_DPCM_STATE_PAUSED state. This will cause the following test case fail if the pcm uses BE:
playback -> pause -> S3 suspend -> S3 resume -> pause release
The playback may exit abnormally when pause is released because the BE dai prepare() is not called.
This patch allows dpcm_be_dai_prepare() to call dai prepare() callback in SND_SOC_DPCM_STATE_PAUSED state.
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 Wed, 2019-05-08 at 10:32 +0800, 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 if RESUME_INFO flag is not set.
Hi Takashi,
This is a question for you. We've run into the problem of not being able to do a pause-release after the system resumes from S3 after we removed INFO_RESUME from the SOF driver.
Apparently, with this flag removed, when the user does a pause release after resuming from S3, the prepare() callback gets invoked for the FE but doesnt happen for the the BE. Could you please guide us on whether this is the right approach and if not, suggest an alternative?
Thanks, Ranjani
Currently, the dpcm_be_dai_prepare() function will block calling prepare() if the pcm is in SND_SOC_DPCM_STATE_PAUSED state. This will cause the following test case fail if the pcm uses BE:
playback -> pause -> S3 suspend -> S3 resume -> pause release
The playback may exit abnormally when pause is released because the BE dai prepare() is not called.
This patch allows dpcm_be_dai_prepare() to call dai prepare() callback in SND_SOC_DPCM_STATE_PAUSED state.
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 Wed, 08 May 2019 18:30:08 +0200, Ranjani Sridharan wrote:
On Wed, 2019-05-08 at 10:32 +0800, 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 if RESUME_INFO flag is not set.
Hi Takashi,
This is a question for you. We've run into the problem of not being able to do a pause-release after the system resumes from S3 after we removed INFO_RESUME from the SOF driver.
Apparently, with this flag removed, when the user does a pause release after resuming from S3, the prepare() callback gets invoked for the FE but doesnt happen for the the BE. Could you please guide us on whether this is the right approach and if not, suggest an alternative?
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause-release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD-audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re-start. But I'm open and would like to hear more opinions.
thanks,
Takashi
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, May 9, 2019 5:21 AM To: Ranjani Sridharan ranjani.sridharan@linux.intel.com Cc: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; Sridharan, Ranjani ranjani.sridharan@intel.com; pierre- louis.bossart@linux.intel.com; Wang, Rander rander.wang@intel.com; broonie@kernel.org Subject: Re: [alsa-devel] [PATCH V2] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
On Wed, 08 May 2019 18:30:08 +0200, Ranjani Sridharan wrote:
On Wed, 2019-05-08 at 10:32 +0800, 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 if RESUME_INFO flag is not set.
Hi Takashi,
This is a question for you. We've run into the problem of not being able to do a pause-release after the system resumes from S3 after we removed INFO_RESUME from the SOF driver.
Apparently, with this flag removed, when the user does a pause release after resuming from S3, the prepare() callback gets invoked for the FE but doesnt happen for the the BE. Could you please guide us on whether this is the right approach and if not, suggest an alternative?
I think this may be a ASoC FE-BE defect. In this case, ASoC will call FE dai prepare(), but it will not call BE dai prepare() because of the judgement. This is why I made the patch.
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause- release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD-audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
I think our current solution is reasonable. we should remove INFO_RESUME for Intel platform. The only side effect is that we will restart the PCM. My understanding is that INFO_RESUME is used for those platforms which can support suspend/resume by hardware. And obviously, on intel platforms, we need do a lot of recovery for resume in driver.
Regards, Libin
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re- start. But I'm open and would like to hear more opinions.
thanks,
Takashi
On Thu, May 09, 2019 at 02:30:39AM +0000, Yang, Libin wrote:
I think this may be a ASoC FE-BE defect. In this case, ASoC will call FE dai prepare(), but it will not call BE dai prepare() because of the judgement. This is why I made the patch.
If it's calling only the front end but not the back end that definitely sounds like DPCM is causing trouble again.
Hi Mark,
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Sunday, May 12, 2019 4:08 PM To: Yang, Libin libin.yang@intel.com Cc: Takashi Iwai tiwai@suse.de; Ranjani Sridharan ranjani.sridharan@linux.intel.com; alsa-devel@alsa-project.org; Sridharan, Ranjani ranjani.sridharan@intel.com; pierre-louis.bossart@linux.intel.com; Wang, Rander rander.wang@intel.com Subject: Re: [alsa-devel] [PATCH V2] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
On Thu, May 09, 2019 at 02:30:39AM +0000, Yang, Libin wrote:
I think this may be a ASoC FE-BE defect. In this case, ASoC will call FE dai prepare(), but it will not call BE dai prepare() because of the judgement. This is why I made the patch.
If it's calling only the front end but not the back end that definitely sounds like DPCM is causing trouble again.
Yes. This is caused by dpcm_be_dai_prepare(). dpcm_be_dai_prepare() should call BE dai prepare() in the case of pause release after S3. But as the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED, it will not call BE dai prepare.
Regards, Libin
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause-release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD-audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re-start. But I'm open and would like to hear more opinions.
Hi Takashi,
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that make sense?
Thanks, Ranjani
On Thu, 09 May 2019 18:56:12 +0200, Ranjani Sridharan wrote:
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause-release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD-audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re-start. But I'm open and would like to hear more opinions.
Hi Takashi,
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that make sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
thanks,
Takashi
On Thu, 2019-05-09 at 19:35 +0200, Takashi Iwai wrote:
On Thu, 09 May 2019 18:56:12 +0200, Ranjani Sridharan wrote:
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause-release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD- audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re-start. But I'm open and would like to hear more opinions.
Hi Takashi,
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that make sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
Thanks, Takashi. We will address this issue in SOF for now and I will also file an enhancement request to address this in the ALSA core.
Ranjani
thanks,
Takashi _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Friday, May 10, 2019 1:35 AM To: Ranjani Sridharan ranjani.sridharan@linux.intel.com Cc: Yang, Libin libin.yang@intel.com; alsa-devel@alsa-project.org; Sridharan, Ranjani ranjani.sridharan@intel.com; pierre- louis.bossart@linux.intel.com; Wang, Rander rander.wang@intel.com; broonie@kernel.org Subject: Re: [alsa-devel] [PATCH V2] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
On Thu, 09 May 2019 18:56:12 +0200, Ranjani Sridharan wrote:
Hm, it's a good question. Currently the PCM core doesn't care about the paused stream wrt PM by the assumption that the paused / stopped stream doesn't need a special resume treatment. But, generally speaking, the pause-release won't work for a hardware that doesn't support the full resume, either. For example, the legacy HD-audio may restart from some wrong position if resumed from the pause.
Maybe this problem hasn't been seen just because the pause function is rarely used.
So, the safe behavior would be to let the stream being SUSPENDED state at snd_pcm_stream_suspend() when it's in the PAUSED and has no INFO_RESUME capability. Then the application does re-prepare the stream like the running one.
But the question is what's expected at next. Should the application re-start? But it was paused. Should PCM core automatically move to pause? But most hardware can't move the pointer to any random position.
My gut feeling is just to treat like a normal error-restart, i.e. re-prepare / re-start. But I'm open and would like to hear more opinions.
Hi Takashi,
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that
make
sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
Ranjani, with "regular pause-release", do you mean pause-release without S3? The prepare() is called from alsa core (pcm_native.c) in S3 case. Prepare() being called in pause-release after S3 is because of S3, not because of pause-release. Actually, if you pause-release without S3 (not sure in pm-runtime case), ASoC's prepare() will not be called. So dpcm_be_dai_prepare() will not be called. So you assumption of "regular pause-release" calling prepare() is wrong.
Please let me describe the flow below: 1. Pause-release after S3 without RESUME_INFO Prepare() -> trigger start 2. pause-release without S3 without/with RESUME_INFO Trigger pause-release 3. Pause-release after S3 with RESUME_INFO Trigger resume
So you will see prepare() is only called in case 1. This is because S3 happens before pause-release. I believe all drivers need prepare() in such case, not only for SOF driver.
Regards, Libin
thanks,
Takashi
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that
make
sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
Ranjani, with "regular pause-release", do you mean pause-release without S3? The prepare() is called from alsa core (pcm_native.c) in S3 case. Prepare() being called in pause-release after S3 is because of S3, not because of pause-release. Actually, if you pause-release without S3 (not sure in pm-runtime case), ASoC's prepare() will not be called. So dpcm_be_dai_prepare() will not be called. So you assumption of "regular pause-release" calling prepare() is wrong.
Oh yes. That's right. Thanks for pointing it out. In this case, the patch sounds like a good fix. Basically, you're saying that if the FE prepare() gets called (which happens in the case of pause-release without INFO_RESUME) it should also call the BE prepare(), right?
Takashi, what do you think?
Please let me describe the flow below:
- Pause-release after S3 without RESUME_INFO
Prepare() -> trigger start 2. pause-release without S3 without/with RESUME_INFO Trigger pause-release
- Pause-release after S3 with RESUME_INFO
Trigger resume
Are you sure about this? A paused stream will not be suspended. So it would still be trigger PAUSE-RELEASE in this case?
Thanks, Ranjani
-----Original Message----- From: Ranjani Sridharan [mailto:ranjani.sridharan@linux.intel.com] Sent: Friday, May 10, 2019 10:03 AM To: Yang, Libin libin.yang@intel.com; Takashi Iwai tiwai@suse.de Cc: alsa-devel@alsa-project.org; Sridharan, Ranjani ranjani.sridharan@intel.com; pierre-louis.bossart@linux.intel.com; Wang, Rander rander.wang@intel.com; broonie@kernel.org Subject: Re: [alsa-devel] [PATCH V2] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that
make
sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
Ranjani, with "regular pause-release", do you mean pause-release without S3? The prepare() is called from alsa core (pcm_native.c) in S3 case. Prepare() being called in pause-release after S3 is because of S3, not because of pause-release. Actually, if you pause-release without S3 (not sure in pm-runtime case), ASoC's prepare() will not be called. So dpcm_be_dai_prepare() will not be called. So you assumption of "regular pause-release" calling prepare() is wrong.
Oh yes. That's right. Thanks for pointing it out. In this case, the patch sounds like a good fix. Basically, you're saying that if the FE prepare() gets called (which happens in the case of pause-release without INFO_RESUME) it should also call the BE prepare(), right?
I mean as there is a S3, we need prepare() for both FE and BE. And logically, if ASoC calls FE prepare(), it should also call BE prepare(). Otherwise, FE and BE are not synced. The behavior is unknown unless we really know what's happening in ASoC.
Takashi, what do you think?
Please let me describe the flow below:
- Pause-release after S3 without RESUME_INFO
Prepare() -> trigger start 2. pause-release without S3 without/with RESUME_INFO Trigger pause-release
- Pause-release after S3 with RESUME_INFO Trigger resume
Are you sure about this? A paused stream will not be suspended. So it would still be trigger PAUSE-RELEASE in this case?
Hum, maybe you are right. I didn't test such case. If we don't need call "trigger resume" even after S3? If it triggers PAUSE-RELEASE, how can we know it is after S3 or not? Driver may do different operations for pause release for with S3 or without S3.
Regards, Libin
Thanks, Ranjani
On Fri, 10 May 2019 04:32:11 +0200, Yang, Libin wrote:
-----Original Message----- From: Ranjani Sridharan [mailto:ranjani.sridharan@linux.intel.com] Sent: Friday, May 10, 2019 10:03 AM To: Yang, Libin libin.yang@intel.com; Takashi Iwai tiwai@suse.de Cc: alsa-devel@alsa-project.org; Sridharan, Ranjani ranjani.sridharan@intel.com; pierre-louis.bossart@linux.intel.com; Wang, Rander rander.wang@intel.com; broonie@kernel.org Subject: Re: [alsa-devel] [PATCH V2] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
So in the current scenario what we see is that after resuming from S3, a pause-release action from the user results in a FE prepare() followed by the START trigger (and not a PAUSE-RELEASE trigger).
Libin's patch proposes to do a prepare() for the BE even in the case of a regular pause-release. But this might not be ideal since other drivers might have logic in the prepare() ioctl that might end up with errors.
Right.
So I am thinking maybe we can have some internal logic in the SOF prepare() callback that will also call the BE prepare() when the be->dpcm[stream].state is SND_SOC_DPCM_STATE_PAUSED? Would that
make
sense?
Yes, that would work, I guess. Eventually this might be needed to be addressed in ALSA core side, too, but it's good to have some fix beforehand in DPCM.
Ranjani, with "regular pause-release", do you mean pause-release without S3? The prepare() is called from alsa core (pcm_native.c) in S3 case. Prepare() being called in pause-release after S3 is because of S3, not because of pause-release. Actually, if you pause-release without S3 (not sure in pm-runtime case), ASoC's prepare() will not be called. So dpcm_be_dai_prepare() will not be called. So you assumption of "regular pause-release" calling prepare() is wrong.
Oh yes. That's right. Thanks for pointing it out. In this case, the patch sounds like a good fix. Basically, you're saying that if the FE prepare() gets called (which happens in the case of pause-release without INFO_RESUME) it should also call the BE prepare(), right?
I mean as there is a S3, we need prepare() for both FE and BE. And logically, if ASoC calls FE prepare(), it should also call BE prepare(). Otherwise, FE and BE are not synced. The behavior is unknown unless we really know what's happening in ASoC.
Takashi, what do you think?
Please let me describe the flow below:
- Pause-release after S3 without RESUME_INFO
Prepare() -> trigger start 2. pause-release without S3 without/with RESUME_INFO Trigger pause-release
- Pause-release after S3 with RESUME_INFO Trigger resume
Are you sure about this? A paused stream will not be suspended. So it would still be trigger PAUSE-RELEASE in this case?
Hum, maybe you are right. I didn't test such case. If we don't need call "trigger resume" even after S3? If it triggers PAUSE-RELEASE, how can we know it is after S3 or not? Driver may do different operations for pause release for with S3 or without S3.
Yes, some change in ALSA PCM core side is needed for that. It's what I mentioned in my previous post.
My rough idea is a patch like below. It'll make trigger(SUSPEND) call for all cases already in PREPARE or later state. You'd need to implement the corresponding trigger stuff properly in the driver side, of course.
thanks,
Takashi
--- --- a/include/uapi/sound/asound.h +++ b/include/uapi/sound/asound.h @@ -298,6 +298,7 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_HAS_LINK_ESTIMATED_ATIME 0x04000000 /* report estimated link audio time */ #define SNDRV_PCM_INFO_HAS_LINK_SYNCHRONIZED_ATIME 0x08000000 /* report synchronized audio/system time */
+#define SNDRV_PCM_INFO_SUSPEND_TRIGGER 0x20000000 /* internal kernel flag - always trigger at suspend */ #define SNDRV_PCM_INFO_DRAIN_TRIGGER 0x40000000 /* internal kernel flag - trigger in drain */ #define SNDRV_PCM_INFO_FIFO_IN_FRAMES 0x80000000 /* internal kernel flag - FIFO size is in frames */
--- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1463,7 +1463,8 @@ static int snd_pcm_do_suspend(struct snd_pcm_substream *substream, int state) struct snd_pcm_runtime *runtime = substream->runtime; if (runtime->trigger_master != substream) return 0; - if (! snd_pcm_running(substream)) + if (!(runtime->info & SNDRV_PCM_INFO_TRIGGER_SUSPEND) && + !snd_pcm_running(substream)) return 0; substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_SUSPEND); return 0; /* suspend unconditionally */
The patch
ASoC: soc-pcm: BE dai needs prepare when pause release after resume
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.2
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 5087a8f17df868601cd7568299e91c28086d2b45 Mon Sep 17 00:00:00 2001
From: Libin Yang libin.yang@intel.com Date: Wed, 8 May 2019 10:32:41 +0800 Subject: [PATCH] ASoC: soc-pcm: BE dai needs prepare when pause release after resume
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 if RESUME_INFO flag is not set.
Currently, the dpcm_be_dai_prepare() function will block calling prepare() if the pcm is in SND_SOC_DPCM_STATE_PAUSED state. This will cause the following test case fail if the pcm uses BE:
playback -> pause -> S3 suspend -> S3 resume -> pause release
The playback may exit abnormally when pause is released because the BE dai prepare() is not called.
This patch allows dpcm_be_dai_prepare() to call dai prepare() callback in SND_SOC_DPCM_STATE_PAUSED state.
Signed-off-by: Libin Yang libin.yang@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- 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 74695355c1f8..7347e6f99248 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 (5)
-
libin.yang＠intel.com -
Mark Brown -
Ranjani Sridharan -
Takashi Iwai -
Yang, Libin