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

["Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>]

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.
>> 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.

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...

-- 
Péter