[v4,6/6] usb: gadget: uvc: add configfs option for sg support
Commit Message
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(-)
Comments
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
@@ -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
@@ -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
@@ -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);
@@ -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;
@@ -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,
};
@@ -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 {