[PATCH v3 0/5] usb: gadget: uvc: improve uvc gadget performance

[PATCH v3 0/5] usb: gadget: uvc: improve uvc gadget performance

[Michael Grzeschik]

This series improves the performance of the uvc video gadget by adding a
zero copy routine using the scatter list interface of the gadget. The
series also increases the amount of allocated requests depending of the
speed and it also reduces the interrupt load by only trigger on every
16th request in case of super-speed.

Michael Grzeschik (5):
  usb: dwc3: gadget: set gadgets parent to the right controller
  usb: gadget: uvc: make uvc_num_requests depend on gadget speed
  usb: gadget: uvc: set v4l2_dev->dev in f_uvc
  usb: gadget: uvc: add scatter gather support
  usb: gadget: uvc: decrease the interrupt load to a quarter

 drivers/usb/dwc3/gadget.c               |   2 +-
 drivers/usb/gadget/Kconfig              |   1 +
 drivers/usb/gadget/function/f_uvc.c     |   1 +
 drivers/usb/gadget/function/uvc.h       |  15 ++-
 drivers/usb/gadget/function/uvc_queue.c |  28 ++++-
 drivers/usb/gadget/function/uvc_queue.h |   7 +-
 drivers/usb/gadget/function/uvc_video.c | 155 +++++++++++++++++++-----
 drivers/usb/gadget/function/uvc_video.h |   2 +
 drivers/usb/gadget/legacy/Kconfig       |   1 +
 9 files changed, 176 insertions(+), 36 deletions(-)

-- 
2.30.2

[PATCH v3 1/5] usb: dwc3: gadget: set gadgets parent to the right controller

[Michael Grzeschik]

In case of dwc3 it is possible that the sysdev is the parent of the
controller. To ensure the right dev is set to the gadget's dev parent we
will hand over sysdev instead of dev, which will always point to the
controller.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
   -> v2: first version of patch
   -> v3: -
---
 drivers/usb/dwc3/gadget.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index af6d7f157989d..8a1b1daff2e97 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -3990,7 +3990,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
 	}
 
 
-	usb_initialize_gadget(dwc->dev, dwc->gadget, dwc_gadget_release);
+	usb_initialize_gadget(dwc->sysdev, dwc->gadget, dwc_gadget_release);
 	dev				= &dwc->gadget->dev;
 	dev->platform_data		= dwc;
 	dwc->gadget->ops		= &dwc3_gadget_ops;
-- 
2.30.2

Re: [PATCH v3 1/5] usb: dwc3: gadget: set gadgets parent to the right controller

[Greg KH]

On Mon, Jun 28, 2021 at 05:53:07PM +0200, Michael Grzeschik wrote:
> In case of dwc3 it is possible that the sysdev is the parent of the
> controller. To ensure the right dev is set to the gadget's dev parent we
> will hand over sysdev instead of dev, which will always point to the
> controller.
> 
> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
> 
> ---
>    -> v2: first version of patch
>    -> v3: -
> ---
>  drivers/usb/dwc3/gadget.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index af6d7f157989d..8a1b1daff2e97 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -3990,7 +3990,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>  	}
>  
>  
> -	usb_initialize_gadget(dwc->dev, dwc->gadget, dwc_gadget_release);
> +	usb_initialize_gadget(dwc->sysdev, dwc->gadget, dwc_gadget_release);

Does this change how the device shows up in sysfs?  Why does the parent
not always work properly here?

thanks,

greg k-h

Re: [PATCH v3 1/5] usb: dwc3: gadget: set gadgets parent to the right controller

[Felipe Balbi]

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


Hi,

Greg KH <greg@kroah.com> writes:
> On Mon, Jun 28, 2021 at 05:53:07PM +0200, Michael Grzeschik wrote:
>> In case of dwc3 it is possible that the sysdev is the parent of the
>> controller. To ensure the right dev is set to the gadget's dev parent we
>> will hand over sysdev instead of dev, which will always point to the
>> controller.
>> 
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> 
>> ---
>>    -> v2: first version of patch
>>    -> v3: -
>> ---
>>  drivers/usb/dwc3/gadget.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index af6d7f157989d..8a1b1daff2e97 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -3990,7 +3990,7 @@ int dwc3_gadget_init(struct dwc3 *dwc)
>>  	}
>>  
>>  
>> -	usb_initialize_gadget(dwc->dev, dwc->gadget, dwc_gadget_release);
>> +	usb_initialize_gadget(dwc->sysdev, dwc->gadget, dwc_gadget_release);
>
> Does this change how the device shows up in sysfs?

it might, but not in all instances of dwc3

> Why does the parent not always work properly here?

This is a really old commit by Arnd from back in 2016 (d64ff406e51e)
where it was noticed that dwc3's dma ops weren't properly setup. Arnd
decided to pass a property to tell dwc3 core to use the parent device
for dma operations.

This is not required in all instances of dwc3, though.

-- 
balbi

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

[PATCH v3 2/5] usb: gadget: uvc: make uvc_num_requests depend on gadget speed

[Michael Grzeschik]

While sending bigger images is possible with USB_SPEED_SUPER it is
better to use more isochronous requests in flight. This patch makes the
number uvc_num_requests dynamic by changing it depending on the gadget
speed.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: - using u8 instead of __u8
          - made uvc_num_requests usigned int
	  - directly referencing usb_composite_dev
	  - removed extra include module.h
	  - formated coding style
	  - returning -ENOMEM on alloc error
v2 -> v3: -
---
 drivers/usb/gadget/function/uvc.h       | 11 +++--
 drivers/usb/gadget/function/uvc_queue.c |  6 +++
 drivers/usb/gadget/function/uvc_video.c | 55 +++++++++++++++----------
 3 files changed, 47 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 23ee25383c1f7..52f894183ecf0 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -65,13 +65,17 @@ extern unsigned int uvc_gadget_trace_param;
  * Driver specific constants
  */
 
-#define UVC_NUM_REQUESTS			4
 #define UVC_MAX_REQUEST_SIZE			64
 #define UVC_MAX_EVENTS				4
 
 /* ------------------------------------------------------------------------
  * Structures
  */
+struct uvc_request {
+	struct usb_request *req;
+	u8 *req_buffer;
+	struct uvc_video *video;
+};
 
 struct uvc_video {
 	struct uvc_device *uvc;
@@ -87,10 +91,11 @@ struct uvc_video {
 	unsigned int imagesize;
 	struct mutex mutex;	/* protects frame parameters */
 
+	unsigned int uvc_num_requests;
+
 	/* Requests */
 	unsigned int req_size;
-	struct usb_request *req[UVC_NUM_REQUESTS];
-	__u8 *req_buffer[UVC_NUM_REQUESTS];
+	struct uvc_request *ureq;
 	struct list_head req_free;
 	spinlock_t req_lock;
 
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 61e2c94cc0b0c..ff0cc08531d2b 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -43,6 +43,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 {
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vq);
 	struct uvc_video *video = container_of(queue, struct uvc_video, queue);
+	struct usb_composite_dev *cdev = video->uvc->func.config->cdev;
 
 	if (*nbuffers > UVC_MAX_VIDEO_BUFFERS)
 		*nbuffers = UVC_MAX_VIDEO_BUFFERS;
@@ -51,6 +52,11 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 
 	sizes[0] = video->imagesize;
 
+	if (cdev->gadget->speed < USB_SPEED_SUPER)
+		video->uvc_num_requests = 4;
+	else
+		video->uvc_num_requests = 64;
+
 	return 0;
 }
 
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 633e23d58d868..303cb427f687e 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -145,7 +145,8 @@ static int uvcg_video_ep_queue(struct uvc_video *video, struct usb_request *req)
 static void
 uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
 {
-	struct uvc_video *video = req->context;
+	struct uvc_request *ureq = req->context;
+	struct uvc_video *video = ureq->video;
 	struct uvc_video_queue *queue = &video->queue;
 	unsigned long flags;
 
@@ -177,16 +178,21 @@ uvc_video_free_requests(struct uvc_video *video)
 {
 	unsigned int i;
 
-	for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
-		if (video->req[i]) {
-			usb_ep_free_request(video->ep, video->req[i]);
-			video->req[i] = NULL;
+	if (video->ureq) {
+		for (i = 0; i < video->uvc_num_requests; ++i) {
+			if (video->ureq[i].req) {
+				usb_ep_free_request(video->ep, video->ureq[i].req);
+				video->ureq[i].req = NULL;
+			}
+
+			if (video->ureq[i].req_buffer) {
+				kfree(video->ureq[i].req_buffer);
+				video->ureq[i].req_buffer = NULL;
+			}
 		}
 
-		if (video->req_buffer[i]) {
-			kfree(video->req_buffer[i]);
-			video->req_buffer[i] = NULL;
-		}
+		kfree(video->ureq);
+		video->ureq = NULL;
 	}
 
 	INIT_LIST_HEAD(&video->req_free);
@@ -207,21 +213,26 @@ uvc_video_alloc_requests(struct uvc_video *video)
 		 * max_t(unsigned int, video->ep->maxburst, 1)
 		 * (video->ep->mult);
 
-	for (i = 0; i < UVC_NUM_REQUESTS; ++i) {
-		video->req_buffer[i] = kmalloc(req_size, GFP_KERNEL);
-		if (video->req_buffer[i] == NULL)
+	video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
+	if (video->ureq == NULL)
+		return -ENOMEM;
+
+	for (i = 0; i < video->uvc_num_requests; ++i) {
+		video->ureq[i].req_buffer = kmalloc(req_size, GFP_KERNEL);
+		if (video->ureq[i].req_buffer == NULL)
 			goto error;
 
-		video->req[i] = usb_ep_alloc_request(video->ep, GFP_KERNEL);
-		if (video->req[i] == NULL)
+		video->ureq[i].req = usb_ep_alloc_request(video->ep, GFP_KERNEL);
+		if (video->ureq[i].req == NULL)
 			goto error;
 
-		video->req[i]->buf = video->req_buffer[i];
-		video->req[i]->length = 0;
-		video->req[i]->complete = uvc_video_complete;
-		video->req[i]->context = video;
+		video->ureq[i].req->buf = video->ureq[i].req_buffer;
+		video->ureq[i].req->length = 0;
+		video->ureq[i].req->complete = uvc_video_complete;
+		video->ureq[i].req->context = &video->ureq[i];
+		video->ureq[i].video = video;
 
-		list_add_tail(&video->req[i]->list, &video->req_free);
+		list_add_tail(&video->ureq[i].req->list, &video->req_free);
 	}
 
 	video->req_size = req_size;
@@ -312,9 +323,9 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
 		cancel_work_sync(&video->pump);
 		uvcg_queue_cancel(&video->queue, 0);
 
-		for (i = 0; i < UVC_NUM_REQUESTS; ++i)
-			if (video->req[i])
-				usb_ep_dequeue(video->ep, video->req[i]);
+		for (i = 0; i < video->uvc_num_requests; ++i)
+			if (video->ureq && video->ureq[i].req)
+				usb_ep_dequeue(video->ep, video->ureq[i].req);
 
 		uvc_video_free_requests(video);
 		uvcg_queue_enable(&video->queue, 0);
-- 
2.30.2

[PATCH v3 3/5] usb: gadget: uvc: set v4l2_dev->dev in f_uvc

[Michael Grzeschik]

The v4l2_dev has no corresponding device to it. We will
point it to the gadget's dev.

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: Extracted from previous bigger patch
v2 -> v3: -
---
 drivers/usb/gadget/function/f_uvc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index f48a00e497945..9d87c0fb8f92e 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -418,6 +418,7 @@ uvc_register_video(struct uvc_device *uvc)
 
 	/* TODO reference counting. */
 	uvc->vdev.v4l2_dev = &uvc->v4l2_dev;
+	uvc->vdev.v4l2_dev->dev = &cdev->gadget->dev;
 	uvc->vdev.fops = &uvc_v4l2_fops;
 	uvc->vdev.ioctl_ops = &uvc_v4l2_ioctl_ops;
 	uvc->vdev.release = video_device_release_empty;
-- 
2.30.2

[PATCH v3 4/5] usb: gadget: uvc: add scatter gather support

[Michael Grzeschik]

This patch adds support for scatter gather transfers. If the underlying
gadgets sg_supported == true, then the videeobuf2-dma-sg is used and the
encode routine maps all scatter entries to separate scatterlists for the
usb gadget.

When streaming 1080p with request size of 1024 times 3 bytes top shows a
difference of about 6.4% CPU load applying this patch:

 PID USER      PR  NI    VIRT    RES  %CPU  %MEM     TIME+ S COMMAND

  64 root       0 -20    0.0m   0.0m   7.7   0.0   0:01.25 I [kworker/u5:0-uvcvideo]
  83 root       0 -20    0.0m   0.0m   4.5   0.0   0:03.71 I [kworker/u5:3-uvcvideo]
 307 root     -51   0    0.0m   0.0m   3.8   0.0   0:01.05 S [irq/51-dwc3]

vs.

  64 root       0 -20    0.0m   0.0m   5.8   0.0   0:01.79 I [kworker/u5:0-uvcvideo]
 306 root     -51   0    0.0m   0.0m   3.2   0.0   0:01.97 S [irq/51-dwc3]
  82 root       0 -20    0.0m   0.0m   0.6   0.0   0:01.86 I [kworker/u5:1-uvcvideo]

Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

---
v1 -> v2: - fixed missing VIDEOBUF2_DMA_SG select for legacy webcam found by kbuild-all
	  - ordered header files alphabetically
	  - used unsigned int in encode_isoc_sg where possible
	  - changed function parameter order in uvcg_queue_init
          - added and used UVCG_REQUEST_HEADER_LEN for the 2 header bytes
	  - added use_sg into uvc_video_queue instead of pocking through layers
	  - added sg_free_table to uvc_video_free_requests
	  - removed unneeded memset
v2 -> v3: - setting queue->dev directly to udc
          - removed setting of alloc_devs in uvc_queue_setup
	  - added measurement results to patch description
---
 drivers/usb/gadget/Kconfig              |  1 +
 drivers/usb/gadget/function/uvc.h       |  2 +
 drivers/usb/gadget/function/uvc_queue.c | 22 ++++++-
 drivers/usb/gadget/function/uvc_queue.h |  7 ++-
 drivers/usb/gadget/function/uvc_video.c | 82 +++++++++++++++++++++++--
 drivers/usb/gadget/function/uvc_video.h |  2 +
 drivers/usb/gadget/legacy/Kconfig       |  1 +
 7 files changed, 108 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index 2d152571a7de8..dd58094f0b85b 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -450,6 +450,7 @@ config USB_CONFIGFS_F_UVC
 	depends on USB_CONFIGFS
 	depends on VIDEO_V4L2
 	depends on VIDEO_DEV
+	select VIDEOBUF2_DMA_SG
 	select VIDEOBUF2_VMALLOC
 	select USB_F_UVC
 	help
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 52f894183ecf0..36b78f8b3cd4c 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -75,6 +75,8 @@ struct uvc_request {
 	struct usb_request *req;
 	u8 *req_buffer;
 	struct uvc_video *video;
+	struct sg_table sgt;
+	u8 header[2];
 };
 
 struct uvc_video {
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index ff0cc08531d2b..7d00ad7c154c2 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -17,6 +17,7 @@
 #include <linux/wait.h>
 
 #include <media/v4l2-common.h>
+#include <media/videobuf2-dma-sg.h>
 #include <media/videobuf2-vmalloc.h>
 
 #include "uvc.h"
@@ -76,7 +77,12 @@ static int uvc_buffer_prepare(struct vb2_buffer *vb)
 		return -ENODEV;
 
 	buf->state = UVC_BUF_STATE_QUEUED;
-	buf->mem = vb2_plane_vaddr(vb, 0);
+	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->length = vb2_plane_size(vb, 0);
 	if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		buf->bytesused = 0;
@@ -116,9 +122,11 @@ static const struct vb2_ops uvc_queue_qops = {
 	.wait_finish = vb2_ops_wait_finish,
 };
 
-int uvcg_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
+int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2_buf_type type,
 		    struct mutex *lock)
 {
+	struct uvc_video *video = container_of(queue, struct uvc_video, queue);
+	struct usb_composite_dev *cdev = video->uvc->func.config->cdev;
 	int ret;
 
 	queue->queue.type = type;
@@ -127,9 +135,17 @@ int uvcg_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.ops = &uvc_queue_qops;
 	queue->queue.lock = lock;
-	queue->queue.mem_ops = &vb2_vmalloc_memops;
+	if (cdev->gadget->sg_supported) {
+		queue->queue.mem_ops = &vb2_dma_sg_memops;
+		queue->use_sg = 1;
+	} else {
+		queue->queue.mem_ops = &vb2_vmalloc_memops;
+	}
+
 	queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
 				     | V4L2_BUF_FLAG_TSTAMP_SRC_EOF;
+	queue->queue.dev = dev;
+
 	ret = vb2_queue_init(&queue->queue);
 	if (ret)
 		return ret;
diff --git a/drivers/usb/gadget/function/uvc_queue.h b/drivers/usb/gadget/function/uvc_queue.h
index 2f0fff7698430..05360a0767f61 100644
--- a/drivers/usb/gadget/function/uvc_queue.h
+++ b/drivers/usb/gadget/function/uvc_queue.h
@@ -34,6 +34,9 @@ struct uvc_buffer {
 
 	enum uvc_buffer_state state;
 	void *mem;
+	struct sg_table *sgt;
+	struct scatterlist *sg;
+	unsigned int offset;
 	unsigned int length;
 	unsigned int bytesused;
 };
@@ -50,6 +53,8 @@ struct uvc_video_queue {
 
 	unsigned int buf_used;
 
+	bool use_sg;
+
 	spinlock_t irqlock;	/* Protects flags and irqqueue */
 	struct list_head irqqueue;
 };
@@ -59,7 +64,7 @@ static inline int uvc_queue_streaming(struct uvc_video_queue *queue)
 	return vb2_is_streaming(&queue->queue);
 }
 
-int uvcg_queue_init(struct uvc_video_queue *queue, enum v4l2_buf_type type,
+int uvcg_queue_init(struct uvc_video_queue *queue, struct device *dev, enum v4l2_buf_type type,
 		    struct mutex *lock);
 
 void uvcg_free_buffers(struct uvc_video_queue *queue);
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 303cb427f687e..2cefb8bd4f15b 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -27,10 +27,10 @@ static int
 uvc_video_encode_header(struct uvc_video *video, struct uvc_buffer *buf,
 		u8 *data, int len)
 {
-	data[0] = 2;
+	data[0] = UVCG_REQUEST_HEADER_LEN;
 	data[1] = UVC_STREAM_EOH | video->fid;
 
-	if (buf->bytesused - video->queue.buf_used <= len - 2)
+	if (buf->bytesused - video->queue.buf_used <= len - UVCG_REQUEST_HEADER_LEN)
 		data[1] |= UVC_STREAM_EOF;
 
 	return 2;
@@ -94,6 +94,71 @@ uvc_video_encode_bulk(struct usb_request *req, struct uvc_video *video,
 		video->payload_size = 0;
 }
 
+static void
+uvc_video_encode_isoc_sg(struct usb_request *req, struct uvc_video *video,
+		struct uvc_buffer *buf)
+{
+	unsigned int pending = buf->bytesused - video->queue.buf_used;
+	struct uvc_request *ureq = req->context;
+	struct scatterlist *sg, *iter;
+	unsigned int len = video->req_size;
+	unsigned int sg_left, part = 0;
+	unsigned int i;
+	int ret;
+
+	sg = ureq->sgt.sgl;
+	sg_init_table(sg, ureq->sgt.nents);
+
+	/* Init the header. */
+	ret = uvc_video_encode_header(video, buf, ureq->header,
+				      video->req_size);
+	sg_set_buf(sg, ureq->header, UVCG_REQUEST_HEADER_LEN);
+	len -= ret;
+
+	if (pending <= len)
+		len = pending;
+
+	req->length = (len == pending) ?
+		len + UVCG_REQUEST_HEADER_LEN : video->req_size;
+
+	/* Init the pending sgs with payload */
+	sg = sg_next(sg);
+
+	for_each_sg(sg, iter, ureq->sgt.nents - 1, i) {
+		if (!len || !buf->sg)
+			break;
+
+		sg_left = sg_dma_len(buf->sg) - buf->offset;
+		part = min_t(unsigned int, len, sg_left);
+
+		sg_set_page(iter, sg_page(buf->sg), part, buf->offset);
+
+		if (part == sg_left) {
+			buf->offset = 0;
+			buf->sg = sg_next(buf->sg);
+		} else {
+			buf->offset += part;
+		}
+		len -= part;
+	}
+
+	/* Assign the video data with header. */
+	req->buf = NULL;
+	req->sg	= ureq->sgt.sgl;
+	req->num_sgs = i + 1;
+
+	req->length -= len;
+	video->queue.buf_used += req->length - UVCG_REQUEST_HEADER_LEN;
+
+	if (buf->bytesused == video->queue.buf_used || !buf->sg) {
+		video->queue.buf_used = 0;
+		buf->state = UVC_BUF_STATE_DONE;
+		buf->offset = 0;
+		uvcg_queue_next_buffer(&video->queue, buf);
+		video->fid ^= UVC_STREAM_FID;
+	}
+}
+
 static void
 uvc_video_encode_isoc(struct usb_request *req, struct uvc_video *video,
 		struct uvc_buffer *buf)
@@ -180,6 +245,8 @@ uvc_video_free_requests(struct uvc_video *video)
 
 	if (video->ureq) {
 		for (i = 0; i < video->uvc_num_requests; ++i) {
+			sg_free_table(&video->ureq[i].sgt);
+
 			if (video->ureq[i].req) {
 				usb_ep_free_request(video->ep, video->ureq[i].req);
 				video->ureq[i].req = NULL;
@@ -233,6 +300,10 @@ uvc_video_alloc_requests(struct uvc_video *video)
 		video->ureq[i].video = video;
 
 		list_add_tail(&video->ureq[i].req->list, &video->req_free);
+		/* req_size/PAGE_SIZE + 1 for overruns and + 1 for header */
+		sg_alloc_table(&video->ureq[i].sgt,
+			       DIV_ROUND_UP(req_size - 2, PAGE_SIZE) + 2,
+			       GFP_KERNEL);
 	}
 
 	video->req_size = req_size;
@@ -342,7 +413,8 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
 		video->encode = uvc_video_encode_bulk;
 		video->payload_size = 0;
 	} else
-		video->encode = uvc_video_encode_isoc;
+		video->encode = video->queue.use_sg ?
+			uvc_video_encode_isoc_sg : uvc_video_encode_isoc;
 
 	schedule_work(&video->pump);
 
@@ -366,8 +438,8 @@ int uvcg_video_init(struct uvc_video *video, struct uvc_device *uvc)
 	video->imagesize = 320 * 240 * 2;
 
 	/* Initialize the video buffers queue. */
-	uvcg_queue_init(&video->queue, V4L2_BUF_TYPE_VIDEO_OUTPUT,
-			&video->mutex);
+	uvcg_queue_init(&video->queue, uvc->v4l2_dev.dev->parent,
+			V4L2_BUF_TYPE_VIDEO_OUTPUT, &video->mutex);
 	return 0;
 }
 
diff --git a/drivers/usb/gadget/function/uvc_video.h b/drivers/usb/gadget/function/uvc_video.h
index 03adeefa343b7..9bf19475f6f9a 100644
--- a/drivers/usb/gadget/function/uvc_video.h
+++ b/drivers/usb/gadget/function/uvc_video.h
@@ -12,6 +12,8 @@
 #ifndef __UVC_VIDEO_H__
 #define __UVC_VIDEO_H__
 
+#define UVCG_REQUEST_HEADER_LEN			2
+
 struct uvc_video;
 
 int uvcg_video_enable(struct uvc_video *video, int enable);
diff --git a/drivers/usb/gadget/legacy/Kconfig b/drivers/usb/gadget/legacy/Kconfig
index 11dd6e8adc8a7..de6668e584815 100644
--- a/drivers/usb/gadget/legacy/Kconfig
+++ b/drivers/usb/gadget/legacy/Kconfig
@@ -502,6 +502,7 @@ config USB_G_WEBCAM
 	tristate "USB Webcam Gadget"
 	depends on VIDEO_V4L2
 	select USB_LIBCOMPOSITE
+	select VIDEOBUF2_DMA_SG
 	select VIDEOBUF2_VMALLOC
 	select USB_F_UVC
 	help
-- 
2.30.2

[PATCH v3 5/5] usb: gadget: uvc: decrease the interrupt load to a quarter

[Michael Grzeschik]

With usb3 we handle many more requests. Decrease the interrupt load by
only enabling the interrupt every quarter of the allocated requests.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>

--
v1 -> v2: - edited patch description
	  - removed extra parantheses
	  - added a comment for the logic
	  - using unsigned int instead of int
	  - reinitializing req_int_count in uvcg_video_enable
v2 -> v3: -
---
 drivers/usb/gadget/function/uvc.h       |  2 ++
 drivers/usb/gadget/function/uvc_video.c | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 36b78f8b3cd4c..255a61bd6a6a8 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -101,6 +101,8 @@ struct uvc_video {
 	struct list_head req_free;
 	spinlock_t req_lock;
 
+	unsigned int req_int_count;
+
 	void (*encode) (struct usb_request *req, struct uvc_video *video,
 			struct uvc_buffer *buf);
 
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index 2cefb8bd4f15b..b4a763e5f70e1 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -360,6 +360,19 @@ static void uvcg_video_pump(struct work_struct *work)
 
 		video->encode(req, video, buf);
 
+		/* With usb3 we have more requests. This will decrease the
+		 * interrupt load to a quarter but also catches the corner
+		 * cases, which needs to be handled */
+		if (list_empty(&video->req_free) ||
+		    buf->state == UVC_BUF_STATE_DONE ||
+		    !(video->req_int_count %
+		       DIV_ROUND_UP(video->uvc_num_requests, 4))) {
+			video->req_int_count = 0;
+			req->no_interrupt = 0;
+		} else {
+			req->no_interrupt = 1;
+		}
+
 		/* Queue the USB request */
 		ret = uvcg_video_ep_queue(video, req);
 		spin_unlock_irqrestore(&queue->irqlock, flags);
@@ -368,6 +381,7 @@ static void uvcg_video_pump(struct work_struct *work)
 			uvcg_queue_cancel(queue, 0);
 			break;
 		}
+		video->req_int_count++;
 	}
 
 	spin_lock_irqsave(&video->req_lock, flags);
@@ -416,6 +430,8 @@ int uvcg_video_enable(struct uvc_video *video, int enable)
 		video->encode = video->queue.use_sg ?
 			uvc_video_encode_isoc_sg : uvc_video_encode_isoc;
 
+	video->req_int_count = 0;
+
 	schedule_work(&video->pump);
 
 	return ret;
-- 
2.30.2

Re: [PATCH v3 0/5] usb: gadget: uvc: improve uvc gadget performance

[Michael Grzeschik]

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

I just realized that the previous series already should have been v3 and
this v4. However, the changelog of the patches is right and this is the
latest version. So there should be no need for resend.

Sorry for the mess, next time I will keep better bookkeeping.

Michael

On Mon, Jun 28, 2021 at 05:53:06PM +0200, Michael Grzeschik wrote:
>This series improves the performance of the uvc video gadget by adding a
>zero copy routine using the scatter list interface of the gadget. The
>series also increases the amount of allocated requests depending of the
>speed and it also reduces the interrupt load by only trigger on every
>16th request in case of super-speed.
>
>Michael Grzeschik (5):
>  usb: dwc3: gadget: set gadgets parent to the right controller
>  usb: gadget: uvc: make uvc_num_requests depend on gadget speed
>  usb: gadget: uvc: set v4l2_dev->dev in f_uvc
>  usb: gadget: uvc: add scatter gather support
>  usb: gadget: uvc: decrease the interrupt load to a quarter
>
> drivers/usb/dwc3/gadget.c               |   2 +-
> drivers/usb/gadget/Kconfig              |   1 +
> drivers/usb/gadget/function/f_uvc.c     |   1 +
> drivers/usb/gadget/function/uvc.h       |  15 ++-
> drivers/usb/gadget/function/uvc_queue.c |  28 ++++-
> drivers/usb/gadget/function/uvc_queue.h |   7 +-
> drivers/usb/gadget/function/uvc_video.c | 155 +++++++++++++++++++-----
> drivers/usb/gadget/function/uvc_video.h |   2 +
> drivers/usb/gadget/legacy/Kconfig       |   1 +
> 9 files changed, 176 insertions(+), 36 deletions(-)
>
>-- 
>2.30.2
>
>
>

-- 
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 --]