On 2019/11/21 07:59, Patrick Wildt wrote: > On Thu, Nov 21, 2019 at 06:52:36AM +0000, [email protected] wrote: >> Hi, >> >> On 2019/11/18 20:34, Marcus Glocker wrote: >>> On Sun, Nov 17, 2019 at 12:36:49PM +0100, Marcus Glocker wrote: >>> >>>> While recently testing UDL on XHCI I faced a lot of USB timeouts, and >>>> therefore screen rendering issues. >>>> The reason is that XHCI currently only supports single bulk transfers >>>> up to 64k, while UDL can schedule a bulk transfer up to 1m. >>>> >>>> The following diff adds XHCI bulk transfer support >64k and makes UDL >>>> work fine for me on XHCI. >>>> >>>> mpi@ already did an initial re-view, and I have adapted some of his >>>> comments already in this diff. >>>> >>>> More testing and comments welcome. >>> >>> After patrick@ did feedback that the current bulk transfer code in >>> XHCI should also work for larger transfers as is, we have found the >>> bug which resulted in a much smaller diff :-) >>> >>> Attached for your reference. It just has been committed. >>> >>> >>> Index: xhci.c >>> =================================================================== >>> RCS file: /cvs/src/sys/dev/usb/xhci.c,v >>> retrieving revision 1.106 >>> diff -u -p -u -p -r1.106 xhci.c >>> --- xhci.c 6 Oct 2019 17:30:00 -0000 1.106 >>> +++ xhci.c 18 Nov 2019 19:32:30 -0000 >>> @@ -2871,7 +2871,7 @@ xhci_device_generic_start(struct usbd_xf >>> /* If the buffer crosses a 64k boundary, we need one more. */ >>> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >>> if (len < xfer->length) >>> - ntrb++; >>> + ntrb = howmany(xfer->length - len, XHCI_TRB_MAXSIZE) + 1; >>> else >>> len = xfer->length; >>> >>> >> >> Should xhci_device_isoc_start also count up the chunks after boundary? >> > > We had discussed this as well, but since a frame cannot be bigger than > 64k, a hard boundary which is visible when you realize frlengths is a > uint16_t *, that change would essentially be a no-op.
Oh, I missed it. Thank you for explanation. I see, xhci_event_xfer_isoc does not need fix. > > We could change it for the sake of having the same calculation in all > places. > > Thanks, > Patrick > >> >> --- sys/dev/usb/xhci.c.orig Mon Nov 18 22:47:25 2019 >> +++ sys/dev/usb/xhci.c Thu Nov 21 05:13:02 2019 >> @@ -3049,7 +3049,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) >> /* If the buffer crosses a 64k boundary, we need one more. */ >> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >> if (len < xfer->frlengths[i]) >> - ntrb++; >> + ntrb = howmany(xfer->frlengths[i] - len, >> + XHCI_TRB_MAXSIZE) + 1; >> >> paddr += xfer->frlengths[i]; >> } >> @@ -3066,7 +3067,8 @@ xhci_device_isoc_start(struct usbd_xfer *xfer) >> /* If the buffer crosses a 64k boundary, we need one more. */ >> len = XHCI_TRB_MAXSIZE - (paddr & (XHCI_TRB_MAXSIZE - 1)); >> if (len < xfer->frlengths[i]) >> - ntrb++; >> + ntrb = howmany(xfer->frlengths[i] - len, >> + XHCI_TRB_MAXSIZE) + 1; >> else >> len = xfer->frlengths[i]; >> >>
