diff mbox series

[v2,3/7] usb: gadget: uvc: Add struct for color matching in configs

Message ID 20221219144316.757680-4-dan.scally@ideasonboard.com (mailing list archive)
State Superseded
Headers show
Series UVC Gadget: Extend color matching support | expand

Commit Message

Dan Scally Dec. 19, 2022, 2:43 p.m. UTC 
Color matching descriptors are meant to be a per-format piece of data
and we need to be able to support different descriptors for different
formats. As a preliminary step towards that goal, switch the default
color matching configfs functionality to point to an instance of a
new struct uvcg_color_matching. Use the same default values for its
attributes as the currently hard-coded ones so that the interface to
userspace is consistent.

Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:

	- Renamed uvcg_cmd to uvcg_color_matching plus the associated
	  variables (Kieran, Laurent)
	- Added a refcnt member to struct uvcg_color_matching

 drivers/usb/gadget/function/uvc_configfs.c | 58 ++++++++++++++++------
 drivers/usb/gadget/function/uvc_configfs.h |  9 ++++
 2 files changed, 53 insertions(+), 14 deletions(-)

Comments

Laurent Pinchart Dec. 28, 2022, 2:19 a.m. UTC | #1 
Hi Dan,

Thank you for the patch.

On Mon, Dec 19, 2022 at 02:43:12PM +0000, Daniel Scally wrote:
> Color matching descriptors are meant to be a per-format piece of data
> and we need to be able to support different descriptors for different
> formats. As a preliminary step towards that goal, switch the default
> color matching configfs functionality to point to an instance of a
> new struct uvcg_color_matching. Use the same default values for its
> attributes as the currently hard-coded ones so that the interface to
> userspace is consistent.
> 
> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> ---
> Changes in v2:
> 
> 	- Renamed uvcg_cmd to uvcg_color_matching plus the associated
> 	  variables (Kieran, Laurent)
> 	- Added a refcnt member to struct uvcg_color_matching
> 
>  drivers/usb/gadget/function/uvc_configfs.c | 58 ++++++++++++++++------
>  drivers/usb/gadget/function/uvc_configfs.h |  9 ++++
>  2 files changed, 53 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index e28becd435bf..147d3def24dd 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -13,6 +13,7 @@
>  #include "uvc_configfs.h"
>  
>  #include <linux/sort.h>
> +#include <uapi/linux/usb/video.h>

Drop "uapi/", the kernel should handle that automatically.

>  
>  /* -----------------------------------------------------------------------------
>   * Global Utility Structures and Macros
> @@ -1788,20 +1789,21 @@ static ssize_t uvcg_color_matching_##cname##_show(			\
>  	struct config_item *item, char *page)				\
>  {									\
>  	struct config_group *group = to_config_group(item);		\
> +	struct uvcg_color_matching *color_match =			\
> +		to_uvcg_color_matching(group);				\
>  	struct f_uvc_opts *opts;					\
>  	struct config_item *opts_item;					\
>  	struct mutex *su_mutex = &group->cg_subsys->su_mutex;		\
> -	struct uvc_color_matching_descriptor *cd;			\
>  	int result;							\
>  									\
>  	mutex_lock(su_mutex); /* for navigating configfs hierarchy */	\
>  									\
>  	opts_item = group->cg_item.ci_parent->ci_parent->ci_parent;	\
>  	opts = to_f_uvc_opts(opts_item);				\
> -	cd = &opts->uvc_color_matching;					\
>  									\
>  	mutex_lock(&opts->lock);					\
> -	result = sprintf(page, "%u\n", le##bits##_to_cpu(cd->aname));	\
> +	result = sprintf(page, "%u\n",					\
> +			 le##bits##_to_cpu(color_match->desc.aname));	\
>  	mutex_unlock(&opts->lock);					\
>  									\
>  	mutex_unlock(su_mutex);						\
> @@ -1823,29 +1825,57 @@ static struct configfs_attribute *uvcg_color_matching_attrs[] = {
>  	NULL,
>  };
>  
> -static const struct uvcg_config_group_type uvcg_color_matching_type = {
> -	.type = {
> -		.ct_item_ops	= &uvcg_config_item_ops,
> -		.ct_attrs	= uvcg_color_matching_attrs,
> -		.ct_owner	= THIS_MODULE,
> -	},
> -	.name = "default",
> +static void uvcg_color_matching_release(struct config_item *item)
> +{
> +	struct uvcg_color_matching *color_match =
> +		to_uvcg_color_matching(to_config_group(item));
> +
> +	kfree(color_match);
> +}
> +
> +static struct configfs_item_operations uvcg_color_matching_item_ops = {
> +	.release	= uvcg_color_matching_release,
> +};
> +
> +static const struct config_item_type uvcg_color_matching_type = {
> +	.ct_item_ops	= &uvcg_color_matching_item_ops,
> +	.ct_attrs	= uvcg_color_matching_attrs,
> +	.ct_owner	= THIS_MODULE,
>  };
>  
>  /* -----------------------------------------------------------------------------
>   * streaming/color_matching
>   */
>  
> +static int uvcg_color_matching_create_children(struct config_group *parent)
> +{
> +	struct uvcg_color_matching *color_match;
> +
> +	color_match = kzalloc(sizeof(*color_match), GFP_KERNEL);
> +	if (!color_match)
> +		return -ENOMEM;
> +
> +	color_match->desc.bLength = UVC_DT_COLOR_MATCHING_SIZE;
> +	color_match->desc.bDescriptorType = USB_DT_CS_INTERFACE;
> +	color_match->desc.bDescriptorSubType = UVC_VS_COLORFORMAT;
> +	color_match->desc.bColorPrimaries = UVC_COLOR_PRIMARIES_BT_709_SRGB;
> +	color_match->desc.bTransferCharacteristics = UVC_TRANSFER_CHARACTERISTICS_BT_709;
> +	color_match->desc.bMatrixCoefficients = UVC_MATRIX_COEFFICIENTS_SMPTE_170M;
> +
> +	config_group_init_type_name(&color_match->group, "default",
> +				    &uvcg_color_matching_type);
> +	configfs_add_default_group(&color_match->group, parent);
> +
> +	return 0;
> +}
> +
>  static const struct uvcg_config_group_type uvcg_color_matching_grp_type = {
>  	.type = {
>  		.ct_item_ops	= &uvcg_config_item_ops,
>  		.ct_owner	= THIS_MODULE,
>  	},
>  	.name = "color_matching",
> -	.children = (const struct uvcg_config_group_type*[]) {
> -		&uvcg_color_matching_type,
> -		NULL,
> -	},
> +	.create_children = uvcg_color_matching_create_children,
>  };
>  
>  /* -----------------------------------------------------------------------------
> diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
> index ad2ec8c4c78c..c7392c9b840e 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.h
> +++ b/drivers/usb/gadget/function/uvc_configfs.h
> @@ -37,6 +37,15 @@ static inline struct uvcg_control_header *to_uvcg_control_header(struct config_i
>  	return container_of(item, struct uvcg_control_header, item);
>  }
>  
> +struct uvcg_color_matching {
> +	struct config_group group;
> +	struct uvc_color_matching_descriptor desc;
> +	unsigned int refcnt;

As refcnt isn't used in this patch, I would add it to the structure in
the patch that uses it.

Conditionally-Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

with these small issues fixed.

> +};
> +
> +#define to_uvcg_color_matching(group_ptr) \
> +container_of(group_ptr, struct uvcg_color_matching, group)
> +
>  enum uvcg_format_type {
>  	UVCG_UNCOMPRESSED = 0,
>  	UVCG_MJPEG,
diff mbox series

Patch

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index e28becd435bf..147d3def24dd 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -13,6 +13,7 @@ 
 #include "uvc_configfs.h"
 
 #include <linux/sort.h>
+#include <uapi/linux/usb/video.h>
 
 /* -----------------------------------------------------------------------------
  * Global Utility Structures and Macros
@@ -1788,20 +1789,21 @@  static ssize_t uvcg_color_matching_##cname##_show(			\
 	struct config_item *item, char *page)				\
 {									\
 	struct config_group *group = to_config_group(item);		\
+	struct uvcg_color_matching *color_match =			\
+		to_uvcg_color_matching(group);				\
 	struct f_uvc_opts *opts;					\
 	struct config_item *opts_item;					\
 	struct mutex *su_mutex = &group->cg_subsys->su_mutex;		\
-	struct uvc_color_matching_descriptor *cd;			\
 	int result;							\
 									\
 	mutex_lock(su_mutex); /* for navigating configfs hierarchy */	\
 									\
 	opts_item = group->cg_item.ci_parent->ci_parent->ci_parent;	\
 	opts = to_f_uvc_opts(opts_item);				\
-	cd = &opts->uvc_color_matching;					\
 									\
 	mutex_lock(&opts->lock);					\
-	result = sprintf(page, "%u\n", le##bits##_to_cpu(cd->aname));	\
+	result = sprintf(page, "%u\n",					\
+			 le##bits##_to_cpu(color_match->desc.aname));	\
 	mutex_unlock(&opts->lock);					\
 									\
 	mutex_unlock(su_mutex);						\
@@ -1823,29 +1825,57 @@  static struct configfs_attribute *uvcg_color_matching_attrs[] = {
 	NULL,
 };
 
-static const struct uvcg_config_group_type uvcg_color_matching_type = {
-	.type = {
-		.ct_item_ops	= &uvcg_config_item_ops,
-		.ct_attrs	= uvcg_color_matching_attrs,
-		.ct_owner	= THIS_MODULE,
-	},
-	.name = "default",
+static void uvcg_color_matching_release(struct config_item *item)
+{
+	struct uvcg_color_matching *color_match =
+		to_uvcg_color_matching(to_config_group(item));
+
+	kfree(color_match);
+}
+
+static struct configfs_item_operations uvcg_color_matching_item_ops = {
+	.release	= uvcg_color_matching_release,
+};
+
+static const struct config_item_type uvcg_color_matching_type = {
+	.ct_item_ops	= &uvcg_color_matching_item_ops,
+	.ct_attrs	= uvcg_color_matching_attrs,
+	.ct_owner	= THIS_MODULE,
 };
 
 /* -----------------------------------------------------------------------------
  * streaming/color_matching
  */
 
+static int uvcg_color_matching_create_children(struct config_group *parent)
+{
+	struct uvcg_color_matching *color_match;
+
+	color_match = kzalloc(sizeof(*color_match), GFP_KERNEL);
+	if (!color_match)
+		return -ENOMEM;
+
+	color_match->desc.bLength = UVC_DT_COLOR_MATCHING_SIZE;
+	color_match->desc.bDescriptorType = USB_DT_CS_INTERFACE;
+	color_match->desc.bDescriptorSubType = UVC_VS_COLORFORMAT;
+	color_match->desc.bColorPrimaries = UVC_COLOR_PRIMARIES_BT_709_SRGB;
+	color_match->desc.bTransferCharacteristics = UVC_TRANSFER_CHARACTERISTICS_BT_709;
+	color_match->desc.bMatrixCoefficients = UVC_MATRIX_COEFFICIENTS_SMPTE_170M;
+
+	config_group_init_type_name(&color_match->group, "default",
+				    &uvcg_color_matching_type);
+	configfs_add_default_group(&color_match->group, parent);
+
+	return 0;
+}
+
 static const struct uvcg_config_group_type uvcg_color_matching_grp_type = {
 	.type = {
 		.ct_item_ops	= &uvcg_config_item_ops,
 		.ct_owner	= THIS_MODULE,
 	},
 	.name = "color_matching",
-	.children = (const struct uvcg_config_group_type*[]) {
-		&uvcg_color_matching_type,
-		NULL,
-	},
+	.create_children = uvcg_color_matching_create_children,
 };
 
 /* -----------------------------------------------------------------------------
diff --git a/drivers/usb/gadget/function/uvc_configfs.h b/drivers/usb/gadget/function/uvc_configfs.h
index ad2ec8c4c78c..c7392c9b840e 100644
--- a/drivers/usb/gadget/function/uvc_configfs.h
+++ b/drivers/usb/gadget/function/uvc_configfs.h
@@ -37,6 +37,15 @@  static inline struct uvcg_control_header *to_uvcg_control_header(struct config_i
 	return container_of(item, struct uvcg_control_header, item);
 }
 
+struct uvcg_color_matching {
+	struct config_group group;
+	struct uvc_color_matching_descriptor desc;
+	unsigned int refcnt;
+};
+
+#define to_uvcg_color_matching(group_ptr) \
+container_of(group_ptr, struct uvcg_color_matching, group)
+
 enum uvcg_format_type {
 	UVCG_UNCOMPRESSED = 0,
 	UVCG_MJPEG,