From: Takashi Iwai <tiwai@suse.de>
To: "Gordon Chen" <chengordon326@gmail.com>
Cc: "Zhang Heng" <zhangheng@kylinos.cn>, <perex@perex.cz>,
<tiwai@suse.com>, <kees@kernel.org>, <jussi@sonarnerd.net>,
<hulianqin@vivo.com>, <i@rong.moe>, <g@b4.vu>,
<cryolitia@uniontech.com>, <pav@iki.fi>,
<linux-sound@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] usb-audio: Fix boot-time crackling for Generic USB Audio device
Date: Mon, 13 Jul 2026 17:05:40 +0200 [thread overview]
Message-ID: <87zezvx73v.wl-tiwai@suse.de> (raw)
In-Reply-To: <18c1dc0246121271.5ea4dc9a201e9dce.cafe1c196d712416@GordonMsi>
On Mon, 13 Jul 2026 15:20:29 +0200,
Gordon Chen wrote:
>
> On Mon, Jul 13, 2026 at 04:10:48PM +0800, Zhang Heng wrote:
> > + /* Generic USB Audio (0x1e0b:d01e): use SIA for consistent scheduling */
> > + if (ep->chip->usb_id == USB_ID(0x1e0b, 0xd01e))
> > + u->urb->transfer_flags |= URB_ISO_ASAP;
>
> Not a maintainer, just a bystander who was Cc'd -- two small notes on the
> form of the patch, plus one question I can't answer myself.
>
> A per-device usb_id comparison in the endpoint.c fast path seems like the
> kind of thing the quirk_flags_table in quirks.c exists to avoid. Would a
> QUIRK_FLAG_ISO_ASAP (set in the table, tested here as
> chip->quirk_flags & QUIRK_FLAG_ISO_ASAP) work for you? It keeps endpoint.c
> device-agnostic, and the next device with the same symptom becomes a
> one-line table entry rather than another if.
>
> > + if (ep->chip->usb_id == USB_ID(0x1e0b, 0xd01e) &&
> > + ep->type == SND_USB_ENDPOINT_TYPE_SYNC)
> > + ep->skip_packets = 4;
>
> The block immediately above this one already does exactly
> "type == SND_USB_ENDPOINT_TYPE_SYNC -> skip_packets = 4"; adding the ID to
> that condition would avoid the duplicate if. Also, the changelog doesn't
> say what this hunk contributes on its own -- is URB_ISO_ASAP alone
> insufficient, and if so, what does skipping the first 4 sync packets fix
> that ASAP doesn't? Right now the two changes are indistinguishable in the
> commit message, and skip_packets = 4 reads as belt-and-braces.
>
> And the question. If xhci_get_isoc_frame_id() really does compute a
> stale Frame ID for TDs at index > 0 -- because the frame has advanced by
> the time validation runs -- then isn't that generic to any isoc URB with
> number_of_packets > 1, rather than specific to 1e0b:d01e? The index=0 TD
> gets clamped back into range, later TDs don't, and the endpoint ends up
> mixing explicit Frame IDs with SIA. Nothing in that description looks
> device-specific to me. If it is in fact generic, a per-device quirk in
> sound/usb papers over it for one device while every other multi-TD isoc
> consumer keeps hitting it -- and the fix would belong in xhci-ring.c.
>
> I'm not familiar enough with the xHCI scheduling side to say whether
> that's right, so it may be worth Cc'ing Mathias Nyman
> <mathias.nyman@linux.intel.com> to get a verdict before this is settled as
> a usb-audio quirk. If your device is the only one that turns the mixed
> scheduling into audible corruption, then the quirk is arguably the right
> scope after all -- but that reasoning should be in the changelog.
>
> One data point that cuts against my own question above: on an AMD xHCI
> here, streaming to a class-compliant USB audio device, I see no "beyond
> range" / "Ignore frame ID field" messages at all. If the index > 0
> staleness were unconditional I would expect to hit it too. So something
> about your device's packet layout or your host's timing is presumably
> what actually trips it -- which would make the device-specific scope
> defensible, but that is exactly the reasoning I would want to see spelled
> out in the changelog.
Agreed on all points!
Especially the workaround with URB_ISO_ASAP should be applied in a
more generic way.
thanks,
Takashi
next prev parent reply other threads:[~2026-07-13 15:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-13 8:10 [PATCH] usb-audio: Fix boot-time crackling for Generic USB Audio device Zhang Heng
2026-07-13 13:20 ` Gordon Chen
2026-07-13 15:05 ` Takashi Iwai [this message]
2026-07-14 13:09 ` Zhang Heng
2026-07-14 1:58 ` Zhang Heng
2026-07-14 7:40 ` Takashi Iwai
2026-07-14 13:02 ` Zhang Heng
2026-07-14 13:38 ` Takashi Iwai
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=87zezvx73v.wl-tiwai@suse.de \
--to=tiwai@suse.de \
--cc=chengordon326@gmail.com \
--cc=cryolitia@uniontech.com \
--cc=g@b4.vu \
--cc=hulianqin@vivo.com \
--cc=i@rong.moe \
--cc=jussi@sonarnerd.net \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=pav@iki.fi \
--cc=perex@perex.cz \
--cc=tiwai@suse.com \
--cc=zhangheng@kylinos.cn \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.