| Message ID | 20250321014904.129768-1-ccc194101@163.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v5] media: uvcvideo: Fix bandwidth issue for Alcor camera | expand |
Hi Chenchangcheng On Fri, 21 Mar 2025 at 02:51, chenchangcheng <ccc194101@163.com> wrote: > > From: chenchangcheng <chenchangcheng@kylinos.cn> > > Some broken device return wrong dwMaxPayloadTransferSize fields, > as follows: > [ 218.211425] [pid:20391,cpu4,guvcview,3]uvcvideo: Trying format 0x47504a4d (MJPG): 640x480. > [ 218.211425] [pid:20391,cpu4,guvcview,4]uvcvideo: Using default frame interval 33333.3 us (30.0 fps). > [ 218.252532] [pid:20391,cpu4,guvcview,1]uvcvideo: Trying format 0x47504a4d (MJPG): 640x480. > [ 218.252532] [pid:20391,cpu4,guvcview,2]uvcvideo: Using default frame interval 33333.3 us (30.0 fps). > [ 218.293426] [pid:20391,cpu7,guvcview,8]videobuf2_common: __setup_offsets: buffer 0, plane 0 offset 0x00000000 > [ 218.294067] [pid:20391,cpu7,guvcview,9]videobuf2_common: __setup_offsets: buffer 1, plane 0 offset 0x000e1000 > [ 218.294433] [pid:20391,cpu7,guvcview,0]videobuf2_common: __setup_offsets: buffer 2, plane 0 offset 0x001c2000 > [ 218.294677] [pid:20391,cpu7,guvcview,1]videobuf2_common: __setup_offsets: buffer 3, plane 0 offset 0x002a3000 > [ 218.294677] [pid:20391,cpu7,guvcview,2]videobuf2_common: __vb2_queue_alloc: allocated 4 buffers, 1 plane(s) each > [ 218.294738] [pid:20391,cpu7,guvcview,3]uvcvideo: uvc_v4l2_mmap > [ 218.294799] [pid:20391,cpu7,guvcview,4]videobuf2_common: vb2_mmap: buffer 0, plane 0 successfully mapped > [ 218.294799] [pid:20391,cpu7,guvcview,5]uvcvideo: uvc_v4l2_mmap > [ 218.294830] [pid:20391,cpu7,guvcview,6]videobuf2_common: vb2_mmap: buffer 1, plane 0 successfully mapped > [ 218.294830] [pid:20391,cpu7,guvcview,7]uvcvideo: uvc_v4l2_mmap > [ 218.294830] [pid:20391,cpu7,guvcview,8]videobuf2_common: vb2_mmap: buffer 2, plane 0 successfully mapped > [ 218.294860] [pid:20391,cpu7,guvcview,9]uvcvideo: uvc_v4l2_mmap > [ 218.294860] [pid:20391,cpu7,guvcview,0]videobuf2_common: vb2_mmap: buffer 3, plane 0 successfully mapped > [ 218.294860] [pid:20391,cpu7,guvcview,1]videobuf2_common: vb2_core_qbuf: qbuf of buffer 0 succeeded > [ 218.294891] [pid:20391,cpu7,guvcview,2]videobuf2_common: vb2_core_qbuf: qbuf of buffer 1 succeeded > [ 218.294891] [pid:20391,cpu7,guvcview,3]videobuf2_common: vb2_core_qbuf: qbuf of buffer 2 succeeded > [ 218.294891] [pid:20391,cpu7,guvcview,4]videobuf2_common: vb2_core_qbuf: qbuf of buffer 3 succeeded > [ 218.294891] [pid:20391,cpu7,guvcview,5]uvcvideo: Setting frame interval to 1/25 (400000). > [ 218.632537] [pid:20427,cpu6,guvcview,8]uvcvideo: Device requested 2752512 B/frame bandwidth. > [ 218.632598] [pid:20427,cpu6,guvcview,9]uvcvideo: No fast enough alt setting for requested bandwidth. You can copy the whole log messages in the cover letter. I think the commit message should only contain: uvcvideo: Device requested 2752512 B/frame bandwidth. uvcvideo: No fast enough alt setting for requested bandwidth. Please also add the out of lsusb -v -d 1b17:6684 to the cover letter > > The maximum packet size of the device is 3 * 1024, according to the > logs above, the device needs to apply for a bandwidth of 0x2a0000. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202503191330.AveQs7tb-lkp@intel.com/ The code has not landed, please remove the previous two lines. > Signed-off-by: chenchangcheng <chenchangcheng@kylinos.cn> > --- > drivers/media/usb/uvc/uvc_driver.c | 9 ++++ > drivers/media/usb/uvc/uvc_video.c | 82 +++++++++++++++++------------- > drivers/media/usb/uvc/uvcvideo.h | 1 + > 3 files changed, 58 insertions(+), 34 deletions(-) > > diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c > index deadbcea5e22..6d739c3cc88f 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -3188,6 +3188,15 @@ static const struct usb_device_id uvc_ids[] = { > .bInterfaceSubClass = 1, > .bInterfaceProtocol = 0, > .driver_info = UVC_INFO_META(V4L2_META_FMT_D4XX) }, > + /* Alcor Corp. Slave camera */ > + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE > + | USB_DEVICE_ID_MATCH_INT_INFO, > + .idVendor = 0x1b17, > + .idProduct = 0x6684, > + .bInterfaceClass = USB_CLASS_VIDEO, > + .bInterfaceSubClass = 1, > + .bInterfaceProtocol = 0, > + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_OVERFLOW_BANDWIDTH) }, The quirks are ordered by vid:pid. Please move this element to its correct position > /* Generic USB Video Class */ > { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) }, > { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) }, > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index e3567aeb0007..7af77bf1be9b 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -226,41 +226,55 @@ 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) { > - u32 interval; > - u32 bandwidth; > - > - interval = (ctrl->dwFrameInterval > 100000) > - ? ctrl->dwFrameInterval > - : frame->dwFrameInterval[0]; > - > - /* > - * Compute a bandwidth estimation by multiplying the frame > - * size by the number of video frames per second, divide the > - * result by the number of USB frames (or micro-frames for > - * high- and super-speed devices) per second and add the UVC > - * header size (assumed to be 12 bytes long). > - */ > - bandwidth = frame->wWidth * frame->wHeight / 8 * format->bpp; > - bandwidth *= 10000000 / interval + 1; > - bandwidth /= 1000; > - if (stream->dev->udev->speed >= USB_SPEED_HIGH) > - bandwidth /= 8; > - bandwidth += 12; > - > - /* > - * The bandwidth estimate is too low for many cameras. Don't use > - * maximum packet sizes lower than 1024 bytes to try and work > - * around the problem. According to measurements done on two > - * different camera models, the value is high enough to get most > - * resolutions working while not preventing two simultaneous > - * VGA streams at 15 fps. > - */ > - bandwidth = max_t(u32, bandwidth, 1024); > + if (stream->intf->num_altsetting > 1) { > + if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > + stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH) { > + u32 interval; > + u32 bandwidth; > + > + interval = (ctrl->dwFrameInterval > 100000) > + ? ctrl->dwFrameInterval > + : frame->dwFrameInterval[0]; > + > + /* > + * Compute a bandwidth estimation by multiplying the > + * frame size by the number of video frames per second, > + * divide the result by the number of USB frames (or > + * micro-frames for high- and super-speed devices) per > + * second and add the UVC header size (assumed to be > + * 12 bytes long). > + */ > + bandwidth = frame->wWidth * frame->wHeight / 8 > + * format->bpp; > + bandwidth *= 10000000 / interval + 1; > + bandwidth /= 1000; > + if (stream->dev->udev->speed >= USB_SPEED_HIGH) > + bandwidth /= 8; > + bandwidth += 12; > + > + /* > + * The bandwidth estimate is too low for many cameras. > + * Don't use maximum packet sizes lower than 1024 bytes > + * to try and work around the problem. According to > + * measurements done on two different camera models, > + * the value is high enough to get most resolutions > + * working while not preventing two simultaneous VGA > + * streams at 15 fps. > + */ > + bandwidth = max_t(u32, bandwidth, 1024); > + > + ctrl->dwMaxPayloadTransferSize = bandwidth; > + } > > - ctrl->dwMaxPayloadTransferSize = bandwidth; > + if (format->flags & UVC_FMT_FLAG_COMPRESSED && Why only for compressed formats? > + stream->dev->quirks & UVC_QUIRK_OVERFLOW_BANDWIDTH && > + ctrl->dwMaxPayloadTransferSize > stream->maxpsize) { > + dev_warn(&stream->intf->dev, > + "the max payload transmission size (%d) exceededs the size of the ep max packet (%d). use the default value of 1024 bytes.\n", exceeds The max . Use > + ctrl->dwMaxPayloadTransferSize, > + stream->maxpsize); > + ctrl->dwMaxPayloadTransferSize = 1024; Why did you decided 1024 to be the correct transfer Size? Wouldn;t it be more generic to use stream->maxpsize > + } Maybe a bit nicer: diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index e3567aeb0007..b50e6a4048f0 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -262,6 +262,16 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, ctrl->dwMaxPayloadTransferSize = bandwidth; } + + /* Proper comment.... bla bla bla */ + if (stream->intf->num_altsetting > 1 && + ctrl->dwMaxPayloadTransferSize > stream->maxpsize && + stream->dev->quirks & UVC_QUIRK_OVERFLOW_BANDWIDTH) { + dev_warn(&stream->intf->dev, + "the max payload transmission size (%d) exceeds the size of the ep max packet (%d). Using the max size.\n", + ctrl->dwMaxPayloadTransferSize, stream->maxpsize); + ctrl->dwMaxPayloadTransferSize = stream->maxpsize; + } } > } > } > > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h > index 5e388f05f3fc..8b43d725c259 100644 > --- a/drivers/media/usb/uvc/uvcvideo.h > +++ b/drivers/media/usb/uvc/uvcvideo.h > @@ -77,6 +77,7 @@ > #define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00008000 > #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000 > #define UVC_QUIRK_MJPEG_NO_EOF 0x00020000 > +#define UVC_QUIRK_OVERFLOW_BANDWIDTH 0x00040000 > > /* Format flags */ > #define UVC_FMT_FLAG_COMPRESSED 0x00000001 > > base-commit: 4701f33a10702d5fc577c32434eb62adde0a1ae1 > -- > 2.25.1 > >
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index deadbcea5e22..6d739c3cc88f 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -3188,6 +3188,15 @@ static const struct usb_device_id uvc_ids[] = { .bInterfaceSubClass = 1, .bInterfaceProtocol = 0, .driver_info = UVC_INFO_META(V4L2_META_FMT_D4XX) }, + /* Alcor Corp. Slave camera */ + { .match_flags = USB_DEVICE_ID_MATCH_DEVICE + | USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = 0x1b17, + .idProduct = 0x6684, + .bInterfaceClass = USB_CLASS_VIDEO, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 0, + .driver_info = UVC_INFO_QUIRK(UVC_QUIRK_OVERFLOW_BANDWIDTH) }, /* Generic USB Video Class */ { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_UNDEFINED) }, { USB_INTERFACE_INFO(USB_CLASS_VIDEO, 1, UVC_PC_PROTOCOL_15) }, diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index e3567aeb0007..7af77bf1be9b 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -226,41 +226,55 @@ 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) { - u32 interval; - u32 bandwidth; - - interval = (ctrl->dwFrameInterval > 100000) - ? ctrl->dwFrameInterval - : frame->dwFrameInterval[0]; - - /* - * Compute a bandwidth estimation by multiplying the frame - * size by the number of video frames per second, divide the - * result by the number of USB frames (or micro-frames for - * high- and super-speed devices) per second and add the UVC - * header size (assumed to be 12 bytes long). - */ - bandwidth = frame->wWidth * frame->wHeight / 8 * format->bpp; - bandwidth *= 10000000 / interval + 1; - bandwidth /= 1000; - if (stream->dev->udev->speed >= USB_SPEED_HIGH) - bandwidth /= 8; - bandwidth += 12; - - /* - * The bandwidth estimate is too low for many cameras. Don't use - * maximum packet sizes lower than 1024 bytes to try and work - * around the problem. According to measurements done on two - * different camera models, the value is high enough to get most - * resolutions working while not preventing two simultaneous - * VGA streams at 15 fps. - */ - bandwidth = max_t(u32, bandwidth, 1024); + if (stream->intf->num_altsetting > 1) { + if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && + stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH) { + u32 interval; + u32 bandwidth; + + interval = (ctrl->dwFrameInterval > 100000) + ? ctrl->dwFrameInterval + : frame->dwFrameInterval[0]; + + /* + * Compute a bandwidth estimation by multiplying the + * frame size by the number of video frames per second, + * divide the result by the number of USB frames (or + * micro-frames for high- and super-speed devices) per + * second and add the UVC header size (assumed to be + * 12 bytes long). + */ + bandwidth = frame->wWidth * frame->wHeight / 8 + * format->bpp; + bandwidth *= 10000000 / interval + 1; + bandwidth /= 1000; + if (stream->dev->udev->speed >= USB_SPEED_HIGH) + bandwidth /= 8; + bandwidth += 12; + + /* + * The bandwidth estimate is too low for many cameras. + * Don't use maximum packet sizes lower than 1024 bytes + * to try and work around the problem. According to + * measurements done on two different camera models, + * the value is high enough to get most resolutions + * working while not preventing two simultaneous VGA + * streams at 15 fps. + */ + bandwidth = max_t(u32, bandwidth, 1024); + + ctrl->dwMaxPayloadTransferSize = bandwidth; + } - ctrl->dwMaxPayloadTransferSize = bandwidth; + if (format->flags & UVC_FMT_FLAG_COMPRESSED && + stream->dev->quirks & UVC_QUIRK_OVERFLOW_BANDWIDTH && + ctrl->dwMaxPayloadTransferSize > stream->maxpsize) { + dev_warn(&stream->intf->dev, + "the max payload transmission size (%d) exceededs the size of the ep max packet (%d). use the default value of 1024 bytes.\n", + ctrl->dwMaxPayloadTransferSize, + stream->maxpsize); + ctrl->dwMaxPayloadTransferSize = 1024; + } } } diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h index 5e388f05f3fc..8b43d725c259 100644 --- a/drivers/media/usb/uvc/uvcvideo.h +++ b/drivers/media/usb/uvc/uvcvideo.h @@ -77,6 +77,7 @@ #define UVC_QUIRK_DISABLE_AUTOSUSPEND 0x00008000 #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000 #define UVC_QUIRK_MJPEG_NO_EOF 0x00020000 +#define UVC_QUIRK_OVERFLOW_BANDWIDTH 0x00040000 /* Format flags */ #define UVC_FMT_FLAG_COMPRESSED 0x00000001