Re: [PATCH v4] usb/gadget/uvc-configs Fix host unable to negotiate framesizes other than first

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux