RE: [PATCH 6/8] media: ov2740: add manual white balance controls

[Cao, Bingbu]

AMD General

Hi, Maurizio,

Thanks for the patch.

> -----Original Message-----
> From: Maurizio Casciano <mauriziocasciano7@xxxxxxxxx>
> Sent: Wednesday, August 26, 2026 9:23 PM
> To: linux-media@xxxxxxxxxxxxxxx
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>; Sakari Ailus
> <sakari.ailus@xxxxxxxxxxxxxxx>; Cao, Bingbu <Bingbu.Cao@xxxxxxx>; Jacopo
> Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx>; Nicholas Roth
> <nicholas@xxxxxxxxxxxxx>; Andy Shevchenko <andy@xxxxxxxxxx>; Hans de Goede
> <hansg@xxxxxxxxxx>; Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; Jose
> Maria Martin <jmmartinf@xxxxxxxxxxx>; linux-staging@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Maurizio Casciano <mauriziocasciano7@xxxxxxxxx>
> Subject: [PATCH 6/8] media: ov2740: add manual white balance controls
>
> [You don't often get email from mauriziocasciano7@xxxxxxxxx. Learn why this
> is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> The sensor has separate red, green and blue manual white-balance gain
> registers, but the driver currently writes the same digital-gain value
> to all three channels. This prevents userspace from correcting the
> strong color cast of raw Bayer capture.
>
> Expose red- and blue-balance controls relative to the digital gain,
> update all three channels under group hold, and always release and
> launch the group even when a channel write fails.
>
> Tested on the Yoga Book OV2740 with live gain changes and continuous raw
> capture.
>
> Signed-off-by: Maurizio Casciano <mauriziocasciano7@xxxxxxxxx>
> Assisted-by: Codex:gpt-5.6-sol sparse
> ---
>  drivers/media/i2c/ov2740.c | 71 +++++++++++++++++++++++++++-----------
>  1 file changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
> index b760d4dc0e68..7651443b28c1 100644
> --- a/drivers/media/i2c/ov2740.c
> +++ b/drivers/media/i2c/ov2740.c

--snip--

> @@ -881,9 +897,22 @@ static int ov2740_init_controls(struct ov2740 *ov2740)
>         v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
> V4L2_CID_ANALOGUE_GAIN,
>                           OV2740_ANAL_GAIN_MIN, OV2740_ANAL_GAIN_MAX,
>                           OV2740_ANAL_GAIN_STEP, OV2740_ANAL_GAIN_MIN);
> -       v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
> V4L2_CID_DIGITAL_GAIN,
> -                         OV2740_DGTL_GAIN_MIN, OV2740_DGTL_GAIN_MAX,
> -                         OV2740_DGTL_GAIN_STEP, OV2740_DGTL_GAIN_DEFAULT);
> +       ov2740->digital_gain =
> +               v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
> +                                 V4L2_CID_DIGITAL_GAIN,
> +                                 OV2740_DGTL_GAIN_MIN,
> OV2740_DGTL_GAIN_MAX,
> +                                 OV2740_DGTL_GAIN_STEP,
> +                                 OV2740_DGTL_GAIN_DEFAULT);
> +       ov2740->red_balance =
> +               v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
> +                                 V4L2_CID_RED_BALANCE,
> +                                 1, OV2740_DGTL_GAIN_MAX, 1,
> +                                 OV2740_DGTL_GAIN_DEFAULT);
> +       ov2740->blue_balance =
> +               v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
> +                                 V4L2_CID_BLUE_BALANCE,
> +                                 1, OV2740_DGTL_GAIN_MAX, 1,
> +                                 OV2740_DGTL_GAIN_DEFAULT);

Do we need a v4l2_ctrl_cluster() here?

>         exposure_max = ov2740->cur_mode->vts_def -
> OV2740_EXPOSURE_MAX_MARGIN;
>         ov2740->exposure = v4l2_ctrl_new_std(ctrl_hdlr, &ov2740_ctrl_ops,
>                                              V4L2_CID_EXPOSURE,
> --
> 2.53.0