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:
Mon, 02 Dec 2024 00:35:34 +0100

Download raw body.

Thread
On Sun, 01 Dec 2024 22:12:55 +0100,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> On Sun, Dec 01, 2024 at 04:20:04PM GMT, Kirill A. Korinsky wrote:
> 
> > 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?
> 
> This still breaks my Logitech Webcam C930e :-(
>

Thanks for testing. Indeed it is quite... delicate things and quirk seems to
be a safer way to do it.

To avoid any misleading to whoever wants to test it, I re-send the diff:

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 23:31:00 -0000
@@ -380,12 +380,18 @@ const struct uvideo_devs {
 	    NULL,
 	    UVIDEO_FLAG_NOATTACH
 	},
-	{   /* Needs renegotiate after setting alternate interface */
+	{   /* Needs to renegotiate after setting alternate interface */
 	    { USB_VENDOR_GN_NETCOM, USB_PRODUCT_GN_NETCOM_JABRA_PANACAST_20 },
 	    NULL,
 	    NULL,
 	    UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT
 	},
+	{   /* Needs to renegotiate after setting alternate interface */
+	    { USB_VENDOR_LG, USB_PRODUCT_LG_ULTRA_FINE_VIDEO },
+	    NULL,
+	    NULL,
+	    UVIDEO_FLAG_RENEGOTIATE_AFTER_SET_ALT
+	},
 };
 #define uvideo_lookup(v, p) \
 	((const struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
@@ -2051,6 +2057,9 @@ uvideo_vs_start_bulk_thread(void *arg)
 			    DEVNAME(sc), usbd_errstr(error));
 			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);
 
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 23:31:00 -0000
@@ -2704,6 +2704,7 @@ product LEXMARK S2450		0x0009	Optra S 24
 /* LG products */
 product LG RTL8153		0x9819	RTL8153
 product LG RTL8153B		0x9820	RTL8153
+product LG ULTRA_FINE_VIDEO	0x9a68	LG UltraFine Display Camera
 
 /* Liebert products */
 product LIEBERT UPS		0xffff	UPS
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 23:31:04 -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.
@@ -2711,6 +2711,7 @@
 /* LG products */
 #define	USB_PRODUCT_LG_RTL8153	0x9819		/* RTL8153 */
 #define	USB_PRODUCT_LG_RTL8153B	0x9820		/* RTL8153 */
+#define	USB_PRODUCT_LG_ULTRA_FINE_VIDEO	0x9a68		/* LG UltraFine Display Camera */
 
 /* Liebert products */
 #define	USB_PRODUCT_LIEBERT_UPS	0xffff		/* UPS */
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 23:31:04 -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.
@@ -6056,6 +6056,10 @@ const struct usb_known_product usb_known
 	{
 	    USB_VENDOR_LG, USB_PRODUCT_LG_RTL8153B,
 	    "RTL8153",
+	},
+	{
+	    USB_VENDOR_LG, USB_PRODUCT_LG_ULTRA_FINE_VIDEO,
+	    "LG UltraFine Display Camera",
 	},
 	{
 	    USB_VENDOR_LIEBERT, USB_PRODUCT_LIEBERT_UPS,


-- 
wbr, Kirill