Re: [PATCH] usb: xhci: Fix isochronous scheduling regression

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 8/24/26 18:51, Alan Stern wrote:
> On Mon, Aug 24, 2026 at 06:21:22PM +0300, Mathias Nyman wrote:
>> On 8/22/26 05:38, Alan Stern wrote:
>>> On Fri, Aug 21, 2026 at 06:03:50PM +0200, Michal Pecio wrote:
>>>> On Fri, 21 Aug 2026 10:44:07 -0400, Alan Stern wrote:
> 
>>> Possible alternative: Make the URB_ISO_ASAP flag take precedence over
>>> the "queue is non-empty" condition.
>>
>> xhci driver does this. If URB_ISO_ASAP is set then xhci driver always sets
>> the SIA "Start Isoch ASAP" flag for the transfer blocks.
> 
> I wasn't very precise before.  I meant URB_ISO_ASAP should take
> precedence when there are no active URBs but there may still be some
> URBs being given back.  In other words, when the list_empty test
> succeeds.  If the list of queued URBs is not empty then new URBs should
> always be assigned to the next available slot -- unless we decide to
> support a new URB_USE_FRAME flag and the flag is set.
> 

URB_ISO_ASAP always takes precedence in xHC case.

xHC controller has a "SIA" flag that we set for each TD (URB frame)
in the URB when USR_ISO_ASAP is set.
If there are no active URBs mid stream, and ring underruns, then xHC will process
the next TD with SIA flag at its earliest possible slot.
So URBs with URB_ISO_ASAP flag will be out of sync and laggy, but not lose data
in underrun cases.

A frame is only dropped in SIA case if xHC fails for internal reasons to
process that transfer in time. This triggers a Missed Service Error, and xHC
moves to process the next TD (frame).

> The point here is that if the class driver wants to change the alignment
> between URBs and uframes, without worrying about the BH giveback race,
> all it has to do is set URB_ISO_ASAP.  If it doesn't care about the
> alignment (implying that it also doesn't care if some URBs are assigned
> to expired slots) then the race doesn't matter.
> 
> And of course, if the class driver wants to maintain the alignment then
> it should resubmit URBs from the completion handler, so that the queue
> doesn't empty out unless there is an underrun.  Then the BH giveback
> race won't be an issue.
> 
>> The urb->start_frame value set by xhci driver might not be correct in this
>> case.
> 
> Do you mean it might be wrong because xhci-hcd can't tell what uframe
> the xHC hardware actually selects?

xhci driver currently guesses the urb->start_frame based on current harware frame
number + a scheduling threshold. This is small inaccuracy in the start.
Not the real issue.

After this the xhci driver start_frame is just a running number during the stream,
incremented on every queued frame. urb->start_frame mid stream is based on this.

If URB_ISO_ASAP (SIA in xhci) is set then we know that every URB enqueued after a
underrun event will be delayed by at least one frame. In underrun cases where there
aren't any URBs queued yet we should set base the next urb->start_frame on hardware
frame id + scheduling threshold instead of the running number.

This is on my todo list.

Class driver can then detect lagging out of sync URB_ISO_ASAP transfers based on
the unexpected large gaps between urb->start_frame numbers

Thanks
Mathias