From: Michael Grzeschik <m.grzeschik@pengutronix.de>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
Daniel Scally <dan.scally@ideasonboard.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Avichal Rakesh <arakesh@google.com>,
Jayant Chowdhary <jchowdhary@google.com>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel@pengutronix.de,
Michael Grzeschik <m.grzeschik@pengutronix.de>
Subject: [PATCH v7 2/9] usb: gadget: uvc: only enqueue zero length requests in potential underrun
Date: Wed, 16 Oct 2024 15:58:07 +0200 [thread overview]
Message-ID: <20240403-uvc_request_length_by_interval-v7-2-e224bb1035f0@pengutronix.de> (raw)
In-Reply-To: <20240403-uvc_request_length_by_interval-v7-0-e224bb1035f0@pengutronix.de>
The complete handler will at least be called after 16 requests have
completed, but will still handle all finisher requests. Since we have
to maintain a costant filling in the isoc queue we ensure this by
adding zero length requests.
By counting the amount enqueued requests we can ensure that the queue is
never underrun and only need to get active if the queue is running
critical. This patch is setting 32 as the critical level, which
is twice the request amount that is needed to create interrupts.
To properly solve the amount of zero length requests that needs to
be held in the hardware after one interrupt needs to be measured
and depends on the runtime of the first enqueue run after the interrupt
triggered. For now we just use twice the amount of requests between an
interrupt.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v5 -> v7: -
v4 -> v5:
- using min to limit for UVCG_REQ_MAX_INT_COUNT as the interrupt boundary
- using atomic_inc, atomic_dec on one variable to avoid rollover
- reordered this patch in the series
- added UVCG_REQ_MAX_ZERO_COUNT to set the threshold of zero length
requests in the hw
- added UVCG_REQ_MAX_INT_COUNT as quantifier for the
highest amount of request between an interrupt
v1 -> v4: -
---
drivers/usb/gadget/function/uvc.h | 5 +++++
drivers/usb/gadget/function/uvc_video.c | 17 ++++++++++++++++-
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index cb35687b11e7e..55d796f5f5e8d 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -71,6 +71,9 @@ extern unsigned int uvc_gadget_trace_param;
#define UVCG_REQUEST_HEADER_LEN 12
+#define UVCG_REQ_MAX_INT_COUNT 16
+#define UVCG_REQ_MAX_ZERO_COUNT (2 * UVCG_REQ_MAX_INT_COUNT)
+
/* ------------------------------------------------------------------------
* Structures
*/
@@ -91,6 +94,8 @@ struct uvc_video {
struct work_struct pump;
struct workqueue_struct *async_wq;
+ atomic_t queued;
+
/* Frame parameters */
u8 bpp;
u32 fcc;
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 002bf724d8025..c041873cf8560 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -269,6 +269,8 @@ static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req)
}
}
+ atomic_inc(&video->queued);
+
return ret;
}
@@ -304,7 +306,7 @@ static int uvcg_video_usb_req_queue(struct uvc_video *video,
*/
if (list_empty(&video->req_free) || ureq->last_buf ||
!(video->req_int_count %
- DIV_ROUND_UP(video->uvc_num_requests, 4))) {
+ min(DIV_ROUND_UP(video->uvc_num_requests, 4), UVCG_REQ_MAX_INT_COUNT))) {
video->req_int_count = 0;
req->no_interrupt = 0;
} else {
@@ -379,6 +381,7 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
int ret = 0;
spin_lock_irqsave(&video->req_lock, flags);
+ atomic_dec(&video->queued);
if (!video->is_enabled) {
/*
* When is_enabled is false, uvcg_video_disable() ensures
@@ -466,6 +469,16 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
* happen.
*/
queue_work(video->async_wq, &video->pump);
+ } else if (atomic_read(&video->queued) > UVCG_REQ_MAX_ZERO_COUNT) {
+ list_add_tail(&to_queue->list, &video->req_free);
+ /*
+ * There is a new free request - wake up the pump.
+ */
+ queue_work(video->async_wq, &video->pump);
+
+ spin_unlock_irqrestore(&video->req_lock, flags);
+
+ return;
}
/*
* Queue to the endpoint. The actual queueing to ep will
@@ -756,6 +769,8 @@ int uvcg_video_enable(struct uvc_video *video)
video->req_int_count = 0;
+ atomic_set(&video->queued, 0);
+
uvc_video_ep_queue_initial_requests(video);
queue_work(video->async_wq, &video->pump);
--
2.39.5
next prev parent reply other threads:[~2024-10-16 13:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-16 13:58 [PATCH v7 0/9] usb: gadget: uvc: effectively fill the udc isoc pipeline with available video buffers and fixes Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 1/9] usb: gadget: uvc: wake pump everytime we update the free list Michael Grzeschik
2024-10-16 13:58 ` Michael Grzeschik [this message]
2024-10-16 13:58 ` [PATCH v7 3/9] usb: gadget: uvc: rework to enqueue in pump worker from encoded queue Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 4/9] usb: gadget: uvc: add g_parm and s_parm for frame interval Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 5/9] usb: gadget: uvc: set req_size and n_requests based on the " Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 6/9] usb: gadget: uvc: set req_length based on payload by nreqs instead of req_size Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 7/9] usb: gadget: uvc: set nbuffers to minimum STREAMING_MIN_BUFFERS in uvc_queue_setup Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 8/9] usb: gadget: uvc: add trace of enqueued and completed requests Michael Grzeschik
2024-10-16 13:58 ` [PATCH v7 9/9] usb: gadget: uvc: dont call usb_composite_setup_continue when not streaming Michael Grzeschik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240403-uvc_request_length_by_interval-v7-2-e224bb1035f0@pengutronix.de \
--to=m.grzeschik@pengutronix.de \
--cc=arakesh@google.com \
--cc=dan.scally@ideasonboard.com \
--cc=gregkh@linuxfoundation.org \
--cc=jchowdhary@google.com \
--cc=kernel@pengutronix.de \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®