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@xxxxxxxxxxxxxxxxxxxxxxxxxx/ Signed-off-by: Avichal Rakesh <arakesh@xxxxxxxxxx> --- Changelog: v2: - Updated commit message to make it clear that userspace application is not required to match the ISOC rate. - Styling and comment revision based on review v3: - Rebased on to Greg's usb-next where v1 had already merged - Updated commit message to match the actual changes after rebase. drivers/usb/gadget/function/uvc_video.c | 38 ++++++++++++++++--------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index e81865978299..91af3b1ef0d4 100644 --- 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; -- 2.41.0.rc0.172.g3f132b7071-goog