All of lore.kernel.org
 help / color / mirror / Atom feed
* [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-21 13:53 Joel Pepper
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-03-21 13:53 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Felipe Balbi, Greg Kroah-Hartman, Paul Elder, linux-usb

Please disregard this version, I discovered a different bug which needs
to be fixed before the -EBUSY behaviour can implemented, both will come
in v5


On 21.03.2018 11:20, Joel Pepper wrote:
> - 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
>
> 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        | 53 +++++++++++++++++++++++
>  2 files changed, 71 insertions(+)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> index 1ba0d0f..872ed8a 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..a73dbc5 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;
> @@ -929,6 +932,30 @@ static struct uvcg_frame *to_uvcg_frame(struct config_item *item)
>  	return container_of(item, struct uvcg_frame, item);
>  }
>  
> +#define UVCG_FRAME_ATTR_RO(cname, aname, to_cpu_endian, to_little_endian, bits) \
> +static ssize_t uvcg_frame_##cname##_show(struct config_item *item, char *page)\
> +{									\
> +	struct uvcg_frame *f = to_uvcg_frame(item);			\
> +	struct f_uvc_opts *opts;					\
> +	struct config_item *opts_item;					\
> +	struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\
> +	int result;							\
> +									\
> +	mutex_lock(su_mutex); /* for navigating configfs hierarchy */	\
> +									\
> +	opts_item = f->item.ci_parent->ci_parent->ci_parent->ci_parent;	\
> +	opts = to_f_uvc_opts(opts_item);				\
> +									\
> +	mutex_lock(&opts->lock);					\
> +	result = sprintf(page, "%d\n", to_cpu_endian(f->frame.cname));	\
> +	mutex_unlock(&opts->lock);					\
> +									\
> +	mutex_unlock(su_mutex);						\
> +	return result;							\
> +}									\
> +									\
> +UVC_ATTR_RO(uvcg_frame_, cname, aname);
> +
>  #define UVCG_FRAME_ATTR(cname, aname, to_cpu_endian, to_little_endian, bits) \
>  static ssize_t uvcg_frame_##cname##_show(struct config_item *item, char *page)\
>  {									\
> @@ -990,6 +1017,9 @@ UVC_ATTR(uvcg_frame_, cname, aname);
>  
>  #define noop_conversion(x) (x)
>  
> +UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
> +		noop_conversion, 8);
> +
>  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 +1167,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 +1245,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;
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-05-29 19:02 Joel Pepper
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-05-29 19:02 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Felipe Balbi, linux-usb, Paul Elder, Joel Pepper

This patch set is mostly concerned with fixing a bug with the driver
setting incorrect bFrameIndexes, for details refer to the second patch.
However part of the fix also depends on fixing an unrelated bug,
which involved the "linked" flag on formats not beig set.

v6: This is mostly a resubmit of v5 which got buried ahead of the last merge window and never got reviewed. The addition to the Documentation has been updated to reflect this. Also moved version comments out of the actual commit message (Thanks, Felipe.)

(Technically, this is the second resend, because the first resend got buried too.)

Joel Pepper (2):
  usb/gadget/uvc-configfs Fix linked flag in a format not being set when
    format is linked into streaming header
  usb/gadget/uvc-configfs Fix host unable to negotiate framesizes other
    than first

 Documentation/ABI/testing/configfs-usb-gadget-uvc | 18 ++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 67 +++++++++++++++++++++++
 2 files changed, 85 insertions(+)

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

* [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-04-26 10:17 Joel Pepper
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-04-26 10:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, Paul Elder, Joel Pepper

This patch set is mostly concerned with fixing a bug with the driver
setting incorrect bFrameIndexes, for details refer to the second patch.
However part of the fix also depends on fixing an unrelated bug,
which involved the "linked" flag on formats not beig set.

v6: This is mostly a resubmit of v5 which got buried ahead of the last merge window and never got reviewed. The addition to the Documentation has been updated to reflect this. Also moved version comments out of the actual commit message (Thanks, Felipe.)


Joel Pepper (2):
  usb/gadget/uvc-configfs Fix linked flag in a format not being set when
    format is linked into streaming header
  usb/gadget/uvc-configfs Fix host unable to negotiate framesizes other
    than first

 Documentation/ABI/testing/configfs-usb-gadget-uvc | 18 ++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 67 +++++++++++++++++++++++
 2 files changed, 85 insertions(+)

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

* [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-21 14:25 Joel Pepper
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-03-21 14:25 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Felipe Balbi, Greg Kroah-Hartman, linux-usb, Paul Elder, Joel Pepper

This patchset is mostly concerned with fixing the driver setting incorrect
bFrameIndexes which breaks frame descriptor negotiation, for details refer
to the first patch.

However this fix needed another bug relating to the "linked" flag not
being set on formats to be fixed first. As this bug has farther reaching
implications, I have included the relevant fix as an independent patch


Joel Pepper (2):
  usb/gadget/uvc-configs Fix host unable to negotiate framesizes other
    than first
  usb/gadget/uvc-configfs Fix linked flag in a format not being set when
    format is linked into streaming header

 Documentation/ABI/testing/configfs-usb-gadget-uvc | 18 ++++++
 drivers/usb/gadget/function/uvc_configfs.c        | 67 +++++++++++++++++++++++
 2 files changed, 85 insertions(+)

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

* [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first
@ 2018-03-21 10:20 Joel Pepper
  0 siblings, 0 replies; 5+ messages in thread
From: Joel Pepper @ 2018-03-21 10:20 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Felipe Balbi, Greg Kroah-Hartman, Paul Elder, linux-usb, 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

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        | 53 +++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 1ba0d0f..872ed8a 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..a73dbc5 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;
@@ -929,6 +932,30 @@ static struct uvcg_frame *to_uvcg_frame(struct config_item *item)
 	return container_of(item, struct uvcg_frame, item);
 }
 
+#define UVCG_FRAME_ATTR_RO(cname, aname, to_cpu_endian, to_little_endian, bits) \
+static ssize_t uvcg_frame_##cname##_show(struct config_item *item, char *page)\
+{									\
+	struct uvcg_frame *f = to_uvcg_frame(item);			\
+	struct f_uvc_opts *opts;					\
+	struct config_item *opts_item;					\
+	struct mutex *su_mutex = &f->item.ci_group->cg_subsys->su_mutex;\
+	int result;							\
+									\
+	mutex_lock(su_mutex); /* for navigating configfs hierarchy */	\
+									\
+	opts_item = f->item.ci_parent->ci_parent->ci_parent->ci_parent;	\
+	opts = to_f_uvc_opts(opts_item);				\
+									\
+	mutex_lock(&opts->lock);					\
+	result = sprintf(page, "%d\n", to_cpu_endian(f->frame.cname));	\
+	mutex_unlock(&opts->lock);					\
+									\
+	mutex_unlock(su_mutex);						\
+	return result;							\
+}									\
+									\
+UVC_ATTR_RO(uvcg_frame_, cname, aname);
+
 #define UVCG_FRAME_ATTR(cname, aname, to_cpu_endian, to_little_endian, bits) \
 static ssize_t uvcg_frame_##cname##_show(struct config_item *item, char *page)\
 {									\
@@ -990,6 +1017,9 @@ UVC_ATTR(uvcg_frame_, cname, aname);
 
 #define noop_conversion(x) (x)
 
+UVCG_FRAME_ATTR_RO(b_frame_index, bFrameIndex, noop_conversion,
+		noop_conversion, 8);
+
 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 +1167,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 +1245,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;

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

end of thread, other threads:[~2018-05-29 19:02 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-21 13:53 [v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first Joel Pepper
  -- strict thread matches above, loose matches on Subject: below --
2018-05-29 19:02 Joel Pepper
2018-04-26 10:17 Joel Pepper
2018-03-21 14:25 Joel Pepper
2018-03-21 10:20 Joel Pepper

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.