linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ALSA: compress: allow to leave draining state when pausing in draining
@ 2021-07-06 12:44 Robert Lee
  2021-07-07 11:38 ` Takashi Iwai
  0 siblings, 1 reply; 3+ messages in thread
From: Robert Lee @ 2021-07-06 12:44 UTC (permalink / raw)
  To: vkoul, perex, tiwai
  Cc: alsa-devel, linux-kernel, carterhsu, zxinhui, bubblefang, Robert Lee

When compress offload pauses in draining state, not all platforms
need to keep in draining state. Some platforms may call drain or
partial drain again when resume from pause in draining, so it needs
to wake up from snd_compress_wait_for_drain() in this case.

Call API snd_compr_leave_draining_in_pause(), if the platform
doesn't need to keep in draining state when pause in draining
state.

Signed-off-by: Robert Lee <lerobert@google.com>
---
 include/sound/compress_driver.h | 14 ++++++++++++++
 sound/core/compress_offload.c   |  7 ++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
index 277087f635f3..e16524a93a14 100644
--- a/include/sound/compress_driver.h
+++ b/include/sound/compress_driver.h
@@ -145,6 +145,7 @@ struct snd_compr_ops {
  * @lock: device lock
  * @device: device id
  * @use_pause_in_draining: allow pause in draining, true when set
+ * @leave_draining_in_pause: leave draining state when pausing in draining
  */
 struct snd_compr {
 	const char *name;
@@ -156,6 +157,7 @@ struct snd_compr {
 	struct mutex lock;
 	int device;
 	bool use_pause_in_draining;
+	bool leave_draining_in_pause;
 #ifdef CONFIG_SND_VERBOSE_PROCFS
 	/* private: */
 	char id[64];
@@ -182,6 +184,18 @@ static inline void snd_compr_use_pause_in_draining(struct snd_compr_stream *subs
 	substream->device->use_pause_in_draining = true;
 }
 
+/**
+ * snd_compr_leave_draining_in_pause - Leave draining state when pause in draining
+ * @substream: compress substream to set
+ *
+ * In some platform, we need to leave draining state when we use pause in draining.
+ * Add API to allow leave draining state.
+ */
+static inline void snd_compr_leave_draining_in_pause(struct snd_compr_stream *substream)
+{
+	substream->device->leave_draining_in_pause = true;
+}
+
 /* dsp driver callback apis
  * For playback: driver should call snd_compress_fragment_elapsed() to let the
  * framework know that a fragment has been consumed from the ring buffer
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index 21ce4c056a92..9c7bd4db6ecd 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -719,8 +719,13 @@ static int snd_compr_pause(struct snd_compr_stream *stream)
 		if (!stream->device->use_pause_in_draining)
 			return -EPERM;
 		retval = stream->ops->trigger(stream, SNDRV_PCM_TRIGGER_PAUSE_PUSH);
-		if (!retval)
+		if (!retval) {
 			stream->pause_in_draining = true;
+			if (stream->device->leave_draining_in_pause) {
+				stream->runtime->state = SNDRV_PCM_STATE_PAUSED;
+				wake_up(&stream->runtime->sleep);
+			}
+		}
 		break;
 	default:
 		return -EPERM;
-- 
2.32.0.93.g670b81a890-goog


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: compress: allow to leave draining state when pausing in draining
  2021-07-06 12:44 [PATCH] ALSA: compress: allow to leave draining state when pausing in draining Robert Lee
@ 2021-07-07 11:38 ` Takashi Iwai
  2021-07-08  2:22   ` Robert Lee
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2021-07-07 11:38 UTC (permalink / raw)
  To: Robert Lee
  Cc: vkoul, perex, tiwai, alsa-devel, linux-kernel, carterhsu,
	zxinhui, bubblefang

On Tue, 06 Jul 2021 14:44:40 +0200,
Robert Lee wrote:
> 
> When compress offload pauses in draining state, not all platforms
> need to keep in draining state. Some platforms may call drain or
> partial drain again when resume from pause in draining, so it needs
> to wake up from snd_compress_wait_for_drain() in this case.
> 
> Call API snd_compr_leave_draining_in_pause(), if the platform
> doesn't need to keep in draining state when pause in draining
> state.
> 
> Signed-off-by: Robert Lee <lerobert@google.com>

Well, the logic is a bit confusing (hard to understand what really
"leave-draining-in-pause" actually means) but also error-prone;
e.g. you left pause_in_draining flag set while changing the state to
SNDRV_PCM_STATE_PAUSED.  This will keep the pause_in_draining flag
even after snd_compr_resume() call.


thanks,

Takashi

> ---
>  include/sound/compress_driver.h | 14 ++++++++++++++
>  sound/core/compress_offload.c   |  7 ++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> index 277087f635f3..e16524a93a14 100644
> --- a/include/sound/compress_driver.h
> +++ b/include/sound/compress_driver.h
> @@ -145,6 +145,7 @@ struct snd_compr_ops {
>   * @lock: device lock
>   * @device: device id
>   * @use_pause_in_draining: allow pause in draining, true when set
> + * @leave_draining_in_pause: leave draining state when pausing in draining
>   */
>  struct snd_compr {
>  	const char *name;
> @@ -156,6 +157,7 @@ struct snd_compr {
>  	struct mutex lock;
>  	int device;
>  	bool use_pause_in_draining;
> +	bool leave_draining_in_pause;
>  #ifdef CONFIG_SND_VERBOSE_PROCFS
>  	/* private: */
>  	char id[64];
> @@ -182,6 +184,18 @@ static inline void snd_compr_use_pause_in_draining(struct snd_compr_stream *subs
>  	substream->device->use_pause_in_draining = true;
>  }
>  
> +/**
> + * snd_compr_leave_draining_in_pause - Leave draining state when pause in draining
> + * @substream: compress substream to set
> + *
> + * In some platform, we need to leave draining state when we use pause in draining.
> + * Add API to allow leave draining state.
> + */
> +static inline void snd_compr_leave_draining_in_pause(struct snd_compr_stream *substream)
> +{
> +	substream->device->leave_draining_in_pause = true;
> +}
> +
>  /* dsp driver callback apis
>   * For playback: driver should call snd_compress_fragment_elapsed() to let the
>   * framework know that a fragment has been consumed from the ring buffer
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 21ce4c056a92..9c7bd4db6ecd 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -719,8 +719,13 @@ static int snd_compr_pause(struct snd_compr_stream *stream)
>  		if (!stream->device->use_pause_in_draining)
>  			return -EPERM;
>  		retval = stream->ops->trigger(stream, SNDRV_PCM_TRIGGER_PAUSE_PUSH);
> -		if (!retval)
> +		if (!retval) {
>  			stream->pause_in_draining = true;
> +			if (stream->device->leave_draining_in_pause) {
> +				stream->runtime->state = SNDRV_PCM_STATE_PAUSED;
> +				wake_up(&stream->runtime->sleep);
> +			}
> +		}
>  		break;
>  	default:
>  		return -EPERM;
> -- 
> 2.32.0.93.g670b81a890-goog
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] ALSA: compress: allow to leave draining state when pausing in draining
  2021-07-07 11:38 ` Takashi Iwai
@ 2021-07-08  2:22   ` Robert Lee
  0 siblings, 0 replies; 3+ messages in thread
From: Robert Lee @ 2021-07-08  2:22 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: vkoul, perex, tiwai, alsa-devel, linux-kernel, carterhsu,
	zxinhui, bubblefang

Hi Takashi,
Nice catch. I make a change and mail patch v2 for reviewing.

thanks,
Robert.


Takashi Iwai <tiwai@suse.de> 於 2021年7月7日 週三 下午7:38寫道：
>
> On Tue, 06 Jul 2021 14:44:40 +0200,
> Robert Lee wrote:
> >
> > When compress offload pauses in draining state, not all platforms
> > need to keep in draining state. Some platforms may call drain or
> > partial drain again when resume from pause in draining, so it needs
> > to wake up from snd_compress_wait_for_drain() in this case.
> >
> > Call API snd_compr_leave_draining_in_pause(), if the platform
> > doesn't need to keep in draining state when pause in draining
> > state.
> >
> > Signed-off-by: Robert Lee <lerobert@google.com>
>
> Well, the logic is a bit confusing (hard to understand what really
> "leave-draining-in-pause" actually means) but also error-prone;
> e.g. you left pause_in_draining flag set while changing the state to
> SNDRV_PCM_STATE_PAUSED.  This will keep the pause_in_draining flag
> even after snd_compr_resume() call.
>
>
> thanks,
>
> Takashi
>
> > ---
> >  include/sound/compress_driver.h | 14 ++++++++++++++
> >  sound/core/compress_offload.c   |  7 ++++++-
> >  2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> > index 277087f635f3..e16524a93a14 100644
> > --- a/include/sound/compress_driver.h
> > +++ b/include/sound/compress_driver.h
> > @@ -145,6 +145,7 @@ struct snd_compr_ops {
> >   * @lock: device lock
> >   * @device: device id
> >   * @use_pause_in_draining: allow pause in draining, true when set
> > + * @leave_draining_in_pause: leave draining state when pausing in draining
> >   */
> >  struct snd_compr {
> >       const char *name;
> > @@ -156,6 +157,7 @@ struct snd_compr {
> >       struct mutex lock;
> >       int device;
> >       bool use_pause_in_draining;
> > +     bool leave_draining_in_pause;
> >  #ifdef CONFIG_SND_VERBOSE_PROCFS
> >       /* private: */
> >       char id[64];
> > @@ -182,6 +184,18 @@ static inline void snd_compr_use_pause_in_draining(struct snd_compr_stream *subs
> >       substream->device->use_pause_in_draining = true;
> >  }
> >
> > +/**
> > + * snd_compr_leave_draining_in_pause - Leave draining state when pause in draining
> > + * @substream: compress substream to set
> > + *
> > + * In some platform, we need to leave draining state when we use pause in draining.
> > + * Add API to allow leave draining state.
> > + */
> > +static inline void snd_compr_leave_draining_in_pause(struct snd_compr_stream *substream)
> > +{
> > +     substream->device->leave_draining_in_pause = true;
> > +}
> > +
> >  /* dsp driver callback apis
> >   * For playback: driver should call snd_compress_fragment_elapsed() to let the
> >   * framework know that a fragment has been consumed from the ring buffer
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index 21ce4c056a92..9c7bd4db6ecd 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -719,8 +719,13 @@ static int snd_compr_pause(struct snd_compr_stream *stream)
> >               if (!stream->device->use_pause_in_draining)
> >                       return -EPERM;
> >               retval = stream->ops->trigger(stream, SNDRV_PCM_TRIGGER_PAUSE_PUSH);
> > -             if (!retval)
> > +             if (!retval) {
> >                       stream->pause_in_draining = true;
> > +                     if (stream->device->leave_draining_in_pause) {
> > +                             stream->runtime->state = SNDRV_PCM_STATE_PAUSED;
> > +                             wake_up(&stream->runtime->sleep);
> > +                     }
> > +             }
> >               break;
> >       default:
> >               return -EPERM;
> > --
> > 2.32.0.93.g670b81a890-goog
> >

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-08  2:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-06 12:44 [PATCH] ALSA: compress: allow to leave draining state when pausing in draining Robert Lee
2021-07-07 11:38 ` Takashi Iwai
2021-07-08  2:22   ` Robert Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).