[Bug 220748] New: 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

            Bug ID: 220748
           Summary: usb: xhci_queue_isoc_tx_prepare ignore start_frame and
                    always assumes URB_ISO_ASAP is set
           Product: Drivers
           Version: 2.5
          Hardware: All
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P3
         Component: USB
          Assignee: drivers_usb@kernel-bugs.kernel.org
          Reporter: nick83ola@gmail.com
        Regression: No

This commit implemented Isochronous transfer for the xHCI controllers and in
the first implementation always assumes URB_ISO_ASAP is set and ignores the
start_frame field.

"Always assume URB_ISO_ASAP is set, and never use urb->start_frame as input."

https://github.com/torvalds/linux/commit/04e51901dd44f40a5a385ced897f6bca87d5f40a

```
USB: xHCI: Isochronous transfer implementation
This patch implements isochronous urb enqueue and interrupt handler part.

When an isochronous urb is passed to xHCI driver, first check the transfer
ring to guarantee there is enough room for the whole urb. Then update the
start_frame and interval field of the urb. Always assume URB_ISO_ASAP
is set, and never use urb->start_frame as input.

```

On this line the scheduler always schedule as soon as possible

https://github.com/torvalds/linux/blob/17d85f33a83b84e7d36bc3356614ae06c90e7a08/drivers/usb/host/xhci-ring.c#L4332-L4333

        start_frame = readl(&xhci->run_regs->microframe_index);
        start_frame &= 0x3fff;

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

https://github.com/torvalds/linux/blob/17d85f33a83b84e7d36bc3356614ae06c90e7a08/drivers/usb/host/ehci-sched.c#L1638-L1654


                /* URB_ISO_ASAP: make sure that start >= next */
                if (unlikely(start < next &&
                                (urb->transfer_flags & URB_ISO_ASAP)))
                        goto do_ASAP;


                /* Otherwise use start, if it's not in the past */
                if (likely(start >= now2))
                        goto use_start;

...

 use_start:
        /* Tried to schedule too far into the future? */
        if (unlikely(start + span - period >= mod + wrap)) {
                ehci_dbg(ehci, "request %p would overflow (%u+%u >= %u)\n",
                                urb, start, span - period, mod + wrap);
                status = -EFBIG;
                goto fail;
        }


        start += base;
        stream->next_uframe = (start + skip) & (mod - 1);


        /* report high speed start in uframes; full speed, in frames */
        urb->start_frame = start & (mod - 1);
        if (!stream->highspeed)
                urb->start_frame >>= 3;
        return status;


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

See also this thread on linuxmusicians.com

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

Thanks,
Nicola Lunghi

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

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