[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250915072131.GA22385@pendragon.ideasonboard.com>
Date: Mon, 15 Sep 2025 10:21:31 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Ricardo Ribalda <ribalda@...omium.org>
Cc: Hans de Goede <hansg@...nel.org>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Hans Verkuil <hverkuil@...nel.org>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH 4/4] media: uvcvideo: Support
UVC_CROSXU_CONTROL_IQ_PROFILE
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@...omium.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@...asonboard.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
Powered by blists - more mailing lists