diff mbox series

[1/5] media: uvcvideo: Allow custom control mapping

Message ID 20240318-billion-v1-1-2f7bc0ee2030@chromium.org
State New
Headers show
Series [1/5] media: uvcvideo: Allow custom control mapping | expand

Commit Message

Ricardo Ribalda March 18, 2024, 11:55 p.m. UTC
Some advanced controls might not be completely implemented by vendors.

If the controls are a enumeration, UVC does not gives a way to probe
what is implemented and what is not.

Lets create a new callback function where heuristics can be implemented
to detect what is implemented and what not.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 15 ++++++++--
 drivers/media/usb/uvc/uvcvideo.h | 59 +++++++++++++++++++++-------------------
 2 files changed, 43 insertions(+), 31 deletions(-)

Comments

Laurent Pinchart June 10, 2024, 3:12 p.m. UTC | #1
Hi Ricardo,

Thank you for the patch.

On Mon, Mar 18, 2024 at 11:55:23PM +0000, Ricardo Ribalda wrote:
> Some advanced controls might not be completely implemented by vendors.
> 
> If the controls are a enumeration, UVC does not gives a way to probe
> what is implemented and what is not.
> 
> Lets create a new callback function where heuristics can be implemented

s/Lets/Let's/

> to detect what is implemented and what not.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 15 ++++++++--
>  drivers/media/usb/uvc/uvcvideo.h | 59 +++++++++++++++++++++-------------------
>  2 files changed, 43 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index e59a463c27618..3e939b4fbaaaf 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -2434,6 +2434,14 @@ static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	return -ENOMEM;
>  }
>  
> +static int __uvc_ctrl_add_custom_mapping(struct uvc_video_chain *chain,
> +	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)

The name isn't great is in most cases it doesn't add a custom mapping.

> +{
> +	if (mapping && mapping->add_mapping)
> +		return mapping->add_mapping(chain, ctrl, mapping);
> +	return __uvc_ctrl_add_mapping(chain, ctrl, mapping);
> +}
> +
>  int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
>  	const struct uvc_control_mapping *mapping)
>  {
> @@ -2637,7 +2645,8 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  
>  			if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
>  			    ctrl->info.selector == mapping->selector) {
> -				__uvc_ctrl_add_mapping(chain, ctrl, mapping);
> +				__uvc_ctrl_add_custom_mapping(chain, ctrl,
> +							      mapping);
>  				custom = true;
>  			}
>  		}
> @@ -2652,7 +2661,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  
>  		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
>  		    ctrl->info.selector == mapping->selector)
> -			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
> +			__uvc_ctrl_add_custom_mapping(chain, ctrl, mapping);
>  	}
>  
>  	/* Finally process version-specific mappings. */
> @@ -2664,7 +2673,7 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  
>  		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
>  		    ctrl->info.selector == mapping->selector)
> -			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
> +			__uvc_ctrl_add_custom_mapping(chain, ctrl, mapping);
>  	}
>  }
>  
> diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
> index 6fb0a78b1b009..611350a82c37f 100644
> --- a/drivers/media/usb/uvc/uvcvideo.h
> +++ b/drivers/media/usb/uvc/uvcvideo.h
> @@ -101,34 +101,6 @@ struct uvc_control_info {
>  	u32 flags;
>  };
>  
> -struct uvc_control_mapping {
> -	struct list_head list;
> -	struct list_head ev_subs;
> -
> -	u32 id;
> -	char *name;
> -	u8 entity[16];
> -	u8 selector;
> -
> -	u8 size;
> -	u8 offset;
> -	enum v4l2_ctrl_type v4l2_type;
> -	u32 data_type;
> -
> -	const u32 *menu_mapping;
> -	const char (*menu_names)[UVC_MENU_NAME_LEN];
> -	unsigned long menu_mask;
> -
> -	u32 master_id;
> -	s32 master_manual;
> -	u32 slave_ids[2];
> -
> -	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
> -		   const u8 *data);
> -	void (*set)(struct uvc_control_mapping *mapping, s32 value,
> -		    u8 *data);
> -};
> -

You can leave the structure here, close to the other related structuers,
and add forward declarations of the uvc_video_chain and uvc_control
structures just before the existing forward declaration of uvc_device.

>  struct uvc_control {
>  	struct uvc_entity *entity;
>  	struct uvc_control_info info;
> @@ -336,6 +308,37 @@ struct uvc_video_chain {
>  	u8 ctrl_class_bitmap;			/* Bitmap of valid classes */
>  };
>  
> +struct uvc_control_mapping {
> +	struct list_head list;
> +	struct list_head ev_subs;
> +
> +	u32 id;
> +	char *name;
> +	u8 entity[16];
> +	u8 selector;
> +
> +	u8 size;
> +	u8 offset;
> +	enum v4l2_ctrl_type v4l2_type;
> +	u32 data_type;
> +
> +	const u32 *menu_mapping;
> +	const char (*menu_names)[UVC_MENU_NAME_LEN];
> +	unsigned long menu_mask;
> +
> +	u32 master_id;
> +	s32 master_manual;
> +	u32 slave_ids[2];
> +
> +	int (*add_mapping)(struct uvc_video_chain *chain,
> +			   struct uvc_control *ctrl,
> +			   const struct uvc_control_mapping *mapping);
> +	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
> +		   const u8 *data);
> +	void (*set)(struct uvc_control_mapping *mapping, s32 value,
> +		    u8 *data);
> +};
> +
>  struct uvc_stats_frame {
>  	unsigned int size;		/* Number of bytes captured */
>  	unsigned int first_data;	/* Index of the first non-empty packet */
>
diff mbox series

Patch

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index e59a463c27618..3e939b4fbaaaf 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -2434,6 +2434,14 @@  static int __uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	return -ENOMEM;
 }
 
+static int __uvc_ctrl_add_custom_mapping(struct uvc_video_chain *chain,
+	struct uvc_control *ctrl, const struct uvc_control_mapping *mapping)
+{
+	if (mapping && mapping->add_mapping)
+		return mapping->add_mapping(chain, ctrl, mapping);
+	return __uvc_ctrl_add_mapping(chain, ctrl, mapping);
+}
+
 int uvc_ctrl_add_mapping(struct uvc_video_chain *chain,
 	const struct uvc_control_mapping *mapping)
 {
@@ -2637,7 +2645,8 @@  static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 
 			if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
 			    ctrl->info.selector == mapping->selector) {
-				__uvc_ctrl_add_mapping(chain, ctrl, mapping);
+				__uvc_ctrl_add_custom_mapping(chain, ctrl,
+							      mapping);
 				custom = true;
 			}
 		}
@@ -2652,7 +2661,7 @@  static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 
 		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
 		    ctrl->info.selector == mapping->selector)
-			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
+			__uvc_ctrl_add_custom_mapping(chain, ctrl, mapping);
 	}
 
 	/* Finally process version-specific mappings. */
@@ -2664,7 +2673,7 @@  static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 
 		if (uvc_entity_match_guid(ctrl->entity, mapping->entity) &&
 		    ctrl->info.selector == mapping->selector)
-			__uvc_ctrl_add_mapping(chain, ctrl, mapping);
+			__uvc_ctrl_add_custom_mapping(chain, ctrl, mapping);
 	}
 }
 
diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
index 6fb0a78b1b009..611350a82c37f 100644
--- a/drivers/media/usb/uvc/uvcvideo.h
+++ b/drivers/media/usb/uvc/uvcvideo.h
@@ -101,34 +101,6 @@  struct uvc_control_info {
 	u32 flags;
 };
 
-struct uvc_control_mapping {
-	struct list_head list;
-	struct list_head ev_subs;
-
-	u32 id;
-	char *name;
-	u8 entity[16];
-	u8 selector;
-
-	u8 size;
-	u8 offset;
-	enum v4l2_ctrl_type v4l2_type;
-	u32 data_type;
-
-	const u32 *menu_mapping;
-	const char (*menu_names)[UVC_MENU_NAME_LEN];
-	unsigned long menu_mask;
-
-	u32 master_id;
-	s32 master_manual;
-	u32 slave_ids[2];
-
-	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
-		   const u8 *data);
-	void (*set)(struct uvc_control_mapping *mapping, s32 value,
-		    u8 *data);
-};
-
 struct uvc_control {
 	struct uvc_entity *entity;
 	struct uvc_control_info info;
@@ -336,6 +308,37 @@  struct uvc_video_chain {
 	u8 ctrl_class_bitmap;			/* Bitmap of valid classes */
 };
 
+struct uvc_control_mapping {
+	struct list_head list;
+	struct list_head ev_subs;
+
+	u32 id;
+	char *name;
+	u8 entity[16];
+	u8 selector;
+
+	u8 size;
+	u8 offset;
+	enum v4l2_ctrl_type v4l2_type;
+	u32 data_type;
+
+	const u32 *menu_mapping;
+	const char (*menu_names)[UVC_MENU_NAME_LEN];
+	unsigned long menu_mask;
+
+	u32 master_id;
+	s32 master_manual;
+	u32 slave_ids[2];
+
+	int (*add_mapping)(struct uvc_video_chain *chain,
+			   struct uvc_control *ctrl,
+			   const struct uvc_control_mapping *mapping);
+	s32 (*get)(struct uvc_control_mapping *mapping, u8 query,
+		   const u8 *data);
+	void (*set)(struct uvc_control_mapping *mapping, s32 value,
+		    u8 *data);
+};
+
 struct uvc_stats_frame {
 	unsigned int size;		/* Number of bytes captured */
 	unsigned int first_data;	/* Index of the first non-empty packet */