[PATCH 00/12] media: ov5640: Misc cleanup and improvements

[PATCH 00/12] media: ov5640: Misc cleanup and improvements

[Maxime Ripard]

Hi,

Here is a "small" series that mostly cleans up the ov5640 driver code,
slowly getting rid of the big data array for more understandable code
(hopefully).

The biggest addition would be the clock rate computation at runtime,
instead of relying on those arrays to setup the clock tree
properly. As a side effect, it fixes the framerate that was off by
around 10% on the smaller resolutions, and we now support 60fps.

This also introduces a bunch of new features.

Let me know what you think,
Maxime

Maxime Ripard (10):
  media: ov5640: Don't force the auto exposure state at start time
  media: ov5640: Init properly the SCLK dividers
  media: ov5640: Change horizontal and vertical resolutions name
  media: ov5640: Add horizontal and vertical totals
  media: ov5640: Program the visible resolution
  media: ov5640: Adjust the clock based on the expected rate
  media: ov5640: Compute the clock rate at runtime
  media: ov5640: Enhance FPS handling
  media: ov5640: Add 60 fps support
  media: ov5640: Remove duplicate auto-exposure setup

Mylène Josserand (2):
  media: ov5640: Add auto-focus feature
  media: ov5640: Add light frequency control

 drivers/media/i2c/ov5640.c | 777 ++++++++++++++++++++++++++-------------------
 1 file changed, 452 insertions(+), 325 deletions(-)

-- 
2.14.3

[PATCH 01/12] media: ov5640: Add auto-focus feature

[Maxime Ripard]

From: Mylène Josserand <mylene.josserand@bootlin.com>

Add the auto-focus ENABLE/DISABLE feature as V4L2 control.
Disabled by default.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e2dd352224c7..f5e867d47ab8 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -80,8 +80,9 @@
 #define OV5640_REG_POLARITY_CTRL00	0x4740
 #define OV5640_REG_MIPI_CTRL00		0x4800
 #define OV5640_REG_DEBUG_MODE		0x4814
-#define OV5640_REG_ISP_FORMAT_MUX_CTRL	0x501f
+#define OV5640_REG_ISP_CTRL03		0x5003
 #define OV5640_REG_PRE_ISP_TEST_SET1	0x503d
+#define OV5640_REG_ISP_FORMAT_MUX_CTRL	0x501f
 #define OV5640_REG_SDE_CTRL0		0x5580
 #define OV5640_REG_SDE_CTRL1		0x5581
 #define OV5640_REG_SDE_CTRL3		0x5583
@@ -183,6 +184,7 @@ struct ov5640_ctrls {
 		struct v4l2_ctrl *auto_gain;
 		struct v4l2_ctrl *gain;
 	};
+	struct v4l2_ctrl *auto_focus;
 	struct v4l2_ctrl *brightness;
 	struct v4l2_ctrl *saturation;
 	struct v4l2_ctrl *contrast;
@@ -2094,6 +2096,12 @@ static int ov5640_set_ctrl_test_pattern(struct ov5640_dev *sensor, int value)
 			      0xa4, value ? 0xa4 : 0);
 }
 
+static int ov5640_set_ctrl_focus(struct ov5640_dev *sensor, int value)
+{
+	return ov5640_mod_reg(sensor, OV5640_REG_ISP_CTRL03,
+			      BIT(1), value ? BIT(1) : 0);
+}
+
 static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2162,6 +2170,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_TEST_PATTERN:
 		ret = ov5640_set_ctrl_test_pattern(sensor, ctrl->val);
 		break;
+	case V4L2_CID_FOCUS_AUTO:
+		ret = ov5640_set_ctrl_focus(sensor, ctrl->val);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -2224,6 +2235,9 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 					     ARRAY_SIZE(test_pattern_menu) - 1,
 					     0, 0, test_pattern_menu);
 
+	ctrls->auto_focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_AUTO,
+					      0, 1, 1, 0);
+
 	if (hdl->error) {
 		ret = hdl->error;
 		goto free_ctrls;
-- 
2.14.3

[PATCH 02/12] media: ov5640: Add light frequency control

[Maxime Ripard]

From: Mylène Josserand <mylene.josserand@bootlin.com>

Add the light frequency control to be able to set the frequency
to manual (50Hz or 60Hz) or auto.

Signed-off-by: Mylène Josserand <mylene.josserand@bootlin.com>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index f5e867d47ab8..e6a23eb55c1d 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -186,6 +186,7 @@ struct ov5640_ctrls {
 	};
 	struct v4l2_ctrl *auto_focus;
 	struct v4l2_ctrl *brightness;
+	struct v4l2_ctrl *light_freq;
 	struct v4l2_ctrl *saturation;
 	struct v4l2_ctrl *contrast;
 	struct v4l2_ctrl *hue;
@@ -2102,6 +2103,21 @@ static int ov5640_set_ctrl_focus(struct ov5640_dev *sensor, int value)
 			      BIT(1), value ? BIT(1) : 0);
 }
 
+static int ov5640_set_ctl_light_freq(struct ov5640_dev *sensor, int value)
+{
+	int ret;
+
+	ret = ov5640_mod_reg(sensor, OV5640_REG_HZ5060_CTRL01, BIT(7),
+			     (value == V4L2_CID_POWER_LINE_FREQUENCY_AUTO) ?
+			     0: BIT(7));
+	if (ret)
+		return ret;
+
+	return ov5640_mod_reg(sensor, OV5640_REG_HZ5060_CTRL00, BIT(2),
+			      (value == V4L2_CID_POWER_LINE_FREQUENCY_50HZ) ?
+			      BIT(2): 0);
+}
+
 static int ov5640_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
@@ -2173,6 +2189,9 @@ static int ov5640_s_ctrl(struct v4l2_ctrl *ctrl)
 	case V4L2_CID_FOCUS_AUTO:
 		ret = ov5640_set_ctrl_focus(sensor, ctrl->val);
 		break;
+	case V4L2_CID_POWER_LINE_FREQUENCY:
+		ret = ov5640_set_ctl_light_freq(sensor, ctrl->val);
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -2237,6 +2256,11 @@ static int ov5640_init_controls(struct ov5640_dev *sensor)
 
 	ctrls->auto_focus = v4l2_ctrl_new_std(hdl, ops, V4L2_CID_FOCUS_AUTO,
 					      0, 1, 1, 0);
+	ctrls->light_freq =
+		v4l2_ctrl_new_std_menu(hdl, ops,
+				       V4L2_CID_POWER_LINE_FREQUENCY,
+				       V4L2_CID_POWER_LINE_FREQUENCY_AUTO, 0,
+				       V4L2_CID_POWER_LINE_FREQUENCY_50HZ);
 
 	if (hdl->error) {
 		ret = hdl->error;
-- 
2.14.3

[PATCH 03/12] media: ov5640: Don't force the auto exposure state at start time

[Maxime Ripard]

The sensor needs to have the auto exposure stopped while changing mode.
However, when the new mode is set, the driver will force the auto exposure
on, disregarding whether the control has been changed or not.

Bypass the controls code entirely to do that, and only use the control
value cached when restoring the auto exposure mode.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index e6a23eb55c1d..0d8f979416cc 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -1579,7 +1579,9 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
 	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
 	if (ret)
 		return ret;
-	return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_AUTO);
+
+	return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp,
+				  sensor->ctrls.auto_exp->val);
 }
 
 static int ov5640_set_mode(struct ov5640_dev *sensor,
@@ -1596,7 +1598,8 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
 	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0);
 	if (ret)
 		return ret;
-	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_MANUAL);
+
+	ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
 	if (ret)
 		return ret;
 
-- 
2.14.3

Re: [PATCH 03/12] media: ov5640: Don't force the auto exposure state at start time

[Sakari Ailus]

On Fri, Mar 02, 2018 at 03:34:51PM +0100, Maxime Ripard wrote:
> The sensor needs to have the auto exposure stopped while changing mode.
> However, when the new mode is set, the driver will force the auto exposure
> on, disregarding whether the control has been changed or not.
> 
> Bypass the controls code entirely to do that, and only use the control
> value cached when restoring the auto exposure mode.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  drivers/media/i2c/ov5640.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index e6a23eb55c1d..0d8f979416cc 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -1579,7 +1579,9 @@ static int ov5640_set_mode_direct(struct ov5640_dev *sensor,
>  	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 1);
>  	if (ret)
>  		return ret;
> -	return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_AUTO);
> +
> +	return __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp,
> +				  sensor->ctrls.auto_exp->val);
>  }
>  
>  static int ov5640_set_mode(struct ov5640_dev *sensor,
> @@ -1596,7 +1598,8 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>  	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_gain, 0);
>  	if (ret)
>  		return ret;
> -	ret = __v4l2_ctrl_s_ctrl(sensor->ctrls.auto_exp, V4L2_EXPOSURE_MANUAL);
> +
> +	ret = ov5640_set_exposure(sensor, V4L2_EXPOSURE_MANUAL);
>  	if (ret)
>  		return ret;
>  

The s_ctrl callback won't be called if the control framework still has the
same value set. I think you could store the value manually, and retain
__v4l2_ctrl_s_ctrl() call in ov5640_set_mode().

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

[PATCH 04/12] media: ov5640: Init properly the SCLK dividers

[Maxime Ripard]

The SCLK and SCLK2X dividers are fixed in stone in the initialization
array. Let's make explicit what we're doing and move that away from the
huge array to the initialization code.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0d8f979416cc..0415484e32fb 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -90,6 +90,9 @@
 #define OV5640_REG_SDE_CTRL5		0x5585
 #define OV5640_REG_AVG_READOUT		0x56a1
 
+#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT	1
+#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT	2
+
 enum ov5640_mode_id {
 	OV5640_MODE_QCIF_176_144 = 0,
 	OV5640_MODE_QVGA_320_240,
@@ -249,7 +252,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
 	{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
 	{0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
-	{0x3037, 0x13, 0, 0}, {0x3108, 0x01, 0, 0}, {0x3630, 0x36, 0, 0},
+	{0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
 	{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
 	{0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
 	{0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
@@ -1649,6 +1652,12 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor)
 	if (ret < 0)
 		return ret;
 
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f,
+			     (ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) |
+			     ilog2(OV5640_SCLK_ROOT_DIVIDER_DEFAULT));
+	if (ret)
+		return ret;
+
 	/* now restore the last capture mode */
 	return ov5640_set_mode(sensor, &ov5640_mode_init_data);
 }
-- 
2.14.3

[PATCH 05/12] media: ov5640: Change horizontal and vertical resolutions name

[Maxime Ripard]

The current width and height parameters in the struct ov5640_mode_info are
actually the active horizontal and vertical resolutions.

Since we're going to add a few other parameters, let's pick a better, more
precise name for these values.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0415484e32fb..8c04f2880715 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -166,8 +166,8 @@ struct reg_value {
 struct ov5640_mode_info {
 	enum ov5640_mode_id id;
 	enum ov5640_downsize_mode dn_mode;
-	u32 width;
-	u32 height;
+	u32 hact;
+	u32 vact;
 	const struct reg_value *reg_data;
 	u32 reg_data_size;
 };
@@ -1388,10 +1388,10 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 		if (!mode->reg_data)
 			continue;
 
-		if ((nearest && mode->width <= width &&
-		     mode->height <= height) ||
-		    (!nearest && mode->width == width &&
-		     mode->height == height))
+		if ((nearest && mode->hact <= width &&
+		     mode->vact <= height) ||
+		    (!nearest && mode->hact == width &&
+		     mode->vact == height))
 			break;
 	}
 
@@ -1871,8 +1871,8 @@ static int ov5640_try_fmt_internal(struct v4l2_subdev *sd,
 	mode = ov5640_find_mode(sensor, fr, fmt->width, fmt->height, true);
 	if (!mode)
 		return -EINVAL;
-	fmt->width = mode->width;
-	fmt->height = mode->height;
+	fmt->width = mode->hact;
+	fmt->height = mode->vact;
 
 	if (new_mode)
 		*new_mode = mode;
@@ -2304,9 +2304,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	fse->min_width = fse->max_width =
-		ov5640_mode_data[0][fse->index].width;
+		ov5640_mode_data[0][fse->index].hact;
 	fse->min_height = fse->max_height =
-		ov5640_mode_data[0][fse->index].height;
+		ov5640_mode_data[0][fse->index].vact;
 
 	return 0;
 }
@@ -2369,7 +2369,7 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
 	mode = sensor->current_mode;
 
 	frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
-					       mode->width, mode->height);
+					       mode->hact, mode->vact);
 	if (frame_rate < 0)
 		frame_rate = OV5640_15_FPS;
 
-- 
2.14.3

[PATCH 06/12] media: ov5640: Add horizontal and vertical totals

[Maxime Ripard]

All the initialization arrays are changing the horizontal and vertical
totals for some value.

In order to clean up the driver, and since we're going to need that value
later on, let's introduce in the ov5640_mode_info structure the horizontal
and vertical total sizes, and move these out of the bytes array.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 156 ++++++++++++++++++++++++++++-----------------
 1 file changed, 97 insertions(+), 59 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 8c04f2880715..443b167bcd20 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -167,7 +167,9 @@ struct ov5640_mode_info {
 	enum ov5640_mode_id id;
 	enum ov5640_downsize_mode dn_mode;
 	u32 hact;
+	u32 htot;
 	u32 vact;
+	u32 vtot;
 	const struct reg_value *reg_data;
 	u32 reg_data_size;
 };
@@ -270,8 +272,8 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -345,8 +347,8 @@ static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x04, 0, 0}, {0x380f, 0x38, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -366,8 +368,8 @@ static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -388,8 +390,8 @@ static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x04, 0, 0}, {0x380f, 0x38, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -411,8 +413,8 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -433,8 +435,8 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x01, 0, 0}, {0x3809, 0x40, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0xf0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xf0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -454,8 +456,8 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x01, 0, 0}, {0x3809, 0x40, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0xf0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xf0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -475,8 +477,8 @@ static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x00, 0, 0}, {0x3809, 0xb0, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0x90, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x90, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -495,8 +497,8 @@ static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x00, 0, 0}, {0x3809, 0xb0, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0x90, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x90, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -516,8 +518,8 @@ static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -537,8 +539,8 @@ static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xe0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -558,8 +560,8 @@ static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0x40, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x40, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -579,8 +581,8 @@ static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
 	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0x40, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x68, 0, 0},
-	{0x380e, 0x03, 0, 0}, {0x380f, 0xd8, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x40, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -601,8 +603,8 @@ static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
 	{0x3808, 0x05, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0xd0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x64, 0, 0},
-	{0x380e, 0x02, 0, 0}, {0x380f, 0xe4, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xd0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x02, 0, 0},
@@ -623,8 +625,8 @@ static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
 	{0x3808, 0x05, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0xd0, 0, 0}, {0x380c, 0x07, 0, 0}, {0x380d, 0x64, 0, 0},
-	{0x380e, 0x02, 0, 0}, {0x380f, 0xe4, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0xd0, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x02, 0, 0},
@@ -645,8 +647,8 @@ static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
 	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0}, {0x380c, 0x0b, 0, 0}, {0x380d, 0x1c, 0, 0},
-	{0x380e, 0x07, 0, 0}, {0x380f, 0xb0, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x98, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
 	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -662,8 +664,7 @@ static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
 	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0}, {0x3808, 0x07, 0, 0},
 	{0x3809, 0x80, 0, 0}, {0x380a, 0x04, 0, 0}, {0x380b, 0x38, 0, 0},
-	{0x380c, 0x09, 0, 0}, {0x380d, 0xc4, 0, 0}, {0x380e, 0x04, 0, 0},
-	{0x380f, 0x60, 0, 0}, {0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
+	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
 	{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
@@ -682,8 +683,8 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
 	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0}, {0x380c, 0x0b, 0, 0}, {0x380d, 0x1c, 0, 0},
-	{0x380e, 0x07, 0, 0}, {0x380f, 0xb0, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x98, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
 	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -699,8 +700,7 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
 	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0}, {0x3808, 0x07, 0, 0},
 	{0x3809, 0x80, 0, 0}, {0x380a, 0x04, 0, 0}, {0x380b, 0x38, 0, 0},
-	{0x380c, 0x09, 0, 0}, {0x380d, 0xc4, 0, 0}, {0x380e, 0x04, 0, 0},
-	{0x380f, 0x60, 0, 0}, {0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
+	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
 	{0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0},
@@ -718,8 +718,8 @@ static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
 	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0}, {0x380c, 0x0b, 0, 0}, {0x380d, 0x1c, 0, 0},
-	{0x380e, 0x07, 0, 0}, {0x380f, 0xb0, 0, 0}, {0x3810, 0x00, 0, 0},
+	{0x380b, 0x98, 0, 0},
+	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
 	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
@@ -733,66 +733,84 @@ 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, 480, ov5640_init_setting_30fps_VGA,
+	0, SUBSAMPLING, 640, 1896, 480, 984,
+	ov5640_init_setting_30fps_VGA,
 	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 };
 
 static const struct ov5640_mode_info
 ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
 	{
-		{OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 144,
+		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
+		 176, 1896, 144, 984,
 		 ov5640_setting_15fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
-		{OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320,  240,
+		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
+		 320, 1896, 240, 984,
 		 ov5640_setting_15fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
-		{OV5640_MODE_VGA_640_480, SUBSAMPLING, 640,  480,
+		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
+		 640, 1896, 480, 1080,
 		 ov5640_setting_15fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
-		{OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 480,
+		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
+		 720, 1896, 480, 984,
 		 ov5640_setting_15fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
-		{OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 576,
+		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
+		 720, 1896, 576, 984,
 		 ov5640_setting_15fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
-		{OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 768,
+		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
+		 1024, 1896, 768, 1080,
 		 ov5640_setting_15fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
-		{OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 720,
+		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
+		 1280, 1892, 720, 740,
 		 ov5640_setting_15fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
-		{OV5640_MODE_1080P_1920_1080, SCALING, 1920, 1080,
+		{OV5640_MODE_1080P_1920_1080, SCALING,
+		 1920, 2500, 1080, 1120,
 		 ov5640_setting_15fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, SCALING, 2592, 1944,
+		{OV5640_MODE_QSXGA_2592_1944, SCALING,
+		 2592, 2844, 1944, 1968,
 		 ov5640_setting_15fps_QSXGA_2592_1944,
 		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
 	}, {
-		{OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 144,
+		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
+		 176, 1896, 144, 984,
 		 ov5640_setting_30fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
-		{OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320,  240,
+		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
+		 320, 1896, 240, 984,
 		 ov5640_setting_30fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
-		{OV5640_MODE_VGA_640_480, SUBSAMPLING, 640,  480,
+		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
+		 640, 1896, 480, 1080,
 		 ov5640_setting_30fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
-		{OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 480,
+		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
+		 720, 1896, 480, 984,
 		 ov5640_setting_30fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
-		{OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 576,
+		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
+		 720, 1896, 576, 984,
 		 ov5640_setting_30fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
-		{OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 768,
+		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
+		 1024, 1896, 768, 1080,
 		 ov5640_setting_30fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
-		{OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 720,
+		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
+		 1280, 1892, 720, 740,
 		 ov5640_setting_30fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
-		{OV5640_MODE_1080P_1920_1080, SCALING, 1920, 1080,
+		{OV5640_MODE_1080P_1920_1080, SCALING,
+		 1920, 2500, 1080, 1120,
 		 ov5640_setting_30fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, NULL, 0},
+		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
 	},
 };
 
@@ -1375,6 +1393,22 @@ static int ov5640_set_virtual_channel(struct ov5640_dev *sensor)
 	return ov5640_write_reg(sensor, OV5640_REG_DEBUG_MODE, temp);
 }
 
+static int ov5640_set_timings(struct ov5640_dev *sensor,
+			      const struct ov5640_mode_info *mode)
+{
+	int ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, mode->htot);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_VTS, mode->vtot);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static const struct ov5640_mode_info *
 ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 		 int width, int height, bool nearest)
@@ -1621,6 +1655,10 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
 		ret = ov5640_set_mode_direct(sensor, mode);
 	}
 
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_set_timings(sensor, mode);
 	if (ret < 0)
 		return ret;
 
-- 
2.14.3

[PATCH 07/12] media: ov5640: Program the visible resolution

[Maxime Ripard]

The active frame size is set in the initialization arrays, but the value
itself is also available in the struct ov5640_mode_info.

Let's move these values out of the big bytes arrays, and program it with
the value of the mode that we are given.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 58 +++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 44 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 443b167bcd20..0eeb1667bbe7 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -58,6 +58,8 @@
 #define OV5640_REG_AEC_PK_MANUAL	0x3503
 #define OV5640_REG_AEC_PK_REAL_GAIN	0x350a
 #define OV5640_REG_AEC_PK_VTS		0x350c
+#define OV5640_REG_TIMING_DVPHO		0x3808
+#define OV5640_REG_TIMING_DVPVO		0x380a
 #define OV5640_REG_TIMING_HTS		0x380c
 #define OV5640_REG_TIMING_VTS		0x380e
 #define OV5640_REG_TIMING_TC_REG21	0x3821
@@ -271,8 +273,6 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -346,8 +346,6 @@ static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -367,8 +365,6 @@ static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -389,8 +385,6 @@ static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -401,8 +395,7 @@ static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
 	{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}, {0x3503, 0x00, 0, 0},
-	{0x3808, 0x04, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x03, 0, 0},
-	{0x380b, 0x00, 0, 0}, {0x3035, 0x12, 0, 0},
+	{0x3035, 0x12, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
@@ -412,8 +405,6 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0x80, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -423,8 +414,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
 	{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}, {0x3808, 0x04, 0, 0},
-	{0x3809, 0x00, 0, 0}, {0x380a, 0x03, 0, 0}, {0x380b, 0x00, 0, 0},
+	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
@@ -434,8 +424,6 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x01, 0, 0}, {0x3809, 0x40, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0xf0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -455,8 +443,6 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x01, 0, 0}, {0x3809, 0x40, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0xf0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -476,8 +462,6 @@ static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x00, 0, 0}, {0x3809, 0xb0, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0x90, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -496,8 +480,6 @@ static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x00, 0, 0}, {0x3809, 0xb0, 0, 0}, {0x380a, 0x00, 0, 0},
-	{0x380b, 0x90, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -517,8 +499,6 @@ static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -538,8 +518,6 @@ static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x01, 0, 0},
-	{0x380b, 0xe0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -559,8 +537,6 @@ static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0x40, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -580,8 +556,6 @@ static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3808, 0x02, 0, 0}, {0x3809, 0xd0, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0x40, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -602,8 +576,6 @@ static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
-	{0x3808, 0x05, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0xd0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -624,8 +596,6 @@ static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
-	{0x3808, 0x05, 0, 0}, {0x3809, 0x00, 0, 0}, {0x380a, 0x02, 0, 0},
-	{0x380b, 0xd0, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
@@ -646,8 +616,6 @@ static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
@@ -662,8 +630,7 @@ static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3800, 0x01, 0, 0}, {0x3801, 0x50, 0, 0}, {0x3802, 0x01, 0, 0},
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
-	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0}, {0x3808, 0x07, 0, 0},
-	{0x3809, 0x80, 0, 0}, {0x380a, 0x04, 0, 0}, {0x380b, 0x38, 0, 0},
+	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0},
 	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
@@ -682,8 +649,6 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
@@ -698,8 +663,7 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3800, 0x01, 0, 0}, {0x3801, 0x50, 0, 0}, {0x3802, 0x01, 0, 0},
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
-	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0}, {0x3808, 0x07, 0, 0},
-	{0x3809, 0x80, 0, 0}, {0x380a, 0x04, 0, 0}, {0x380b, 0x38, 0, 0},
+	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0},
 	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
 	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
 	{0x3a09, 0x50, 0, 0}, {0x3a0a, 0x01, 0, 0}, {0x3a0b, 0x18, 0, 0},
@@ -717,8 +681,6 @@ static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
 	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
 	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3808, 0x0a, 0, 0}, {0x3809, 0x20, 0, 0}, {0x380a, 0x07, 0, 0},
-	{0x380b, 0x98, 0, 0},
 	{0x3810, 0x00, 0, 0},
 	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
 	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
@@ -1398,6 +1360,14 @@ static int ov5640_set_timings(struct ov5640_dev *sensor,
 {
 	int ret;
 
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPHO, mode->hact);
+	if (ret < 0)
+		return ret;
+
+	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_DVPVO, mode->vact);
+	if (ret < 0)
+		return ret;
+
 	ret = ov5640_write_reg16(sensor, OV5640_REG_TIMING_HTS, mode->htot);
 	if (ret < 0)
 		return ret;
-- 
2.14.3

[PATCH 08/12] media: ov5640: Adjust the clock based on the expected rate

[Maxime Ripard]

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 <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 316 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 291 insertions(+), 25 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 0eeb1667bbe7..323cde27dd8b 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -172,6 +172,7 @@ struct ov5640_mode_info {
 	u32 htot;
 	u32 vact;
 	u32 vtot;
+	u32 clock;
 	const struct reg_value *reg_data;
 	u32 reg_data_size;
 };
@@ -255,8 +256,8 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 
 	{0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0},
 	{0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0},
-	{0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0},
-	{0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0},
+	{0x3034, 0x18, 0, 0},
+	{0x3630, 0x36, 0, 0},
 	{0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0},
 	{0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0},
 	{0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0},
@@ -340,7 +341,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 
 static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
 
-	{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -359,7 +360,7 @@ static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -379,7 +380,7 @@ static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
 
 static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
 
-	{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -395,11 +396,10 @@ static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
 	{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}, {0x3503, 0x00, 0, 0},
-	{0x3035, 0x12, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -418,7 +418,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
-	{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -437,7 +437,7 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -456,7 +456,7 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
-	{0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -474,7 +474,7 @@ static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -493,7 +493,7 @@ static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
-	{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -512,7 +512,7 @@ static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -531,7 +531,7 @@ static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
 };
 
 static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
-	{0x3035, 0x12, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -550,7 +550,7 @@ static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
-	{0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -570,7 +570,7 @@ static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
 
 static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
 	{0x3008, 0x42, 0, 0},
-	{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
+	{0x3c07, 0x07, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -590,7 +590,7 @@ static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
 };
 
 static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
-	{0x3035, 0x41, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0},
+	{0x3c07, 0x07, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
 	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -610,7 +610,7 @@ static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
 
 static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 	{0x3008, 0x42, 0, 0},
-	{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -625,8 +625,8 @@ static const struct reg_value ov5640_setting_30fps_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}, {0x3035, 0x11, 0, 0},
-	{0x3036, 0x54, 0, 0}, {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
+	{0x3824, 0x02, 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},
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
@@ -643,7 +643,7 @@ static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
 
 static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x3008, 0x42, 0, 0},
-	{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -658,8 +658,8 @@ static const struct reg_value ov5640_setting_15fps_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}, {0x3035, 0x21, 0, 0},
-	{0x3036, 0x54, 0, 1}, {0x3c07, 0x07, 0, 0}, {0x3c08, 0x00, 0, 0},
+	{0x3824, 0x02, 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},
 	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
@@ -675,7 +675,7 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 
 static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
 	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0},
-	{0x3035, 0x21, 0, 0}, {0x3036, 0x54, 0, 0}, {0x3c07, 0x08, 0, 0},
+	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
 	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
@@ -696,6 +696,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,
+	112000000,
 	ov5640_init_setting_30fps_VGA,
 	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 };
@@ -705,74 +706,91 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
 	{
 		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 		 176, 1896, 144, 984,
+		 56000000,
 		 ov5640_setting_15fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
 		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 		 320, 1896, 240, 984,
+		 56000000,
 		 ov5640_setting_15fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
 		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 		 640, 1896, 480, 1080,
+		 56000000,
 		 ov5640_setting_15fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
 		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 		 720, 1896, 480, 984,
+		 56000000,
 		 ov5640_setting_15fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
 		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 		 720, 1896, 576, 984,
+		 56000000,
 		 ov5640_setting_15fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
 		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 		 1024, 1896, 768, 1080,
+		 56000000,
 		 ov5640_setting_15fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
 		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 		 1280, 1892, 720, 740,
+		 42000000,
 		 ov5640_setting_15fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
 		{OV5640_MODE_1080P_1920_1080, SCALING,
 		 1920, 2500, 1080, 1120,
+		 84000000,
 		 ov5640_setting_15fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
 		{OV5640_MODE_QSXGA_2592_1944, SCALING,
 		 2592, 2844, 1944, 1968,
+		 168000000,
 		 ov5640_setting_15fps_QSXGA_2592_1944,
 		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
 	}, {
 		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 		 176, 1896, 144, 984,
+		 112000000,
 		 ov5640_setting_30fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
 		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 		 320, 1896, 240, 984,
+		 112000000,
 		 ov5640_setting_30fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
 		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 		 640, 1896, 480, 1080,
+		 112000000,
 		 ov5640_setting_30fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
 		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 		 720, 1896, 480, 984,
+		 112000000,
 		 ov5640_setting_30fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
 		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 		 720, 1896, 576, 984,
+		 112000000,
 		 ov5640_setting_30fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
 		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 		 1024, 1896, 768, 1080,
+		 112000000,
 		 ov5640_setting_30fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
 		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 		 1280, 1892, 720, 740,
+		 84000000,
 		 ov5640_setting_30fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
 		{OV5640_MODE_1080P_1920_1080, SCALING,
 		 1920, 2500, 1080, 1120,
+		 168000000,
 		 ov5640_setting_30fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
+		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, 0, NULL, 0},
 	},
 };
 
@@ -902,6 +920,246 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16 reg,
 	return ov5640_write_reg(sensor, reg, val);
 }
 
+/*
+ * After spending way too much time trying the various combinations, I
+ * believe the clock tree is as follows:
+ *
+ *   +--------------+
+ *   |  Oscillator  |
+ *   +------+-------+
+ *          |
+ *   +------+-------+
+ *   | System clock | - reg 0x3035, bits 4-7
+ *   +------+-------+
+ *          |
+ *   +------+-------+ - reg 0x3036, for the multiplier
+ *   |     PLL      | - reg 0x3037, bits 4 for the root divider
+ *   +------+-------+ - reg 0x3037, bits 0-3 for the pre-divider
+ *          |
+ *   +------+-------+
+ *   |     SCLK     | - reg 0x3108, bits 0-1 for the root divider
+ *   +------+-------+
+ *          |
+ *   +------+-------+
+ *   |    PCLK      | - reg 0x3108, bits 4-5 for the root divider
+ *   +--------------+
+ *
+ * This is deviating from the datasheet at least for the register
+ * 0x3108, since it's said here that the PCLK would be clocked from
+ * the PLL. However, changing the SCLK divider value has a direct
+ * effect on the PCLK rate, which wouldn't be the case if both PCLK
+ * and SCLK were to be sourced from the PLL.
+ *
+ * These parameters also match perfectly the rate that is output by
+ * the sensor, so we shouldn't have too much factors missing (or they
+ * would be set to 1).
+ */
+
+/*
+ * FIXME: 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.
+ */
+#define OV5640_SYSDIV_MIN	1
+#define OV5640_SYSDIV_MAX	2
+
+static unsigned long ov5640_calc_sysclk(struct ov5640_dev *sensor,
+					unsigned long rate,
+					u8 *sysdiv)
+{
+	unsigned long best = ~0;
+	u8 best_sysdiv = 1;
+	u8 _sysdiv;
+
+	for (_sysdiv = OV5640_SYSDIV_MIN;
+	     _sysdiv <= OV5640_SYSDIV_MAX;
+	     _sysdiv++) {
+		unsigned long tmp;
+
+		tmp = sensor->xclk_freq / _sysdiv;
+		if (abs(rate - tmp) < abs(rate - best)) {
+			best = tmp;
+			best_sysdiv = _sysdiv;
+		}
+
+		if (tmp == rate)
+			goto out;
+	}
+
+out:
+	*sysdiv = best_sysdiv;
+	return best;
+}
+
+/*
+ * FIXME: This is supposed to be ranging from 1 to 8, but the value is
+ * always set to 3 in the vendor kernels.
+ */
+#define OV5640_PLL_PREDIV_MIN	3
+#define OV5640_PLL_PREDIV_MAX	3
+
+/*
+ * FIXME: This is supposed to be ranging from 1 to 2, but the value is
+ * always set to 1 in the vendor kernels.
+ */
+#define OV5640_PLL_ROOT_DIV_MIN	1
+#define OV5640_PLL_ROOT_DIV_MAX	1
+
+#define OV5640_PLL_MULT_MIN	4
+#define OV5640_PLL_MULT_MAX	252
+
+static unsigned long ov5640_calc_pll(struct ov5640_dev *sensor,
+				     unsigned long rate,
+				     u8 *sysdiv, u8 *prediv, u8 *rdiv, u8 *mult)
+{
+	unsigned long best = ~0;
+	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_rdiv = 1;
+	u8 _prediv, _mult, _rdiv;
+
+	for (_prediv = OV5640_PLL_PREDIV_MIN;
+	     _prediv <= OV5640_PLL_PREDIV_MAX;
+	     _prediv++) {
+		for (_mult = OV5640_PLL_MULT_MIN;
+		     _mult <= OV5640_PLL_MULT_MAX;
+		     _mult++) {
+			for (_rdiv = OV5640_PLL_ROOT_DIV_MIN;
+			     _rdiv <= OV5640_PLL_ROOT_DIV_MAX;
+			     _rdiv++) {
+				unsigned long pll;
+				unsigned long sysclk;
+				u8 _sysdiv;
+
+				/*
+				 * The PLL multiplier cannot be odd if
+				 * above 127.
+				 */
+				if (_mult > 127 && !(_mult % 2))
+					continue;
+
+				sysclk = rate * _prediv * _rdiv / _mult;
+				sysclk = ov5640_calc_sysclk(sensor, sysclk,
+							    &_sysdiv);
+
+				pll = sysclk / _rdiv / _prediv * _mult;
+				if (abs(rate - pll) < abs(rate - best)) {
+					best = pll;
+					best_sysdiv = _sysdiv;
+					best_prediv = _prediv;
+					best_mult = _mult;
+					best_rdiv = _rdiv;
+				}
+
+				if (pll == rate)
+					goto out;
+			}
+		}
+	}
+
+out:
+	*sysdiv = best_sysdiv;
+	*prediv = best_prediv;
+	*mult = best_mult;
+	*rdiv = best_rdiv;
+
+	return best;
+}
+
+/*
+ * FIXME: 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_MIN	1
+#define OV5640_PCLK_ROOT_DIV_MAX	1
+
+static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
+				      unsigned long rate,
+				      u8 *sysdiv, u8 *prediv, u8 *pll_rdiv,
+				      u8 *mult, u8 *pclk_rdiv)
+{
+	unsigned long best = ~0;
+	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_pll_rdiv = 1;
+	u8 best_pclk_rdiv = 1;
+	u8 _pclk_rdiv;
+
+	for (_pclk_rdiv = OV5640_PCLK_ROOT_DIV_MIN;
+	     _pclk_rdiv <= OV5640_PCLK_ROOT_DIV_MAX;
+	     _pclk_rdiv <<= 1) {
+		unsigned long pll, pclk;
+		u8 sysdiv, prediv, mult, pll_rdiv;
+
+		pll = rate * OV5640_SCLK_ROOT_DIVIDER_DEFAULT * _pclk_rdiv;
+		pll = ov5640_calc_pll(sensor, pll, &sysdiv, &prediv, &pll_rdiv,
+				      &mult);
+
+		pclk = pll / OV5640_SCLK_ROOT_DIVIDER_DEFAULT / _pclk_rdiv;
+		if (abs(rate - pclk) < abs(rate - best)) {
+			best = pclk;
+			best_sysdiv = sysdiv;
+			best_prediv = prediv;
+			best_pll_rdiv = pll_rdiv;
+			best_pclk_rdiv = _pclk_rdiv;
+			best_mult = mult;
+		}
+
+		if (pclk == rate)
+			goto out;
+	}
+
+out:
+	*sysdiv = best_sysdiv;
+	*prediv = best_prediv;
+	*pll_rdiv = best_pll_rdiv;
+	*mult = best_mult;
+	*pclk_rdiv = best_pclk_rdiv;
+	return best;
+}
+
+static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor, unsigned long rate)
+{
+	u8 sysdiv, prediv, mult, pll_rdiv, pclk_rdiv;
+	int ret;
+
+	ov5640_calc_pclk(sensor, rate, &sysdiv, &prediv, &pll_rdiv, &mult,
+			 &pclk_rdiv);
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1,
+			     0xf0, sysdiv << 4);
+	if (ret)
+		return ret;
+
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL2,
+			     0xff, mult);
+	if (ret)
+		return ret;
+
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3,
+			     0xff, prediv | ((pll_rdiv - 1) << 4));
+	if (ret)
+		return ret;
+
+	return ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER,
+			      0x30, ilog2(pclk_rdiv) << 4);
+}
+
+static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor, unsigned long rate)
+{
+	u8 sysdiv, prediv, mult, pll_rdiv, pclk_rdiv;
+	int ret;
+
+	ov5640_calc_pclk(sensor, rate, &sysdiv, &prediv, &pll_rdiv, &mult,
+			 &pclk_rdiv);
+	ret = ov5640_write_reg(sensor, OV5640_REG_SC_PLL_CTRL1,
+			       (sysdiv << 4) | pclk_rdiv);
+	if (ret)
+		return ret;
+
+	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL2,
+			     0xff, mult);
+	if (ret)
+		return ret;
+
+	return ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3,
+			      0xff, prediv | ((pll_rdiv - 1) << 4));
+}
+
 /* download ov5640 settings to sensor through i2c */
 static int ov5640_load_regs(struct ov5640_dev *sensor,
 			    const struct ov5640_mode_info *mode)
@@ -1610,6 +1868,14 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
 	if (ret)
 		return ret;
 
+	if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
+		ret = ov5640_set_mipi_pclk(sensor, mode->clock);
+	else
+		ret = ov5640_set_dvp_pclk(sensor, mode->clock);
+
+	if (ret < 0)
+		return 0;
+
 	if ((dn_mode == SUBSAMPLING && orig_dn_mode == SCALING) ||
 	    (dn_mode == SCALING && orig_dn_mode == SUBSAMPLING)) {
 		/*
-- 
2.14.3

Re: [PATCH 08/12] media: ov5640: Adjust the clock based on the expected rate

[Sakari Ailus]

Hi Maxime,

On Fri, Mar 02, 2018 at 03:34:56PM +0100, Maxime Ripard wrote:
...
> @@ -902,6 +920,246 @@ static int ov5640_mod_reg(struct ov5640_dev *sensor, u16 reg,
>  	return ov5640_write_reg(sensor, reg, val);
>  }
>  
> +/*
> + * After spending way too much time trying the various combinations, I
> + * believe the clock tree is as follows:

Wow! I've never heard of anyone doing this on non-SMIA compliant sensors.

> + *
> + *   +--------------+
> + *   |  Oscillator  |

I wonder if this should be simply called external clock, that's what the
sensor uses.

> + *   +------+-------+
> + *          |
> + *   +------+-------+
> + *   | System clock | - reg 0x3035, bits 4-7
> + *   +------+-------+
> + *          |
> + *   +------+-------+ - reg 0x3036, for the multiplier
> + *   |     PLL      | - reg 0x3037, bits 4 for the root divider
> + *   +------+-------+ - reg 0x3037, bits 0-3 for the pre-divider
> + *          |
> + *   +------+-------+
> + *   |     SCLK     | - reg 0x3108, bits 0-1 for the root divider
> + *   +------+-------+
> + *          |
> + *   +------+-------+
> + *   |    PCLK      | - reg 0x3108, bits 4-5 for the root divider
> + *   +--------------+
> + *
> + * This is deviating from the datasheet at least for the register
> + * 0x3108, since it's said here that the PCLK would be clocked from
> + * the PLL. However, changing the SCLK divider value has a direct
> + * effect on the PCLK rate, which wouldn't be the case if both PCLK
> + * and SCLK were to be sourced from the PLL.
> + *
> + * These parameters also match perfectly the rate that is output by
> + * the sensor, so we shouldn't have too much factors missing (or they
> + * would be set to 1).
> + */
> +
> +/*
> + * FIXME: 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.

There could be limits for the clock rates after the first divider. In
practice the clock rates are mostly one of the common frequencies (9,6; 12;
19,2 or 24 MHz) so there's no need to use the other values.

> + */
> +#define OV5640_SYSDIV_MIN	1
> +#define OV5640_SYSDIV_MAX	2
> +
> +static unsigned long ov5640_calc_sysclk(struct ov5640_dev *sensor,
> +					unsigned long rate,
> +					u8 *sysdiv)
> +{
> +	unsigned long best = ~0;
> +	u8 best_sysdiv = 1;
> +	u8 _sysdiv;
> +
> +	for (_sysdiv = OV5640_SYSDIV_MIN;
> +	     _sysdiv <= OV5640_SYSDIV_MAX;
> +	     _sysdiv++) {
> +		unsigned long tmp;
> +
> +		tmp = sensor->xclk_freq / _sysdiv;
> +		if (abs(rate - tmp) < abs(rate - best)) {
> +			best = tmp;
> +			best_sysdiv = _sysdiv;
> +		}
> +
> +		if (tmp == rate)
> +			goto out;
> +	}
> +
> +out:
> +	*sysdiv = best_sysdiv;
> +	return best;
> +}
> +
> +/*
> + * FIXME: This is supposed to be ranging from 1 to 8, but the value is
> + * always set to 3 in the vendor kernels.
> + */
> +#define OV5640_PLL_PREDIV_MIN	3
> +#define OV5640_PLL_PREDIV_MAX	3


Same reasoning here than above. I might leave a comment documenting the
values the hardware supports, removing FIXME as this isn't really an issue
as far as I see.

> +
> +/*
> + * FIXME: This is supposed to be ranging from 1 to 2, but the value is
> + * always set to 1 in the vendor kernels.
> + */
> +#define OV5640_PLL_ROOT_DIV_MIN	1
> +#define OV5640_PLL_ROOT_DIV_MAX	1
> +
> +#define OV5640_PLL_MULT_MIN	4
> +#define OV5640_PLL_MULT_MAX	252
> +
> +static unsigned long ov5640_calc_pll(struct ov5640_dev *sensor,
> +				     unsigned long rate,
> +				     u8 *sysdiv, u8 *prediv, u8 *rdiv, u8 *mult)
> +{
> +	unsigned long best = ~0;
> +	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_rdiv = 1;
> +	u8 _prediv, _mult, _rdiv;
> +
> +	for (_prediv = OV5640_PLL_PREDIV_MIN;
> +	     _prediv <= OV5640_PLL_PREDIV_MAX;
> +	     _prediv++) {
> +		for (_mult = OV5640_PLL_MULT_MIN;
> +		     _mult <= OV5640_PLL_MULT_MAX;
> +		     _mult++) {
> +			for (_rdiv = OV5640_PLL_ROOT_DIV_MIN;
> +			     _rdiv <= OV5640_PLL_ROOT_DIV_MAX;
> +			     _rdiv++) {
> +				unsigned long pll;
> +				unsigned long sysclk;
> +				u8 _sysdiv;
> +
> +				/*
> +				 * The PLL multiplier cannot be odd if
> +				 * above 127.
> +				 */
> +				if (_mult > 127 && !(_mult % 2))
> +					continue;
> +
> +				sysclk = rate * _prediv * _rdiv / _mult;
> +				sysclk = ov5640_calc_sysclk(sensor, sysclk,
> +							    &_sysdiv);
> +
> +				pll = sysclk / _rdiv / _prediv * _mult;
> +				if (abs(rate - pll) < abs(rate - best)) {
> +					best = pll;
> +					best_sysdiv = _sysdiv;
> +					best_prediv = _prediv;
> +					best_mult = _mult;
> +					best_rdiv = _rdiv;

The smiapp PLL calculator only accepts an exact match. That hasn't been an
issue previously, I wonder if this would work here, too.

I think you could remove the inner loop, too, if put what
ov5640_calc_sysclk() does here. You know the desired clock rate so you can
calculate the total divisor (_rdiv * sysclk divider).

> +				}
> +
> +				if (pll == rate)
> +					goto out;
> +			}
> +		}
> +	}
> +
> +out:
> +	*sysdiv = best_sysdiv;
> +	*prediv = best_prediv;
> +	*mult = best_mult;
> +	*rdiv = best_rdiv;
> +
> +	return best;
> +}
> +
> +/*
> + * FIXME: 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_MIN	1
> +#define OV5640_PCLK_ROOT_DIV_MAX	1
> +
> +static unsigned long ov5640_calc_pclk(struct ov5640_dev *sensor,
> +				      unsigned long rate,
> +				      u8 *sysdiv, u8 *prediv, u8 *pll_rdiv,
> +				      u8 *mult, u8 *pclk_rdiv)
> +{
> +	unsigned long best = ~0;
> +	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_pll_rdiv = 1;
> +	u8 best_pclk_rdiv = 1;
> +	u8 _pclk_rdiv;
> +
> +	for (_pclk_rdiv = OV5640_PCLK_ROOT_DIV_MIN;
> +	     _pclk_rdiv <= OV5640_PCLK_ROOT_DIV_MAX;
> +	     _pclk_rdiv <<= 1) {
> +		unsigned long pll, pclk;
> +		u8 sysdiv, prediv, mult, pll_rdiv;
> +
> +		pll = rate * OV5640_SCLK_ROOT_DIVIDER_DEFAULT * _pclk_rdiv;
> +		pll = ov5640_calc_pll(sensor, pll, &sysdiv, &prediv, &pll_rdiv,
> +				      &mult);
> +
> +		pclk = pll / OV5640_SCLK_ROOT_DIVIDER_DEFAULT / _pclk_rdiv;
> +		if (abs(rate - pclk) < abs(rate - best)) {
> +			best = pclk;
> +			best_sysdiv = sysdiv;
> +			best_prediv = prediv;
> +			best_pll_rdiv = pll_rdiv;
> +			best_pclk_rdiv = _pclk_rdiv;
> +			best_mult = mult;
> +		}
> +
> +		if (pclk == rate)
> +			goto out;
> +	}
> +
> +out:
> +	*sysdiv = best_sysdiv;
> +	*prediv = best_prediv;
> +	*pll_rdiv = best_pll_rdiv;
> +	*mult = best_mult;
> +	*pclk_rdiv = best_pclk_rdiv;
> +	return best;
> +}
> +
> +static int ov5640_set_dvp_pclk(struct ov5640_dev *sensor, unsigned long rate)
> +{
> +	u8 sysdiv, prediv, mult, pll_rdiv, pclk_rdiv;
> +	int ret;
> +
> +	ov5640_calc_pclk(sensor, rate, &sysdiv, &prediv, &pll_rdiv, &mult,
> +			 &pclk_rdiv);
> +	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1,
> +			     0xf0, sysdiv << 4);
> +	if (ret)
> +		return ret;
> +
> +	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL2,
> +			     0xff, mult);
> +	if (ret)
> +		return ret;
> +
> +	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3,
> +			     0xff, prediv | ((pll_rdiv - 1) << 4));
> +	if (ret)
> +		return ret;
> +
> +	return ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER,
> +			      0x30, ilog2(pclk_rdiv) << 4);
> +}
> +
> +static int ov5640_set_mipi_pclk(struct ov5640_dev *sensor, unsigned long rate)
> +{
> +	u8 sysdiv, prediv, mult, pll_rdiv, pclk_rdiv;
> +	int ret;
> +
> +	ov5640_calc_pclk(sensor, rate, &sysdiv, &prediv, &pll_rdiv, &mult,
> +			 &pclk_rdiv);
> +	ret = ov5640_write_reg(sensor, OV5640_REG_SC_PLL_CTRL1,
> +			       (sysdiv << 4) | pclk_rdiv);
> +	if (ret)
> +		return ret;
> +
> +	ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL2,
> +			     0xff, mult);
> +	if (ret)
> +		return ret;
> +
> +	return ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL3,
> +			      0xff, prediv | ((pll_rdiv - 1) << 4));
> +}
> +
>  /* download ov5640 settings to sensor through i2c */
>  static int ov5640_load_regs(struct ov5640_dev *sensor,
>  			    const struct ov5640_mode_info *mode)
> @@ -1610,6 +1868,14 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>  	if (ret)
>  		return ret;
>  
> +	if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
> +		ret = ov5640_set_mipi_pclk(sensor, mode->clock);
> +	else
> +		ret = ov5640_set_dvp_pclk(sensor, mode->clock);
> +
> +	if (ret < 0)
> +		return 0;
> +
>  	if ((dn_mode == SUBSAMPLING && orig_dn_mode == SCALING) ||
>  	    (dn_mode == SCALING && orig_dn_mode == SUBSAMPLING)) {
>  		/*

-- 
Kind regards,

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

Re: [PATCH 08/12] media: ov5640: Adjust the clock based on the expected rate

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 6008 bytes --]

Hi Sakari,

On Fri, Mar 09, 2018 at 01:16:24PM +0200, Sakari Ailus wrote:
> > + *
> > + *   +--------------+
> > + *   |  Oscillator  |
> 
> I wonder if this should be simply called external clock, that's what the
> sensor uses.

Ack

> > + *   +------+-------+
> > + *          |
> > + *   +------+-------+
> > + *   | System clock | - reg 0x3035, bits 4-7
> > + *   +------+-------+
> > + *          |
> > + *   +------+-------+ - reg 0x3036, for the multiplier
> > + *   |     PLL      | - reg 0x3037, bits 4 for the root divider
> > + *   +------+-------+ - reg 0x3037, bits 0-3 for the pre-divider
> > + *          |
> > + *   +------+-------+
> > + *   |     SCLK     | - reg 0x3108, bits 0-1 for the root divider
> > + *   +------+-------+
> > + *          |
> > + *   +------+-------+
> > + *   |    PCLK      | - reg 0x3108, bits 4-5 for the root divider
> > + *   +--------------+
> > + *
> > + * This is deviating from the datasheet at least for the register
> > + * 0x3108, since it's said here that the PCLK would be clocked from
> > + * the PLL. However, changing the SCLK divider value has a direct
> > + * effect on the PCLK rate, which wouldn't be the case if both PCLK
> > + * and SCLK were to be sourced from the PLL.
> > + *
> > + * These parameters also match perfectly the rate that is output by
> > + * the sensor, so we shouldn't have too much factors missing (or they
> > + * would be set to 1).
> > + */
> > +
> > +/*
> > + * FIXME: 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.
> 
> There could be limits for the clock rates after the first divider. In
> practice the clock rates are mostly one of the common frequencies (9,6; 12;
> 19,2 or 24 MHz) so there's no need to use the other values.

There's probably some limits on the various combinations as well. I
first tried to use the full range, and there was some combinations
that were not usable, even though the clock rate should have been
correct.

> > + */
> > +#define OV5640_SYSDIV_MIN	1
> > +#define OV5640_SYSDIV_MAX	2
> > +
> > +static unsigned long ov5640_calc_sysclk(struct ov5640_dev *sensor,
> > +					unsigned long rate,
> > +					u8 *sysdiv)
> > +{
> > +	unsigned long best = ~0;
> > +	u8 best_sysdiv = 1;
> > +	u8 _sysdiv;
> > +
> > +	for (_sysdiv = OV5640_SYSDIV_MIN;
> > +	     _sysdiv <= OV5640_SYSDIV_MAX;
> > +	     _sysdiv++) {
> > +		unsigned long tmp;
> > +
> > +		tmp = sensor->xclk_freq / _sysdiv;
> > +		if (abs(rate - tmp) < abs(rate - best)) {
> > +			best = tmp;
> > +			best_sysdiv = _sysdiv;
> > +		}
> > +
> > +		if (tmp == rate)
> > +			goto out;
> > +	}
> > +
> > +out:
> > +	*sysdiv = best_sysdiv;
> > +	return best;
> > +}
> > +
> > +/*
> > + * FIXME: This is supposed to be ranging from 1 to 8, but the value is
> > + * always set to 3 in the vendor kernels.
> > + */
> > +#define OV5640_PLL_PREDIV_MIN	3
> > +#define OV5640_PLL_PREDIV_MAX	3
> 
> Same reasoning here than above. I might leave a comment documenting the
> values the hardware supports, removing FIXME as this isn't really an issue
> as far as I see.

Ok, I'll do it then

> > +
> > +/*
> > + * FIXME: This is supposed to be ranging from 1 to 2, but the value is
> > + * always set to 1 in the vendor kernels.
> > + */
> > +#define OV5640_PLL_ROOT_DIV_MIN	1
> > +#define OV5640_PLL_ROOT_DIV_MAX	1
> > +
> > +#define OV5640_PLL_MULT_MIN	4
> > +#define OV5640_PLL_MULT_MAX	252
> > +
> > +static unsigned long ov5640_calc_pll(struct ov5640_dev *sensor,
> > +				     unsigned long rate,
> > +				     u8 *sysdiv, u8 *prediv, u8 *rdiv, u8 *mult)
> > +{
> > +	unsigned long best = ~0;
> > +	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_rdiv = 1;
> > +	u8 _prediv, _mult, _rdiv;
> > +
> > +	for (_prediv = OV5640_PLL_PREDIV_MIN;
> > +	     _prediv <= OV5640_PLL_PREDIV_MAX;
> > +	     _prediv++) {
> > +		for (_mult = OV5640_PLL_MULT_MIN;
> > +		     _mult <= OV5640_PLL_MULT_MAX;
> > +		     _mult++) {
> > +			for (_rdiv = OV5640_PLL_ROOT_DIV_MIN;
> > +			     _rdiv <= OV5640_PLL_ROOT_DIV_MAX;
> > +			     _rdiv++) {
> > +				unsigned long pll;
> > +				unsigned long sysclk;
> > +				u8 _sysdiv;
> > +
> > +				/*
> > +				 * The PLL multiplier cannot be odd if
> > +				 * above 127.
> > +				 */
> > +				if (_mult > 127 && !(_mult % 2))
> > +					continue;
> > +
> > +				sysclk = rate * _prediv * _rdiv / _mult;
> > +				sysclk = ov5640_calc_sysclk(sensor, sysclk,
> > +							    &_sysdiv);
> > +
> > +				pll = sysclk / _rdiv / _prediv * _mult;
> > +				if (abs(rate - pll) < abs(rate - best)) {
> > +					best = pll;
> > +					best_sysdiv = _sysdiv;
> > +					best_prediv = _prediv;
> > +					best_mult = _mult;
> > +					best_rdiv = _rdiv;
> 
> The smiapp PLL calculator only accepts an exact match. That hasn't been an
> issue previously, I wonder if this would work here, too.

I don't recall which one exactly, but I think at least 480p@30fps
wasn't an exact match in our case. Given the insanely high blanking
periods, we might reduce them in order to fall back to something
exact, but I really wanted to be as close to what was already done in
the bytes array as possible, given the already quite intrusive nature
of the patches.

> I think you could remove the inner loop, too, if put what
> ov5640_calc_sysclk() does here. You know the desired clock rate so you can
> calculate the total divisor (_rdiv * sysclk divider).

I guess we can have two approaches here. Since most of the dividers
are fixed, I could have a much simpler code anyway, or I could keep it
that way if we ever want to extend it.

It seems you imply that you'd like something simpler, so I can even
simplify much more than what's already there if you want.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 08/12] media: ov5640: Adjust the clock based on the expected rate

[Sakari Ailus]

On Tue, Mar 13, 2018 at 01:49:37PM +0100, Maxime Ripard wrote:
> Hi Sakari,
> 
> On Fri, Mar 09, 2018 at 01:16:24PM +0200, Sakari Ailus wrote:
> > > + *
> > > + *   +--------------+
> > > + *   |  Oscillator  |
> > 
> > I wonder if this should be simply called external clock, that's what the
> > sensor uses.
> 
> Ack
> 
> > > + *   +------+-------+
> > > + *          |
> > > + *   +------+-------+
> > > + *   | System clock | - reg 0x3035, bits 4-7
> > > + *   +------+-------+
> > > + *          |
> > > + *   +------+-------+ - reg 0x3036, for the multiplier
> > > + *   |     PLL      | - reg 0x3037, bits 4 for the root divider
> > > + *   +------+-------+ - reg 0x3037, bits 0-3 for the pre-divider
> > > + *          |
> > > + *   +------+-------+
> > > + *   |     SCLK     | - reg 0x3108, bits 0-1 for the root divider
> > > + *   +------+-------+
> > > + *          |
> > > + *   +------+-------+
> > > + *   |    PCLK      | - reg 0x3108, bits 4-5 for the root divider
> > > + *   +--------------+
> > > + *
> > > + * This is deviating from the datasheet at least for the register
> > > + * 0x3108, since it's said here that the PCLK would be clocked from
> > > + * the PLL. However, changing the SCLK divider value has a direct
> > > + * effect on the PCLK rate, which wouldn't be the case if both PCLK
> > > + * and SCLK were to be sourced from the PLL.
> > > + *
> > > + * These parameters also match perfectly the rate that is output by
> > > + * the sensor, so we shouldn't have too much factors missing (or they
> > > + * would be set to 1).
> > > + */
> > > +
> > > +/*
> > > + * FIXME: 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.
> > 
> > There could be limits for the clock rates after the first divider. In
> > practice the clock rates are mostly one of the common frequencies (9,6; 12;
> > 19,2 or 24 MHz) so there's no need to use the other values.
> 
> There's probably some limits on the various combinations as well. I
> first tried to use the full range, and there was some combinations
> that were not usable, even though the clock rate should have been
> correct.

Apart from the multipliers and dividers, each node in the clock tree likely
has minimum and maximum frequencies. One way to try to figure out those
limits is to figure out which frequencies are used in existing mode
definitions. Sensor documentation seldom contains that information.

> 
> > > + */
> > > +#define OV5640_SYSDIV_MIN	1
> > > +#define OV5640_SYSDIV_MAX	2
> > > +
> > > +static unsigned long ov5640_calc_sysclk(struct ov5640_dev *sensor,
> > > +					unsigned long rate,
> > > +					u8 *sysdiv)
> > > +{
> > > +	unsigned long best = ~0;
> > > +	u8 best_sysdiv = 1;
> > > +	u8 _sysdiv;
> > > +
> > > +	for (_sysdiv = OV5640_SYSDIV_MIN;
> > > +	     _sysdiv <= OV5640_SYSDIV_MAX;
> > > +	     _sysdiv++) {
> > > +		unsigned long tmp;
> > > +
> > > +		tmp = sensor->xclk_freq / _sysdiv;
> > > +		if (abs(rate - tmp) < abs(rate - best)) {
> > > +			best = tmp;
> > > +			best_sysdiv = _sysdiv;
> > > +		}
> > > +
> > > +		if (tmp == rate)
> > > +			goto out;
> > > +	}
> > > +
> > > +out:
> > > +	*sysdiv = best_sysdiv;
> > > +	return best;
> > > +}
> > > +
> > > +/*
> > > + * FIXME: This is supposed to be ranging from 1 to 8, but the value is
> > > + * always set to 3 in the vendor kernels.
> > > + */
> > > +#define OV5640_PLL_PREDIV_MIN	3
> > > +#define OV5640_PLL_PREDIV_MAX	3
> > 
> > Same reasoning here than above. I might leave a comment documenting the
> > values the hardware supports, removing FIXME as this isn't really an issue
> > as far as I see.
> 
> Ok, I'll do it then
> 
> > > +
> > > +/*
> > > + * FIXME: This is supposed to be ranging from 1 to 2, but the value is
> > > + * always set to 1 in the vendor kernels.
> > > + */
> > > +#define OV5640_PLL_ROOT_DIV_MIN	1
> > > +#define OV5640_PLL_ROOT_DIV_MAX	1
> > > +
> > > +#define OV5640_PLL_MULT_MIN	4
> > > +#define OV5640_PLL_MULT_MAX	252
> > > +
> > > +static unsigned long ov5640_calc_pll(struct ov5640_dev *sensor,
> > > +				     unsigned long rate,
> > > +				     u8 *sysdiv, u8 *prediv, u8 *rdiv, u8 *mult)
> > > +{
> > > +	unsigned long best = ~0;
> > > +	u8 best_sysdiv = 1, best_prediv = 1, best_mult = 1, best_rdiv = 1;
> > > +	u8 _prediv, _mult, _rdiv;
> > > +
> > > +	for (_prediv = OV5640_PLL_PREDIV_MIN;
> > > +	     _prediv <= OV5640_PLL_PREDIV_MAX;
> > > +	     _prediv++) {
> > > +		for (_mult = OV5640_PLL_MULT_MIN;
> > > +		     _mult <= OV5640_PLL_MULT_MAX;
> > > +		     _mult++) {
> > > +			for (_rdiv = OV5640_PLL_ROOT_DIV_MIN;
> > > +			     _rdiv <= OV5640_PLL_ROOT_DIV_MAX;
> > > +			     _rdiv++) {
> > > +				unsigned long pll;
> > > +				unsigned long sysclk;
> > > +				u8 _sysdiv;
> > > +
> > > +				/*
> > > +				 * The PLL multiplier cannot be odd if
> > > +				 * above 127.
> > > +				 */
> > > +				if (_mult > 127 && !(_mult % 2))
> > > +					continue;
> > > +
> > > +				sysclk = rate * _prediv * _rdiv / _mult;
> > > +				sysclk = ov5640_calc_sysclk(sensor, sysclk,
> > > +							    &_sysdiv);
> > > +
> > > +				pll = sysclk / _rdiv / _prediv * _mult;
> > > +				if (abs(rate - pll) < abs(rate - best)) {
> > > +					best = pll;
> > > +					best_sysdiv = _sysdiv;
> > > +					best_prediv = _prediv;
> > > +					best_mult = _mult;
> > > +					best_rdiv = _rdiv;
> > 
> > The smiapp PLL calculator only accepts an exact match. That hasn't been an
> > issue previously, I wonder if this would work here, too.
> 
> I don't recall which one exactly, but I think at least 480p@30fps
> wasn't an exact match in our case. Given the insanely high blanking
> periods, we might reduce them in order to fall back to something
> exact, but I really wanted to be as close to what was already done in
> the bytes array as possible, given the already quite intrusive nature
> of the patches.

:-)

What's the target CSI-2 bus frequency (or pixel rate)? If you configure the
PLL without one, then you could end up having as high frequencies the
sensor allows but the host might not be able to handle them.

The smiapp driver uses a list of allowed frequencies for a given board
which effectively also ensures also EMC compatibility.

> 
> > I think you could remove the inner loop, too, if put what
> > ov5640_calc_sysclk() does here. You know the desired clock rate so you can
> > calculate the total divisor (_rdiv * sysclk divider).
> 
> I guess we can have two approaches here. Since most of the dividers
> are fixed, I could have a much simpler code anyway, or I could keep it
> that way if we ever want to extend it.
> 
> It seems you imply that you'd like something simpler, so I can even
> simplify much more than what's already there if you want.

It just seems that the complexity of this loop is somewhere around O(n^6)
even though most variables have a rather small set of possible values. The
et8ek8 driver used to have a mode list with similar complexity and the
handling of which actually noticeably slowed down the system. :-)

-- 
Regards,

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

[PATCH 09/12] media: ov5640: Compute the clock rate at runtime

[Maxime Ripard]

The clock rate, while hardcoded until now, is actually a function of the
resolution, framerate and bytes per pixel. Now that we have an algorithm to
adjust our clock rate, we can select it dynamically when we change the
mode.

This changes a bit the clock rate being used, with the following effect:

+------+------+------+------+-----+-----------------+----------------+-----------+
| Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | Deviation |
+------+------+------+------+-----+-----------------+----------------+-----------+
|  640 |  480 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
|  640 |  480 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
| 1024 |  768 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
| 1024 |  768 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
|  320 |  240 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
|  320 |  240 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
|  176 |  144 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
|  176 |  144 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
|  720 |  480 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
|  720 |  480 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
|  720 |  576 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
|  720 |  576 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
| 1280 |  720 | 1892 |  740 |  15 |        42000000 |       42002400 | 0.01 %    |
| 1280 |  720 | 1892 |  740 |  30 |        84000000 |       84004800 | 0.01 %    |
| 1920 | 1080 | 2500 | 1120 |  15 |        84000000 |       84000000 | 0.00 %    |
| 1920 | 1080 | 2500 | 1120 |  30 |       168000000 |      168000000 | 0.00 %    |
| 2592 | 1944 | 2844 | 1944 |  15 |        84000000 |      165862080 | 49.36 %   |
+------+------+------+------+-----+-----------------+----------------+-----------+

Only the 640x480, 1024x768 and 2592x1944 modes are significantly affected
by the new formula.

In this case, 640x480 and 1024x768 are actually fixed by this driver.
Indeed, the sensor was sending data at, for example, 27.33fps instead of
30fps. This is -9%, which is roughly what we're seeing in the array.
Testing these modes with the new clock setup actually fix that error, and
data are now sent at around 30fps.

2592x1944, on the other hand, is probably due to the fact that this mode
can only be used using MIPI-CSI2, in a two lane mode. This would have to be
tested though.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 41 +++++++++++++++++------------------------
 1 file changed, 17 insertions(+), 24 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 323cde27dd8b..bdf378d80e07 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -126,6 +126,12 @@ static const struct ov5640_pixfmt ov5640_formats[] = {
 	{ MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
 };
 
+/*
+ * FIXME: If we ever have something else, we'll obviously need to have
+ * something smarter.
+ */
+#define OV5640_FORMATS_BPP	2
+
 /*
  * FIXME: remove this when a subdev API becomes available
  * to set the MIPI CSI-2 virtual channel.
@@ -172,7 +178,6 @@ struct ov5640_mode_info {
 	u32 htot;
 	u32 vact;
 	u32 vtot;
-	u32 clock;
 	const struct reg_value *reg_data;
 	u32 reg_data_size;
 };
@@ -696,7 +701,6 @@ 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,
-	112000000,
 	ov5640_init_setting_30fps_VGA,
 	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
 };
@@ -706,91 +710,74 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
 	{
 		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 		 176, 1896, 144, 984,
-		 56000000,
 		 ov5640_setting_15fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
 		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 		 320, 1896, 240, 984,
-		 56000000,
 		 ov5640_setting_15fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
 		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 		 640, 1896, 480, 1080,
-		 56000000,
 		 ov5640_setting_15fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
 		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 		 720, 1896, 480, 984,
-		 56000000,
 		 ov5640_setting_15fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
 		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 		 720, 1896, 576, 984,
-		 56000000,
 		 ov5640_setting_15fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
 		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 		 1024, 1896, 768, 1080,
-		 56000000,
 		 ov5640_setting_15fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
 		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 		 1280, 1892, 720, 740,
-		 42000000,
 		 ov5640_setting_15fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
 		{OV5640_MODE_1080P_1920_1080, SCALING,
 		 1920, 2500, 1080, 1120,
-		 84000000,
 		 ov5640_setting_15fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
 		{OV5640_MODE_QSXGA_2592_1944, SCALING,
 		 2592, 2844, 1944, 1968,
-		 168000000,
 		 ov5640_setting_15fps_QSXGA_2592_1944,
 		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
 	}, {
 		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
 		 176, 1896, 144, 984,
-		 112000000,
 		 ov5640_setting_30fps_QCIF_176_144,
 		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
 		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
 		 320, 1896, 240, 984,
-		 112000000,
 		 ov5640_setting_30fps_QVGA_320_240,
 		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
 		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
 		 640, 1896, 480, 1080,
-		 112000000,
 		 ov5640_setting_30fps_VGA_640_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
 		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
 		 720, 1896, 480, 984,
-		 112000000,
 		 ov5640_setting_30fps_NTSC_720_480,
 		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
 		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
 		 720, 1896, 576, 984,
-		 112000000,
 		 ov5640_setting_30fps_PAL_720_576,
 		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
 		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
 		 1024, 1896, 768, 1080,
-		 112000000,
 		 ov5640_setting_30fps_XGA_1024_768,
 		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
 		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
 		 1280, 1892, 720, 740,
-		 84000000,
 		 ov5640_setting_30fps_720P_1280_720,
 		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
 		{OV5640_MODE_1080P_1920_1080, SCALING,
 		 1920, 2500, 1080, 1120,
-		 168000000,
 		 ov5640_setting_30fps_1080P_1920_1080,
 		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, 0, NULL, 0},
+		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
 	},
 };
 
@@ -1854,6 +1841,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
 {
 	const struct ov5640_mode_info *mode = sensor->current_mode;
 	enum ov5640_downsize_mode dn_mode, orig_dn_mode;
+	unsigned long rate;
 	int ret;
 
 	dn_mode = mode->dn_mode;
@@ -1868,10 +1856,15 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
 	if (ret)
 		return ret;
 
-	if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
-		ret = ov5640_set_mipi_pclk(sensor, mode->clock);
-	else
-		ret = ov5640_set_dvp_pclk(sensor, mode->clock);
+	rate = mode->vtot * mode->htot * OV5640_FORMATS_BPP;
+	rate *= ov5640_framerates[sensor->current_fr];
+
+	if (sensor->ep.bus_type == V4L2_MBUS_CSI2) {
+		rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
+		ret = ov5640_set_mipi_pclk(sensor, rate);
+	} else {
+		ret = ov5640_set_dvp_pclk(sensor, rate);
+	}
 
 	if (ret < 0)
 		return 0;
-- 
2.14.3

Re: [PATCH 09/12] media: ov5640: Compute the clock rate at runtime

[Hugues FRUCHET]

Hi Maxime,

Below comments about JPEG issue.

On 03/02/2018 03:34 PM, Maxime Ripard wrote:
> The clock rate, while hardcoded until now, is actually a function of the
> resolution, framerate and bytes per pixel. Now that we have an algorithm to
> adjust our clock rate, we can select it dynamically when we change the
> mode.
> 
> This changes a bit the clock rate being used, with the following effect:
> 
> +------+------+------+------+-----+-----------------+----------------+-----------+
> | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | Deviation |
> +------+------+------+------+-----+-----------------+----------------+-----------+
> |  640 |  480 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
> |  640 |  480 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
> | 1024 |  768 | 1896 | 1080 |  15 |        56000000 |       61430400 | 8.84 %    |
> | 1024 |  768 | 1896 | 1080 |  30 |       112000000 |      122860800 | 8.84 %    |
> |  320 |  240 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> |  320 |  240 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> |  176 |  144 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> |  176 |  144 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> |  720 |  480 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> |  720 |  480 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> |  720 |  576 | 1896 |  984 |  15 |        56000000 |       55969920 | 0.05 %    |
> |  720 |  576 | 1896 |  984 |  30 |       112000000 |      111939840 | 0.05 %    |
> | 1280 |  720 | 1892 |  740 |  15 |        42000000 |       42002400 | 0.01 %    |
> | 1280 |  720 | 1892 |  740 |  30 |        84000000 |       84004800 | 0.01 %    |
> | 1920 | 1080 | 2500 | 1120 |  15 |        84000000 |       84000000 | 0.00 %    |
> | 1920 | 1080 | 2500 | 1120 |  30 |       168000000 |      168000000 | 0.00 %    |
> | 2592 | 1944 | 2844 | 1944 |  15 |        84000000 |      165862080 | 49.36 %   |
> +------+------+------+------+-----+-----------------+----------------+-----------+
> 
> Only the 640x480, 1024x768 and 2592x1944 modes are significantly affected
> by the new formula.
> 
> In this case, 640x480 and 1024x768 are actually fixed by this driver.
> Indeed, the sensor was sending data at, for example, 27.33fps instead of
> 30fps. This is -9%, which is roughly what we're seeing in the array.
> Testing these modes with the new clock setup actually fix that error, and
> data are now sent at around 30fps.
> 
> 2592x1944, on the other hand, is probably due to the fact that this mode
> can only be used using MIPI-CSI2, in a two lane mode. This would have to be
> tested though.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>   drivers/media/i2c/ov5640.c | 41 +++++++++++++++++------------------------
>   1 file changed, 17 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 323cde27dd8b..bdf378d80e07 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -126,6 +126,12 @@ static const struct ov5640_pixfmt ov5640_formats[] = {
>   	{ MEDIA_BUS_FMT_RGB565_2X8_BE, V4L2_COLORSPACE_SRGB, },
>   };
>   
> +/*
> + * FIXME: If we ever have something else, we'll obviously need to have
> + * something smarter.
> + */
> +#define OV5640_FORMATS_BPP	2
> +

We have the case with JPEG which is 1 byte.

>   /*
>    * FIXME: remove this when a subdev API becomes available
>    * to set the MIPI CSI-2 virtual channel.
> @@ -172,7 +178,6 @@ struct ov5640_mode_info {
>   	u32 htot;
>   	u32 vact;
>   	u32 vtot;
> -	u32 clock;
>   	const struct reg_value *reg_data;
>   	u32 reg_data_size;
>   };
> @@ -696,7 +701,6 @@ 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,
> -	112000000,
>   	ov5640_init_setting_30fps_VGA,
>   	ARRAY_SIZE(ov5640_init_setting_30fps_VGA),
>   };
> @@ -706,91 +710,74 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
>   	{
>   		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>   		 176, 1896, 144, 984,
> -		 56000000,
>   		 ov5640_setting_15fps_QCIF_176_144,
>   		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
>   		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>   		 320, 1896, 240, 984,
> -		 56000000,
>   		 ov5640_setting_15fps_QVGA_320_240,
>   		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
>   		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
>   		 640, 1896, 480, 1080,
> -		 56000000,
>   		 ov5640_setting_15fps_VGA_640_480,
>   		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
>   		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>   		 720, 1896, 480, 984,
> -		 56000000,
>   		 ov5640_setting_15fps_NTSC_720_480,
>   		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
>   		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
>   		 720, 1896, 576, 984,
> -		 56000000,
>   		 ov5640_setting_15fps_PAL_720_576,
>   		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
>   		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>   		 1024, 1896, 768, 1080,
> -		 56000000,
>   		 ov5640_setting_15fps_XGA_1024_768,
>   		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
>   		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
>   		 1280, 1892, 720, 740,
> -		 42000000,
>   		 ov5640_setting_15fps_720P_1280_720,
>   		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
>   		{OV5640_MODE_1080P_1920_1080, SCALING,
>   		 1920, 2500, 1080, 1120,
> -		 84000000,
>   		 ov5640_setting_15fps_1080P_1920_1080,
>   		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
>   		{OV5640_MODE_QSXGA_2592_1944, SCALING,
>   		 2592, 2844, 1944, 1968,
> -		 168000000,
>   		 ov5640_setting_15fps_QSXGA_2592_1944,
>   		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
>   	}, {
>   		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
>   		 176, 1896, 144, 984,
> -		 112000000,
>   		 ov5640_setting_30fps_QCIF_176_144,
>   		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
>   		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
>   		 320, 1896, 240, 984,
> -		 112000000,
>   		 ov5640_setting_30fps_QVGA_320_240,
>   		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
>   		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
>   		 640, 1896, 480, 1080,
> -		 112000000,
>   		 ov5640_setting_30fps_VGA_640_480,
>   		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
>   		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
>   		 720, 1896, 480, 984,
> -		 112000000,
>   		 ov5640_setting_30fps_NTSC_720_480,
>   		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
>   		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
>   		 720, 1896, 576, 984,
> -		 112000000,
>   		 ov5640_setting_30fps_PAL_720_576,
>   		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
>   		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
>   		 1024, 1896, 768, 1080,
> -		 112000000,
>   		 ov5640_setting_30fps_XGA_1024_768,
>   		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
>   		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
>   		 1280, 1892, 720, 740,
> -		 84000000,
>   		 ov5640_setting_30fps_720P_1280_720,
>   		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
>   		{OV5640_MODE_1080P_1920_1080, SCALING,
>   		 1920, 2500, 1080, 1120,
> -		 168000000,
>   		 ov5640_setting_30fps_1080P_1920_1080,
>   		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
> -		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, 0, NULL, 0},
> +		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
>   	},
>   };
>   
> @@ -1854,6 +1841,7 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>   {
>   	const struct ov5640_mode_info *mode = sensor->current_mode;
>   	enum ov5640_downsize_mode dn_mode, orig_dn_mode;
> +	unsigned long rate;
>   	int ret;
>   
>   	dn_mode = mode->dn_mode;
> @@ -1868,10 +1856,15 @@ static int ov5640_set_mode(struct ov5640_dev *sensor,
>   	if (ret)
>   		return ret;
>   
> -	if (sensor->ep.bus_type == V4L2_MBUS_CSI2)
> -		ret = ov5640_set_mipi_pclk(sensor, mode->clock);
> -	else
> -		ret = ov5640_set_dvp_pclk(sensor, mode->clock);
> +	rate = mode->vtot * mode->htot * OV5640_FORMATS_BPP;

Proposal:
	rate = mode->vtot * mode->htot *
		(sensor->fmt.code == MEDIA_BUS_FMT_JPEG_1X8 ? 1 : 2);


> +	rate *= ov5640_framerates[sensor->current_fr];
> +
> +	if (sensor->ep.bus_type == V4L2_MBUS_CSI2) {
> +		rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes;
> +		ret = ov5640_set_mipi_pclk(sensor, rate);
> +	} else {
> +		ret = ov5640_set_dvp_pclk(sensor, rate);
> +	}
>   
>   	if (ret < 0)
>   		return 0;
> 

BR
Hugues.

[PATCH 10/12] media: ov5640: Enhance FPS handling

[Maxime Ripard]

Now that we have moved the clock generation logic out of the bytes array,
these arrays are identical between the 15fps and 30fps variants.

Remove the duplicate entries, and convert the code accordingly.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 312 ++++++++-------------------------------------
 1 file changed, 56 insertions(+), 256 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index bdf378d80e07..5510a19281a4 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -344,66 +344,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
 	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
 };
 
-static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
-
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
-	{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}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{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},
-};
-
-static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
-
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
-	{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}, {0x3503, 0x00, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
+static const struct reg_value ov5640_setting_VGA_640_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -422,7 +363,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
+static const struct reg_value ov5640_setting_XGA_1024_768[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -441,7 +382,7 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
+static const struct reg_value ov5640_setting_QVGA_320_240[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -460,25 +401,7 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{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},
-};
-static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
+static const struct reg_value ov5640_setting_QCIF_176_144[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -497,26 +420,7 @@ static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{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},
-};
-
-static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
+static const struct reg_value ov5640_setting_NTSC_720_480[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -535,26 +439,7 @@ static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{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},
-};
-
-static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
+static const struct reg_value ov5640_setting_PAL_720_576[] = {
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -573,28 +458,7 @@ static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
 	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
-	{0x3008, 0x42, 0, 0},
-	{0x3c07, 0x07, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
-	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
-	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x02, 0, 0},
-	{0x3a03, 0xe4, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0xbc, 0, 0},
-	{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}, {0x4005, 0x1a, 0, 0},
-	{0x3008, 0x02, 0, 0}, {0x3503, 0,    0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
+static const struct reg_value ov5640_setting_720P_1280_720[] = {
 	{0x3c07, 0x07, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
 	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
@@ -613,40 +477,7 @@ static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
 	{0x3824, 0x04, 0, 0}, {0x5001, 0x83, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
-	{0x3008, 0x42, 0, 0},
-	{0x3c07, 0x08, 0, 0},
-	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
-	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
-	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
-	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
-	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
-	{0x3810, 0x00, 0, 0},
-	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
-	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
-	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
-	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
-	{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, 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},
-	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
-	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0},
-	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
-	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
-	{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},
-	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0},
-	{0x3503, 0, 0, 0},
-};
-
-static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
+static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{0x3008, 0x42, 0, 0},
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
@@ -678,7 +509,7 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
 	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
 };
 
-static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
+static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
 	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0},
 	{0x3c07, 0x08, 0, 0},
 	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
@@ -706,79 +537,43 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
 };
 
 static const struct ov5640_mode_info
-ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
-	{
-		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
-		 176, 1896, 144, 984,
-		 ov5640_setting_15fps_QCIF_176_144,
-		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
-		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
-		 320, 1896, 240, 984,
-		 ov5640_setting_15fps_QVGA_320_240,
-		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
-		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
-		 640, 1896, 480, 1080,
-		 ov5640_setting_15fps_VGA_640_480,
-		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
-		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
-		 720, 1896, 480, 984,
-		 ov5640_setting_15fps_NTSC_720_480,
-		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
-		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
-		 720, 1896, 576, 984,
-		 ov5640_setting_15fps_PAL_720_576,
-		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
-		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
-		 1024, 1896, 768, 1080,
-		 ov5640_setting_15fps_XGA_1024_768,
-		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
-		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
-		 1280, 1892, 720, 740,
-		 ov5640_setting_15fps_720P_1280_720,
-		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
-		{OV5640_MODE_1080P_1920_1080, SCALING,
-		 1920, 2500, 1080, 1120,
-		 ov5640_setting_15fps_1080P_1920_1080,
-		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, SCALING,
-		 2592, 2844, 1944, 1968,
-		 ov5640_setting_15fps_QSXGA_2592_1944,
-		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
-	}, {
-		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
-		 176, 1896, 144, 984,
-		 ov5640_setting_30fps_QCIF_176_144,
-		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
-		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
-		 320, 1896, 240, 984,
-		 ov5640_setting_30fps_QVGA_320_240,
-		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
-		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
-		 640, 1896, 480, 1080,
-		 ov5640_setting_30fps_VGA_640_480,
-		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
-		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
-		 720, 1896, 480, 984,
-		 ov5640_setting_30fps_NTSC_720_480,
-		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
-		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
-		 720, 1896, 576, 984,
-		 ov5640_setting_30fps_PAL_720_576,
-		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
-		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
-		 1024, 1896, 768, 1080,
-		 ov5640_setting_30fps_XGA_1024_768,
-		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
-		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
-		 1280, 1892, 720, 740,
-		 ov5640_setting_30fps_720P_1280_720,
-		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
-		{OV5640_MODE_1080P_1920_1080, SCALING,
-		 1920, 2500, 1080, 1120,
-		 ov5640_setting_30fps_1080P_1920_1080,
-		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
-		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
-	},
+ov5640_mode_data[OV5640_NUM_MODES] = {
+	{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
+	 176, 1896, 144, 984,
+	 ov5640_setting_QCIF_176_144,
+	 ARRAY_SIZE(ov5640_setting_QCIF_176_144)},
+	{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
+	 320, 1896, 240, 984,
+	 ov5640_setting_QVGA_320_240,
+	 ARRAY_SIZE(ov5640_setting_QVGA_320_240)},
+	{OV5640_MODE_VGA_640_480, SUBSAMPLING,
+	 640, 1896, 480, 1080,
+	 ov5640_setting_VGA_640_480,
+	 ARRAY_SIZE(ov5640_setting_VGA_640_480)},
+	{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
+	 720, 1896, 480, 984,
+	 ov5640_setting_NTSC_720_480,
+	 ARRAY_SIZE(ov5640_setting_NTSC_720_480)},
+	{OV5640_MODE_PAL_720_576, SUBSAMPLING,
+	 720, 1896, 576, 984,
+	 ov5640_setting_PAL_720_576,
+	 ARRAY_SIZE(ov5640_setting_PAL_720_576)},
+	{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
+	 1024, 1896, 768, 1080,
+	 ov5640_setting_XGA_1024_768,
+	 ARRAY_SIZE(ov5640_setting_XGA_1024_768)},
+	{OV5640_MODE_720P_1280_720, SUBSAMPLING,
+	 1280, 1892, 720, 740,
+	 ov5640_setting_720P_1280_720,
+	 ARRAY_SIZE(ov5640_setting_720P_1280_720)},
+	{OV5640_MODE_1080P_1920_1080, SCALING,
+	 1920, 2500, 1080, 1120,
+	 ov5640_setting_1080P_1920_1080,
+	 ARRAY_SIZE(ov5640_setting_1080P_1920_1080)},
+	{OV5640_MODE_QSXGA_2592_1944, SCALING,
+	 2592, 2844, 1944, 1968,
+	 ov5640_setting_QSXGA_2592_1944,
+	 ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944)},
 };
 
 static int ov5640_init_slave_id(struct ov5640_dev *sensor)
@@ -1632,7 +1427,7 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 	int i;
 
 	for (i = OV5640_NUM_MODES - 1; i >= 0; i--) {
-		mode = &ov5640_mode_data[fr][i];
+		mode = &ov5640_mode_data[i];
 
 		if (!mode->reg_data)
 			continue;
@@ -1645,7 +1440,12 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 	}
 
 	if (nearest && i < 0)
-		mode = &ov5640_mode_data[fr][0];
+		mode = &ov5640_mode_data[0];
+
+	/* 2592x1944 can only operate at 15fps */
+	if (width == 2592 && height == 1944 &&
+	    fr != OV5640_15_FPS)
+		return NULL;
 
 	return mode;
 }
@@ -2571,9 +2371,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
 		return -EINVAL;
 
 	fse->min_width = fse->max_width =
-		ov5640_mode_data[0][fse->index].hact;
+		ov5640_mode_data[fse->index].hact;
 	fse->min_height = fse->max_height =
-		ov5640_mode_data[0][fse->index].vact;
+		ov5640_mode_data[fse->index].vact;
 
 	return 0;
 }
@@ -2777,7 +2577,7 @@ static int ov5640_probe(struct i2c_client *client,
 	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
 	sensor->current_fr = OV5640_30_FPS;
 	sensor->current_mode =
-		&ov5640_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
+		&ov5640_mode_data[OV5640_MODE_VGA_640_480];
 	sensor->pending_mode_change = true;
 
 	sensor->ae_target = 52;
-- 
2.14.3

Re: [PATCH 10/12] media: ov5640: Enhance FPS handling

[Hugues FRUCHET]

Hi Maxime,

See below comments related to format compliance failing.

On 03/02/2018 03:34 PM, Maxime Ripard wrote:
> Now that we have moved the clock generation logic out of the bytes array,
> these arrays are identical between the 15fps and 30fps variants.
> 
> Remove the duplicate entries, and convert the code accordingly.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>   drivers/media/i2c/ov5640.c | 312 ++++++++-------------------------------------
>   1 file changed, 56 insertions(+), 256 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index bdf378d80e07..5510a19281a4 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -344,66 +344,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = {
>   	{0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = {
> -
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
> -	{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}, {0x3503, 0x00, 0, 0},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = {
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
> -	{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},
> -};
> -
> -static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = {
> -
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0},
> -	{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}, {0x3503, 0x00, 0, 0},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
> +static const struct reg_value ov5640_setting_VGA_640_480[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -422,7 +363,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
> +static const struct reg_value ov5640_setting_XGA_1024_768[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -441,7 +382,7 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
> +static const struct reg_value ov5640_setting_QVGA_320_240[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -460,25 +401,7 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = {
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
> -	{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},
> -};
> -static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
> +static const struct reg_value ov5640_setting_QCIF_176_144[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -497,26 +420,7 @@ static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_NTSC_720_480[] = {
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x3c, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
> -	{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},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
> +static const struct reg_value ov5640_setting_NTSC_720_480[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -535,26 +439,7 @@ static const struct reg_value ov5640_setting_15fps_NTSC_720_480[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_PAL_720_576[] = {
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x38, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
> -	{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},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
> +static const struct reg_value ov5640_setting_PAL_720_576[] = {
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -573,28 +458,7 @@ static const struct reg_value ov5640_setting_15fps_PAL_720_576[] = {
>   	{0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_720P_1280_720[] = {
> -	{0x3008, 0x42, 0, 0},
> -	{0x3c07, 0x07, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> -	{0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0xfa, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x06, 0, 0}, {0x3807, 0xa9, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
> -	{0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x02, 0, 0},
> -	{0x3a03, 0xe4, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0xbc, 0, 0},
> -	{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}, {0x4005, 0x1a, 0, 0},
> -	{0x3008, 0x02, 0, 0}, {0x3503, 0,    0, 0},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
> +static const struct reg_value ov5640_setting_720P_1280_720[] = {
>   	{0x3c07, 0x07, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
>   	{0x3820, 0x41, 0, 0}, {0x3821, 0x07, 0, 0}, {0x3814, 0x31, 0, 0},
> @@ -613,40 +477,7 @@ static const struct reg_value ov5640_setting_15fps_720P_1280_720[] = {
>   	{0x3824, 0x04, 0, 0}, {0x5001, 0x83, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_30fps_1080P_1920_1080[] = {
> -	{0x3008, 0x42, 0, 0},
> -	{0x3c07, 0x08, 0, 0},
> -	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> -	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0}, {0x3814, 0x11, 0, 0},
> -	{0x3815, 0x11, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0},
> -	{0x3802, 0x00, 0, 0}, {0x3803, 0x00, 0, 0}, {0x3804, 0x0a, 0, 0},
> -	{0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9f, 0, 0},
> -	{0x3810, 0x00, 0, 0},
> -	{0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x04, 0, 0},
> -	{0x3618, 0x04, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x21, 0, 0},
> -	{0x3709, 0x12, 0, 0}, {0x370c, 0x00, 0, 0}, {0x3a02, 0x03, 0, 0},
> -	{0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0},
> -	{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, 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},
> -	{0x3803, 0xb2, 0, 0}, {0x3804, 0x08, 0, 0}, {0x3805, 0xef, 0, 0},
> -	{0x3806, 0x05, 0, 0}, {0x3807, 0xf1, 0, 0},
> -	{0x3612, 0x2b, 0, 0}, {0x3708, 0x64, 0, 0},
> -	{0x3a02, 0x04, 0, 0}, {0x3a03, 0x60, 0, 0}, {0x3a08, 0x01, 0, 0},
> -	{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},
> -	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0},
> -	{0x3503, 0, 0, 0},
> -};
> -
> -static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
> +static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
>   	{0x3008, 0x42, 0, 0},
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> @@ -678,7 +509,7 @@ static const struct reg_value ov5640_setting_15fps_1080P_1920_1080[] = {
>   	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
>   };
>   
> -static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = {
> +static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
>   	{0x3820, 0x40, 0, 0}, {0x3821, 0x06, 0, 0},
>   	{0x3c07, 0x08, 0, 0},
>   	{0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0},
> @@ -706,79 +537,43 @@ static const struct ov5640_mode_info ov5640_mode_init_data = {
>   };
>   
>   static const struct ov5640_mode_info
> -ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = {
> -	{
> -		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
> -		 176, 1896, 144, 984,
> -		 ov5640_setting_15fps_QCIF_176_144,
> -		 ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)},
> -		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> -		 320, 1896, 240, 984,
> -		 ov5640_setting_15fps_QVGA_320_240,
> -		 ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)},
> -		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
> -		 640, 1896, 480, 1080,
> -		 ov5640_setting_15fps_VGA_640_480,
> -		 ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)},
> -		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> -		 720, 1896, 480, 984,
> -		 ov5640_setting_15fps_NTSC_720_480,
> -		 ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)},
> -		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
> -		 720, 1896, 576, 984,
> -		 ov5640_setting_15fps_PAL_720_576,
> -		 ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)},
> -		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> -		 1024, 1896, 768, 1080,
> -		 ov5640_setting_15fps_XGA_1024_768,
> -		 ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)},
> -		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
> -		 1280, 1892, 720, 740,
> -		 ov5640_setting_15fps_720P_1280_720,
> -		 ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)},
> -		{OV5640_MODE_1080P_1920_1080, SCALING,
> -		 1920, 2500, 1080, 1120,
> -		 ov5640_setting_15fps_1080P_1920_1080,
> -		 ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)},
> -		{OV5640_MODE_QSXGA_2592_1944, SCALING,
> -		 2592, 2844, 1944, 1968,
> -		 ov5640_setting_15fps_QSXGA_2592_1944,
> -		 ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)},
> -	}, {
> -		{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
> -		 176, 1896, 144, 984,
> -		 ov5640_setting_30fps_QCIF_176_144,
> -		 ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)},
> -		{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> -		 320, 1896, 240, 984,
> -		 ov5640_setting_30fps_QVGA_320_240,
> -		 ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)},
> -		{OV5640_MODE_VGA_640_480, SUBSAMPLING,
> -		 640, 1896, 480, 1080,
> -		 ov5640_setting_30fps_VGA_640_480,
> -		 ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)},
> -		{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> -		 720, 1896, 480, 984,
> -		 ov5640_setting_30fps_NTSC_720_480,
> -		 ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)},
> -		{OV5640_MODE_PAL_720_576, SUBSAMPLING,
> -		 720, 1896, 576, 984,
> -		 ov5640_setting_30fps_PAL_720_576,
> -		 ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)},
> -		{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> -		 1024, 1896, 768, 1080,
> -		 ov5640_setting_30fps_XGA_1024_768,
> -		 ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)},
> -		{OV5640_MODE_720P_1280_720, SUBSAMPLING,
> -		 1280, 1892, 720, 740,
> -		 ov5640_setting_30fps_720P_1280_720,
> -		 ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)},
> -		{OV5640_MODE_1080P_1920_1080, SCALING,
> -		 1920, 2500, 1080, 1120,
> -		 ov5640_setting_30fps_1080P_1920_1080,
> -		 ARRAY_SIZE(ov5640_setting_30fps_1080P_1920_1080)},
> -		{OV5640_MODE_QSXGA_2592_1944, -1, 0, 0, 0, 0, NULL, 0},
> -	},
> +ov5640_mode_data[OV5640_NUM_MODES] = {
> +	{OV5640_MODE_QCIF_176_144, SUBSAMPLING,
> +	 176, 1896, 144, 984,
> +	 ov5640_setting_QCIF_176_144,
> +	 ARRAY_SIZE(ov5640_setting_QCIF_176_144)},
> +	{OV5640_MODE_QVGA_320_240, SUBSAMPLING,
> +	 320, 1896, 240, 984,
> +	 ov5640_setting_QVGA_320_240,
> +	 ARRAY_SIZE(ov5640_setting_QVGA_320_240)},
> +	{OV5640_MODE_VGA_640_480, SUBSAMPLING,
> +	 640, 1896, 480, 1080,
> +	 ov5640_setting_VGA_640_480,
> +	 ARRAY_SIZE(ov5640_setting_VGA_640_480)},
> +	{OV5640_MODE_NTSC_720_480, SUBSAMPLING,
> +	 720, 1896, 480, 984,
> +	 ov5640_setting_NTSC_720_480,
> +	 ARRAY_SIZE(ov5640_setting_NTSC_720_480)},
> +	{OV5640_MODE_PAL_720_576, SUBSAMPLING,
> +	 720, 1896, 576, 984,
> +	 ov5640_setting_PAL_720_576,
> +	 ARRAY_SIZE(ov5640_setting_PAL_720_576)},
> +	{OV5640_MODE_XGA_1024_768, SUBSAMPLING,
> +	 1024, 1896, 768, 1080,
> +	 ov5640_setting_XGA_1024_768,
> +	 ARRAY_SIZE(ov5640_setting_XGA_1024_768)},
> +	{OV5640_MODE_720P_1280_720, SUBSAMPLING,
> +	 1280, 1892, 720, 740,
> +	 ov5640_setting_720P_1280_720,
> +	 ARRAY_SIZE(ov5640_setting_720P_1280_720)},
> +	{OV5640_MODE_1080P_1920_1080, SCALING,
> +	 1920, 2500, 1080, 1120,
> +	 ov5640_setting_1080P_1920_1080,
> +	 ARRAY_SIZE(ov5640_setting_1080P_1920_1080)},
> +	{OV5640_MODE_QSXGA_2592_1944, SCALING,
> +	 2592, 2844, 1944, 1968,
> +	 ov5640_setting_QSXGA_2592_1944,
> +	 ARRAY_SIZE(ov5640_setting_QSXGA_2592_1944)},
>   };
>   
>   static int ov5640_init_slave_id(struct ov5640_dev *sensor)
> @@ -1632,7 +1427,7 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
>   	int i;
>   
>   	for (i = OV5640_NUM_MODES - 1; i >= 0; i--) {
> -		mode = &ov5640_mode_data[fr][i];
> +		mode = &ov5640_mode_data[i];
>   
>   		if (!mode->reg_data)
>   			continue;
> @@ -1645,7 +1440,12 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
>   	}
>   
>   	if (nearest && i < 0)
> -		mode = &ov5640_mode_data[fr][0];
> +		mode = &ov5640_mode_data[0];
> +
> +	/* 2592x1944 can only operate at 15fps */
> +	if (width == 2592 && height == 1944 &&
> +	    fr != OV5640_15_FPS)
> +		return NULL;

Instead returning NULL we need to return something acceptable for driver 
if nearest is true (set_format must not fail but instead return what 
driver can do regarding request).
To fix this, I have put the 5Mp exceptions inside the for loop:

	for (i = OV5640_NUM_MODES - 1; i >= 0; i--) {
		mode = &ov5640_mode_data[i];

		if (!mode->reg_data)
			continue;

		if ((nearest && mode->hact <= width &&
		     mode->vact <= height) ||
		    (!nearest && mode->hact == width &&
		     mode->vact == height)) {

			/* 2592x1944 can only operate at 15fps */
			if (mode->hact == 2592 && mode->vact == 1944 &&
			    fr != OV5640_15_FPS)
				continue;/* next lower resolution */

			break;/* select this resolution */
		}
	}


>   
>   	return mode;
>   }
> @@ -2571,9 +2371,9 @@ static int ov5640_enum_frame_size(struct v4l2_subdev *sd,
>   		return -EINVAL;
>   
>   	fse->min_width = fse->max_width =
> -		ov5640_mode_data[0][fse->index].hact;
> +		ov5640_mode_data[fse->index].hact;
>   	fse->min_height = fse->max_height =
> -		ov5640_mode_data[0][fse->index].vact;
> +		ov5640_mode_data[fse->index].vact;
>   
>   	return 0;
>   }
> @@ -2777,7 +2577,7 @@ static int ov5640_probe(struct i2c_client *client,
>   	sensor->frame_interval.denominator = ov5640_framerates[OV5640_30_FPS];
>   	sensor->current_fr = OV5640_30_FPS;
>   	sensor->current_mode =
> -		&ov5640_mode_data[OV5640_30_FPS][OV5640_MODE_VGA_640_480];
> +		&ov5640_mode_data[OV5640_MODE_VGA_640_480];
>   	sensor->pending_mode_change = true;
>   
>   	sensor->ae_target = 52;
> 

With that change, mode is now independent from framerate, so we can now 
change the ov5640_s_frame_interval() code about finding the new mode 
which correspond to new framerate:

static int ov5640_s_frame_interval(struct v4l2_subdev *sd,
  				   struct v4l2_subdev_frame_interval *fi)
  {
[...]
   	mode = sensor->current_mode;

  	frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
  					       mode->hact, mode->vact);

-	mode = ov5640_find_mode(sensor, frame_rate, mode->hact,
-				mode->vact, true);
-	if (!mode) {
-		ret = -EINVAL;
- 		goto out;
- 	}
-	sensor->current_mode = mode;


Best regards,
Hugues.

[PATCH 11/12] media: ov5640: Add 60 fps support

[Maxime Ripard]

Now that we have everything in place to compute the clock rate at runtime,
we can enable the 60fps framerate for the mode we tested it with.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 5510a19281a4..03838f42fb82 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -111,6 +111,7 @@ enum ov5640_mode_id {
 enum ov5640_frame_rate {
 	OV5640_15_FPS = 0,
 	OV5640_30_FPS,
+	OV5640_60_FPS,
 	OV5640_NUM_FRAMERATES,
 };
 
@@ -144,6 +145,7 @@ MODULE_PARM_DESC(virtual_channel,
 static const int ov5640_framerates[] = {
 	[OV5640_15_FPS] = 15,
 	[OV5640_30_FPS] = 30,
+	[OV5640_60_FPS] = 60,
 };
 
 /* regulator supplies */
@@ -1447,6 +1449,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
 	    fr != OV5640_15_FPS)
 		return NULL;
 
+	/* Only 640x480 can operate at 60fps (for now) */
+	if (fr == OV5640_60_FPS &&
+	    width != 640 && height != 480)
+		return NULL;
+
 	return mode;
 }
 
@@ -1875,12 +1882,12 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor,
 	int ret;
 
 	minfps = ov5640_framerates[OV5640_15_FPS];
-	maxfps = ov5640_framerates[OV5640_30_FPS];
+	maxfps = ov5640_framerates[OV5640_60_FPS];
 
 	if (fi->numerator == 0) {
 		fi->denominator = maxfps;
 		fi->numerator = 1;
-		return OV5640_30_FPS;
+		return OV5640_60_FPS;
 	}
 
 	fps = DIV_ROUND_CLOSEST(fi->denominator, fi->numerator);
@@ -1892,10 +1899,13 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor,
 		fi->denominator = minfps;
 	else if (2 * fps >= 2 * minfps + (maxfps - minfps))
 		fi->denominator = maxfps;
-	else
-		fi->denominator = minfps;
 
-	ret = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
+	if (fi->denominator == minfps)
+		ret = OV5640_15_FPS;
+	else if (fi->denominator == maxfps)
+		ret = OV5640_60_FPS;
+	else
+		ret = OV5640_30_FPS;
 
 	mode = ov5640_find_mode(sensor, ret, width, height, false);
 	return mode ? ret : -EINVAL;
-- 
2.14.3

Re: [PATCH 11/12] media: ov5640: Add 60 fps support

[Hugues FRUCHET]

Hi Maxime,

See below rest of comments regarding framerate and compliance failure.

On 03/02/2018 03:34 PM, Maxime Ripard wrote:
> Now that we have everything in place to compute the clock rate at runtime,
> we can enable the 60fps framerate for the mode we tested it with.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>   drivers/media/i2c/ov5640.c | 20 +++++++++++++++-----
>   1 file changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
> index 5510a19281a4..03838f42fb82 100644
> --- a/drivers/media/i2c/ov5640.c
> +++ b/drivers/media/i2c/ov5640.c
> @@ -111,6 +111,7 @@ enum ov5640_mode_id {
>   enum ov5640_frame_rate {
>   	OV5640_15_FPS = 0,
>   	OV5640_30_FPS,
> +	OV5640_60_FPS,
>   	OV5640_NUM_FRAMERATES,
>   };
>   
> @@ -144,6 +145,7 @@ MODULE_PARM_DESC(virtual_channel,
>   static const int ov5640_framerates[] = {
>   	[OV5640_15_FPS] = 15,
>   	[OV5640_30_FPS] = 30,
> +	[OV5640_60_FPS] = 60,
>   };
>   
>   /* regulator supplies */
> @@ -1447,6 +1449,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
>   	    fr != OV5640_15_FPS)
>   		return NULL;
>   
> +	/* Only 640x480 can operate at 60fps (for now) */
> +	if (fr == OV5640_60_FPS &&
> +	    width != 640 && height != 480)
> +		return NULL;
> +

Same comment as on patchset 10/12, VGA exception put in for loop:

	for (i = OV5640_NUM_MODES - 1; i >= 0; i--) {
		mode = &ov5640_mode_data[i];

		if (!mode->reg_data)
			continue;

		if ((nearest && mode->hact <= width &&
		     mode->vact <= height) ||
		    (!nearest && mode->hact == width &&
		     mode->vact == height)) {

			/* 2592x1944 can only operate at 15fps */
			if (mode->hact == 2592 && mode->vact == 1944 &&
			    fr != OV5640_15_FPS)
				continue;/* next lower resolution */

			/* Only 640x480 can operate at 60fps (for now) */
			if (fr == OV5640_60_FPS &&
			    !(mode->hact == 640 && mode->vact == 480))
				continue;/* next lower resolution */

			break;/* select this resolution */
		}
	}


>   	return mode;
>   }
>   
> @@ -1875,12 +1882,12 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor,
>   	int ret;
>   
>   	minfps = ov5640_framerates[OV5640_15_FPS];
> -	maxfps = ov5640_framerates[OV5640_30_FPS];
> +	maxfps = ov5640_framerates[OV5640_60_FPS];
>   
>   	if (fi->numerator == 0) {
>   		fi->denominator = maxfps;
>   		fi->numerator = 1;
> -		return OV5640_30_FPS;
> +		return OV5640_60_FPS;

There is a problem here because we don't validate that 60fps is 
supported in the currently set mode, we must inject this framerate value 
in ov5640_find_mode(framerate, width, height) in order to validate it 
(-EINVAL if not supported):

  +		ret = OV5640_60_FPS;
  +		goto find_mode;
  +	}
[...]
+find_mode:
	mode = ov5640_find_mode(sensor, frame_rate, width, height, false);
	return mode ? ret : -EINVAL;
  }

Then we have to catch error in ov5640_s_frame_interval() and return an 
acceptable frame interval to user:

  	frame_rate = ov5640_try_frame_interval(sensor, &fi->interval,
  					       mode->hact, mode->vact);
-	if (frame_rate < 0)
-		frame_rate = OV5640_15_FPS;
-
-	sensor->current_fr = frame_rate;
-	sensor->frame_interval = fi->interval;
We also have to update current framerate only if framerate has been 
validated.

+	if (frame_rate < 0) {
+		/* return a valid frame interval value */
+		fi->interval = sensor->frame_interval;
  		goto out;
  	}

+	sensor->current_fr = frame_rate;
+	sensor->frame_interval = fi->interval;
  	sensor->pending_mode_change = true;
  out:


About 60 fps by default if (fi->numerator == 0): shouldn't we stick to a 
default value supported by all modes such as 30fps ?

>   	}
>   
>   	fps = DIV_ROUND_CLOSEST(fi->denominator, fi->numerator);
> @@ -1892,10 +1899,13 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor,
>   		fi->denominator = minfps;
>   	else if (2 * fps >= 2 * minfps + (maxfps - minfps))
>   		fi->denominator = maxfps;
> -	else
> -		fi->denominator = minfps;
>   
> -	ret = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS;
> +	if (fi->denominator == minfps)
> +		ret = OV5640_15_FPS;
> +	else if (fi->denominator == maxfps)
> +		ret = OV5640_60_FPS;
> +	else
> +		ret = OV5640_30_FPS;
>   
>   	mode = ov5640_find_mode(sensor, ret, width, height, false);
>   	return mode ? ret : -EINVAL;
> 

With this 60fps change, we have also to change the default mode to VGA, 
because it's the only one resolution that supports all the 3 framerates 
15/30/60:

static const struct ov5640_mode_info *
  ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr,
  		 int width, int height, bool nearest)
  {

  	if (i < 0) {
  		/* no match */
  		if (!nearest)
  			return NULL;
-		mode = &ov5640_mode_data[0];
+		mode = &ov5640_mode_data[OV5640_MODE_VGA_640_480];/* VGA can do all 
fps */
  	}


Best regards,
Hugues.

Re: [PATCH 11/12] media: ov5640: Add 60 fps support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 693 bytes --]

Hi Hugues,

Thanks for all your feedback, I'll merge your suggested changes in the
next iteration.

On Tue, Mar 13, 2018 at 02:32:14PM +0000, Hugues FRUCHET wrote:
> >   	if (fi->numerator == 0) {
> >   		fi->denominator = maxfps;
> >   		fi->numerator = 1;
> > -		return OV5640_30_FPS;
> > +		return OV5640_60_FPS;
>
> [...]
>
> About 60 fps by default if (fi->numerator == 0): shouldn't we stick to a 
> default value supported by all modes such as 30fps ?

30 fps is not supported by all modes either, so I guess 15 fps would
be a better pick?

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 12/12] media: ov5640: Remove duplicate auto-exposure setup

[Maxime Ripard]

The autoexposure setup in the 1080p init array is redundant with the
default value of the sensor.

Remove it.

Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 drivers/media/i2c/ov5640.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c
index 03838f42fb82..7c68c7013324 100644
--- a/drivers/media/i2c/ov5640.c
+++ b/drivers/media/i2c/ov5640.c
@@ -508,7 +508,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = {
 	{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},
-	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0},
+	{0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0},
 };
 
 static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = {
-- 
2.14.3

Re: [PATCH 00/12] media: ov5640: Misc cleanup and improvements

[Hugues FRUCHET]

Thanks Maxime for this great series !

I've tested this series successfully on STM32 platform, I had a 
regression on JPEG capture linked to revisit of clocking, but easy to fix.
I had another problem claimed by v4l2-compliance on format tests:
v4l2-test-formats.cpp(961): Video Capture: S_FMT(G_FMT) != G_FMT
this was more tricky to fix, it is linked to changes around framerate 
handling.

See my further comments in corresponding patchsets.

Best regards,
Hugues.

On 03/02/2018 03:34 PM, Maxime Ripard wrote:
> Hi,
> 
> Here is a "small" series that mostly cleans up the ov5640 driver code,
> slowly getting rid of the big data array for more understandable code
> (hopefully).
> 
> The biggest addition would be the clock rate computation at runtime,
> instead of relying on those arrays to setup the clock tree
> properly. As a side effect, it fixes the framerate that was off by
> around 10% on the smaller resolutions, and we now support 60fps.
> 
> This also introduces a bunch of new features.
> 
> Let me know what you think,
> Maxime
> 
> Maxime Ripard (10):
>    media: ov5640: Don't force the auto exposure state at start time
>    media: ov5640: Init properly the SCLK dividers
>    media: ov5640: Change horizontal and vertical resolutions name
>    media: ov5640: Add horizontal and vertical totals
>    media: ov5640: Program the visible resolution
>    media: ov5640: Adjust the clock based on the expected rate
>    media: ov5640: Compute the clock rate at runtime
>    media: ov5640: Enhance FPS handling
>    media: ov5640: Add 60 fps support
>    media: ov5640: Remove duplicate auto-exposure setup
> 
> Mylène Josserand (2):
>    media: ov5640: Add auto-focus feature
>    media: ov5640: Add light frequency control
> 
>   drivers/media/i2c/ov5640.c | 777 ++++++++++++++++++++++++++-------------------
>   1 file changed, 452 insertions(+), 325 deletions(-)
>