Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-20 Thread jacopo mondi
Hi Maxime,

On Tue, Nov 20, 2018 at 10:48:39AM +0100, Maxime Ripard wrote:
> Hi Jacopo,
>
> On Wed, Nov 14, 2018 at 08:48:47PM +0100, jacopo mondi wrote:
> > On Tue, Nov 13, 2018 at 02:03:15PM +0100, Maxime Ripard wrote:
> > > The clock structure for the PCLK is quite obscure in the documentation, 
> > > and
> > > was hardcoded through the bytes array of each and every mode.
> > >
> > > This is troublesome, since we cannot adjust it at runtime based on other
> > > parameters (such as the number of bytes per pixel), and we can't support
> > > either framerates that have not been used by the various vendors, since we
> > > don't have the needed initialization sequence.
> > >
> > > We can however understand how the clock tree works, and then implement 
> > > some
> > > functions to derive the various parameters from a given rate. And now that
> > > those parameters are calculated at runtime, we can remove them from the
> > > initialization sequence.
> > >
> > > The modes also gained a new parameter which is the clock that they are
> > > running at, from the register writes they were doing, so for now the 
> > > switch
> > > to the new algorithm should be transparent.
> > >
> > > Signed-off-by: Maxime Ripard 
> >
> > As you've squashed in my MIPI CSI-2 fixes, do you think it is
> > appropriate adding my So-b tag here?
>
> Yeah, I'll add your co-developped-by tag as well, if that's ok.
>

Yeah, whatever works for you here... Thanks ;)

> > > +/*
> > > + * This is supposed to be ranging from 1 to 16, but the value is
> > > + * always set to either 1 or 2 in the vendor kernels.
> > > + */
> >
> > I forgot to fix this comment in my patches you squahed in here (the
> > value now ranges from 1 to 16
>
> Ok.
>
> > > +#define OV5640_SYSDIV_MIN1
> > > +#define OV5640_SYSDIV_MAX16
> > > +
> > > +/*
> > > + * This is supposed to be ranging from 1 to 16, but the value is always
> > > + * set to 2 in the vendor kernels.
> > > + */
> > > +#define OV5640_MIPI_DIV  2
> > > +
> > > +/*
> > > + * This is supposed to be ranging from 1 to 2, but the value is always
> > > + * set to 2 in the vendor kernels.
> > > + */
> > > +#define OV5640_PLL_ROOT_DIV  2
> > > +#define OV5640_PLL_CTRL3_PLL_ROOT_DIV_2  BIT(4)
> > > +
> > > +/*
> > > + * We only supports 8-bit formats at the moment
> > > + */
> > > +#define OV5640_BIT_DIV   2
> > > +#define OV5640_PLL_CTRL0_MIPI_MODE_8BIT  0x08
> > > +
> > > +/*
> > > + * This is supposed to be ranging from 1 to 8, but the value is always
> > > + * set to 2 in the vendor kernels.
> > > + */
> > > +#define OV5640_SCLK_ROOT_DIV 2
> > > +
> > > +/*
> > > + * This is hardcoded so that the consistency is maintained between SCLK 
> > > and
> > > + * SCLK 2x.
> > > + */
> > > +#define OV5640_SCLK2X_ROOT_DIV (OV5640_SCLK_ROOT_DIV / 2)
> > > +
> > > +/*
> > > + * This is supposed to be ranging from 1 to 8, but the value is always
> > > + * set to 1 in the vendor kernels.
> > > + */
> > > +#define OV5640_PCLK_ROOT_DIV 1
> > > +#define OV5640_PLL_SYS_ROOT_DIVIDER_BYPASS   0x00
> > > +
> > > +static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> > > + u8 pll_prediv, u8 pll_mult,
> > > + u8 sysdiv)
> > > +{
> > > + unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
> > > +
> > > + /* PLL1 output cannot exceed 1GHz. */
> > > + if (sysclk / 100 > 1000)
> > > + return 0;
> > > +
> > > + return sysclk / sysdiv;
> > > +}
> >
> > That's better, but Sam's version is even better. I plan to pick some
> > part of his patch and apply them on top of this (for this and a few
> > other things I'm pointing out a here below). How would you like to
> > have those updates? Incremental patches on top of this series once it
> > gets merged (and it can now be merged, since it works for both CSI-2
> > and DVP), or would you like to receive those patches, squash them in
> > and re-send?
>
> The first solution would be better, having to constantly piling up
> patches in a series is a very efficient way to not get anything
> merged.
>

I know -.-

Fine, I'll have some more patches for ov5640 for next cycle then :)
(For this and all other changes to the CSI-2 portion of this patch)

> > > +
> > > +static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> > > +  unsigned long rate,
> > > +  u8 *pll_prediv, u8 *pll_mult,
> > > +  u8 *sysdiv)
> > > +{
> > > + unsigned long best = ~0;
> > > + u8 best_sysdiv = 1, best_mult = 1;
> > > + u8 _sysdiv, _pll_mult;
> > > +
> > > + for (_sysdiv = OV5640_SYSDIV_MIN;
> > > +  _sysdiv <= OV5640_SYSDIV_MAX;
> > > +  _sysdiv++) {
> > > + for (_pll_mult = OV5640_PLL_MULT_MIN;
> > > +  _pll_mult <= OV5640_PLL_MULT_MAX;
> > > +  

Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-20 Thread Maxime Ripard
Hi Jacopo,

On Wed, Nov 14, 2018 at 08:48:47PM +0100, jacopo mondi wrote:
> On Tue, Nov 13, 2018 at 02:03:15PM +0100, Maxime Ripard wrote:
> > The clock structure for the PCLK is quite obscure in the documentation, and
> > was hardcoded through the bytes array of each and every mode.
> >
> > This is troublesome, since we cannot adjust it at runtime based on other
> > parameters (such as the number of bytes per pixel), and we can't support
> > either framerates that have not been used by the various vendors, since we
> > don't have the needed initialization sequence.
> >
> > We can however understand how the clock tree works, and then implement some
> > functions to derive the various parameters from a given rate. And now that
> > those parameters are calculated at runtime, we can remove them from the
> > initialization sequence.
> >
> > The modes also gained a new parameter which is the clock that they are
> > running at, from the register writes they were doing, so for now the switch
> > to the new algorithm should be transparent.
> >
> > Signed-off-by: Maxime Ripard 
> 
> As you've squashed in my MIPI CSI-2 fixes, do you think it is
> appropriate adding my So-b tag here?

Yeah, I'll add your co-developped-by tag as well, if that's ok.

> > +/*
> > + * This is supposed to be ranging from 1 to 16, but the value is
> > + * always set to either 1 or 2 in the vendor kernels.
> > + */
> 
> I forgot to fix this comment in my patches you squahed in here (the
> value now ranges from 1 to 16

Ok.

> > +#define OV5640_SYSDIV_MIN  1
> > +#define OV5640_SYSDIV_MAX  16
> > +
> > +/*
> > + * This is supposed to be ranging from 1 to 16, but the value is always
> > + * set to 2 in the vendor kernels.
> > + */
> > +#define OV5640_MIPI_DIV2
> > +
> > +/*
> > + * This is supposed to be ranging from 1 to 2, but the value is always
> > + * set to 2 in the vendor kernels.
> > + */
> > +#define OV5640_PLL_ROOT_DIV2
> > +#define OV5640_PLL_CTRL3_PLL_ROOT_DIV_2BIT(4)
> > +
> > +/*
> > + * We only supports 8-bit formats at the moment
> > + */
> > +#define OV5640_BIT_DIV 2
> > +#define OV5640_PLL_CTRL0_MIPI_MODE_8BIT0x08
> > +
> > +/*
> > + * This is supposed to be ranging from 1 to 8, but the value is always
> > + * set to 2 in the vendor kernels.
> > + */
> > +#define OV5640_SCLK_ROOT_DIV   2
> > +
> > +/*
> > + * This is hardcoded so that the consistency is maintained between SCLK and
> > + * SCLK 2x.
> > + */
> > +#define OV5640_SCLK2X_ROOT_DIV (OV5640_SCLK_ROOT_DIV / 2)
> > +
> > +/*
> > + * This is supposed to be ranging from 1 to 8, but the value is always
> > + * set to 1 in the vendor kernels.
> > + */
> > +#define OV5640_PCLK_ROOT_DIV   1
> > +#define OV5640_PLL_SYS_ROOT_DIVIDER_BYPASS 0x00
> > +
> > +static unsigned long ov5640_compute_sys_clk(struct ov5640_dev *sensor,
> > +   u8 pll_prediv, u8 pll_mult,
> > +   u8 sysdiv)
> > +{
> > +   unsigned long sysclk = sensor->xclk_freq / pll_prediv * pll_mult;
> > +
> > +   /* PLL1 output cannot exceed 1GHz. */
> > +   if (sysclk / 100 > 1000)
> > +   return 0;
> > +
> > +   return sysclk / sysdiv;
> > +}
> 
> That's better, but Sam's version is even better. I plan to pick some
> part of his patch and apply them on top of this (for this and a few
> other things I'm pointing out a here below). How would you like to
> have those updates? Incremental patches on top of this series once it
> gets merged (and it can now be merged, since it works for both CSI-2
> and DVP), or would you like to receive those patches, squash them in
> and re-send?

The first solution would be better, having to constantly piling up
patches in a series is a very efficient way to not get anything
merged.

> > +
> > +static unsigned long ov5640_calc_sys_clk(struct ov5640_dev *sensor,
> > +unsigned long rate,
> > +u8 *pll_prediv, u8 *pll_mult,
> > +u8 *sysdiv)
> > +{
> > +   unsigned long best = ~0;
> > +   u8 best_sysdiv = 1, best_mult = 1;
> > +   u8 _sysdiv, _pll_mult;
> > +
> > +   for (_sysdiv = OV5640_SYSDIV_MIN;
> > +_sysdiv <= OV5640_SYSDIV_MAX;
> > +_sysdiv++) {
> > +   for (_pll_mult = OV5640_PLL_MULT_MIN;
> > +_pll_mult <= OV5640_PLL_MULT_MAX;
> > +_pll_mult++) {
> > +   unsigned long _rate;
> > +
> > +   /*
> > +* The PLL multiplier cannot be odd if above
> > +* 127.
> > +*/
> > +   if (_pll_mult > 127 && (_pll_mult % 2))
> > +   continue;
> > +
> > +   _rate = ov5640_compute_sys_clk(sensor,
> > +  OV5640_PLL_PREDIV,
> > + 

Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-19 Thread Adam Ford
On Wed, Nov 14, 2018 at 7:20 AM Adam Ford  wrote:
>
> On Tue, Nov 13, 2018 at 7:04 AM Maxime Ripard  
> wrote:
> >
> > The clock structure for the PCLK is quite obscure in the documentation, and
> > was hardcoded through the bytes array of each and every mode.
> >
> > This is troublesome, since we cannot adjust it at runtime based on other
> > parameters (such as the number of bytes per pixel), and we can't support
> > either framerates that have not been used by the various vendors, since we
> > don't have the needed initialization sequence.
> >
> > We can however understand how the clock tree works, and then implement some
> > functions to derive the various parameters from a given rate. And now that
> > those parameters are calculated at runtime, we can remove them from the
> > initialization sequence.
> >
> > The modes also gained a new parameter which is the clock that they are
> > running at, from the register writes they were doing, so for now the switch
> > to the new algorithm should be transparent.
> >
> > Signed-off-by: Maxime Ripard 
>
> Thanks for the patches! I tested the whole series.  I am stil learning
> the v4l2 stuff, but I'm trying to test what and where I can.
> media-ctl shows the camera is talking at 60fps, but my imx6 is only
> capturing at 30, but I don't think it's the fault of the ov5640
> driver.
>
> Tested-by: Adam Ford  #imx6dq
>

For what it's worth, I would I like to request that this series be
applied to 4.20 fixes (possibly 4.19 if appropriate) as it fixes some
issues where I am trying to capture JPEG images that previously came
up garbled, and now they are clear.  The only change to my kernel is
this series.

adam

> adam
> > ---
> >  drivers/media/i2c/ov5640.c | 366 -
> >  1 file changed, 365 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index eaefdb58653b..8476f85bb8e7 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> > u32 htot;
> > u32 vact;
> > u32 vtot;
> > +   u32 pixel_clock;
> > const struct reg_value *reg_data;
> > u32 reg_data_size;
> >  };
> > @@ -700,6 +701,7 @@ static const struct reg_value 
> > ov5640_setting_15fps_QSXGA_2592_1944[] = {
> >  /* power-on sensor init reg table */
> >  static const struct ov5640_mode_info ov5640_mode_init_data = {
> > 0, SUBSAMPLING, 640, 1896, 480, 984,
> > +   5600,
> > ov5640_init_setting_30fps_VGA,
> > ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
> >  };
> > @@ -709,74 +711,91 @@ 
> > ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> > {
> > {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
> >  176, 1896, 144, 984,
> > +2800,
> >  ov5640_setting_15fps_QCIF_176_144,
> >  ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> > {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> >  320, 1896, 240, 984,
> > +2800,
> >  ov5640_setting_15fps_QVGA_320_240,
> >  ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> > {OV5640_MODE_VGA_640_480, SUBSAMPLING,
> >  640, 1896, 480, 1080,
> > +2800,
> >  ov5640_setting_15fps_VGA_640_480,
> >  ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> > {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> >  720, 1896, 480, 984,
> > +2800,
> >  ov5640_setting_15fps_NTSC_720_480,
> >  ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> > {OV5640_MODE_PAL_720_576, SUBSAMPLING,
> >  720, 1896, 576, 984,
> > +2800,
> >  ov5640_setting_15fps_PAL_720_576,
> >  ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> > {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> >  1024, 1896, 768, 1080,
> > +2800,
> >  ov5640_setting_15fps_XGA_1024_768,
> >  ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> > {OV5640_MODE_720P_1280_720, SUBSAMPLING,
> >  1280, 1892, 720, 740,
> > +2100,
> >  ov5640_setting_15fps_720P_1280_720,
> >  ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> > {OV5640_MODE_1080P_1920_1080, SCALING,
> >  1920, 2500, 1080, 1120,
> > +4200,
> >  ov5640_setting_15fps_1080P_1920_1080,
> >  ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> > {OV5640_MODE_QSXGA_2592_1944, SCALING,
> >  2592, 2844, 1944, 1968,
> > +8400,
> >  

Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-14 Thread jacopo mondi
Hi Maxime,
   many thanks for re-sending this updated version

On Tue, Nov 13, 2018 at 02:03:15PM +0100, Maxime Ripard wrote:
> The clock structure for the PCLK is quite obscure in the documentation, and
> was hardcoded through the bytes array of each and every mode.
>
> This is troublesome, since we cannot adjust it at runtime based on other
> parameters (such as the number of bytes per pixel), and we can't support
> either framerates that have not been used by the various vendors, since we
> don't have the needed initialization sequence.
>
> We can however understand how the clock tree works, and then implement some
> functions to derive the various parameters from a given rate. And now that
> those parameters are calculated at runtime, we can remove them from the
> initialization sequence.
>
> The modes also gained a new parameter which is the clock that they are
> running at, from the register writes they were doing, so for now the switch
> to the new algorithm should be transparent.
>
> Signed-off-by: Maxime Ripard 

As you've squashed in my MIPI CSI-2 fixes, do you think it is
appropriate adding my So-b tag here?

> ---
>  drivers/media/i2c/ov5640.c | 366 -
>  1 file changed, 365 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb58653b..8476f85bb8e7 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -175,6 +175,7 @@ struct ov5640_mode_info {
>   u32 htot;
>   u32 vact;
>   u32 vtot;
> + u32 pixel_clock;
>   const struct reg_value *reg_data;
>   u32 reg_data_size;
>  };
> @@ -700,6 +701,7 @@ static const struct reg_value 
> ov5640_setting_15fps_QSXGA_2592_1944[] = {
>  /* power-on sensor init reg table */
>  static const struct ov5640_mode_info ov5640_mode_init_data = {
>   0, SUBSAMPLING, 640, 1896, 480, 984,
> + 5600,
>   ov5640_init_setting_30fps_VGA,
>   ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
>  };
> @@ -709,74 +711,91 @@ 
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
>   {
>   {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> +  2800,
>ov5640_setting_15fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
>   {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> +  2800,
>ov5640_setting_15fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
>   {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> +  2800,
>ov5640_setting_15fps_VGA_640_480,
>ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
>   {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>720, 1896, 480, 984,
> +  2800,
>ov5640_setting_15fps_NTSC_720_480,
>ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
>   {OV5640_MODE_PAL_720_576, SUBSAMPLING,
>720, 1896, 576, 984,
> +  2800,
>ov5640_setting_15fps_PAL_720_576,
>ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
>   {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>1024, 1896, 768, 1080,
> +  2800,
>ov5640_setting_15fps_XGA_1024_768,
>ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
>   {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>1280, 1892, 720, 740,
> +  2100,
>ov5640_setting_15fps_720P_1280_720,
>ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
>   {OV5640_MODE_1080P_1920_1080, SCALING,
>1920, 2500, 1080, 1120,
> +  4200,
>ov5640_setting_15fps_1080P_1920_1080,
>ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
>   {OV5640_MODE_QSXGA_2592_1944, SCALING,
>2592, 2844, 1944, 1968,
> +  8400,
>ov5640_setting_15fps_QSXGA_2592_1944,
>ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
>   }, {
>   {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>176, 1896, 144, 984,
> +  5600,
>ov5640_setting_30fps_QCIF_176_144,
>ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
>   {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>320, 1896, 240, 984,
> +  5600,
>ov5640_setting_30fps_QVGA_320_240,
>ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
>   {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>640, 1896, 480, 1080,
> +  5600,
>ov5640_setting_30fps_VGA_640_480,
>

Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate

2018-11-14 Thread Adam Ford
On Tue, Nov 13, 2018 at 7:04 AM Maxime Ripard  wrote:
>
> The clock structure for the PCLK is quite obscure in the documentation, and
> was hardcoded through the bytes array of each and every mode.
>
> This is troublesome, since we cannot adjust it at runtime based on other
> parameters (such as the number of bytes per pixel), and we can't support
> either framerates that have not been used by the various vendors, since we
> don't have the needed initialization sequence.
>
> We can however understand how the clock tree works, and then implement some
> functions to derive the various parameters from a given rate. And now that
> those parameters are calculated at runtime, we can remove them from the
> initialization sequence.
>
> The modes also gained a new parameter which is the clock that they are
> running at, from the register writes they were doing, so for now the switch
> to the new algorithm should be transparent.
>
> Signed-off-by: Maxime Ripard 

Thanks for the patches! I tested the whole series.  I am stil learning
the v4l2 stuff, but I'm trying to test what and where I can.
media-ctl shows the camera is talking at 60fps, but my imx6 is only
capturing at 30, but I don't think it's the fault of the ov5640
driver.

Tested-by: Adam Ford  #imx6dq

adam
> ---
>  drivers/media/i2c/ov5640.c | 366 -
>  1 file changed, 365 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index eaefdb58653b..8476f85bb8e7 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -175,6 +175,7 @@ struct ov5640_mode_info {
> u32 htot;
> u32 vact;
> u32 vtot;
> +   u32 pixel_clock;
> const struct reg_value *reg_data;
> u32 reg_data_size;
>  };
> @@ -700,6 +701,7 @@ static const struct reg_value 
> ov5640_setting_15fps_QSXGA_2592_1944[] = {
>  /* power-on sensor init reg table */
>  static const struct ov5640_mode_info ov5640_mode_init_data = {
> 0, SUBSAMPLING, 640, 1896, 480, 984,
> +   5600,
> ov5640_init_setting_30fps_VGA,
> ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
>  };
> @@ -709,74 +711,91 @@ 
> ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>  176, 1896, 144, 984,
> +2800,
>  ov5640_setting_15fps_QCIF_176_144,
>  ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>  320, 1896, 240, 984,
> +2800,
>  ov5640_setting_15fps_QVGA_320_240,
>  ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> {OV5640_MODE_VGA_640_480, SUBSAMPLING,
>  640, 1896, 480, 1080,
> +2800,
>  ov5640_setting_15fps_VGA_640_480,
>  ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> {OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>  720, 1896, 480, 984,
> +2800,
>  ov5640_setting_15fps_NTSC_720_480,
>  ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> {OV5640_MODE_PAL_720_576, SUBSAMPLING,
>  720, 1896, 576, 984,
> +2800,
>  ov5640_setting_15fps_PAL_720_576,
>  ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> {OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>  1024, 1896, 768, 1080,
> +2800,
>  ov5640_setting_15fps_XGA_1024_768,
>  ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> {OV5640_MODE_720P_1280_720, SUBSAMPLING,
>  1280, 1892, 720, 740,
> +2100,
>  ov5640_setting_15fps_720P_1280_720,
>  ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> {OV5640_MODE_1080P_1920_1080, SCALING,
>  1920, 2500, 1080, 1120,
> +4200,
>  ov5640_setting_15fps_1080P_1920_1080,
>  ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> {OV5640_MODE_QSXGA_2592_1944, SCALING,
>  2592, 2844, 1944, 1968,
> +8400,
>  ov5640_setting_15fps_QSXGA_2592_1944,
>  ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
> }, {
> {OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>  176, 1896, 144, 984,
> +5600,
>  ov5640_setting_30fps_QCIF_176_144,
>  ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
> {OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>  320, 1896, 240, 984,
> +5600,
>  ov5640_setting_30fps_QVGA_320_240,
>