Index | Thread | Search

From:
Kirill A. Korinsky <kirill@korins.ky>
Subject:
Re: Support LG UltraFine Display Camera
To:
Marcus Glocker <marcus@nazgul.ch>
Cc:
OpenBSD tech <tech@openbsd.org>, Klemens Nanni <kn@openbsd.org>
Date:
Sun, 01 Dec 2024 16:20:04 +0100

Download raw body.

Thread
On Sun, 01 Dec 2024 09:36:31 +0100,
Marcus Glocker <marcus@nazgul.ch> 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