| Message ID | 20221018215044.765044-7-w36195@motorola.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | uvc gadget performance issues | expand |
On Tue, Oct 18, 2022 at 04:50:42PM -0500, Dan Vacura wrote: > The scatter gather support doesn't appear to work well with some UDC hw. > Add the ability to turn off the feature depending on the controller in > use or other platform quirks. The default is for the uvc gadget to > support sg as long as the UDC hw supports it. > > The specific failure was with the dwc3 controller, but fixes and > improvements are pending for those failures. This capability is now > more intended for future unexpected failures or poor sg support on a > given platform. > > Signed-off-by: Dan Vacura <w36195@motorola.com> Again, this should be dynamic. Can't we detect this based on the packet size and either do sg or not? If the UDC hardware says it is supported, it should be supported. Otherwise we need to fix the UDC hardware or it saying it is allowed. > --- a/Documentation/usb/gadget-testing.rst > +++ b/Documentation/usb/gadget-testing.rst > @@ -796,6 +796,10 @@ The uvc function provides these attributes in its function directory: > function_name name of the interface > req_int_skip_div divisor of total requests to aid in calculating > interrupt frequency, 0 indicates all interrupt > + sg_supported allow for scatter gather to be used if the UDC > + hw supports it, this is default on and only > + intended to be temporally turned off if a given > + platform doesn't work well with scatter gather How do you know if it "doesn't work well"? That's vague and not good and nothing we want to support for forever, sorry. thanks, greg k-h
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 5dfaa3f7f6a4..839a75fc28ee 100644 --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc @@ -9,6 +9,7 @@ Description: UVC function directory streaming_interval 1..16 function_name string [32] req_int_skip_div unsigned int + sg_supported 0..1 =================== ============================= What: /config/usb-gadget/gadget/functions/uvc.name/control diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst index f9b5a09be1f4..be72577a03e9 100644 --- a/Documentation/usb/gadget-testing.rst +++ b/Documentation/usb/gadget-testing.rst @@ -796,6 +796,10 @@ The uvc function provides these attributes in its function directory: function_name name of the interface req_int_skip_div divisor of total requests to aid in calculating interrupt frequency, 0 indicates all interrupt + sg_supported allow for scatter gather to be used if the UDC + hw supports it, this is default on and only + intended to be temporally turned off if a given + platform doesn't work well with scatter gather =================== ================================================ 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 e40ca26b9c55..83d96e81c05f 100644 --- a/drivers/usb/gadget/function/f_uvc.c +++ b/drivers/usb/gadget/function/f_uvc.c @@ -875,6 +875,7 @@ static struct usb_function_instance *uvc_alloc_inst(void) opts->streaming_interval = 1; opts->streaming_maxpacket = 1024; opts->req_int_skip_div = 4; + opts->sg_supported = 1; 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 6f73bd5638ed..5ccced629925 100644 --- a/drivers/usb/gadget/function/u_uvc.h +++ b/drivers/usb/gadget/function/u_uvc.h @@ -25,6 +25,7 @@ struct f_uvc_opts { unsigned int streaming_maxpacket; unsigned int streaming_maxburst; unsigned int req_int_skip_div; + unsigned int sg_supported; unsigned int control_interface; unsigned int streaming_interface; diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c index 419e926ab57e..3784c0e02d01 100644 --- a/drivers/usb/gadget/function/uvc_configfs.c +++ b/drivers/usb/gadget/function/uvc_configfs.c @@ -2351,6 +2351,7 @@ 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); +UVCG_OPTS_ATTR(sg_supported, sg_supported, 1); #undef UVCG_OPTS_ATTR @@ -2401,6 +2402,7 @@ static struct configfs_attribute *uvc_attrs[] = { &f_uvc_opts_attr_streaming_maxpacket, &f_uvc_opts_attr_streaming_maxburst, &f_uvc_opts_attr_req_int_skip_div, + &f_uvc_opts_attr_sg_supported, &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 02559906a55a..aee405663d6e 100644 --- a/drivers/usb/gadget/function/uvc_queue.c +++ b/drivers/usb/gadget/function/uvc_queue.c @@ -21,6 +21,7 @@ #include <media/videobuf2-vmalloc.h> #include "uvc.h" +#include "u_uvc.h" /* ------------------------------------------------------------------------ * Video buffers queue management. @@ -141,6 +142,7 @@ int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2 { struct uvc_video *video = container_of(queue, struct uvc_video, queue); struct usb_composite_dev *cdev = video->uvc->func.config->cdev; + struct f_uvc_opts *opts = fi_to_f_uvc_opts(video->uvc->func.fi); int ret; queue->queue.type = type; @@ -149,7 +151,7 @@ int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2 queue->queue.buf_struct_size = sizeof(struct uvc_buffer); queue->queue.ops = &uvc_queue_qops; queue->queue.lock = lock; - if (cdev->gadget->sg_supported) { + if (opts->sg_supported && cdev->gadget->sg_supported) { queue->queue.mem_ops = &vb2_dma_sg_memops; queue->use_sg = 1; } else {
The scatter gather support doesn't appear to work well with some UDC hw. Add the ability to turn off the feature depending on the controller in use or other platform quirks. The default is for the uvc gadget to support sg as long as the UDC hw supports it. The specific failure was with the dwc3 controller, but fixes and improvements are pending for those failures. This capability is now more intended for future unexpected failures or poor sg support on a given platform. Signed-off-by: Dan Vacura <w36195@motorola.com> --- V1 -> V2: - no change, new patch in series V2 -> V3: - default on, same as baseline V3 -> V4: - update comment and documentation, refactor use of opts->sg_supported directly in uvc_queue Documentation/ABI/testing/configfs-usb-gadget-uvc | 1 + Documentation/usb/gadget-testing.rst | 4 ++++ drivers/usb/gadget/function/f_uvc.c | 1 + drivers/usb/gadget/function/u_uvc.h | 1 + drivers/usb/gadget/function/uvc_configfs.c | 2 ++ drivers/usb/gadget/function/uvc_queue.c | 4 +++- 6 files changed, 12 insertions(+), 1 deletion(-)