From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-15.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ADD0BC07E95 for ; Wed, 7 Jul 2021 11:39:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8418E6128D for ; Wed, 7 Jul 2021 11:39:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231391AbhGGLlk (ORCPT ); Wed, 7 Jul 2021 07:41:40 -0400 Received: from smtp-out1.suse.de ([195.135.220.28]:59430 "EHLO smtp-out1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231358AbhGGLlj (ORCPT ); Wed, 7 Jul 2021 07:41:39 -0400 Received: from relay2.suse.de (relay2.suse.de [149.44.160.134]) by smtp-out1.suse.de (Postfix) with ESMTP id 0685222058; Wed, 7 Jul 2021 11:38:59 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1625657939; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=izSVaruZZK1uuDh40h+Wau6dNLjAVEv1QpDRBpfpZsc=; b=SZBk6UtMcM26ce3cTDoruC+UIpTuNcjFwYV0bqn0jw5j44VeEod9iDt+EFJ1kCsogdV/Fr kD/MreAz974RrgC2TVT6ngdbh3GE7xUtJpI2Oth9MIIVbEVysg5uDgwMEcJWQ1sesJCupN 3QyRfB4RIL1FdyB1KUH3NWKiUE0vTg0= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1625657939; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=izSVaruZZK1uuDh40h+Wau6dNLjAVEv1QpDRBpfpZsc=; b=qw6vy09zlRBA+OKwTwQYSQIJTcNhAF2Z/k0tzUEuOV57YCE9N7TwORASZ1x7+N2S9iDKnH zKsoFwiuHCets9Aw== Received: from alsa1.suse.de (alsa1.suse.de [10.160.4.42]) by relay2.suse.de (Postfix) with ESMTP id E4EC0A3BA4; Wed, 7 Jul 2021 11:38:58 +0000 (UTC) Date: Wed, 07 Jul 2021 13:38:58 +0200 Message-ID: From: Takashi Iwai To: Robert Lee Cc: vkoul@kernel.org, perex@perex.cz, tiwai@suse.com, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, carterhsu@google.com, zxinhui@google.com, bubblefang@google.com Subject: Re: [PATCH] ALSA: compress: allow to leave draining state when pausing in draining In-Reply-To: <20210706124440.3247283-1-lerobert@google.com> References: <20210706124440.3247283-1-lerobert@google.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/25.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 >