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

[Michal Pecio <michal.pecio@gmail.com>]

On Thu, 14 May 2026 17:16:35 -0400, Dylan Robinson wrote:
> On Wed, May 13, 2026 at 10:30 AM Mathias Nyman wrote:
> > 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.  
> 
> Testing this patch on a non-CFC host with a bInterval = 1 device, I
> can confirm that urb->start_frame is now accurately reported when
> URB_ISO_ASAP is not set.
> 
> When URB_ISO_ASAP is set, urb->start_frame is slightly off as
> expected. For example, I observed urb->start_frame reporting 1645.0
> (frame.uframe), while the stream actually began at 1643.2.
> 
> This seems good.

Did you try with short enough playback buffer to get occasional xHCI
ring underruns?

Is Frame ID still correct then?

> > 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  
> 
> Regarding the valid window check for mid-stream URBs, what is the
> concern with an URB extending beyond the current scheduling window?

MFINDEX overflows every 2 seconds and the HC may consider such URB
1.1s in the past and drop it with Missed Service Error.

> Since the controller only evaluates TDs as it advances through the
> transfer ring, isn't it only important that a TD's frame ID falls
> within the valid scheduling window at the time the host controller
> reaches it, rather than when the URB is submitted?

That's true, but the HC is allowed to look at the URB as soon as it
is submitted. The spec states that SW shall never do such things, so
HW is allowed to assume that SW doesn't and make the dumbest
assumptions, if this saves them implementation complexity somehow.
 
> Additionally, due to wrapping, and depending on the values of
> urb->number_of_packets and uinterval, I don't think this check will
> reliably detect when urb_end extends too far into the future. That
> said, I'm not sure this check is actually meaningful.

FWIW I have an implementation which checks it reliably, though it
doesn't support ESIT > 512ms to save a few lines of code.

> On 5/12/26 12:08, Michal Pecio wrote:
> > A particularly fascinating case is period > 895ms; it seems that
> > xHCI only allows submitting shortly before interval boundary, or
> > "SIA".  
> 
> For endpoints with ESIT greater than 895ms, wouldn't SIA become
> necessary to avoid ring underrun/overrun?

Not sure, and it's hard to test when most likely no such HW exists.
It's also unclear how much HW would support ESIT > MFINDEX period.

Regards,
Michal