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

On Wed, Feb 05, 2025 at 10:56:35PM GMT, Kirill A. Korinsky wrote:

> 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.

Thanks.

ok mglocker@
 
> 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
>