usb: gadget: uvc: clean up comments and styling in video_pump

This patch elaborates on some of the edge cases handled by
video_pump around setting no_interrupt flag, and brings the
code style in line with rest of the file.

Link: https://lore.kernel.org/20230602151916.GH26944@pendragon.ideasonboard.com/
Signed-off-by: Avichal Rakesh <arakesh@google.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Message-ID: <20230602220455.313801-1-arakesh@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index e81865978299ccea3fdd652b0160c892cf99f617..91af3b1ef0d412e9d71720afb38f24a703ec4e6e 100644 (file)
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -382,13 +382,13 @@ static void uvcg_video_pump(struct work_struct *work)
 {
        struct uvc_video *video = container_of(work, struct uvc_video, pump);
        struct uvc_video_queue *queue = &video->queue;
+       /* video->max_payload_size is only set when using bulk transfer */
+       bool is_bulk = video->max_payload_size;
        struct usb_request *req = NULL;
        struct uvc_buffer *buf;
        unsigned long flags;
+       bool buf_done;
        int ret;
-       bool buf_int;
-       /* video->max_payload_size is only set when using bulk transfer */
-       bool is_bulk = video->max_payload_size;
 
        while (video->ep->enabled) {
                /*
@@ -414,20 +414,19 @@ static void uvcg_video_pump(struct work_struct *work)
 
                if (buf != NULL) {
                        video->encode(req, video, buf);
-                       /* Always interrupt for the last request of a video buffer */
-                       buf_int = buf->state == UVC_BUF_STATE_DONE;
+                       buf_done = buf->state == UVC_BUF_STATE_DONE;
                } else if (!(queue->flags & UVC_QUEUE_DISCONNECTED) && !is_bulk) {
                        /*
                         * No video buffer available; the queue is still connected and
-                        * we're traferring over ISOC. Queue a 0 length request to
+                        * we're transferring over ISOC. Queue a 0 length request to
                         * prevent missed ISOC transfers.
                         */
                        req->length = 0;
-                       buf_int = false;
+                       buf_done = false;
                } else {
                        /*
-                        * Either queue has been disconnected or no video buffer
-                        * available to bulk transfer. Either way, stop processing
+                        * Either the queue has been disconnected or no video buffer
+                        * available for bulk transfer. Either way, stop processing
                         * further.
                         */
                        spin_unlock_irqrestore(&queue->irqlock, flags);
@@ -435,11 +434,24 @@ static void uvcg_video_pump(struct work_struct *work)
                }
 
                /*
-                * With usb3 we have more requests. This will decrease the
-                * interrupt load to a quarter but also catches the corner
-                * cases, which needs to be handled.
+                * With USB3 handling more requests at a higher speed, we can't
+                * afford to generate an interrupt for every request. Decide to
+                * interrupt:
+                *
+                * - When no more requests are available in the free queue, as
+                *   this may be our last chance to refill the endpoint's
+                *   request queue.
+                *
+                * - When this is request is the last request for the video
+                *   buffer, as we want to start sending the next video buffer
+                *   ASAP in case it doesn't get started already in the next
+                *   iteration of this loop.
+                *
+                * - Four times over the length of the requests queue (as
+                *   indicated by video->uvc_num_requests), as a trade-off
+                *   between latency and interrupt load.
                 */
-               if (list_empty(&video->req_free) || buf_int ||
+               if (list_empty(&video->req_free) || buf_done ||
                    !(video->req_int_count %
                       DIV_ROUND_UP(video->uvc_num_requests, 4))) {
                        video->req_int_count = 0;