[RFC][PATCH 0/3] OMAP: DSS2: Allow us to use SDI

[RFC][PATCH 0/3] OMAP: DSS2: Allow us to use SDI

[Roger Quadros]

I want to get DSS2 working on N900. N900 requires SDI.

Technically we should be able to use SDI without having DSI
regulators enabled.
Thes patches also add the vdds_sdi supply support to SDI.

NOTE: I cannot yet confirm that SDI was working or will work with this.

---
Roger Quadros (3):
  OMAP: DSS2: Do regulator stuff in dpi_init_display()
  OMAP: DSS2: Prevent recursive Panel Enable/Disable calls from SDI
  OMAP: DSS2: Use vdds_sdi regulator supply in SDI

 drivers/video/omap2/dss/dpi.c |   10 +++++-----
 drivers/video/omap2/dss/sdi.c |   32 +++++++++++++++++++++-----------
 2 files changed, 26 insertions(+), 16 deletions(-)

[RFC][PATCH 1/3] OMAP: DSS2: Do regulator stuff in dpi_init_display()

[Roger Quadros]

From: Roger Quadros <roger.quadros@nokia.com>

We don't want dpi_init() to fail in any case as it will prevent us
from using DSS2 if we don't have DSI in the system. This is because
dpi_init() is done unconditionally by the DSS2 core.

We will only need to do regulator initialization when DPI is actually
present in the system. So we do regulator init stuff in dpi_init_display()

Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
---
 drivers/video/omap2/dss/dpi.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 960e977..bcedafd 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -299,11 +299,6 @@ int dpi_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
 
-	return 0;
-}
-
-int dpi_init(struct platform_device *pdev)
-{
 	if (cpu_is_omap34xx()) {
 		dpi.vdds_dsi_reg = dss_get_vdds_dsi();
 		if (IS_ERR(dpi.vdds_dsi_reg)) {
@@ -315,6 +310,11 @@ int dpi_init(struct platform_device *pdev)
 	return 0;
 }
 
+int dpi_init(struct platform_device *pdev)
+{
+	return 0;
+}
+
 void dpi_exit(void)
 {
 }
-- 
1.6.0.4

[RFC][PATCH 2/3] OMAP: DSS2: Prevent recursive Panel Enable/Disable calls from SDI

[Roger Quadros]

From: Roger Quadros <roger.quadros@nokia.com>

Panel enable/disable is now done inside display manager,
so don't need to do it again.

Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
---
 drivers/video/omap2/dss/sdi.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 12eb404..45aab89 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -115,12 +115,6 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 
 	dssdev->manager->enable(dssdev->manager);
 
-	if (dssdev->driver->enable) {
-		r = dssdev->driver->enable(dssdev);
-		if (r)
-			goto err3;
-	}
-
 	sdi.skip_init = 0;
 
 	return 0;
@@ -137,9 +131,6 @@ EXPORT_SYMBOL(omapdss_sdi_display_enable);
 
 void omapdss_sdi_display_disable(struct omap_dss_device *dssdev)
 {
-	if (dssdev->driver->disable)
-		dssdev->driver->disable(dssdev);
-
 	dssdev->manager->disable(dssdev->manager);
 
 	dss_sdi_disable();
-- 
1.6.0.4

[RFC][PATCH 3/3] OMAP: DSS2: Use vdds_sdi regulator supply in SDI

[Roger Quadros]

From: Roger Quadros <roger.quadros@nokia.com>

This patch enables the use of vdds_sdi regulator in SDI subsystem.
We can disable the vdds_sdi voltage when not in use to save
power.

Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
---
 drivers/video/omap2/dss/sdi.c |   23 +++++++++++++++++++++--
 1 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
index 45aab89..e816e80 100644
--- a/drivers/video/omap2/dss/sdi.c
+++ b/drivers/video/omap2/dss/sdi.c
@@ -23,13 +23,16 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/err.h>
+#include <linux/regulator/consumer.h>
 
 #include <plat/display.h>
+#include <plat/cpu.h>
 #include "dss.h"
 
 static struct {
 	bool skip_init;
 	bool update_enabled;
+	struct regulator *vdds_sdi_reg;
 } sdi;
 
 static void sdi_basic_init(void)
@@ -57,6 +60,12 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 		goto err0;
 	}
 
+	if (cpu_is_omap34xx()) {
+		r = regulator_enable(sdi.vdds_sdi_reg);
+		if (r)
+			goto err1;
+	}
+
 	/* In case of skip_init sdi_init has already enabled the clocks */
 	if (!sdi.skip_init)
 		dss_clk_enable(DSS_CLK_ICK | DSS_CLK_FCK1);
@@ -118,10 +127,10 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
 	sdi.skip_init = 0;
 
 	return 0;
-err3:
-	dssdev->manager->disable(dssdev->manager);
 err2:
 	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
+	if (cpu_is_omap34xx())
+		regulator_enable(sdi.vdds_sdi_reg);
 err1:
 	omap_dss_stop_device(dssdev);
 err0:
@@ -137,6 +146,9 @@ void omapdss_sdi_display_disable(struct omap_dss_device *dssdev)
 
 	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
 
+	if (cpu_is_omap34xx())
+		regulator_enable(sdi.vdds_sdi_reg);
+
 	omap_dss_stop_device(dssdev);
 }
 EXPORT_SYMBOL(omapdss_sdi_display_disable);
@@ -144,6 +156,13 @@ EXPORT_SYMBOL(omapdss_sdi_display_disable);
 int sdi_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("SDI init\n");
+	if (cpu_is_omap34xx()) {
+		sdi.vdds_sdi_reg = dss_get_vdds_sdi();
+		if (IS_ERR(sdi.vdds_sdi_reg)) {
+			DSSERR("can't get VDDS_SDI regulator\n");
+			return PTR_ERR(sdi.vdds_sdi_reg);
+		}
+	}
 
 	return 0;
 }
-- 
1.6.0.4

[RFC][PATCH 1/3] OMAP: DSS2: Do regulator stuff in dpi_init_display()

[Roger Quadros]

From: Roger Quadros <roger.quadros@nokia.com>

We don't want dpi_init() to fail in any case as it will prevent us
from using DSS2 if we don't have DSI in the system. This is because
dpi_init() is done unconditionally by the DSS2 core.

We will only need to do regulator initialization when DPI is actually
present in the system. So we do regulator init stuff in dpi_init_display()

Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
---
 drivers/video/omap2/dss/dpi.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
index 960e977..bcedafd 100644
--- a/drivers/video/omap2/dss/dpi.c
+++ b/drivers/video/omap2/dss/dpi.c
@@ -299,11 +299,6 @@ int dpi_init_display(struct omap_dss_device *dssdev)
 {
 	DSSDBG("init_display\n");
 
-	return 0;
-}
-
-int dpi_init(struct platform_device *pdev)
-{
 	if (cpu_is_omap34xx()) {
 		dpi.vdds_dsi_reg = dss_get_vdds_dsi();
 		if (IS_ERR(dpi.vdds_dsi_reg)) {
@@ -315,6 +310,11 @@ int dpi_init(struct platform_device *pdev)
 	return 0;
 }
 
+int dpi_init(struct platform_device *pdev)
+{
+	return 0;
+}
+
 void dpi_exit(void)
 {
 }
-- 
1.6.0.4

Re: [RFC][PATCH 1/3] OMAP: DSS2: Do regulator stuff in

[Tomi Valkeinen]

On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
wrote:
> From: Roger Quadros <roger.quadros@nokia.com>
> 
> We don't want dpi_init() to fail in any case as it will prevent us
> from using DSS2 if we don't have DSI in the system. This is because
> dpi_init() is done unconditionally by the DSS2 core.
> 
> We will only need to do regulator initialization when DPI is actually
> present in the system. So we do regulator init stuff in dpi_init_display()
> 
> Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
> ---
>  drivers/video/omap2/dss/dpi.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
> index 960e977..bcedafd 100644
> --- a/drivers/video/omap2/dss/dpi.c
> +++ b/drivers/video/omap2/dss/dpi.c
> @@ -299,11 +299,6 @@ int dpi_init_display(struct omap_dss_device *dssdev)
>  {
>  	DSSDBG("init_display\n");
>  
> -	return 0;
> -}
> -
> -int dpi_init(struct platform_device *pdev)
> -{
>  	if (cpu_is_omap34xx()) {
>  		dpi.vdds_dsi_reg = dss_get_vdds_dsi();
>  		if (IS_ERR(dpi.vdds_dsi_reg)) {
> @@ -315,6 +310,11 @@ int dpi_init(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +int dpi_init(struct platform_device *pdev)
> +{
> +	return 0;
> +}
> +

I don't think this is quite correct. dpi_init_display() will be called
for all DPI displays, which means that it may be called more than once.
dpi_init() is meant for global initializations.

Perhaps it would be best to have DPI compiled conditionally like
SDI/DSI/RFBI.

 Tomi

Ps. Fixed the fbdev list address in the cc

Re: [RFC][PATCH 2/3] OMAP: DSS2: Prevent recursive Panel

[Tomi Valkeinen]

On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
wrote:
> From: Roger Quadros <roger.quadros@nokia.com>
> 
> Panel enable/disable is now done inside display manager,
> so don't need to do it again.
> 

This looks correct, thanks.

 Tomi

Re: [RFC][PATCH 2/3] OMAP: DSS2: Prevent recursive Panel

[Tomi Valkeinen]

On Mon, 2010-03-15 at 10:32 +0100, Tomi Valkeinen wrote:
> On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
> wrote:
> > From: Roger Quadros <roger.quadros@nokia.com>
> > 
> > Panel enable/disable is now done inside display manager,
> > so don't need to do it again.
> > 
> 
> This looks correct, thanks.

Actually, I take my word back =).

This patch leaves the err3 label there, and you remove it in the next
patch. It should be removed in this same patch.

And the patch description is a bit strange. I think it should be
something like:

"Panel enable/disable is now done via the panel driver, so we should not
call the panel driver again."

 Tomi

Re: [RFC][PATCH 3/3] OMAP: DSS2: Use vdds_sdi regulator supply in

[Tomi Valkeinen]

On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
wrote:
> From: Roger Quadros <roger.quadros@nokia.com>
> 
> This patch enables the use of vdds_sdi regulator in SDI subsystem.
> We can disable the vdds_sdi voltage when not in use to save
> power.
> 
> Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
> ---
>  drivers/video/omap2/dss/sdi.c |   23 +++++++++++++++++++++--
>  1 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
> index 45aab89..e816e80 100644
> --- a/drivers/video/omap2/dss/sdi.c
> +++ b/drivers/video/omap2/dss/sdi.c
> @@ -23,13 +23,16 @@
>  #include <linux/clk.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <plat/display.h>
> +#include <plat/cpu.h>
>  #include "dss.h"
>  
>  static struct {
>  	bool skip_init;
>  	bool update_enabled;
> +	struct regulator *vdds_sdi_reg;
>  } sdi;
>  
>  static void sdi_basic_init(void)
> @@ -57,6 +60,12 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
>  		goto err0;
>  	}
>  
> +	if (cpu_is_omap34xx()) {
> +		r = regulator_enable(sdi.vdds_sdi_reg);
> +		if (r)
> +			goto err1;
> +	}
> +
>  	/* In case of skip_init sdi_init has already enabled the clocks */
>  	if (!sdi.skip_init)
>  		dss_clk_enable(DSS_CLK_ICK | DSS_CLK_FCK1);
> @@ -118,10 +127,10 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
>  	sdi.skip_init = 0;
>  
>  	return 0;
> -err3:
> -	dssdev->manager->disable(dssdev->manager);
>  err2:
>  	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
> +	if (cpu_is_omap34xx())
> +		regulator_enable(sdi.vdds_sdi_reg);
>  err1:
>  	omap_dss_stop_device(dssdev);
>  err0:
> @@ -137,6 +146,9 @@ void omapdss_sdi_display_disable(struct omap_dss_device *dssdev)
>  
>  	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
>  
> +	if (cpu_is_omap34xx())
> +		regulator_enable(sdi.vdds_sdi_reg);
> +
>  	omap_dss_stop_device(dssdev);
>  }
>  EXPORT_SYMBOL(omapdss_sdi_display_disable);
> @@ -144,6 +156,13 @@ EXPORT_SYMBOL(omapdss_sdi_display_disable);
>  int sdi_init_display(struct omap_dss_device *dssdev)
>  {
>  	DSSDBG("SDI init\n");
> +	if (cpu_is_omap34xx()) {
> +		sdi.vdds_sdi_reg = dss_get_vdds_sdi();
> +		if (IS_ERR(sdi.vdds_sdi_reg)) {
> +			DSSERR("can't get VDDS_SDI regulator\n");
> +			return PTR_ERR(sdi.vdds_sdi_reg);
> +		}
> +	}
>  
>  	return 0;
>  }

Same comment here as to DPI patch, I think the regulator init should be
done in sdi_init().

And why do you test for omap34xx?

 Tomi

Re: [RFC][PATCH 1/3] OMAP: DSS2: Do regulator stuff in dpi_init_display()

[Roger Quadros]

Hi,

Valkeinen Tomi (Nokia-D/Helsinki) wrote:

> I don't think this is quite correct. dpi_init_display() will be called
> for all DPI displays, which means that it may be called more than once.
> dpi_init() is meant for global initializations.
> 
> Perhaps it would be best to have DPI compiled conditionally like
> SDI/DSI/RFBI.
> 
OK, will do it this way. Compiled conditionally makes sense.

-roger

Re: [RFC][PATCH 3/3] OMAP: DSS2: Use vdds_sdi regulator supply in

[Roger Quadros]

Hi,

Valkeinen Tomi (Nokia-D/Helsinki) wrote:
> On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
> wrote:
>> From: Roger Quadros <roger.quadros@nokia.com>
>>
>> This patch enables the use of vdds_sdi regulator in SDI subsystem.
>> We can disable the vdds_sdi voltage when not in use to save
>> power.
>>
>> Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
>> ---
>>  drivers/video/omap2/dss/sdi.c |   23 +++++++++++++++++++++--
>>  1 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c
>> index 45aab89..e816e80 100644
>> --- a/drivers/video/omap2/dss/sdi.c
>> +++ b/drivers/video/omap2/dss/sdi.c
>> @@ -23,13 +23,16 @@
>>  #include <linux/clk.h>
>>  #include <linux/delay.h>
>>  #include <linux/err.h>
>> +#include <linux/regulator/consumer.h>
>>  
>>  #include <plat/display.h>
>> +#include <plat/cpu.h>
>>  #include "dss.h"
>>  
>>  static struct {
>>  	bool skip_init;
>>  	bool update_enabled;
>> +	struct regulator *vdds_sdi_reg;
>>  } sdi;
>>  
>>  static void sdi_basic_init(void)
>> @@ -57,6 +60,12 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
>>  		goto err0;
>>  	}
>>  
>> +	if (cpu_is_omap34xx()) {
>> +		r = regulator_enable(sdi.vdds_sdi_reg);
>> +		if (r)
>> +			goto err1;
>> +	}
>> +
>>  	/* In case of skip_init sdi_init has already enabled the clocks */
>>  	if (!sdi.skip_init)
>>  		dss_clk_enable(DSS_CLK_ICK | DSS_CLK_FCK1);
>> @@ -118,10 +127,10 @@ int omapdss_sdi_display_enable(struct omap_dss_device *dssdev)
>>  	sdi.skip_init = 0;
>>  
>>  	return 0;
>> -err3:
>> -	dssdev->manager->disable(dssdev->manager);
>>  err2:
>>  	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
>> +	if (cpu_is_omap34xx())
>> +		regulator_enable(sdi.vdds_sdi_reg);
>>  err1:
>>  	omap_dss_stop_device(dssdev);
>>  err0:
>> @@ -137,6 +146,9 @@ void omapdss_sdi_display_disable(struct omap_dss_device *dssdev)
>>  
>>  	dss_clk_disable(DSS_CLK_ICK | DSS_CLK_FCK1);
>>  
>> +	if (cpu_is_omap34xx())
>> +		regulator_enable(sdi.vdds_sdi_reg);
>> +
>>  	omap_dss_stop_device(dssdev);
>>  }
>>  EXPORT_SYMBOL(omapdss_sdi_display_disable);
>> @@ -144,6 +156,13 @@ EXPORT_SYMBOL(omapdss_sdi_display_disable);
>>  int sdi_init_display(struct omap_dss_device *dssdev)
>>  {
>>  	DSSDBG("SDI init\n");
>> +	if (cpu_is_omap34xx()) {
>> +		sdi.vdds_sdi_reg = dss_get_vdds_sdi();
>> +		if (IS_ERR(sdi.vdds_sdi_reg)) {
>> +			DSSERR("can't get VDDS_SDI regulator\n");
>> +			return PTR_ERR(sdi.vdds_sdi_reg);
>> +		}
>> +	}
>>  
>>  	return 0;
>>  }
> 
> Same comment here as to DPI patch, I think the regulator init should be
> done in sdi_init().
> 
OK.

> And why do you test for omap34xx?
> 
I just assumed that, so i guess that was wrong. Will check the TRM and fix it.

-roger

Re: [RFC][PATCH 3/3] OMAP: DSS2: Use vdds_sdi regulator supply in

[Tomi Valkeinen]

On Mon, 2010-03-15 at 12:26 +0100, Quadros Roger (Nokia-D/Helsinki)
wrote:
> Hi,
> 
> Valkeinen Tomi (Nokia-D/Helsinki) wrote:
> > On Fri, 2010-03-12 at 16:27 +0100, Quadros Roger (Nokia-D/Helsinki)
> > wrote:
> >> From: Roger Quadros <roger.quadros@nokia.com>
> >>
> >> This patch enables the use of vdds_sdi regulator in SDI subsystem.
> >> We can disable the vdds_sdi voltage when not in use to save
> >> power.
> >>
> >> Signed-off-by: Roger Quadros <roger.quadros@nokia.com>
> >> ---
> >>  drivers/video/omap2/dss/sdi.c |   23 +++++++++++++++++++++--
> >>  1 files changed, 21 insertions(+), 2 deletions(-)
> >>

<snip>

> > And why do you test for omap34xx?
> > 
> I just assumed that, so i guess that was wrong. Will check the TRM and fix it.

Well, SDI is only for omap34xx. But why do you test it inside sdi.c, and
only for the regulator? I don't think SDI will work any better on
OMAP3630 (which doesn't have the SDI block), even if you don't use the
regulators ;).

So I think it's safe to assume inside sdi.c that we are running on a
device with SDI HW block.

 Tomi