[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

The LCD was turned on if the variable power was > 0, but that was incorrect. The
LCD has to be turned on in every case but POWERDOWN.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/video/backlight/l4f00242t03.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
index 9093ef0..ac20b2e 100644
--- a/drivers/video/backlight/l4f00242t03.c
+++ b/drivers/video/backlight/l4f00242t03.c
@@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
 	const u16 slpin = 0x10;
 	const u16 disoff = 0x28;
 
-	if (power) {
+	if (power < FB_BLANK_POWERDOWN) {
 		if (priv->lcd_on)
 			return 0;
 
-- 
1.7.1

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Alberto Panizzo]

On ven, 2010-06-04 at 07:52 +0200, Marek Vasut wrote:
> The LCD was turned on if the variable power was > 0, but that was incorrect. The
> LCD has to be turned on in every case but POWERDOWN.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Ack

> ---
>  drivers/video/backlight/l4f00242t03.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
> index 9093ef0..ac20b2e 100644
> --- a/drivers/video/backlight/l4f00242t03.c
> +++ b/drivers/video/backlight/l4f00242t03.c
> @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
>  	const u16 slpin = 0x10;
>  	const u16 disoff = 0x28;
>  
> -	if (power) {
> +	if (power < FB_BLANK_POWERDOWN) {
>  		if (priv->lcd_on)
>  			return 0;
>  

lcd.h states:
struct lcd_ops {
	/* Get the LCD panel power status (0: full on, 1..3: controller
	   power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
	int (*get_power)(struct lcd_device *);
	/* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) */
	int (*set_power)(struct lcd_device *, int power);

AFAIK the gray area between FB_BLANK_NORMAL and FB_BLANK_HSYNC_SUSPEND
can be chosen as needs.

Why not:

+ #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)

...

-	if (power) {
+	if (POWER_IS_ON(power)) {
 		if (priv->lcd_on)
 			return 0;

So with FB_BLANK_NORMAL the lcd is active and on -> low latencies
in re-enabling the video out, but for !POWER_IS_ON(power) we will send  
the lcd to standby.

It have to be implemented for power == FB_BLANK_POWERDOWN to turn off the 
regulators of the lcd.

What do you think about?

Regards,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

The LCD was turned on if the variable power was > 0, but that was incorrect. The
LCD has to be turned on in every case but POWERDOWN.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/video/backlight/l4f00242t03.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
index 9093ef0..c67801e 100644
--- a/drivers/video/backlight/l4f00242t03.c
+++ b/drivers/video/backlight/l4f00242t03.c
@@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
 	const u16 slpin = 0x10;
 	const u16 disoff = 0x28;
 
-	if (power) {
+	if (power <= FB_BLANK_NORMAL) {
 		if (priv->lcd_on)
 			return 0;
 
-- 
1.7.1

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

Dne P? 4. ?ervna 2010 09:29:43 Alberto Panizzo napsal(a):
> On ven, 2010-06-04 at 07:52 +0200, Marek Vasut wrote:
> > The LCD was turned on if the variable power was > 0, but that was
> > incorrect. The LCD has to be turned on in every case but POWERDOWN.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> Ack
> 
> > ---
> > 
> >  drivers/video/backlight/l4f00242t03.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/l4f00242t03.c
> > b/drivers/video/backlight/l4f00242t03.c index 9093ef0..ac20b2e 100644
> > --- a/drivers/video/backlight/l4f00242t03.c
> > +++ b/drivers/video/backlight/l4f00242t03.c
> > @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device
> > *ld, int power)
> > 
> >  	const u16 slpin = 0x10;
> >  	const u16 disoff = 0x28;
> > 
> > -	if (power) {
> > +	if (power < FB_BLANK_POWERDOWN) {
> > 
> >  		if (priv->lcd_on)
> >  		
> >  			return 0;
> 
> lcd.h states:
> struct lcd_ops {
> 	/* Get the LCD panel power status (0: full on, 1..3: controller
> 	   power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
> 	int (*get_power)(struct lcd_device *);
> 	/* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) 
*/
> 	int (*set_power)(struct lcd_device *, int power);
> 
> AFAIK the gray area between FB_BLANK_NORMAL and FB_BLANK_HSYNC_SUSPEND
> can be chosen as needs.
> 
> Why not:
> 
> + #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
> 
> ...
> 
> -	if (power) {
> +	if (POWER_IS_ON(power)) {
>  		if (priv->lcd_on)
>  			return 0;
> 
> So with FB_BLANK_NORMAL the lcd is active and on -> low latencies
> in re-enabling the video out, but for !POWER_IS_ON(power) we will send
> the lcd to standby.
> 
> It have to be implemented for power == FB_BLANK_POWERDOWN to turn off the
> regulators of the lcd.
> 
> What do you think about?

Thanks! I sent a new (better) version. If you want credit in it, tell me how 
should I add you one ;-)
> 
> Regards,

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Alberto Panizzo]

On sab, 2010-06-05 at 03:25 +0200, Marek Vasut wrote:
> Dne P? 4. ?ervna 2010 09:29:43 Alberto Panizzo napsal(a):
> > On ven, 2010-06-04 at 07:52 +0200, Marek Vasut wrote:
> > > The LCD was turned on if the variable power was > 0, but that was
> > > incorrect. The LCD has to be turned on in every case but POWERDOWN.
> > > 
> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > 
> > Ack
> > 
> > > ---
> > > 
> > >  drivers/video/backlight/l4f00242t03.c |    2 +-
> > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/drivers/video/backlight/l4f00242t03.c
> > > b/drivers/video/backlight/l4f00242t03.c index 9093ef0..ac20b2e 100644
> > > --- a/drivers/video/backlight/l4f00242t03.c
> > > +++ b/drivers/video/backlight/l4f00242t03.c
> > > @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device
> > > *ld, int power)
> > > 
> > >  	const u16 slpin = 0x10;
> > >  	const u16 disoff = 0x28;
> > > 
> > > -	if (power) {
> > > +	if (power < FB_BLANK_POWERDOWN) {
> > > 
> > >  		if (priv->lcd_on)
> > >  		
> > >  			return 0;
> > 
> > lcd.h states:
> > struct lcd_ops {
> > 	/* Get the LCD panel power status (0: full on, 1..3: controller
> > 	   power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
> > 	int (*get_power)(struct lcd_device *);
> > 	/* Enable or disable power to the LCD (0: on; 4: off, see FB_BLANK_XXX) 
> */
> > 	int (*set_power)(struct lcd_device *, int power);
> > 
> > AFAIK the gray area between FB_BLANK_NORMAL and FB_BLANK_HSYNC_SUSPEND
> > can be chosen as needs.
> > 
> > Why not:
> > 
> > + #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
> > 
> > ...
> > 
> > -	if (power) {
> > +	if (POWER_IS_ON(power)) {
> >  		if (priv->lcd_on)
> >  			return 0;
> > 
> > So with FB_BLANK_NORMAL the lcd is active and on -> low latencies
> > in re-enabling the video out, but for !POWER_IS_ON(power) we will send
> > the lcd to standby.
> > 
> > It have to be implemented for power == FB_BLANK_POWERDOWN to turn off the
> > regulators of the lcd.
> > 
> > What do you think about?
> 
> Thanks! I sent a new (better) version. If you want credit in it, tell me how 
> should I add you one ;-)

Oh no, you have the credit on it. I admit that my L4F00242T03 is not my 
best driver, but soon I will work on it again ;)

Thank you for your fix!

Regards,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Alberto Panizzo]

On sab, 2010-06-05 at 03:25 +0200, Marek Vasut wrote:
> The LCD was turned on if the variable power was > 0, but that was incorrect. The
> LCD has to be turned on in every case but POWERDOWN.

The patch is ok for me, but the comment could much more reflect what
the patch does, no?

> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/video/backlight/l4f00242t03.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
> index 9093ef0..c67801e 100644
> --- a/drivers/video/backlight/l4f00242t03.c
> +++ b/drivers/video/backlight/l4f00242t03.c
> @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
>  	const u16 slpin = 0x10;
>  	const u16 disoff = 0x28;
>  
> -	if (power) {
> +	if (power <= FB_BLANK_NORMAL) {
>  		if (priv->lcd_on)
>  			return 0;
>  

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

Dne So 5. ?ervna 2010 11:22:02 Alberto Panizzo napsal(a):
> On sab, 2010-06-05 at 03:25 +0200, Marek Vasut wrote:
> > Dne P? 4. ?ervna 2010 09:29:43 Alberto Panizzo napsal(a):
> > > On ven, 2010-06-04 at 07:52 +0200, Marek Vasut wrote:
> > > > The LCD was turned on if the variable power was > 0, but that was
> > > > incorrect. The LCD has to be turned on in every case but POWERDOWN.
> > > > 
> > > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > > 
> > > Ack
> > > 
> > > > ---
> > > > 
> > > >  drivers/video/backlight/l4f00242t03.c |    2 +-
> > > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/drivers/video/backlight/l4f00242t03.c
> > > > b/drivers/video/backlight/l4f00242t03.c index 9093ef0..ac20b2e 100644
> > > > --- a/drivers/video/backlight/l4f00242t03.c
> > > > +++ b/drivers/video/backlight/l4f00242t03.c
> > > > @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct
> > > > lcd_device *ld, int power)
> > > > 
> > > >  	const u16 slpin = 0x10;
> > > >  	const u16 disoff = 0x28;
> > > > 
> > > > -	if (power) {
> > > > +	if (power < FB_BLANK_POWERDOWN) {
> > > > 
> > > >  		if (priv->lcd_on)
> > > >  		
> > > >  			return 0;
> > > 
> > > lcd.h states:
> > > struct lcd_ops {
> > > 
> > > 	/* Get the LCD panel power status (0: full on, 1..3: controller
> > > 	
> > > 	   power on, flat panel power off, 4: full off), see FB_BLANK_XXX */
> > > 	
> > > 	int (*get_power)(struct lcd_device *);
> > > 	/* Enable or disable power to the LCD (0: on; 4: off, see
> > > 	FB_BLANK_XXX)
> > 
> > */
> > 
> > > 	int (*set_power)(struct lcd_device *, int power);
> > > 
> > > AFAIK the gray area between FB_BLANK_NORMAL and FB_BLANK_HSYNC_SUSPEND
> > > can be chosen as needs.
> > > 
> > > Why not:
> > > 
> > > + #define POWER_IS_ON(pwr)	((pwr) <= FB_BLANK_NORMAL)
> > > 
> > > ...
> > > 
> > > -	if (power) {
> > > +	if (POWER_IS_ON(power)) {
> > > 
> > >  		if (priv->lcd_on)
> > >  		
> > >  			return 0;
> > > 
> > > So with FB_BLANK_NORMAL the lcd is active and on -> low latencies
> > > in re-enabling the video out, but for !POWER_IS_ON(power) we will send
> > > the lcd to standby.
> > > 
> > > It have to be implemented for power == FB_BLANK_POWERDOWN to turn off
> > > the regulators of the lcd.
> > > 
> > > What do you think about?
> > 
> > Thanks! I sent a new (better) version. If you want credit in it, tell me
> > how should I add you one ;-)
> 
> Oh no, you have the credit on it. I admit that my L4F00242T03 is not my
> best driver, but soon I will work on it again ;)

You're very welcome, you did a great job!
> 
> Thank you for your fix!
> 
> Regards,

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

Dne So 5. ?ervna 2010 11:25:24 Alberto Panizzo napsal(a):
> On sab, 2010-06-05 at 03:25 +0200, Marek Vasut wrote:
> > The LCD was turned on if the variable power was > 0, but that was
> > incorrect. The LCD has to be turned on in every case but POWERDOWN.
> 
> The patch is ok for me, but the comment could much more reflect what
> the patch does, no?

Thanks for validating my work, we'll have another round ;)
> 
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > ---
> > 
> >  drivers/video/backlight/l4f00242t03.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/l4f00242t03.c
> > b/drivers/video/backlight/l4f00242t03.c index 9093ef0..c67801e 100644
> > --- a/drivers/video/backlight/l4f00242t03.c
> > +++ b/drivers/video/backlight/l4f00242t03.c
> > @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device
> > *ld, int power)
> > 
> >  	const u16 slpin = 0x10;
> >  	const u16 disoff = 0x28;
> > 
> > -	if (power) {
> > +	if (power <= FB_BLANK_NORMAL) {
> > 
> >  		if (priv->lcd_on)
> >  		
> >  			return 0;

[PATCH 1/2] backlight: Fix blanking for LMS283GF05 LCD

[Marek Vasut]

The LCD was turned on if the variable power was > 0, but that was incorrect. The
LCD has to be turned on in NORMAL and UNBLANK case.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/video/backlight/lms283gf05.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/lms283gf05.c b/drivers/video/backlight/lms283gf05.c
index abc43a0..5d3cf33 100644
--- a/drivers/video/backlight/lms283gf05.c
+++ b/drivers/video/backlight/lms283gf05.c
@@ -129,7 +129,7 @@ static int lms283gf05_power_set(struct lcd_device *ld, int power)
 	struct spi_device *spi = st->spi;
 	struct lms283gf05_pdata *pdata = spi->dev.platform_data;
 
-	if (power) {
+	if (power <= FB_BLANK_NORMAL) {
 		if (pdata)
 			lms283gf05_reset(pdata->reset_gpio,
 					pdata->reset_inverted);
-- 
1.7.1

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Marek Vasut]

The LCD was turned on if the variable power was > 0, but that was incorrect. The
LCD has to be turned on in NORMAL and UNBLANK case.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/video/backlight/l4f00242t03.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
index 9093ef0..c67801e 100644
--- a/drivers/video/backlight/l4f00242t03.c
+++ b/drivers/video/backlight/l4f00242t03.c
@@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
 	const u16 slpin = 0x10;
 	const u16 disoff = 0x28;
 
-	if (power) {
+	if (power <= FB_BLANK_NORMAL) {
 		if (priv->lcd_on)
 			return 0;
 
-- 
1.7.1

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Alberto Panizzo]

On sab, 2010-06-05 at 11:50 +0200, Marek Vasut wrote:
> The LCD was turned on if the variable power was > 0, but that was incorrect. The
> LCD has to be turned on in NORMAL and UNBLANK case.
> 
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

Richard,
Are you still maintaining the backlight tree?
These two patches never hit the mainline since gen, I hope this will 
happen soon since they really fix bugs :)

Best Regards,
Alberto!

Oh, add also this tag if possible:

Acked-by: Alberto Panizzo <maramaopercheseimorto@gmail.com>
 
> ---
>  drivers/video/backlight/l4f00242t03.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
> index 9093ef0..c67801e 100644
> --- a/drivers/video/backlight/l4f00242t03.c
> +++ b/drivers/video/backlight/l4f00242t03.c
> @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
>  	const u16 slpin = 0x10;
>  	const u16 disoff = 0x28;
>  
> -	if (power) {
> +	if (power <= FB_BLANK_NORMAL) {
>  		if (priv->lcd_on)
>  			return 0;
>  


-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Richard Purdie]

On Fri, 2010-11-05 at 11:48 +0100, Alberto Panizzo wrote:
> On sab, 2010-06-05 at 11:50 +0200, Marek Vasut wrote:
> > The LCD was turned on if the variable power was > 0, but that was incorrect. The
> > LCD has to be turned on in NORMAL and UNBLANK case.
> > 
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> 
> Richard,
> Are you still maintaining the backlight tree?
> These two patches never hit the mainline since gen, I hope this will 
> happen soon since they really fix bugs :)
> 
> Best Regards,
> Alberto!
> 
> Oh, add also this tag if possible:
> 
> Acked-by: Alberto Panizzo <maramaopercheseimorto@gmail.com>

I've talked with Andrew Morton and we've come up with a new plan to
handle the backlight patches. I acked these the other day and they
should be heading into -rc2.

Cheers,

Richard


> > ---
> >  drivers/video/backlight/l4f00242t03.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/video/backlight/l4f00242t03.c b/drivers/video/backlight/l4f00242t03.c
> > index 9093ef0..c67801e 100644
> > --- a/drivers/video/backlight/l4f00242t03.c
> > +++ b/drivers/video/backlight/l4f00242t03.c
> > @@ -78,7 +78,7 @@ static int l4f00242t03_lcd_power_set(struct lcd_device *ld, int power)
> >  	const u16 slpin = 0x10;
> >  	const u16 disoff = 0x28;
> >  
> > -	if (power) {
> > +	if (power <= FB_BLANK_NORMAL) {
> >  		if (priv->lcd_on)
> >  			return 0;
> >  
> 
> 

[PATCH 2/2] backlight: Fix blanking for L4F00242T03 LCD

[Alberto Panizzo]

On ven, 2010-11-05 at 11:09 +0000, Richard Purdie wrote:
> On Fri, 2010-11-05 at 11:48 +0100, Alberto Panizzo wrote:
> > On sab, 2010-06-05 at 11:50 +0200, Marek Vasut wrote:
> > > The LCD was turned on if the variable power was > 0, but that was incorrect. The
> > > LCD has to be turned on in NORMAL and UNBLANK case.
> > > 
> > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > 
> > Richard,
> > Are you still maintaining the backlight tree?
> > These two patches never hit the mainline since gen, I hope this will 
> > happen soon since they really fix bugs :)
> > 
> > Best Regards,
> > Alberto!
> > 
> > Oh, add also this tag if possible:
> > 
> > Acked-by: Alberto Panizzo <maramaopercheseimorto@gmail.com>
> 
> I've talked with Andrew Morton and we've come up with a new plan to
> handle the backlight patches. I acked these the other day and they
> should be heading into -rc2.
> 
> Cheers,
> 
> Richard

Ok, thank you for the quick answer!

Cheers,

-- 
Alberto!

        Be Persistent!
                - Greg Kroah-Hartman (FOSDEM 2010)