en Fri, Feb 28, 2025 at 10:07:23AM GMT, Marcus Glocker wrote:

> 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.

I guess we will not get much more feedback on this diff, given it's
only useful for isoc devices.  I have re-tested the diff again, on
the latest code base, and I still didn't face any regression.

I would prefer to get this diff rather in early, so that we could fix
any possible regression soon.

Therefore, ok mglocker@
 
> > > 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;
> > >  	}
> > 
>