From: Marcus Glocker Subject: Re: sys/uvideo: support of USB 3.0 speeds To: "Kirill A. Korinsky" Cc: tech@openbsd.org Date: Wed, 26 Feb 2025 21:04:11 +0100 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 >