usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

PATCH Superseded Laurent Pinchart 1 participant 1 message Mar 20, 2018 USB

lore

Share & tools

lore

Laurent Pinchart

author

2018-03-20 09:02 UTC #0 lore raw

PATCH v2 1/1 usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

Hi Joel,

(CC'ing Paul Elder who is working on the UVC gadget driver)

Thank you for the patch.

Quoted 8 lines · On Monday, 19 March 2018 21:36:41 EET Joel Pepper wrote:

Add bFrameIndex as a UVCG_FRAME_ATTR for each frame size. Before all "bFrameindex" attributes were set to "1" with no way to configure the gadget otherwise. This resulted in the host always negotiating for bFrameIndex 1 (i.e. the first framesize of the gadget). After the negotiation the host driver will set the user or application selected framesize, while the gadget is actually set to the first framesize.

Quoted 6 lines

Note that this still requires the gadget to be configured with unique bFrameIndexes for each frameSize of each format through configfs. An alternative might be to automatically assign ascending indices when the format is linked into the streaming header, but the user space gadget would need a way to check or predict the indices so that it can properly interpret to PROBE/COMMIT CONTROL requests

It would indeed be nice if the indices could be automatically generated, to avoid giving userspace another possibility to create invalid descriptors. As you've correctly noted, that would require a way for the userspace helper application to coordinate with the UVC gadget driver. Would it be difficult to do so by defining the bFrameIndex attribute as read-only ? There's the additional issue of finding a place to store the index counter locally to the format, I'm not sure how that's done with the configfs API, but if it's not too difficult I think it would be a good option.

Quoted 18 lines

v2: Add the new attribute to both MJPEG and uncompressedframe descriptos in Documentation/ABI, with note that it was added only in a later kernel version
Signed-off-by Joel Pepper <[email protected]>
---
 Documentation/ABI/testing/configfs-usb-gadget-uvc | 17 +++++++++++++++++
 drivers/usb/gadget/function/uvc_configfs.c        |  3 +++
 2 files changed, 20 insertions(+)
(quoted diff) +0 -0
b/Documentation/ABI/testing/configfs-usb-gadget-uvc index 1ba0d0f..d435cf7 100644
Documentation/ABI/testing/configfs-usb-gadget-uvc +0 -0
@@ -194,6 +194,14 @@ Description:	Specific MJPEG frame descriptors
 		bmCapabilities		- still image support, fixed frame-rate
 					support

Documentation/ABI/testing/configfs-usb-gadget-uvc Description: Specific MJPEG frame descriptors · line 205

Quoted 10 lines

Documentation/ABI/testing/configfs-usb-gadget-uvc +8 -0

 
+Date:		Mar 2018
+KernelVersion:	4.16
+
+		bFrameIndex		- unique id for this framedescriptor;
+					if using multiple framedescriptors for
+					same format, user needs to set distinct
+					value for each frame descriptor
+
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/

uncompressed

Quoted 41 lines

Documentation/ABI/testing/configfs-usb-gadget-uvc +4 -0
 Date:		Dec 2014
 KernelVersion:	4.0
@@ -241,6 +249,15 @@ Description:	Specific uncompressed frame descriptors
 		bmCapabilities		- still image support, fixed frame-rate
 					support
 
+Date:           Mar 2018
+KernelVersion:  4.16
+
+                bFrameIndex             - unique id for this
framedescriptor; + if using multiple
framedescriptors for + same format,
user needs to set distinct + value
for each frame descriptor +
Documentation/ABI/testing/configfs-usb-gadget-uvc +1 -0
+
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/header
 Date:		Dec 2014
 KernelVersion:	4.0
(quoted diff) +0 -0
b/drivers/usb/gadget/function/uvc_configfs.c index c9b8cc4a..5966d65 100644
drivers/usb/gadget/function/uvc_configfs.c +2 -0
@@ -992,6 +992,8 @@ UVC_ATTR(uvcg_frame_, cname, aname);
 
 UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
 		noop_conversion, 8);
+UVCG_FRAME_ATTR(b_frame_index, bFrameIndex, noop_conversion,
+		noop_conversion, 8);
 UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
 UVCG_FRAME_ATTR(w_height, wHeight, le16_to_cpu, cpu_to_le16, 16);
 UVCG_FRAME_ATTR(dw_min_bit_rate, dwMinBitRate, le32_to_cpu, cpu_to_le32,
32);
drivers/usb/gadget/function/uvc_configfs.c +1 -0
@@ -1137,6 +1139,7 @@ UVC_ATTR(uvcg_frame_, dw_frame_interval,
 dwFrameInterval);
 
 static struct configfs_attribute *uvcg_frame_attrs[] = {
 	&uvcg_frame_attr_bm_capabilities,
+	&uvcg_frame_attr_b_frame_index,
 	&uvcg_frame_attr_w_width,
 	&uvcg_frame_attr_w_height,
&uvcg_frame_attr_dw_min_bit_rate,