Re: [PATCH 1/2] usb: xhci: fix isoc silent reschedule creating stream gap on CFC controllers

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

On 5/6/26 22:55, Dylan Robinson wrote:
> On 5/5/26 16:32, Mathias Nyman wrote:
>> Agreed, setting start_frame to start_frame_id + 1 would only make sense for the very first URB, otherwise we create glitches.
>>
>> Looks like the whole start_frame_id calculation is incorrect.
> 
> I'd also like to call attention to the fact that index in
> xhci_get_isoc_frame_id() refers to the isoc packet index within the
> URB, not the position of the transfer in the overall stream. A driver
> could (although probably shouldn't) submit multiple URBs, each
> describing less than a microframe's worth of transfers, so index == 0
> does not guarantee that the computation is for a frame-aligned
> transfer.

ep->next_frame_id should be used for frame ID calculation after
the first URB is enqueued and endpoint running.

Need to makse sure it's correct, and in microframe units, and rounded to
to the correct frame for the TRB Frame ID field.

> 
> Additionally, urb->start_frame is initially computed in
> xhci_queue_isoc_tx_prepare(), and in the current implementation it is
> validated, and potentially modified again in xhci_get_isoc_frame_id().
> It is worth considering that xhci_queue_isoc_tx_prepare() computes a
> start frame close to the current IST, and if the system is preempted
> before xhci_queue_isoc_tx() runs and calls xhci_get_isoc_frame_id(),
> that start frame may already fall outside the valid scheduling window.

Hmm, looks like we are doing way too much checking here.
All the above are done with spin_lock_irqsave() held, meaning there
shouldn't be any delay by preemptions or interrupts.

re-reading the MFINDEX (microframe index) register for every TD in this
URB looks like a waste of time.
xhci_queue_isoc_tx_prepare() should read MFINDEX once, then:

a) If first URB (no ep->next_frame_id value) then calculate a proper
    start frame ID, and check last TD of urb still fits the widow

b) if ep->next_frame_id is set, then check if first TD of this URB
    can make it in time, also check last TD of urb is within window.
    I think all other register reads and checks in xhci_get_isoc_frame_id()
    can be skipped.
   (I'm confident that the non-preemtable loop queuing TRBs
    does a cycle faster than shortest isoc interval (125us)

Can I ask you to test some additional patches if I write them?
I don't have a good setup with frame sensitive usb devices to test this

Thanks
Mathias