Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
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
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
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
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
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,
>
[PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
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
---
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,
ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
720, 1896, 480, 984,
+5600,
ov5640_setting_30fps_NTSC_720_480,
ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
{OV5640_MODE_PAL_720_576,
