[Bug 220748] usb: xhci_queue_isoc_tx_prepare ignore start_frame and always assumes URB_ISO_ASAP is set

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=220748

Michał Pecio (michal.pecio@gmail.com) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |michal.pecio@gmail.com

--- Comment #1 from Michał Pecio (michal.pecio@gmail.com) ---
Looks like you are right. I guess it means that synchronizing multiple
soundcards or playback/capture streams isn't working either? (If class drivers
implement that in the first place.)

I wonder if this could be the reason for some weird snd-usb-audio behavior
reported in the past (repeatedly unlinking and retrying URBs for unclear reason
when running full-duplex with very low latency). But it's just a wild guess.

Did you try the obvious?

+       if (!(urb->transfer_flags & URB_ISO_ASAP))
+               goto skip_start_over;
        /* Calculate the start frame and put it in urb->start_frame. */

> instead for example in drivers/usb/host/xhci-ring.c, function
> xhci_queue_isoc_tx_prepare() the URB_ISO_ASAP flag is correctly used

Actually, this function looks OK but it isn't called correctly because this
shouldn't be limited to CFC *if* we are scheduling the first TD in a new "data
flow" (xHCI 4.11.2.5). But on modern HW it seems to be correct, and if you skip
the earlier urb->start_frame overwrite, chances are things may work for you.

> This causes issues with devices that require a specific frame interval (like
> professional audio interfaces, for example the Motu AVB line)

Does it mean there is hardware which breaks when polled at the rate specified
in its own endpoint descriptor? Would it still break if you insert zero-length
frames in the "forbidden" periods?

Using start_frame like that seems complicated and it's not feasible on Gen-1
controllers (no CFC). Per Documentation/driver-api/usb/URB drivers may specify
any service interval on per-URB basis, but this of course can't work with xHCI
and results in a warning. However, as suggested in check_interval() comments,
there could be a way of setting different polling interval at endpoint
creation. I see no reason for xHCI HW not to accept that, even Gen-1.

> https://linuxmusicians.com/viewtopic.php?p=179979#p179979

Hmm, using the sideband API to bypass kernel bugs seems even more extreme :)

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.