linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] media: i2c: ov5645: Fix power sequence
@ 2019-07-10 18:40 Ezequiel Garcia
  2019-07-30  8:16 ` Ezequiel Garcia
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ezequiel Garcia @ 2019-07-10 18:40 UTC (permalink / raw)
  To: Hans Verkuil, Sakari Ailus
  Cc: kernel, Fabio Estevam, linux-media, Philipp Zabel, Ezequiel Garcia

This is mostly a port of Jacopo's fix:

  commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
  Author: Jacopo Mondi <jacopo@jmondi.org>
  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.

Tested-by: Fabio Estevam <festevam@gmail.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
Changes in v2:
* As suggested by Philipp, move the initial configuration
  to the ov5645_global_init_setting array.
---
 drivers/media/i2c/ov5645.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
index 124c8df04633..58972c884705 100644
--- a/drivers/media/i2c/ov5645.c
+++ b/drivers/media/i2c/ov5645.c
@@ -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 },
@@ -350,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[] = {
@@ -737,13 +742,9 @@ 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);
-			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 +1050,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)
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] media: i2c: ov5645: Fix power sequence
  2019-07-10 18:40 [PATCH v2] media: i2c: ov5645: Fix power sequence Ezequiel Garcia
@ 2019-07-30  8:16 ` Ezequiel Garcia
  2019-07-31  8:25 ` Philipp Zabel
  2019-07-31  8:48 ` Jacopo Mondi
  2 siblings, 0 replies; 4+ messages in thread
From: Ezequiel Garcia @ 2019-07-30  8:16 UTC (permalink / raw)
  To: Hans Verkuil, Sakari Ailus
  Cc: kernel, Fabio Estevam, linux-media, Philipp Zabel

Hi Hans, Sakari,

On Wed, 2019-07-10 at 15:40 -0300, Ezequiel Garcia wrote:
> This is mostly a port of Jacopo's fix:
> 
>   commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
>   Author: Jacopo Mondi <jacopo@jmondi.org>
>   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.
> 
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes in v2:
> * As suggested by Philipp, move the initial configuration
>   to the ov5645_global_init_setting array.
> ---
>  drivers/media/i2c/ov5645.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index 124c8df04633..58972c884705 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -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 },
> @@ -350,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[] = {
> @@ -737,13 +742,9 @@ 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);
> -			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 +1050,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)

Gentle reminder about this one.

Thanks!
Eze



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] media: i2c: ov5645: Fix power sequence
  2019-07-10 18:40 [PATCH v2] media: i2c: ov5645: Fix power sequence Ezequiel Garcia
  2019-07-30  8:16 ` Ezequiel Garcia
@ 2019-07-31  8:25 ` Philipp Zabel
  2019-07-31  8:48 ` Jacopo Mondi
  2 siblings, 0 replies; 4+ messages in thread
From: Philipp Zabel @ 2019-07-31  8:25 UTC (permalink / raw)
  To: Ezequiel Garcia, Hans Verkuil, Sakari Ailus
  Cc: kernel, Fabio Estevam, linux-media

On Wed, 2019-07-10 at 15:40 -0300, Ezequiel Garcia wrote:
> This is mostly a port of Jacopo's fix:
> 
>   commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
>   Author: Jacopo Mondi <jacopo@jmondi.org>
>   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.
> 
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes in v2:
> * As suggested by Philipp, move the initial configuration
>   to the ov5645_global_init_setting array.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>

regards
Philipp

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] media: i2c: ov5645: Fix power sequence
  2019-07-10 18:40 [PATCH v2] media: i2c: ov5645: Fix power sequence Ezequiel Garcia
  2019-07-30  8:16 ` Ezequiel Garcia
  2019-07-31  8:25 ` Philipp Zabel
@ 2019-07-31  8:48 ` Jacopo Mondi
  2 siblings, 0 replies; 4+ messages in thread
From: Jacopo Mondi @ 2019-07-31  8:48 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: Hans Verkuil, Sakari Ailus, kernel, Fabio Estevam, linux-media,
	Philipp Zabel

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

Hi Ezequiel,

On Wed, Jul 10, 2019 at 03:40:00PM -0300, Ezequiel Garcia wrote:
> This is mostly a port of Jacopo's fix:
>
>   commit aa4bb8b8838ffcc776a79f49a4d7476b82405349
>   Author: Jacopo Mondi <jacopo@jmondi.org>
>   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.

You should change this, as I still see this part of the initial
settings blob.

Apart from that
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

> - 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.
>
> Tested-by: Fabio Estevam <festevam@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> Changes in v2:
> * As suggested by Philipp, move the initial configuration
>   to the ov5645_global_init_setting array.
> ---
>  drivers/media/i2c/ov5645.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/i2c/ov5645.c b/drivers/media/i2c/ov5645.c
> index 124c8df04633..58972c884705 100644
> --- a/drivers/media/i2c/ov5645.c
> +++ b/drivers/media/i2c/ov5645.c
> @@ -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 },
> @@ -350,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[] = {
> @@ -737,13 +742,9 @@ 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);
> -			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 +1050,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)
> --
> 2.20.1
>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-07-31  8:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-10 18:40 [PATCH v2] media: i2c: ov5645: Fix power sequence Ezequiel Garcia
2019-07-30  8:16 ` Ezequiel Garcia
2019-07-31  8:25 ` Philipp Zabel
2019-07-31  8:48 ` Jacopo Mondi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).