media: i2c: ov5645: Fix power up sequence

PATCH Superseded Ezequiel Garcia 3 participants 4 messages Jul 3, 2019 – Jul 4, 2019

Subsystems (2)

Media (V4L/DVB) V4L2 camera sensor drivers

lore

Share & tools

lore

Ezequiel Garcia

author

2019-07-03 13:10 UTC #0 lore raw

PATCH 1/1 media: i2c: ov5645: Fix power up sequence

This is mostly a port of Jacopo's fix:

commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
Author: Jacopo Mondi <[email protected]>
Date: Fri Jul 6 05:51:52 2018 -0400

media: ov5640: Re-work MIPI startup sequence

In the OV5645 case, the changes are:

- Move OV5645_IO_MIPI_CTRL00 (0x300e) out of the initial setting blob.
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At set_power(0) time power down MIPI Tx/Rx (in addition to the current power down of regulators and clock gating).
- At s_stream time enable/disable the MIPI interface output.

With this commit the sensor is able to enter LP-11 mode during power up, as expected by some CSI-2 controllers.

Many thanks to Fabio Estevam for his help debugging this issue.

Signed-off-by Ezequiel Garcia <[email protected]>

---
 drivers/media/i2c/ov5645.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

drivers/media/i2c/ov5645.c +31 -3

index 124c8df04633..05430a81c977 100644
@@ -45,6 +45,8 @@
 #define		OV5645_CHIP_ID_HIGH_BYTE	0x56
 #define OV5645_CHIP_ID_LOW		0x300b
 #define		OV5645_CHIP_ID_LOW_BYTE		0x45
+#define OV5645_IO_MIPI_CTRL00		0x300e
+#define OV5645_PAD_OUTPUT00		0x3019
 #define OV5645_AWB_MANUAL_CONTROL	0x3406
 #define		OV5645_AWB_MANUAL_ENABLE	BIT(0)
 #define OV5645_AEC_PK_MANUAL		0x3503

@@ -55,6 +57,7 @@
 #define		OV5645_ISP_VFLIP		BIT(2)
 #define OV5645_TIMING_TC_REG21		0x3821
 #define		OV5645_SENSOR_MIRROR		BIT(1)
+#define OV5645_MIPI_CTRL00		0x4800
 #define OV5645_PRE_ISP_TEST_SETTING_1	0x503d
 #define		OV5645_TEST_PATTERN_MASK	0x3
 #define		OV5645_SET_TEST_PATTERN(x)	((x) & OV5645_TEST_PATTERN_MASK)

@@ -121,7 +124,6 @@ static const struct reg_value ov5645_global_init_setting[] = {
 	{ 0x3503, 0x07 },
 	{ 0x3002, 0x1c },
 	{ 0x3006, 0xc3 },
-	{ 0x300e, 0x45 },
 	{ 0x3017, 0x00 },
 	{ 0x3018, 0x00 },
 	{ 0x302e, 0x0b },

@@ -737,13 +739,30 @@ static int ov5645_s_power(struct v4l2_subdev *sd, int on)
 				goto exit;
 			}
 
-			ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
-					       OV5645_SYSTEM_CTRL0_STOP);
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_IO_MIPI_CTRL00, 0x40);
 			if (ret < 0) {
 				ov5645_set_power_off(ov5645);
 				goto exit;
 			}
+
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_MIPI_CTRL00, 0x24);
+			if (ret < 0) {
+				ov5645_set_power_off(ov5645);
+				goto exit;
+			}
+
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_PAD_OUTPUT00, 0x70);
+			if (ret < 0) {
+				ov5645_set_power_off(ov5645);
+				goto exit;
+			}
+
+			usleep_range(500, 1000);
 		} else {
+			ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x58);
 			ov5645_set_power_off(ov5645);
 		}
 	}

@@ -1049,11 +1068,20 @@ static int ov5645_s_stream(struct v4l2_subdev *subdev, int enable)
 			dev_err(ov5645->dev, "could not sync v4l2 controls\n");
 			return ret;
 		}
+
+		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x45);
+		if (ret < 0)
+			return ret;
+
 		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
 				       OV5645_SYSTEM_CTRL0_START);
 		if (ret < 0)
 			return ret;
 	} else {
+		ret = ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x40);
+		if (ret < 0)
+			return ret;
+
 		ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
 				       OV5645_SYSTEM_CTRL0_STOP);
 		if (ret < 0)

Signature · 1 line

2.20.1

Fabio Estevam

Tested-by on v1 1/1

2019-07-04 01:26 UTC #1 lore raw

Hi Ezequiel,

Quoted 18 lines · Ezequiel Garcia

This is mostly a port of Jacopo's fix:
commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
Author: Jacopo Mondi <[email protected]>
Date: Fri Jul 6 05:51:52 2018 -0400
media: ov5640: Re-work MIPI startup sequence
In the OV5645 case, the changes are:
- Move OV5645_IO_MIPI_CTRL00 (0x300e) out of the initial setting blob.
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At set_power(0) time power down MIPI Tx/Rx (in addition to the current power down of regulators and clock gating).
- At s_stream time enable/disable the MIPI interface output.

Quoted 6 lines

With this commit the sensor is able to enter LP-11 mode during power up, as expected by some CSI-2 controllers.
Many thanks to Fabio Estevam for his help debugging this issue.
Signed-off-by Ezequiel Garcia <[email protected]>

With this patch the applied the OV5645 camera successfully enters in LP-11 and the imx6 capture driver does not complain anymore.

Tested-by Fabio Estevam <[email protected]>

Thanks!

Philipp Zabel

2019-07-04 09:34 UTC #2 lore raw

In reply to Ezequiel Garcia

Hi Ezequiel,

Quoted 72 lines · Ezequiel Garcia

This is mostly a port of Jacopo's fix:
commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
Author: Jacopo Mondi <[email protected]>
Date: Fri Jul 6 05:51:52 2018 -0400
media: ov5640: Re-work MIPI startup sequence
In the OV5645 case, the changes are:
- Move OV5645_IO_MIPI_CTRL00 (0x300e) out of the initial setting blob.
- At set_power(1) time power up MIPI Tx/Rx and set data and clock lanes in LP11 during 'sleep' and 'idle' with MIPI clock in non-continuous mode.
- At set_power(0) time power down MIPI Tx/Rx (in addition to the current power down of regulators and clock gating).
- At s_stream time enable/disable the MIPI interface output.
With this commit the sensor is able to enter LP-11 mode during power up, as expected by some CSI-2 controllers.
Many thanks to Fabio Estevam for his help debugging this issue.
Signed-off-by Ezequiel Garcia <[email protected]>
---
 drivers/media/i2c/ov5645.c | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)
drivers/media/i2c/ov5645.c +9 -3
index 124c8df04633..05430a81c977 100644
@@ -45,6 +45,8 @@
 #define		OV5645_CHIP_ID_HIGH_BYTE	0x56
 #define OV5645_CHIP_ID_LOW		0x300b
 #define		OV5645_CHIP_ID_LOW_BYTE		0x45
+#define OV5645_IO_MIPI_CTRL00		0x300e
+#define OV5645_PAD_OUTPUT00		0x3019
 #define OV5645_AWB_MANUAL_CONTROL	0x3406
 #define		OV5645_AWB_MANUAL_ENABLE	BIT(0)
 #define OV5645_AEC_PK_MANUAL		0x3503
@@ -55,6 +57,7 @@
 #define		OV5645_ISP_VFLIP		BIT(2)
 #define OV5645_TIMING_TC_REG21		0x3821
 #define		OV5645_SENSOR_MIRROR		BIT(1)
+#define OV5645_MIPI_CTRL00		0x4800
 #define OV5645_PRE_ISP_TEST_SETTING_1	0x503d
 #define		OV5645_TEST_PATTERN_MASK	0x3
 #define		OV5645_SET_TEST_PATTERN(x)	((x) & OV5645_TEST_PATTERN_MASK)
@@ -121,7 +124,6 @@ static const struct reg_value ov5645_global_init_setting[] = {
 	{ 0x3503, 0x07 },
 	{ 0x3002, 0x1c },
 	{ 0x3006, 0xc3 },
-	{ 0x300e, 0x45 },
 	{ 0x3017, 0x00 },
 	{ 0x3018, 0x00 },
 	{ 0x302e, 0x0b },
@@ -737,13 +739,30 @@ static int ov5645_s_power(struct v4l2_subdev *sd, int on)
 				goto exit;
 			}
 
-			ret = ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
-					       OV5645_SYSTEM_CTRL0_STOP);
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_IO_MIPI_CTRL00, 0x40);
 			if (ret < 0) {
 				ov5645_set_power_off(ov5645);
 				goto exit;
 			}
+
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_MIPI_CTRL00, 0x24);
+			if (ret < 0) {

drivers/media/i2c/ov5645.c static int ov5645_s_power(struct v4l2_subdev *sd, int on) · line 761

Quoted 10 lines

drivers/media/i2c/ov5645.c +10 -0

+				ov5645_set_power_off(ov5645);
+				goto exit;
+			}
+
+			ret = ov5645_write_reg(ov5645,
+					       OV5645_PAD_OUTPUT00, 0x70);
+			if (ret < 0) {
+				ov5645_set_power_off(ov5645);
+				goto exit;
+			}

Could this just be added to the end of ov5645_global_init_setting?

----------8<----------

drivers/media/i2c/ov5645.c +0 -0

index 05430a81c977..d978f7aa44c3 100644
@@ -352,7 +352,10 @@ static const struct reg_value ov5645_global_init_setting[] = {

{ 0x3a1f, 0x14 },
{ 0x0601, 0x02 },
{ 0x3008, 0x42 },
- { 0x3008, 0x02 }
+ { 0x3008, 0x02 },
+ { OV5645_IO_MIPI_CTRL00, 0x40 },
+ { OV5645_MIPI_CTRL00,    0x24 },
+ { OV5645_PAD_OUTPUT00,   0x70 }
};

static const struct reg_value ov5645_setting_sxga[] = {
@@ -739,27 +742,6 @@ static int ov5645_s_power(struct v4l2_subdev *sd, int on) goto exit;
}

- ret = ov5645_write_reg(ov5645,
-        OV5645_IO_MIPI_CTRL00, 0x40);
- if (ret < 0) {
- ov5645_set_power_off(ov5645);
- goto exit;
- }
-
- ret = ov5645_write_reg(ov5645,
-        OV5645_MIPI_CTRL00, 0x24);
- if (ret < 0) {
- ov5645_set_power_off(ov5645);
- goto exit;
- }
-
- ret = ov5645_write_reg(ov5645,
-        OV5645_PAD_OUTPUT00, 0x70);
- if (ret < 0) {
- ov5645_set_power_off(ov5645);
- goto exit;
- }
-
usleep_range(500, 1000);
} else {
ov5645_write_reg(ov5645, OV5645_IO_MIPI_CTRL00, 0x58); ---------->8----------

regards
Philipp

Fabio Estevam

2019-07-04 11:31 UTC #3 lore raw

Hi Philipp,

Quoted 1 line · Philipp Zabel

Could this just be added to the end of ov5645_global_init_setting?

Just tested your suggestion and it also works.

Thanks