Re: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
On Mon, Feb 05, 2018 at 11:42:11AM +, Fabrizio Castro wrote: > Hello Maxime, > > thank you for your feedback. > > > > > +/* > > > > + * configure parallel port control lines polarity > > > > + * > > > > + * POLARITY CTRL0 > > > > + * - [5]:PCLK polarity (0: active low, 1: active high) > > > > + * - [1]:HREF polarity (0: active low, 1: active high) > > > > + * - [0]:VSYNC polarity (mismatch here between > > > > + *datasheet and hardware, 0 is active high > > > > + *and 1 is active low...) > > > > > > I know that yourself and Maxime have both confirmed that VSYNC > > > polarity is inverted, but I am looking at HSYNC and VSYNC with a > > > logic analyser and I am dumping the values written to > > > OV5640_REG_POLARITY_CTRL00 and to me it looks like that HSYNC is > > > active HIGH when hsync_pol == 0, and VSYNC is active HIGH when > > > vsync_pol == 1. > > > > Which mode are you testing this on? > > My testing environment is made of the sensor connected to a SoC with > 8 data lines, vsync, hsync, and pclk, and of course I am specifying > hsync-active, vsync-active, and pclk-sample in the device tree on > both ends so that they configure themselves accordingly to work in > DVP mode (V4L2_MBUS_PARALLEL), with the correct polarities. > > Between the sensor and the SoC there is a noninverting bus > transceiver (voltage translator), for my experiments I have plugged > myself onto the outputs of this transceiver only to be compliant > with the voltage level of my logic analyser. Sorry, my question was more about the resolution, refresh rate, etc. > > The non-active periods are insanely high in most modes (1896 for an > > active horizontal length of 640 in 640x480 for example), especially > > hsync, and it's really easy to invert the two. > > I am looking at all the data lines, so that I don't confuse the > non-active periods with the active periods, and with my setup what I > reported is what I get. I wonder if this difference comes from the > sensor revision at all? Unfortunately I can only test the one camera > I have got. I don't really know then. I've had issues with the polarities on my side, but it was on the receiver side and the sensor part looked like what is documented. Maxime -- Maxime Ripard, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com signature.asc Description: PGP signature
RE: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
Hello Maxime, thank you for your feedback. > > > +/* > > > + * configure parallel port control lines polarity > > > + * > > > + * POLARITY CTRL0 > > > + * - [5]:PCLK polarity (0: active low, 1: active high) > > > + * - [1]:HREF polarity (0: active low, 1: active high) > > > + * - [0]:VSYNC polarity (mismatch here between > > > + *datasheet and hardware, 0 is active high > > > + *and 1 is active low...) > > > > I know that yourself and Maxime have both confirmed that VSYNC > > polarity is inverted, but I am looking at HSYNC and VSYNC with a > > logic analyser and I am dumping the values written to > > OV5640_REG_POLARITY_CTRL00 and to me it looks like that HSYNC is > > active HIGH when hsync_pol == 0, and VSYNC is active HIGH when > > vsync_pol == 1. > > Which mode are you testing this on? My testing environment is made of the sensor connected to a SoC with 8 data lines, vsync, hsync, and pclk, and of course I am specifying hsync-active, vsync-active, and pclk-sample in the device tree on both ends so that they configure themselves accordingly to work in DVP mode (V4L2_MBUS_PARALLEL), with the correct polarities. Between the sensor and the SoC there is a noninverting bus transceiver (voltage translator), for my experiments I have plugged myself onto the outputs of this transceiver only to be compliant with the voltage level of my logic analyser. > > The non-active periods are insanely high in most modes (1896 for an > active horizontal length of 640 in 640x480 for example), especially > hsync, and it's really easy to invert the two. I am looking at all the data lines, so that I don't confuse the non-active periods with the active periods, and with my setup what I reported is what I get. I wonder if this difference comes from the sensor revision at all? Unfortunately I can only test the one camera I have got. Thanks, Fab > > Maxime > > -- > Maxime Ripard, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > http://bootlin.com Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.
Re: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
Hi Fabrizio,
On Thu, Feb 01, 2018 at 05:53:18PM +, Fabrizio Castro wrote:
> > Subject: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
> >
> > Add support of DVP parallel mode in addition of
> > existing MIPI CSI mode. The choice between two modes
> > and configuration is made through device tree.
> >
> > Signed-off-by: Hugues Fruchet
> > ---
> > drivers/media/i2c/ov5640.c | 148
> > +++--
> > 1 file changed, 130 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> > index 9f031f3..a44b680 100644
> > --- a/drivers/media/i2c/ov5640.c
> > +++ b/drivers/media/i2c/ov5640.c
> > @@ -34,13 +34,19 @@
> >
> > #define OV5640_DEFAULT_SLAVE_ID 0x3c
> >
> > +#define OV5640_REG_SYS_CTRL00x3008
> > #define OV5640_REG_CHIP_ID0x300a
> > +#define OV5640_REG_IO_MIPI_CTRL000x300e
> > +#define OV5640_REG_PAD_OUTPUT_ENABLE010x3017
> > +#define OV5640_REG_PAD_OUTPUT_ENABLE020x3018
> > #define OV5640_REG_PAD_OUTPUT000x3019
> > +#define OV5640_REG_SYSTEM_CONTROL10x302e
> > #define OV5640_REG_SC_PLL_CTRL00x3034
> > #define OV5640_REG_SC_PLL_CTRL10x3035
> > #define OV5640_REG_SC_PLL_CTRL20x3036
> > #define OV5640_REG_SC_PLL_CTRL30x3037
> > #define OV5640_REG_SLAVE_ID0x3100
> > +#define OV5640_REG_SCCB_SYS_CTRL10x3103
> > #define OV5640_REG_SYS_ROOT_DIVIDER0x3108
> > #define OV5640_REG_AWB_R_GAIN0x3400
> > #define OV5640_REG_AWB_G_GAIN0x3402
> > @@ -70,6 +76,7 @@
> > #define OV5640_REG_HZ5060_CTRL010x3c01
> > #define OV5640_REG_SIGMADELTA_CTRL0C0x3c0c
> > #define OV5640_REG_FRAME_CTRL010x4202
> > +#define OV5640_REG_POLARITY_CTRL000x4740
> > #define OV5640_REG_MIPI_CTRL000x4800
> > #define OV5640_REG_DEBUG_MODE0x4814
> > #define OV5640_REG_PRE_ISP_TEST_SET10x503d
> > @@ -982,7 +989,111 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
> > return gain & 0x3ff;
> > }
> >
> > -static int ov5640_set_stream(struct ov5640_dev *sensor, bool on)
> > +static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> > +{
> > +int ret;
> > +unsigned int flags = sensor->ep.bus.parallel.flags;
> > +u8 pclk_pol = 0;
> > +u8 hsync_pol = 0;
> > +u8 vsync_pol = 0;
> > +
> > +/*
> > + * Note about parallel port configuration.
> > + *
> > + * When configured in parallel mode, the OV5640 will
> > + * output 10 bits data on DVP data lines [9:0].
> > + * If only 8 bits data are wanted, the 8 bits data lines
> > + * of the camera interface must be physically connected
> > + * on the DVP data lines [9:2].
> > + *
> > + * Control lines polarity can be configured through
> > + * devicetree endpoint control lines properties.
> > + * If no endpoint control lines properties are set,
> > + * polarity will be as below:
> > + * - VSYNC:active high
> > + * - HREF:active low
> > + * - PCLK:active low
> > + */
> > +
> > +if (on) {
> > +/*
> > + * reset MIPI PCLK/SERCLK divider
> > + *
> > + * SC PLL CONTRL1 0
> > + * - [3..0]:MIPI PCLK/SERCLK divider
> > + */
> > +ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0);
> > +if (ret)
> > +return ret;
> > +
> > +/*
> > + * configure parallel port control lines polarity
> > + *
> > + * POLARITY CTRL0
> > + * - [5]:PCLK polarity (0: active low, 1: active high)
> > + * - [1]:HREF polarity (0: active low, 1: active high)
> > + * - [0]:VSYNC polarity (mismatch here between
> > + *datasheet and hardware, 0 is active high
> > + *and 1 is active low...)
>
> I know that yourself and Maxime have both confirmed that VSYNC
> polarity is inverted, but I am looking at HSYNC and VSYNC with a
> logic analyser and I am dumping the values written to
> OV5640_REG_POLARITY_CTRL00 and to me it looks like that HSYNC is
> active HIGH when hsync_pol == 0, and VSYNC is active HIGH when
> vsync_pol == 1.
Which mode are you testing this on?
The non-active periods are insanely high in most modes (1896 for an
active horizontal length of 640 in 640x480 for example), especially
hsync, and it's really easy to invert the two.
Maxime
--
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com
signature.asc
Description: PGP signature
RE: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
Hello Hugues,
> Subject: [PATCH v5 4/5] media: ov5640: add support of DVP parallel interface
>
> Add support of DVP parallel mode in addition of
> existing MIPI CSI mode. The choice between two modes
> and configuration is made through device tree.
>
> Signed-off-by: Hugues Fruchet
> ---
> drivers/media/i2c/ov5640.c | 148
> +++--
> 1 file changed, 130 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 9f031f3..a44b680 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -34,13 +34,19 @@
>
> #define OV5640_DEFAULT_SLAVE_ID 0x3c
>
> +#define OV5640_REG_SYS_CTRL00x3008
> #define OV5640_REG_CHIP_ID0x300a
> +#define OV5640_REG_IO_MIPI_CTRL000x300e
> +#define OV5640_REG_PAD_OUTPUT_ENABLE010x3017
> +#define OV5640_REG_PAD_OUTPUT_ENABLE020x3018
> #define OV5640_REG_PAD_OUTPUT000x3019
> +#define OV5640_REG_SYSTEM_CONTROL10x302e
> #define OV5640_REG_SC_PLL_CTRL00x3034
> #define OV5640_REG_SC_PLL_CTRL10x3035
> #define OV5640_REG_SC_PLL_CTRL20x3036
> #define OV5640_REG_SC_PLL_CTRL30x3037
> #define OV5640_REG_SLAVE_ID0x3100
> +#define OV5640_REG_SCCB_SYS_CTRL10x3103
> #define OV5640_REG_SYS_ROOT_DIVIDER0x3108
> #define OV5640_REG_AWB_R_GAIN0x3400
> #define OV5640_REG_AWB_G_GAIN0x3402
> @@ -70,6 +76,7 @@
> #define OV5640_REG_HZ5060_CTRL010x3c01
> #define OV5640_REG_SIGMADELTA_CTRL0C0x3c0c
> #define OV5640_REG_FRAME_CTRL010x4202
> +#define OV5640_REG_POLARITY_CTRL000x4740
> #define OV5640_REG_MIPI_CTRL000x4800
> #define OV5640_REG_DEBUG_MODE0x4814
> #define OV5640_REG_PRE_ISP_TEST_SET10x503d
> @@ -982,7 +989,111 @@ static int ov5640_get_gain(struct ov5640_dev *sensor)
> return gain & 0x3ff;
> }
>
> -static int ov5640_set_stream(struct ov5640_dev *sensor, bool on)
> +static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on)
> +{
> +int ret;
> +unsigned int flags = sensor->ep.bus.parallel.flags;
> +u8 pclk_pol = 0;
> +u8 hsync_pol = 0;
> +u8 vsync_pol = 0;
> +
> +/*
> + * Note about parallel port configuration.
> + *
> + * When configured in parallel mode, the OV5640 will
> + * output 10 bits data on DVP data lines [9:0].
> + * If only 8 bits data are wanted, the 8 bits data lines
> + * of the camera interface must be physically connected
> + * on the DVP data lines [9:2].
> + *
> + * Control lines polarity can be configured through
> + * devicetree endpoint control lines properties.
> + * If no endpoint control lines properties are set,
> + * polarity will be as below:
> + * - VSYNC:active high
> + * - HREF:active low
> + * - PCLK:active low
> + */
> +
> +if (on) {
> +/*
> + * reset MIPI PCLK/SERCLK divider
> + *
> + * SC PLL CONTRL1 0
> + * - [3..0]:MIPI PCLK/SERCLK divider
> + */
> +ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * configure parallel port control lines polarity
> + *
> + * POLARITY CTRL0
> + * - [5]:PCLK polarity (0: active low, 1: active high)
> + * - [1]:HREF polarity (0: active low, 1: active high)
> + * - [0]:VSYNC polarity (mismatch here between
> + *datasheet and hardware, 0 is active high
> + *and 1 is active low...)
I know that yourself and Maxime have both confirmed that VSYNC polarity is
inverted, but I am looking at HSYNC and VSYNC with a logic analyser and I am
dumping the values written to OV5640_REG_POLARITY_CTRL00 and to me it looks
like that HSYNC is active HIGH when hsync_pol == 0, and VSYNC is active HIGH
when vsync_pol == 1.
Between the SoC and the sensor I have a voltage translator, 2.8V input -> 3.3V
output, I am measuring the signals at the translator outputs.
Register 0x302A (chip revision register) on my sensor contains 0xb0.
Could you please double check this? How is it possible that this works
differently on my sensor?
Thanks,
Fabrizio
> + */
> +if (flags & V4L2_MBUS_PCLK_SAMPLE_RISING)
> +pclk_pol = 1;
> +if (flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH)
> +hsync_pol = 1;
> +if (flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)
> +vsync_pol = 1;
> +
> +ret = ov5640_write_reg(sensor,
> + OV5640_REG_POLARITY_CTRL00,
> + (pclk_pol << 5) |
> + (hsync_pol << 1) |
> + vsync_pol);
> +
> +if (ret)
> +return ret;
> +}
> +
> +/*
> + * powerdown MIPI TX/RX PHY & disable MIPI
> + *
> + * MIPI CONTROL 00
> + * 4: PWDN PHY TX
> + * 3: PWDN PHY RX
> + * 2: MIPI enable
> + */
> +ret = ov5640_write_reg(sensor,
> + OV5640_REG_IO_MIPI_CTRL00, on ? 0x18 : 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * enable VSYNC/HREF/PCLK DVP control lines
> + * & D[9:6] DVP data lines
> + *
> + * PAD OUTPUT ENABLE 01
> + * - 6:VSYNC output enable
> + * - 5:HREF output enable
> + * - 4:PCLK output enable
> + * - [3:0]:D[9:6] output enable
> + */
> +ret = ov5640_write_reg(sensor,
> + OV5640_REG_PAD_OUTPUT_ENABLE01,
> + on ? 0x7f : 0);
> +if (ret)
> +return ret;
> +
> +/*
> + * enable D[5:0] DVP data lines
> + *
> + * PAD OUTPUT ENABLE 02
> + * - [7:2]:D[5:0] output
