[PATCH] media: uvcvideo: use dynamic allocation for uvc_copy_op

* [PATCH] media: uvcvideo: use dynamic allocation for uvc_copy_op
@ 2021-09-02  8:49 Kyeongdon Kim
  2021-09-02 15:18 ` kernel test robot
  2021-09-02 17:30 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Kyeongdon Kim @ 2021-09-02  8:49 UTC (permalink / raw)
  To: laurent.pinchart; +Cc: mchehab, linux-media, linux-kernel, Kyeongdon Kim

There are some issues to handle frame throughput with camera devices

Using devices:
- Logitech Webcam
- Intel(R) RealSense(TM) Depth Camera

To improve efficiency, and maximise throughput,
use dynamic allocation for uvc_copy_op.

Change from struct uvc_copy_op copy_operations[UVC_MAX_PACKETS];
to struct uvc_copy_op *copy_operations;

Now, only tested bulk video options.

On test device & own debug log to check frame duration(us),
refer to test result the below:

Use copy_operations[UVC_MAX_PACKETS]
[UVC] Check time duration(us) : 64732 / 66000
[UVC] Check time duration(us) : 67452 / 66000
[UVC] Check time duration(us) : 67413 / 66000
[UVC] Check time duration(us) : 66713 / 66000
[UVC] Check time duration(us) : 67967 / 66000

Use *copy_operations
[UVC] Check time duration(us) : 30804 / 66000
[UVC] Check time duration(us) : 38642 / 66000
[UVC] Check time duration(us) : 26011 / 66000
[UVC] Check time duration(us) : 30116 / 66000
[UVC] Check time duration(us) : 29265 / 66000

Signed-off-by: Kyeongdon Kim <kyeongdon.kim@lge.com>
---
 drivers/media/usb/uvc/uvc_video.c | 52 ++++++++++++++++++++++++++++++++++++---
 drivers/media/usb/uvc/uvcvideo.h  |  3 ++-
 2 files changed, 51 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
index e164646..32bc98e 100644
--- a/drivers/media/usb/uvc/uvc_video.c
+++ b/drivers/media/usb/uvc/uvc_video.c
@@ -1614,6 +1614,33 @@ static void uvc_video_complete(struct urb *urb)
 	queue_work(stream->async_wq, &uvc_urb->work);
 }
 
+static void uvc_free_urb_cop(struct uvc_streaming *stream)
+{
+	for_each_uvc_urb(uvc_urb, stream) {
+		if (uvc_urb->copy_operations) {
+			kfree(uvc_urb->copy_operations);
+			uvc_urb->copy_operations = NULL;
+		}
+	}
+}
+
+static int uvc_alloc_urb_cop(struct uvc_streaming *stream, gfp_t gfp_flags)
+{
+	int max_packet = stream->urb_max_packets;
+
+	for_each_uvc_urb(uvc_urb, stream) {
+		uvc_urb->copy_operations
+				= kcalloc(max_packet, sizeof(struct uvc_copy_op), gfp_flags);
+		if (uvc_urb->copy_operations == NULL)
+			goto error;
+	}
+	return 0;
+error:
+	uvc_free_urb_cop(stream);
+
+	return -ENOMEM;
+}
+
 /*
  * Free transfer buffers.
  */
@@ -1687,8 +1714,8 @@ static int uvc_alloc_urb_buffers(struct uvc_streaming *stream,
 	 * payloads across multiple URBs.
 	 */
 	npackets = DIV_ROUND_UP(size, psize);
-	if (npackets > UVC_MAX_PACKETS)
-		npackets = UVC_MAX_PACKETS;
+	if (npackets > stream->urb_max_packets)
+		npackets = stream->urb_max_packets;
 
 	/* Retry allocations until one succeed. */
 	for (; npackets > 1; npackets /= 2) {
@@ -1744,8 +1771,10 @@ static void uvc_video_stop_transfer(struct uvc_streaming *stream,
 		uvc_urb->urb = NULL;
 	}
 
-	if (free_buffers)
+	if (free_buffers) {
 		uvc_free_urb_buffers(stream);
+		uvc_free_urb_cop(stream);
+	}
 }
 
 /*
@@ -1790,10 +1819,18 @@ static int uvc_init_video_isoc(struct uvc_streaming *stream,
 	psize = uvc_endpoint_max_bpi(stream->dev->udev, ep);
 	size = stream->ctrl.dwMaxVideoFrameSize;
 
+	stream->urb_max_packets = UVC_MAX_PACKETS;
+
 	npackets = uvc_alloc_urb_buffers(stream, size, psize, gfp_flags);
 	if (npackets == 0)
 		return -ENOMEM;
 
+	if (uvc_alloc_urb_cop(stream, gfp_flags) != 0) {
+		uvc_dbg(stream->dev, VIDEO,
+				"Failed to init URBs copy operations.\n");
+		return -ENOMEM;
+	}
+
 	size = npackets * psize;
 
 	for_each_uvc_urb(uvc_urb, stream) {
@@ -1842,11 +1879,18 @@ static int uvc_init_video_bulk(struct uvc_streaming *stream,
 	psize = usb_endpoint_maxp(&ep->desc);
 	size = stream->ctrl.dwMaxPayloadTransferSize;
 	stream->bulk.max_payload_size = size;
+	stream->urb_max_packets = DIV_ROUND_UP(size, psize);
 
 	npackets = uvc_alloc_urb_buffers(stream, size, psize, gfp_flags);
 	if (npackets == 0)
 		return -ENOMEM;
 
+	if (uvc_alloc_urb_cop(stream, gfp_flags) != 0) {
+		uvc_dbg(stream->dev, VIDEO,
+				"Failed to init URBs copy operations.\n");
+		return -ENOMEM;
+	}
+
 	size = npackets * psize;
 
 	if (usb_endpoint_dir_in(&ep->desc))
@@ -2147,6 +2191,8 @@ int uvc_video_init(struct uvc_streaming *stream)
 		}
 	}
 
+	stream->urb_max_packets = UVC_MAX_PACKETS;
+
 	/* Prepare asynchronous work items. */
 	for_each_uvc_urb(uvc_urb, stream)
 		INIT_WORK(&uvc_urb->work, uvc_video_copy_data_work);
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index cce5e38..00cf6c9 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -561,7 +561,7 @@ struct uvc_urb {
 	struct sg_table *sgt;
 
 	unsigned int async_operations;
-	struct uvc_copy_op copy_operations[UVC_MAX_PACKETS];
+	struct uvc_copy_op *copy_operations;
 	struct work_struct work;
 };
 
@@ -616,6 +616,7 @@ struct uvc_streaming {
 
 	struct uvc_urb uvc_urb[UVC_URBS];
 	unsigned int urb_size;
+	unsigned int urb_max_packets;
 
 	u32 sequence;
 	u8 last_fid;
-- 
2.10.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread