Hi Steve,
On 12/03/2017 10:58 PM, Steve Longerbeam wrote:
>
>
> On 11/29/2017 09:11 AM, Hugues Fruchet wrote:
>> 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 <hugues.fruchet@xxxxxx>
>> ---
>> drivers/media/i2c/ov5640.c | 101
>> +++++++++++++++++++++++++++++++++++++--------
>> 1 file changed, 83 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
>> index a576d11..826b102 100644
>> --- a/drivers/media/i2c/ov5640.c
>> +++ b/drivers/media/i2c/ov5640.c
>> @@ -34,14 +34,20 @@
>> #define OV5640_DEFAULT_SLAVE_ID 0x3c
>> +#define OV5640_REG_SYS_CTRL0 0x3008
>> #define OV5640_REG_CHIP_ID_HIGH 0x300a
>> #define OV5640_REG_CHIP_ID_LOW 0x300b
>> +#define OV5640_REG_IO_MIPI_CTRL00 0x300e
>> +#define OV5640_REG_PAD_OUTPUT_ENABLE01 0x3017
>> +#define OV5640_REG_PAD_OUTPUT_ENABLE02 0x3018
>> #define OV5640_REG_PAD_OUTPUT00 0x3019
>> +#define OV5640_REG_SYSTEM_CONTROL1 0x302e
>> #define OV5640_REG_SC_PLL_CTRL0 0x3034
>> #define OV5640_REG_SC_PLL_CTRL1 0x3035
>> #define OV5640_REG_SC_PLL_CTRL2 0x3036
>> #define OV5640_REG_SC_PLL_CTRL3 0x3037
>> #define OV5640_REG_SLAVE_ID 0x3100
>> +#define OV5640_REG_SCCB_SYS_CTRL1 0x3103
>> #define OV5640_REG_SYS_ROOT_DIVIDER 0x3108
>> #define OV5640_REG_AWB_R_GAIN 0x3400
>> #define OV5640_REG_AWB_G_GAIN 0x3402
>> @@ -1006,7 +1012,65 @@ 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;
>> +
>> + 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, 0xF, 0);
>> + 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:2] DVP data lines (D[0:1] are unused with 8 bits
>> + * parallel mode, 8 bits output are mapped on D[9:2])
>> + *
>> + * PAD OUTPUT ENABLE 02
>> + * - [7:4]: D[5:2] output enable
>> + * 0:1 are unused with 8 bits
>> + * parallel mode (8 bits output
>> + * are on D[9:2])
>> + */
>
> It should be verified in this driver, at probe, that the device tree
> endpoint for the OV5640 output parallel interface has specified this
> with "bus-width=<8>; data-shift=<2>;"
>
> Steve
>
I have changed the code enabling the whole 10 bits:
/*
* 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);
doing so, no need to verify bus-width/data-shift, and sensor is ready
for 10 bits output.
In addition to this I can do a check at probe verifying that
bus-width/data-shift are valid, ie 8/2 or 10/0, what do you think about
this ?
>> + return ov5640_write_reg(sensor,
>> + OV5640_REG_PAD_OUTPUT_ENABLE02, on ? 0xf0 : 0);
>> +}
>> +
>> +static int ov5640_set_stream_mipi(struct ov5640_dev *sensor, bool on)
>> {
>> int ret;
>> @@ -1598,17 +1662,19 @@ static int ov5640_set_power(struct ov5640_dev
>> *sensor, bool on)
>> if (ret)
>> goto power_off;
>> - /*
>> - * start streaming briefly followed by stream off in
>> - * order to coax the clock lane into LP-11 state.
>> - */
>> - ret = ov5640_set_stream(sensor, true);
>> - if (ret)
>> - goto power_off;
>> - usleep_range(1000, 2000);
>> - ret = ov5640_set_stream(sensor, false);
>> - if (ret)
>> - goto power_off;
>> + if (sensor->ep.bus_type == V4L2_MBUS_CSI2) {
>> + /*
>> + * start streaming briefly followed by stream off in
>> + * order to coax the clock lane into LP-11 state.
>> + */
>> + ret = ov5640_set_stream_mipi(sensor, true);
>> + if (ret)
>> + goto power_off;
>> + usleep_range(1000, 2000);
>> + ret = ov5640_set_stream_mipi(sensor, false);
>> + if (ret)
>> + goto power_off;
>> + }
>> return 0;
>> }
>> @@ -2185,7 +2251,11 @@ static int ov5640_s_stream(struct v4l2_subdev
>> *sd, int enable)
>> goto out;
>> }
>> - ret = ov5640_set_stream(sensor, enable);
>> + if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
>> + ret = ov5640_set_stream_mipi(sensor, enable);
>> + else
>> + ret = ov5640_set_stream_dvp(sensor, enable);
>> +
>> if (!ret)
>> sensor->streaming = enable;
>> }
>> @@ -2270,11 +2340,6 @@ static int ov5640_probe(struct i2c_client *client,
>> return ret;
>> }
>> - if (sensor->ep.bus_type != V4L2_MBUS_CSI2) {
>> - dev_err(dev, "invalid bus type, must be MIPI CSI2\n");
>> - return -EINVAL;
>> - }
>> -
>> /* get system clock (xclk) */
>> sensor->xclk = devm_clk_get(dev, "xclk");
>> if (IS_ERR(sensor->xclk)) {
>
Best regards,
Hugues.
