Recenly when test uvc gadget function I find some YUYV pixel format 720p and 1080p stream can't output normally. However, small resulution and MJPEG format stream works fine. The first patch#1 is to fix the issue.
Patch#2 and #3 are small fix or improvement.
For patch#4: it's a workaround for a long-term issue in videobuf2. With it, many device can work well and not solely based on the SG allocation method.
Signed-off-by: Xu Yang xu.yang_2@nxp.com --- Xu Yang (4): usb: gadget: uvc: fix req_payload_size calculation usb: gadget: uvc: fix interval_duration calculation usb: gadget: uvc: improve error handling in uvcg_video_init() usb: gadget: uvc: retry vb2_reqbufs() with vb_vmalloc_memops if use_sg fail
drivers/usb/gadget/function/f_uvc.c | 4 ++++ drivers/usb/gadget/function/uvc.h | 3 ++- drivers/usb/gadget/function/uvc_queue.c | 23 +++++++++++++++++++---- drivers/usb/gadget/function/uvc_video.c | 14 +++++++------- 4 files changed, 32 insertions(+), 12 deletions(-) --- base-commit: 56a512a9b4107079f68701e7d55da8507eb963d9 change-id: 20260108-uvc-gadget-fix-patch-aa5996332bb5
Best regards,
Current req_payload_size calculation has 2 issue:
(1) When the first time calculate req_payload_size for all the buffers, reqs_per_frame = 0 will be the divisor of DIV_ROUND_UP(). So the result is undefined. This happens because VIDIOC_STREAMON is always executed after VIDIOC_QBUF. So video->reqs_per_frame will be 0 until VIDIOC_STREAMON is run.
(2) The buf->req_payload_size may be bigger than max_req_size.
Take YUYV pixel format as example: If bInterval = 1, video->interval = 666666, high-speed: video->reqs_per_frame = 666666 / 1250 = 534 720p: buf->req_payload_size = 1843200 / 534 = 3452 1080p: buf->req_payload_size = 4147200 / 534 = 7766
Based on such req_payload_size, the controller can't run normally.
To fix above issue, assign max_req_size to buf->req_payload_size when video->reqs_per_frame = 0. And limit buf->req_payload_size to video->req_size if it's large than video->req_size. Since max_req_size is used at many place, add it to struct uvc_video and set the value once endpoint is enabled.
Fixes: 98ad03291560 ("usb: gadget: uvc: set req_length based on payload by nreqs instead of req_size") Cc: stable@vger.kernel.org Signed-off-by: Xu Yang xu.yang_2@nxp.com --- drivers/usb/gadget/function/f_uvc.c | 4 ++++ drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_queue.c | 15 +++++++++++---- drivers/usb/gadget/function/uvc_video.c | 4 +--- 4 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index aa6ab666741a9518690995ccdc04e742b4359a0e..a96476507d2fdf4eb0817f3aac09b7ee08df593a 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -362,6 +362,10 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt) return ret; usb_ep_enable(uvc->video.ep);
+ uvc->video.max_req_size = uvc->video.ep->maxpacket + * max_t(unsigned int, uvc->video.ep->maxburst, 1) + * (uvc->video.ep->mult); + memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_STREAMON; v4l2_event_queue(&uvc->vdev, &v4l2_event); diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 9e79cbe50715791a7f7ddd3bc20e9a28d221db61..b3f88670bff801a43d084646974602e5995bb192 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -117,6 +117,7 @@ struct uvc_video { /* Requests */ bool is_enabled; /* tracks whether video stream is enabled */ unsigned int req_size; + unsigned int max_req_size; struct list_head ureqs; /* all uvc_requests allocated by uvc_video */
/* USB requests that the video pump thread can encode into */ diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c index 9a1bbd79ff5af945bdd5dcf0c1cb1b6dbdc12a9c..21d80322cb6148ed87eb77f453a1f1644e4923ae 100644 --- a/drivers/usb/gadget/function/uvc_queue.c +++ b/drivers/usb/gadget/function/uvc_queue.c @@ -86,10 +86,17 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb) buf->bytesused = 0; } else { buf->bytesused = vb2_get_plane_payload(vb, 0); - buf->req_payload_size = - DIV_ROUND_UP(buf->bytesused + - (video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN), - video->reqs_per_frame); + + if (video->reqs_per_frame != 0) { + buf->req_payload_size = + DIV_ROUND_UP(buf->bytesused + + (video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN), + video->reqs_per_frame); + if (buf->req_payload_size > video->req_size) + buf->req_payload_size = video->req_size; + } else { + buf->req_payload_size = video->max_req_size; + } }
return 0; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index fb77b0b21790178751d36a23f07d5b1efff5c25f..1c0672f707e4e5f29c937a1868f0400aad62e5cb 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -503,9 +503,7 @@ uvc_video_prep_requests(struct uvc_video *video) unsigned int max_req_size, req_size, header_size; unsigned int nreq;
- max_req_size = video->ep->maxpacket - * max_t(unsigned int, video->ep->maxburst, 1) - * (video->ep->mult); + max_req_size = video->max_req_size;
if (!usb_endpoint_xfer_isoc(video->ep->desc)) { video->req_size = max_req_size;
On Thu, Jan 08, 2026 at 03:43:02PM +0800, Xu Yang wrote:
Current req_payload_size calculation has 2 issue:
(1) When the first time calculate req_payload_size for all the buffers, reqs_per_frame = 0 will be the divisor of DIV_ROUND_UP(). So the result is undefined. This happens because VIDIOC_STREAMON is always executed after VIDIOC_QBUF. So video->reqs_per_frame will be 0 until VIDIOC_STREAMON is run.
(2) The buf->req_payload_size may be bigger than max_req_size.
Take YUYV pixel format as example: If bInterval = 1, video->interval = 666666, high-speed: video->reqs_per_frame = 666666 / 1250 = 534 720p: buf->req_payload_size = 1843200 / 534 = 3452 1080p: buf->req_payload_size = 4147200 / 534 = 7766 Based on such req_payload_size, the controller can't run normally.To fix above issue, assign max_req_size to buf->req_payload_size when video->reqs_per_frame = 0. And limit buf->req_payload_size to video->req_size if it's large than video->req_size. Since max_req_size is used at many place, add it to struct uvc_video and set the value once endpoint is enabled.
Fixes: 98ad03291560 ("usb: gadget: uvc: set req_length based on payload by nreqs instead of req_size") Cc: stable@vger.kernel.org Signed-off-by: Xu Yang xu.yang_2@nxp.com
Reviewed-by: Frank Li Frank.Li@nxp.com
drivers/usb/gadget/function/f_uvc.c | 4 ++++ drivers/usb/gadget/function/uvc.h | 1 + drivers/usb/gadget/function/uvc_queue.c | 15 +++++++++++---- drivers/usb/gadget/function/uvc_video.c | 4 +--- 4 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index aa6ab666741a9518690995ccdc04e742b4359a0e..a96476507d2fdf4eb0817f3aac09b7ee08df593a 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -362,6 +362,10 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt) return ret; usb_ep_enable(uvc->video.ep);
uvc->video.max_req_size = uvc->video.ep->maxpacket* max_t(unsigned int, uvc->video.ep->maxburst, 1)* (uvc->video.ep->mult);- memset(&v4l2_event, 0, sizeof(v4l2_event)); v4l2_event.type = UVC_EVENT_STREAMON; v4l2_event_queue(&uvc->vdev, &v4l2_event);
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 9e79cbe50715791a7f7ddd3bc20e9a28d221db61..b3f88670bff801a43d084646974602e5995bb192 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -117,6 +117,7 @@ struct uvc_video { /* Requests */ bool is_enabled; /* tracks whether video stream is enabled */ unsigned int req_size;
unsigned int max_req_size; struct list_head ureqs; /* all uvc_requests allocated by uvc_video */
/* USB requests that the video pump thread can encode into */
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c index 9a1bbd79ff5af945bdd5dcf0c1cb1b6dbdc12a9c..21d80322cb6148ed87eb77f453a1f1644e4923ae 100644 --- a/drivers/usb/gadget/function/uvc_queue.c +++ b/drivers/usb/gadget/function/uvc_queue.c @@ -86,10 +86,17 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb) buf->bytesused = 0; } else { buf->bytesused = vb2_get_plane_payload(vb, 0);
buf->req_payload_size =DIV_ROUND_UP(buf->bytesused +(video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN),video->reqs_per_frame);
if (video->reqs_per_frame != 0) {buf->req_payload_size =DIV_ROUND_UP(buf->bytesused +(video->reqs_per_frame * UVCG_REQUEST_HEADER_LEN),video->reqs_per_frame);if (buf->req_payload_size > video->req_size)buf->req_payload_size = video->req_size;} else {buf->req_payload_size = video->max_req_size;}}
return 0;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index fb77b0b21790178751d36a23f07d5b1efff5c25f..1c0672f707e4e5f29c937a1868f0400aad62e5cb 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -503,9 +503,7 @@ uvc_video_prep_requests(struct uvc_video *video) unsigned int max_req_size, req_size, header_size; unsigned int nreq;
- max_req_size = video->ep->maxpacket
* max_t(unsigned int, video->ep->maxburst, 1)* (video->ep->mult);
max_req_size = video->max_req_size;
if (!usb_endpoint_xfer_isoc(video->ep->desc)) { video->req_size = max_req_size;
-- 2.34.1
According to USB specification:
For full-/high-speed isochronous endpoints, the bInterval value is used as the exponent for a 2^(bInterval-1) value.
To correctly convert bInterval as interval_duration: interval_duration = 2^(bInterval-1) * frame_interval
Because the unit of video->interval is 100ns, add a comment info to make it clear.
Fixes: 48dbe731171e ("usb: gadget: uvc: set req_size and n_requests based on the frame interval") Cc: stable@vger.kernel.org Signed-off-by: Xu Yang xu.yang_2@nxp.com --- drivers/usb/gadget/function/uvc.h | 2 +- drivers/usb/gadget/function/uvc_video.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index b3f88670bff801a43d084646974602e5995bb192..676419a049762f9eb59e1ac68b19fa34f153b793 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -107,7 +107,7 @@ struct uvc_video { unsigned int width; unsigned int height; unsigned int imagesize; - unsigned int interval; + unsigned int interval; /* in 100ns units */ struct mutex mutex; /* protects frame parameters */
unsigned int uvc_num_requests; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 1c0672f707e4e5f29c937a1868f0400aad62e5cb..b1c5c1d3e9390c82cc84e736a7f288626ee69d51 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -499,7 +499,7 @@ uvc_video_prep_requests(struct uvc_video *video) { struct uvc_device *uvc = container_of(video, struct uvc_device, video); struct usb_composite_dev *cdev = uvc->func.config->cdev; - unsigned int interval_duration = video->ep->desc->bInterval * 1250; + unsigned int interval_duration; unsigned int max_req_size, req_size, header_size; unsigned int nreq;
@@ -513,8 +513,11 @@ uvc_video_prep_requests(struct uvc_video *video) return; }
+ interval_duration = int_pow(2, video->ep->desc->bInterval - 1); if (cdev->gadget->speed < USB_SPEED_HIGH) - interval_duration = video->ep->desc->bInterval * 10000; + interval_duration *= 10000; + else + interval_duration *= 1250;
nreq = DIV_ROUND_UP(video->interval, interval_duration);
On Thu, Jan 08, 2026 at 03:43:03PM +0800, Xu Yang wrote:
According to USB specification:
For full-/high-speed isochronous endpoints, the bInterval value is used as the exponent for a 2^(bInterval-1) value.
To correctly convert bInterval as interval_duration: interval_duration = 2^(bInterval-1) * frame_interval
Because the unit of video->interval is 100ns, add a comment info to make it clear.
Fixes: 48dbe731171e ("usb: gadget: uvc: set req_size and n_requests based on the frame interval") Cc: stable@vger.kernel.org Signed-off-by: Xu Yang xu.yang_2@nxp.com
drivers/usb/gadget/function/uvc.h | 2 +- drivers/usb/gadget/function/uvc_video.c | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index b3f88670bff801a43d084646974602e5995bb192..676419a049762f9eb59e1ac68b19fa34f153b793 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -107,7 +107,7 @@ struct uvc_video { unsigned int width; unsigned int height; unsigned int imagesize;
- unsigned int interval;
unsigned int interval; /* in 100ns units */ struct mutex mutex; /* protects frame parameters */
unsigned int uvc_num_requests;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index 1c0672f707e4e5f29c937a1868f0400aad62e5cb..b1c5c1d3e9390c82cc84e736a7f288626ee69d51 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -499,7 +499,7 @@ uvc_video_prep_requests(struct uvc_video *video) { struct uvc_device *uvc = container_of(video, struct uvc_device, video); struct usb_composite_dev *cdev = uvc->func.config->cdev;
- unsigned int interval_duration = video->ep->desc->bInterval * 1250;
- unsigned int interval_duration; unsigned int max_req_size, req_size, header_size; unsigned int nreq;
@@ -513,8 +513,11 @@ uvc_video_prep_requests(struct uvc_video *video) return; }
- interval_duration = int_pow(2, video->ep->desc->bInterval - 1);
2 << (video->ep->desc->bInterval - 1) or BIT(video->ep->desc->bInterval - 1);
int_pow() use while loop. slice better.
Reviewed-by: Frank Li Frank.Li@nxp.com
Frank
if (cdev->gadget->speed < USB_SPEED_HIGH)
interval_duration = video->ep->desc->bInterval * 10000;
interval_duration *= 10000;else
interval_duration *= 1250;nreq = DIV_ROUND_UP(video->interval, interval_duration);
-- 2.34.1
linux-stable-mirror@lists.linaro.org
-
Frank Li -
Xu Yang