2.6.21-rc1 dims my LCD

2.6.21-rc1 dims my LCD

[Jiri Kosina]

Richard,

2.6.21-rc1 on my ibm t42p dims a LCD after some time (I guess it happens 
at the time the console should normally be blanked). 

When I hit the keyboard, the brightness stays low (it's 50% of light or 
so, so I could read what's on the screen, but it's uncomfortably dim), and 
I have to manually raise the brightness on the LCD. Quite annoying :)

I have bisected this to your commit 
994efacdf9a087b52f71e620b58dfa526b0cf928

Thanks,

-- 
Jiri Kosina

Re: 2.6.21-rc1 dims my LCD

[Richard Purdie]

Hi,

On Mon, 2007-02-26 at 01:59 +0100, Jiri Kosina wrote:
> 2.6.21-rc1 on my ibm t42p dims a LCD after some time (I guess it happens 
> at the time the console should normally be blanked). 
> 
> When I hit the keyboard, the brightness stays low (it's 50% of light or 
> so, so I could read what's on the screen, but it's uncomfortably dim), and 
> I have to manually raise the brightness on the LCD. Quite annoying :)
>
> I have bisected this to your commit 
> 994efacdf9a087b52f71e620b58dfa526b0cf928

Which framebuffer driver and backlight driver are you using?
("ls /sys/class/backlight/" will show which backlight it is)

Is the machine in active use when it dims or at idle and does the screen
blank at the same time it dims? If so, does the keypress unblank the
screen (but not change the brightness)? 

Also, is this on a console or under something like X? 

Thanks,

Richard

Re: 2.6.21-rc1 dims my LCD

[Jiri Kosina]

On Mon, 26 Feb 2007, Richard Purdie wrote:

> > When I hit the keyboard, the brightness stays low (it's 50% of light 
> > or so, so I could read what's on the screen, but it's uncomfortably 
> > dim), and I have to manually raise the brightness on the LCD. Quite 
> > annoying :) I have bisected this to your commit 
> > 994efacdf9a087b52f71e620b58dfa526b0cf928
> Which framebuffer driver and backlight driver are you using?
> ("ls /sys/class/backlight/" will show which backlight it is)

It's IBM:

$ ll /sys/class/backlight/
drwxr-xr-x 2 root root 0 Feb 26 13:28 ibm

At the time the brightness goes low, there is '0' in 
/sys/class/backlight/ibm/actual_brightness and 
/sys/class/backlight/ibm/brightness

Echoing '7' into /sys/class/backlight/ibm/brightness gets the brightness 
back again to normal.

> Is the machine in active use when it dims or at idle and does the screen 
> blank at the same time it dims? If so, does the keypress unblank the 
> screen (but not change the brightness)?

It doesn't blank at the time it dims - it just decreases brightness. I 
will do some more tests, but it seemed on a first sight that it happens 
only when the machine is idle.

> Also, is this on a console or under something like X? 

I observed it only on console, but didn't experiment with it too much yet.

-- 
Jiri Kosina

Re: 2.6.21-rc1 dims my LCD

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Jiri Kosina wrote:
> On Mon, 26 Feb 2007, Richard Purdie wrote:
> > > When I hit the keyboard, the brightness stays low (it's 50% of light 
> > > or so, so I could read what's on the screen, but it's uncomfortably 
> > > dim), and I have to manually raise the brightness on the LCD. Quite 
> > > annoying :) I have bisected this to your commit 
> > > 994efacdf9a087b52f71e620b58dfa526b0cf928
> > Which framebuffer driver and backlight driver are you using?
> > ("ls /sys/class/backlight/" will show which backlight it is)
> 
> It's IBM:

Please add me to the CC, then.  I noticed this thread by accident.

> $ ll /sys/class/backlight/
> drwxr-xr-x 2 root root 0 Feb 26 13:28 ibm
> 
> At the time the brightness goes low, there is '0' in 
> /sys/class/backlight/ibm/actual_brightness and 
> /sys/class/backlight/ibm/brightness

ibm-acpi does not implement display poweroff, so if a screen-saving function
on the kernel is trying to use it to shut the display off, it will either
fail to do anything, or if it ALSO sets brightness to zero, it will just dim
the display.

Richard, would you like a patch to -ENOSYS if a display power management
event reaches ibm-acpi?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: 2.6.21-rc1 dims my LCD

[Richard Purdie]

On Mon, 2007-02-26 at 11:21 -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 26 Feb 2007, Jiri Kosina wrote:
> > On Mon, 26 Feb 2007, Richard Purdie wrote:
> > > > When I hit the keyboard, the brightness stays low (it's 50% of light 
> > > > or so, so I could read what's on the screen, but it's uncomfortably 
> > > > dim), and I have to manually raise the brightness on the LCD. Quite 
> > > > annoying :) I have bisected this to your commit 
> > > > 994efacdf9a087b52f71e620b58dfa526b0cf928
> > > Which framebuffer driver and backlight driver are you using?
> > > ("ls /sys/class/backlight/" will show which backlight it is)
> > 
> > It's IBM:
> 
> Please add me to the CC, then.  I noticed this thread by accident.
> 
> > $ ll /sys/class/backlight/
> > drwxr-xr-x 2 root root 0 Feb 26 13:28 ibm
> > 
> > At the time the brightness goes low, there is '0' in 
> > /sys/class/backlight/ibm/actual_brightness and 
> > /sys/class/backlight/ibm/brightness
> 
> ibm-acpi does not implement display poweroff, so if a screen-saving function
> on the kernel is trying to use it to shut the display off, it will either
> fail to do anything, or if it ALSO sets brightness to zero, it will just dim
> the display.

I think I can explain whats going on. The commit you pointed to isn't
really at fault, it just happens to trigger an extra framebuffer
blanking call and this exposes a bug in the ibm backlight code which
there is already a fix for.

Basically, console blanking triggers a backlight_update_status() call
and bd->props.brightness wasn't set correctly at boot time. If
bd->props.brightness is set correctly, that call won't cause a problem. 

Jiri: I've appended a patch that should already be queued, could you
test and see if it solves the problem.

> Richard, would you like a patch to -ENOSYS if a display power management
> event reaches ibm-acpi?

The ibm backlight driver should really try and work with the system a
little more. Even if it can't turn the display off, the power attribute
should  minimise power usage as best it can. If that means just dimming
the display, so be it. Ideally, I'd still prefer for it to gain support
for turning the display off.

In the update_status function, there are three sources of information
you need to combine:

bd->props.brightness
bd->props.power
bd->props.fb_blank

Since the backlight class doesn't know the capabilities of the hardware,
it is left to each implementation to combine these options in a way that
makes sense. Currently ibm just looks at the first one but it could dim
if the latter two are anything other than FB_BLANK_UNBLANK. See corgi_bl
as an example (which also looks at its own variables too).

Richard

----

From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

ACPI: ibm-acpi: fix initial status of backlight device

The brightness class core does not update the initial status of the
device's brightness at register time.  Do it by ourselves.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Acked-by: Richard Purdie <rpurdie@rpsys.net>
---
 drivers/acpi/ibm_acpi.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 4cc534e..7c1b418 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -1711,6 +1711,12 @@ static struct backlight_ops ibm_backlight_data = {
 
 static int brightness_init(void)
 {
+       int b;
+
+       b = brightness_get(NULL);
+       if (b < 0)
+               return b;
+
        ibm_backlight_device = backlight_device_register("ibm", NULL, NULL,
                                                         &ibm_backlight_data);
        if (IS_ERR(ibm_backlight_device)) {
@@ -1718,7 +1724,9 @@ static int brightness_init(void)
                return PTR_ERR(ibm_backlight_device);
        }
 
-        ibm_backlight_device->props.max_brightness = 7;
+       ibm_backlight_device->props.max_brightness = 7;
+       ibm_backlight_device->props.brightness = b;
+       backlight_update_status(ibm_backlight_device);
 
        return 0;
 }

Re: 2.6.21-rc1 dims my LCD

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Richard Purdie wrote:
> > Richard, would you like a patch to -ENOSYS if a display power management
> > event reaches ibm-acpi?
> 
> The ibm backlight driver should really try and work with the system a
> little more. Even if it can't turn the display off, the power attribute
> should  minimise power usage as best it can. If that means just dimming
> the display, so be it. Ideally, I'd still prefer for it to gain support
> for turning the display off.

Well, the thinkpad firmware doesn't know how to turn the backlight off, or
doesn't export that AFAIK.  It *can* do it in hardware (cuts power to the
inverter or somesuch in the lid switch, etc) but I know of no way to do it
using the firmware.  If I ever find a way, I will certainly add it.

The video card *can* do it, but we are talking either intel fb or radeon fb
here, I'd need a way to communicate with them, or they should be getting the
same events and doing it anyway.

> In the update_status function, there are three sources of information
> you need to combine:
> 
> bd->props.brightness
> bd->props.power
> bd->props.fb_blank
> 
> Since the backlight class doesn't know the capabilities of the hardware,
> it is left to each implementation to combine these options in a way that
> makes sense. Currently ibm just looks at the first one but it could dim
> if the latter two are anything other than FB_BLANK_UNBLANK. See corgi_bl
> as an example (which also looks at its own variables too).

I will implement that, and as usual I will request your ACK since it deals
with your subsystem too.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: 2.6.21-rc1 dims my LCD

[Jiri Kosina]

On Mon, 26 Feb 2007, Richard Purdie wrote:

> Jiri: I've appended a patch that should already be queued, could you
> test and see if it solves the problem.

Thanks. In the meantime I have gone through the code and I can confirm 
that this is the root cause of what I am observing.

Now regarding the patch - at the time when the dim happened previously, 
currently there is a observable blink (after which the brightness is 
correct). I have put some debugging printk() into fb_notifier_callback(), 
and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
to backlight_update_status(), second immediately following the first one:

Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0

Is this really a right thing to do?

-- 
Jiri Kosina

Re: 2.6.21-rc1 dims my LCD

[Richard Purdie]

On Mon, 2007-02-26 at 16:24 +0100, Jiri Kosina wrote:
> On Mon, 26 Feb 2007, Richard Purdie wrote:
> 
> > Jiri: I've appended a patch that should already be queued, could you
> > test and see if it solves the problem.
> 
> Thanks. In the meantime I have gone through the code and I can confirm 
> that this is the root cause of what I am observing.
> 
> Now regarding the patch - at the time when the dim happened previously, 
> currently there is a observable blink (after which the brightness is 
> correct). 

The reason for the behaviour is that its turning the backlight down/off
to save power as it thinks the screen is blank. The blink therefore
makes sense as far as the backlight class is concerned and its working
as intended now.

> I have put some debugging printk() into fb_notifier_callback(), 
> and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> to backlight_update_status(), second immediately following the first one:
> 
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0
> 
> Is this really a right thing to do?

This should come from two calls to fbcon_generic_blank() in
drivers/video/console/fbcon.c with a different blank parameter and is
therefore in a way outside the scope of the backlight class.

It would be interesting to know why it decides to blank then immediately
unblank the display though. I've cc'd the fbdev-devel list.

Richard

Re: 2.6.21-rc1 dims my LCD

[Richard Purdie]

On Mon, 2007-02-26 at 16:24 +0100, Jiri Kosina wrote:
> On Mon, 26 Feb 2007, Richard Purdie wrote:
> 
> > Jiri: I've appended a patch that should already be queued, could you
> > test and see if it solves the problem.
> 
> Thanks. In the meantime I have gone through the code and I can confirm 
> that this is the root cause of what I am observing.
> 
> Now regarding the patch - at the time when the dim happened previously, 
> currently there is a observable blink (after which the brightness is 
> correct). 

The reason for the behaviour is that its turning the backlight down/off
to save power as it thinks the screen is blank. The blink therefore
makes sense as far as the backlight class is concerned and its working
as intended now.

> I have put some debugging printk() into fb_notifier_callback(), 
> and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> to backlight_update_status(), second immediately following the first one:
> 
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0
> 
> Is this really a right thing to do?

This should come from two calls to fbcon_generic_blank() in
drivers/video/console/fbcon.c with a different blank parameter and is
therefore in a way outside the scope of the backlight class.

It would be interesting to know why it decides to blank then immediately
unblank the display though. I've cc'd the fbdev-devel list.

Richard



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: 2.6.21-rc1 dims my LCD

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Jiri Kosina wrote:
> Now regarding the patch - at the time when the dim happened previously, 
> currently there is a observable blink (after which the brightness is 
> correct). I have put some debugging printk() into fb_notifier_callback(), 
> and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> to backlight_update_status(), second immediately following the first one:
> 
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0

This should cause *no* blink. It is setting brightness to zero anyway, which
is all ibm-acpi cares about (without a patch I will be sending in soon).
And ibm-acpi doesn't issue hardware calls that would not change the
brightness value.

If a brightness value query is causing blinks on your thinkpad, something is
very weird.

Maybe something else is also getting these events and processing them?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

[PATCH] ACPI: ibm-acpi: improve backlight power handling

[Henrique de Moraes Holschuh]

Improve the backlight code to emulate as much as possible the power
management events, as we are unable to really power on or power off the
backlight.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Richard Purdie <rpurdie@rpsys.net>
---

 Status: waiting ACK from Richard Purdie <rpurdie@rpsys.net>

 drivers/acpi/ibm_acpi.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index e7309a6..cb5885e 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -86,6 +86,7 @@
 
 #include <linux/proc_fs.h>
 #include <linux/backlight.h>
+#include <linux/fb.h>
 #include <asm/uaccess.h>
 
 #include <linux/dmi.h>
@@ -1707,7 +1708,8 @@ static int brightness_write(char *buf)
 
 static int brightness_update_status(struct backlight_device *bd)
 {
-	return brightness_set(bd->props.brightness);
+	return brightness_set((bd->props.fb_blank == FB_BLANK_UNBLANK)? 
+				bd->props.brightness : 0);
 }
 
 static struct backlight_ops ibm_backlight_data = {
-- 
1.5.0.1

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling

[Richard Purdie]

On Mon, 2007-02-26 at 13:12 -0300, Henrique de Moraes Holschuh wrote:
> @@ -1707,7 +1708,8 @@ static int brightness_write(char *buf)
>  
>  static int brightness_update_status(struct backlight_device *bd)
>  {
> -	return brightness_set(bd->props.brightness);
> +	return brightness_set((bd->props.fb_blank == FB_BLANK_UNBLANK)? 
> +				bd->props.brightness : 0);
>  }
>  
>  static struct backlight_ops ibm_backlight_data = {

Should we be looking at bd->props.power here too?

Richard

Re: 2.6.21-rc1 dims my LCD

[Richard Purdie]

On Mon, 2007-02-26 at 13:03 -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 26 Feb 2007, Jiri Kosina wrote:
> > Now regarding the patch - at the time when the dim happened previously, 
> > currently there is a observable blink (after which the brightness is 
> > correct). I have put some debugging printk() into fb_notifier_callback(), 
> > and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> > to backlight_update_status(), second immediately following the first one:
> > 
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0
> 
> This should cause *no* blink. It is setting brightness to zero anyway, which
> is all ibm-acpi cares about (without a patch I will be sending in soon).
> And ibm-acpi doesn't issue hardware calls that would not change the
> brightness value.

I'm assuming this was a paste from before your patch was applied and
that bd->props.brightness has a positive value when these calls were
made afterwards. Since the ibm backlight currently ignores fb_blank, it
shouldn't blink the backlight.

The screen still probably blinks since the framebuffer driver will see
the blank request too.

Richard

Re: [Linux-fbdev-devel] 2.6.21-rc1 dims my LCD

[Antonino A. Daplas]

On Mon, 2007-02-26 at 15:43 +0000, Richard Purdie wrote:
> On Mon, 2007-02-26 at 16:24 +0100, Jiri Kosina wrote:
> > On Mon, 26 Feb 2007, Richard Purdie wrote:
> > 
> > > Jiri: I've appended a patch that should already be queued, could you
> > > test and see if it solves the problem.
> > 
> > Thanks. In the meantime I have gone through the code and I can confirm 
> > that this is the root cause of what I am observing.
> > 
> > Now regarding the patch - at the time when the dim happened previously, 
> > currently there is a observable blink (after which the brightness is 
> > correct). 
> 
> The reason for the behaviour is that its turning the backlight down/off
> to save power as it thinks the screen is blank. The blink therefore
> makes sense as far as the backlight class is concerned and its working
> as intended now.
> 
> > I have put some debugging printk() into fb_notifier_callback(), 
> > and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> > to backlight_update_status(), second immediately following the first one:
> > 
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0
> > 
> > Is this really a right thing to do?

Well, no, the console should not blank and immediately unblank. Of
course, the console can blank after some time and can be induced to
unblank after a keyboard event, for example.

Besides the debugging printk's, can you also add something like
WARN_ON(1); so you can get a tracing too.

Tony

Re: 2.6.21-rc1 dims my LCD

[Antonino A. Daplas]

On Mon, 2007-02-26 at 15:43 +0000, Richard Purdie wrote:
> On Mon, 2007-02-26 at 16:24 +0100, Jiri Kosina wrote:
> > On Mon, 26 Feb 2007, Richard Purdie wrote:
> > 
> > > Jiri: I've appended a patch that should already be queued, could you
> > > test and see if it solves the problem.
> > 
> > Thanks. In the meantime I have gone through the code and I can confirm 
> > that this is the root cause of what I am observing.
> > 
> > Now regarding the patch - at the time when the dim happened previously, 
> > currently there is a observable blink (after which the brightness is 
> > correct). 
> 
> The reason for the behaviour is that its turning the backlight down/off
> to save power as it thinks the screen is blank. The blink therefore
> makes sense as far as the backlight class is concerned and its working
> as intended now.
> 
> > I have put some debugging printk() into fb_notifier_callback(), 
> > and it turns out that on FB_EVENT_CONBLANK, there are two successive calls 
> > to backlight_update_status(), second immediately following the first one:
> > 
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 1, bd->props.brightness == 0
> > Feb 26 15:11:14 thunder kernel: calling backlight_update_status() with bd->props.fb_blank == 0, bd->props.brightness == 0
> > 
> > Is this really a right thing to do?

Well, no, the console should not blank and immediately unblank. Of
course, the console can blank after some time and can be induced to
unblank after a keyboard event, for example.

Besides the debugging printk's, can you also add something like
WARN_ON(1); so you can get a tracing too.

Tony



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling

[Jiri Kosina]

On Mon, 26 Feb 2007, Henrique de Moraes Holschuh wrote:

> Improve the backlight code to emulate as much as possible the power
> management events, as we are unable to really power on or power off the
> backlight.

This still easily leads to confusing behavior, doesn't it? As there are 
power-related calls from backlight driver, which won't get handled 
properly by your code, in result confusing the brightness status.

I would suggest applying something like the patch below instead, if you 
find it OK.



From: Jiri Kosina <jkosina@suse.cz>

[PATCH] ibm-acpi: handle power calls from backlight class

Don't ignore the power-related calls from backlight class driver
and always adjust the brightness accordingly.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>

--- 

 drivers/acpi/ibm_acpi.c             |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 7c1b418..4cfa5f8 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -87,6 +87,7 @@
 #include <linux/proc_fs.h>
 #include <linux/backlight.h>
 #include <asm/uaccess.h>
+#include <linux/fb.h>
 
 #include <linux/dmi.h>
 #include <linux/jiffies.h>
@@ -1701,7 +1702,12 @@ static int brightness_write(char *buf)
 
 static int brightness_update_status(struct backlight_device *bd)
 {
-	return brightness_set(bd->props.brightness);
+	int brightness = 0;
+
+	if (bd->props.fb_blank == FB_BLANK_UNBLANK || bd->props.power == FB_BLANK_UNBLANK)
+		brightness = bd->props.brightness;
+	return brightness_set(brightness); 
+
 }
 
 static struct backlight_ops ibm_backlight_data = {

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Richard Purdie wrote:
> On Mon, 2007-02-26 at 13:12 -0300, Henrique de Moraes Holschuh wrote:
> > @@ -1707,7 +1708,8 @@ static int brightness_write(char *buf)
> >  
> >  static int brightness_update_status(struct backlight_device *bd)
> >  {
> > -	return brightness_set(bd->props.brightness);
> > +	return brightness_set((bd->props.fb_blank == FB_BLANK_UNBLANK)? 
> > +				bd->props.brightness : 0);
> >  }
> >  
> >  static struct backlight_ops ibm_backlight_data = {
> 
> Should we be looking at bd->props.power here too?

Probably.  I will update the patch.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Jiri Kosina wrote:
> On Mon, 26 Feb 2007, Henrique de Moraes Holschuh wrote:
> > Improve the backlight code to emulate as much as possible the power
> > management events, as we are unable to really power on or power off the
> > backlight.
> 
> This still easily leads to confusing behavior, doesn't it? As there are 
> power-related calls from backlight driver, which won't get handled 

I failed to notice the power thing, I will update the patch to handle it.
Thanks.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

[PATCH] ACPI: ibm-acpi: improve backlight power handling (v2)

[Henrique de Moraes Holschuh]

Improve the backlight code to emulate as much as possible the power
management events, as we are unable to really power on or power off the
backlight.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Richard Purdie <rpurdie@rpsys.net>
---

 Waiting ACK from Richard Purdie <rpurdie@rpsys.net>

 drivers/acpi/ibm_acpi.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index e7309a6..3690136 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -86,6 +86,7 @@
 
 #include <linux/proc_fs.h>
 #include <linux/backlight.h>
+#include <linux/fb.h>
 #include <asm/uaccess.h>
 
 #include <linux/dmi.h>
@@ -1707,7 +1708,10 @@ static int brightness_write(char *buf)
 
 static int brightness_update_status(struct backlight_device *bd)
 {
-	return brightness_set(bd->props.brightness);
+	return brightness_set(
+		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
+		 bd->props.power == FB_BLANK_UNBLANK) ?
+				bd->props.brightness : 0);
 }
 
 static struct backlight_ops ibm_backlight_data = {
-- 
1.5.0.1

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling (v2)

[Jiri Kosina]

On Mon, 26 Feb 2007, Henrique de Moraes Holschuh wrote:

>  static int brightness_update_status(struct backlight_device *bd)
>  {
> -	return brightness_set(bd->props.brightness);
> +	return brightness_set(
> +		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
> +		 bd->props.power == FB_BLANK_UNBLANK) ?
> +				bd->props.brightness : 0);
>  }

Are you sure about the '&&'? The original patch I submitted to you earlier 
today was checking for (bd->props.fb_blank == FB_BLANK_UNBLANK || 
bd->props.power == FB_BLANK_UNBLANK), and I tested it (to some extent) and 
it worked well - no sudden unblanking without reason, no blinking, etc.

I also think that common sense implies that the condition should be 
logical or - backlight layer could request blanking without requesting 
powering the device off, right? We want to handle unblanking from such 
situation properly, which doesn't necessairly mean we will get 
bd->props.power == FB_BLANK_UNBLANK, right?

-- 
Jiri Kosina

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling (v2)

[Richard Purdie]

On Mon, 2007-02-26 at 22:25 +0100, Jiri Kosina wrote:
> On Mon, 26 Feb 2007, Henrique de Moraes Holschuh wrote:
> 
> >  static int brightness_update_status(struct backlight_device *bd)
> >  {
> > -	return brightness_set(bd->props.brightness);
> > +	return brightness_set(
> > +		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
> > +		 bd->props.power == FB_BLANK_UNBLANK) ?
> > +				bd->props.brightness : 0);
> >  }
> 
> Are you sure about the '&&'? The original patch I submitted to you earlier 
> today was checking for (bd->props.fb_blank == FB_BLANK_UNBLANK || 
> bd->props.power == FB_BLANK_UNBLANK), and I tested it (to some extent) and 
> it worked well - no sudden unblanking without reason, no blinking, etc.
> 
> I also think that common sense implies that the condition should be 
> logical or - backlight layer could request blanking without requesting 
> powering the device off, right? We want to handle unblanking from such 
> situation properly, which doesn't necessairly mean we will get 
> bd->props.power == FB_BLANK_UNBLANK, right?

In the above context, && is correct, || isn't.

We want to blank (set to 0) if either fb_blank or power isn't set to
FB_BLANK_UNBLANK. This is the same as setting to brightness if both
fb_blank and power are set to FB_BLANK_UNBLANK. This is what the above
expression does.

Richard

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling (v2)

[Henrique de Moraes Holschuh]

On Mon, 26 Feb 2007, Richard Purdie wrote:
> We want to blank (set to 0) if either fb_blank or power isn't set to
> FB_BLANK_UNBLANK. This is the same as setting to brightness if both
> fb_blank and power are set to FB_BLANK_UNBLANK. This is what the above
> expression does.

Is that an ACK, then?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [PATCH] ACPI: ibm-acpi: improve backlight power handling (v2)

[Richard Purdie]

On Mon, 2007-02-26 at 15:26 -0300, Henrique de Moraes Holschuh wrote: 
> Improve the backlight code to emulate as much as possible the power
> management events, as we are unable to really power on or power off the
> backlight.
> 
> Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Acked-by: Richard Purdie <rpurdie@rpsys.net>
> 
>  drivers/acpi/ibm_acpi.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
> index e7309a6..3690136 100644
> --- a/drivers/acpi/ibm_acpi.c
> +++ b/drivers/acpi/ibm_acpi.c
> @@ -86,6 +86,7 @@
>  
>  #include <linux/proc_fs.h>
>  #include <linux/backlight.h>
> +#include <linux/fb.h>
>  #include <asm/uaccess.h>
>  
>  #include <linux/dmi.h>
> @@ -1707,7 +1708,10 @@ static int brightness_write(char *buf)
>  
>  static int brightness_update_status(struct backlight_device *bd)
>  {
> -	return brightness_set(bd->props.brightness);
> +	return brightness_set(
> +		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
> +		 bd->props.power == FB_BLANK_UNBLANK) ?
> +				bd->props.brightness : 0);
>  }
>  
>  static struct backlight_ops ibm_backlight_data = {