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

[Takashi Iwai <tiwai@suse.de>]

On Tue, 01 Apr 2025 18:58:40 +0200,
Jaroslav Kysela wrote:
> 
> On 01. 04. 25 16:49, Takashi Iwai wrote:
> > On Tue, 01 Apr 2025 15:36:52 +0200,
> > Peter Ujfalusi wrote:
> >> 
> >> Streams moved to SUSPENDED state from PAUSED without trigger. If a stream
> >> does not support RESUME then on system resume the RESUME trigger is not
> >> sent, the stream's state and suspended_state remains untouched.
> >> When the user space releases the pause then the core will reject this
> >> because the state of the stream is _not_ PAUSED, it is still SUSPENDED.
> >> 
> >>  From this point user space will do the normal (hw_params) prepare and
> >> START, PAUSE_RELEASE trigger will not be sent by the core after the
> >> system has resumed.
> 
> Why you expect to see PAUSE_RELEASE trigger before SUSPEND trigger
> when resume is not supported ? I think that the driver has complete
> information:
> 
> PAUSE -> SUSPEND [-> no RESUME]
> 
> The driver should stay in the suspend state until a new stream
> re-initialization is invoked.
> 
> It looks like that other logic should be moved to the end driver (if
> some parts must be reinitialized in the suspend phase when the resume
> is not supported). I don't think that this is job for PCM core nor SoC
> PCM core routines.
> 
> The SUSPEND trigger is always invoked, isn't?

No, that's a part of the problems.  The ops->suspend is called only
for the running state, while at snd_pcm_post_suspend(), the state is
changed to SUSPENDED.  The original state is saved as
suspended_state, and this can be restored via snd_pcm_resume(), but
only if the driver supports the full resume.

We may allow calling ops->suspend also for PAUSED state, too.  But
then each driver would need to have to handle this state change, too.
Currently, when snd_pcm_prepare(), *_drop() or *_drain() is called at
PAUSED state, the core automatically releases the pause beforehand.

> >> To fix this issue, release the paused stream before handling the suspend
> >> if the resume is not supported.
> >> This will ensure that the stream is properly suspended for suspend entry.
> >> On resume no attempt will be taken to resume the suspended stream as the
> >> resume is not supported, userspace will eventually going restart the audio.
> >> 
> >> Link: https://github.com/thesofproject/linux/issues/5035
> >> Link: https://github.com/thesofproject/linux/issues/5341
> >> Suggested-by: Takashi Iwai <tiwai@suse.de>
> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> > 
> > Applied now.  Thanks.
> 
> I would consider revert if my objection is appropriate.

OK, I dropped the patch for now until we get agreement.


thanks,

Takashi