Hi Michael,
Thank you for the patch.
On Sun, Apr 03, 2022 at 01:39:12AM +0200, Michael Grzeschik wrote:
> Likewise to the uvcvideo hostside driver, this patch is changing the
> simple workqueue to an async_wq with higher priority. This ensures that
> the worker will not be scheduled away while the video stream is handled.
>
> Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> ---
> drivers/usb/gadget/function/uvc.h | 1 +
> drivers/usb/gadget/function/uvc_v4l2.c | 2 +-
> drivers/usb/gadget/function/uvc_video.c | 9 +++++++--
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index c3607a32b98624..ab537acdae3184 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -86,6 +86,7 @@ struct uvc_video {
> struct usb_ep *ep;
>
> struct work_struct pump;
> + struct workqueue_struct *async_wq;
>
> /* Frame parameters */
> u8 bpp;
> diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
> index a2c78690c5c288..9b1488f7abd736 100644
> --- a/drivers/usb/gadget/function/uvc_v4l2.c
> +++ b/drivers/usb/gadget/function/uvc_v4l2.c
> @@ -170,7 +170,7 @@ uvc_v4l2_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
> return ret;
>
> if (uvc->state == UVC_STATE_STREAMING)
> - schedule_work(&video->pump);
> + queue_work(video->async_wq, &video->pump);
>
> return ret;
> }
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index 7f59a0c4740209..b1075e23a61010 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -269,7 +269,7 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
> spin_unlock_irqrestore(&video->req_lock, flags);
>
> if (uvc->state == UVC_STATE_STREAMING)
> - schedule_work(&video->pump);
> + queue_work(video->async_wq, &video->pump);
> }
>
> static int
> @@ -469,7 +469,7 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
>
> video->req_int_count = 0;
>
> - schedule_work(&video->pump);
> + queue_work(video->async_wq, &video->pump);
>
> return ret;
> }
> @@ -483,6 +483,11 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
> spin_lock_init(&video->req_lock);
> INIT_WORK(&video->pump, uvcg_video_pump);
>
> + /* Allocate a stream specific work queue for asynchronous tasks. */
You can drop the "stream" here. The gadget driver handles a single
stream.
> + video->async_wq = alloc_workqueue("uvcvideo", WQ_UNBOUND | WQ_HIGHPRI, 0);
Unless I'm mistaken, an unbound work queue means that multiple CPUs will
handle tasks in parallel. Is that safe ?
> + if (!video->async_wq)
> + return -EINVAL;
No need to destroy the work queue somewhere ?
> +
> video->uvc = uvc;
> video->fcc = V4L2_PIX_FMT_YUYV;
> video->bpp = 16;
--
Regards,
Laurent Pinchart
