* [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera @ 2023-01-15 21:17 Laurent Pinchart 2023-01-16 14:14 ` Ricardo Ribalda 2023-01-17 14:08 ` Ricardo Ribalda 0 siblings, 2 replies; 6+ messages in thread From: Laurent Pinchart @ 2023-01-15 21:17 UTC (permalink / raw) To: linux-media; +Cc: Ai Chao, Jackie Liu, Ricardo Ribalda From: Ai Chao <aichao@kylinos.cn> The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong dwMaxPayloadTransferSize value for compressed formats. Valid values are typically up to 3072 bytes per interval (for high-speed, high-bandwidth devices), and those faulty devices request 2752512 bytes per interval. This is a firmware issue, but the manufacturer cannot provide a fixed firmware. Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding a value of 1024 if it exceeds 3072 for compressed formats transferred over isochronous endpoints. While at it, document the other quirk that handles a bandwidth issue for uncompressed formats. Signed-off-by: Ai Chao <aichao@kylinos.cn> --- I have dropped the Reviewed-by tags as the patch has changed significantly. Ricardo, do you know if the 3072 bytes limit is fine with super-speed devices, or does it need to be increased ? --- drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c index d4b023d4de7c..c6351d3b24cf 100644 --- a/drivers/media/usb/uvc/uvc_video.c +++ b/drivers/media/usb/uvc/uvc_video.c @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; + /* + * Many devices report an incorrect dwMaxPayloadTransferSize value. The + * most common issue is devices requesting the maximum possible USB + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth + * isochronous endpoints) while they actually require less, preventing + * multiple cameras from being used at the same time due to bandwidth + * overallocation. + * + * For those devices, replace the dwMaxPayloadTransferSize value based + * on an estimation calculated from the frame format and size. This is + * only possible for uncompressed formats, as not enough information is + * available to reliably estimate the bandwidth requirements for + * compressed formats. + */ if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && stream->intf->num_altsetting > 1) { @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, ctrl->dwMaxPayloadTransferSize = bandwidth; } + + /* + * Another issue is with devices that report a transfer size that + * greatly exceeds the maximum supported by any existing USB version + * for isochronous transfers. For instance, the "Slave camera" devices + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per + * interval. + * + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH + * quirk, but for compressed format we can't meaningfully estimate the + * required bandwidth. Just hardcode it to 1024 bytes per interval, + * which should be large enough for compressed formats. + */ + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && + ctrl->dwMaxPayloadTransferSize > 3072 && + stream->intf->num_altsetting > 1) + ctrl->dwMaxPayloadTransferSize = 1024; } static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera 2023-01-15 21:17 [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera Laurent Pinchart @ 2023-01-16 14:14 ` Ricardo Ribalda 2023-01-16 14:40 ` Laurent Pinchart 2023-01-17 14:08 ` Ricardo Ribalda 1 sibling, 1 reply; 6+ messages in thread From: Ricardo Ribalda @ 2023-01-16 14:14 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, Ai Chao, Jackie Liu Hi Laurent On Sun, 15 Jan 2023 at 22:17, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > From: Ai Chao <aichao@kylinos.cn> > > The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong > dwMaxPayloadTransferSize value for compressed formats. Valid values are > typically up to 3072 bytes per interval (for high-speed, high-bandwidth > devices), and those faulty devices request 2752512 bytes per interval. > This is a firmware issue, but the manufacturer cannot provide a fixed > firmware. > > Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding > a value of 1024 if it exceeds 3072 for compressed formats transferred > over isochronous endpoints. While at it, document the other quirk that > handles a bandwidth issue for uncompressed formats. > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > --- > I have dropped the Reviewed-by tags as the patch has changed > significantly. > > Ricardo, do you know if the 3072 bytes limit is fine with super-speed > devices, or does it need to be increased ? We have enough documentation to let ChatGPT make the code for us :) I am going to try tonight on two superspeed cameras. Will let you know tomorow. > --- > drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index d4b023d4de7c..c6351d3b24cf 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > + /* > + * Many devices report an incorrect dwMaxPayloadTransferSize value. The > + * most common issue is devices requesting the maximum possible USB > + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth > + * isochronous endpoints) while they actually require less, preventing > + * multiple cameras from being used at the same time due to bandwidth > + * overallocation. > + * > + * For those devices, replace the dwMaxPayloadTransferSize value based > + * on an estimation calculated from the frame format and size. This is > + * only possible for uncompressed formats, as not enough information is > + * available to reliably estimate the bandwidth requirements for > + * compressed formats. > + */ > if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > stream->intf->num_altsetting > 1) { > @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > ctrl->dwMaxPayloadTransferSize = bandwidth; > } > + > + /* > + * Another issue is with devices that report a transfer size that > + * greatly exceeds the maximum supported by any existing USB version > + * for isochronous transfers. For instance, the "Slave camera" devices > + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per > + * interval. > + * > + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH > + * quirk, but for compressed format we can't meaningfully estimate the > + * required bandwidth. Just hardcode it to 1024 bytes per interval, > + * which should be large enough for compressed formats. > + */ > + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && > + ctrl->dwMaxPayloadTransferSize > 3072 && > + stream->intf->num_altsetting > 1) > + ctrl->dwMaxPayloadTransferSize = 1024; - Maybe we should add a debug message if we are doing this? - Also I do not like that the value that we use as trigger (3072) is different than the quirked value (1024) Something like: value = min(3072, value) makes more sense Thanks! > } > > static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) > -- > Regards, > > Laurent Pinchart > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera 2023-01-16 14:14 ` Ricardo Ribalda @ 2023-01-16 14:40 ` Laurent Pinchart 0 siblings, 0 replies; 6+ messages in thread From: Laurent Pinchart @ 2023-01-16 14:40 UTC (permalink / raw) To: Ricardo Ribalda; +Cc: linux-media, Ai Chao, Jackie Liu Hi Ricardo, On Mon, Jan 16, 2023 at 03:14:39PM +0100, Ricardo Ribalda wrote: > On Sun, 15 Jan 2023 at 22:17, Laurent Pinchart wrote: > > > > From: Ai Chao <aichao@kylinos.cn> > > > > The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong > > dwMaxPayloadTransferSize value for compressed formats. Valid values are > > typically up to 3072 bytes per interval (for high-speed, high-bandwidth > > devices), and those faulty devices request 2752512 bytes per interval. > > This is a firmware issue, but the manufacturer cannot provide a fixed > > firmware. > > > > Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding > > a value of 1024 if it exceeds 3072 for compressed formats transferred > > over isochronous endpoints. While at it, document the other quirk that > > handles a bandwidth issue for uncompressed formats. > > > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > > --- > > I have dropped the Reviewed-by tags as the patch has changed > > significantly. > > > > Ricardo, do you know if the 3072 bytes limit is fine with super-speed > > devices, or does it need to be increased ? > > We have enough documentation to let ChatGPT make the code for us :) It will still need some improvements :-) > I am going to try tonight on two superspeed cameras. Will let you know tomorow. > > > --- > > drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index d4b023d4de7c..c6351d3b24cf 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > > > + /* > > + * Many devices report an incorrect dwMaxPayloadTransferSize value. The > > + * most common issue is devices requesting the maximum possible USB > > + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth > > + * isochronous endpoints) while they actually require less, preventing > > + * multiple cameras from being used at the same time due to bandwidth > > + * overallocation. > > + * > > + * For those devices, replace the dwMaxPayloadTransferSize value based > > + * on an estimation calculated from the frame format and size. This is > > + * only possible for uncompressed formats, as not enough information is > > + * available to reliably estimate the bandwidth requirements for > > + * compressed formats. > > + */ > > if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > > stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > > stream->intf->num_altsetting > 1) { > > @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > > > ctrl->dwMaxPayloadTransferSize = bandwidth; > > } > > + > > + /* > > + * Another issue is with devices that report a transfer size that > > + * greatly exceeds the maximum supported by any existing USB version > > + * for isochronous transfers. For instance, the "Slave camera" devices > > + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per > > + * interval. > > + * > > + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH > > + * quirk, but for compressed format we can't meaningfully estimate the > > + * required bandwidth. Just hardcode it to 1024 bytes per interval, > > + * which should be large enough for compressed formats. > > + */ > > + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && > > + ctrl->dwMaxPayloadTransferSize > 3072 && > > + stream->intf->num_altsetting > 1) > > + ctrl->dwMaxPayloadTransferSize = 1024; > > - Maybe we should add a debug message if we are doing this? I can do that. > - Also I do not like that the value that we use as trigger (3072) is > different than the quirked value (1024) > > Something like: > > value = min(3072, value) > > makes more sense That would prevent two such devices from working in parallel, when they likely could as 1024 bytes should be enough. That's the reason for the UVC_QUIRK_FIX_BANDWIDTH for uncompressed formats. > > } > > > > static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera 2023-01-15 21:17 [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera Laurent Pinchart 2023-01-16 14:14 ` Ricardo Ribalda @ 2023-01-17 14:08 ` Ricardo Ribalda 2023-01-17 14:18 ` Laurent Pinchart 1 sibling, 1 reply; 6+ messages in thread From: Ricardo Ribalda @ 2023-01-17 14:08 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, Ai Chao, Jackie Liu Hi Laurent On Sun, 15 Jan 2023 at 22:17, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > From: Ai Chao <aichao@kylinos.cn> > > The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong > dwMaxPayloadTransferSize value for compressed formats. Valid values are > typically up to 3072 bytes per interval (for high-speed, high-bandwidth > devices), and those faulty devices request 2752512 bytes per interval. > This is a firmware issue, but the manufacturer cannot provide a fixed > firmware. > > Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding > a value of 1024 if it exceeds 3072 for compressed formats transferred > over isochronous endpoints. While at it, document the other quirk that > handles a bandwidth issue for uncompressed formats. > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > --- > I have dropped the Reviewed-by tags as the patch has changed > significantly. > > Ricardo, do you know if the 3072 bytes limit is fine with super-speed > devices, or does it need to be increased ? Tried with a couple of super-speed: If I print: ctrl->dwMaxPayloadTransferSize [ 237.269972] drivers/media/usb/uvc/uvc_video.c:239 bw 3072 [ 175.761041] drivers/media/usb/uvc/uvc_video.c:239 bw 3060 Format YUYV stall when I cap the dwMaxPayloadTransferSize to 1024, but works fine with MJPEG and even NV12 How does it sound to cap dwMaxPayloadTransferSize to 3072 for superspeed and 1024 for high speed? Regards! > --- > drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > index d4b023d4de7c..c6351d3b24cf 100644 > --- a/drivers/media/usb/uvc/uvc_video.c > +++ b/drivers/media/usb/uvc/uvc_video.c > @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > + /* > + * Many devices report an incorrect dwMaxPayloadTransferSize value. The > + * most common issue is devices requesting the maximum possible USB > + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth > + * isochronous endpoints) while they actually require less, preventing > + * multiple cameras from being used at the same time due to bandwidth > + * overallocation. > + * > + * For those devices, replace the dwMaxPayloadTransferSize value based > + * on an estimation calculated from the frame format and size. This is > + * only possible for uncompressed formats, as not enough information is > + * available to reliably estimate the bandwidth requirements for > + * compressed formats. > + */ > if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > stream->intf->num_altsetting > 1) { > @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > ctrl->dwMaxPayloadTransferSize = bandwidth; > } > + > + /* > + * Another issue is with devices that report a transfer size that > + * greatly exceeds the maximum supported by any existing USB version > + * for isochronous transfers. For instance, the "Slave camera" devices > + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per > + * interval. > + * > + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH > + * quirk, but for compressed format we can't meaningfully estimate the > + * required bandwidth. Just hardcode it to 1024 bytes per interval, > + * which should be large enough for compressed formats. > + */ > + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && > + ctrl->dwMaxPayloadTransferSize > 3072 && > + stream->intf->num_altsetting > 1) > + ctrl->dwMaxPayloadTransferSize = 1024; > } > > static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) > -- > Regards, > > Laurent Pinchart > -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera 2023-01-17 14:08 ` Ricardo Ribalda @ 2023-01-17 14:18 ` Laurent Pinchart 2023-01-17 14:38 ` Ricardo Ribalda 0 siblings, 1 reply; 6+ messages in thread From: Laurent Pinchart @ 2023-01-17 14:18 UTC (permalink / raw) To: Ricardo Ribalda; +Cc: linux-media, Ai Chao, Jackie Liu Hi Ricardo, On Tue, Jan 17, 2023 at 03:08:07PM +0100, Ricardo Ribalda wrote: > On Sun, 15 Jan 2023 at 22:17, Laurent Pinchart wrote: > > > > From: Ai Chao <aichao@kylinos.cn> > > > > The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong > > dwMaxPayloadTransferSize value for compressed formats. Valid values are > > typically up to 3072 bytes per interval (for high-speed, high-bandwidth > > devices), and those faulty devices request 2752512 bytes per interval. > > This is a firmware issue, but the manufacturer cannot provide a fixed > > firmware. > > > > Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding > > a value of 1024 if it exceeds 3072 for compressed formats transferred > > over isochronous endpoints. While at it, document the other quirk that > > handles a bandwidth issue for uncompressed formats. > > > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > > --- > > I have dropped the Reviewed-by tags as the patch has changed > > significantly. > > > > Ricardo, do you know if the 3072 bytes limit is fine with super-speed > > devices, or does it need to be increased ? > Tried with a couple of super-speed: > > If I print: ctrl->dwMaxPayloadTransferSize > > [ 237.269972] drivers/media/usb/uvc/uvc_video.c:239 bw 3072 > [ 175.761041] drivers/media/usb/uvc/uvc_video.c:239 bw 3060 > > Format YUYV stall when I cap the dwMaxPayloadTransferSize to 1024, but > works fine with MJPEG and even NV12 > > How does it sound to cap dwMaxPayloadTransferSize to 3072 for > superspeed and 1024 for high speed? Won't you still run out of bandwidth when using multiple cameras concurrently ? Is it the interval that is shorter with SS, or the maximum bytes per interval that is larger ? > > --- > > drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > index d4b023d4de7c..c6351d3b24cf 100644 > > --- a/drivers/media/usb/uvc/uvc_video.c > > +++ b/drivers/media/usb/uvc/uvc_video.c > > @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > > > + /* > > + * Many devices report an incorrect dwMaxPayloadTransferSize value. The > > + * most common issue is devices requesting the maximum possible USB > > + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth > > + * isochronous endpoints) while they actually require less, preventing > > + * multiple cameras from being used at the same time due to bandwidth > > + * overallocation. > > + * > > + * For those devices, replace the dwMaxPayloadTransferSize value based > > + * on an estimation calculated from the frame format and size. This is > > + * only possible for uncompressed formats, as not enough information is > > + * available to reliably estimate the bandwidth requirements for > > + * compressed formats. > > + */ > > if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > > stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > > stream->intf->num_altsetting > 1) { > > @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > > > ctrl->dwMaxPayloadTransferSize = bandwidth; > > } > > + > > + /* > > + * Another issue is with devices that report a transfer size that > > + * greatly exceeds the maximum supported by any existing USB version > > + * for isochronous transfers. For instance, the "Slave camera" devices > > + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per > > + * interval. > > + * > > + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH > > + * quirk, but for compressed format we can't meaningfully estimate the > > + * required bandwidth. Just hardcode it to 1024 bytes per interval, > > + * which should be large enough for compressed formats. > > + */ > > + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && > > + ctrl->dwMaxPayloadTransferSize > 3072 && > > + stream->intf->num_altsetting > 1) > > + ctrl->dwMaxPayloadTransferSize = 1024; > > } > > > > static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera 2023-01-17 14:18 ` Laurent Pinchart @ 2023-01-17 14:38 ` Ricardo Ribalda 0 siblings, 0 replies; 6+ messages in thread From: Ricardo Ribalda @ 2023-01-17 14:38 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-media, Ai Chao, Jackie Liu On Tue, 17 Jan 2023 at 15:18, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > Hi Ricardo, > > On Tue, Jan 17, 2023 at 03:08:07PM +0100, Ricardo Ribalda wrote: > > On Sun, 15 Jan 2023 at 22:17, Laurent Pinchart wrote: > > > > > > From: Ai Chao <aichao@kylinos.cn> > > > > > > The Alcor Corp. Slave camera (1b17:6684 and 2017:0011) returns a wrong > > > dwMaxPayloadTransferSize value for compressed formats. Valid values are > > > typically up to 3072 bytes per interval (for high-speed, high-bandwidth > > > devices), and those faulty devices request 2752512 bytes per interval. > > > This is a firmware issue, but the manufacturer cannot provide a fixed > > > firmware. > > > > > > Fix this by checking the dwMaxPayloadTransferSize field, and hardcoding > > > a value of 1024 if it exceeds 3072 for compressed formats transferred > > > over isochronous endpoints. While at it, document the other quirk that > > > handles a bandwidth issue for uncompressed formats. > > > > > > Signed-off-by: Ai Chao <aichao@kylinos.cn> > > > --- > > > I have dropped the Reviewed-by tags as the patch has changed > > > significantly. > > > > > > Ricardo, do you know if the 3072 bytes limit is fine with super-speed > > > devices, or does it need to be increased ? > > Tried with a couple of super-speed: > > > > If I print: ctrl->dwMaxPayloadTransferSize > > > > [ 237.269972] drivers/media/usb/uvc/uvc_video.c:239 bw 3072 > > [ 175.761041] drivers/media/usb/uvc/uvc_video.c:239 bw 3060 > > > > Format YUYV stall when I cap the dwMaxPayloadTransferSize to 1024, but > > works fine with MJPEG and even NV12 > > > > How does it sound to cap dwMaxPayloadTransferSize to 3072 for > > superspeed and 1024 for high speed? > > Won't you still run out of bandwidth when using multiple cameras > concurrently ? Is it the interval that is shorter with SS, or the > maximum bytes per interval that is larger ? From UVC: In the context of the USB Video Class, a Payload Transfer is a unit of data transfer common to bulk and isochronous endpoints. Each Payload Transfer includes a Payload Header followed by Payload Data. For isochronous endpoints, a Payload Transfer is contained in the data transmitted during a single (micro) frame: up to 1023 bytes for a fullspeed endpoint; up to 1024 bytes for a high-speed endpoint; and up to 3072 bytes for a high-speed/high-bandwidth endpoint. For bulk endpoints, a Payload Transfer is contained in the data transmitted in a single bulk transfer (which may consist of multiple bulk data transactions). What are the chances of having multiple broken cameras on the single device VS having a single broken camera? Actually, now that I think about this, maybe we should go back to the previous version where we quirk based on vid:pid, hoping that this bug is just one of. Maybe we can use this logic to something like if payloadTransfer > standard: print(Blame vendor, contact mailing list) :) > > > > --- > > > drivers/media/usb/uvc/uvc_video.c | 31 +++++++++++++++++++++++++++++++ > > > 1 file changed, 31 insertions(+) > > > > > > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c > > > index d4b023d4de7c..c6351d3b24cf 100644 > > > --- a/drivers/media/usb/uvc/uvc_video.c > > > +++ b/drivers/media/usb/uvc/uvc_video.c > > > @@ -200,6 +200,20 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > > if ((ctrl->dwMaxPayloadTransferSize & 0xffff0000) == 0xffff0000) > > > ctrl->dwMaxPayloadTransferSize &= ~0xffff0000; > > > > > > + /* > > > + * Many devices report an incorrect dwMaxPayloadTransferSize value. The > > > + * most common issue is devices requesting the maximum possible USB > > > + * bandwidth (3072 bytes per interval for high-speed, high-bandwidth > > > + * isochronous endpoints) while they actually require less, preventing > > > + * multiple cameras from being used at the same time due to bandwidth > > > + * overallocation. > > > + * > > > + * For those devices, replace the dwMaxPayloadTransferSize value based > > > + * on an estimation calculated from the frame format and size. This is > > > + * only possible for uncompressed formats, as not enough information is > > > + * available to reliably estimate the bandwidth requirements for > > > + * compressed formats. > > > + */ > > > if (!(format->flags & UVC_FMT_FLAG_COMPRESSED) && > > > stream->dev->quirks & UVC_QUIRK_FIX_BANDWIDTH && > > > stream->intf->num_altsetting > 1) { > > > @@ -236,6 +250,23 @@ static void uvc_fixup_video_ctrl(struct uvc_streaming *stream, > > > > > > ctrl->dwMaxPayloadTransferSize = bandwidth; > > > } > > > + > > > + /* > > > + * Another issue is with devices that report a transfer size that > > > + * greatly exceeds the maximum supported by any existing USB version > > > + * for isochronous transfers. For instance, the "Slave camera" devices > > > + * from Alcor Corp. (2017:0011 and 1b17:66B8) request 2752512 bytes per > > > + * interval. > > > + * > > > + * For uncompressed formats, this can be addressed by the FIX_BANDWIDTH > > > + * quirk, but for compressed format we can't meaningfully estimate the > > > + * required bandwidth. Just hardcode it to 1024 bytes per interval, > > > + * which should be large enough for compressed formats. > > > + */ > > > + if ((format->flags & UVC_FMT_FLAG_COMPRESSED) && > > > + ctrl->dwMaxPayloadTransferSize > 3072 && > > > + stream->intf->num_altsetting > 1) > > > + ctrl->dwMaxPayloadTransferSize = 1024; > > > } > > > > > > static size_t uvc_video_ctrl_size(struct uvc_streaming *stream) > > -- > Regards, > > Laurent Pinchart -- Ricardo Ribalda ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-17 14:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-15 21:17 [PATCH v5] media: uvcvideo: Fix bandwidth error for Alcor camera Laurent Pinchart 2023-01-16 14:14 ` Ricardo Ribalda 2023-01-16 14:40 ` Laurent Pinchart 2023-01-17 14:08 ` Ricardo Ribalda 2023-01-17 14:18 ` Laurent Pinchart 2023-01-17 14:38 ` Ricardo Ribalda
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.