Re: [RFT PATCH] xhci: fix frame id calculation for isoc transfer

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

On 5/12/26 12:08, Michal Pecio wrote:
> On Mon, 11 May 2026 22:21:05 +0300, Mathias Nyman wrote:
>> Always calculate estimated start frame, and set urb->start_frame
> 
> This may cause driver developers lots of grey hair when running on old
> (non-CFC, xHCI 1.0, USB 3.0, "gen 1") HCs where nothing is done to
> enforce frame number correctness so far.
> 
> Writing obviously bogus urb->start_frame at least was obvious :)

v2 writes the TRB frame id field of the first TD of the first URB
for older non-CFC hosts as well. So urb->start_frame should be
quite accurate. Its probably more off if URB_ISO_ASAP is set.

> 
>> Fix valid frame window start and end calculation to match xhci
>> spec 4.11.2.5
>>
>> Don't inclease frame id with 1 if a URB mod transfer is queued late.
>> Queue it with next expected frame ID but print a message if URB is
>> next expected frame id does not fir valid frame window range (URB mid
>> transfer is queued late)
> 
> Failing to meet the IST is not the same as violating the 895ms limit.
> The former may (or may not) cause the TD to complete with MSE instead
> of Success in its interval. The latter is apparently supposed to cause
> MSE completion when the HC reads the TD, even though the IST was met.
> 
> A particularly fascinating case is period > 895ms; it seems that xHCI
> only allows submitting shortly before interval boundary, or "SIA".
> 
> Another nice one is ESIT >= 4096ms - exceeding MFINDEX limit.
> 
> That being said, I found that validating end_frame_id before enqueuing
> doesn't take more effort than generating these warnings, and I think it
> would be more productive - drivers know that their submission is lost
> cause, instead of getting bizarre completions later.

Added last frame of urb < +895ms to v2

But only for urbs mid stream, and only print a message if check fails.
queue it anyways

> 
> As for ignoring the IST, my ASMedia chips seem to take it well. And
> they specify absurdly long IST of 2ms, while actually running with
> 3x0.5ms URBs or less. The "less" tends to be glitchy, but I'm not yet
> sure if it's xHCI problem or snd-usb-audio / other software problem.

I think it makes sense to only enforce IST for the very first URB when
starting the endpoint.

Otherwise just print a debug message so that class driver developers
can notice they use more URBs, queue them sooner, or both.

My v2 is a bit cleaner, fixed a missing "% MAX_UFRAME", and is split
into 2 patches.
Can be found in my fix_frame_id branch:

git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git fix_frame_id
https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=fix_frame_id

-Mathias