Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: sys/xhci: suppoort USB3 speeds
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Thu, 27 Feb 2025 20:49:16 +0100

Download raw body.

Thread
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
 
> 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;
>  	}