Date:   Fri, 19 Nov 2021 10:13:54 +0100
Message-ID: <s5hwnl4fqy5.wl-tiwai@suse.de>
From:   Takashi Iwai <tiwai@suse.de>
To:     "Kirill A. Shutemov" <kirill@shutemov.name>
Cc:     Takashi Iwai <tiwai@suse.de>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Mark Brown <broonie@kernel.org>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [GIT PULL] sound updates for 5.16-rc1
In-Reply-To: <20211118232628.hynb5vy6phsqj5oi@box.shutemov.name>
References: <s5hwnlpmt9x.wl-tiwai@suse.de>
        <20211117161855.m45mxcqszkfcetai@box.shutemov.name>
        <s5hmtm2lphf.wl-tiwai@suse.de>
        <20211117174826.23eakoivl33tawb2@box.shutemov.name>
        <s5hfsrulkxk.wl-tiwai@suse.de>
        <s5hpmqxki3h.wl-tiwai@suse.de>
        <s5hilwph05c.wl-tiwai@suse.de>
        <20211118232628.hynb5vy6phsqj5oi@box.shutemov.name>
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: multipart/mixed;
 boundary="Multipart_Fri_Nov_19_10:13:54_2021-1"
Precedence: bulk

--Multipart_Fri_Nov_19_10:13:54_2021-1
Content-Type: text/plain; charset=US-ASCII

On Fri, 19 Nov 2021 00:26:28 +0100,
Kirill A. Shutemov wrote:
> 
> On Thu, Nov 18, 2021 at 05:57:35PM +0100, Takashi Iwai wrote:
> > And it'd be helpful if you can gather the log (from tracing) with the
> > patch below (from the start of speaker-test until the stall).
> 
> The trace is attached.

Thanks!  From the previous log, I noticed that you're using dmix
plugin, and that was the problem.  The dmix plugin changes the PCM
sw_params parameters at a late stage, and this confused the driver as
if it were OK with the low-latency mode.  And, while reviewing the
code, I also found another bug.

Below are two fix patches.  Please give it a try.


Takashi


--Multipart_Fri_Nov_19_10:13:54_2021-1
Content-Type: application/octet-stream; type=patch
Content-Disposition: attachment; filename="0001-ALSA-usb-audio-Switch-back-to-non-latency-mode-at-a-.patch"
Content-Transfer-Encoding: 7bit

From 4b9f4f8e236ef3b40ca4f3e0e59f6e094d090d59 Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Fri, 19 Nov 2021 10:04:49 +0100
Subject: [PATCH 1/2] ALSA: usb-audio: Switch back to non-latency mode at a
 later point

The recent regression report revealed that the judgment of the
low-latency playback mode based on the runtime->stop_threshold cannot
work reliably at the prepare stage, as sw_params call may happen at
any time, and PCM dmix actually sets it up after the prepare call.
This ended up with the stall of the stream as PCM ack won't be issued
at all.

For addressing this, check the free-wheeling mode again at the PCM
trigger right before starting the stream again, and allow switching to
the non-LL mode at a late stage.

Fixes: d5f871f89e21 ("ALSA: usb-audio: Improved lowlatency playback support")
Reported-by: Kirill A. Shutemov <kirill@shutemov.name>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/pcm.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 95ec8eec1bb0..57b046e73bfe 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -581,6 +581,12 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 	return 0;
 }
 
+/* free-wheeling mode? (e.g. dmix) */
+static int in_free_wheeling_mode(struct snd_pcm_runtime *runtime)
+{
+	return runtime->stop_threshold > runtime->buffer_size;
+}
+
 /* check whether early start is needed for playback stream */
 static int lowlatency_playback_available(struct snd_pcm_runtime *runtime,
 					 struct snd_usb_substream *subs)
@@ -592,8 +598,7 @@ static int lowlatency_playback_available(struct snd_pcm_runtime *runtime,
 	/* disabled via module option? */
 	if (!chip->lowlatency)
 		return false;
-	/* free-wheeling mode? (e.g. dmix) */
-	if (runtime->stop_threshold > runtime->buffer_size)
+	if (in_free_wheeling_mode(runtime))
 		return false;
 	/* implicit feedback mode has own operation mode */
 	if (snd_usb_endpoint_implicit_feedback_sink(subs->data_endpoint))
@@ -1552,6 +1557,8 @@ static int snd_usb_substream_playback_trigger(struct snd_pcm_substream *substrea
 					      subs);
 		if (subs->lowlatency_playback &&
 		    cmd == SNDRV_PCM_TRIGGER_START) {
+			if (in_free_wheeling_mode(substream->runtime))
+				subs->lowlatency_playback = false;
 			err = start_endpoints(subs);
 			if (err < 0) {
 				snd_usb_endpoint_set_callback(subs->data_endpoint,
-- 
2.31.1


--Multipart_Fri_Nov_19_10:13:54_2021-1
Content-Type: application/octet-stream; type=patch
Content-Disposition: attachment; filename="0002-ALSA-usb-audio-Don-t-start-stream-for-capture-at-pre.patch"
Content-Transfer-Encoding: 7bit

From 2eeb527a4f3481452fd00263d8cee99d5297178a Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Fri, 19 Nov 2021 10:02:42 +0100
Subject: [PATCH 2/2] ALSA: usb-audio: Don't start stream for capture at
 prepare

The recent change made mistakenly the stream for capture started at
prepare stage.  Add the stream direction check to avoid it.

Fixes: 9c9a3b9da891 ("ALSA: usb-audio: Rename early_playback_start flag with lowlatency_playback")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/usb/pcm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 57b046e73bfe..cec6e91afea2 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -640,7 +640,8 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	runtime->delay = 0;
 
 	subs->lowlatency_playback = lowlatency_playback_available(runtime, subs);
-	if (!subs->lowlatency_playback)
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
+	    !subs->lowlatency_playback)
 		ret = start_endpoints(subs);
 
  unlock:
-- 
2.31.1


--Multipart_Fri_Nov_19_10:13:54_2021-1--