[v5,1/2] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

[v5,1/2] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

[Joel Pepper]

- Add bFrameIndex as a UVCG_FRAME_ATTR_RO for each frame size.
- Automatically assign ascending bFrameIndex to each frame in a format.

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.

Now, when the containing format is linked into the streaming header,
iterate over all child frame descriptors and assign ascending indices.
The automatically assigned indices can be read from the new read only
bFrameIndex configsfs attribute in each frame descriptor item.

v2: Add the new attribute to both MJPEG and uncompressed frame descriptors
in Documentation/ABI, with note that it was added only in a later
kernel version

v3: Changed from simply allowing user to set the value for bFrameIndex to
automatically assigning correct distinct frame indexes. Changed
bFrameIndex from RW to RO

v4: Actually include updated patch

v5: bFrameIndex now returns -EBUSY if the parent fmt is not linked yet

Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
---
 Documentation/ABI/testing/configfs-usb-gadget-uvc | 18 +++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 64 +++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 1ba0d0f..7bfec94 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -194,6 +194,14 @@ Description:	Specific MJPEG frame descriptors
 		bmCapabilities		- still image support, fixed frame-rate
 					support
 
+Date:		Mar 2018
+KernelVersion:	4.16
+
+		bFrameIndex		- unique id for this framedescriptor;
+					only defined after parent format is
+					linked into the streaming header;
+					read-only
+
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/uncompressed
 Date:		Dec 2014
 KernelVersion:	4.0
@@ -241,6 +249,16 @@ 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;
+                                        only defined after parent format is
+                                        linked into the streaming header;
+                                        read-only
+
+
+
 What:		/config/usb-gadget/gadget/functions/uvc.name/streaming/header
 Date:		Dec 2014
 KernelVersion:	4.0
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index c9b8cc4a..ab64925 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -32,6 +32,7 @@ static struct configfs_attribute prefix##attr_##cname = { \
 }
 
 static inline struct f_uvc_opts *to_f_uvc_opts(struct config_item *item);
+static void __uvcg_fmt_set_indices(struct config_group *fmt);
 
 /* control/header/<NAME> */
 DECLARE_UVC_HEADER_DESCRIPTOR(1);
@@ -751,6 +752,8 @@ static int uvcg_streaming_header_allow_link(struct config_item *src,
 	if (!target_fmt)
 		goto out;
 
+	__uvcg_fmt_set_indices(to_config_group(target));
+
 	format_ptr = kzalloc(sizeof(*format_ptr), GFP_KERNEL);
 	if (!format_ptr) {
 		ret = -ENOMEM;
@@ -988,8 +991,46 @@ end:									\
 									\
 UVC_ATTR(uvcg_frame_, cname, aname);
 
+
+static ssize_t uvcg_frame_b_frame_index_show(struct config_item *item,
+		char *page)
+{
+	struct uvcg_frame *f = to_uvcg_frame(item);
+	struct uvcg_format *fmt;
+	struct f_uvc_opts *opts;
+	struct config_item *opts_item;
+	struct config_item *fmt_item;
+	struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;
+	int result;
+
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */
+
+	fmt_item = f->item.ci_parent;
+	fmt = to_uvcg_format(fmt_item);
+
+	if (!fmt->linked) {
+		result = -EBUSY;
+		goto out;
+	}
+
+	opts_item = fmt_item->ci_parent->ci_parent->ci_parent;
+	opts = to_f_uvc_opts(opts_item);
+
+	mutex_lock(&opts->lock);
+	result = sprintf(page, "%d\n", f->frame.b_frame_index);
+	mutex_unlock(&opts->lock);
+
+out:
+	mutex_unlock(su_mutex);
+	return result;
+}
+
+UVC_ATTR_RO(uvcg_frame_, b_frame_index, bFrameIndex);
+
 #define noop_conversion(x) (x)
 
+
+
 UVCG_FRAME_ATTR(bm_capabilities, bmCapabilities, noop_conversion,
 		noop_conversion, 8);
 UVCG_FRAME_ATTR(w_width, wWidth, le16_to_cpu, cpu_to_le16, 16);
@@ -1137,6 +1178,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,
@@ -1214,6 +1256,28 @@ static void uvcg_frame_drop(struct config_group *group, struct config_item *item
 	mutex_unlock(&opts->lock);
 }
 
+static void __uvcg_fmt_set_indices(struct config_group *fmt)
+{
+	u8 i = 1;
+	struct list_head *entry;
+	struct config_item *ci;
+	struct uvcg_frame *frm;
+
+	list_for_each(entry, &(fmt->cg_children)) {
+
+		ci = container_of(entry, struct config_item, ci_entry);
+
+		if (ci->ci_type != &uvcg_frame_type)
+			continue;
+
+		frm = to_uvcg_frame(ci);
+		frm->frame.b_frame_index = i;
+		i++;
+
+	}
+
+}
+
 /* streaming/uncompressed/<NAME> */
 struct uvcg_uncompressed {
 	struct uvcg_format		fmt;

[v5,1/2] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

[Felipe Balbi]

Hi,

Joel Pepper <joel.pepper@rwth-aachen.de> writes:
> - Add bFrameIndex as a UVCG_FRAME_ATTR_RO for each frame size.
> - Automatically assign ascending bFrameIndex to each frame in a format.
>
> 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.
>
> Now, when the containing format is linked into the streaming header,
> iterate over all child frame descriptors and assign ascending indices.
> The automatically assigned indices can be read from the new read only
> bFrameIndex configsfs attribute in each frame descriptor item.
>
> v2: Add the new attribute to both MJPEG and uncompressed frame descriptors
> in Documentation/ABI, with note that it was added only in a later
> kernel version
>
> v3: Changed from simply allowing user to set the value for bFrameIndex to
> automatically assigning correct distinct frame indexes. Changed
> bFrameIndex from RW to RO
>
> v4: Actually include updated patch
>
> v5: bFrameIndex now returns -EBUSY if the parent fmt is not linked yet
>
> Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>

no Ack from Laurent yet? hmm, I'll have to delay this one for the next
merge window, sorry. Just as a side note, version changes (v2, v3, v4,
v5 above) should comer after the tearline (---) below. We don't need
these changes in the commit log, it's just for maintainers to read :-)

If we had an ack from Laurent, I'd fix that while applying, but since
you're gonna have to resend anyway, please fix that once we get an Ack
from Laurent.