Re: [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs

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);