| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANiDSCs9p=JkGs8w8_n8oPkYg5J3rQFF2RT1bbdme_sNKuzwmg@mail.gmail.com>
Date: Fri, 21 Mar 2025 10:43:52 +0100
From: Ricardo Ribalda <ribalda@...omium.org>
To: chenchangcheng <ccc194101@....com>
Cc: laurent.pinchart@...asonboard.com, hdegoede@...hat.com, mchehab@...nel.org,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
chenchangcheng <chenchangcheng@...inos.cn>, kernel test robot <lkp@...el.com>
Subject: Re: [PATCH v5] media: uvcvideo: Fix bandwidth issue for Alcor camera
Hi Chenchangcheng
On Fri, 21 Mar 2025 at 02:51, chenchangcheng <ccc194101@....com> wrote:
>
> From: chenchangcheng <chenchangcheng@...inos.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@...el.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@...inos.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
>
>
--
Ricardo Ribalda
Powered by blists - more mailing lists