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

Laurent Pinchart
2022-10-16T04:51:40Z

Hi Dave,

On Thu, Jul 21, 2022 at 05:08:37PM +0100, Dave Stevenson wrote:
> On Thu, 21 Jul 2022 at 09:36, Laurent Pinchart 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.

Good point, I had missed that. I'll fix it.

> 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.

Why does it have to be so complicated ? :-) I'll see what I can do.

> > 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