Re: [PROBLEM] ALSA: hda: PCM streams are suspended while opening

[Jon Hunter <jonathanh@nvidia.com>]

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@suse.de>
> 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@nvidia.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  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@nvidia.com>

Cheers
Jon

-- 
nvpublic