Re: [PATCH] ALSA: pcm: Release paused streams before suspend if resume is not supported

[Takashi Iwai <tiwai@suse.de>]

On Wed, 02 Apr 2025 14:52:48 +0200,
Péter Ujfalusi wrote:
> 
> 
> 
> On 02/04/2025 14:50, Takashi Iwai wrote:
> > On Wed, 02 Apr 2025 13:43:57 +0200,
> > Péter Ujfalusi wrote:
> >>
> >>
> >>
> >> On 02/04/2025 14:21, Takashi Iwai wrote:
> >>>>> On the second thought, yet another variant would be to introduce only
> >>>>> one new trigger, SNDRV_PCM_TRIGGER_PAUSE_RESET, for resetting the
> >>>>> PAUSED state as a step before stopping the stream.  This can be
> >>>>> applied to all cases for snd_pcm_drop() and snd_pcm_suspend(), too.
> >>>>> If the driver doesn't support this mode, the PCM core may fall back to
> >>>>> SNDRV_PCM_TRIGGER_PAUSE_RELEASE.
> >>>>
> >>>> This looks identical to my proposed
> >>>> SNDRV_PCM_TRIGGER_PAUSE_RELEASE_AND_STOP which is more
> >>>> explanatory. It's not clear what PAUSE_RESET should do (probably the
> >>>> stream should be kept inactive).
> >>>
> >>> There is a slight difference.  My proposal, *_TRIGGER_PAUSE_RESET,
> >>> doesn't have to stop the stream by itself, but it's only a preparation
> >>> for stopping.  That is, a proper TRIGGER_STOP always follows after
> >>> TRIGGER_PAUSE_RESET.
> >>
> >> But what state that we move after we reset the PAUSE? is it RUNNING
> >> (iow, equals to PAUSE_RELEASE) or something else?
> > 
> > It'll be temporarily set to RUNNING.
> > 
> >> In case of suspend while the state is paused we will:
> >> TRIGGER_PAUSE_RESET followed by STOP or SUSPEND?
> > 
> > For suspend, PAUSE_RESET, then SUSPEND.
> > For dropping, PAUSE_RESET, then STOP.
> 
> OK, but this is still the same as PAUSE_RELEASE+SUSPEND when it comes to
> suspended_state, no?
> I mean you need to move to RUNNING to be suspend, or stop to skip the
> suspend.

Yes, and this needs to be addressed.  e.g. save the suspended_state
beforehand.

> >> Most drivers are doing the same thing for STOP and SUSPEND trigger,
> >> however it is possible to do extra steps for SUSPEND to make sure that
> >> the whole system is in a lowest power state.
> > 
> > Yes, that's the reason I proposed PAUSE_RESET.  Then the driver can
> > handle the following stop or suspend operation differently.
> > 
> > That said, PAUSE_RESET is just another variant of PAUSE_RELEASE.
> > But this shouldn't actually enable the audio output again, but prepare
> > for the next SUSPEND or STOP trigger.
> 
> I'm not sure how this would be seen in ASoC level. Surely we need to
> have flag per components that it is handling PAUSE_RESET as codecs would
> like don't want to do it or they migh, hm.
> But ASoC maintains another set of state for the dpcm:
> SND_SOC_DPCM_STATE_* and it has this as well among other state-machine
> things:
> case SNDRV_PCM_TRIGGER_SUSPEND:
> 	if (be->dpcm[stream].state != SND_SOC_DPCM_STATE_START)
> 		goto next;
> 
> So, this is dpcm level, but one component can support PAUSE_RESET while
> the other does not and the transitions are not too clear to me.

It's a good question.  ASoC may have the own intermediate state
(SND_SOC_DPCM_STATE_PAUSED_RESET) or just use
SND_SOC_DPCM_STATE_START but leave all to BE trigger (by passing
SNDRV_PCM_TRIGGER_PAUSE_RESET).

> But the idea of resurrecting a paused stream with mute held is not bad.
>
> I'm still uncertain how this actually works now on systems where RESUME
> is supported and we suspend while a PCM is in paused...

Is it a general question about suspend+resume during pause, or the
behavior change after PAUSE_RESET?  PAUSE_RESET won't be applied to
the drivers that support RESUME.

BTW, I'm not pushing to sell PAUSE_RESET so much for now at all.
Currently rather in a brain storming phase.  Let's try to pick up the
more appropriate one.


thanks,

Takashi