Hi Richard On Tue, 25 Feb 2025 at 13:09, Richard Leitner <richard.leitner@xxxxxxxxx> wrote: > > The sensors analogue gain is stored within two "LONG GAIN" registers > (0x3508 and 0x3509) where the first one holds the upper 5 bits of the > value. The second register (0x3509) holds the lower 4 bits of the gain > value in its upper 4 bits. The lower 4 register bits are fraction bits. > > This patch changes the gain control to adhere to the datasheet and > make the "upper gain register" (0x3508) accessible via the gain control, > resulting in a new maximum of 0x1fff instead of 0xff. > > As the "upper gain register" is now written during exposure/gain update > remove the hard-coded 0x00 write to it from common_regs. > > We cover only the "real gain format" use-case. The "sensor gain > format" one is ignored as based on the hard-coded "AEC MANUAL" register > configuration it is disabled. > > All values are based on the OV9281 datasheet v1.01 (09.18.2015). My web searches turn up a 1.53 from Jan 2019 - http://www.sinotimes-tech.com/product/20220217221034589.pdf That lists 0x3508 as DEBUG, not LONG_GAIN. The current range allows analogue gain to x15.9375. Expanding it to 0x1ff.f would be up to x511.9375. I believe that equates to ~54dB as we're scaling voltages, not power. The spec sheet for the sensor lists S/N of 38dB and dynamic range of 68dB, so x511 will be almost pure noise. Doing a very basic test using i2ctransfer to set gain values whilst the sensor is running suggests that the image is the same regardless of bits 2-4 of 0x3508. Setting either bits 0 or 1 increases the gain by around x8.5, but they don't combine. Overall can I ask how you've tested that a range up to 0x1fff works, and on which module? I currently don't believe this works as intended. Dave > Signed-off-by: Richard Leitner <richard.leitner@xxxxxxxxx> > --- > drivers/media/i2c/ov9282.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c > index c882a021cf18852237bf9b9524d3de0c5b48cbcb..e6effb2b42d4d5d0ca3d924df59c60512f9ce65d 100644 > --- a/drivers/media/i2c/ov9282.c > +++ b/drivers/media/i2c/ov9282.c > @@ -54,11 +54,11 @@ > #define OV9282_AEC_MANUAL_DEFAULT 0x00 > > /* Analog gain control */ > -#define OV9282_REG_AGAIN 0x3509 > -#define OV9282_AGAIN_MIN 0x10 > -#define OV9282_AGAIN_MAX 0xff > -#define OV9282_AGAIN_STEP 1 > -#define OV9282_AGAIN_DEFAULT 0x10 > +#define OV9282_REG_AGAIN 0x3508 > +#define OV9282_AGAIN_MIN 0x0010 > +#define OV9282_AGAIN_MAX 0x1fff > +#define OV9282_AGAIN_STEP 0x0001 > +#define OV9282_AGAIN_DEFAULT 0x0010 > > /* Group hold register */ > #define OV9282_REG_HOLD 0x3308 > @@ -226,7 +226,6 @@ static const struct ov9282_reg common_regs[] = { > {OV9282_REG_AEC_MANUAL, OV9282_GAIN_PREC16_EN}, > {0x3505, 0x8c}, > {0x3507, 0x03}, > - {0x3508, 0x00}, > {0x3610, 0x80}, > {0x3611, 0xa0}, > {0x3620, 0x6e}, > @@ -605,7 +604,11 @@ static int ov9282_update_exp_gain(struct ov9282 *ov9282, u32 exposure, u32 gain) > if (ret) > goto error_release_group_hold; > > - ret = ov9282_write_reg(ov9282, OV9282_REG_AGAIN, 1, gain); > + ret = ov9282_write_reg(ov9282, OV9282_REG_AGAIN, 1, (gain >> 8) & 0x1f); > + if (ret) > + goto error_release_group_hold; > + > + ret = ov9282_write_reg(ov9282, OV9282_REG_AGAIN + 1, 1, gain & 0xff); > > error_release_group_hold: > ov9282_write_reg(ov9282, OV9282_REG_HOLD, 1, 0); > > -- > 2.47.2 > >
