| Message ID | 20221018215044.765044-6-w36195@motorola.com |
|---|---|
| State | New |
| Headers | show |
| Series | uvc gadget performance issues | expand |
On Tue, Oct 18, 2022 at 04:50:41PM -0500, Dan Vacura wrote: > Some UDC hw may not support skipping interrupts, but still support the > request. Allow the interrupt frequency to be configurable to the user. > > Signed-off-by: Dan Vacura <w36195@motorola.com> > --- > V1 -> V2: > - no change, new patch in series > V2 -> V3: > - default to baseline value of 4, fix storing the initial value > V3 -> V4: > - no change > > Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 + > Documentation/usb/gadget-testing.rst | 2 ++ Adding more tunable knobs always just adds complexity and fragility as things do not get tested. Can't we just make this dynamic and work properly on all systems? What UDC hardware types do not allow skipping interrupts? Why can't we just detect that and only do that if this is the case or not? > drivers/usb/gadget/function/f_uvc.c | 3 +++ > drivers/usb/gadget/function/u_uvc.h | 1 + > drivers/usb/gadget/function/uvc.h | 2 ++ > drivers/usb/gadget/function/uvc_configfs.c | 2 ++ > drivers/usb/gadget/function/uvc_queue.c | 6 ++++++ > drivers/usb/gadget/function/uvc_video.c | 3 ++- > 8 files changed, 19 insertions(+), 1 deletion(-) > > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc > index 611b23e6488d..5dfaa3f7f6a4 100644 > --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc > @@ -8,6 +8,7 @@ Description: UVC function directory > streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss) > streaming_interval 1..16 > function_name string [32] > + req_int_skip_div unsigned int > =================== ============================= > > What: /config/usb-gadget/gadget/functions/uvc.name/control > diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst > index 2278c9ffb74a..f9b5a09be1f4 100644 > --- a/Documentation/usb/gadget-testing.rst > +++ b/Documentation/usb/gadget-testing.rst > @@ -794,6 +794,8 @@ The uvc function provides these attributes in its function directory: > sending or receiving when this configuration is > selected > function_name name of the interface > + req_int_skip_div divisor of total requests to aid in calculating > + interrupt frequency, 0 indicates all interrupt This provides no information that would help to determine how to configure this at all. I wouldn't know what to do with this, so that's a huge sign that this is not a good interface :( thanks, greg k-h
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 611b23e6488d..5dfaa3f7f6a4 100644 --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -8,6 +8,7 @@ Description: UVC function directory streaming_maxpacket 1..1023 (fs), 1..3072 (hs/ss) streaming_interval 1..16 function_name string [32] + req_int_skip_div unsigned int =================== ============================= What: /config/usb-gadget/gadget/functions/uvc.name/control diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst index 2278c9ffb74a..f9b5a09be1f4 100644 --- a/Documentation/usb/gadget-testing.rst +++ b/Documentation/usb/gadget-testing.rst @@ -794,6 +794,8 @@ The uvc function provides these attributes in its function directory: sending or receiving when this configuration is selected function_name name of the interface + req_int_skip_div divisor of total requests to aid in calculating + interrupt frequency, 0 indicates all interrupt =================== ================================================ There are also "control" and "streaming" subdirectories, each of which contain diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c index 6e196e06181e..e40ca26b9c55 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -655,6 +655,8 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f) cpu_to_le16(max_packet_size * max_packet_mult * (opts->streaming_maxburst + 1)); + uvc->config_skip_int_div = opts->req_int_skip_div; + /* Allocate endpoints. */ ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep); if (!ep) { @@ -872,6 +874,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) opts->streaming_interval = 1; opts->streaming_maxpacket = 1024; + opts->req_int_skip_div = 4; snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera"); ret = uvcg_attach_configfs(opts); diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h index 24b8681b0d6f..6f73bd5638ed 100644 --- a/drivers/usb/gadget/function/u_uvc.h +++ b/drivers/usb/gadget/function/u_uvc.h @@ -24,6 +24,7 @@ struct f_uvc_opts { unsigned int streaming_interval; unsigned int streaming_maxpacket; unsigned int streaming_maxburst; + unsigned int req_int_skip_div; unsigned int control_interface; unsigned int streaming_interface; diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h index 40226b1f7e14..29f9477c92cc 100644 --- a/drivers/usb/gadget/function/uvc.h +++ b/drivers/usb/gadget/function/uvc.h @@ -107,6 +107,7 @@ struct uvc_video { spinlock_t req_lock; unsigned int req_int_count; + unsigned int req_int_skip_div; void (*encode) (struct usb_request *req, struct uvc_video *video, struct uvc_buffer *buf); @@ -155,6 +156,7 @@ struct uvc_device { /* Events */ unsigned int event_length; unsigned int event_setup_out : 1; + unsigned int config_skip_int_div; }; static inline struct uvc_device *to_uvc(struct usb_function *f) diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 4303a3283ba0..419e926ab57e 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -2350,6 +2350,7 @@ UVC_ATTR(f_uvc_opts_, cname, cname) UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16); UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072); UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15); +UVCG_OPTS_ATTR(req_int_skip_div, req_int_skip_div, UINT_MAX); #undef UVCG_OPTS_ATTR @@ -2399,6 +2400,7 @@ static struct configfs_attribute *uvc_attrs[] = { &f_uvc_opts_attr_streaming_interval, &f_uvc_opts_attr_streaming_maxpacket, &f_uvc_opts_attr_streaming_maxburst, + &f_uvc_opts_attr_req_int_skip_div, &f_uvc_opts_string_attr_function_name, NULL, }; diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c index 0aa3d7e1f3cc..02559906a55a 100644 --- a/drivers/usb/gadget/function/uvc_queue.c +++ b/drivers/usb/gadget/function/uvc_queue.c @@ -63,6 +63,12 @@ static int uvc_queue_setup(struct vb2_queue *vq, */ nreq = DIV_ROUND_UP(DIV_ROUND_UP(sizes[0], 2), req_size); nreq = clamp(nreq, 4U, 64U); + if (0 == video->uvc->config_skip_int_div) { + video->req_int_skip_div = nreq; + } else { + video->req_int_skip_div = min_t(unsigned int, nreq, + video->uvc->config_skip_int_div); + } video->uvc_num_requests = nreq; return 0; diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c index dd1c6b2ca7c6..c1a80c5d1f63 100644 --- a/drivers/usb/gadget/function/uvc_video.c +++ b/drivers/usb/gadget/function/uvc_video.c @@ -423,7 +423,8 @@ static void uvcg_video_pump(struct work_struct *work) if (list_empty(&video->req_free) || buf->state == UVC_BUF_STATE_DONE || !(video->req_int_count % - DIV_ROUND_UP(video->uvc_num_requests, 4))) { + DIV_ROUND_UP(video->uvc_num_requests, + video->req_int_skip_div))) { video->req_int_count = 0; req->no_interrupt = 0; } else {
Some UDC hw may not support skipping interrupts, but still support the request. Allow the interrupt frequency to be configurable to the user. Signed-off-by: Dan Vacura <w36195@motorola.com> --- V1 -> V2: - no change, new patch in series V2 -> V3: - default to baseline value of 4, fix storing the initial value V3 -> V4: - no change Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 + Documentation/usb/gadget-testing.rst | 2 ++ drivers/usb/gadget/function/f_uvc.c | 3 +++ drivers/usb/gadget/function/u_uvc.h | 1 + drivers/usb/gadget/function/uvc.h | 2 ++ drivers/usb/gadget/function/uvc_configfs.c | 2 ++ drivers/usb/gadget/function/uvc_queue.c | 6 ++++++ drivers/usb/gadget/function/uvc_video.c | 3 ++- 8 files changed, 19 insertions(+), 1 deletion(-)