[PATCH v2] usb: gadget: uvc: limit isoc_sg to super speed gadgets

[PATCH v2] usb: gadget: uvc: limit isoc_sg to super speed gadgets

[Michael Grzeschik]

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@pengutronix.de>

---
v1 -> v2: - always setting mem and sg elements since now both is working in runtime

 drivers/usb/gadget/function/uvc_queue.c | 9 +++------
 drivers/usb/gadget/function/uvc_video.c | 9 +++++++--
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index ec500ee499eed1..31c50ba1774f0d 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -84,12 +84,9 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
 		return -ENODEV;
 
 	buf->state = UVC_BUF_STATE_QUEUED;
-	if (queue->use_sg) {
-		buf->sgt = vb2_dma_sg_plane_desc(vb, 0);
-		buf->sg = buf->sgt->sgl;
-	} else {
-		buf->mem = vb2_plane_vaddr(vb, 0);
-	}
+	buf->sgt = vb2_dma_sg_plane_desc(vb, 0);
+	buf->sg = buf->sgt->sgl;
+	buf->mem = vb2_plane_vaddr(vb, 0);
 	buf->length = vb2_plane_size(vb, 0);
 	if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		buf->bytesused = 0;
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)) ?
 			uvc_video_encode_isoc_sg : uvc_video_encode_isoc;
+	}
 
 	video->req_int_count = 0;
 
-- 
2.30.2

Re: [PATCH v2] usb: gadget: uvc: limit isoc_sg to super speed gadgets

[Greg KH]

On Tue, Oct 18, 2022 at 12:11:41AM +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@pengutronix.de>
> 
> ---
> v1 -> v2: - always setting mem and sg elements since now both is working in runtime

I'm guessing this is a "fix"?  If so, what commit id is this a fix for?

And any reason you aren't cc:ing me on these patches?

thanks,

greg k-h

Re: [PATCH v2] usb: gadget: uvc: limit isoc_sg to super speed gadgets

[Michael Grzeschik]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

Hi!

On Sat, Oct 22, 2022 at 01:35:32PM +0200, Greg KH wrote:
>On Tue, Oct 18, 2022 at 12:11:41AM +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@pengutronix.de>
>>
>> ---
>> v1 -> v2: - always setting mem and sg elements since now both is working in runtime
>
>I'm guessing this is a "fix"?  If so, what commit id is this a fix for?

This is not a fix but a feature. I am working to improve it
also to work with dmabuf memory comming in as vaddr. This needs some
extra mapping. Since you already took this patch, I will send fix for
this one then.

>And any reason you aren't cc:ing me on these patches?

This was not intentional. I have my scripts that I usually recycle.
They probably need to be updatet.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] usb: gadget: uvc: limit isoc_sg to super speed gadgets

[Greg KH]

On Tue, Oct 25, 2022 at 09:25:37PM +0200, Michael Grzeschik wrote:
> Hi!
> 
> On Sat, Oct 22, 2022 at 01:35:32PM +0200, Greg KH wrote:
> > On Tue, Oct 18, 2022 at 12:11:41AM +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@pengutronix.de>
> > > 
> > > ---
> > > v1 -> v2: - always setting mem and sg elements since now both is working in runtime
> > 
> > I'm guessing this is a "fix"?  If so, what commit id is this a fix for?
> 
> This is not a fix but a feature. I am working to improve it
> also to work with dmabuf memory comming in as vaddr. This needs some
> extra mapping. Since you already took this patch, I will send fix for
> this one then.

As it's not a fix, and it needs a fixup, I'll drop this from my tree for
now as it should not go into 6.1-final.  Please resend an updated, and
fixed, patch instead.

thanks,

greg k-h