Hi Laurent
Just minor nipitks.
On Mon, 16 Jan 2023 at 17:04, Laurent Pinchart
<laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>
> The size of the video probe & commit controls depends on the UVC
> protocol version. Some devices, namely the Quanta ACER HD User Facing
> (0408:4033 and 0408:4035), claim to support the UVC 1.5 protocol, but
> return only 26 bytes of data when queried with GET_CUR for those
> controls. This causes probing of the device to fail.
>
> Work around this non-compliance by lowering the size of the control when
> the device returns less data than expected, but still a valid size for
> one of the existing UVC versions. The size is cached and used for
> subsequent queries of the probe and commit controls.
>
> Reported-by: Giuliano Lotta <giuliano.lotta@xxxxxxxxx>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/usb/uvc/uvc_video.c | 48 +++++++++++++++++++------------
> drivers/media/usb/uvc/uvcvideo.h | 1 +
> include/uapi/linux/usb/video.h | 4 +++
> 3 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
> index 9634596f3dc7..23f3179a013f 100644
> --- a/drivers/media/usb/uvc/uvc_video.c
> +++ b/drivers/media/usb/uvc/uvc_video.c
> @@ -272,24 +272,10 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream,
> }
> }
>
> -static size_t uvc_video_ctrl_size(struct uvc_streaming *stream)
> -{
> - /*
> - * Return the size of the video probe and commit controls, which depends
> - * on the protocol version.
> - */
> - if (stream->dev->uvc_version < 0x0110)
> - return 26;
> - else if (stream->dev->uvc_version < 0x0150)
> - return 34;
> - else
> - return 48;
> -}
> -
> static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> struct uvc_streaming_control *ctrl, int probe, u8 query)
> {
> - u16 size = uvc_video_ctrl_size(stream);
> + u16 size = stream->ctrl_size;
> u8 *data;
> int ret;
>
> @@ -329,6 +315,20 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> "Enabling workaround.\n");
> ret = -EIO;
> goto out;
> + } else if (ret < size &&
what about (ret != size) {
switch (ret) {
case UVC_STREAMING_CONTROL_SIZE_V1_0:
case UVC_STREAMING_CONTROL_SIZE_V1_1:
case UVC_STREAMING_CONTROL_SIZE_V1_5:
printk(stream at vendor)
stream->ctrl_size = ret;
size = ret;
break;
default:
printk(fatal error)
return -EIO;
break;
}
}
> + (ret == UVC_STREAMING_CONTROL_SIZE_V1_0 ||
> + ret == UVC_STREAMING_CONTROL_SIZE_V1_1)) {
> + /*
> + * Some Quanta cameras (for instance 0408:4033 and 0408:4035)
> + * advertise UVC 1.5 compliance but only returns 26 bytes of
> + * data for the video probe and commit controls.
> + */
> + dev_warn(&stream->intf->dev,
> + "UVC non compliance: video control size %u < %u as required by UVC v%u.%02x. Enabling workaround.\n",
> + ret, size, stream->dev->uvc_version >> 8,
> + stream->dev->uvc_version & 0xff);
> + stream->ctrl_size = ret;
> + size = ret;
> } else if (ret != size) {
> dev_err(&stream->intf->dev,
> "Failed to query (%u) UVC %s control : %d (exp. %u).\n",
> @@ -349,7 +349,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> ctrl->dwMaxVideoFrameSize = get_unaligned_le32(&data[18]);
> ctrl->dwMaxPayloadTransferSize = get_unaligned_le32(&data[22]);
>
> - if (size >= 34) {
> + if (size >= UVC_STREAMING_CONTROL_SIZE_V1_1) {
> ctrl->dwClockFrequency = get_unaligned_le32(&data[26]);
> ctrl->bmFramingInfo = data[30];
> ctrl->bPreferedVersion = data[31];
> @@ -379,7 +379,7 @@ static int uvc_get_video_ctrl(struct uvc_streaming *stream,
> static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> struct uvc_streaming_control *ctrl, int probe)
> {
> - u16 size = uvc_video_ctrl_size(stream);
> + u16 size = stream->ctrl_size;
> u8 *data;
> int ret;
>
> @@ -399,7 +399,7 @@ static int uvc_set_video_ctrl(struct uvc_streaming *stream,
> put_unaligned_le32(ctrl->dwMaxVideoFrameSize, &data[18]);
> put_unaligned_le32(ctrl->dwMaxPayloadTransferSize, &data[22]);
>
> - if (size >= 34) {
> + if (size >= UVC_STREAMING_CONTROL_SIZE_V1_1) {
> put_unaligned_le32(ctrl->dwClockFrequency, &data[26]);
> data[30] = ctrl->bmFramingInfo;
> data[31] = ctrl->bPreferedVersion;
> @@ -2148,6 +2148,18 @@ int uvc_video_init(struct uvc_streaming *stream)
>
> atomic_set(&stream->active, 0);
>
> + /*
> + * Set the probe & commit control size based on the UVC protocol
> + * version. It may be overridden by uvc_get_video_ctrl() for devices
> + * that don't comply with the UVC version they report.
> + */
> + if (stream->dev->uvc_version < 0x0110)
> + stream->ctrl_size = UVC_STREAMING_CONTROL_SIZE_V1_0;
> + else if (stream->dev->uvc_version < 0x0150)
> + stream->ctrl_size = UVC_STREAMING_CONTROL_SIZE_V1_1;
> + else
> + stream->ctrl_size = UVC_STREAMING_CONTROL_SIZE_V1_5;
> +
> /*
> * Alternate setting 0 should be the default, yet the XBox Live Vision
> * Cam (and possibly other devices) crash or otherwise misbehave if
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 72189249719e..151d8d27cbbc 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -441,6 +441,7 @@ struct uvc_streaming {
> struct uvc_format *format;
>
> struct uvc_streaming_control ctrl;
> + unsigned int ctrl_size;
> struct uvc_format *def_format;
> struct uvc_format *cur_format;
> struct uvc_frame *cur_frame;
> diff --git a/include/uapi/linux/usb/video.h b/include/uapi/linux/usb/video.h
> index 6e8e572c2980..7cb7713db757 100644
> --- a/include/uapi/linux/usb/video.h
> +++ b/include/uapi/linux/usb/video.h
> @@ -454,6 +454,10 @@ struct uvc_streaming_control {
> __u8 bMaxVersion;
> } __attribute__((__packed__));
>
> +#define UVC_STREAMING_CONTROL_SIZE_V1_0 26
> +#define UVC_STREAMING_CONTROL_SIZE_V1_1 34
> +#define UVC_STREAMING_CONTROL_SIZE_V1_5 48
Maybe a future improvement, convert this to a nice structure
> +
> /* Uncompressed Payload - 3.1.1. Uncompressed Video Format Descriptor */
> struct uvc_format_uncompressed {
> __u8 bLength;
> --
> Regards,
>
> Laurent Pinchart
>
--
Ricardo Ribalda
