mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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
Date: Fri, 19 Nov 2021 10:13:54 +0100	[thread overview]
Message-ID: <s5hwnl4fqy5.wl-tiwai@suse.de> (raw)
In-Reply-To: <20211118232628.hynb5vy6phsqj5oi@box.shutemov.name>

[-- Attachment #1: Type: text/plain, Size: 661 bytes --]

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


[-- Attachment #2: 0001-ALSA-usb-audio-Switch-back-to-non-latency-mode-at-a-.patch --]
[-- Type: application/octet-stream, Size: 2451 bytes --]

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


[-- Attachment #3: 0002-ALSA-usb-audio-Don-t-start-stream-for-capture-at-pre.patch --]
[-- Type: application/octet-stream, Size: 1075 bytes --]

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


  reply	other threads:[~2021-11-19  9:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-03 10:24 Takashi Iwai
2021-11-03 15:04 ` pr-tracker-bot
2021-11-17 16:18 ` Kirill A. Shutemov
2021-11-17 16:24   ` Takashi Iwai
2021-11-17 17:48     ` Kirill A. Shutemov
2021-11-17 18:02       ` Takashi Iwai
2021-11-17 18:20         ` Kirill A. Shutemov
2021-11-18  8:01         ` Takashi Iwai
2021-11-18 16:57           ` Takashi Iwai
2021-11-18 23:26             ` Kirill A. Shutemov
2021-11-19  9:13               ` Takashi Iwai [this message]
2021-11-19 10:09                 ` Kirill A. Shutemov
2021-11-19 10:24                   ` Takashi Iwai
2021-11-18 23:17           ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hwnl4fqy5.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=broonie@kernel.org \
    --cc=kirill@shutemov.name \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®