From: Michal Pecio <michal.pecio@gmail.com>
To: Mathias Nyman <mathias.nyman@linux.intel.com>
Cc: dylan_robinson@motu.com, linux-usb@vger.kernel.org,
mathias.nyman@intel.com, stern@rowland.harvard.edu
Subject: Re: [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs
Date: Thu, 13 Aug 2026 00:56:35 +0200 [thread overview]
Message-ID: <20260813005635.34750f8c.michal.pecio@gmail.com> (raw)
In-Reply-To: <310ccfe5-0212-4c8b-9213-891a7340e2f4@linux.intel.com>
On Fri, 7 Aug 2026 14:10:35 +0300, Mathias Nyman wrote:
> I'm doing exactly what kerneldoc and comments in bugzilla state.
> If queue empties out we schedule the next TD based on URB_ISO_ASAP
> flag.
> > "If the queue is idle" means all completions have already returned.
>
> That's an odd interpretation of "queue is idle"
>
> This would mean that an URB queued after "queue runs out" (underrun)
> should always be treated as USB_ISO_ASAP case, making the flag
> useless.
Well, there are HW queues and SW queues, and HW and SW underruns.
If the HW queue underruns but the SW queue still has pending URBs
(not completed yet) then we do care about URB_ISO_ASAP. If the SW
queue is completely empty (all completed), we aren't expected to.
This does make a difference with snd-usb-audio. If I run
jackd -d alsa -d hw:... -p 24 -n 2
Ring Underrun and Missed Service are reported every now and then,
sometimes repeatedly, and it doesn't take long to enter this loop:
1. playback Ring Underrun (xHCI EP state is Running)
2. capture URB unlink (xHCI EP state is Stopped)
3. multiple capture URBs scheduled to the same start_frame before
EP state becomes Running
4. new playback URB is scheduled into a distant past due Running EP
5. 2 uframes later OUT endpoint reports Missed Service for all
those misscheduled TDs
6. goto 1
Changing the condition from "running endpoint" back to "empty list"
breaks this pathological cycle. Experimental patch below.
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1ff84ab9955a..8e380d1c066e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -4303,6 +4303,12 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci, gfp_t mem_flags,
return ret;
}
+static int _ep(int ep_index)
+{
+ ep_index++;
+ return ep_index / 2 + 0x80 * (ep_index & 1);
+}
+
/*
* Check transfer ring to guarantee there is enough room for the urb.
* Update ISO URB start_frame and interval.
@@ -4348,7 +4354,22 @@ int xhci_queue_isoc_tx_prepare(struct xhci_hcd *xhci, gfp_t mem_flags,
* Check if this starts the isoc data flow. Relies on hw setting ep ctx
* state after doorbell ring. Consider adding list_empty(td_list) check
*/
- if (GET_EP_CTX_STATE(ep_ctx) != EP_STATE_RUNNING)
+ bool running = GET_EP_CTX_STATE(ep_ctx) == EP_STATE_RUNNING;
+
+ bool pending = !list_empty(&ep_ring->td_list) ||
+ hcd_periodic_completion_in_progress(xhci_to_hcd(xhci), urb->ep);
+
+ if (running && !pending)
+ xhci_info(xhci, "ep %.2x running without pending URBs\n", _ep(ep_index));
+ if (pending && !running)
+ xhci_info(xhci, "ep %.2x pending URBs but not running\n", _ep(ep_index));
+
+ //if (!running)
+ if (!pending)
xep->next_uframe = -1;
return xhci_queue_isoc_tx(xhci, mem_flags, urb, slot_id, ep_index);
next prev parent reply other threads:[~2026-08-12 22:51 UTC|newest]
Thread overview: 76+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-04 21:25 [Bug 220748] New: usb: xhci_queue_isoc_tx_prepare ignore start_frame and always assumes URB_ISO_ASAP is set bugzilla-daemon
2025-11-05 8:40 ` [Bug 220748] " bugzilla-daemon
2025-11-05 8:48 ` bugzilla-daemon
2025-11-05 8:53 ` bugzilla-daemon
2025-11-05 9:06 ` bugzilla-daemon
2025-11-05 15:47 ` bugzilla-daemon
2025-11-05 18:28 ` bugzilla-daemon
2025-11-05 18:30 ` bugzilla-daemon
2025-11-05 20:16 ` bugzilla-daemon
2025-11-05 21:18 ` bugzilla-daemon
2025-11-06 3:52 ` bugzilla-daemon
2025-11-06 8:27 ` bugzilla-daemon
2025-11-06 8:35 ` bugzilla-daemon
2025-11-06 15:03 ` bugzilla-daemon
2025-11-08 10:33 ` bugzilla-daemon
2025-11-08 16:09 ` bugzilla-daemon
2025-11-10 10:23 ` bugzilla-daemon
2025-11-10 10:42 ` bugzilla-daemon
2026-05-04 23:53 ` bugzilla-daemon
2026-05-04 23:54 ` bugzilla-daemon
2026-05-05 1:14 ` bugzilla-daemon
2026-05-05 9:59 ` bugzilla-daemon
2026-05-05 17:09 ` bugzilla-daemon
2026-05-05 17:10 ` bugzilla-daemon
2026-05-05 17:10 ` bugzilla-daemon
2026-05-05 17:13 ` bugzilla-daemon
2026-05-06 13:32 ` bugzilla-daemon
2026-05-06 15:03 ` bugzilla-daemon
2026-05-07 2:38 ` Alan Stern
2026-05-07 16:17 ` Dylan Robinson
2026-05-07 17:24 ` Alan Stern
2026-05-07 21:16 ` Dylan Robinson
2026-05-08 3:02 ` Alan Stern
2026-05-08 17:20 ` Dylan Robinson
2026-05-09 1:25 ` Alan Stern
2026-05-09 22:12 ` Michal Pecio
2026-05-10 12:39 ` Dylan Robinson
2026-05-11 19:21 ` [RFT PATCH] xhci: fix frame id calculation for isoc transfer Mathias Nyman
2026-05-11 19:36 ` Mathias Nyman
2026-05-12 9:08 ` Michal Pecio
2026-05-13 14:30 ` Mathias Nyman
2026-05-13 14:35 ` [RFT PATCHv2 1/2] xhci: fix frame id calculation and checks for isoc URBs Mathias Nyman
2026-05-13 14:35 ` [RFT PATCHv2 2/2] xhci: Set frame ID field of isoc TRB when starting an isoch stream Mathias Nyman
2026-05-14 21:16 ` [RFT PATCH] xhci: fix frame id calculation for isoc transfer Dylan Robinson
2026-05-14 22:10 ` Dylan Robinson
2026-05-15 4:32 ` Michal Pecio
2026-05-15 18:13 ` Dylan Robinson
2026-05-18 7:13 ` Michal Pecio
2026-05-21 14:20 ` Dylan Robinson
2026-05-21 15:24 ` Mathias Nyman
2026-05-21 15:27 ` [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs Mathias Nyman
2026-05-21 15:27 ` [RFT PATCHv3 2/3] xhci: Set frame ID field of isoc TRB when starting an isoch stream Mathias Nyman
2026-05-21 15:27 ` [RFT PATCHv3 3/3] xhci: tune urb->start_frame in ring overrun and underrun cases Mathias Nyman
2026-05-26 0:49 ` Dylan Robinson
2026-05-26 12:15 ` Michal Pecio
2026-05-27 20:20 ` Dylan Robinson
2026-08-06 8:17 ` [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs Michal Pecio
2026-08-06 13:25 ` Mathias Nyman
2026-08-06 22:01 ` Michal Pecio
2026-08-07 0:07 ` Mathias Nyman
2026-08-07 8:26 ` Michal Pecio
2026-08-07 11:10 ` Mathias Nyman
2026-08-07 16:48 ` Alan Stern
2026-08-07 17:30 ` Dylan Robinson
2026-08-07 21:29 ` Michal Pecio
2026-08-10 14:12 ` Mathias Nyman
2026-08-11 22:11 ` Dylan Robinson
2026-08-12 22:56 ` Michal Pecio [this message]
2026-05-07 21:54 ` [Bug 220748] usb: xhci_queue_isoc_tx_prepare ignore start_frame and always assumes URB_ISO_ASAP is set Michal Pecio
2026-05-08 3:09 ` Alan Stern
2026-05-08 9:41 ` Michal Pecio
2026-05-08 14:54 ` Alan Stern
2026-05-08 21:39 ` Dylan Robinson
2026-05-09 11:10 ` Michal Pecio
2026-05-09 20:18 ` Dylan Robinson
2026-05-11 19:15 ` bugzilla-daemon
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=20260813005635.34750f8c.michal.pecio@gmail.com \
--to=michal.pecio@gmail.com \
--cc=dylan_robinson@motu.com \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@intel.com \
--cc=mathias.nyman@linux.intel.com \
--cc=stern@rowland.harvard.edu \
/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;
as well as URLs for NNTP newsgroup(s).