Re: [PATCH 12/19] media: i2c: imx290: Fix max gain value

Dave Stevenson
2022-07-21T16:08:59Z

On Thu, 21 Jul 2022 at 09:36, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The gain is expressed in multiple of 0.3dB, as a value between 0.0dB
> and 72.0dB. The maximum value is thus 240, not 72.

At this point in the series I'll agree with you as it is for V4L2_CID_GAIN.
However later in the series you convert to V4L2_CID_ANALOGUE_GAIN, and
there I disagree.

The register is for a combined 0-30dB of analogue gain, and 0-42dB of
digital gain, both in 0.3dB steps.
V4L2_CID_GAIN can have a range of 0-240.
V4L2_CID_ANALOGUE_GAIN has a range of 0-100.

Minor additional point: IMX327 is the previous version of this and
only goes up to 1080p60 instead of 1080p120 (10bit only). That
supports 0-29.4dB of analogue gain, and 42dB of digital gain, for a
max value of 238. If using the definition for analogue gain only, then
you may end up with 0.6dB of digital gain instead, but it will work.
IMX462 is the newer version and supports 1080p120 in 10 or 12bit. I
don't have a full datasheet for it, but the product brief lists
0-29.4dB of analogue, and 42dB of digital gain, same as IMX327.
Seeing as the 120fps modes are not implemented in this driver, it
currently supports all 3 models.

Dave

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/i2c/imx290.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> index 3cb024b73ee7..1bd464932432 100644
> --- a/drivers/media/i2c/imx290.c
> +++ b/drivers/media/i2c/imx290.c
> @@ -1020,7 +1020,7 @@ static int imx290_probe(struct i2c_client *client)
> imx290->ctrls.lock = &imx290->lock;
>
> v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> - V4L2_CID_GAIN, 0, 72, 1, 0);
> + V4L2_CID_GAIN, 0, 240, 1, 0);
>
> v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> V4L2_CID_EXPOSURE, 1, IMX290_VMAX_DEFAULT - 2, 1,
> --
> Regards,
>
> Laurent Pinchart
>