On Tue, Oct 11, 2022 at 11:32:56PM +0200, Michael Grzeschik wrote:
> On Tue, Oct 11, 2022 at 10:57:07PM +0200, Michael Grzeschik wrote:
> > The overhead of preparing sg data is high for transfers with limited
> > payload. When transferring isoc over high-speed usb the maximum payload
> > is rather small which is a good argument no to use sg. This patch is
> > changing the uvc_video_encode_isoc_sg encode function only to be used
> > for super speed gadgets.
> >
> > Signed-off-by: Michael Grzeschik <m.grzeschik@xxxxxxxxxxxxxx>
> > ---
> > drivers/usb/gadget/function/uvc_video.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> > index bb037fcc90e69e..5081eb3bc5484c 100644
> > --- a/drivers/usb/gadget/function/uvc_video.c
> > +++ b/drivers/usb/gadget/function/uvc_video.c
> > @@ -448,6 +448,9 @@ static void uvcg_video_pump(struct work_struct *work)
> > */
> > int uvcg_video_enable(struct uvc_video *video, int enable)
> > {
> > + struct uvc_device *uvc = video->uvc;
> > + struct usb_composite_dev *cdev = uvc->func.config->cdev;
> > + struct usb_gadget *gadget = cdev->gadget;
> > unsigned int i;
> > int ret;
> >
> > @@ -479,9 +482,11 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
> > if (video->max_payload_size) {
> > video->encode = uvc_video_encode_bulk;
> > video->payload_size = 0;
> > - } else
> > - video->encode = video->queue.use_sg ?
> > + } else {
> > + video->encode = (video->queue.use_sg &&
> > + !(gadget->speed <= USB_SPEED_HIGH)) ?
>
> I also came up with the following Idea:
>
> - !(gadget->speed <= USB_SPEED_HIGH)) ?
> + video->req_size > 4096) ?
>
> Would this threshold of 4096 make sense? What should be preferred?
Where did you pick 4096 from? Even if you pick PAGE_SIZE, why? Last
time I ran memcpy() tests vs. sg on a range of processors the benifit
did not kick in until the value was MUCH larger than just 4k, but I
guess that depends on the overall codepath involved here.
So please test, and don't just guess, and then document it really really
well why you picked that value.
For now, the SPEED_HIGH should be sufficient I think.
thanks,
greg k-h
