diff mbox series

[v2,16/30] media: ov5647: Rename SBGGR8 VGA mode

Message ID 20201104103622.595908-17-jacopo@jmondi.org
State Superseded
Headers show
Series media: ov5647: Support RaspberryPi Camera Module v1 | expand

Commit Message

Jacopo Mondi Nov. 4, 2020, 10:36 a.m. UTC
Before adding new modes, rename the only existing one to report
the media bus format in the name to distinguish it from future
additions.

While at it, briefly describe the mode.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 drivers/media/i2c/ov5647.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Dave Stevenson Nov. 4, 2020, 2:52 p.m. UTC | #1
Hi Jacopo

On Wed, 4 Nov 2020 at 10:37, Jacopo Mondi <jacopo@jmondi.org> wrote:
>

> Before adding new modes, rename the only existing one to report

> the media bus format in the name to distinguish it from future

> additions.


You are aware that if flips get added into the mix then they alter the
Bayer order, and therefore the media bus format will change?
Adding the bit depth makes sense, but adding the Bayer order less so.

  Dave

> While at it, briefly describe the mode.

>

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

> ---

>  drivers/media/i2c/ov5647.c | 13 +++++++------

>  1 file changed, 7 insertions(+), 6 deletions(-)

>

> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c

> index 65fcd86dcba96..9cbe3b675fb52 100644

> --- a/drivers/media/i2c/ov5647.c

> +++ b/drivers/media/i2c/ov5647.c

> @@ -115,7 +115,7 @@ static struct regval_list sensor_oe_enable_regs[] = {

>         {0x3002, 0xe4},

>  };

>

> -static const struct regval_list ov5647_640x480[] = {

> +static const struct regval_list ov5647_640x480_sbggr8[] = {

>         {0x0100, 0x00},

>         {0x0103, 0x01},

>         {0x3034, 0x08},

> @@ -205,7 +205,8 @@ static const struct regval_list ov5647_640x480[] = {

>         {0x0100, 0x01},

>  };

>

> -static const struct ov5647_mode ov5647_8bit_modes[] = {

> +static const struct ov5647_mode ov5647_sbggr8_modes[] = {

> +       /* 8-bit VGA mode: Uncentred crop 2x2 binned 1296x972 image. */

>         {

>                 .format = {

>                         .code           = MEDIA_BUS_FMT_SBGGR8_1X8,

> @@ -220,16 +221,16 @@ static const struct ov5647_mode ov5647_8bit_modes[] = {

>                         .width          = 1280,

>                         .height         = 960,

>                 },

> -               .reg_list       = ov5647_640x480,

> -               .num_regs       = ARRAY_SIZE(ov5647_640x480)

> +               .reg_list       = ov5647_640x480_sbggr8,

> +               .num_regs       = ARRAY_SIZE(ov5647_640x480_sbggr8)

>         },

>  };

>

>  static const struct ov5647_format_list ov5647_formats[] = {

>         {

>                 .mbus_code      = MEDIA_BUS_FMT_SBGGR8_1X8,

> -               .modes          = ov5647_8bit_modes,

> -               .num_modes      = ARRAY_SIZE(ov5647_8bit_modes),

> +               .modes          = ov5647_sbggr8_modes,

> +               .num_modes      = ARRAY_SIZE(ov5647_sbggr8_modes),

>         },

>  };

>

> --

> 2.29.1

>
Jacopo Mondi Nov. 6, 2020, 8:41 a.m. UTC | #2
Hi Dave,

On Wed, Nov 04, 2020 at 02:52:17PM +0000, Dave Stevenson wrote:
> Hi Jacopo
>
> On Wed, 4 Nov 2020 at 10:37, Jacopo Mondi <jacopo@jmondi.org> wrote:
> >
> > Before adding new modes, rename the only existing one to report
> > the media bus format in the name to distinguish it from future
> > additions.
>
> You are aware that if flips get added into the mix then they alter the
> Bayer order, and therefore the media bus format will change?
> Adding the bit depth makes sense, but adding the Bayer order less so.

mmm, adding the sensor's Bayer pattern to the modes list has a limited
value and might be misleading, you're right.

I'll drop it, thanks


>
>   Dave
>
> > While at it, briefly describe the mode.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  drivers/media/i2c/ov5647.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> > index 65fcd86dcba96..9cbe3b675fb52 100644
> > --- a/drivers/media/i2c/ov5647.c
> > +++ b/drivers/media/i2c/ov5647.c
> > @@ -115,7 +115,7 @@ static struct regval_list sensor_oe_enable_regs[] = {
> >         {0x3002, 0xe4},
> >  };
> >
> > -static const struct regval_list ov5647_640x480[] = {
> > +static const struct regval_list ov5647_640x480_sbggr8[] = {
> >         {0x0100, 0x00},
> >         {0x0103, 0x01},
> >         {0x3034, 0x08},
> > @@ -205,7 +205,8 @@ static const struct regval_list ov5647_640x480[] = {
> >         {0x0100, 0x01},
> >  };
> >
> > -static const struct ov5647_mode ov5647_8bit_modes[] = {
> > +static const struct ov5647_mode ov5647_sbggr8_modes[] = {
> > +       /* 8-bit VGA mode: Uncentred crop 2x2 binned 1296x972 image. */
> >         {
> >                 .format = {
> >                         .code           = MEDIA_BUS_FMT_SBGGR8_1X8,
> > @@ -220,16 +221,16 @@ static const struct ov5647_mode ov5647_8bit_modes[] = {
> >                         .width          = 1280,
> >                         .height         = 960,
> >                 },
> > -               .reg_list       = ov5647_640x480,
> > -               .num_regs       = ARRAY_SIZE(ov5647_640x480)
> > +               .reg_list       = ov5647_640x480_sbggr8,
> > +               .num_regs       = ARRAY_SIZE(ov5647_640x480_sbggr8)
> >         },
> >  };
> >
> >  static const struct ov5647_format_list ov5647_formats[] = {
> >         {
> >                 .mbus_code      = MEDIA_BUS_FMT_SBGGR8_1X8,
> > -               .modes          = ov5647_8bit_modes,
> > -               .num_modes      = ARRAY_SIZE(ov5647_8bit_modes),
> > +               .modes          = ov5647_sbggr8_modes,
> > +               .num_modes      = ARRAY_SIZE(ov5647_sbggr8_modes),
> >         },
> >  };
> >
> > --
> > 2.29.1
> >
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 65fcd86dcba96..9cbe3b675fb52 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -115,7 +115,7 @@  static struct regval_list sensor_oe_enable_regs[] = {
 	{0x3002, 0xe4},
 };
 
-static const struct regval_list ov5647_640x480[] = {
+static const struct regval_list ov5647_640x480_sbggr8[] = {
 	{0x0100, 0x00},
 	{0x0103, 0x01},
 	{0x3034, 0x08},
@@ -205,7 +205,8 @@  static const struct regval_list ov5647_640x480[] = {
 	{0x0100, 0x01},
 };
 
-static const struct ov5647_mode ov5647_8bit_modes[] = {
+static const struct ov5647_mode ov5647_sbggr8_modes[] = {
+	/* 8-bit VGA mode: Uncentred crop 2x2 binned 1296x972 image. */
 	{
 		.format	= {
 			.code		= MEDIA_BUS_FMT_SBGGR8_1X8,
@@ -220,16 +221,16 @@  static const struct ov5647_mode ov5647_8bit_modes[] = {
 			.width		= 1280,
 			.height		= 960,
 		},
-		.reg_list	= ov5647_640x480,
-		.num_regs	= ARRAY_SIZE(ov5647_640x480)
+		.reg_list	= ov5647_640x480_sbggr8,
+		.num_regs	= ARRAY_SIZE(ov5647_640x480_sbggr8)
 	},
 };
 
 static const struct ov5647_format_list ov5647_formats[] = {
 	{
 		.mbus_code	= MEDIA_BUS_FMT_SBGGR8_1X8,
-		.modes		= ov5647_8bit_modes,
-		.num_modes	= ARRAY_SIZE(ov5647_8bit_modes),
+		.modes		= ov5647_sbggr8_modes,
+		.num_modes	= ARRAY_SIZE(ov5647_sbggr8_modes),
 	},
 };