On Mon, 25 Mar 2019 16:31:26 +0100,
Jon Hunter wrote:
>
>
> On 25/03/2019 09:59, Takashi Iwai wrote:
>
> ...
>
> > I reconsidered the problem again, and noticed that the very same
> > problem may appear with the system PM, not only with runtime PM.
> > The suspend can happen at any time, so even a stream in OPEN state may
> > go to suspend, and you'll hit the same problem. It's just a corner
> > case so no one really cared much.
> >
> > So, I think the patch like below should fix the problem.
> > This can be easily backported to all stable trees, and it alone should
> > work without backporting the else intrusive changes.
> >
> > Could you check whether my theory is correct?
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > From: Takashi Iwai <tiwai@xxxxxxx>
> > Subject: [PATCH] ALSA: pcm: Don't suspend stream in unrecoverable PCM state
> >
> > Currently PCM core sets each opened stream forcibly to SUSPENDED state
> > via snd_pcm_suspend_all() call, and the user-space is responsible for
> > re-triggering the resume manually either via snd_pcm_resume() or
> > prepare call. The scheme works fine usually, but there are corner
> > cases where the stream can't be resumed by that call: the streams
> > still in OPEN state before finishing hw_params. When they are
> > suspended, user-space cannot perform resume or prepare because they
> > haven't been set up yet. The only possible recovery is to re-open the
> > device, which isn't nice at all. Similarly, when a stream is in
> > DISCONNECTED state, it makes no sense to change it to SUSPENDED
> > state. Ditto for in SETUP state; which you can re-prepare directly.
> >
> > So, this patch addresses these issues by filtering the PCM streams to
> > be suspended by checking the PCM state. When a stream is in either
> > OPEN, SETUP or DISCONNECTED as well as already SUSPENDED, the suspend
> > action is skipped.
> >
> > To be noted, this problem was originally reported for the PCM runtime
> > PM on HD-audio. And, the runtime PM problem itself was already
> > addressed (although not intended) by the code refactoring commits
> > 3d21ef0b49f8 ("ALSA: pcm: Suspend streams globally via device type PM
> > ops") and 17bc4815de58 ("ALSA: pci: Remove superfluous
> > snd_pcm_suspend*() calls"). These commits eliminated the
> > snd_pcm_suspend*() calls from the runtime PM suspend callback code
> > path, hence the racy OPEN state won't appear while runtime PM.
> > (FWIW, the race window is between snd_pcm_open_substream() and the
> > first power up in azx_pcm_open().)
> >
> > Although the runtime PM issue was already "fixed", the same problem is
> > still present for the system PM, hence this patch is still needed.
> > And for stable trees, this patch alone should suffice for fixing the
> > runtime PM problem, too.
> >
> > Reported-by: Jonathan Hunter <jonathanh@xxxxxxxxxx>
> > Cc: <stable@xxxxxxxxxxxxxxx>
> > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
> > ---
> > sound/core/pcm_native.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> > index f731f904e8cc..1d8452912b14 100644
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -1445,8 +1445,15 @@ static int snd_pcm_pause(struct snd_pcm_substream *substream, int push)
> > static int snd_pcm_pre_suspend(struct snd_pcm_substream *substream, int state)
> > {
> > struct snd_pcm_runtime *runtime = substream->runtime;
> > - if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED)
> > + switch (runtime->status->state) {
> > + case SNDRV_PCM_STATE_SUSPENDED:
> > return -EBUSY;
> > + /* unresumable PCM state; return -EBUSY for skipping suspend */
> > + case SNDRV_PCM_STATE_OPEN:
> > + case SNDRV_PCM_STATE_SETUP:
> > + case SNDRV_PCM_STATE_DISCONNECTED:
> > + return -EBUSY;
> > + }
> > runtime->trigger_master = substream;
> > return 0;
> > }
>
> Thanks, this works for me! I have tested this on stable branch
> linux-5.0.y with Tegra194. So feel free to add my ...
>
> Tested-by: Jon Hunter <jonathanh@xxxxxxxxxx>
OK, I'm going to queue the fix now. Thanks for quick testing!
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
