From: Kirill A. Korinsky <kirill@korins.ky>
Subject: Re: sys/uvideo: add quriks for Elgato devices
To: Marcus Glocker <marcus@nazgul.ch>
Cc: OpenBSD tech <tech@openbsd.org>, Laurence Tratt <laurie@tratt.net>
Date: Wed, 05 Feb 2025 22:56:35 +0100

On Wed, 05 Feb 2025 21:03:52 +0100,
Marcus Glocker <marcus@nazgul.ch> wrote:
> 
> According to the Linux drivers comment, the issue seems to exist with
> the latest Elgato Cam Link 4K firmware, which indicates that the issue
> might not exist with older firmware's.  Therefore, they also check if
> the second byte of bmHint contains a value, since with a correct bmHint
> field this byte should be always zero, according the UVC specs
> (reserved bits).
> 
> Should we do the same?
> 
> if (sc->sc_quirk &&
>     sc->sc_quirk->flags & UVIDEO_FLAG_FORMAT_INDEX_IN_BMHINT &&
>     UGETW(pc->bmHint) > 255)
> 
> I think a comment inside of the quirks 'if' branch would be also
> helpful to understand what's going on.  Something like
> 
> /*
>  * The Elgato Cam Link 4K returns an invalid bmHint response, which
>  * contains the bFormatIndex in the second byte.  But it should be
>  * always zero since it's a reserved bit field.
>  */
> 
> maybe.
>

Thanks for feedback.

I had spend some time for investigation of should > 255 to be a part of
condition, and I haven't found any trace of such device.

But, original author of this hack to Linux kernel had reported it to Elgato:
https://github.com/torvalds/linux/commit/4c6e0976295add7f0ed94d276c04a3d6f1ea8f83

So, probably someday it migth be fixed, and new firmware won't need it.

Thus, here an updated diff which includes both your suggestions.

Index: sys/dev/usb/usbdevs
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/usbdevs,v
diff -u -p -r1.773 usbdevs
--- sys/dev/usb/usbdevs	14 Dec 2024 10:02:47 -0000	1.773
+++ sys/dev/usb/usbdevs	4 Feb 2025 08:40:13 -0000
@@ -692,6 +692,7 @@ vendor DATAAPEX		0xdaae	DataApex
 vendor EVOLUTION	0xdeee	Evolution Robotics
 vendor EMPIA		0xeb1a	eMPIA Technology
 vendor HP2		0xf003	Hewlett Packard
+vendor ELGATO		0x0fd9	Elgato
 
 /*
  * List of known products.  Grouped by vendor.
@@ -1783,6 +1784,9 @@ product EICON DIVA852		0x4905	Diva 852 I
 /* EIZO products */
 product EIZO HUB		0x0000	hub
 product EIZO MONITOR		0x0001	monitor
+
+/* Elgato products */
+product ELGATO HD60		0x006a	Game Capture HD60 S+
 
 /* Elan Microelectronics products */
 product ELAN BARCODE		0x0001	Barcode Scanner
Index: sys/dev/usb/uvideo.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/uvideo.c,v
diff -u -p -r1.236 uvideo.c
--- sys/dev/usb/uvideo.c	16 Jan 2025 22:58:19 -0000	1.236
+++ sys/dev/usb/uvideo.c	5 Feb 2025 21:52:31 -0000
@@ -294,10 +294,11 @@ const struct video_hw_if uvideo_hw_if = 
  * Devices which either fail to declare themselves as UICLASS_VIDEO,
  * or which need firmware uploads or other quirk handling later on.
  */
-#define UVIDEO_FLAG_ISIGHT_STREAM_HEADER	0x1
-#define UVIDEO_FLAG_REATTACH			0x2
-#define UVIDEO_FLAG_VENDOR_CLASS		0x4
-#define UVIDEO_FLAG_NOATTACH			0x8
+#define UVIDEO_FLAG_ISIGHT_STREAM_HEADER	0x01
+#define UVIDEO_FLAG_REATTACH			0x02
+#define UVIDEO_FLAG_VENDOR_CLASS		0x04
+#define UVIDEO_FLAG_NOATTACH			0x08
+#define UVIDEO_FLAG_FORMAT_INDEX_IN_BMHINT	0x10
 const struct uvideo_devs {
 	struct usb_devno	 uv_dev;
 	char			*ucode_name;
@@ -379,6 +380,12 @@ const struct uvideo_devs {
 	    NULL,
 	    UVIDEO_FLAG_NOATTACH
 	},
+	{   /* Has incorrect control response */
+	    { USB_VENDOR_ELGATO, USB_PRODUCT_ELGATO_HD60 },
+	    NULL,
+	    NULL,
+	    UVIDEO_FLAG_FORMAT_INDEX_IN_BMHINT
+	},
 };
 #define uvideo_lookup(v, p) \
 	((const struct uvideo_devs *)usb_lookup(uvideo_devs, v, p))
@@ -1725,6 +1732,21 @@ uvideo_vs_get_probe(struct uvideo_softc 
 		    DEVNAME(sc), usbd_errstr(error));
 		return (USBD_INVAL);
 	}
+
+	if (sc->sc_quirk &&
+	    sc->sc_quirk->flags & UVIDEO_FLAG_FORMAT_INDEX_IN_BMHINT &&
+	    UGETW(pc->bmHint) > 255) {
+		/*
+		 * Some devices such as the Elgato Cam Link 4K or Elgato
+		 * Game Capture HD60 returns an invalid bmHint response,
+		 * which contains the bFormatIndex in the second byte.
+		 * But it should be always zero since it's a reserved
+		 * bit field.
+		 */
+		pc->bFormatIndex = UGETW(pc->bmHint) >> 8;
+		USETW(pc->bmHint, 1);
+	}
+
 	DPRINTF(1, "%s: GET probe request successfully\n", DEVNAME(sc));
 
 	DPRINTF(1, "bmHint=0x%02x\n", UGETW(pc->bmHint));



-- 
wbr, Kirill