From: Marcus Glocker Subject: Re: Support LG UltraFine Display Camera To: "Kirill A. Korinsky" Cc: OpenBSD tech , Klemens Nanni Date: Sat, 7 Dec 2024 00:00:43 +0100 On Thu, Dec 05, 2024 at 01:22:59AM GMT, Kirill A. Korinsky wrote: > On Sun, 01 Dec 2024 22:12:55 +0100, > Marcus Glocker wrote: > > > > This still breaks my Logitech Webcam C930e :-( > > > > I think the quirk approach is wrong. So I re-read the UVC 1.5 class > specification, and I think I figured it out. > > Section 2.4.3 states that the bulk endpoint only supports the alternative > setting of zero, which is the default stream, and which is switched at > uvideo_attach_hook. Inside uvideo_vs_close the code uses the same switch to > turn off the cam LED. Makes sense. > And I see this behaviour on both bulk cams: it flashes the LED and looks > like it is switched off. > > I had checked the linux, and it doesn't set alt interface for bulk: > https://github.com/torvalds/linux/blob/v6.12/drivers/media/usb/uvc/uvc_video.c#L2054-L2064 > > Here is the diff that skips uvideo_vs_set_alt inside uvideo_vs_open for > bulk, and cleaned up quirk and usbdevs. > > This diff is enough to make the Jabra PanaCast 20 work, and by adding the > missing usbd_get_xfer_status the LG UltraFine Display Camera works as well. Yep, that was already OK for me in the last diff. > Ok? I've tested your diff against my two cams, and they both still work. Can you please stop including the usbdevs.h and ubsdevs_data.h changes? It just complicates the application of the patch. If there is a change in usbdevs, one needs to run 'make'. And it's also fine to keep two different changes in one diff for the same file. Otherwise ok mglocker@ > Index: sys/dev/usb/uvideo.c > =================================================================== > RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v > diff -u -p -r1.225 uvideo.c > --- sys/dev/usb/uvideo.c 1 Dec 2024 10:38:47 -0000 1.225 > +++ sys/dev/usb/uvideo.c 5 Dec 2024 00:20:16 -0000 > @@ -298,7 +298,6 @@ const struct video_hw_if uvideo_hw_if = > #define UVIDEO_FLAG_REATTACH 0x2 > #define UVIDEO_FLAG_VENDOR_CLASS 0x4 > #define UVIDEO_FLAG_NOATTACH 0x8 > -#define UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT 0x10 > const struct uvideo_devs { > struct usb_devno uv_dev; > char *ucode_name; > @@ -380,12 +379,6 @@ const struct uvideo_devs { > NULL, > UVIDEO_FLAG_NOATTACH > }, > - { /* Needs renegotiate after setting alternate interface */ > - { USB_VENDOR_GN_NETCOM, USB_PRODUCT_GN_NETCOM_JABRA_PANACAST_20 }, > - NULL, > - NULL, > - UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT > - }, > }; > #define uvideo_lookup(v, p) \ > ((const struct uvideo_devs *)usb_lookup(uvideo_devs, v, p)) > @@ -1889,6 +1882,10 @@ uvideo_vs_open(struct uvideo_softc *sc) > return (error); > } > > + /* 2.4.3 the bulk endpoint only supports the alternative setting of 0 */ > + if (sc->sc_vs_cur->bulk_endpoint) > + goto skip_set_alt; > + > error = uvideo_vs_set_alt(sc, sc->sc_vs_cur->ifaceh, > UGETDW(sc->sc_desc_probe.dwMaxPayloadTransferSize)); > if (error != USBD_NORMAL_COMPLETION) { > @@ -1906,17 +1903,7 @@ uvideo_vs_open(struct uvideo_softc *sc) > return (USBD_INVAL); > } > > - /* renegotiate with commit after setting alternate interface */ > - if (sc->sc_quirk && > - sc->sc_quirk->flags & UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT) { > - error = uvideo_vs_negotiation(sc, 1); > - if (error != USBD_NORMAL_COMPLETION) { > - printf("%s: could not renegotiate after setting " > - "alternate interface!\n", DEVNAME(sc)); > - return (error); > - } > - } > - > +skip_set_alt: > DPRINTF(1, "%s: open pipe for bEndpointAddress=0x%02x\n", > DEVNAME(sc), sc->sc_vs_cur->endpoint); > error = usbd_open_pipe( > Index: sys/dev/usb/usbdevs > =================================================================== > RCS file: /home/cvs/src/sys/dev/usb/usbdevs,v > diff -u -p -r1.772 usbdevs > --- sys/dev/usb/usbdevs 30 Nov 2024 17:41:57 -0000 1.772 > +++ sys/dev/usb/usbdevs 4 Dec 2024 23:47:04 -0000 > @@ -409,7 +409,6 @@ vendor NEODIO 0x0aec Neodio > vendor OPTION 0x0af0 Option > vendor ASUS 0x0b05 ASUS > vendor TODOS 0x0b0c Todos Data System > -vendor GN_NETCOM 0x0b0e GN Jabra A/S > vendor GEARHEAD 0x0b38 Gear Head > vendor OCT 0x0b39 Omnidirectional Control Technology > vendor TEKRAM 0x0b3b Tekram Technology > @@ -2204,9 +2203,6 @@ product GMATE YP3X00 0x1001 YP3X00 PDA > > /* GN Otometrics products */ > product GNOTOMETRICS AURICAL 0x0010 Aurical > - > -/* GN Netcom products */ > -product GN_NETCOM JABRA_PANACAST_20 0x3021 Jabra PanaCast 20 > > /* GoHubs products */ > product GOHUBS GOCOM232 0x1001 GoCOM232 Serial converter > Index: sys/dev/usb/usbdevs.h > =================================================================== > RCS file: /home/cvs/src/sys/dev/usb/usbdevs.h,v > diff -u -p -r1.784 usbdevs.h > --- sys/dev/usb/usbdevs.h 30 Nov 2024 17:43:46 -0000 1.784 > +++ sys/dev/usb/usbdevs.h 4 Dec 2024 23:47:17 -0000 > @@ -1,4 +1,4 @@ > -/* $OpenBSD: usbdevs.h,v 1.784 2024/11/30 17:43:46 kirill Exp $ */ > +/* $OpenBSD$ */ > > /* > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. > @@ -416,7 +416,6 @@ > #define USB_VENDOR_OPTION 0x0af0 /* Option */ > #define USB_VENDOR_ASUS 0x0b05 /* ASUS */ > #define USB_VENDOR_TODOS 0x0b0c /* Todos Data System */ > -#define USB_VENDOR_GN_NETCOM 0x0b0e /* GN Jabra A/S */ > #define USB_VENDOR_GEARHEAD 0x0b38 /* Gear Head */ > #define USB_VENDOR_OCT 0x0b39 /* Omnidirectional Control Technology */ > #define USB_VENDOR_TEKRAM 0x0b3b /* Tekram Technology */ > @@ -2211,9 +2210,6 @@ > > /* GN Otometrics products */ > #define USB_PRODUCT_GNOTOMETRICS_AURICAL 0x0010 /* Aurical */ > - > -/* GN Netcom products */ > -#define USB_PRODUCT_GN_NETCOM_JABRA_PANACAST_20 0x3021 /* Jabra PanaCast 20 */ > > /* GoHubs products */ > #define USB_PRODUCT_GOHUBS_GOCOM232 0x1001 /* GoCOM232 Serial converter */ > Index: sys/dev/usb/usbdevs_data.h > =================================================================== > RCS file: /home/cvs/src/sys/dev/usb/usbdevs_data.h,v > diff -u -p -r1.778 usbdevs_data.h > --- sys/dev/usb/usbdevs_data.h 30 Nov 2024 17:43:46 -0000 1.778 > +++ sys/dev/usb/usbdevs_data.h 4 Dec 2024 23:47:17 -0000 > @@ -1,4 +1,4 @@ > -/* $OpenBSD: usbdevs_data.h,v 1.778 2024/11/30 17:43:46 kirill Exp $ */ > +/* $OpenBSD$ */ > > /* > * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT. > @@ -4618,10 +4618,6 @@ const struct usb_known_product usb_known > "Aurical", > }, > { > - USB_VENDOR_GN_NETCOM, USB_PRODUCT_GN_NETCOM_JABRA_PANACAST_20, > - "Jabra PanaCast 20", > - }, > - { > USB_VENDOR_GOHUBS, USB_PRODUCT_GOHUBS_GOCOM232, > "GoCOM232 Serial converter", > }, > @@ -13940,10 +13936,6 @@ const struct usb_known_vendor usb_known_ > { > USB_VENDOR_TODOS, > "Todos Data System", > - }, > - { > - USB_VENDOR_GN_NETCOM, > - "GN Jabra A/S", > }, > { > USB_VENDOR_GEARHEAD, > > Index: sys/dev/usb/uvideo.c > =================================================================== > RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v > diff -u -p -r1.225 uvideo.c > --- sys/dev/usb/uvideo.c 1 Dec 2024 10:38:47 -0000 1.225 > +++ sys/dev/usb/uvideo.c 4 Dec 2024 22:55:18 -0000 > @@ -2052,6 +2052,9 @@ uvideo_vs_start_bulk_thread(void *arg) > break; > } > > + usbd_get_xfer_status(sc->sc_vs_cur->bxfer.xfer, > + NULL, NULL, &size, NULL); > + > DPRINTF(2, "%s: *** buffer len = %d\n", DEVNAME(sc), size); > > (void)sc->sc_decode_stream_header(sc, > > -- > wbr, Kirill >