On Mon, 6 Jan 2025 at 15:09, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > From: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > > This device: > 534d:2109 MacroSilicon > > Announces that it supports several frame intervals for > their resolutions for MJPEG compression: > > VideoStreaming Interface Descriptor: > bLength 46 > bDescriptorType 36 > bDescriptorSubtype 7 (FRAME_MJPEG) > bFrameIndex 1 > bmCapabilities 0x00 > Still image unsupported > wWidth 1920 > wHeight 1080 > dwMinBitRate 768000 > dwMaxBitRate 196608000 > dwMaxVideoFrameBufferSize 4147200 > dwDefaultFrameInterval 166666 > bFrameIntervalType 5 > dwFrameInterval( 0) 166666 > dwFrameInterval( 1) 333333 > dwFrameInterval( 2) 400000 > dwFrameInterval( 3) 500000 > dwFrameInterval( 4) 1000000 > > However, the highest frame interval (166666), which means 60 fps > is not supported. If set to it, URB packages will be dropped, > causing JPEG decoding errors and part of the video frames will > be missed. > > Also, as specified at the device's documentation, for such > resolution, the maximum interval is 30 fps (interval == 333333). > > The last format that supports such frame interval is 1280x720. > > Add a quirk to estimate a raw bandwidth, by doing: > width * height * framerate > E. g.: > 1920 * 1080 * 30 = 62208000 > > if the bandwidth is greater than such threshold, get > the next value from the dwFrameInterval. > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx> > Link: https://lore.kernel.org/r/bca0634691cea503c2c5df62b98ba66f0c85bf85.1624350539.git.mchehab+huawei@xxxxxxxxxx > [hdegoede@xxxxxxxxxx add "pixels per second" comment, use UVC_FIVAL_DENOM] > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > drivers/media/usb/uvc/uvc_driver.c | 14 ++++++++++++++ > drivers/media/usb/uvc/uvc_video.c | 29 ++++++++++++++++++++++++++--- > drivers/media/usb/uvc/uvcvideo.h | 1 + > 3 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index f754640fddf5..6d001d4f0902 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -2491,6 +2491,11 @@ static const struct uvc_device_info uvc_quirk_fix_bandwidth = { > .quirks = UVC_QUIRK_FIX_BANDWIDTH, > }; > > +static const struct uvc_device_info uvc_quirk_fix_bw_622 = { > + .quirks = UVC_QUIRK_FIX_BANDWIDTH, > + .max_bandwidth = 62208000, > +}; > + > static const struct uvc_device_info uvc_quirk_probe_def = { > .quirks = UVC_QUIRK_PROBE_DEF, > }; > @@ -2794,6 +2799,15 @@ static const struct usb_device_id uvc_ids[] = { > .bInterfaceSubClass = 1, > .bInterfaceProtocol = 0, > .driver_info = (kernel_ulong_t)&uvc_quirk_fix_bandwidth }, > + /* MacroSilicon HDMI capture */ > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE > + | USB_DEVICE_ID_MATCH_INT_INFO, > + .idVendor = 0x534d, > + .idProduct = 0x2109, > + .bInterfaceClass = USB_CLASS_VIDEO, > + .bInterfaceSubClass = 1, > + .bInterfaceProtocol = 0, > + .driver_info = (kernel_ulong_t)&uvc_quirk_fix_bw_622 }, > /* Genesys Logic USB 2.0 PC Camera */ > { .match_flags = USB_DEVICE_ID_MATCH_DEVICE > | USB_DEVICE_ID_MATCH_INT_INFO, > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index 41cb1b45fa23..af5233ab667e 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -225,9 +225,32 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > - if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > - stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > - stream->intf->num_altsetting > 1) { > + if (!(stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH)) > + return; > + > + /* Handle UVC_QUIRK_FIX_BANDWIDTH */ > + Shouldn't this quirk be better implemented in uvc_try_frame_interval() ? > + if (format->flags & UVC_FMT_FLAG_COMPRESSED) { > + u32 bandwidth; > + > + if (!stream->dev->info->max_bandwidth || !frame->bFrameIntervalType) > + return; > + > + for (i = 0; i < frame->bFrameIntervalType; ++i) { > + bandwidth = frame->wWidth * frame->wHeight; > + bandwidth *= UVC_FIVAL_DENOM / frame->dwFrameInterval[i]; frame->dwFrameInterval[i] comes from the device, it is not sanitized. We need to make sure it is not zero. > + > + if (bandwidth <= stream->dev->info->max_bandwidth) > + break; > + } > + > + if (ctrl->dwFrameInterval < frame->dwFrameInterval[i]) > + ctrl->dwFrameInterval = frame->dwFrameInterval[i]; > + > + return; > + } > + > + if (stream->intf->num_altsetting > 1) { > u32 interval; > u32 bandwidth; > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 966ff82c2ba8..6b702219173f 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -535,6 +535,7 @@ static inline u32 uvc_urb_index(const struct uvc_urb *uvc_urb) > > struct uvc_device_info { > u32 quirks; > + u32 max_bandwidth; /* In pixels per second */ > u32 meta_format; > u16 uvc_version; > }; > -- > 2.47.1 > > -- Ricardo Ribalda
