| Message ID | 20220721083540.1525-13-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: imx290: Miscellaneous improvements | expand |
Am Donnerstag, 21. Juli 2022, 10:35:33 CEST schrieb Laurent Pinchart: > 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. > > 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, Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
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 >
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,
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,
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. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/media/i2c/imx290.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)