Index | Thread | Search

From:
Marcus Glocker <marcus@nazgul.ch>
Subject:
Re: sys/uvideo: support of USB 3.0 speeds
To:
"Kirill A. Korinsky" <kirill@korins.ky>
Cc:
tech@openbsd.org
Date:
Wed, 26 Feb 2025 21:04:11 +0100

Download raw body.

Thread
On Sun, Feb 23, 2025 at 04:06:23PM GMT, Kirill A. Korinsky wrote:

> tech@,
> 
> here a diff which reads USB 3.0 speed for webcam when it is supported.
> 
> USB 3.0 Section 9.6.7 states that wBytesPerInterval is only valid for
> periodic endpoints (isochronous and interrupt), so here bulk endpoints
> should skip it.
> 
> But for isochronous it should increase quality.
> 
> I had tested it with two USB 3 webcam which I have (Elgato Facecam Pro
> and Jabra PanaCast 20), but both of them is bulk only, no regression and
> no change.
> 
> I also had tested it with two isochronous webcam which is embeded into
> used laptops, but both of them USB 2. No regression and no change.
> 
> Anyway, enabled debug confirms that it dumps correct values for
> interrupt endpoints on both USB 3 webcam, and it should work, I think.
> 
> BTW, Linux driver has almost the same logic.
> 
> Tests? Ok?

I currently have no USB 3.0 cam with isoc here to test.  I think it
would be good if we would have some testers for this diff.
 
> Index: sys/dev/usb/uvideo.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/usb/uvideo.c,v
> diff -u -p -r1.240 uvideo.c
> --- sys/dev/usb/uvideo.c	23 Feb 2025 14:12:15 -0000	1.240
> +++ sys/dev/usb/uvideo.c	23 Feb 2025 14:53:14 -0000
> @@ -138,7 +138,8 @@ usbd_status	uvideo_vs_parse_desc_format_
>  usbd_status	uvideo_vs_parse_desc_frame(struct uvideo_softc *);
>  usbd_status	uvideo_vs_parse_desc_frame_sub(struct uvideo_softc *,
>  		    const usb_descriptor_t *);
> -uint32_t	uvideo_vc_parse_max_packet_size(usb_endpoint_descriptor_t *);
> +uint32_t	uvideo_vc_parse_max_packet_size(struct uvideo_softc *,
> +		    usb_endpoint_descriptor_t *);
>  usbd_status	uvideo_vs_parse_desc_alt(struct uvideo_softc *, int, int, int);
>  usbd_status	uvideo_vs_set_alt(struct uvideo_softc *,
>  		    struct usbd_interface *, int);
> @@ -191,6 +192,8 @@ void		uvideo_dump_desc_output(struct uvi
>  		    const usb_descriptor_t *);
>  void		uvideo_dump_desc_endpoint(struct uvideo_softc *,
>  		    const usb_descriptor_t *);
> +void		uvideo_dump_desc_endpoint_ss_comp(struct uvideo_softc *,
> +		    const usb_descriptor_t *);
>  void		uvideo_dump_desc_iface_assoc(struct uvideo_softc *,
>  		    const usb_descriptor_t *);
>  void		uvideo_dump_desc_interface(struct uvideo_softc *,
> @@ -1237,12 +1240,39 @@ uvideo_vs_parse_desc_frame_sub(struct uv
>  }
>  
>  uint32_t
> -uvideo_vc_parse_max_packet_size(usb_endpoint_descriptor_t *ed)
> +uvideo_vc_parse_max_packet_size(struct uvideo_softc *sc,
> +    usb_endpoint_descriptor_t *ed)
>  {
>  	uint32_t psize;
> +	struct usbd_desc_iter iter;
> +	const usb_descriptor_t *desc;
> +	usb_endpoint_ss_comp_descriptor_t *esscd;
> +
> +	/*
> +	 * USB 3.0 Section 9.6.7 states that wBytesPerInterval is only
> +	 * valid for periodic endpoints (isochronous and interrupt).
> +	 */
> +	if (UE_GET_XFERTYPE(ed->bmAttributes) == UE_BULK)
> +		goto skip_ss_comp;
> +
> +	usbd_desc_iter_init(sc->sc_udev, &iter);
> +	while ((desc = usbd_desc_iter_next(&iter))) {
> +		if (desc == (const usb_descriptor_t *)ed) {
> +			desc = usbd_desc_iter_next(&iter);
> +			break;
> +		}
> +	}
>  
> -	/* XXX: get USB 3.0 speed from wBytesPerInterval */
> +	if (desc && sc->sc_udev->speed >= USB_SPEED_SUPER &&
> +	    desc->bDescriptorType == UDESC_ENDPOINT_SS_COMP) {
> +		esscd = (usb_endpoint_ss_comp_descriptor_t *)desc;
> +		psize = UGETW(esscd->wBytesPerInterval);
> +		DPRINTF(1, "%s: wBytesPerInterval=%d\n",
> +		    DEVNAME(sc), psize);
> +		return psize;
> +	}
>  
> +skip_ss_comp:
>  	psize = UGETW(ed->wMaxPacketSize);
>  	psize = UE_GET_SIZE(psize) * (1 + UE_GET_TRANS(psize));
>  
> @@ -1321,7 +1351,7 @@ uvideo_vs_parse_desc_alt(struct uvideo_s
>  		if (bulk_endpoint && !vs->bulk_endpoint)
>  			goto next;
>  
> -		psize = uvideo_vc_parse_max_packet_size(ed);
> +		psize = uvideo_vc_parse_max_packet_size(sc, ed);
>  		/* save endpoint with largest bandwidth */
>  		if (psize > vs->psize) {
>  			vs->ifaceh = &sc->sc_udev->ifaces[iface];
> @@ -1399,7 +1429,7 @@ uvideo_vs_set_alt(struct uvideo_softc *s
>  			goto next;
>  
>  		/* save endpoint with requested bandwidth */
> -		psize = uvideo_vc_parse_max_packet_size(ed);
> +		psize = uvideo_vc_parse_max_packet_size(sc, ed);
>  		if (psize >= max_packet_size)
>  			diff = psize - max_packet_size;
>  		else
> @@ -2606,6 +2636,11 @@ uvideo_dump_desc_all(struct uvideo_softc
>  			printf("|\n");
>  			uvideo_dump_desc_endpoint(sc, desc);
>  			break;
> +		case UDESC_ENDPOINT_SS_COMP:
> +			printf(" (UDESC_ENDPOINT_SS_COMP)\n");
> +			printf("|\n");
> +			uvideo_dump_desc_endpoint_ss_comp(sc, desc);
> +			break;
>  		case UDESC_INTERFACE:
>  			printf(" (UDESC_INTERFACE)\n");
>  			printf("|\n");
> @@ -2741,6 +2776,21 @@ uvideo_dump_desc_endpoint(struct uvideo_
>  		printf(" (UE_INTERRUPT)\n");
>  	printf("wMaxPacketSize=%d\n", UGETW(d->wMaxPacketSize));
>  	printf("bInterval=0x%02x\n", d->bInterval);
> +}
> +
> +void
> +uvideo_dump_desc_endpoint_ss_comp(struct uvideo_softc *sc,
> +    const usb_descriptor_t *desc)
> +{
> +	usb_endpoint_ss_comp_descriptor_t *d;
> +
> +	d = (usb_endpoint_ss_comp_descriptor_t *)(uint8_t *)desc;
> +
> +	printf("bLength=%d\n", d->bLength);
> +	printf("bDescriptorType=0x%02x\n", d->bDescriptorType);
> +	printf("bMaxBurst=0x%02x\n", d->bMaxBurst);
> +	printf("bmAttributes=0x%02x\n", d->bmAttributes);
> +	printf("wBytesPerInterval=%d\n", UGETW(d->wBytesPerInterval));
>  }
>  
>  void
>