Re: [RESEND PATCH] ALSA: dmaengine_pcm: terminate dmaengine before synchronize

[Shengjiu Wang <shengjiu.wang@xxxxxxxxx>]

On Thu, Jun 20, 2024 at 3:56 PM Takashi Iwai <tiwai@xxxxxxx> wrote:
>
> On Thu, 20 Jun 2024 04:40:18 +0200,
> Shengjiu Wang wrote:
> >
> > When dmaengine supports pause function, in suspend state,
> > dmaengine_pause() is called instead of dmaengine_terminate_async(),
> >
> > In end of playback stream, the runtime->state will go to
> > SNDRV_PCM_STATE_DRAINING, if system suspend & resume happen
> > at this time, application will not resume playback stream, the
> > stream will be closed directly, the dmaengine_terminate_async()
> > will not be called before the dmaengine_synchronize(), which
> > violates the call sequence for dmaengine_synchronize().
>
> Hmm, I can't follow this state change.
> Do you mean that:
> - snd_pcm_drain() is performed for a playback stream
> - while draining operation, the system goes to suspend
> - the system resumes (but the application doesn't call resume yet)
> - The stream is closed (without calling resume)
> ??

yes. this is the case.

>
> If so, it's rather an inconsistent PCM state in the core side, and can
> be fixed by a simple call like below:
>
> -- 8< --
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2700,6 +2700,7 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream)
>         if (substream->ref_count > 0)
>                 return;
>
> +       snd_pcm_resume(substream);
>         snd_pcm_drop(substream);
>         if (substream->hw_opened) {
>                 if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
> -- 8< --
>
> This will be no-op for the PCM device without SNDRV_PCM_INFO_RESUME.
>
> But, this may need more rework, too; admittedly it imposes the
> unnecessary resume of the stream although it shall be stopped and
> closed immediately after that.  We may have some optimization in
> addition.

The suspended_state is not cleared that the resume may be called again
at the end of stream.

Will you push the code?

Best regards
Shengjiu Wang

>
>
> thanks,
>
> Takashi