Re: [PATCH v2 1/4] media: i2c: imx290: Limit analogue gain according to module

[Alexander Stein]

Hi Dave,

Am Donnerstag, 5. Dezember 2024, 16:37:01 CET schrieb Dave Stevenson:
> Hi Alexander
> 
> On Thu, 5 Dec 2024 at 15:22, Alexander Stein
> <alexander.stein@xxxxxxxxxxxxxxx> wrote:
> >
> > Hi Dave,
> >
> > Am Mittwoch, 20. November 2024, 20:17:03 CET schrieb Dave Stevenson:
> > > The imx327 only supports up to 29.4dB of analogue gain, vs
> > > the imx290 going up to 30dB. Both are in 0.3dB steps.
> >
> > While I agree for 30dB on imx290, my (maybe outdated) Rev0.2 datasheet says
> > up to 27dB in 0.3dB steps.
> 
> For IMX327, I have revision E17Z06B93 2019/03/25.
> 
> The revision control section for Rev0.3 lists
> Page 1: "Correction: Max analog gain 27dB - 29.4dB"
> Page 74: "Correction: Max Gain 69dB -> 71.4dB"
> 
> The graph in "Gain Adjustment Function" (page 74) also shows values
> above 27dB as being purely analogue gain. Certainly 0x62 is red for
> analogue, which would be a gain of 29.4dB. The next dot is 0x64 (30dB)
> and blue, so we have to trust the text for 0x63 being 29.4dB analogue
> and 0.3dB digital gain.
> 
> So I'm happy that the limit is 29.4dB.

Okay, this is a newer datasheet. So assume this is correct.

Thanks for clarification.
Reviewed-by: Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx>

Best regards,
Alexander


>   Dave
> 
> > Despite that this change looks good.
> >
> > Best regards,
> > Alexander
> >
> > > As we now have model specific config, fix this mismatch,
> > > and delete the comment referencing it.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > > ---
> > >  drivers/media/i2c/imx290.c | 12 ++++++------
> > >  1 file changed, 6 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/media/i2c/imx290.c b/drivers/media/i2c/imx290.c
> > > index ee698c99001d..da654deb444a 100644
> > > --- a/drivers/media/i2c/imx290.c
> > > +++ b/drivers/media/i2c/imx290.c
> > > @@ -176,6 +176,7 @@ struct imx290_model_info {
> > >       enum imx290_colour_variant colour_variant;
> > >       const struct cci_reg_sequence *init_regs;
> > >       size_t init_regs_num;
> > > +     unsigned int max_analog_gain;
> > >       const char *name;
> > >  };
> > >
> > > @@ -876,14 +877,10 @@ static int imx290_ctrl_init(struct imx290 *imx290)
> > >        * up to 72.0dB (240) add further digital gain. Limit the range to
> > >        * analog gain only, support for digital gain can be added separately
> > >        * if needed.
> > > -      *
> > > -      * The IMX327 and IMX462 are largely compatible with the IMX290, but
> > > -      * have an analog gain range of 0.0dB to 29.4dB and 42dB of digital
> > > -      * gain. When support for those sensors gets added to the driver, the
> > > -      * gain control should be adjusted accordingly.
> > >        */
> > >       v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
> > > -                       V4L2_CID_ANALOGUE_GAIN, 0, 100, 1, 0);
> > > +                       V4L2_CID_ANALOGUE_GAIN, 0,
> > > +                       imx290->model->max_analog_gain, 1, 0);
> > >
> > >       /*
> > >        * Correct range will be determined through imx290_ctrl_update setting
> > > @@ -1441,18 +1438,21 @@ static const struct imx290_model_info imx290_models[] = {
> > >               .colour_variant = IMX290_VARIANT_COLOUR,
> > >               .init_regs = imx290_global_init_settings_290,
> > >               .init_regs_num = ARRAY_SIZE(imx290_global_init_settings_290),
> > > +             .max_analog_gain = 100,
> > >               .name = "imx290",
> > >       },
> > >       [IMX290_MODEL_IMX290LLR] = {
> > >               .colour_variant = IMX290_VARIANT_MONO,
> > >               .init_regs = imx290_global_init_settings_290,
> > >               .init_regs_num = ARRAY_SIZE(imx290_global_init_settings_290),
> > > +             .max_analog_gain = 100,
> > >               .name = "imx290",
> > >       },
> > >       [IMX290_MODEL_IMX327LQR] = {
> > >               .colour_variant = IMX290_VARIANT_COLOUR,
> > >               .init_regs = imx290_global_init_settings_327,
> > >               .init_regs_num = ARRAY_SIZE(imx290_global_init_settings_327),
> > > +             .max_analog_gain = 98,
> > >               .name = "imx327",
> > >       },
> > >  };
> > >
> > >
> >
> >
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.tq-group.com/
> >
> >
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/