[PATCH 0/4] media: uvcvideo: Add support for UVC_CROSXU_CONTROL_IQ_PROFILE

[PATCH 0/4] media: uvcvideo: Add support for UVC_CROSXU_CONTROL_IQ_PROFILE

[Ricardo Ribalda]

Add support for switching the IQ profile of a ChromeOS camera.

This patchset depends on 2 patches in uvc/for-next:
- media: uvcvideo: Fix comments in uvc_meta_detect_msxu
- media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
Laurent Pinchart (1):
      media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

Ricardo Ribalda (3):
      media: uvcvideo: Fix comments in uvc_meta_detect_msxu
      media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls
      media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

 drivers/media/usb/uvc/uvc_ctrl.c     | 41 ++++++++++++++++++++++++++++--------
 drivers/media/usb/uvc/uvc_metadata.c | 22 +++++++++++--------
 include/linux/usb/uvc.h              | 22 +++++++++++++++++++
 3 files changed, 67 insertions(+), 18 deletions(-)
---
base-commit: 078f1a7eb48eef9b3cb78bcd2254356f3a332358
change-id: 20250813-uvc-iq-switch-37f461657f95

Best regards,
-- 
Ricardo Ribalda <ribalda@chromium.org>

[PATCH 1/4] media: uvcvideo: Fix comments in uvc_meta_detect_msxu

[Ricardo Ribalda]

The comments can be more precise. Let's fix them.

Fixes: 6cb786f040ad ("media: uvcvideo: Auto-set UVC_QUIRK_MSXU_META")
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
Link: https://lore.kernel.org/r/20250716-uvc-meta-followup-v2-1-d3c2b995af3d@chromium.org
Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/media/usb/uvc/uvc_metadata.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index 3b0b392895119bb6ac300fc40b89a7ea00e56b40..b0960f0553cfcb30b0036d2ad8877fafa225c6a4 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -196,7 +196,10 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 	if (!data)
 		return -ENOMEM;
 
-	/* Check if the metadata is already enabled. */
+	/*
+	 * Check if the metadata is already enabled, or if the device always
+	 * returns metadata.
+	 */
 	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
 			     MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (ret)
@@ -208,9 +211,11 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 	}
 
 	/*
-	 * We have seen devices that require 1 to enable the metadata, others
-	 * requiring a value != 1 and others requiring a value >1. Luckily for
-	 * us, the value from GET_MAX seems to work all the time.
+	 * Set the value of MSXU_CONTROL_METADATA to the value reported by
+	 * GET_MAX to enable production of MSXU metadata. The GET_MAX request
+	 * reports the maximum size of the metadata, if its value is 0 then MSXU
+	 * metadata is not supported. For more information, see
+	 * https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#2229-metadata-control
 	 */
 	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
 			     MSXU_CONTROL_METADATA, data, sizeof(*data));

-- 
2.51.0.rc1.167.g924127e9c0-goog

[PATCH 2/4] media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header

[Ricardo Ribalda]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Move the MSXU_CONTROL_METADATA control definitino to the
include/linux/usb/uvc.h header, alongside the corresponding XU GUID. Add
a UVC_ prefix to avoid namespace clashes.

While at it, add the definition for the other controls for that
extension unit, as defined in
https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Link: https://lore.kernel.org/r/20250715185254.6592-4-laurent.pinchart@ideasonboard.com
Signed-off-by: Hans de Goede <hansg@kernel.org>
---
 drivers/media/usb/uvc/uvc_metadata.c | 11 +++++------
 include/linux/usb/uvc.h              | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_metadata.c b/drivers/media/usb/uvc/uvc_metadata.c
index b0960f0553cfcb30b0036d2ad8877fafa225c6a4..2d4998bd098f3ac7f51421535ccaa9e20a71f8e9 100644
--- a/drivers/media/usb/uvc/uvc_metadata.c
+++ b/drivers/media/usb/uvc/uvc_metadata.c
@@ -177,7 +177,6 @@ static struct uvc_entity *uvc_meta_find_msxu(struct uvc_device *dev)
 	return NULL;
 }
 
-#define MSXU_CONTROL_METADATA 0x9
 static int uvc_meta_detect_msxu(struct uvc_device *dev)
 {
 	u32 *data __free(kfree) = NULL;
@@ -201,7 +200,7 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 	 * returns metadata.
 	 */
 	ret = uvc_query_ctrl(dev, UVC_GET_CUR, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (ret)
 		return 0;
 
@@ -211,23 +210,23 @@ static int uvc_meta_detect_msxu(struct uvc_device *dev)
 	}
 
 	/*
-	 * Set the value of MSXU_CONTROL_METADATA to the value reported by
+	 * Set the value of UVC_MSXU_CONTROL_METADATA to the value reported by
 	 * GET_MAX to enable production of MSXU metadata. The GET_MAX request
 	 * reports the maximum size of the metadata, if its value is 0 then MSXU
 	 * metadata is not supported. For more information, see
 	 * https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#2229-metadata-control
 	 */
 	ret = uvc_query_ctrl(dev, UVC_GET_MAX, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (ret || !*data)
 		return 0;
 
 	/*
-	 * If we can set MSXU_CONTROL_METADATA, the device will report
+	 * If we can set UVC_MSXU_CONTROL_METADATA, the device will report
 	 * metadata.
 	 */
 	ret = uvc_query_ctrl(dev, UVC_SET_CUR, entity->id, dev->intfnum,
-			     MSXU_CONTROL_METADATA, data, sizeof(*data));
+			     UVC_MSXU_CONTROL_METADATA, data, sizeof(*data));
 	if (!ret)
 		dev->quirks |= UVC_QUIRK_MSXU_META;
 
diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
index ee19e9f915b8370c333c426dc1ee4202c7b75c5b..12a57e1d34674a3a264ed7f88bed43926661fcd4 100644
--- a/include/linux/usb/uvc.h
+++ b/include/linux/usb/uvc.h
@@ -33,6 +33,23 @@
 	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
 	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
 
+/* https://learn.microsoft.com/en-us/windows-hardware/drivers/stream/uvc-extensions-1-5#222-extension-unit-controls */
+#define UVC_MSXU_CONTROL_FOCUS			0x01
+#define UVC_MSXU_CONTROL_EXPOSURE		0x02
+#define UVC_MSXU_CONTROL_EVCOMPENSATION		0x03
+#define UVC_MSXU_CONTROL_WHITEBALANCE		0x04
+#define UVC_MSXU_CONTROL_FACE_AUTHENTICATION	0x06
+#define UVC_MSXU_CONTROL_CAMERA_EXTRINSICS	0x07
+#define UVC_MSXU_CONTROL_CAMERA_INTRINSICS	0x08
+#define UVC_MSXU_CONTROL_METADATA		0x09
+#define UVC_MSXU_CONTROL_IR_TORCH		0x0a
+#define UVC_MSXU_CONTROL_DIGITALWINDOW		0x0b
+#define UVC_MSXU_CONTROL_DIGITALWINDOW_CONFIG	0x0c
+#define UVC_MSXU_CONTROL_VIDEO_HDR		0x0d
+#define UVC_MSXU_CONTROL_FRAMERATE_THROTTLE	0x0e
+#define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
+#define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
+
 #define UVC_GUID_FORMAT_MJPEG \
 	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
 	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}

-- 
2.51.0.rc1.167.g924127e9c0-goog

[PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls

[Ricardo Ribalda]

The function uvc_ctrl_init_ctrl() is called for every control for every
entity, but it exits early if the entity is a extension unit. The comment
claims that this is done to avoid querying XU controls during probe.

We only query a control if its entity GUIDs and index matches the
uvc_ctrls list. There are only controls for the following GUIDs:
UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
UVC_GUID_EXT_GPIO_CONTROLLER.

In other words, XU controls will not be queried even without this
condition.

In future patches we want to add ChromeOS XU controls that need to the
initialized. We will make sure that all cameras with ChromeOS XU can
be queried at probe time.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
 {
 	unsigned int i;
 
-	/*
-	 * XU controls initialization requires querying the device for control
-	 * information. As some buggy UVC devices will crash when queried
-	 * repeatedly in a tight loop, delay XU controls initialization until
-	 * first use.
-	 */
-	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
-		return;
-
 	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
 		const struct uvc_control_info *info = &uvc_ctrls[i];
 

-- 
2.51.0.rc1.167.g924127e9c0-goog

[PATCH 4/4] media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

[Ricardo Ribalda]

The ChromeOS XU provides a control to change the IQ profile for a camera.
It can be switched from VIVID (a.k.a. standard) to NONE (a.k.a. natural).

Wire it up to the standard v4l2 control.

Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
---
 drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++++++++++++
 include/linux/usb/uvc.h          |  5 +++++
 2 files changed, 37 insertions(+)

diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
index ff975f96e1325532e2299047c07de5d1b9cf09db..8766a441ad1d8554c0daaed3f87758321684246b 100644
--- a/drivers/media/usb/uvc/uvc_ctrl.c
+++ b/drivers/media/usb/uvc/uvc_ctrl.c
@@ -376,6 +376,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
 				| UVC_CTRL_FLAG_GET_DEF
 				| UVC_CTRL_FLAG_AUTO_UPDATE,
 	},
+	{
+		.entity		= UVC_GUID_CHROMEOS_XU,
+		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
+		.index		= 3,
+		.size		= 1,
+		.flags		= UVC_CTRL_FLAG_SET_CUR
+				| UVC_CTRL_FLAG_GET_RANGE
+				| UVC_CTRL_FLAG_RESTORE,
+	},
 };
 
 static const u32 uvc_control_classes[] = {
@@ -384,6 +393,17 @@ static const u32 uvc_control_classes[] = {
 };
 
 static const int exposure_auto_mapping[] = { 2, 1, 4, 8 };
+static const int cros_colorfx_mapping[] = { 1, // V4L2_COLORFX_NONE
+					    -1, // V4L2_COLORFX_BW
+					    -1, // V4L2_COLORFX_SEPIA
+					    -1, // V4L2_COLORFX_NEGATIVE
+					    -1, // V4L2_COLORFX_EMBOSS
+					    -1, // V4L2_COLORFX_SKETCH
+					    -1, // V4L2_COLORFX_SKY_BLUE
+					    -1, // V4L2_COLORFX_GRASS_GREEN
+					    -1, // V4L2_COLORFX_SKIN_WHITEN
+					    0}; // V4L2_COLORFX_VIVID};
+
 
 static bool uvc_ctrl_mapping_is_compound(struct uvc_control_mapping *mapping)
 {
@@ -975,6 +995,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
 		.data_type	= UVC_CTRL_DATA_TYPE_BITMASK,
 		.name		= "Region of Interest Auto Ctrls",
 	},
+	{
+		.id		= V4L2_CID_COLORFX,
+		.entity		= UVC_GUID_CHROMEOS_XU,
+		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
+		.size		= 8,
+		.offset		= 0,
+		.v4l2_type	= V4L2_CTRL_TYPE_MENU,
+		.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
+		.menu_mapping	= cros_colorfx_mapping,
+		.menu_mask	= BIT(V4L2_COLORFX_VIVID) |
+				  BIT(V4L2_COLORFX_NONE),
+	},
 };
 
 /* ------------------------------------------------------------------------
diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
index 12a57e1d34674a3a264ed7f88bed43926661fcd4..22e0dab0809e296e089940620ae0e8838e109701 100644
--- a/include/linux/usb/uvc.h
+++ b/include/linux/usb/uvc.h
@@ -29,6 +29,9 @@
 #define UVC_GUID_EXT_GPIO_CONTROLLER \
 	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
 	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
+#define UVC_GUID_CHROMEOS_XU \
+	{0x24, 0xe9, 0xd7, 0x74, 0xc9, 0x49, 0x45, 0x4a, \
+	 0x98, 0xa3, 0xc8, 0x07, 0x7e, 0x05, 0x1c, 0xa3}
 #define UVC_GUID_MSXU_1_5 \
 	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
 	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
@@ -50,6 +53,8 @@
 #define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
 #define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
 
+#define UVC_CROSXU_CONTROL_IQ_PROFILE		0x04
+
 #define UVC_GUID_FORMAT_MJPEG \
 	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
 	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}

-- 
2.51.0.rc1.167.g924127e9c0-goog

Re: [PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls

[Hans de Goede]

Hi,

On 18-Aug-25 22:15, Ricardo Ribalda wrote:
> The function uvc_ctrl_init_ctrl() is called for every control for every
> entity, but it exits early if the entity is a extension unit. The comment
> claims that this is done to avoid querying XU controls during probe.
> 
> We only query a control if its entity GUIDs and index matches the
> uvc_ctrls list. There are only controls for the following GUIDs:
> UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
> UVC_GUID_EXT_GPIO_CONTROLLER.
> 
> In other words, XU controls will not be queried even without this
> condition.
> 
> In future patches we want to add ChromeOS XU controls that need to the
> initialized. We will make sure that all cameras with ChromeOS XU can
> be queried at probe time.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans



> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  {
>  	unsigned int i;
>  
> -	/*
> -	 * XU controls initialization requires querying the device for control
> -	 * information. As some buggy UVC devices will crash when queried
> -	 * repeatedly in a tight loop, delay XU controls initialization until
> -	 * first use.
> -	 */
> -	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> -		return;
> -
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
>  		const struct uvc_control_info *info = &uvc_ctrls[i];
>  
> 

Re: [PATCH 4/4] media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

[Hans de Goede]

Hi,

On 18-Aug-25 22:15, Ricardo Ribalda wrote:
> The ChromeOS XU provides a control to change the IQ profile for a camera.
> It can be switched from VIVID (a.k.a. standard) to NONE (a.k.a. natural).
> 
> Wire it up to the standard v4l2 control.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>

Thanks, patch looks good to me:

Reviewed-by: Hans de Goede <hansg@kernel.org>

Regards,

Hans


> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/usb/uvc.h          |  5 +++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index ff975f96e1325532e2299047c07de5d1b9cf09db..8766a441ad1d8554c0daaed3f87758321684246b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -376,6 +376,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
>  				| UVC_CTRL_FLAG_GET_DEF
>  				| UVC_CTRL_FLAG_AUTO_UPDATE,
>  	},
> +	{
> +		.entity		= UVC_GUID_CHROMEOS_XU,
> +		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
> +		.index		= 3,
> +		.size		= 1,
> +		.flags		= UVC_CTRL_FLAG_SET_CUR
> +				| UVC_CTRL_FLAG_GET_RANGE
> +				| UVC_CTRL_FLAG_RESTORE,
> +	},
>  };
>  
>  static const u32 uvc_control_classes[] = {
> @@ -384,6 +393,17 @@ static const u32 uvc_control_classes[] = {
>  };
>  
>  static const int exposure_auto_mapping[] = { 2, 1, 4, 8 };
> +static const int cros_colorfx_mapping[] = { 1, // V4L2_COLORFX_NONE
> +					    -1, // V4L2_COLORFX_BW
> +					    -1, // V4L2_COLORFX_SEPIA
> +					    -1, // V4L2_COLORFX_NEGATIVE
> +					    -1, // V4L2_COLORFX_EMBOSS
> +					    -1, // V4L2_COLORFX_SKETCH
> +					    -1, // V4L2_COLORFX_SKY_BLUE
> +					    -1, // V4L2_COLORFX_GRASS_GREEN
> +					    -1, // V4L2_COLORFX_SKIN_WHITEN
> +					    0}; // V4L2_COLORFX_VIVID};
> +
>  
>  static bool uvc_ctrl_mapping_is_compound(struct uvc_control_mapping *mapping)
>  {
> @@ -975,6 +995,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>  		.data_type	= UVC_CTRL_DATA_TYPE_BITMASK,
>  		.name		= "Region of Interest Auto Ctrls",
>  	},
> +	{
> +		.id		= V4L2_CID_COLORFX,
> +		.entity		= UVC_GUID_CHROMEOS_XU,
> +		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
> +		.size		= 8,
> +		.offset		= 0,
> +		.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> +		.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> +		.menu_mapping	= cros_colorfx_mapping,
> +		.menu_mask	= BIT(V4L2_COLORFX_VIVID) |
> +				  BIT(V4L2_COLORFX_NONE),
> +	},
>  };
>  
>  /* ------------------------------------------------------------------------
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index 12a57e1d34674a3a264ed7f88bed43926661fcd4..22e0dab0809e296e089940620ae0e8838e109701 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -29,6 +29,9 @@
>  #define UVC_GUID_EXT_GPIO_CONTROLLER \
>  	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
>  	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> +#define UVC_GUID_CHROMEOS_XU \
> +	{0x24, 0xe9, 0xd7, 0x74, 0xc9, 0x49, 0x45, 0x4a, \
> +	 0x98, 0xa3, 0xc8, 0x07, 0x7e, 0x05, 0x1c, 0xa3}
>  #define UVC_GUID_MSXU_1_5 \
>  	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
>  	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> @@ -50,6 +53,8 @@
>  #define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
>  #define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
>  
> +#define UVC_CROSXU_CONTROL_IQ_PROFILE		0x04
> +
>  #define UVC_GUID_FORMAT_MJPEG \
>  	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
>  	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
> 

Re: [PATCH 0/4] media: uvcvideo: Add support for UVC_CROSXU_CONTROL_IQ_PROFILE

[Hans de Goede]

Hi,

On 18-Aug-25 22:15, Ricardo Ribalda wrote:
> Add support for switching the IQ profile of a ChromeOS camera.
> 
> This patchset depends on 2 patches in uvc/for-next:
> - media: uvcvideo: Fix comments in uvc_meta_detect_msxu
> - media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
> Laurent Pinchart (1):
>       media: uvcvideo: Move MSXU_CONTROL_METADATA definition to header
> 
> Ricardo Ribalda (3):
>       media: uvcvideo: Fix comments in uvc_meta_detect_msxu
>       media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls
>       media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE
> 
>  drivers/media/usb/uvc/uvc_ctrl.c     | 41 ++++++++++++++++++++++++++++--------
>  drivers/media/usb/uvc/uvc_metadata.c | 22 +++++++++++--------
>  include/linux/usb/uvc.h              | 22 +++++++++++++++++++
>  3 files changed, 67 insertions(+), 18 deletions(-)
> ---
> base-commit: 078f1a7eb48eef9b3cb78bcd2254356f3a332358
> change-id: 20250813-uvc-iq-switch-37f461657f95

Thank you for your patches.

I have merged patches 3-4 into:

https://gitlab.freedesktop.org/linux-media/users/uvc/-/commits/for-next/

and patches 1-2 are already there.

Regards,

Hans

Re: [PATCH 3/4] media: uvcvideo: Run uvc_ctrl_init_ctrl for all controls

[Laurent Pinchart]

Hi Ricardo,

Thank you for the patch.

On Mon, Aug 18, 2025 at 08:15:38PM +0000, Ricardo Ribalda wrote:
> The function uvc_ctrl_init_ctrl() is called for every control for every
> entity, but it exits early if the entity is a extension unit. The comment
> claims that this is done to avoid querying XU controls during probe.
> 
> We only query a control if its entity GUIDs and index matches the
> uvc_ctrls list. There are only controls for the following GUIDs:
> UVC_GUID_UVC_PROCESSING, UVC_GUID_UVC_CAMERA and
> UVC_GUID_EXT_GPIO_CONTROLLER.
> 
> In other words, XU controls will not be queried even without this
> condition.

Looking at the commit that introduced this code

commit 52c58ad6f95ff60343bf0c517182d5f649ca0403
Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date:   Wed Sep 29 16:03:03 2010 -0300

    [media] uvcvideo: Delay initialization of XU controls

we see it contains the following change

-       /* Query XU controls for control information */
-       if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT) {
-               struct uvc_control_info info;
-               int ret;
-
-               ret = uvc_ctrl_fill_xu_info(dev, ctrl, &info);
-               if (ret < 0)
-                       return;
-
-               ret = uvc_ctrl_add_info(dev, ctrl, &info);
-               if (ret < 0) {
-                       /* Skip the control */
-                       uvc_trace(UVC_TRACE_CONTROL, "Failed to initialize "
-                               "control %pUl/%u on device %s entity %u\n",
-                               info.entity, info.selector, dev->udev->devpath,
-                               ctrl->entity->id);
-                       memset(ctrl, 0, sizeof(*ctrl));
-               }
+       /* XU controls initialization requires querying the device for control
+        * information. As some buggy UVC devices will crash when queried
+        * repeatedly in a tight loop, delay XU controls initialization until
+        * first use.
+        */
+       if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
                return;
-       }


This is followed by

        for (; info < iend; ++info) {
                if (uvc_entity_match_guid(ctrl->entity, info->entity) &&
                    ctrl->index == info->index) {
                        uvc_ctrl_add_info(dev, ctrl, info);
                        break;
                 }
        }

        if (!ctrl->initialized)
                return;

(this loops over uvc_ctrls). As uvc_ctrls doesn't contain any non-XU
control, you're right that the function never calls uvc_ctrl_add_info()
for XU controls. The ctrl->initialized check then causes the function to
return, as uvc_ctrl_add_info() wasn't called and initialized is false.
The entity type check was therefore not needed, and can be removed.

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

> In future patches we want to add ChromeOS XU controls that need to the
> initialized. We will make sure that all cameras with ChromeOS XU can
> be queried at probe time.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 9 ---------
>  1 file changed, 9 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index efe609d7087752cb2ef516eef0fce12acd13e747..ff975f96e1325532e2299047c07de5d1b9cf09db 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -3181,15 +3181,6 @@ static void uvc_ctrl_init_ctrl(struct uvc_video_chain *chain,
>  {
>  	unsigned int i;
>  
> -	/*
> -	 * XU controls initialization requires querying the device for control
> -	 * information. As some buggy UVC devices will crash when queried
> -	 * repeatedly in a tight loop, delay XU controls initialization until
> -	 * first use.
> -	 */
> -	if (UVC_ENTITY_TYPE(ctrl->entity) == UVC_VC_EXTENSION_UNIT)
> -		return;
> -
>  	for (i = 0; i < ARRAY_SIZE(uvc_ctrls); ++i) {
>  		const struct uvc_control_info *info = &uvc_ctrls[i];
>  

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/4] media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

[Laurent Pinchart]

On Mon, Aug 18, 2025 at 08:15:39PM +0000, Ricardo Ribalda wrote:
> The ChromeOS XU provides a control to change the IQ profile for a camera.
> It can be switched from VIVID (a.k.a. standard) to NONE (a.k.a. natural).
>
> Wire it up to the standard v4l2 control.
> 
> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++++++++++++
>  include/linux/usb/uvc.h          |  5 +++++
>  2 files changed, 37 insertions(+)
> 
> diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> index ff975f96e1325532e2299047c07de5d1b9cf09db..8766a441ad1d8554c0daaed3f87758321684246b 100644
> --- a/drivers/media/usb/uvc/uvc_ctrl.c
> +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> @@ -376,6 +376,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
>  				| UVC_CTRL_FLAG_GET_DEF
>  				| UVC_CTRL_FLAG_AUTO_UPDATE,
>  	},
> +	{
> +		.entity		= UVC_GUID_CHROMEOS_XU,
> +		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
> +		.index		= 3,
> +		.size		= 1,
> +		.flags		= UVC_CTRL_FLAG_SET_CUR
> +				| UVC_CTRL_FLAG_GET_RANGE
> +				| UVC_CTRL_FLAG_RESTORE,
> +	},
>  };
>  
>  static const u32 uvc_control_classes[] = {
> @@ -384,6 +393,17 @@ static const u32 uvc_control_classes[] = {
>  };
>  
>  static const int exposure_auto_mapping[] = { 2, 1, 4, 8 };
> +static const int cros_colorfx_mapping[] = { 1, // V4L2_COLORFX_NONE
> +					    -1, // V4L2_COLORFX_BW
> +					    -1, // V4L2_COLORFX_SEPIA
> +					    -1, // V4L2_COLORFX_NEGATIVE
> +					    -1, // V4L2_COLORFX_EMBOSS
> +					    -1, // V4L2_COLORFX_SKETCH
> +					    -1, // V4L2_COLORFX_SKY_BLUE
> +					    -1, // V4L2_COLORFX_GRASS_GREEN
> +					    -1, // V4L2_COLORFX_SKIN_WHITEN
> +					    0}; // V4L2_COLORFX_VIVID};

Extar '};' at the end of the line. The indentation also looks a bit
weird. I'll replace it with

static const int cros_colorfx_mapping[] = {
	1,	/* V4L2_COLORFX_NONE */
	-1,	/* V4L2_COLORFX_BW */
	-1,	/* V4L2_COLORFX_SEPIA */
	-1,	/* V4L2_COLORFX_NEGATIVE */
	-1,	/* V4L2_COLORFX_EMBOSS */
	-1,	/* V4L2_COLORFX_SKETCH */
	-1,	/* V4L2_COLORFX_SKY_BLUE */
	-1,	/* V4L2_COLORFX_GRASS_GREEN */
	-1,	/* V4L2_COLORFX_SKIN_WHITEN */
	0,	/* V4L2_COLORFX_VIVID */
};

> +
>  
>  static bool uvc_ctrl_mapping_is_compound(struct uvc_control_mapping *mapping)
>  {
> @@ -975,6 +995,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
>  		.data_type	= UVC_CTRL_DATA_TYPE_BITMASK,
>  		.name		= "Region of Interest Auto Ctrls",
>  	},
> +	{
> +		.id		= V4L2_CID_COLORFX,
> +		.entity		= UVC_GUID_CHROMEOS_XU,
> +		.selector	= UVC_CROSXU_CONTROL_IQ_PROFILE,
> +		.size		= 8,
> +		.offset		= 0,
> +		.v4l2_type	= V4L2_CTRL_TYPE_MENU,
> +		.data_type	= UVC_CTRL_DATA_TYPE_ENUM,
> +		.menu_mapping	= cros_colorfx_mapping,
> +		.menu_mask	= BIT(V4L2_COLORFX_VIVID) |
> +				  BIT(V4L2_COLORFX_NONE),
> +	},
>  };
>  
>  /* ------------------------------------------------------------------------
> diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> index 12a57e1d34674a3a264ed7f88bed43926661fcd4..22e0dab0809e296e089940620ae0e8838e109701 100644
> --- a/include/linux/usb/uvc.h
> +++ b/include/linux/usb/uvc.h
> @@ -29,6 +29,9 @@
>  #define UVC_GUID_EXT_GPIO_CONTROLLER \
>  	{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
>  	 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> +#define UVC_GUID_CHROMEOS_XU \
> +	{0x24, 0xe9, 0xd7, 0x74, 0xc9, 0x49, 0x45, 0x4a, \
> +	 0x98, 0xa3, 0xc8, 0x07, 0x7e, 0x05, 0x1c, 0xa3}

I'd like to add a link to the documentation, but searching for the GUID
didn't turn up any meaningful result. Where can I find documentation for
this XU ?

The link can be added later, so

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

>  #define UVC_GUID_MSXU_1_5 \
>  	{0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
>  	 0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> @@ -50,6 +53,8 @@
>  #define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG	0x0f
>  #define UVC_MSXU_CONTROL_FIELDOFVIEW2		0x10
>  
> +#define UVC_CROSXU_CONTROL_IQ_PROFILE		0x04
> +
>  #define UVC_GUID_FORMAT_MJPEG \
>  	{ 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
>  	 0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}

-- 
Regards,

Laurent Pinchart

Re: [PATCH 4/4] media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

[Ricardo Ribalda]

Hi Laurent

On Sat, 13 Sept 2025 at 16:06, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Mon, Aug 18, 2025 at 08:15:39PM +0000, Ricardo Ribalda wrote:
> > The ChromeOS XU provides a control to change the IQ profile for a camera.
> > It can be switched from VIVID (a.k.a. standard) to NONE (a.k.a. natural).
> >
> > Wire it up to the standard v4l2 control.
> >
> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++++++++++++
> >  include/linux/usb/uvc.h          |  5 +++++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > index ff975f96e1325532e2299047c07de5d1b9cf09db..8766a441ad1d8554c0daaed3f87758321684246b 100644
> > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > @@ -376,6 +376,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
> >                               | UVC_CTRL_FLAG_GET_DEF
> >                               | UVC_CTRL_FLAG_AUTO_UPDATE,
> >       },
> > +     {
> > +             .entity         = UVC_GUID_CHROMEOS_XU,
> > +             .selector       = UVC_CROSXU_CONTROL_IQ_PROFILE,
> > +             .index          = 3,
> > +             .size           = 1,
> > +             .flags          = UVC_CTRL_FLAG_SET_CUR
> > +                             | UVC_CTRL_FLAG_GET_RANGE
> > +                             | UVC_CTRL_FLAG_RESTORE,
> > +     },
> >  };
> >
> >  static const u32 uvc_control_classes[] = {
> > @@ -384,6 +393,17 @@ static const u32 uvc_control_classes[] = {
> >  };
> >
> >  static const int exposure_auto_mapping[] = { 2, 1, 4, 8 };
> > +static const int cros_colorfx_mapping[] = { 1, // V4L2_COLORFX_NONE
> > +                                         -1, // V4L2_COLORFX_BW
> > +                                         -1, // V4L2_COLORFX_SEPIA
> > +                                         -1, // V4L2_COLORFX_NEGATIVE
> > +                                         -1, // V4L2_COLORFX_EMBOSS
> > +                                         -1, // V4L2_COLORFX_SKETCH
> > +                                         -1, // V4L2_COLORFX_SKY_BLUE
> > +                                         -1, // V4L2_COLORFX_GRASS_GREEN
> > +                                         -1, // V4L2_COLORFX_SKIN_WHITEN
> > +                                         0}; // V4L2_COLORFX_VIVID};
>
> Extar '};' at the end of the line. The indentation also looks a bit
> weird. I'll replace it with
>
> static const int cros_colorfx_mapping[] = {
>         1,      /* V4L2_COLORFX_NONE */
>         -1,     /* V4L2_COLORFX_BW */
>         -1,     /* V4L2_COLORFX_SEPIA */
>         -1,     /* V4L2_COLORFX_NEGATIVE */
>         -1,     /* V4L2_COLORFX_EMBOSS */
>         -1,     /* V4L2_COLORFX_SKETCH */
>         -1,     /* V4L2_COLORFX_SKY_BLUE */
>         -1,     /* V4L2_COLORFX_GRASS_GREEN */
>         -1,     /* V4L2_COLORFX_SKIN_WHITEN */
>         0,      /* V4L2_COLORFX_VIVID */
> };
>
> > +
> >
> >  static bool uvc_ctrl_mapping_is_compound(struct uvc_control_mapping *mapping)
> >  {
> > @@ -975,6 +995,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> >               .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> >               .name           = "Region of Interest Auto Ctrls",
> >       },
> > +     {
> > +             .id             = V4L2_CID_COLORFX,
> > +             .entity         = UVC_GUID_CHROMEOS_XU,
> > +             .selector       = UVC_CROSXU_CONTROL_IQ_PROFILE,
> > +             .size           = 8,
> > +             .offset         = 0,
> > +             .v4l2_type      = V4L2_CTRL_TYPE_MENU,
> > +             .data_type      = UVC_CTRL_DATA_TYPE_ENUM,
> > +             .menu_mapping   = cros_colorfx_mapping,
> > +             .menu_mask      = BIT(V4L2_COLORFX_VIVID) |
> > +                               BIT(V4L2_COLORFX_NONE),
> > +     },
> >  };
> >
> >  /* ------------------------------------------------------------------------
> > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > index 12a57e1d34674a3a264ed7f88bed43926661fcd4..22e0dab0809e296e089940620ae0e8838e109701 100644
> > --- a/include/linux/usb/uvc.h
> > +++ b/include/linux/usb/uvc.h
> > @@ -29,6 +29,9 @@
> >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > +#define UVC_GUID_CHROMEOS_XU \
> > +     {0x24, 0xe9, 0xd7, 0x74, 0xc9, 0x49, 0x45, 0x4a, \
> > +      0x98, 0xa3, 0xc8, 0x07, 0x7e, 0x05, 0x1c, 0xa3}
>
> I'd like to add a link to the documentation, but searching for the GUID
> didn't turn up any meaningful result. Where can I find documentation for
> this XU ?

It is not public yet. Not because there is anything secret about it,
but because of the "making documentation process".

Once there is a public document I will add a link.

Regards!
>
> The link can be added later, so
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> >  #define UVC_GUID_MSXU_1_5 \
> >       {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> >        0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> > @@ -50,6 +53,8 @@
> >  #define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG 0x0f
> >  #define UVC_MSXU_CONTROL_FIELDOFVIEW2                0x10
> >
> > +#define UVC_CROSXU_CONTROL_IQ_PROFILE                0x04
> > +
> >  #define UVC_GUID_FORMAT_MJPEG \
> >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> >        0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}
>
> --
> Regards,
>
> Laurent Pinchart



-- 
Ricardo Ribalda

Re: [PATCH 4/4] media: uvcvideo: Support UVC_CROSXU_CONTROL_IQ_PROFILE

[Laurent Pinchart]

On Mon, Sep 15, 2025 at 09:18:33AM +0200, Ricardo Ribalda wrote:
> On Sat, 13 Sept 2025 at 16:06, Laurent Pinchart wrote:
> > On Mon, Aug 18, 2025 at 08:15:39PM +0000, Ricardo Ribalda wrote:
> > > The ChromeOS XU provides a control to change the IQ profile for a camera.
> > > It can be switched from VIVID (a.k.a. standard) to NONE (a.k.a. natural).
> > >
> > > Wire it up to the standard v4l2 control.
> > >
> > > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org>
> > > ---
> > >  drivers/media/usb/uvc/uvc_ctrl.c | 32 ++++++++++++++++++++++++++++++++
> > >  include/linux/usb/uvc.h          |  5 +++++
> > >  2 files changed, 37 insertions(+)
> > >
> > > diff --git a/drivers/media/usb/uvc/uvc_ctrl.c b/drivers/media/usb/uvc/uvc_ctrl.c
> > > index ff975f96e1325532e2299047c07de5d1b9cf09db..8766a441ad1d8554c0daaed3f87758321684246b 100644
> > > --- a/drivers/media/usb/uvc/uvc_ctrl.c
> > > +++ b/drivers/media/usb/uvc/uvc_ctrl.c
> > > @@ -376,6 +376,15 @@ static const struct uvc_control_info uvc_ctrls[] = {
> > >                               | UVC_CTRL_FLAG_GET_DEF
> > >                               | UVC_CTRL_FLAG_AUTO_UPDATE,
> > >       },
> > > +     {
> > > +             .entity         = UVC_GUID_CHROMEOS_XU,
> > > +             .selector       = UVC_CROSXU_CONTROL_IQ_PROFILE,
> > > +             .index          = 3,
> > > +             .size           = 1,
> > > +             .flags          = UVC_CTRL_FLAG_SET_CUR
> > > +                             | UVC_CTRL_FLAG_GET_RANGE
> > > +                             | UVC_CTRL_FLAG_RESTORE,
> > > +     },
> > >  };
> > >
> > >  static const u32 uvc_control_classes[] = {
> > > @@ -384,6 +393,17 @@ static const u32 uvc_control_classes[] = {
> > >  };
> > >
> > >  static const int exposure_auto_mapping[] = { 2, 1, 4, 8 };
> > > +static const int cros_colorfx_mapping[] = { 1, // V4L2_COLORFX_NONE
> > > +                                         -1, // V4L2_COLORFX_BW
> > > +                                         -1, // V4L2_COLORFX_SEPIA
> > > +                                         -1, // V4L2_COLORFX_NEGATIVE
> > > +                                         -1, // V4L2_COLORFX_EMBOSS
> > > +                                         -1, // V4L2_COLORFX_SKETCH
> > > +                                         -1, // V4L2_COLORFX_SKY_BLUE
> > > +                                         -1, // V4L2_COLORFX_GRASS_GREEN
> > > +                                         -1, // V4L2_COLORFX_SKIN_WHITEN
> > > +                                         0}; // V4L2_COLORFX_VIVID};
> >
> > Extar '};' at the end of the line. The indentation also looks a bit
> > weird. I'll replace it with
> >
> > static const int cros_colorfx_mapping[] = {
> >         1,      /* V4L2_COLORFX_NONE */
> >         -1,     /* V4L2_COLORFX_BW */
> >         -1,     /* V4L2_COLORFX_SEPIA */
> >         -1,     /* V4L2_COLORFX_NEGATIVE */
> >         -1,     /* V4L2_COLORFX_EMBOSS */
> >         -1,     /* V4L2_COLORFX_SKETCH */
> >         -1,     /* V4L2_COLORFX_SKY_BLUE */
> >         -1,     /* V4L2_COLORFX_GRASS_GREEN */
> >         -1,     /* V4L2_COLORFX_SKIN_WHITEN */
> >         0,      /* V4L2_COLORFX_VIVID */
> > };
> >
> > > +
> > >
> > >  static bool uvc_ctrl_mapping_is_compound(struct uvc_control_mapping *mapping)
> > >  {
> > > @@ -975,6 +995,18 @@ static const struct uvc_control_mapping uvc_ctrl_mappings[] = {
> > >               .data_type      = UVC_CTRL_DATA_TYPE_BITMASK,
> > >               .name           = "Region of Interest Auto Ctrls",
> > >       },
> > > +     {
> > > +             .id             = V4L2_CID_COLORFX,
> > > +             .entity         = UVC_GUID_CHROMEOS_XU,
> > > +             .selector       = UVC_CROSXU_CONTROL_IQ_PROFILE,
> > > +             .size           = 8,
> > > +             .offset         = 0,
> > > +             .v4l2_type      = V4L2_CTRL_TYPE_MENU,
> > > +             .data_type      = UVC_CTRL_DATA_TYPE_ENUM,
> > > +             .menu_mapping   = cros_colorfx_mapping,
> > > +             .menu_mask      = BIT(V4L2_COLORFX_VIVID) |
> > > +                               BIT(V4L2_COLORFX_NONE),
> > > +     },
> > >  };
> > >
> > >  /* ------------------------------------------------------------------------
> > > diff --git a/include/linux/usb/uvc.h b/include/linux/usb/uvc.h
> > > index 12a57e1d34674a3a264ed7f88bed43926661fcd4..22e0dab0809e296e089940620ae0e8838e109701 100644
> > > --- a/include/linux/usb/uvc.h
> > > +++ b/include/linux/usb/uvc.h
> > > @@ -29,6 +29,9 @@
> > >  #define UVC_GUID_EXT_GPIO_CONTROLLER \
> > >       {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, \
> > >        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x03}
> > > +#define UVC_GUID_CHROMEOS_XU \
> > > +     {0x24, 0xe9, 0xd7, 0x74, 0xc9, 0x49, 0x45, 0x4a, \
> > > +      0x98, 0xa3, 0xc8, 0x07, 0x7e, 0x05, 0x1c, 0xa3}
> >
> > I'd like to add a link to the documentation, but searching for the GUID
> > didn't turn up any meaningful result. Where can I find documentation for
> > this XU ?
> 
> It is not public yet. Not because there is anything secret about it,
> but because of the "making documentation process".

That makes reviewing patches annoying :-/ Any expected time frame ?

You don't have to polish the documentation, if it was good enough to be
shared with webcam vendors to be implemented, it's good enough for the
community too :-)

> Once there is a public document I will add a link.
> 
> > The link can be added later, so
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > >  #define UVC_GUID_MSXU_1_5 \
> > >       {0xdc, 0x95, 0x3f, 0x0f, 0x32, 0x26, 0x4e, 0x4c, \
> > >        0x92, 0xc9, 0xa0, 0x47, 0x82, 0xf4, 0x3b, 0xc8}
> > > @@ -50,6 +53,8 @@
> > >  #define UVC_MSXU_CONTROL_FIELDOFVIEW2_CONFIG 0x0f
> > >  #define UVC_MSXU_CONTROL_FIELDOFVIEW2                0x10
> > >
> > > +#define UVC_CROSXU_CONTROL_IQ_PROFILE                0x04
> > > +
> > >  #define UVC_GUID_FORMAT_MJPEG \
> > >       { 'M',  'J',  'P',  'G', 0x00, 0x00, 0x10, 0x00, \
> > >        0x80, 0x00, 0x00, 0xaa, 0x00, 0x38, 0x9b, 0x71}

-- 
Regards,

Laurent Pinchart