[PATCH 0/4] OV5640: reduce rate according to maximum pixel clock

[PATCH 0/4] OV5640: reduce rate according to maximum pixel clock

[Hugues Fruchet]

This patch serie aims to reduce parallel port rate according to maximum pixel
clock frequency admissible by camera interface in front of the sensor.
This allows to support any resolutions/framerate requests by decreasing
the framerate according to maximum camera interface capabilities.
This allows typically to enable 5Mp YUV/RGB frame capture even if 15fps
framerate could not be reached by platform.

This work is based on OV5640 Maxime Ripard's runtime clock computing serie [1]
which allows to adapt the clock tree registers according to maximum pixel
clock.

Then the first patch adds handling of pclk divider registers
DVP_PCLK_DIVIDER (0x3824) and VFIFO_CTRL0C (0x460c) in order to
correlate the rate to the effective pixel clock output on parallel interface.

A new devicetree property "pclk-max-frequency" is introduced in order
to inform sensor of the camera interface maximum admissible pixel clock.
This new devicetree property handling is added to V4L2 core.

Then OV5640 ov5640_set_dvp_pclk() is modified to clip rate according
to optional maximum pixel clock property.

References:
  [1] [PATCH v3 00/12] media: ov5640: Misc cleanup and improvements https://www.mail-archive.com/linux-media@vger.kernel.org/msg131655.html

Hugues Fruchet (4):
  media: ov5640: move parallel port pixel clock divider out of registers
    set
  media: v4l2-core: add pixel clock max frequency parallel port property
  media: dt-bindings: media: Document pclk-max-frequency property
  media: ov5640: reduce rate according to maximum pixel clock frequency

 .../devicetree/bindings/media/video-interfaces.txt |  2 +
 drivers/media/i2c/ov5640.c                         | 78 ++++++++++++++++------
 drivers/media/v4l2-core/v4l2-fwnode.c              |  3 +
 include/media/v4l2-fwnode.h                        |  2 +
 4 files changed, 65 insertions(+), 20 deletions(-)

-- 
2.7.4

[PATCH 0/4] OV5640: reduce rate according to maximum pixel clock

[Hugues Fruchet]

This patch serie aims to reduce parallel port rate according to maximum pixel
clock frequency admissible by camera interface in front of the sensor.
This allows to support any resolutions/framerate requests by decreasing
the framerate according to maximum camera interface capabilities.
This allows typically to enable 5Mp YUV/RGB frame capture even if 15fps
framerate could not be reached by platform.

This work is based on OV5640 Maxime Ripard's runtime clock computing serie [1]
which allows to adapt the clock tree registers according to maximum pixel
clock.

Then the first patch adds handling of pclk divider registers
DVP_PCLK_DIVIDER (0x3824) and VFIFO_CTRL0C (0x460c) in order to
correlate the rate to the effective pixel clock output on parallel interface.

A new devicetree property "pclk-max-frequency" is introduced in order
to inform sensor of the camera interface maximum admissible pixel clock.
This new devicetree property handling is added to V4L2 core.

Then OV5640 ov5640_set_dvp_pclk() is modified to clip rate according
to optional maximum pixel clock property.

References:
  [1] [PATCH v3 00/12] media: ov5640: Misc cleanup and improvements https://www.mail-archive.com/linux-media@vger.kernel.org/msg131655.html

Hugues Fruchet (4):
  media: ov5640: move parallel port pixel clock divider out of registers
    set
  media: v4l2-core: add pixel clock max frequency parallel port property
  media: dt-bindings: media: Document pclk-max-frequency property
  media: ov5640: reduce rate according to maximum pixel clock frequency

 .../devicetree/bindings/media/video-interfaces.txt |  2 +
 drivers/media/i2c/ov5640.c                         | 78 ++++++++++++++++------
 drivers/media/v4l2-core/v4l2-fwnode.c              |  3 +
 include/media/v4l2-fwnode.h                        |  2 +
 4 files changed, 65 insertions(+), 20 deletions(-)

-- 
2.7.4

[PATCH 1/4] media: ov5640: move parallel port pixel clock divider out of registers set

[Hugues Fruchet]

Set DVP_PCLK_DIVIDER (0x3824) and VFIFO_CTRL0C (0x460c) registers
in ov5640_set_dvp_pclk() according to mode selected.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov5640.c | 66 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1a64bb6..da4d754 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -66,6 +66,7 @@
 #define OV5640_REG_TIMING_VTS		0x380e
 #define OV5640_REG_TIMING_TC_REG20	0x3820
 #define OV5640_REG_TIMING_TC_REG21	0x3821
+#define OV5640_REG_DVP_PCLK_DIVIDER	0x3824
 #define OV5640_REG_AEC_CTRL00		0x3a00
 #define OV5640_REG_AEC_B50_STEP		0x3a08
 #define OV5640_REG_AEC_B60_STEP		0x3a0a
@@ -82,6 +83,7 @@
 #define OV5640_REG_SIGMADELTA_CTRL0C	0x3c0c
 #define OV5640_REG_FRAME_CTRL01		0x4202
 #define OV5640_REG_FORMAT_CONTROL00	0x4300
+#define OV5640_REG_VFIFO_CTRL0C		0x460C
 #define OV5640_REG_POLARITY_CTRL00	0x4740
 #define OV5640_REG_MIPI_CTRL00		0x4800
 #define OV5640_REG_DEBUG_MODE		0x4814
@@ -355,8 +357,8 @@ static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_XGA_1024_768[] = {
@@ -374,8 +376,8 @@ static const struct reg_value ov5640_setting_XGA_1024_768[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QVGA_320_240[] = {
@@ -393,8 +395,8 @@ static const struct reg_value ov5640_setting_QVGA_320_240[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QCIF_176_144[] = {
@@ -412,8 +414,8 @@ static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_NTSC_720_480[] = {
@@ -431,8 +433,8 @@ static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_PAL_720_576[] = {
@@ -450,8 +452,8 @@ static const struct reg_value ov5640_setting_PAL_720_576[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_720P_1280_720[] = {
@@ -469,8 +471,8 @@ static const struct reg_value ov5640_setting_720P_1280_720[] = {
 	{0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x72, 0, 0}, {0x3a0e, 0x01, 0, 0},
 	{0x3a0d, 0x02, 0, 0}, {0x3a14, 0x02, 0, 0}, {0x3a15, 0xe4, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0},
-	{0x3824, 0x04, 0, 0}, {0x5001, 0x83, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x37, 0, 0},
+	{0x5001, 0x83, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
@@ -490,7 +492,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
 	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0},
+	{0x5001, 0x83, 0, 0},
 	{0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3800, 0x01, 0, 0}, {0x3801, 0x50, 0, 0}, {0x3802, 0x01, 0, 0},
@@ -501,7 +503,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
 	{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
 	{0x3a15, 0x60, 0, 0}, {0x4713, 0x02, 0, 0}, {0x4407, 0x04, 0, 0},
-	{0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0}, {0x3824, 0x04, 0, 0},
+	{0x460b, 0x37, 0, 0},
 	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
 };
 
@@ -520,8 +522,8 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 70},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0x83, 0, 70},
 };
 
 /* power-on sensor init reg table */
@@ -910,10 +912,34 @@ static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
 	return _rate / *pll_rdiv / *bit_div / *pclk_div;
 }
 
-static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor, unsigned long rate)
+static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
+			       const struct ov5640_mode_info *mode,
+			       unsigned long rate)
 {
 	u8 prediv, mult, sysdiv, pll_rdiv, bit_div, pclk_div;
 	int ret;
+	/*
+	 * FIXME, value of PCLK divider deduced from
+	 * mode registers hardcoded sequence and tests
+	 */
+	u8 dvp_pclk_divider = mode->hact < 1024 ? 2 : 1;
+
+	/*
+	 * VIFO CTRL0C:
+	 * - [1]:	PCLK manual enable (0: control by auto-mode,
+	 *		1: manual control by OV5640_REG_DVP_PCLK_DIVIDER)
+	 *
+	 * Always enable PCLK manual control.
+	 */
+	ret = ov5640_mod_reg(sensor, OV5640_REG_VFIFO_CTRL0C,
+			     BIT(1), BIT(1));
+	if (ret)
+		return ret;
+
+	ret = ov5640_write_reg(sensor, OV5640_REG_DVP_PCLK_DIVIDER,
+			       dvp_pclk_divider);
+	if (ret)
+		return ret;
 
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
 			 &bit_div, &pclk_div);
@@ -1699,7 +1725,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 		ret = ov5640_set_mipi_pclk(sensor, rate);
 	} else {
 		rate = rate / sensor->ep.bus.parallel.bus_width;
-		ret = ov5640_set_dvp_pclk(sensor, rate);
+		ret = ov5640_set_dvp_pclk(sensor, mode, rate);
 	}
 
 	if (ret < 0)
-- 
2.7.4

[PATCH 1/4] media: ov5640: move parallel port pixel clock divider out of registers set

[Hugues Fruchet]

Set DVP_PCLK_DIVIDER (0x3824) and VFIFO_CTRL0C (0x460c) registers
in ov5640_set_dvp_pclk() according to mode selected.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov5640.c | 66 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 1a64bb6..da4d754 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -66,6 +66,7 @@
 #define OV5640_REG_TIMING_VTS		0x380e
 #define OV5640_REG_TIMING_TC_REG20	0x3820
 #define OV5640_REG_TIMING_TC_REG21	0x3821
+#define OV5640_REG_DVP_PCLK_DIVIDER	0x3824
 #define OV5640_REG_AEC_CTRL00		0x3a00
 #define OV5640_REG_AEC_B50_STEP		0x3a08
 #define OV5640_REG_AEC_B60_STEP		0x3a0a
@@ -82,6 +83,7 @@
 #define OV5640_REG_SIGMADELTA_CTRL0C	0x3c0c
 #define OV5640_REG_FRAME_CTRL01		0x4202
 #define OV5640_REG_FORMAT_CONTROL00	0x4300
+#define OV5640_REG_VFIFO_CTRL0C		0x460C
 #define OV5640_REG_POLARITY_CTRL00	0x4740
 #define OV5640_REG_MIPI_CTRL00		0x4800
 #define OV5640_REG_DEBUG_MODE		0x4814
@@ -355,8 +357,8 @@ static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_XGA_1024_768[] = {
@@ -374,8 +376,8 @@ static const struct reg_value ov5640_setting_XGA_1024_768[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QVGA_320_240[] = {
@@ -393,8 +395,8 @@ static const struct reg_value ov5640_setting_QVGA_320_240[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QCIF_176_144[] = {
@@ -412,8 +414,8 @@ static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_NTSC_720_480[] = {
@@ -431,8 +433,8 @@ static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_PAL_720_576[] = {
@@ -450,8 +452,8 @@ static const struct reg_value ov5640_setting_PAL_720_576[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_720P_1280_720[] = {
@@ -469,8 +471,8 @@ static const struct reg_value ov5640_setting_720P_1280_720[] = {
 	{0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x72, 0, 0}, {0x3a0e, 0x01, 0, 0},
 	{0x3a0d, 0x02, 0, 0}, {0x3a14, 0x02, 0, 0}, {0x3a15, 0xe4, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x02, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0},
-	{0x3824, 0x04, 0, 0}, {0x5001, 0x83, 0, 0},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x37, 0, 0},
+	{0x5001, 0x83, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
@@ -490,7 +492,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
 	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 0},
+	{0x5001, 0x83, 0, 0},
 	{0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3800, 0x01, 0, 0}, {0x3801, 0x50, 0, 0}, {0x3802, 0x01, 0, 0},
@@ -501,7 +503,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
 	{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
 	{0x3a15, 0x60, 0, 0}, {0x4713, 0x02, 0, 0}, {0x4407, 0x04, 0, 0},
-	{0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0}, {0x3824, 0x04, 0, 0},
+	{0x460b, 0x37, 0, 0},
 	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
 };
 
@@ -520,8 +522,8 @@ static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 	{0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0},
 	{0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0},
 	{0x4001, 0x02, 0, 0}, {0x4004, 0x06, 0, 0}, {0x4713, 0x03, 0, 0},
-	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0},
-	{0x3824, 0x02, 0, 0}, {0x5001, 0x83, 0, 70},
+	{0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0},
+	{0x5001, 0x83, 0, 70},
 };
 
 /* power-on sensor init reg table */
@@ -910,10 +912,34 @@ static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
 	return _rate / *pll_rdiv / *bit_div / *pclk_div;
 }
 
-static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor, unsigned long rate)
+static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
+			       const struct ov5640_mode_info *mode,
+			       unsigned long rate)
 {
 	u8 prediv, mult, sysdiv, pll_rdiv, bit_div, pclk_div;
 	int ret;
+	/*
+	 * FIXME, value of PCLK divider deduced from
+	 * mode registers hardcoded sequence and tests
+	 */
+	u8 dvp_pclk_divider = mode->hact < 1024 ? 2 : 1;
+
+	/*
+	 * VIFO CTRL0C:
+	 * - [1]:	PCLK manual enable (0: control by auto-mode,
+	 *		1: manual control by OV5640_REG_DVP_PCLK_DIVIDER)
+	 *
+	 * Always enable PCLK manual control.
+	 */
+	ret = ov5640_mod_reg(sensor, OV5640_REG_VFIFO_CTRL0C,
+			     BIT(1), BIT(1));
+	if (ret)
+		return ret;
+
+	ret = ov5640_write_reg(sensor, OV5640_REG_DVP_PCLK_DIVIDER,
+			       dvp_pclk_divider);
+	if (ret)
+		return ret;
 
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
 			 &bit_div, &pclk_div);
@@ -1699,7 +1725,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor)
 		ret = ov5640_set_mipi_pclk(sensor, rate);
 	} else {
 		rate = rate / sensor->ep.bus.parallel.bus_width;
-		ret = ov5640_set_dvp_pclk(sensor, rate);
+		ret = ov5640_set_dvp_pclk(sensor, mode, rate);
 	}
 
 	if (ret < 0)
-- 
2.7.4

[PATCH 2/4] media: v4l2-core: add pixel clock max frequency parallel port property

[Hugues Fruchet]

Add pclk-max-frequency property in parallel port endpoint in order
to inform sensor of the maximum pixel clock frequency admissible
by camera interface that is connected on.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 3 +++
 include/media/v4l2-fwnode.h           | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 169bdbb..505338e 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -158,6 +158,9 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
 		flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
 			V4L2_MBUS_DATA_ENABLE_LOW;
 
+	if (!fwnode_property_read_u32(fwnode, "pclk-max-frequency", &v))
+		bus->pclk_max_frequency = v;
+
 	bus->flags = flags;
 
 }
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 9cccab6..946b48d 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -52,11 +52,13 @@ struct v4l2_fwnode_bus_mipi_csi2 {
  * @flags: media bus (V4L2_MBUS_*) flags
  * @bus_width: bus width in bits
  * @data_shift: data shift in bits
+ * @max_pclk_frequency: maximum pixel clock in hertz
  */
 struct v4l2_fwnode_bus_parallel {
 	unsigned int flags;
 	unsigned char bus_width;
 	unsigned char data_shift;
+	unsigned int pclk_max_frequency;
 };
 
 /**
-- 
2.7.4

[PATCH 2/4] media: v4l2-core: add pixel clock max frequency parallel port property

[Hugues Fruchet]

Add pclk-max-frequency property in parallel port endpoint in order
to inform sensor of the maximum pixel clock frequency admissible
by camera interface that is connected on.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/v4l2-core/v4l2-fwnode.c | 3 +++
 include/media/v4l2-fwnode.h           | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 169bdbb..505338e 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -158,6 +158,9 @@ static void v4l2_fwnode_endpoint_parse_parallel_bus(
 		flags |= v ? V4L2_MBUS_DATA_ENABLE_HIGH :
 			V4L2_MBUS_DATA_ENABLE_LOW;
 
+	if (!fwnode_property_read_u32(fwnode, "pclk-max-frequency", &v))
+		bus->pclk_max_frequency = v;
+
 	bus->flags = flags;
 
 }
diff --git a/include/media/v4l2-fwnode.h b/include/media/v4l2-fwnode.h
index 9cccab6..946b48d 100644
--- a/include/media/v4l2-fwnode.h
+++ b/include/media/v4l2-fwnode.h
@@ -52,11 +52,13 @@ struct v4l2_fwnode_bus_mipi_csi2 {
  * @flags: media bus (V4L2_MBUS_*) flags
  * @bus_width: bus width in bits
  * @data_shift: data shift in bits
+ * @max_pclk_frequency: maximum pixel clock in hertz
  */
 struct v4l2_fwnode_bus_parallel {
 	unsigned int flags;
 	unsigned char bus_width;
 	unsigned char data_shift;
+	unsigned int pclk_max_frequency;
 };
 
 /**
-- 
2.7.4

[PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property

[Hugues Fruchet]

This optional property aims to inform parallel video devices
of the maximum pixel clock frequency admissible by host video
interface. If bandwidth of data to be transferred requires a
pixel clock which is higher than this value, parallel video
device could then typically adapt framerate to reach
this constraint.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index baf9d97..fa4c112 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -147,6 +147,8 @@ Optional endpoint properties
   as 0 (normal). This property is valid for serial busses only.
 - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
   with CCP2, for instance.
+- pclk-max-frequency: maximum pixel clock frequency admissible by video
+  host interface.
 
 Example
 -------
-- 
2.7.4

[PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property

[Hugues Fruchet]

This optional property aims to inform parallel video devices
of the maximum pixel clock frequency admissible by host video
interface. If bandwidth of data to be transferred requires a
pixel clock which is higher than this value, parallel video
device could then typically adapt framerate to reach
this constraint.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index baf9d97..fa4c112 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -147,6 +147,8 @@ Optional endpoint properties
   as 0 (normal). This property is valid for serial busses only.
 - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
   with CCP2, for instance.
+- pclk-max-frequency: maximum pixel clock frequency admissible by video
+  host interface.
 
 Example
 -------
-- 
2.7.4

[PATCH 4/4] media: ov5640: reduce rate according to maximum pixel clock frequency

[Hugues Fruchet]

Reduce parallel port rate according to maximum pixel clock frequency
admissible by camera interface.
This allows to support any resolutions/framerate requests by decreasing
the framerate according to maximum camera interface capabilities.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov5640.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index da4d754..9f3c32e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -918,6 +918,8 @@ static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
 {
 	u8 prediv, mult, sysdiv, pll_rdiv, bit_div, pclk_div;
 	int ret;
+	struct i2c_client *client = sensor->i2c_client;
+	unsigned int pclk_freq, max_pclk_freq;
 	/*
 	 * FIXME, value of PCLK divider deduced from
 	 * mode registers hardcoded sequence and tests
@@ -941,6 +943,16 @@ static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
 	if (ret)
 		return ret;
 
+	pclk_freq = rate / dvp_pclk_divider;
+	max_pclk_freq = sensor->ep.bus.parallel.pclk_max_frequency;
+
+	/* clip rate according to optional maximum pixel clock limit */
+	if (max_pclk_freq && pclk_freq > max_pclk_freq) {
+		rate = max_pclk_freq * dvp_pclk_divider;
+		dev_dbg(&client->dev, "DVP pixel clock too high (%d > %d Hz), reducing rate...\n",
+			pclk_freq, max_pclk_freq);
+	}
+
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
 			 &bit_div, &pclk_div);
 
-- 
2.7.4

[PATCH 4/4] media: ov5640: reduce rate according to maximum pixel clock frequency

[Hugues Fruchet]

Reduce parallel port rate according to maximum pixel clock frequency
admissible by camera interface.
This allows to support any resolutions/framerate requests by decreasing
the framerate according to maximum camera interface capabilities.

Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
---
 drivers/media/i2c/ov5640.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index da4d754..9f3c32e 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -918,6 +918,8 @@ static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
 {
 	u8 prediv, mult, sysdiv, pll_rdiv, bit_div, pclk_div;
 	int ret;
+	struct i2c_client *client = sensor->i2c_client;
+	unsigned int pclk_freq, max_pclk_freq;
 	/*
 	 * FIXME, value of PCLK divider deduced from
 	 * mode registers hardcoded sequence and tests
@@ -941,6 +943,16 @@ static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor,
 	if (ret)
 		return ret;
 
+	pclk_freq = rate / dvp_pclk_divider;
+	max_pclk_freq = sensor->ep.bus.parallel.pclk_max_frequency;
+
+	/* clip rate according to optional maximum pixel clock limit */
+	if (max_pclk_freq && pclk_freq > max_pclk_freq) {
+		rate = max_pclk_freq * dvp_pclk_divider;
+		dev_dbg(&client->dev, "DVP pixel clock too high (%d > %d Hz), reducing rate...\n",
+			pclk_freq, max_pclk_freq);
+	}
+
 	ov5640_calc_pclk(sensor, rate, &prediv, &mult, &sysdiv, &pll_rdiv,
 			 &bit_div, &pclk_div);
 
-- 
2.7.4

Re: [PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property

[Maxime Ripard]

Hi!

On Thu, Sep 27, 2018 at 04:46:06PM +0200, Hugues Fruchet wrote:
> This optional property aims to inform parallel video devices
> of the maximum pixel clock frequency admissible by host video
> interface. If bandwidth of data to be transferred requires a
> pixel clock which is higher than this value, parallel video
> device could then typically adapt framerate to reach
> this constraint.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index baf9d97..fa4c112 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -147,6 +147,8 @@ Optional endpoint properties
>    as 0 (normal). This property is valid for serial busses only.
>  - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
>    with CCP2, for instance.
> +- pclk-max-frequency: maximum pixel clock frequency admissible by video
> +  host interface.

That seems to be a property of the capture device, not the camera
itself. Can't that be negotiated through the media API?

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property

[Sakari Ailus]

Hi Hugues,

On Thu, Sep 27, 2018 at 04:46:06PM +0200, Hugues Fruchet wrote:
> This optional property aims to inform parallel video devices
> of the maximum pixel clock frequency admissible by host video
> interface. If bandwidth of data to be transferred requires a
> pixel clock which is higher than this value, parallel video
> device could then typically adapt framerate to reach
> this constraint.
> 
> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
> index baf9d97..fa4c112 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -147,6 +147,8 @@ Optional endpoint properties
>    as 0 (normal). This property is valid for serial busses only.
>  - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
>    with CCP2, for instance.
> +- pclk-max-frequency: maximum pixel clock frequency admissible by video
> +  host interface.

Is there a limit on the pixel clock or the link frequency?

We do have a property for the link frequency and a control for the pixel
lock as well as for the link frequency. Could these be used for the
purpose?

The link frequency in general should be specified for the board, and that
limits the pixel clock as well in the case the bus transfers a given number
of pixels per clock.

The OMAP3ISP driver also address this by reading back the pixel clock from
the sensor before starting streaming.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi

Re: [PATCH 3/4] media: dt-bindings: media: Document pclk-max-frequency property

[Hugues FRUCHET]

Hi Sakari,

On 09/28/2018 09:03 AM, Sakari Ailus wrote:
> Hi Hugues,
> 
> On Thu, Sep 27, 2018 at 04:46:06PM +0200, Hugues Fruchet wrote:
>> This optional property aims to inform parallel video devices
>> of the maximum pixel clock frequency admissible by host video
>> interface. If bandwidth of data to be transferred requires a
>> pixel clock which is higher than this value, parallel video
>> device could then typically adapt framerate to reach
>> this constraint.
>>
>> Signed-off-by: Hugues Fruchet <hugues.fruchet@st.com>
>> ---
>>   Documentation/devicetree/bindings/media/video-interfaces.txt | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> index baf9d97..fa4c112 100644
>> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
>> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
>> @@ -147,6 +147,8 @@ Optional endpoint properties
>>     as 0 (normal). This property is valid for serial busses only.
>>   - strobe: Whether the clock signal is used as clock (0) or strobe (1). Used
>>     with CCP2, for instance.
>> +- pclk-max-frequency: maximum pixel clock frequency admissible by video
>> +  host interface.
> 
> Is there a limit on the pixel clock or the link frequency?

The constraint is the frequency of the clock in input of the SoC (pixel 
clock line).

> 
> We do have a property for the link frequency and a control for the pixel
> lock as well as for the link frequency. Could these be used for the
> purpose?

As this was documented mainly for MIPI-CSI2 I was not clear if this 
could be used or not, but video-interface.txt binding let open the door
to parallel port usage...
I had also some hesitations to use this property because what I was 
searching for here was a maximum limit to not exceed while 
"link-frequencies" is described as frequencies to use: "Allowed data bus 
frequencies".
The fact that there was several entries for this property was also quite 
confusing.
What I can do is to use this property and add a comment explaining that 
this can also be used for parallel port as the frequency to not exceed 
on pixel clock signal, what do you think about it ?

Checking drivers which are implementing "link-frequencies", I've found 
OV2659 sensor which is doing almost what I want to, ie compute the clock 
rate depending on link-frequency, see ov2659_pll_calc_params().

> 
> The link frequency in general should be specified for the board, and that
> limits the pixel clock as well in the case the bus transfers a given number
> of pixels per clock.
> 
> The OMAP3ISP driver also address this by reading back the pixel clock from
> the sensor before starting streaming.
> 

BR,
Hugues.