From: Kirill A. Korinsky Subject: Re: Support LG UltraFine Display Camera To: Marcus Glocker Cc: OpenBSD tech , Klemens Nanni Date: Sun, 01 Dec 2024 16:20:04 +0100 On Sun, 01 Dec 2024 09:36:31 +0100, Marcus Glocker wrote: > > On Sat, Nov 30, 2024 at 11:25:15PM GMT, Kirill A. Korinsky wrote: > > > tech@, > > > > A new quirk UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT from this thread [1] is > > required for at least one more camera: LG UltraFine Display Camera. And this > > camera allows to discover a bug with bulk processing inside uvideo: here > > usbd_get_xfer_status is missing and current code is built on the assumption > > that device always returns dwMaxPayloadTransferSize bytes. > > > > Here is a diff that adds the missing usbd_get_xfer_status and the quirk. > > > > Feedback? Tests? Ok? > > I currently have no bulk device here to regression test, but the size > fix makes sense to me. > Frankly, I don't like idea to add quirk for each cam, especially that both of these cams work in Linux and I haven't found any quirk for them. I see that I need it for two quite different webcams, and I guess that it might be needed for some others. So, after carefully reading the UVC 1.5 Class specification.pdf, I think I found a better, cleaner way that doesn't require any quirk and is allowed by the specification. This diff works on all three webcams available to me. Marcus, can I ask you to test this diff? Thanks. Index: sys/dev/usb/uvideo.c =================================================================== RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v diff -u -p -r1.224 uvideo.c --- sys/dev/usb/uvideo.c 30 Nov 2024 17:47:23 -0000 1.224 +++ sys/dev/usb/uvideo.c 1 Dec 2024 15:01:46 -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)) @@ -1906,17 +1899,6 @@ 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); - } - } - DPRINTF(1, "%s: open pipe for bEndpointAddress=0x%02x\n", DEVNAME(sc), sc->sc_vs_cur->endpoint); error = usbd_open_pipe( @@ -2052,6 +2034,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, @@ -3414,6 +3399,11 @@ uvideo_streamon(void *v, int type) uvideo_vs_start_bulk(sc); else uvideo_vs_start_isoc(sc); + + /* 4.3.1.2.1 Allows dynamic modification of stream settings */ + error = uvideo_vs_set_commit(sc, (uint8_t *)&sc->sc_desc_probe); + if (error != USBD_NORMAL_COMPLETION) + return (error); return (0); } 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 1 Dec 2024 15:13:45 -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 1 Dec 2024 15:13:58 -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 1 Dec 2024 15:13:58 -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, -- wbr, Kirill