Re: [PATCH v2 02/10] media: ar0521: Add V4L2_CID_ANALOG_GAIN

[Sakari Ailus <sakari.ailus@iki.fi>]

Hi Jacopo,

On Mon, Oct 24, 2022 at 02:31:05PM +0200, Jacopo Mondi wrote:
> Hi Dave,
> 
> On Mon, Oct 24, 2022 at 01:13:58PM +0100, Dave Stevenson wrote:
> > Hi Jacopo
> >
> > On Sat, 22 Oct 2022 at 11:47, Jacopo Mondi <jacopo@jmondi.org> wrote:
> > >
> > > Add support for V4L2_CID_ANALOG_GAIN. The control programs the global
> > > gain register which applies to all color channels.
> > >
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >  drivers/media/i2c/ar0521.c | 16 ++++++++++++++++
> > >  1 file changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
> > > index 0daa61df2603..ba169f0218a9 100644
> > > --- a/drivers/media/i2c/ar0521.c
> > > +++ b/drivers/media/i2c/ar0521.c
> > > @@ -35,6 +35,11 @@
> > >  #define AR0521_HEIGHT_BLANKING_MIN     38u /* must be even */
> > >  #define AR0521_TOTAL_WIDTH_MIN      2968u
> > >
> > > +#define AR0521_ANA_GAIN_MIN            0x00
> > > +#define AR0521_ANA_GAIN_MAX            0x3f
> >
> > The register reference I have says it is u3.4 format, which would make
> > the max value 0x7f rather than 0x3f.
> >
> > Functionally it makes no real difference, but a max gain of nearly x7
> > 15/16ths  is preferable to nearly x3 15/16ths.
> >
> > If it is u3.4 I'd have expected the minimum to be 0x10 to avoid a gain
> > of less than x1 (does it even work?)
> >
> 
> The value of the 0x3280 register is written in the 7 low bits of the
> 0x305e register. Whatever is written to 0x305e is similalrly reflected
> in the 0x3280 one. The 0x305e[6:4] bits are the coarse analog gain
> value and the lower 4 bits are the fine analog gain value. I wonder if
> u3.4 is used there to describe the that, even if u3.4 has a different
> meaning
> 
> > Looking at the listed m0, c0, m1, c1 values for gain (1, 0, 0, and 4
> > respectively), that maps to a formula:
> > gain = code / 4
> > Min and max codes are 0 and 0x1f, so that implies it will do a gain of
> > less than x1, and goes up to x7.75.
> 
> The sensor does not use the CCS gain model, but an exponential
> piecewise function documented as a table of register values/gains in
> the application manual.
> 
> The sensor exposes a list of CCS-compatible registers, including
> "0x0204 analogue_gain_code_global", from which I presume one should
> contorl the gain if the CCS compatible model works. From my tests,
> writing to that register is a no-op.
> 
> I presume the CCS compatible interface is not plumbed, or maybe it
> depdends on the FW version, or else :)

It is also possible this is a sensor developed based on an earlier one that
was compatible with e.g. SMIA. If the newer CCS-only registers aren't
defined, it is unlikely it would be compatible.

-- 
Regards,

Sakari Ailus

Ps. this would be a good time to get patches towards upstream. :-)