All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: uvc: add bulk transfer support
@ 2022-02-25  7:00 3090101217
  2022-05-12  9:42 ` [PATCH v2] " 3090101217
  0 siblings, 1 reply; 10+ messages in thread
From: 3090101217 @ 2022-02-25  7:00 UTC (permalink / raw)
  To: gregkh, corbet, laurent.pinchart, balbi, bilbao, mchehab+huawei, rdunlap
  Cc: linux-kernel, linux-usb, linux-doc, Jing Leng

From: Jing Leng <jleng@ambarella.com>

The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

In user space, isochronous/bulk modes are handled a little differently:

1. APP prepares buffers and streams when it receives an UVC_EVENT_STREAMON
message in the isochronous mode, but APP should do them when it receives a
SET_CUR request of UVC_VS_COMMIT_CONTROL due to no UVC_EVENT_STREAMON
message reported from the kernel in the bulk mode (Do them only once).

2. In Addition, APP should set the value of dwMaxPayloadTransferSize to
streaming_maxpacket in the isochronous mode or streaming_bulk_mult * 1024
in the bulk mode.

Here shows an example of the configfs differences:
  if [ $BULK -eq 1 ]; then
      echo 128 > functions/$FUNC/streaming_bulk_mult
  else
      echo 1024 > functions/$FUNC/streaming_maxpacket
  fi

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
 .../ABI/testing/configfs-usb-gadget-uvc       |   1 +
 Documentation/usb/gadget-testing.rst          |   4 +
 drivers/usb/gadget/function/f_uvc.c           | 245 ++++++++++++------
 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, 175 insertions(+), 82 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..52ca04a619ff 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -7,6 +7,7 @@ Description:	UVC function directory
 		streaming_maxburst	0..15 (ss only)
 		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
 		streaming_interval	1..16
+		streaming_bulk_mult	0..0x3fffffU
 		===================	=============================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/control
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index cbbd948c626f..58f58979f49c 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -784,6 +784,10 @@ The uvc function provides these attributes in its function directory:
 	streaming_maxpacket maximum packet size this endpoint is capable of
 			    sending or receiving when this configuration is
 			    selected
+	streaming_bulk_mult Multiples to configure max_payload_size. If it's
+			    0, the transport mode is isochronous; otherwise
+			    the transport mode is bulk and max_payload_size
+			    is equal to streaming_bulk_mult * 1024.
 	=================== ================================================
 
 There are also "control" and "streaming" subdirectories, each of which contain
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 71bb5e477dba..61a05aa301ec 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -178,19 +178,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
 	 */
 };
 
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -251,9 +251,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 
 void uvc_function_setup_continue(struct uvc_device *uvc)
 {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
 	struct usb_composite_dev *cdev = uvc->func.config->cdev;
 
-	usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (!opts->streaming_bulk_mult)
+		usb_composite_setup_continue(cdev);
 }
 
 static int
@@ -278,6 +281,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
 	int ret;
 
 	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -310,49 +314,72 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	if (interface != uvc->streaming_intf)
 		return -EINVAL;
 
-	/* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/
+	if (opts->streaming_bulk_mult) {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
 
-	switch (alt) {
-	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+			if (!uvc->video.ep)
+				return -EINVAL;
 
-		if (uvc->video.ep)
+			uvcg_info(f, "reset UVC\n");
 			usb_ep_disable(uvc->video.ep);
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMOFF;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
 
-		uvc->state = UVC_STATE_CONNECTED;
-		return 0;
-
-	case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 			return 0;
 
-		if (!uvc->video.ep)
+		default:
 			return -EINVAL;
+		}
+	} else {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
 
-		uvcg_info(f, "reset UVC\n");
-		usb_ep_disable(uvc->video.ep);
+			if (uvc->video.ep)
+				usb_ep_disable(uvc->video.ep);
 
-		ret = config_ep_by_speed(f->config->cdev->gadget,
-				&(uvc->func), uvc->video.ep);
-		if (ret)
-			return ret;
-		usb_ep_enable(uvc->video.ep);
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMON;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
-		return USB_GADGET_DELAYED_STATUS;
+			uvc->state = UVC_STATE_CONNECTED;
+			return 0;
 
-	default:
-		return -EINVAL;
+		case 1:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
+
+			if (!uvc->video.ep)
+				return -EINVAL;
+
+			uvcg_info(f, "reset UVC\n");
+			usb_ep_disable(uvc->video.ep);
+
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
+
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMON;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
+			return USB_GADGET_DELAYED_STATUS;
+
+		default:
+			return -EINVAL;
+		}
 	}
 }
 
@@ -598,57 +625,90 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvcg_info(f, "%s()\n", __func__);
 
 	opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters.
-	 */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
 
-	/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_bulk_mult) {
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_XFER_BULK;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_bulk_mult * 1024;
 	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_SYNC_ASYNC
+						| USB_ENDPOINT_XFER_ISOC;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters.
+		 */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
 
-	uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
 
-	uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
 
-	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
 
-	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
+
+		uvc->video.max_payload_size = 0;
+	}
 
 	/* Allocate endpoints. */
 	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -662,7 +722,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
 					  &uvc_ss_streaming_comp);
 	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
 	else
 		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 
@@ -703,6 +763,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
+	/* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_bulk_mult) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
 	/* Copy descriptors */
 	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
 	if (IS_ERR(f->fs_descriptors)) {
@@ -866,6 +948,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
+	opts->streaming_bulk_mult = 0;
 
 	ret = uvcg_attach_configfs(opts);
 	if (ret < 0) {
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 9a01a7d4f17f..5607a239d55e 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
 	unsigned int					streaming_interval;
 	unsigned int					streaming_maxpacket;
 	unsigned int					streaming_maxburst;
+	unsigned int					streaming_bulk_mult;
 
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..9b08e7b25168 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2422,6 +2422,7 @@ UVC_ATTR(f_uvc_opts_, cname, cname)
 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(streaming_bulk_mult, streaming_bulk_mult, 0x3fffffU);
 
 #undef UVCG_OPTS_ATTR
 
@@ -2429,6 +2430,7 @@ static struct configfs_attribute *uvc_attrs[] = {
 	&f_uvc_opts_attr_streaming_interval,
 	&f_uvc_opts_attr_streaming_maxpacket,
 	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_bulk_mult,
 	NULL,
 };
 
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index d852ac9e47e7..e866efa1e7da 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -135,7 +135,9 @@ 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) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
 		queue->queue.mem_ops = &vb2_dma_sg_memops;
 		queue->use_sg = 1;
 	} else {
-- 
2.17.1


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

* [PATCH v2] usb: gadget: uvc: add bulk transfer support
  2022-02-25  7:00 [PATCH] usb: gadget: uvc: add bulk transfer support 3090101217
@ 2022-05-12  9:42 ` 3090101217
  2022-05-12 14:47   ` kernel test robot
  2022-05-13  0:42   ` [PATCH v3] " 3090101217
  0 siblings, 2 replies; 10+ messages in thread
From: 3090101217 @ 2022-05-12  9:42 UTC (permalink / raw)
  To: gregkh, corbet, laurent.pinchart, balbi, rdunlap, mchehab+huawei, bilbao
  Cc: linux-doc, linux-kernel, linux-usb, Jing Leng

From: Jing Leng <jleng@ambarella.com>

The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

In user space, isochronous/bulk modes are handled a little differently:

1. APP prepares buffers and streams when it receives an UVC_EVENT_STREAMON
message in the isochronous mode, but APP should do them when it receives a
SET_CUR request of UVC_VS_COMMIT_CONTROL due to no UVC_EVENT_STREAMON
message reported from the kernel in the bulk mode (Do them only once).

2. In Addition, APP should set the value of dwMaxPayloadTransferSize to
streaming_maxpacket in the isochronous mode or streaming_bulk_mult * 1024
in the bulk mode.

Here shows an example of the configfs differences:
  if [ $BULK -eq 1 ]; then
      echo 128 > functions/$FUNC/streaming_bulk_mult
  else
      echo 1024 > functions/$FUNC/streaming_maxpacket
  fi

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v1->v2:
- Handle imagesize in uvc_v4l2_set_format. If it's not handled,
- switching from low resolution to high resolution will fail to play.
---
 .../ABI/testing/configfs-usb-gadget-uvc       |   1 +
 Documentation/usb/gadget-testing.rst          |   4 +
 drivers/usb/gadget/function/f_uvc.c           | 245 ++++++++++++------
 drivers/usb/gadget/function/u_uvc.h           |   1 +
 drivers/usb/gadget/function/uvc_configfs.c    |   2 +
 drivers/usb/gadget/function/uvc_queue.c       |   4 +-
 drivers/usb/gadget/function/uvc_v4l2.c        |   8 +
 7 files changed, 183 insertions(+), 82 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..52ca04a619ff 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -7,6 +7,7 @@ Description:	UVC function directory
 		streaming_maxburst	0..15 (ss only)
 		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
 		streaming_interval	1..16
+		streaming_bulk_mult	0..0x3fffffU
 		===================	=============================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/control
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index c6d034abce3a..2cbe3e2e05c3 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -787,6 +787,10 @@ The uvc function provides these attributes in its function directory:
 	streaming_maxpacket maximum packet size this endpoint is capable of
 			    sending or receiving when this configuration is
 			    selected
+	streaming_bulk_mult Multiples to configure max_payload_size. If it's
+			    0, the transport mode is isochronous; otherwise
+			    the transport mode is bulk and max_payload_size
+			    is equal to streaming_bulk_mult * 1024.
 	=================== ================================================
 
 There are also "control" and "streaming" subdirectories, each of which contain
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 71bb5e477dba..61a05aa301ec 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -178,19 +178,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
 	 */
 };
 
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -251,9 +251,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 
 void uvc_function_setup_continue(struct uvc_device *uvc)
 {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
 	struct usb_composite_dev *cdev = uvc->func.config->cdev;
 
-	usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (!opts->streaming_bulk_mult)
+		usb_composite_setup_continue(cdev);
 }
 
 static int
@@ -278,6 +281,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
 	int ret;
 
 	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -310,49 +314,72 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	if (interface != uvc->streaming_intf)
 		return -EINVAL;
 
-	/* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/
+	if (opts->streaming_bulk_mult) {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
 
-	switch (alt) {
-	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+			if (!uvc->video.ep)
+				return -EINVAL;
 
-		if (uvc->video.ep)
+			uvcg_info(f, "reset UVC\n");
 			usb_ep_disable(uvc->video.ep);
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMOFF;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
 
-		uvc->state = UVC_STATE_CONNECTED;
-		return 0;
-
-	case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 			return 0;
 
-		if (!uvc->video.ep)
+		default:
 			return -EINVAL;
+		}
+	} else {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
 
-		uvcg_info(f, "reset UVC\n");
-		usb_ep_disable(uvc->video.ep);
+			if (uvc->video.ep)
+				usb_ep_disable(uvc->video.ep);
 
-		ret = config_ep_by_speed(f->config->cdev->gadget,
-				&(uvc->func), uvc->video.ep);
-		if (ret)
-			return ret;
-		usb_ep_enable(uvc->video.ep);
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMON;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
-		return USB_GADGET_DELAYED_STATUS;
+			uvc->state = UVC_STATE_CONNECTED;
+			return 0;
 
-	default:
-		return -EINVAL;
+		case 1:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
+
+			if (!uvc->video.ep)
+				return -EINVAL;
+
+			uvcg_info(f, "reset UVC\n");
+			usb_ep_disable(uvc->video.ep);
+
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
+
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMON;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
+			return USB_GADGET_DELAYED_STATUS;
+
+		default:
+			return -EINVAL;
+		}
 	}
 }
 
@@ -598,57 +625,90 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvcg_info(f, "%s()\n", __func__);
 
 	opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters.
-	 */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
 
-	/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_bulk_mult) {
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_XFER_BULK;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_bulk_mult * 1024;
 	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_SYNC_ASYNC
+						| USB_ENDPOINT_XFER_ISOC;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters.
+		 */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
 
-	uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
 
-	uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
 
-	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
 
-	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
+
+		uvc->video.max_payload_size = 0;
+	}
 
 	/* Allocate endpoints. */
 	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -662,7 +722,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
 					  &uvc_ss_streaming_comp);
 	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
 	else
 		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 
@@ -703,6 +763,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
+	/* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_bulk_mult) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
 	/* Copy descriptors */
 	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
 	if (IS_ERR(f->fs_descriptors)) {
@@ -866,6 +948,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
+	opts->streaming_bulk_mult = 0;
 
 	ret = uvcg_attach_configfs(opts);
 	if (ret < 0) {
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 9a01a7d4f17f..5607a239d55e 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
 	unsigned int					streaming_interval;
 	unsigned int					streaming_maxpacket;
 	unsigned int					streaming_maxburst;
+	unsigned int					streaming_bulk_mult;
 
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..9b08e7b25168 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2422,6 +2422,7 @@ UVC_ATTR(f_uvc_opts_, cname, cname)
 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(streaming_bulk_mult, streaming_bulk_mult, 0x3fffffU);
 
 #undef UVCG_OPTS_ATTR
 
@@ -2429,6 +2430,7 @@ static struct configfs_attribute *uvc_attrs[] = {
 	&f_uvc_opts_attr_streaming_interval,
 	&f_uvc_opts_attr_streaming_maxpacket,
 	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_bulk_mult,
 	NULL,
 };
 
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 2cda982f3765..98d0e933b5e1 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -135,7 +135,9 @@ 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) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
 		queue->queue.mem_ops = &vb2_dma_sg_memops;
 		queue->use_sg = 1;
 	} else {
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a2c78690c5c2..767f1a2ace04 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -119,6 +119,14 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt)
 	bpl = format->bpp * fmt->fmt.pix.width / 8;
 	imagesize = bpl ? bpl * fmt->fmt.pix.height : fmt->fmt.pix.sizeimage;
 
+	/*
+	 * Bulk mode only allocates memory once, so user should give the
+	 * maximum image size in all formats and kernel should not decrease
+	 * the imagesize.
+	 */
+	if (video->max_payload_size && imagesize < fmt->fmt.pix.sizeimage)
+		imagesize = fmt->fmt.pix.sizeimage;
+
 	video->fcc = format->fcc;
 	video->bpp = format->bpp;
 	video->width = fmt->fmt.pix.width;
-- 
2.17.1


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

* Re: [PATCH v2] usb: gadget: uvc: add bulk transfer support
  2022-05-12  9:42 ` [PATCH v2] " 3090101217
@ 2022-05-12 14:47   ` kernel test robot
  2022-05-13  0:42   ` [PATCH v3] " 3090101217
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2022-05-12 14:47 UTC (permalink / raw)
  To: 3090101217, gregkh, corbet, laurent.pinchart, balbi, rdunlap,
	mchehab+huawei, bilbao
  Cc: kbuild-all, linux-doc, linux-kernel, linux-usb, Jing Leng

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on v5.18-rc6]
[cannot apply to usb/usb-testing balbi-usb/testing/next next-20220512]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/3090101217-zju-edu-cn/usb-gadget-uvc-add-bulk-transfer-support/20220512-174815
base:    c5eb0a61238dd6faf37f58c9ce61c9980aaffd7a
config: sh-allmodconfig (https://download.01.org/0day-ci/archive/20220512/202205122203.E1HlX3x5-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/39a05ac7e24c36ed8f2aa2f65e4225d75a96a894
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review 3090101217-zju-edu-cn/usb-gadget-uvc-add-bulk-transfer-support/20220512-174815
        git checkout 39a05ac7e24c36ed8f2aa2f65e4225d75a96a894
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash drivers/usb/gadget/function/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/usb/gadget/function/f_uvc.c: In function 'uvc_function_bind':
>> drivers/usb/gadget/function/f_uvc.c:767:9: error: 'i' undeclared (first use in this function)
     767 |         i = 0;
         |         ^
   drivers/usb/gadget/function/f_uvc.c:767:9: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/usb/gadget/function/f_uvc.c:773:39: error: implicit declaration of function 'USBDHDR' [-Werror=implicit-function-declaration]
     773 |                 uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
         |                                       ^~~~~~~
   cc1: some warnings being treated as errors


vim +/i +767 drivers/usb/gadget/function/f_uvc.c

   612	
   613	static int
   614	uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
   615	{
   616		struct usb_composite_dev *cdev = c->cdev;
   617		struct uvc_device *uvc = to_uvc(f);
   618		struct usb_string *us;
   619		unsigned int max_packet_mult;
   620		unsigned int max_packet_size;
   621		struct usb_ep *ep;
   622		struct f_uvc_opts *opts;
   623		int ret = -EINVAL;
   624	
   625		uvcg_info(f, "%s()\n", __func__);
   626	
   627		opts = fi_to_f_uvc_opts(f->fi);
   628	
   629		/* Handle different transfer mode for stream endpoints */
   630		if (opts->streaming_bulk_mult) {
   631			uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_XFER_BULK;
   632			uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
   633			uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
   634	
   635			opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
   636	
   637			uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
   638			uvc_fs_streaming_ep.bInterval = 0;
   639	
   640			uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
   641			uvc_hs_streaming_ep.bInterval = 0;
   642	
   643			uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
   644			uvc_ss_streaming_ep.bInterval = 0;
   645	
   646			uvc_ss_streaming_comp.bmAttributes = 0;
   647			uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
   648			uvc_ss_streaming_comp.wBytesPerInterval = 0;
   649	
   650			uvc->video.max_payload_size = opts->streaming_bulk_mult * 1024;
   651		} else {
   652			uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_SYNC_ASYNC
   653							| USB_ENDPOINT_XFER_ISOC;
   654			uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
   655			uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
   656	
   657			/* Sanity check the streaming endpoint module parameters.
   658			 */
   659			opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
   660			opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
   661			opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
   662	
   663			/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
   664			if (opts->streaming_maxburst &&
   665				(opts->streaming_maxpacket % 1024) != 0) {
   666				opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
   667				uvcg_info(f, "overriding streaming_maxpacket to %d\n",
   668					opts->streaming_maxpacket);
   669			}
   670	
   671			/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
   672			 * module parameters.
   673			 *
   674			 * NOTE: We assume that the user knows what they are doing and won't
   675			 * give parameters that their UDC doesn't support.
   676			 */
   677	
   678			if (opts->streaming_maxpacket <= 1024) {
   679				max_packet_mult = 0;
   680				max_packet_size = opts->streaming_maxpacket;
   681			} else if (opts->streaming_maxpacket <= 2048) {
   682				max_packet_mult = 1;
   683				max_packet_size = opts->streaming_maxpacket / 2;
   684			} else {
   685				max_packet_mult = 2;
   686				max_packet_size = opts->streaming_maxpacket / 3;
   687			}
   688	
   689			uvc_fs_streaming_ep.wMaxPacketSize =
   690				cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
   691			uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
   692	
   693			uvc_hs_streaming_ep.wMaxPacketSize =
   694				cpu_to_le16(max_packet_size | (max_packet_mult << 11));
   695			/* A high-bandwidth endpoint must specify a bInterval value of 1 */
   696			if (max_packet_mult > 0)
   697				uvc_hs_streaming_ep.bInterval = 1;
   698			else
   699				uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
   700	
   701			uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
   702			uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
   703	
   704			uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
   705			uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
   706			uvc_ss_streaming_comp.wBytesPerInterval =
   707				cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
   708					(opts->streaming_maxburst + 1));
   709	
   710			uvc->video.max_payload_size = 0;
   711		}
   712	
   713		/* Allocate endpoints. */
   714		ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
   715		if (!ep) {
   716			uvcg_info(f, "Unable to allocate control EP\n");
   717			goto error;
   718		}
   719		uvc->control_ep = ep;
   720	
   721		if (gadget_is_superspeed(c->cdev->gadget))
   722			ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
   723						  &uvc_ss_streaming_comp);
   724		else if (gadget_is_dualspeed(cdev->gadget))
   725			ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
   726		else
   727			ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
   728	
   729		if (!ep) {
   730			uvcg_info(f, "Unable to allocate streaming EP\n");
   731			goto error;
   732		}
   733		uvc->video.ep = ep;
   734	
   735		uvc_fs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
   736		uvc_hs_streaming_ep.bEndpointAddress = uvc->video.ep->address;
   737		uvc_ss_streaming_ep.bEndpointAddress = uvc->video.ep->address;
   738	
   739		us = usb_gstrings_attach(cdev, uvc_function_strings,
   740					 ARRAY_SIZE(uvc_en_us_strings));
   741		if (IS_ERR(us)) {
   742			ret = PTR_ERR(us);
   743			goto error;
   744		}
   745		uvc_iad.iFunction = us[UVC_STRING_CONTROL_IDX].id;
   746		uvc_control_intf.iInterface = us[UVC_STRING_CONTROL_IDX].id;
   747		ret = us[UVC_STRING_STREAMING_IDX].id;
   748		uvc_streaming_intf_alt0.iInterface = ret;
   749		uvc_streaming_intf_alt1.iInterface = ret;
   750	
   751		/* Allocate interface IDs. */
   752		if ((ret = usb_interface_id(c, f)) < 0)
   753			goto error;
   754		uvc_iad.bFirstInterface = ret;
   755		uvc_control_intf.bInterfaceNumber = ret;
   756		uvc->control_intf = ret;
   757		opts->control_interface = ret;
   758	
   759		if ((ret = usb_interface_id(c, f)) < 0)
   760			goto error;
   761		uvc_streaming_intf_alt0.bInterfaceNumber = ret;
   762		uvc_streaming_intf_alt1.bInterfaceNumber = ret;
   763		uvc->streaming_intf = ret;
   764		opts->streaming_interface = ret;
   765	
   766		/* Handle different transfer mode for descriptors */
 > 767		i = 0;
   768		if (opts->streaming_bulk_mult) {
   769			uvc_streaming_intf_alt0.bNumEndpoints = 1;
   770		} else {
   771			uvc_streaming_intf_alt0.bNumEndpoints = 0;
   772	
 > 773			uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
   774			uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
   775			uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
   776			++i;
   777		}
   778		uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
   779		uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
   780		uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
   781		++i;
   782		uvc_fs_streaming[i] = NULL;
   783		uvc_hs_streaming[i] = NULL;
   784		uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
   785		++i;
   786		uvc_ss_streaming[i] = NULL;
   787	
   788		/* Copy descriptors */
   789		f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
   790		if (IS_ERR(f->fs_descriptors)) {
   791			ret = PTR_ERR(f->fs_descriptors);
   792			f->fs_descriptors = NULL;
   793			goto error;
   794		}
   795		if (gadget_is_dualspeed(cdev->gadget)) {
   796			f->hs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_HIGH);
   797			if (IS_ERR(f->hs_descriptors)) {
   798				ret = PTR_ERR(f->hs_descriptors);
   799				f->hs_descriptors = NULL;
   800				goto error;
   801			}
   802		}
   803		if (gadget_is_superspeed(c->cdev->gadget)) {
   804			f->ss_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_SUPER);
   805			if (IS_ERR(f->ss_descriptors)) {
   806				ret = PTR_ERR(f->ss_descriptors);
   807				f->ss_descriptors = NULL;
   808				goto error;
   809			}
   810		}
   811	
   812		/* Preallocate control endpoint request. */
   813		uvc->control_req = usb_ep_alloc_request(cdev->gadget->ep0, GFP_KERNEL);
   814		uvc->control_buf = kmalloc(UVC_MAX_REQUEST_SIZE, GFP_KERNEL);
   815		if (uvc->control_req == NULL || uvc->control_buf == NULL) {
   816			ret = -ENOMEM;
   817			goto error;
   818		}
   819	
   820		uvc->control_req->buf = uvc->control_buf;
   821		uvc->control_req->complete = uvc_function_ep0_complete;
   822		uvc->control_req->context = uvc;
   823	
   824		if (v4l2_device_register(&cdev->gadget->dev, &uvc->v4l2_dev)) {
   825			uvcg_err(f, "failed to register V4L2 device\n");
   826			goto error;
   827		}
   828	
   829		/* Initialise video. */
   830		ret = uvcg_video_init(&uvc->video, uvc);
   831		if (ret < 0)
   832			goto v4l2_error;
   833	
   834		/* Register a V4L2 device. */
   835		ret = uvc_register_video(uvc);
   836		if (ret < 0) {
   837			uvcg_err(f, "failed to register video device\n");
   838			goto v4l2_error;
   839		}
   840	
   841		return 0;
   842	
   843	v4l2_error:
   844		v4l2_device_unregister(&uvc->v4l2_dev);
   845	error:
   846		if (uvc->control_req)
   847			usb_ep_free_request(cdev->gadget->ep0, uvc->control_req);
   848		kfree(uvc->control_buf);
   849	
   850		usb_free_all_descriptors(f);
   851		return ret;
   852	}
   853	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

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

* [PATCH v3] usb: gadget: uvc: add bulk transfer support
  2022-05-12  9:42 ` [PATCH v2] " 3090101217
  2022-05-12 14:47   ` kernel test robot
@ 2022-05-13  0:42   ` 3090101217
  2022-10-25  9:05     ` [PATCH v4] " 3090101217
  2022-11-03  6:13     ` [PATCH v5] " Jing Leng
  1 sibling, 2 replies; 10+ messages in thread
From: 3090101217 @ 2022-05-13  0:42 UTC (permalink / raw)
  To: gregkh, corbet, laurent.pinchart, balbi, rdunlap, mchehab+huawei, bilbao
  Cc: linux-doc, linux-kernel, linux-usb, Jing Leng

From: Jing Leng <jleng@ambarella.com>

The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

In user space, isochronous/bulk modes are handled a little differently:

1. APP prepares buffers and streams when it receives an UVC_EVENT_STREAMON
message in the isochronous mode, but APP should do them when it receives a
SET_CUR request of UVC_VS_COMMIT_CONTROL due to no UVC_EVENT_STREAMON
message reported from the kernel in the bulk mode (Do them only once).

2. In Addition, APP should set the value of dwMaxPayloadTransferSize to
streaming_maxpacket in the isochronous mode or streaming_bulk_mult * 1024
in the bulk mode.

Here shows an example of the configfs differences:
  if [ $BULK -eq 1 ]; then
      echo 128 > functions/$FUNC/streaming_bulk_mult
  else
      echo 1024 > functions/$FUNC/streaming_maxpacket
  fi

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v2->v3:
- Mistakenly deleted the definition of i and USBDHDR when porting from my workdir.
- Reported-by: kernel test robot <lkp@intel.com>
ChangeLog v1->v2:
- Handle imagesize in uvc_v4l2_set_format. If it's not handled,
- switching from low resolution to high resolution will fail to play.
---
 .../ABI/testing/configfs-usb-gadget-uvc       |   1 +
 Documentation/usb/gadget-testing.rst          |   4 +
 drivers/usb/gadget/function/f_uvc.c           | 248 ++++++++++++------
 drivers/usb/gadget/function/u_uvc.h           |   1 +
 drivers/usb/gadget/function/uvc_configfs.c    |   2 +
 drivers/usb/gadget/function/uvc_queue.c       |   4 +-
 drivers/usb/gadget/function/uvc_v4l2.c        |   8 +
 7 files changed, 186 insertions(+), 82 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 889ed45be4ca..52ca04a619ff 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -7,6 +7,7 @@ Description:	UVC function directory
 		streaming_maxburst	0..15 (ss only)
 		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
 		streaming_interval	1..16
+		streaming_bulk_mult	0..0x3fffffU
 		===================	=============================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/control
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index c6d034abce3a..2cbe3e2e05c3 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -787,6 +787,10 @@ The uvc function provides these attributes in its function directory:
 	streaming_maxpacket maximum packet size this endpoint is capable of
 			    sending or receiving when this configuration is
 			    selected
+	streaming_bulk_mult Multiples to configure max_payload_size. If it's
+			    0, the transport mode is isochronous; otherwise
+			    the transport mode is bulk and max_payload_size
+			    is equal to streaming_bulk_mult * 1024.
 	=================== ================================================
 
 There are also "control" and "streaming" subdirectories, each of which contain
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 71bb5e477dba..2c54b482a902 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -30,6 +30,8 @@
 #include "uvc_v4l2.h"
 #include "uvc_video.h"
 
+#define USBDHDR(p) ((struct usb_descriptor_header *)(p))
+
 unsigned int uvc_gadget_trace_param;
 module_param_named(trace, uvc_gadget_trace_param, uint, 0644);
 MODULE_PARM_DESC(trace, "Trace level bitmask");
@@ -178,19 +180,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
 	 */
 };
 
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -251,9 +253,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 
 void uvc_function_setup_continue(struct uvc_device *uvc)
 {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
 	struct usb_composite_dev *cdev = uvc->func.config->cdev;
 
-	usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (!opts->streaming_bulk_mult)
+		usb_composite_setup_continue(cdev);
 }
 
 static int
@@ -278,6 +283,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
 	int ret;
 
 	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -310,49 +316,72 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	if (interface != uvc->streaming_intf)
 		return -EINVAL;
 
-	/* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/
+	if (opts->streaming_bulk_mult) {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
 
-	switch (alt) {
-	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+			if (!uvc->video.ep)
+				return -EINVAL;
 
-		if (uvc->video.ep)
+			uvcg_info(f, "reset UVC\n");
 			usb_ep_disable(uvc->video.ep);
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMOFF;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
-
-		uvc->state = UVC_STATE_CONNECTED;
-		return 0;
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
 
-	case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 			return 0;
 
-		if (!uvc->video.ep)
+		default:
 			return -EINVAL;
+		}
+	} else {
+		switch (alt) {
+		case 0:
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
+
+			if (uvc->video.ep)
+				usb_ep_disable(uvc->video.ep);
+
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMOFF;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		uvcg_info(f, "reset UVC\n");
-		usb_ep_disable(uvc->video.ep);
+			uvc->state = UVC_STATE_CONNECTED;
+			return 0;
 
-		ret = config_ep_by_speed(f->config->cdev->gadget,
-				&(uvc->func), uvc->video.ep);
-		if (ret)
-			return ret;
-		usb_ep_enable(uvc->video.ep);
+		case 1:
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
 
-		memset(&v4l2_event, 0, sizeof(v4l2_event));
-		v4l2_event.type = UVC_EVENT_STREAMON;
-		v4l2_event_queue(&uvc->vdev, &v4l2_event);
-		return USB_GADGET_DELAYED_STATUS;
+			if (!uvc->video.ep)
+				return -EINVAL;
 
-	default:
-		return -EINVAL;
+			uvcg_info(f, "reset UVC\n");
+			usb_ep_disable(uvc->video.ep);
+
+			ret = config_ep_by_speed(f->config->cdev->gadget,
+					&(uvc->func), uvc->video.ep);
+			if (ret)
+				return ret;
+			usb_ep_enable(uvc->video.ep);
+
+			memset(&v4l2_event, 0, sizeof(v4l2_event));
+			v4l2_event.type = UVC_EVENT_STREAMON;
+			v4l2_event_queue(&uvc->vdev, &v4l2_event);
+			return USB_GADGET_DELAYED_STATUS;
+
+		default:
+			return -EINVAL;
+		}
 	}
 }
 
@@ -593,62 +622,96 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	unsigned int max_packet_size;
 	struct usb_ep *ep;
 	struct f_uvc_opts *opts;
+	int i = 0;
 	int ret = -EINVAL;
 
 	uvcg_info(f, "%s()\n", __func__);
 
 	opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters.
-	 */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
 
-	/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_bulk_mult) {
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_XFER_BULK;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_bulk_mult * 1024;
 	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = USB_ENDPOINT_SYNC_ASYNC
+						| USB_ENDPOINT_XFER_ISOC;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters.
+		 */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
 
-	uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/* Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
 
-	uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
 
-	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
 
-	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
+
+		uvc->video.max_payload_size = 0;
+	}
 
 	/* Allocate endpoints. */
 	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -662,7 +725,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
 					  &uvc_ss_streaming_comp);
 	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
 	else
 		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 
@@ -703,6 +766,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
+	/* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_bulk_mult) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
 	/* Copy descriptors */
 	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
 	if (IS_ERR(f->fs_descriptors)) {
@@ -866,6 +951,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
+	opts->streaming_bulk_mult = 0;
 
 	ret = uvcg_attach_configfs(opts);
 	if (ret < 0) {
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 9a01a7d4f17f..5607a239d55e 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
 	unsigned int					streaming_interval;
 	unsigned int					streaming_maxpacket;
 	unsigned int					streaming_maxburst;
+	unsigned int					streaming_bulk_mult;
 
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 77d64031aa9c..9b08e7b25168 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2422,6 +2422,7 @@ UVC_ATTR(f_uvc_opts_, cname, cname)
 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(streaming_bulk_mult, streaming_bulk_mult, 0x3fffffU);
 
 #undef UVCG_OPTS_ATTR
 
@@ -2429,6 +2430,7 @@ static struct configfs_attribute *uvc_attrs[] = {
 	&f_uvc_opts_attr_streaming_interval,
 	&f_uvc_opts_attr_streaming_maxpacket,
 	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_bulk_mult,
 	NULL,
 };
 
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 2cda982f3765..98d0e933b5e1 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -135,7 +135,9 @@ 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) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
 		queue->queue.mem_ops = &vb2_dma_sg_memops;
 		queue->use_sg = 1;
 	} else {
diff --git a/drivers/usb/gadget/function/uvc_v4l2.c b/drivers/usb/gadget/function/uvc_v4l2.c
index a2c78690c5c2..767f1a2ace04 100644
--- a/drivers/usb/gadget/function/uvc_v4l2.c
+++ b/drivers/usb/gadget/function/uvc_v4l2.c
@@ -119,6 +119,14 @@ uvc_v4l2_set_format(struct file *file, void *fh, struct v4l2_format *fmt)
 	bpl = format->bpp * fmt->fmt.pix.width / 8;
 	imagesize = bpl ? bpl * fmt->fmt.pix.height : fmt->fmt.pix.sizeimage;
 
+	/*
+	 * Bulk mode only allocates memory once, so user should give the
+	 * maximum image size in all formats and kernel should not decrease
+	 * the imagesize.
+	 */
+	if (video->max_payload_size && imagesize < fmt->fmt.pix.sizeimage)
+		imagesize = fmt->fmt.pix.sizeimage;
+
 	video->fcc = format->fcc;
 	video->bpp = format->bpp;
 	video->width = fmt->fmt.pix.width;
-- 
2.17.1


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

* [PATCH v4] usb: gadget: uvc: add bulk transfer support
  2022-05-13  0:42   ` [PATCH v3] " 3090101217
@ 2022-10-25  9:05     ` 3090101217
  2022-10-25 10:32       ` Greg KH
  2022-11-03  6:13     ` [PATCH v5] " Jing Leng
  1 sibling, 1 reply; 10+ messages in thread
From: 3090101217 @ 2022-10-25  9:05 UTC (permalink / raw)
  Cc: balbi, bilbao, corbet, gregkh, laurent.pinchart, linux-doc,
	linux-kernel, linux-usb, mchehab+huawei, rdunlap, Jing Leng

From: Jing Leng <jleng@ambarella.com>

The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

Here shows an example of the configfs differences:
  if [ $BULK -eq 1 ]; then
      echo "bulk" > functions/$FUNC/streaming_transfer
      echo $(( 1024 * N )) > functions/$FUNC/streaming_maxpacket
  else
      echo "isoc" > functions/$FUNC/streaming_transfer
      echo 1024 > functions/$FUNC/streaming_maxpacket
  fi

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v3->v4:
- echo "bulk" > functions/$FUNC/streaming_transfer to set bulk mode
ChangeLog v2->v3:
- Mistakenly deleted the definition of i and USBDHDR when porting from my workdir.
- Reported-by: kernel test robot <lkp@intel.com>
ChangeLog v1->v2:
- Handle imagesize in uvc_v4l2_set_format. If it's not handled,
- switching from low resolution to high resolution will fail to play.
---
 .../ABI/testing/configfs-usb-gadget-uvc       |   8 +-
 Documentation/usb/gadget-testing.rst          |   1 +
 drivers/usb/gadget/function/f_uvc.c           | 228 +++++++++++++-----
 drivers/usb/gadget/function/u_uvc.h           |   1 +
 drivers/usb/gadget/function/uvc.h             |   2 +
 drivers/usb/gadget/function/uvc_configfs.c    |  64 ++++-
 drivers/usb/gadget/function/uvc_queue.c       |  14 +-
 drivers/usb/gadget/function/uvc_video.c       |  16 +-
 8 files changed, 261 insertions(+), 73 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 611b23e6488d..ffa2fd8c7fcd 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -3,12 +3,14 @@ Date:		Dec 2014
 KernelVersion:	4.0
 Description:	UVC function directory
 
-		===================	=============================
+		===================	===================================
 		streaming_maxburst	0..15 (ss only)
-		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
+		streaming_maxpacket	isoc: 1..1023 (fs), 1..3072 (hs/ss)
+					bulk: 1024..0x40000000
 		streaming_interval	1..16
+		streaming_transfer	isoc/bulk
 		function_name		string [32]
-		===================	=============================
+		===================	===================================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/control
 Date:		Dec 2014
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 2278c9ffb74a..880af56abe8b 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -793,6 +793,7 @@ The uvc function provides these attributes in its function directory:
 	streaming_maxpacket maximum packet size this endpoint is capable of
 			    sending or receiving when this configuration is
 			    selected
+	streaming_transfer  specify data transmission mode (isoc/bulk)
 	function_name       name of the interface
 	=================== ================================================
 
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..8a4df750e00d 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -29,6 +29,8 @@
 #include "uvc_v4l2.h"
 #include "uvc_video.h"
 
+#define USBDHDR(p) ((struct usb_descriptor_header *)(p))
+
 unsigned int uvc_gadget_trace_param;
 module_param_named(trace, uvc_gadget_trace_param, uint, 0644);
 MODULE_PARM_DESC(trace, "Trace level bitmask");
@@ -181,19 +183,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
 	 */
 };
 
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -204,6 +206,10 @@ static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
  * Control requests
  */
 
+
+static int uvc_function_set_alt(struct usb_function *f,
+		unsigned int interface, unsigned int alt);
+
 static void
 uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
 {
@@ -219,6 +225,13 @@ uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
 		uvc_event->data.length = req->actual;
 		memcpy(&uvc_event->data.data, req->buf, req->actual);
 		v4l2_event_queue(&uvc->vdev, &v4l2_event);
+
+		/*
+		 * Bulk mode only has one alt, so we should set STREAM ON after
+		 * responding the SET UVC_VS_COMMIT_CONTROL request.
+		 */
+		if (uvc->state == UVC_STATE_BULK_SETTING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 1);
 	}
 }
 
@@ -228,6 +241,9 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	struct uvc_device *uvc = to_uvc(f);
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
+	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
+	unsigned int cs = le16_to_cpu(ctrl->wValue) >> 8 & 0xff;
 
 	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
 		uvcg_info(f, "invalid request type\n");
@@ -245,6 +261,21 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	uvc->event_setup_out = !(ctrl->bRequestType & USB_DIR_IN);
 	uvc->event_length = le16_to_cpu(ctrl->wLength);
 
+	/*
+	 * Bulk mode only has one alt, when the SET UVC_VS_COMMIT_CONTROL request
+	 * is received, if the streaming is in transit, we need to set STREAM OFF,
+	 * if the uvc state is UVC_STATE_BULK_WAITING, we only need to change it.
+	 */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK &&
+		uvc->event_setup_out &&
+		uvc->streaming_intf == interface &&
+		cs == UVC_VS_COMMIT_CONTROL) {
+		if (uvc->state == UVC_STATE_STREAMING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 0);
+		else if (uvc->state == UVC_STATE_BULK_WAITING)
+			uvc->state = UVC_STATE_BULK_SETTING;
+	}
+
 	memset(&v4l2_event, 0, sizeof(v4l2_event));
 	v4l2_event.type = UVC_EVENT_SETUP;
 	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
@@ -255,9 +286,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 
 void uvc_function_setup_continue(struct uvc_device *uvc)
 {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
 	struct usb_composite_dev *cdev = uvc->func.config->cdev;
 
-	usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+		usb_composite_setup_continue(cdev);
 }
 
 static int
@@ -282,6 +316,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
 	int ret;
 
 	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -314,15 +349,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	if (interface != uvc->streaming_intf)
 		return -EINVAL;
 
-	/* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/
-
 	switch (alt) {
 	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state == UVC_STATE_CONNECTED)
+				uvc->state = UVC_STATE_BULK_WAITING;
+			else if (uvc->state == UVC_STATE_STREAMING)
+				uvc->state = UVC_STATE_BULK_SETTING;
+			else
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
+		}
 
 		if (uvc->video.ep)
 			usb_ep_disable(uvc->video.ep);
@@ -331,12 +370,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		v4l2_event.type = UVC_EVENT_STREAMOFF;
 		v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		uvc->state = UVC_STATE_CONNECTED;
+		if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+			uvc->state = UVC_STATE_CONNECTED;
+
 		return 0;
 
 	case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state != UVC_STATE_BULK_SETTING)
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
+		}
 
 		if (!uvc->video.ep)
 			return -EINVAL;
@@ -598,62 +644,101 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	unsigned int max_packet_size;
 	struct usb_ep *ep;
 	struct f_uvc_opts *opts;
+	int i = 0;
 	int ret = -EINVAL;
 
 	uvcg_info(f, "%s()\n", __func__);
 
 	opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters. */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
 
-	/*
-	 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		if (opts->streaming_maxpacket % 1024 != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_maxpacket;
 	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters. */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
 
-	uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/*
+		 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
 
-	uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
 
-	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
 
-	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc->video.max_payload_size = 0;
+	}
 
 	/* Allocate endpoints. */
 	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -667,7 +752,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
 					  &uvc_ss_streaming_comp);
 	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
 	else
 		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 
@@ -709,6 +794,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
+	/* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
 	/* Copy descriptors */
 	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
 	if (IS_ERR(f->fs_descriptors)) {
@@ -872,6 +979,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
+	opts->streaming_transfer = USB_ENDPOINT_SYNC_ASYNC | USB_ENDPOINT_XFER_ISOC;
 	snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
 
 	ret = uvcg_attach_configfs(opts);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 24b8681b0d6f..88cd235e0ea8 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
 	unsigned int					streaming_interval;
 	unsigned int					streaming_maxpacket;
 	unsigned int					streaming_maxburst;
+	unsigned int					streaming_transfer;
 
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e14..b00cba322643 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -123,6 +123,8 @@ enum uvc_state {
 	UVC_STATE_DISCONNECTED,
 	UVC_STATE_CONNECTED,
 	UVC_STATE_STREAMING,
+	UVC_STATE_BULK_WAITING,
+	UVC_STATE_BULK_SETTING,
 };
 
 struct uvc_device {
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 4303a3283ba0..ef7c4f6630d4 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2348,11 +2348,72 @@ end:									\
 UVC_ATTR(f_uvc_opts_, cname, cname)
 
 UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
-UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
+UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 0x40000000U);
 UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
 
 #undef UVCG_OPTS_ATTR
 
+#define UVCG_OPTS_ATTR_TRANSFER(cname, aname)				\
+static ssize_t f_uvc_opts_##cname##_show(				\
+	struct config_item *item, char *page)				\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int result;							\
+	char *str;							\
+									\
+	mutex_lock(&opts->lock);					\
+	switch (opts->cname) {						\
+	case USB_ENDPOINT_XFER_BULK:					\
+		str = "bulk";						\
+		break;							\
+	case USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC:		\
+		str = "isoc";						\
+		break;							\
+	default:							\
+		str = "unknown";					\
+		break;							\
+	}								\
+	result = sprintf(page, "%s\n", str);				\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t								\
+f_uvc_opts_##cname##_store(struct config_item *item,			\
+			   const char *page, size_t len)		\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int ret = 0;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	if (!strncmp(page, "bulk", 4))					\
+		opts->cname = USB_ENDPOINT_XFER_BULK;			\
+	else if (!strncmp(page, "isoc", 4))				\
+		opts->cname = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC; \
+	else {								\
+		ret = -EINVAL;						\
+		goto end;						\
+	}								\
+									\
+	ret = len;							\
+									\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+UVC_ATTR(f_uvc_opts_, cname, cname)
+
+UVCG_OPTS_ATTR_TRANSFER(streaming_transfer, streaming_transfer);
+
+#undef UVCG_OPTS_ATTR_TRANSFER
+
 #define UVCG_OPTS_STRING_ATTR(cname, aname)				\
 static ssize_t f_uvc_opts_string_##cname##_show(struct config_item *item,\
 					 char *page)			\
@@ -2399,6 +2460,7 @@ static struct configfs_attribute *uvc_attrs[] = {
 	&f_uvc_opts_attr_streaming_interval,
 	&f_uvc_opts_attr_streaming_maxpacket,
 	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_transfer,
 	&f_uvc_opts_string_attr_function_name,
 	NULL,
 };
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index ec500ee499ee..33213e3f6635 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -54,9 +54,13 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 
 	sizes[0] = video->imagesize;
 
-	req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
 
 	/* We divide by two, to increase the chance to run
 	 * into fewer requests for smaller framesizes.
@@ -143,7 +147,9 @@ 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) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
 		queue->queue.mem_ops = &vb2_dma_sg_memops;
 		queue->use_sg = 1;
 	} else {
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index bb037fcc90e6..76806ff32d61 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -261,8 +261,10 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
 		break;
 
 	default:
-		uvcg_warn(&video->uvc->func,
-			  "VS request completed with status %d.\n",
+		if (uvc->state == UVC_STATE_BULK_WAITING ||
+			uvc->state == UVC_STATE_BULK_SETTING)
+			break;
+		uvcg_warn(&uvc->func, "VS request completed with status %d.\n",
 			  req->status);
 		uvcg_queue_cancel(queue, 0);
 	}
@@ -318,9 +320,13 @@ uvc_video_alloc_requests(struct uvc_video *video)
 
 	BUG_ON(video->req_size);
 
-	req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
 
 	video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
 	if (video->ureq == NULL)
-- 
2.17.1


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

* Re: [PATCH v4] usb: gadget: uvc: add bulk transfer support
  2022-10-25  9:05     ` [PATCH v4] " 3090101217
@ 2022-10-25 10:32       ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2022-10-25 10:32 UTC (permalink / raw)
  To: 3090101217
  Cc: balbi, bilbao, corbet, laurent.pinchart, linux-doc, linux-kernel,
	linux-usb, mchehab+huawei, rdunlap, Jing Leng

On Tue, Oct 25, 2022 at 05:05:01PM +0800, 3090101217@zju.edu.cn wrote:
> From: Jing Leng <jleng@ambarella.com>

This email address does not match your From: address on the email, _AND_
that email address does not validate as actually being from that
address.

So this really looks like a "fake" email sent to us.  How can we know
differently?

Please work with your company email admins to fix up their systems so
you can properly send from your domain.  Then we can take your patches.

thanks,

greg k-h

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

* [PATCH v5] usb: gadget: uvc: add bulk transfer support
  2022-05-13  0:42   ` [PATCH v3] " 3090101217
  2022-10-25  9:05     ` [PATCH v4] " 3090101217
@ 2022-11-03  6:13     ` Jing Leng
  2022-11-08 15:47       ` Greg KH
  2022-11-10 14:22       ` Dan Scally
  1 sibling, 2 replies; 10+ messages in thread
From: Jing Leng @ 2022-11-03  6:13 UTC (permalink / raw)
  To: balbi, bilbao, corbet, gregkh, laurent.pinchart, mchehab+huawei
  Cc: linux-doc, linux-kernel, linux-usb, rdunlap, Jing Leng

The video data endpoint of uvc can be implemented as either an
isochronous or a bulk endpoint.

The transmission speed of bulk mode is faster than isochronous mode.
I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
mode.

A VideoStreaming interface with isochronous endpoints must have alternate
settings that can be used to change certain characteristics of the
interface and underlying endpoint(s). A typical use of alternate settings
is to provide a way to change the bandwidth requirements an active
isochronous pipe imposes on the USB.

A VideoStreaming interface containing a bulk endpoint for streaming shall
support only alternate setting zero. Additional alternate settings
containing bulk endpoints are not permitted in a device that is compliant
with the Video Class specification.

Here shows an example of the configfs differences:
  if [ $BULK -eq 1 ]; then
      echo "bulk" > functions/$FUNC/streaming_transfer
      echo $(( 1024 * N )) > functions/$FUNC/streaming_maxpacket
  else
      echo "isoc" > functions/$FUNC/streaming_transfer
      echo 1024 > functions/$FUNC/streaming_maxpacket
  fi

Signed-off-by: Jing Leng <jleng@ambarella.com>
---
ChangeLog v4->v5:
- Rebase the patch.
- Make email addresses ('From' and 'Signed-off-by') consistent.
ChangeLog v3->v4:
- echo "bulk" > functions/$FUNC/streaming_transfer to set bulk mode
ChangeLog v2->v3:
- Mistakenly deleted the definition of i and USBDHDR when porting from my workdir.
- Reported-by: kernel test robot <lkp@intel.com>
ChangeLog v1->v2:
- Handle imagesize in uvc_v4l2_set_format. If it's not handled,
- switching from low resolution to high resolution will fail to play.
---
 .../ABI/testing/configfs-usb-gadget-uvc       |   8 +-
 Documentation/usb/gadget-testing.rst          |   1 +
 drivers/usb/gadget/function/f_uvc.c           | 228 +++++++++++++-----
 drivers/usb/gadget/function/u_uvc.h           |   1 +
 drivers/usb/gadget/function/uvc.h             |   2 +
 drivers/usb/gadget/function/uvc_configfs.c    |  64 ++++-
 drivers/usb/gadget/function/uvc_queue.c       |  14 +-
 drivers/usb/gadget/function/uvc_video.c       |  16 +-
 8 files changed, 261 insertions(+), 73 deletions(-)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 611b23e6488d..ffa2fd8c7fcd 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -3,12 +3,14 @@ Date:		Dec 2014
 KernelVersion:	4.0
 Description:	UVC function directory
 
-		===================	=============================
+		===================	===================================
 		streaming_maxburst	0..15 (ss only)
-		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
+		streaming_maxpacket	isoc: 1..1023 (fs), 1..3072 (hs/ss)
+					bulk: 1024..0x40000000
 		streaming_interval	1..16
+		streaming_transfer	isoc/bulk
 		function_name		string [32]
-		===================	=============================
+		===================	===================================
 
 What:		/config/usb-gadget/gadget/functions/uvc.name/control
 Date:		Dec 2014
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 2278c9ffb74a..880af56abe8b 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -793,6 +793,7 @@ The uvc function provides these attributes in its function directory:
 	streaming_maxpacket maximum packet size this endpoint is capable of
 			    sending or receiving when this configuration is
 			    selected
+	streaming_transfer  specify data transmission mode (isoc/bulk)
 	function_name       name of the interface
 	=================== ================================================
 
diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 6e196e06181e..8a4df750e00d 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -29,6 +29,8 @@
 #include "uvc_v4l2.h"
 #include "uvc_video.h"
 
+#define USBDHDR(p) ((struct usb_descriptor_header *)(p))
+
 unsigned int uvc_gadget_trace_param;
 module_param_named(trace, uvc_gadget_trace_param, uint, 0644);
 MODULE_PARM_DESC(trace, "Trace level bitmask");
@@ -181,19 +183,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
 	 */
 };
 
-static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
+static const struct usb_descriptor_header *uvc_fs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
+static const struct usb_descriptor_header *uvc_hs_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
 	NULL,
 };
 
-static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
+static const struct usb_descriptor_header *uvc_ss_streaming[] = {
 	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
 	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
@@ -204,6 +206,10 @@ static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
  * Control requests
  */
 
+
+static int uvc_function_set_alt(struct usb_function *f,
+		unsigned int interface, unsigned int alt);
+
 static void
 uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
 {
@@ -219,6 +225,13 @@ uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
 		uvc_event->data.length = req->actual;
 		memcpy(&uvc_event->data.data, req->buf, req->actual);
 		v4l2_event_queue(&uvc->vdev, &v4l2_event);
+
+		/*
+		 * Bulk mode only has one alt, so we should set STREAM ON after
+		 * responding the SET UVC_VS_COMMIT_CONTROL request.
+		 */
+		if (uvc->state == UVC_STATE_BULK_SETTING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 1);
 	}
 }
 
@@ -228,6 +241,9 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	struct uvc_device *uvc = to_uvc(f);
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
+	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
+	unsigned int cs = le16_to_cpu(ctrl->wValue) >> 8 & 0xff;
 
 	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
 		uvcg_info(f, "invalid request type\n");
@@ -245,6 +261,21 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 	uvc->event_setup_out = !(ctrl->bRequestType & USB_DIR_IN);
 	uvc->event_length = le16_to_cpu(ctrl->wLength);
 
+	/*
+	 * Bulk mode only has one alt, when the SET UVC_VS_COMMIT_CONTROL request
+	 * is received, if the streaming is in transit, we need to set STREAM OFF,
+	 * if the uvc state is UVC_STATE_BULK_WAITING, we only need to change it.
+	 */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK &&
+		uvc->event_setup_out &&
+		uvc->streaming_intf == interface &&
+		cs == UVC_VS_COMMIT_CONTROL) {
+		if (uvc->state == UVC_STATE_STREAMING)
+			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 0);
+		else if (uvc->state == UVC_STATE_BULK_WAITING)
+			uvc->state = UVC_STATE_BULK_SETTING;
+	}
+
 	memset(&v4l2_event, 0, sizeof(v4l2_event));
 	v4l2_event.type = UVC_EVENT_SETUP;
 	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
@@ -255,9 +286,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
 
 void uvc_function_setup_continue(struct uvc_device *uvc)
 {
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
 	struct usb_composite_dev *cdev = uvc->func.config->cdev;
 
-	usb_composite_setup_continue(cdev);
+	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
+	if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+		usb_composite_setup_continue(cdev);
 }
 
 static int
@@ -282,6 +316,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	struct usb_composite_dev *cdev = f->config->cdev;
 	struct v4l2_event v4l2_event;
 	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
+	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
 	int ret;
 
 	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
@@ -314,15 +349,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 	if (interface != uvc->streaming_intf)
 		return -EINVAL;
 
-	/* TODO
-	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
-		return alt ? -EINVAL : 0;
-	*/
-
 	switch (alt) {
 	case 0:
-		if (uvc->state != UVC_STATE_STREAMING)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state == UVC_STATE_CONNECTED)
+				uvc->state = UVC_STATE_BULK_WAITING;
+			else if (uvc->state == UVC_STATE_STREAMING)
+				uvc->state = UVC_STATE_BULK_SETTING;
+			else
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_STREAMING)
+				return 0;
+		}
 
 		if (uvc->video.ep)
 			usb_ep_disable(uvc->video.ep);
@@ -331,12 +370,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
 		v4l2_event.type = UVC_EVENT_STREAMOFF;
 		v4l2_event_queue(&uvc->vdev, &v4l2_event);
 
-		uvc->state = UVC_STATE_CONNECTED;
+		if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
+			uvc->state = UVC_STATE_CONNECTED;
+
 		return 0;
 
 	case 1:
-		if (uvc->state != UVC_STATE_CONNECTED)
-			return 0;
+		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+			if (uvc->state != UVC_STATE_BULK_SETTING)
+				return 0;
+		} else {
+			if (uvc->state != UVC_STATE_CONNECTED)
+				return 0;
+		}
 
 		if (!uvc->video.ep)
 			return -EINVAL;
@@ -598,62 +644,101 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	unsigned int max_packet_size;
 	struct usb_ep *ep;
 	struct f_uvc_opts *opts;
+	int i = 0;
 	int ret = -EINVAL;
 
 	uvcg_info(f, "%s()\n", __func__);
 
 	opts = fi_to_f_uvc_opts(f->fi);
-	/* Sanity check the streaming endpoint module parameters. */
-	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
-	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
-	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
-
-	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
-	if (opts->streaming_maxburst &&
-	    (opts->streaming_maxpacket % 1024) != 0) {
-		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
-		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
-			  opts->streaming_maxpacket);
-	}
 
-	/*
-	 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
-	 * module parameters.
-	 *
-	 * NOTE: We assume that the user knows what they are doing and won't
-	 * give parameters that their UDC doesn't support.
-	 */
-	if (opts->streaming_maxpacket <= 1024) {
-		max_packet_mult = 1;
-		max_packet_size = opts->streaming_maxpacket;
-	} else if (opts->streaming_maxpacket <= 2048) {
-		max_packet_mult = 2;
-		max_packet_size = opts->streaming_maxpacket / 2;
+	/* Handle different transfer mode for stream endpoints */
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		if (opts->streaming_maxpacket % 1024 != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
+
+		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
+		uvc_fs_streaming_ep.bInterval = 0;
+
+		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
+		uvc_hs_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
+		uvc_ss_streaming_ep.bInterval = 0;
+
+		uvc_ss_streaming_comp.bmAttributes = 0;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval = 0;
+
+		uvc->video.max_payload_size = opts->streaming_maxpacket;
 	} else {
-		max_packet_mult = 3;
-		max_packet_size = opts->streaming_maxpacket / 3;
-	}
+		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
+		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
+
+		/* Sanity check the streaming endpoint module parameters. */
+		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
+		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
+		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
+
+		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
+		if (opts->streaming_maxburst &&
+			(opts->streaming_maxpacket % 1024) != 0) {
+			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
+			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
+				opts->streaming_maxpacket);
+		}
 
-	uvc_fs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
-	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
+		/*
+		 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
+		 * module parameters.
+		 *
+		 * NOTE: We assume that the user knows what they are doing and won't
+		 * give parameters that their UDC doesn't support.
+		 */
+		if (opts->streaming_maxpacket <= 1024) {
+			max_packet_mult = 0;
+			max_packet_size = opts->streaming_maxpacket;
+		} else if (opts->streaming_maxpacket <= 2048) {
+			max_packet_mult = 1;
+			max_packet_size = opts->streaming_maxpacket / 2;
+		} else {
+			max_packet_mult = 2;
+			max_packet_size = opts->streaming_maxpacket / 3;
+		}
 
-	uvc_hs_streaming_ep.wMaxPacketSize =
-		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
+		uvc_fs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
+		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
 
-	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
-	if (max_packet_mult > 1)
-		uvc_hs_streaming_ep.bInterval = 1;
-	else
-		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+		uvc_hs_streaming_ep.wMaxPacketSize =
+			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
+
+		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
+		if (max_packet_mult > 0)
+			uvc_hs_streaming_ep.bInterval = 1;
+		else
+			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
+		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
+
+		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
+		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
+		uvc_ss_streaming_comp.wBytesPerInterval =
+			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
+				(opts->streaming_maxburst + 1));
 
-	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
-	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
-	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
-	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
-	uvc_ss_streaming_comp.wBytesPerInterval =
-		cpu_to_le16(max_packet_size * max_packet_mult *
-			    (opts->streaming_maxburst + 1));
+		uvc->video.max_payload_size = 0;
+	}
 
 	/* Allocate endpoints. */
 	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
@@ -667,7 +752,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
 					  &uvc_ss_streaming_comp);
 	else if (gadget_is_dualspeed(cdev->gadget))
-		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
+		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);
 	else
 		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
 
@@ -709,6 +794,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
 	uvc->streaming_intf = ret;
 	opts->streaming_interface = ret;
 
+	/* Handle different transfer mode for descriptors */
+	i = 0;
+	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
+		uvc_streaming_intf_alt0.bNumEndpoints = 1;
+	} else {
+		uvc_streaming_intf_alt0.bNumEndpoints = 0;
+
+		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
+		++i;
+	}
+	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
+	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
+	++i;
+	uvc_fs_streaming[i] = NULL;
+	uvc_hs_streaming[i] = NULL;
+	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
+	++i;
+	uvc_ss_streaming[i] = NULL;
+
 	/* Copy descriptors */
 	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
 	if (IS_ERR(f->fs_descriptors)) {
@@ -872,6 +979,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
 
 	opts->streaming_interval = 1;
 	opts->streaming_maxpacket = 1024;
+	opts->streaming_transfer = USB_ENDPOINT_SYNC_ASYNC | USB_ENDPOINT_XFER_ISOC;
 	snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
 
 	ret = uvcg_attach_configfs(opts);
diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
index 24b8681b0d6f..88cd235e0ea8 100644
--- a/drivers/usb/gadget/function/u_uvc.h
+++ b/drivers/usb/gadget/function/u_uvc.h
@@ -24,6 +24,7 @@ struct f_uvc_opts {
 	unsigned int					streaming_interval;
 	unsigned int					streaming_maxpacket;
 	unsigned int					streaming_maxburst;
+	unsigned int					streaming_transfer;
 
 	unsigned int					control_interface;
 	unsigned int					streaming_interface;
diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
index 40226b1f7e14..b00cba322643 100644
--- a/drivers/usb/gadget/function/uvc.h
+++ b/drivers/usb/gadget/function/uvc.h
@@ -123,6 +123,8 @@ enum uvc_state {
 	UVC_STATE_DISCONNECTED,
 	UVC_STATE_CONNECTED,
 	UVC_STATE_STREAMING,
+	UVC_STATE_BULK_WAITING,
+	UVC_STATE_BULK_SETTING,
 };
 
 struct uvc_device {
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 4303a3283ba0..ef7c4f6630d4 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -2348,11 +2348,72 @@ end:									\
 UVC_ATTR(f_uvc_opts_, cname, cname)
 
 UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
-UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
+UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 0x40000000U);
 UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
 
 #undef UVCG_OPTS_ATTR
 
+#define UVCG_OPTS_ATTR_TRANSFER(cname, aname)				\
+static ssize_t f_uvc_opts_##cname##_show(				\
+	struct config_item *item, char *page)				\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int result;							\
+	char *str;							\
+									\
+	mutex_lock(&opts->lock);					\
+	switch (opts->cname) {						\
+	case USB_ENDPOINT_XFER_BULK:					\
+		str = "bulk";						\
+		break;							\
+	case USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC:		\
+		str = "isoc";						\
+		break;							\
+	default:							\
+		str = "unknown";					\
+		break;							\
+	}								\
+	result = sprintf(page, "%s\n", str);				\
+	mutex_unlock(&opts->lock);					\
+									\
+	return result;							\
+}									\
+									\
+static ssize_t								\
+f_uvc_opts_##cname##_store(struct config_item *item,			\
+			   const char *page, size_t len)		\
+{									\
+	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
+	int ret = 0;							\
+									\
+	mutex_lock(&opts->lock);					\
+	if (opts->refcnt) {						\
+		ret = -EBUSY;						\
+		goto end;						\
+	}								\
+									\
+	if (!strncmp(page, "bulk", 4))					\
+		opts->cname = USB_ENDPOINT_XFER_BULK;			\
+	else if (!strncmp(page, "isoc", 4))				\
+		opts->cname = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC; \
+	else {								\
+		ret = -EINVAL;						\
+		goto end;						\
+	}								\
+									\
+	ret = len;							\
+									\
+end:									\
+	mutex_unlock(&opts->lock);					\
+	return ret;							\
+}									\
+									\
+UVC_ATTR(f_uvc_opts_, cname, cname)
+
+UVCG_OPTS_ATTR_TRANSFER(streaming_transfer, streaming_transfer);
+
+#undef UVCG_OPTS_ATTR_TRANSFER
+
 #define UVCG_OPTS_STRING_ATTR(cname, aname)				\
 static ssize_t f_uvc_opts_string_##cname##_show(struct config_item *item,\
 					 char *page)			\
@@ -2399,6 +2460,7 @@ static struct configfs_attribute *uvc_attrs[] = {
 	&f_uvc_opts_attr_streaming_interval,
 	&f_uvc_opts_attr_streaming_maxpacket,
 	&f_uvc_opts_attr_streaming_maxburst,
+	&f_uvc_opts_attr_streaming_transfer,
 	&f_uvc_opts_string_attr_function_name,
 	NULL,
 };
diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
index 0aa3d7e1f3cc..2695b3ed29b5 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -54,9 +54,13 @@ static int uvc_queue_setup(struct vb2_queue *vq,
 
 	sizes[0] = video->imagesize;
 
-	req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
 
 	/* We divide by two, to increase the chance to run
 	 * into fewer requests for smaller framesizes.
@@ -143,7 +147,9 @@ 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) {
+
+	/* UDC supports scatter gather and transfer mode isn't bulk. */
+	if (cdev->gadget->sg_supported && !video->max_payload_size) {
 		queue->queue.mem_ops = &vb2_dma_sg_memops;
 		queue->use_sg = 1;
 	} else {
diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
index dd1c6b2ca7c6..70737340acfb 100644
--- a/drivers/usb/gadget/function/uvc_video.c
+++ b/drivers/usb/gadget/function/uvc_video.c
@@ -271,8 +271,10 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
 		break;
 
 	default:
-		uvcg_warn(&video->uvc->func,
-			  "VS request completed with status %d.\n",
+		if (uvc->state == UVC_STATE_BULK_WAITING ||
+			uvc->state == UVC_STATE_BULK_SETTING)
+			break;
+		uvcg_warn(&uvc->func, "VS request completed with status %d.\n",
 			  req->status);
 		uvcg_queue_cancel(queue, 0);
 	}
@@ -328,9 +330,13 @@ uvc_video_alloc_requests(struct uvc_video *video)
 
 	BUG_ON(video->req_size);
 
-	req_size = video->ep->maxpacket
-		 * max_t(unsigned int, video->ep->maxburst, 1)
-		 * (video->ep->mult);
+	/* Bulk mode uses max_payload_size as req_size */
+	if (video->max_payload_size)
+		req_size = video->max_payload_size;
+	else
+		req_size = video->ep->maxpacket
+			 * max_t(unsigned int, video->ep->maxburst, 1)
+			 * (video->ep->mult);
 
 	video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
 	if (video->ureq == NULL)
-- 
2.17.1


**********************************************************************
This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.

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

* Re: [PATCH v5] usb: gadget: uvc: add bulk transfer support
  2022-11-03  6:13     ` [PATCH v5] " Jing Leng
@ 2022-11-08 15:47       ` Greg KH
  2022-11-10 14:22       ` Dan Scally
  1 sibling, 0 replies; 10+ messages in thread
From: Greg KH @ 2022-11-08 15:47 UTC (permalink / raw)
  To: Jing Leng
  Cc: balbi, bilbao, corbet, laurent.pinchart, mchehab+huawei,
	linux-doc, linux-kernel, linux-usb, rdunlap

On Thu, Nov 03, 2022 at 02:13:03PM +0800, Jing Leng wrote:
> **********************************************************************
> This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.

Now deleted.

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

* Re: [PATCH v5] usb: gadget: uvc: add bulk transfer support
  2022-11-03  6:13     ` [PATCH v5] " Jing Leng
  2022-11-08 15:47       ` Greg KH
@ 2022-11-10 14:22       ` Dan Scally
  2022-11-11  3:51         ` [EXT] " Jing Leng
  1 sibling, 1 reply; 10+ messages in thread
From: Dan Scally @ 2022-11-10 14:22 UTC (permalink / raw)
  To: Jing Leng, balbi, bilbao, corbet, gregkh, laurent.pinchart,
	mchehab+huawei
  Cc: linux-doc, linux-kernel, linux-usb, rdunlap

Hi Jing - thanks for the patch

On 03/11/2022 06:13, Jing Leng wrote:
> The video data endpoint of uvc can be implemented as either an
> isochronous or a bulk endpoint.
>
> The transmission speed of bulk mode is faster than isochronous mode.
> I tested the speed with cdns3 (USB 3.2 Gen1), it's difficult to reach
> 2 Gbps in the isochronous mode, and it can exceed 4 Gbps in the bulk
> mode.
>
> A VideoStreaming interface with isochronous endpoints must have alternate
> settings that can be used to change certain characteristics of the
> interface and underlying endpoint(s). A typical use of alternate settings
> is to provide a way to change the bandwidth requirements an active
> isochronous pipe imposes on the USB.
>
> A VideoStreaming interface containing a bulk endpoint for streaming shall
> support only alternate setting zero. Additional alternate settings
> containing bulk endpoints are not permitted in a device that is compliant
> with the Video Class specification.
>
> Here shows an example of the configfs differences:
>    if [ $BULK -eq 1 ]; then
>        echo "bulk" > functions/$FUNC/streaming_transfer
>        echo $(( 1024 * N )) > functions/$FUNC/streaming_maxpacket
>    else
>        echo "isoc" > functions/$FUNC/streaming_transfer
>        echo 1024 > functions/$FUNC/streaming_maxpacket
>    fi
>
> Signed-off-by: Jing Leng <jleng@ambarella.com>
> ---
> ChangeLog v4->v5:
> - Rebase the patch.
> - Make email addresses ('From' and 'Signed-off-by') consistent.
> ChangeLog v3->v4:
> - echo "bulk" > functions/$FUNC/streaming_transfer to set bulk mode
> ChangeLog v2->v3:
> - Mistakenly deleted the definition of i and USBDHDR when porting from my workdir.
> - Reported-by: kernel test robot <lkp@intel.com>
> ChangeLog v1->v2:
> - Handle imagesize in uvc_v4l2_set_format. If it's not handled,
> - switching from low resolution to high resolution will fail to play.
> ---
>   .../ABI/testing/configfs-usb-gadget-uvc       |   8 +-
>   Documentation/usb/gadget-testing.rst          |   1 +
>   drivers/usb/gadget/function/f_uvc.c           | 228 +++++++++++++-----
>   drivers/usb/gadget/function/u_uvc.h           |   1 +
>   drivers/usb/gadget/function/uvc.h             |   2 +
>   drivers/usb/gadget/function/uvc_configfs.c    |  64 ++++-
>   drivers/usb/gadget/function/uvc_queue.c       |  14 +-
>   drivers/usb/gadget/function/uvc_video.c       |  16 +-
>   8 files changed, 261 insertions(+), 73 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> index 611b23e6488d..ffa2fd8c7fcd 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -3,12 +3,14 @@ Date:		Dec 2014
>   KernelVersion:	4.0
>   Description:	UVC function directory
>   
> -		===================	=============================
> +		===================	===================================
>   		streaming_maxburst	0..15 (ss only)
> -		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
> +		streaming_maxpacket	isoc: 1..1023 (fs), 1..3072 (hs/ss)
> +					bulk: 1024..0x40000000


I'm not really sure that I like this way of representing things, since 
what you're setting with streaming_maxpacket here is not wMaxPacketSize 
but the internal max_payload_size variable. I think that that's apt to 
be quite confusing to people, since the possible values you've listed 
disagree with the specs. It also precludes setting non-maximum values 
for full-speed endpoints, which ought to be able to support 8, 16 and 32 
bits too. I'd prefer another attribute / some other way of determining 
the max_payload_size and full control over the bulk endpoint sizes 
through streaming_maxpacket.


Also, might be worth bobbing another update onto 
Documentation/usb/gadget-testing.rst

>   		streaming_interval	1..16
> +		streaming_transfer	isoc/bulk
>   		function_name		string [32]
> -		===================	=============================
> +		===================	===================================
>   
>   What:		/config/usb-gadget/gadget/functions/uvc.name/control
>   Date:		Dec 2014
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index 2278c9ffb74a..880af56abe8b 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -793,6 +793,7 @@ The uvc function provides these attributes in its function directory:
>   	streaming_maxpacket maximum packet size this endpoint is capable of
>   			    sending or receiving when this configuration is
>   			    selected
> +	streaming_transfer  specify data transmission mode (isoc/bulk)
>   	function_name       name of the interface
>   	=================== ================================================
>   
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 6e196e06181e..8a4df750e00d 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -29,6 +29,8 @@
>   #include "uvc_v4l2.h"
>   #include "uvc_video.h"
>   
> +#define USBDHDR(p) ((struct usb_descriptor_header *)(p))
> +
>   unsigned int uvc_gadget_trace_param;
>   module_param_named(trace, uvc_gadget_trace_param, uint, 0644);
>   MODULE_PARM_DESC(trace, "Trace level bitmask");
> @@ -181,19 +183,19 @@ static struct usb_ss_ep_comp_descriptor uvc_ss_streaming_comp = {
>   	 */
>   };
>   
> -static const struct usb_descriptor_header * const uvc_fs_streaming[] = {
> +static const struct usb_descriptor_header *uvc_fs_streaming[] = {
>   	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
>   	(struct usb_descriptor_header *) &uvc_fs_streaming_ep,
>   	NULL,
>   };
>   
> -static const struct usb_descriptor_header * const uvc_hs_streaming[] = {
> +static const struct usb_descriptor_header *uvc_hs_streaming[] = {
>   	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
>   	(struct usb_descriptor_header *) &uvc_hs_streaming_ep,
>   	NULL,
>   };
>   
> -static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
> +static const struct usb_descriptor_header *uvc_ss_streaming[] = {
>   	(struct usb_descriptor_header *) &uvc_streaming_intf_alt1,
>   	(struct usb_descriptor_header *) &uvc_ss_streaming_ep,
>   	(struct usb_descriptor_header *) &uvc_ss_streaming_comp,
> @@ -204,6 +206,10 @@ static const struct usb_descriptor_header * const uvc_ss_streaming[] = {
>    * Control requests
>    */
>   
> +
> +static int uvc_function_set_alt(struct usb_function *f,
> +		unsigned int interface, unsigned int alt);
> +


Can we just move the function definition here?

>   static void
>   uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
>   {
> @@ -219,6 +225,13 @@ uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
>   		uvc_event->data.length = req->actual;
>   		memcpy(&uvc_event->data.data, req->buf, req->actual);
>   		v4l2_event_queue(&uvc->vdev, &v4l2_event);
> +
> +		/*
> +		 * Bulk mode only has one alt, so we should set STREAM ON after
> +		 * responding the SET UVC_VS_COMMIT_CONTROL request.
> +		 */
> +		if (uvc->state == UVC_STATE_BULK_SETTING)
> +			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 1);
>   	}
>   }


Given bulk mode only has one alt, perhaps it's better to add new 
functions for stream on/off and call those (including in 
uvc_function_set_alt()) to make it clear what's happening.


>   
> @@ -228,6 +241,9 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>   	struct uvc_device *uvc = to_uvc(f);
>   	struct v4l2_event v4l2_event;
>   	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
> +	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
> +	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
> +	unsigned int cs = le16_to_cpu(ctrl->wValue) >> 8 & 0xff;
>   
>   	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
>   		uvcg_info(f, "invalid request type\n");
> @@ -245,6 +261,21 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>   	uvc->event_setup_out = !(ctrl->bRequestType & USB_DIR_IN);
>   	uvc->event_length = le16_to_cpu(ctrl->wLength);
>   
> +	/*
> +	 * Bulk mode only has one alt, when the SET UVC_VS_COMMIT_CONTROL request
> +	 * is received, if the streaming is in transit, we need to set STREAM OFF,
> +	 * if the uvc state is UVC_STATE_BULK_WAITING, we only need to change it.
> +	 */
> +	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK &&
> +		uvc->event_setup_out &&
> +		uvc->streaming_intf == interface &&
> +		cs == UVC_VS_COMMIT_CONTROL) {
> +		if (uvc->state == UVC_STATE_STREAMING)
> +			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 0);
> +		else if (uvc->state == UVC_STATE_BULK_WAITING)
> +			uvc->state = UVC_STATE_BULK_SETTING;
> +	}


As far as I can tell this path doesn't quite work out; when I test the 
code I don't get uvc_function_set_alt(1, 0) being called, so the 
streamoff isn't happening correctly when in bulk mode. 
uvc_video_stop_streaming() in drivers/media/usb/uvc/uvc_video.c has this 
comment:


/*
* UVC doesn't specify how to inform a bulk-based device
* when the video stream is stopped. Windows sends a
* CLEAR_FEATURE(HALT) request to the video streaming
* bulk endpoint, mimic the same behaviour.
*/


Perhaps it would be good to accommodate that method?


> +
>   	memset(&v4l2_event, 0, sizeof(v4l2_event));
>   	v4l2_event.type = UVC_EVENT_SETUP;
>   	memcpy(&uvc_event->req, ctrl, sizeof(uvc_event->req));
> @@ -255,9 +286,12 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>   
>   void uvc_function_setup_continue(struct uvc_device *uvc)
>   {
> +	struct f_uvc_opts *opts = fi_to_f_uvc_opts(uvc->func.fi);
>   	struct usb_composite_dev *cdev = uvc->func.config->cdev;
>   
> -	usb_composite_setup_continue(cdev);
> +	/* delayed_status in bulk mode is 0, so it doesn't need to continue. */
> +	if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
> +		usb_composite_setup_continue(cdev);
>   }
>   
>   static int
> @@ -282,6 +316,7 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
>   	struct usb_composite_dev *cdev = f->config->cdev;
>   	struct v4l2_event v4l2_event;
>   	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
> +	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
>   	int ret;
>   
>   	uvcg_info(f, "%s(%u, %u)\n", __func__, interface, alt);
> @@ -314,15 +349,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
>   	if (interface != uvc->streaming_intf)
>   		return -EINVAL;
>   
> -	/* TODO
> -	if (usb_endpoint_xfer_bulk(&uvc->desc.vs_ep))
> -		return alt ? -EINVAL : 0;
> -	*/


I think this probably should stay (uncommented) to be strictly 
compliant, with the other changes to this function moved to stream 
on/off functions.

> -
>   	switch (alt) {
>   	case 0:
> -		if (uvc->state != UVC_STATE_STREAMING)
> -			return 0;
> +		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
> +			if (uvc->state == UVC_STATE_CONNECTED)
> +				uvc->state = UVC_STATE_BULK_WAITING;
> +			else if (uvc->state == UVC_STATE_STREAMING)
> +				uvc->state = UVC_STATE_BULK_SETTING;
> +			else
> +				return 0;
> +		} else {
> +			if (uvc->state != UVC_STATE_STREAMING)
> +				return 0;
> +		}
>   
>   		if (uvc->video.ep)
>   			usb_ep_disable(uvc->video.ep);
> @@ -331,12 +370,19 @@ uvc_function_set_alt(struct usb_function *f, unsigned interface, unsigned alt)
>   		v4l2_event.type = UVC_EVENT_STREAMOFF;
>   		v4l2_event_queue(&uvc->vdev, &v4l2_event);
>   
> -		uvc->state = UVC_STATE_CONNECTED;
> +		if (opts->streaming_transfer != USB_ENDPOINT_XFER_BULK)
> +			uvc->state = UVC_STATE_CONNECTED;
> +
>   		return 0;
>   
>   	case 1:
> -		if (uvc->state != UVC_STATE_CONNECTED)
> -			return 0;
> +		if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
> +			if (uvc->state != UVC_STATE_BULK_SETTING)
> +				return 0;
> +		} else {
> +			if (uvc->state != UVC_STATE_CONNECTED)
> +				return 0;
> +		}
>   
>   		if (!uvc->video.ep)
>   			return -EINVAL;
> @@ -598,62 +644,101 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>   	unsigned int max_packet_size;
>   	struct usb_ep *ep;
>   	struct f_uvc_opts *opts;
> +	int i = 0;
>   	int ret = -EINVAL;
>   
>   	uvcg_info(f, "%s()\n", __func__);
>   
>   	opts = fi_to_f_uvc_opts(f->fi);
> -	/* Sanity check the streaming endpoint module parameters. */
> -	opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
> -	opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
> -	opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
> -
> -	/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
> -	if (opts->streaming_maxburst &&
> -	    (opts->streaming_maxpacket % 1024) != 0) {
> -		opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
> -		uvcg_info(f, "overriding streaming_maxpacket to %d\n",
> -			  opts->streaming_maxpacket);
> -	}
>   
> -	/*
> -	 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
> -	 * module parameters.
> -	 *
> -	 * NOTE: We assume that the user knows what they are doing and won't
> -	 * give parameters that their UDC doesn't support.
> -	 */
> -	if (opts->streaming_maxpacket <= 1024) {
> -		max_packet_mult = 1;
> -		max_packet_size = opts->streaming_maxpacket;
> -	} else if (opts->streaming_maxpacket <= 2048) {
> -		max_packet_mult = 2;
> -		max_packet_size = opts->streaming_maxpacket / 2;
> +	/* Handle different transfer mode for stream endpoints */
> +	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
> +		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
> +		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
> +		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
> +
> +		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
> +
> +		if (opts->streaming_maxpacket % 1024 != 0) {
> +			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
> +			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
> +				opts->streaming_maxpacket);
> +		}
> +
> +		uvc_fs_streaming_ep.wMaxPacketSize = cpu_to_le16(64);
> +		uvc_fs_streaming_ep.bInterval = 0;
> +
> +		uvc_hs_streaming_ep.wMaxPacketSize = cpu_to_le16(512);
> +		uvc_hs_streaming_ep.bInterval = 0;
> +
> +		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(1024);
> +		uvc_ss_streaming_ep.bInterval = 0;
> +


As I say, I think that these ought to be set by streaming_maxpacket

> +		uvc_ss_streaming_comp.bmAttributes = 0;
> +		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
> +		uvc_ss_streaming_comp.wBytesPerInterval = 0;
> +
> +		uvc->video.max_payload_size = opts->streaming_maxpacket;
>   	} else {
> -		max_packet_mult = 3;
> -		max_packet_size = opts->streaming_maxpacket / 3;
> -	}
> +		uvc_fs_streaming_ep.bmAttributes = opts->streaming_transfer;
> +		uvc_hs_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
> +		uvc_ss_streaming_ep.bmAttributes = uvc_fs_streaming_ep.bmAttributes;
> +
> +		/* Sanity check the streaming endpoint module parameters. */
> +		opts->streaming_interval = clamp(opts->streaming_interval, 1U, 16U);
> +		opts->streaming_maxpacket = clamp(opts->streaming_maxpacket, 1U, 3072U);
> +		opts->streaming_maxburst = min(opts->streaming_maxburst, 15U);
> +
> +		/* For SS, wMaxPacketSize has to be 1024 if bMaxBurst is not 0 */
> +		if (opts->streaming_maxburst &&
> +			(opts->streaming_maxpacket % 1024) != 0) {
> +			opts->streaming_maxpacket = roundup(opts->streaming_maxpacket, 1024);
> +			uvcg_info(f, "overriding streaming_maxpacket to %d\n",
> +				opts->streaming_maxpacket);
> +		}
>   
> -	uvc_fs_streaming_ep.wMaxPacketSize =
> -		cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
> -	uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
> +		/*
> +		 * Fill in the FS/HS/SS Video Streaming specific descriptors from the
> +		 * module parameters.
> +		 *
> +		 * NOTE: We assume that the user knows what they are doing and won't
> +		 * give parameters that their UDC doesn't support.
> +		 */
> +		if (opts->streaming_maxpacket <= 1024) {
> +			max_packet_mult = 0;
> +			max_packet_size = opts->streaming_maxpacket;
> +		} else if (opts->streaming_maxpacket <= 2048) {
> +			max_packet_mult = 1;
> +			max_packet_size = opts->streaming_maxpacket / 2;
> +		} else {
> +			max_packet_mult = 2;
> +			max_packet_size = opts->streaming_maxpacket / 3;
> +		}
>   
> -	uvc_hs_streaming_ep.wMaxPacketSize =
> -		cpu_to_le16(max_packet_size | ((max_packet_mult - 1) << 11));
> +		uvc_fs_streaming_ep.wMaxPacketSize =
> +			cpu_to_le16(min(opts->streaming_maxpacket, 1023U));
> +		uvc_fs_streaming_ep.bInterval = opts->streaming_interval;
>   
> -	/* A high-bandwidth endpoint must specify a bInterval value of 1 */
> -	if (max_packet_mult > 1)
> -		uvc_hs_streaming_ep.bInterval = 1;
> -	else
> -		uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
> +		uvc_hs_streaming_ep.wMaxPacketSize =
> +			cpu_to_le16(max_packet_size | (max_packet_mult << 11));
> +
> +		/* A high-bandwidth endpoint must specify a bInterval value of 1 */
> +		if (max_packet_mult > 0)
> +			uvc_hs_streaming_ep.bInterval = 1;
> +		else
> +			uvc_hs_streaming_ep.bInterval = opts->streaming_interval;
> +
> +		uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
> +		uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
> +
Unneeded blank line
> +		uvc_ss_streaming_comp.bmAttributes = max_packet_mult;
> +		uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
> +		uvc_ss_streaming_comp.wBytesPerInterval =
> +			cpu_to_le16(max_packet_size * (max_packet_mult + 1) *
> +				(opts->streaming_maxburst + 1));
>   
> -	uvc_ss_streaming_ep.wMaxPacketSize = cpu_to_le16(max_packet_size);
> -	uvc_ss_streaming_ep.bInterval = opts->streaming_interval;
> -	uvc_ss_streaming_comp.bmAttributes = max_packet_mult - 1;
> -	uvc_ss_streaming_comp.bMaxBurst = opts->streaming_maxburst;
> -	uvc_ss_streaming_comp.wBytesPerInterval =
> -		cpu_to_le16(max_packet_size * max_packet_mult *
> -			    (opts->streaming_maxburst + 1));
> +		uvc->video.max_payload_size = 0;
> +	}
>   
>   	/* Allocate endpoints. */
>   	ep = usb_ep_autoconfig(cdev->gadget, &uvc_control_ep);
> @@ -667,7 +752,7 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>   		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_ss_streaming_ep,
>   					  &uvc_ss_streaming_comp);
>   	else if (gadget_is_dualspeed(cdev->gadget))
> -		ep = usb_ep_autoconfig(cdev->gadget, &uvc_hs_streaming_ep);
> +		ep = usb_ep_autoconfig_ss(cdev->gadget, &uvc_hs_streaming_ep, NULL);


This doesn't seem right.

>   	else
>   		ep = usb_ep_autoconfig(cdev->gadget, &uvc_fs_streaming_ep);
>   
> @@ -709,6 +794,28 @@ uvc_function_bind(struct usb_configuration *c, struct usb_function *f)
>   	uvc->streaming_intf = ret;
>   	opts->streaming_interface = ret;
>   
> +	/* Handle different transfer mode for descriptors */
> +	i = 0;
> +	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK) {
> +		uvc_streaming_intf_alt0.bNumEndpoints = 1;
> +	} else {
> +		uvc_streaming_intf_alt0.bNumEndpoints = 0;
> +
> +		uvc_fs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
> +		uvc_hs_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
> +		uvc_ss_streaming[i] = USBDHDR(&uvc_streaming_intf_alt1);
> +		++i;
> +	}
> +	uvc_fs_streaming[i] = USBDHDR(&uvc_fs_streaming_ep);
> +	uvc_hs_streaming[i] = USBDHDR(&uvc_hs_streaming_ep);
> +	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_ep);
> +	++i;
> +	uvc_fs_streaming[i] = NULL;
> +	uvc_hs_streaming[i] = NULL;
> +	uvc_ss_streaming[i] = USBDHDR(&uvc_ss_streaming_comp);
> +	++i;
> +	uvc_ss_streaming[i] = NULL;
> +
>   	/* Copy descriptors */
>   	f->fs_descriptors = uvc_copy_descriptors(uvc, USB_SPEED_FULL);
>   	if (IS_ERR(f->fs_descriptors)) {
> @@ -872,6 +979,7 @@ static struct usb_function_instance *uvc_alloc_inst(void)
>   
>   	opts->streaming_interval = 1;
>   	opts->streaming_maxpacket = 1024;
> +	opts->streaming_transfer = USB_ENDPOINT_SYNC_ASYNC | USB_ENDPOINT_XFER_ISOC;
>   	snprintf(opts->function_name, sizeof(opts->function_name), "UVC Camera");
>   
>   	ret = uvcg_attach_configfs(opts);
> diff --git a/drivers/usb/gadget/function/u_uvc.h b/drivers/usb/gadget/function/u_uvc.h
> index 24b8681b0d6f..88cd235e0ea8 100644
> --- a/drivers/usb/gadget/function/u_uvc.h
> +++ b/drivers/usb/gadget/function/u_uvc.h
> @@ -24,6 +24,7 @@ struct f_uvc_opts {
>   	unsigned int					streaming_interval;
>   	unsigned int					streaming_maxpacket;
>   	unsigned int					streaming_maxburst;
> +	unsigned int					streaming_transfer;
>   
>   	unsigned int					control_interface;
>   	unsigned int					streaming_interface;
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index 40226b1f7e14..b00cba322643 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -123,6 +123,8 @@ enum uvc_state {
>   	UVC_STATE_DISCONNECTED,
>   	UVC_STATE_CONNECTED,
>   	UVC_STATE_STREAMING,
> +	UVC_STATE_BULK_WAITING,
> +	UVC_STATE_BULK_SETTING,
>   };
>   
>   struct uvc_device {


I think it's worth splitting the new configfs attribute at least into a 
separate patch - this combined one does a lot.

> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index 4303a3283ba0..ef7c4f6630d4 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -2348,11 +2348,72 @@ end:									\
>   UVC_ATTR(f_uvc_opts_, cname, cname)
>   
>   UVCG_OPTS_ATTR(streaming_interval, streaming_interval, 16);
> -UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 3072);
> +UVCG_OPTS_ATTR(streaming_maxpacket, streaming_maxpacket, 0x40000000U);
>   UVCG_OPTS_ATTR(streaming_maxburst, streaming_maxburst, 15);
>   
>   #undef UVCG_OPTS_ATTR
>   
> +#define UVCG_OPTS_ATTR_TRANSFER(cname, aname)				\
> +static ssize_t f_uvc_opts_##cname##_show(				\
> +	struct config_item *item, char *page)				\
> +{									\
> +	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
> +	int result;							\
> +	char *str;							\
> +									\
> +	mutex_lock(&opts->lock);					\
> +	switch (opts->cname) {						\
> +	case USB_ENDPOINT_XFER_BULK:					\
> +		str = "bulk";						\
> +		break;							\
> +	case USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC:		\
> +		str = "isoc";						\
> +		break;							\
> +	default:							\
> +		str = "unknown";					\
> +		break;							\
> +	}								\
> +	result = sprintf(page, "%s\n", str);				\
> +	mutex_unlock(&opts->lock);					\
> +									\
> +	return result;							\
> +}									\
> +									\
> +static ssize_t								\
> +f_uvc_opts_##cname##_store(struct config_item *item,			\
> +			   const char *page, size_t len)		\
> +{									\
> +	struct f_uvc_opts *opts = to_f_uvc_opts(item);			\
> +	int ret = 0;							\
> +									\
> +	mutex_lock(&opts->lock);					\
> +	if (opts->refcnt) {						\
> +		ret = -EBUSY;						\
> +		goto end;						\
> +	}								\
> +									\
> +	if (!strncmp(page, "bulk", 4))					\
> +		opts->cname = USB_ENDPOINT_XFER_BULK;			\
> +	else if (!strncmp(page, "isoc", 4))				\
> +		opts->cname = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC; \
> +	else {								\
> +		ret = -EINVAL;						\
> +		goto end;						\
> +	}								\
> +									\
> +	ret = len;							\
> +									\
> +end:									\
> +	mutex_unlock(&opts->lock);					\
> +	return ret;							\
> +}									\
> +									\
> +UVC_ATTR(f_uvc_opts_, cname, cname)
> +
> +UVCG_OPTS_ATTR_TRANSFER(streaming_transfer, streaming_transfer);


Is it worth the macro for a single attribute? Particularly since the 
set/store is too specific for it to be reusable in the future anyway. I 
think I'd just write the bare function.

> +
> +#undef UVCG_OPTS_ATTR_TRANSFER
> +
>   #define UVCG_OPTS_STRING_ATTR(cname, aname)				\
>   static ssize_t f_uvc_opts_string_##cname##_show(struct config_item *item,\
>   					 char *page)			\
> @@ -2399,6 +2460,7 @@ static struct configfs_attribute *uvc_attrs[] = {
>   	&f_uvc_opts_attr_streaming_interval,
>   	&f_uvc_opts_attr_streaming_maxpacket,
>   	&f_uvc_opts_attr_streaming_maxburst,
> +	&f_uvc_opts_attr_streaming_transfer,
>   	&f_uvc_opts_string_attr_function_name,
>   	NULL,
>   };
> diff --git a/drivers/usb/gadget/function/uvc_queue.c b/drivers/usb/gadget/function/uvc_queue.c
> index 0aa3d7e1f3cc..2695b3ed29b5 100644
> --- a/drivers/usb/gadget/function/uvc_queue.c
> +++ b/drivers/usb/gadget/function/uvc_queue.c
> @@ -54,9 +54,13 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>   
>   	sizes[0] = video->imagesize;
>   
> -	req_size = video->ep->maxpacket
> -		 * max_t(unsigned int, video->ep->maxburst, 1)
> -		 * (video->ep->mult);
> +	/* Bulk mode uses max_payload_size as req_size */
> +	if (video->max_payload_size)
> +		req_size = video->max_payload_size;
> +	else
> +		req_size = video->ep->maxpacket
> +			 * max_t(unsigned int, video->ep->maxburst, 1)
> +			 * (video->ep->mult);
>   
>   	/* We divide by two, to increase the chance to run
>   	 * into fewer requests for smaller framesizes.
> @@ -143,7 +147,9 @@ 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) {
> +
> +	/* UDC supports scatter gather and transfer mode isn't bulk. */
> +	if (cdev->gadget->sg_supported && !video->max_payload_size) {
>   		queue->queue.mem_ops = &vb2_dma_sg_memops;
>   		queue->use_sg = 1;
>   	} else {
> diff --git a/drivers/usb/gadget/function/uvc_video.c b/drivers/usb/gadget/function/uvc_video.c
> index dd1c6b2ca7c6..70737340acfb 100644
> --- a/drivers/usb/gadget/function/uvc_video.c
> +++ b/drivers/usb/gadget/function/uvc_video.c
> @@ -271,8 +271,10 @@ uvc_video_complete(struct usb_ep *ep, struct usb_request *req)
>   		break;
>   
>   	default:
> -		uvcg_warn(&video->uvc->func,
> -			  "VS request completed with status %d.\n",
> +		if (uvc->state == UVC_STATE_BULK_WAITING ||
> +			uvc->state == UVC_STATE_BULK_SETTING)
> +			break;
> +		uvcg_warn(&uvc->func, "VS request completed with status %d.\n",
>   			  req->status);
>   		uvcg_queue_cancel(queue, 0);
>   	}
> @@ -328,9 +330,13 @@ uvc_video_alloc_requests(struct uvc_video *video)
>   
>   	BUG_ON(video->req_size);
>   
> -	req_size = video->ep->maxpacket
> -		 * max_t(unsigned int, video->ep->maxburst, 1)
> -		 * (video->ep->mult);
> +	/* Bulk mode uses max_payload_size as req_size */
> +	if (video->max_payload_size)
> +		req_size = video->max_payload_size;
> +	else
> +		req_size = video->ep->maxpacket
> +			 * max_t(unsigned int, video->ep->maxburst, 1)
> +			 * (video->ep->mult);
>   
>   	video->ureq = kcalloc(video->uvc_num_requests, sizeof(struct uvc_request), GFP_KERNEL);
>   	if (video->ureq == NULL)

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

* RE: [EXT] Re: [PATCH v5] usb: gadget: uvc: add bulk transfer support
  2022-11-10 14:22       ` Dan Scally
@ 2022-11-11  3:51         ` Jing Leng
  0 siblings, 0 replies; 10+ messages in thread
From: Jing Leng @ 2022-11-11  3:51 UTC (permalink / raw)
  To: Dan Scally, balbi, bilbao, corbet, gregkh, laurent.pinchart,
	mchehab+huawei
  Cc: linux-doc, linux-kernel, linux-usb, rdunlap

Hi Dan Scally,

>> -		===================	=============================
>> +		===================	===================================
>>   		streaming_maxburst	0..15 (ss only)
>> -		streaming_maxpacket	1..1023 (fs), 1..3072 (hs/ss)
>> +		streaming_maxpacket	isoc: 1..1023 (fs), 1..3072 (hs/ss)
>> +					bulk: 1024..0x40000000
>
>
>I'm not really sure that I like this way of representing things, since what you're setting with streaming_maxpacket here is not wMaxPacketSize but the internal max_payload_size variable. I think that that's apt to be quite confusing to people, since the possible values you've listed disagree with the specs. It also precludes setting non-maximum values for full-speed endpoints, which ought to be able to support 8, 16 and 32 bits too. I'd prefer another attribute / some other way of determining the max_payload_size and full control over the bulk endpoint sizes through streaming_maxpacket.

UVC requires high bandwidth, so we'd better give the maximum wMaxPacketSize for full-speed ep.
And wMaxPacketSize of bulk ep is a fixed value in high-speed and supper-speed, so if we use streaming_maxpacket to set wMaxPacketSize for bulk ep, it not a good idea.
If we respond to UVC_SC_VIDEOSTREAMING request, we should set max_payload_size to dwMaxPayloadTransferSize in bulk mode, Reusing streaming_maxpacket keeps setting dwMaxPayloadTransferSize uniform in bulk mode and isoc mode.


>>   static void
>>   uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
>>   {
>> @@ -219,6 +225,13 @@ uvc_function_ep0_complete(struct usb_ep *ep, struct usb_request *req)
>>   		uvc_event->data.length = req->actual;
>>   		memcpy(&uvc_event->data.data, req->buf, req->actual);
>>   		v4l2_event_queue(&uvc->vdev, &v4l2_event);
>> +
>> +		/*
>> +		 * Bulk mode only has one alt, so we should set STREAM ON after
>> +		 * responding the SET UVC_VS_COMMIT_CONTROL request.
>> +		 */
>> +		if (uvc->state == UVC_STATE_BULK_SETTING)
>> +			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 1);
>>   	}
>>   }
>
>
>Given bulk mode only has one alt, perhaps it's better to add new functions for stream on/off and call those (including in
>uvc_function_set_alt()) to make it clear what's happening.

UVC will set 'alt 0' when be configured, not only in the SET UVC_VS_COMMIT_CONTROL request  added by me.

>>   
>> @@ -228,6 +241,9 @@ uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>>   	struct uvc_device *uvc = to_uvc(f);
>>   	struct v4l2_event v4l2_event;
>>   	struct uvc_event *uvc_event = (void *)&v4l2_event.u.data;
>> +	struct f_uvc_opts *opts = fi_to_f_uvc_opts(f->fi);
>> +	unsigned int interface = le16_to_cpu(ctrl->wIndex) & 0xff;
>> +	unsigned int cs = le16_to_cpu(ctrl->wValue) >> 8 & 0xff;
>>   
>>   	if ((ctrl->bRequestType & USB_TYPE_MASK) != USB_TYPE_CLASS) {
>>   		uvcg_info(f, "invalid request type\n"); @@ -245,6 +261,21 @@ 
>> uvc_function_setup(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
>>   	uvc->event_setup_out = !(ctrl->bRequestType & USB_DIR_IN);
>>   	uvc->event_length = le16_to_cpu(ctrl->wLength);
>>   
>> +	/*
>> +	 * Bulk mode only has one alt, when the SET UVC_VS_COMMIT_CONTROL request
>> +	 * is received, if the streaming is in transit, we need to set STREAM OFF,
>> +	 * if the uvc state is UVC_STATE_BULK_WAITING, we only need to change it.
>> +	 */
>> +	if (opts->streaming_transfer == USB_ENDPOINT_XFER_BULK &&
>> +		uvc->event_setup_out &&
>> +		uvc->streaming_intf == interface &&
>> +		cs == UVC_VS_COMMIT_CONTROL) {
>> +		if (uvc->state == UVC_STATE_STREAMING)
>> +			uvc_function_set_alt(&uvc->func, uvc->streaming_intf, 0);
>> +		else if (uvc->state == UVC_STATE_BULK_WAITING)
>> +			uvc->state = UVC_STATE_BULK_SETTING;
>> +	}
>
>
>As far as I can tell this path doesn't quite work out; when I test the code I don't get uvc_function_set_alt(1, 0) being called, so the streamoff isn't happening correctly when in bulk mode. 
uvc_video_stop_streaming() in drivers/media/usb/uvc/uvc_video.c has this
comment:

UVC will set 'alt 0' when be configured, so the first play will execute 'uvc->state = UVC_STATE_BULK_SETTING', and  'alt 0' will be executed when replaying.
So I add these logics in uvc_function_setup and uvc_function_ep0_complete.

>/*
>* UVC doesn't specify how to inform a bulk-based device
>* when the video stream is stopped. Windows sends a
>* CLEAR_FEATURE(HALT) request to the video streaming
>* bulk endpoint, mimic the same behaviour.
>*/
>
>Perhaps it would be good to accommodate that method?

I test uvc on Windows 10 (PotPlayer) and Ubuntu 20.04 (v4l2-ctl) with cdnsp UDC, composite_setup doesn't recieve a CLEAR_FEATURE request when replaying, See the following print info:

[  764.069697] composite_setup: 21 1 100 1 1a
[  764.069914] composite_setup: a1 82 100 1 1a
[  764.070088] composite_setup: a1 83 100 1 1a
[  764.070389] composite_setup: 21 1 100 1 1a
[  764.070552] composite_setup: a1 81 100 1 1a
[  764.072136] composite_setup: 21 1 200 1 1a
[  764.072141] configfs-gadget gadget: uvc: uvc_function_set_alt(1, 0)
[  765.057620] configfs-gadget gadget: uvc: uvc_function_set_alt(1, 1)
[  765.063911] configfs-gadget gadget: uvc: reset UVC
[  767.094115] composite_setup: 21 1 100 1 1a
[  767.094321] composite_setup: a1 82 100 1 1a
[  767.094502] composite_setup: a1 83 100 1 1a
[  767.094683] composite_setup: 21 1 100 1 1a
[  767.094864] composite_setup: a1 81 100 1 1a
[  767.096469] composite_setup: 21 1 200 1 1a
[  767.096475] configfs-gadget gadget: uvc: uvc_function_set_alt(1, 0)
[  768.082019] configfs-gadget gadget: uvc: uvc_function_set_alt(1, 1)
[  768.088310] configfs-gadget gadget: uvc: reset UVC

**********************************************************************
This email and attachments contain Ambarella Proprietary and/or Confidential Information and is intended solely for the use of the individual(s) to whom it is addressed. Any unauthorized review, use, disclosure, distribute, copy, or print is prohibited. If you are not an intended recipient, please contact the sender by reply email and destroy all copies of the original message. Thank you.

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

end of thread, other threads:[~2022-11-11  5:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25  7:00 [PATCH] usb: gadget: uvc: add bulk transfer support 3090101217
2022-05-12  9:42 ` [PATCH v2] " 3090101217
2022-05-12 14:47   ` kernel test robot
2022-05-13  0:42   ` [PATCH v3] " 3090101217
2022-10-25  9:05     ` [PATCH v4] " 3090101217
2022-10-25 10:32       ` Greg KH
2022-11-03  6:13     ` [PATCH v5] " Jing Leng
2022-11-08 15:47       ` Greg KH
2022-11-10 14:22       ` Dan Scally
2022-11-11  3:51         ` [EXT] " Jing Leng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.