[14/16] media: i2c: ov9282: Add support for 1280x800 and 640x400 modes

Message ID 20221005152809.3785786-15-dave.stevenson@raspberrypi.com (mailing list archive)
State Changes Requested
Delegated to: Sakari Ailus
Headers
Series Updates to ov9282 sensor driver |

Commit Message

Dave Stevenson Oct. 5, 2022, 3:28 p.m. UTC
  Adds register settings for additional modes.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
---
 drivers/media/i2c/ov9282.c | 97 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 97 insertions(+)
  

Comments

Jacopo Mondi Oct. 6, 2022, 9:48 a.m. UTC | #1
On Wed, Oct 05, 2022 at 04:28:07PM +0100, Dave Stevenson wrote:
> Adds register settings for additional modes.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> ---
>  drivers/media/i2c/ov9282.c | 97 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 97 insertions(+)
>
> diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> index d892f53fb1ea..ec1599488f21 100644
> --- a/drivers/media/i2c/ov9282.c
> +++ b/drivers/media/i2c/ov9282.c
> @@ -251,6 +251,37 @@ struct ov9282_reg_list common_regs_list = {
>  };
>
>  /* Sensor mode registers */
> +static const struct ov9282_reg mode_1280x800_regs[] = {
> +	{0x3778, 0x00},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x00},
> +	{0x3804, 0x05},
> +	{0x3805, 0x0f},
> +	{0x3806, 0x03},
> +	{0x3807, 0x2f},
> +	{0x3808, 0x05},
> +	{0x3809, 0x00},
> +	{0x380a, 0x03},
> +	{0x380b, 0x20},
> +	{0x3810, 0x00},
> +	{0x3811, 0x08},
> +	{0x3812, 0x00},
> +	{0x3813, 0x08},
> +	{0x3814, 0x11},
> +	{0x3815, 0x11},
> +	{0x3820, 0x40},
> +	{0x3821, 0x00},
> +	{0x4003, 0x40},
> +	{0x4008, 0x04},
> +	{0x4009, 0x0b},
> +	{0x400c, 0x00},
> +	{0x400d, 0x07},
> +	{0x4507, 0x00},
> +	{0x4509, 0x00},
> +};
> +
>  static const struct ov9282_reg mode_1280x720_regs[] = {
>  	{0x3778, 0x00},
>  	{0x3800, 0x00},
> @@ -282,6 +313,36 @@ static const struct ov9282_reg mode_1280x720_regs[] = {
>  	{0x4509, 0x80},
>  };
>
> +static const struct ov9282_reg mode_640x400_regs[] = {
> +	{0x3778, 0x10},
> +	{0x3800, 0x00},
> +	{0x3801, 0x00},
> +	{0x3802, 0x00},
> +	{0x3803, 0x00},
> +	{0x3804, 0x05},
> +	{0x3805, 0x0f},

I assume this mode is then binned as the analog crop rectangle is the
same as the other modes, right ?

> +	{0x3806, 0x03},
> +	{0x3807, 0x2f},
> +	{0x3808, 0x02},
> +	{0x3809, 0x80},
> +	{0x380a, 0x01},
> +	{0x380b, 0x90},
> +	{0x3810, 0x00},
> +	{0x3811, 0x04},
> +	{0x3812, 0x00},
> +	{0x3813, 0x04},
> +	{0x3814, 0x31},
> +	{0x3815, 0x22},
> +	{0x3820, 0x60},
> +	{0x3821, 0x01},
> +	{0x4008, 0x02},
> +	{0x4009, 0x05},
> +	{0x400c, 0x00},
> +	{0x400d, 0x03},
> +	{0x4507, 0x03},
> +	{0x4509, 0x80},
> +};
> +
>  /* Supported sensor mode configurations */
>  static const struct ov9282_mode supported_modes[] = {
>  	{
> @@ -306,6 +367,42 @@ static const struct ov9282_mode supported_modes[] = {
>  			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
>  			.regs = mode_1280x720_regs,
>  		},
> +	}, {
> +		.width = 1280,
> +		.height = 800,
> +		.hblank_min = { 250, 176 },
> +		.vblank = 1022,
> +		.vblank_min = 110,
> +		.vblank_max = 51540,
> +		.link_freq_idx = 0,
> +		.crop = {
> +			.left = OV9282_PIXEL_ARRAY_LEFT,
> +			.top = OV9282_PIXEL_ARRAY_TOP,
> +			.width = 1280,
> +			.height = 800
> +		},
> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_1280x800_regs),
> +			.regs = mode_1280x800_regs,
> +		},
> +	}, {
> +		.width = 640,
> +		.height = 400,
> +		.hblank_min = { 890, 816 },
> +		.vblank = 1022,
> +		.vblank_min = 22,
> +		.vblank_max = 51540,
> +		.link_freq_idx = 0,
> +		.crop = {
> +			.left = OV9282_PIXEL_ARRAY_LEFT,
> +			.top = OV9282_PIXEL_ARRAY_TOP,
> +			.width = 1280,
> +			.height = 800
> +		},

This seems to confirm it.

I would have ordered them, but this will change the default mode
unless we define its index and use it at initialization time

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
  j

> +		.reg_list = {
> +			.num_of_regs = ARRAY_SIZE(mode_640x400_regs),
> +			.regs = mode_640x400_regs,
> +		},
>  	},
>  };
>
> --
> 2.34.1
>
  
Dave Stevenson Oct. 6, 2022, 11:46 a.m. UTC | #2
Hi Jacopo

Thanks for the review

On Thu, 6 Oct 2022 at 10:48, Jacopo Mondi <jacopo@jmondi.org> wrote:
>
> On Wed, Oct 05, 2022 at 04:28:07PM +0100, Dave Stevenson wrote:
> > Adds register settings for additional modes.
> >
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > ---
> >  drivers/media/i2c/ov9282.c | 97 ++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 97 insertions(+)
> >
> > diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
> > index d892f53fb1ea..ec1599488f21 100644
> > --- a/drivers/media/i2c/ov9282.c
> > +++ b/drivers/media/i2c/ov9282.c
> > @@ -251,6 +251,37 @@ struct ov9282_reg_list common_regs_list = {
> >  };
> >
> >  /* Sensor mode registers */
> > +static const struct ov9282_reg mode_1280x800_regs[] = {
> > +     {0x3778, 0x00},
> > +     {0x3800, 0x00},
> > +     {0x3801, 0x00},
> > +     {0x3802, 0x00},
> > +     {0x3803, 0x00},
> > +     {0x3804, 0x05},
> > +     {0x3805, 0x0f},
> > +     {0x3806, 0x03},
> > +     {0x3807, 0x2f},
> > +     {0x3808, 0x05},
> > +     {0x3809, 0x00},
> > +     {0x380a, 0x03},
> > +     {0x380b, 0x20},
> > +     {0x3810, 0x00},
> > +     {0x3811, 0x08},
> > +     {0x3812, 0x00},
> > +     {0x3813, 0x08},
> > +     {0x3814, 0x11},
> > +     {0x3815, 0x11},
> > +     {0x3820, 0x40},
> > +     {0x3821, 0x00},
> > +     {0x4003, 0x40},
> > +     {0x4008, 0x04},
> > +     {0x4009, 0x0b},
> > +     {0x400c, 0x00},
> > +     {0x400d, 0x07},
> > +     {0x4507, 0x00},
> > +     {0x4509, 0x00},
> > +};
> > +
> >  static const struct ov9282_reg mode_1280x720_regs[] = {
> >       {0x3778, 0x00},
> >       {0x3800, 0x00},
> > @@ -282,6 +313,36 @@ static const struct ov9282_reg mode_1280x720_regs[] = {
> >       {0x4509, 0x80},
> >  };
> >
> > +static const struct ov9282_reg mode_640x400_regs[] = {
> > +     {0x3778, 0x10},
> > +     {0x3800, 0x00},
> > +     {0x3801, 0x00},
> > +     {0x3802, 0x00},
> > +     {0x3803, 0x00},
> > +     {0x3804, 0x05},
> > +     {0x3805, 0x0f},
>
> I assume this mode is then binned as the analog crop rectangle is the
> same as the other modes, right ?

Yes, it is binned.
0x3820 bit 1 controls vertical binning. 0x3821 bit 0 is horizontal
binning. These are the same registers that control flips, hence the
flip handler having to read-modify-write.

> > +     {0x3806, 0x03},
> > +     {0x3807, 0x2f},
> > +     {0x3808, 0x02},
> > +     {0x3809, 0x80},
> > +     {0x380a, 0x01},
> > +     {0x380b, 0x90},
> > +     {0x3810, 0x00},
> > +     {0x3811, 0x04},
> > +     {0x3812, 0x00},
> > +     {0x3813, 0x04},
> > +     {0x3814, 0x31},
> > +     {0x3815, 0x22},
> > +     {0x3820, 0x60},
> > +     {0x3821, 0x01},
> > +     {0x4008, 0x02},
> > +     {0x4009, 0x05},
> > +     {0x400c, 0x00},
> > +     {0x400d, 0x03},
> > +     {0x4507, 0x03},
> > +     {0x4509, 0x80},
> > +};
> > +
> >  /* Supported sensor mode configurations */
> >  static const struct ov9282_mode supported_modes[] = {
> >       {
> > @@ -306,6 +367,42 @@ static const struct ov9282_mode supported_modes[] = {
> >                       .num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
> >                       .regs = mode_1280x720_regs,
> >               },
> > +     }, {
> > +             .width = 1280,
> > +             .height = 800,
> > +             .hblank_min = { 250, 176 },
> > +             .vblank = 1022,
> > +             .vblank_min = 110,
> > +             .vblank_max = 51540,
> > +             .link_freq_idx = 0,
> > +             .crop = {
> > +                     .left = OV9282_PIXEL_ARRAY_LEFT,
> > +                     .top = OV9282_PIXEL_ARRAY_TOP,
> > +                     .width = 1280,
> > +                     .height = 800
> > +             },
> > +             .reg_list = {
> > +                     .num_of_regs = ARRAY_SIZE(mode_1280x800_regs),
> > +                     .regs = mode_1280x800_regs,
> > +             },
> > +     }, {
> > +             .width = 640,
> > +             .height = 400,
> > +             .hblank_min = { 890, 816 },
> > +             .vblank = 1022,
> > +             .vblank_min = 22,
> > +             .vblank_max = 51540,
> > +             .link_freq_idx = 0,
> > +             .crop = {
> > +                     .left = OV9282_PIXEL_ARRAY_LEFT,
> > +                     .top = OV9282_PIXEL_ARRAY_TOP,
> > +                     .width = 1280,
> > +                     .height = 800
> > +             },
>
> This seems to confirm it.
>
> I would have ordered them, but this will change the default mode
> unless we define its index and use it at initialization time

Yes, I didn't want to change the default mode, although why the
existing mode was a non-centred crop I have no idea!

I'm happy to add a define or enum for each mode, initialise this table
using them, and then have a define for which mode is the default. That
removes any ambiguity, and means the table can be ordered.

  Dave
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Thanks
>   j
>
> > +             .reg_list = {
> > +                     .num_of_regs = ARRAY_SIZE(mode_640x400_regs),
> > +                     .regs = mode_640x400_regs,
> > +             },
> >       },
> >  };
> >
> > --
> > 2.34.1
> >
  

Patch

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index d892f53fb1ea..ec1599488f21 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -251,6 +251,37 @@  struct ov9282_reg_list common_regs_list = {
 };
 
 /* Sensor mode registers */
+static const struct ov9282_reg mode_1280x800_regs[] = {
+	{0x3778, 0x00},
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x05},
+	{0x3805, 0x0f},
+	{0x3806, 0x03},
+	{0x3807, 0x2f},
+	{0x3808, 0x05},
+	{0x3809, 0x00},
+	{0x380a, 0x03},
+	{0x380b, 0x20},
+	{0x3810, 0x00},
+	{0x3811, 0x08},
+	{0x3812, 0x00},
+	{0x3813, 0x08},
+	{0x3814, 0x11},
+	{0x3815, 0x11},
+	{0x3820, 0x40},
+	{0x3821, 0x00},
+	{0x4003, 0x40},
+	{0x4008, 0x04},
+	{0x4009, 0x0b},
+	{0x400c, 0x00},
+	{0x400d, 0x07},
+	{0x4507, 0x00},
+	{0x4509, 0x00},
+};
+
 static const struct ov9282_reg mode_1280x720_regs[] = {
 	{0x3778, 0x00},
 	{0x3800, 0x00},
@@ -282,6 +313,36 @@  static const struct ov9282_reg mode_1280x720_regs[] = {
 	{0x4509, 0x80},
 };
 
+static const struct ov9282_reg mode_640x400_regs[] = {
+	{0x3778, 0x10},
+	{0x3800, 0x00},
+	{0x3801, 0x00},
+	{0x3802, 0x00},
+	{0x3803, 0x00},
+	{0x3804, 0x05},
+	{0x3805, 0x0f},
+	{0x3806, 0x03},
+	{0x3807, 0x2f},
+	{0x3808, 0x02},
+	{0x3809, 0x80},
+	{0x380a, 0x01},
+	{0x380b, 0x90},
+	{0x3810, 0x00},
+	{0x3811, 0x04},
+	{0x3812, 0x00},
+	{0x3813, 0x04},
+	{0x3814, 0x31},
+	{0x3815, 0x22},
+	{0x3820, 0x60},
+	{0x3821, 0x01},
+	{0x4008, 0x02},
+	{0x4009, 0x05},
+	{0x400c, 0x00},
+	{0x400d, 0x03},
+	{0x4507, 0x03},
+	{0x4509, 0x80},
+};
+
 /* Supported sensor mode configurations */
 static const struct ov9282_mode supported_modes[] = {
 	{
@@ -306,6 +367,42 @@  static const struct ov9282_mode supported_modes[] = {
 			.num_of_regs = ARRAY_SIZE(mode_1280x720_regs),
 			.regs = mode_1280x720_regs,
 		},
+	}, {
+		.width = 1280,
+		.height = 800,
+		.hblank_min = { 250, 176 },
+		.vblank = 1022,
+		.vblank_min = 110,
+		.vblank_max = 51540,
+		.link_freq_idx = 0,
+		.crop = {
+			.left = OV9282_PIXEL_ARRAY_LEFT,
+			.top = OV9282_PIXEL_ARRAY_TOP,
+			.width = 1280,
+			.height = 800
+		},
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_1280x800_regs),
+			.regs = mode_1280x800_regs,
+		},
+	}, {
+		.width = 640,
+		.height = 400,
+		.hblank_min = { 890, 816 },
+		.vblank = 1022,
+		.vblank_min = 22,
+		.vblank_max = 51540,
+		.link_freq_idx = 0,
+		.crop = {
+			.left = OV9282_PIXEL_ARRAY_LEFT,
+			.top = OV9282_PIXEL_ARRAY_TOP,
+			.width = 1280,
+			.height = 800
+		},
+		.reg_list = {
+			.num_of_regs = ARRAY_SIZE(mode_640x400_regs),
+			.regs = mode_640x400_regs,
+		},
 	},
 };