From: Marcus Glocker <marcus@nazgul.ch>
Subject: Re: sys/xhci: suppoort USB3 speeds
To: tech@openbsd.org
Date: Fri, 28 Feb 2025 10:07:23 +0100

On Thu, Feb 27, 2025 at 08:49:16PM GMT, Marcus Glocker wrote:

> On Thu, Feb 27, 2025 at 08:21:25PM GMT, Kirill A. Korinsky wrote:
> 
> > tech@,
> > 
> > I'd like to add support of USB3 speeds to XHCI driver.
> > 
> > This diff was tested on Snapdragon SoC:
> > 
> > 	xhci0 at qcdwusb0, xHCI 1.10
> > 	usb0 at xhci0: USB revision 3.0
> > 	uhub0 at usb0 configuration 1 interface 0 "Generic xHCI root hub" rev 3.00/1.00 addr 1
> > 
> > I also used diff: https://marc.info/?l=openbsd-tech&m=174032299500347&w=2
> > and switched Elgato Facecam Pro to isochronous mode with streaming
> > 3840x2160 30fps with 33740 isoc frames / packets.
> > 
> > It works.
> > 
> > Ok?
> 
> After borrowing an USB3 Logitech BRIO cam from the office, I'm also
> able to provide some testing on this topic :-)
> 
> With this diff, and the uvideo USB3 diff [1], I'm able to stream
> uncompressed 1920x1080 video with a isoc packet/frame size of 18432 bytes,
> which is quite nice.
> 
> I'm basically OK with this diff, but I would like to see one or two
> more OKs, and some more regression test reports.
> 
> [1] https://marc.info/?l=openbsd-tech&m=174060005721417&w=2

uvideo1 at uhub2 port 1 configuration 1 interface 0 "Chicony Tech. Inc. Dell Web
cam WB7022" rev 3.20/8.21 addr 8

is now also working fine with those two patches.  Previously it was
only delivering broken frames.

> > Index: sys/dev/usb/usb_subr.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/dev/usb/usb_subr.c,v
> > diff -u -p -r1.163 usb_subr.c
> > --- sys/dev/usb/usb_subr.c	23 May 2024 03:21:09 -0000	1.163
> > +++ sys/dev/usb/usb_subr.c	27 Feb 2025 18:49:51 -0000
> > @@ -514,7 +514,8 @@ int
> >  usbd_parse_idesc(struct usbd_device *dev, struct usbd_interface *ifc)
> >  {
> >  #define ed ((usb_endpoint_descriptor_t *)p)
> > -	char *p, *end;
> > +#define essd ((usb_endpoint_ss_comp_descriptor_t *)pp)
> > +	char *p, *pp, *end;
> >  	int i;
> > 
> >  	p = (char *)ifc->idesc + ifc->idesc->bLength;
> > @@ -534,6 +535,11 @@ usbd_parse_idesc(struct usbd_device *dev
> >  		if (p >= end)
> >  			return (-1);
> > 
> > +		pp = p + ed->bLength;
> > +		if (pp >= end || essd->bLength == 0 ||
> > +		    essd->bDescriptorType != UDESC_ENDPOINT_SS_COMP)
> > +			pp = NULL;
> > +
> >  		if (dev->speed == USB_SPEED_HIGH) {
> >  			unsigned int mps;
> > 
> > @@ -557,6 +563,7 @@ usbd_parse_idesc(struct usbd_device *dev
> >  		}
> > 
> >  		ifc->endpoints[i].edesc = ed;
> > +		ifc->endpoints[i].esscd = essd;
> >  		ifc->endpoints[i].refcnt = 0;
> >  		ifc->endpoints[i].savedtoggle = 0;
> >  		p += ed->bLength;
> > @@ -564,6 +571,7 @@ usbd_parse_idesc(struct usbd_device *dev
> > 
> >  	return (0);
> >  #undef ed
> > +#undef essd
> >  }
> > 
> >  void
> > Index: sys/dev/usb/usbdivar.h
> > ===================================================================
> > RCS file: /home/cvs/src/sys/dev/usb/usbdivar.h,v
> > diff -u -p -r1.84 usbdivar.h
> > --- sys/dev/usb/usbdivar.h	8 Oct 2024 19:42:31 -0000	1.84
> > +++ sys/dev/usb/usbdivar.h	27 Feb 2025 18:48:11 -0000
> > @@ -55,6 +55,7 @@ struct usbd_pipe;
> > 
> >  struct usbd_endpoint {
> >  	usb_endpoint_descriptor_t *edesc;
> > +	usb_endpoint_ss_comp_descriptor_t *esscd;
> >  	int			refcnt;
> >  	int			savedtoggle;
> >  };
> > Index: sys/dev/usb/xhci.c
> > ===================================================================
> > RCS file: /home/cvs/src/sys/dev/usb/xhci.c,v
> > diff -u -p -r1.135 xhci.c
> > --- sys/dev/usb/xhci.c	8 Oct 2024 19:42:31 -0000	1.135
> > +++ sys/dev/usb/xhci.c	27 Feb 2025 18:51:05 -0000
> > @@ -1355,6 +1355,7 @@ static inline uint32_t
> >  xhci_get_txinfo(struct xhci_softc *sc, struct usbd_pipe *pipe)
> >  {
> >  	usb_endpoint_descriptor_t *ed = pipe->endpoint->edesc;
> > +	usb_endpoint_ss_comp_descriptor_t *esscd = pipe->endpoint->esscd;
> >  	uint32_t mep, atl, mps = UGETW(ed->wMaxPacketSize);
> > 
> >  	switch (UE_GET_XFERTYPE(ed->bmAttributes)) {
> > @@ -1364,8 +1365,10 @@ xhci_get_txinfo(struct xhci_softc *sc, s
> >  		break;
> >  	case UE_INTERRUPT:
> >  	case UE_ISOCHRONOUS:
> > -		if (pipe->device->speed == USB_SPEED_SUPER) {
> > -			/*  XXX Read the companion descriptor */
> > +		if (esscd && pipe->device->speed >= USB_SPEED_SUPER) {
> > +			mep = UGETW(esscd->wBytesPerInterval);
> > +			atl = mep;
> > +			break;
> >  		}
> > 
> >  		mep = (UE_GET_TRANS(mps) + 1) * UE_GET_SIZE(mps);
> > @@ -1441,6 +1444,7 @@ uint32_t
> >  xhci_pipe_maxburst(struct usbd_pipe *pipe)
> >  {
> >  	usb_endpoint_descriptor_t *ed = pipe->endpoint->edesc;
> > +	usb_endpoint_ss_comp_descriptor_t *esscd = pipe->endpoint->esscd;
> >  	uint32_t mps = UGETW(ed->wMaxPacketSize);
> >  	uint8_t xfertype = UE_GET_XFERTYPE(ed->bmAttributes);
> >  	uint32_t maxb = 0;
> > @@ -1451,7 +1455,9 @@ xhci_pipe_maxburst(struct usbd_pipe *pip
> >  			maxb = UE_GET_TRANS(mps);
> >  		break;
> >  	case USB_SPEED_SUPER:
> > -		/*  XXX Read the companion descriptor */
> > +		if (esscd &&
> > +		    (xfertype == UE_ISOCHRONOUS || xfertype == UE_INTERRUPT))
> > +			maxb = esscd->bMaxBurst;
> >  	default:
> >  		break;
> >  	}
>