[PATCH v4 04/10] usb: gadget: uvc: rework to enqueue in pump worker from encoded queue

Michael Grzeschik
2024-08-13T09:09:38Z

We install an kthread with pfifo prioroity that is iterating over all
prepared requests and keeps the isoc queue busy. It also watches the
theshold to enqueue zero length requests if needed. This way it will be
scheduled with the same priority as the interrupt handler.

But the interrupt handler will not be running into the time consuming
and eventually locking work of actually enqueueing the requests back
into its own pipeline. This work can now even can be scheduled on
another cpu.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v3 -> v4: -
v1 -> v3: new patch
---
drivers/usb/gadget/function/f_uvc.c | 3 +
drivers/usb/gadget/function/uvc.h | 3 +
drivers/usb/gadget/function/uvc_v4l2.c | 3 +
drivers/usb/gadget/function/uvc_video.c | 118 ++++++++++++++++++++------------
4 files changed, 84 insertions(+), 43 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index aeaf355a86eb3..1609daf85a258 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -986,10 +986,13 @@ static void uvc_function_unbind(struct usb_configuration *c,
{
struct usb_composite_dev *cdev = c->cdev;
struct uvc_device *uvc = to_uvc(f);
+ struct uvc_video *video = &uvc->video;
long wait_ret = 1;

uvcg_info(f, "%s()\n", __func__);

+ kthread_cancel_work_sync(&video->pump);
+
/*
* If we know we're connected via v4l2, then there should be a cleanup
* of the device from userspace either via UVC_EVENT_DISCONNECT or
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index e252c3db73072..b3a5165ac70ec 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -88,6 +88,9 @@ struct uvc_video {
struct uvc_device *uvc;
struct usb_ep *ep;

+ struct kthread_worker *kworker;
+ struct kthread_work pump;
+
int enqueued;
int dequeued;

diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index 4085f459c3c70..de41519ce9aa0 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -429,6 +429,9 @@ uvc_v4l2_qbuf(struct file *file, void *fh, struct v4l2_buffer *b)
if (ret < 0)
return ret;

+ if (uvc->state == UVC_STATE_STREAMING)
+ kthread_queue_work(video->kworker, &video->pump);
+
return ret;
}

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 5b0bf8069d48f..a51e95e3b717c 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -376,10 +376,7 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
struct uvc_video *video = ureq->video;
struct uvc_video_queue *queue = &video->queue;
struct uvc_buffer *last_buf;
- struct usb_request *to_queue = req;
unsigned long flags;
- bool is_bulk = video->max_payload_size;
- int ret = 0;

spin_lock_irqsave(&video->req_lock, flags);
video->dequeued++;
@@ -442,54 +439,78 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
return;
}

+ list_add_tail(&req->list, &video->req_free);
+
+ spin_unlock_irqrestore(&video->req_lock, flags);
+
/*
- * Here we check whether any request is available in the ready
- * list. If it is, queue it to the ep and add the current
- * usb_request to the req_free list - for qbuf to fill in.
- * Otherwise, just use the current usb_request to queue a 0
- * length request to the ep. Since we always add to the req_free
- * list if we dequeue from the ready list, there will never
- * be a situation where the req_free list is completely out of
- * requests and cannot recover.
+ * Queue work to the wq as well since it is possible that a
+ * buffer may not have been completely encoded with the set of
+ * in-flight usb requests for whih the complete callbacks are
+ * firing.
+ * In that case, if we do not queue work to the worker thread,
+ * the buffer will never be marked as complete - and therefore
+ * not be returned to userpsace. As a result,
+ * dequeue -> queue -> dequeue flow of uvc buffers will not
+ * happen.
*/
- to_queue->length = 0;
- if (!list_empty(&video->req_ready)) {
- to_queue = list_first_entry(&video->req_ready,
- struct usb_request, list);
- list_del(&to_queue->list);
- list_add_tail(&req->list, &video->req_free);
+ kthread_queue_work(video->kworker, &video->pump);
+}
+
+static void uvcg_video_pump(struct kthread_work *work)
+{
+ struct uvc_video *video = container_of(work, struct uvc_video, pump);
+ bool is_bulk = video->max_payload_size;
+ unsigned long flags;
+ struct usb_request *req;
+ int ret = 0;
+
+ while (true) {
+ if (!video->ep->enabled)
+ return;
+ spin_lock_irqsave(&video->req_lock, flags);
/*
- * Queue work to the wq as well since it is possible that a
- * buffer may not have been completely encoded with the set of
- * in-flight usb requests for whih the complete callbacks are
- * firing.
- * In that case, if we do not queue work to the worker thread,
- * the buffer will never be marked as complete - and therefore
- * not be returned to userpsace. As a result,
- * dequeue -> queue -> dequeue flow of uvc buffers will not
- * happen.
+ * Here we check whether any request is available in the ready
+ * list. If it is, queue it to the ep and add the current
+ * usb_request to the req_free list - for video_pump to fill in.
+ * Otherwise, just use the current usb_request to queue a 0
+ * length request to the ep. Since we always add to the req_free
+ * list if we dequeue from the ready list, there will never
+ * be a situation where the req_free list is completely out of
+ * requests and cannot recover.
*/
- } else if ((video->enqueued - video->dequeued) > 32) {
- spin_unlock_irqrestore(&video->req_lock, flags);
+ if (!list_empty(&video->req_ready)) {
+ req = list_first_entry(&video->req_ready,
+ struct usb_request, list);
+ } else {
+ if (list_empty(&video->req_free) || (video->enqueued - video->dequeued) > 32) {
+ spin_unlock_irqrestore(&video->req_lock, flags);
+
+ return;
+ }
+ req = list_first_entry(&video->req_free, struct usb_request,
+ list);
+ req->length = 0;
+ }
+ list_del(&req->list);

- return;
- }
- /*
- * Queue to the endpoint. The actual queueing to ep will
- * only happen on one thread - the async_wq for bulk endpoints
- * and this thread for isoc endpoints.
- */
- ret = uvcg_video_usb_req_queue(video, to_queue, !is_bulk);
- if (ret < 0) {
/*
- * Endpoint error, but the stream is still enabled.
- * Put request back in req_free for it to be cleaned
- * up later.
+ * Queue to the endpoint. The actual queueing to ep will
+ * only happen on one thread - the async_wq for bulk endpoints
+ * and this thread for isoc endpoints.
*/
- list_add_tail(&to_queue->list, &video->req_free);
- }
+ ret = uvcg_video_usb_req_queue(video, req, !is_bulk);
+ if (ret < 0) {
+ /*
+ * Endpoint error, but the stream is still enabled.
+ * Put request back in req_free for it to be cleaned
+ * up later.
+ */
+ list_add_tail(&req->list, &video->req_free);
+ }

- spin_unlock_irqrestore(&video->req_lock, flags);
+ spin_unlock_irqrestore(&video->req_lock, flags);
+ }
}

static int
@@ -755,6 +776,17 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
INIT_LIST_HEAD(&video->req_ready);
spin_lock_init(&video->req_lock);

+ /* Allocate a work queue for asynchronous video pump handler. */
+ video->kworker = kthread_create_worker(0, "UVCG");
+ if (IS_ERR(video->kworker)) {
+ uvcg_err(&video->uvc->func, "failed to create message pump kworker\n");
+ return PTR_ERR(video->kworker);
+ }
+
+ kthread_init_work(&video->pump, uvcg_video_pump);
+
+ sched_set_fifo(video->kworker->task);
+
video->uvc = uvc;
video->fcc = V4L2_PIX_FMT_YUYV;
video->bpp = 16;

--
2.39.2