Re: no backlight on radeon after recent kernel "upgrade"s

Richard Purdie [off-list ref] writes:

On Mon, 2007-02-19 at 00:04 -0800, Andrew Morton wrote:

quoted

On Sun, 18 Feb 2007 23:46:16 -0500 Yaroslav Halchenko [off-list ref] wrote:

quoted

Since the most recent successful for me kernel 2.6.19-rc6-mm1, I've
tried few times to build more recent snapshots and now finally
2.6.20-mm2. In all those cases I have a sad outcome -- kernel boots but
at some point during boot (few moments after Penguin icon in the top left
corner appears), light turns off... I still can see something, especially
if I light a strong flashlight at a sharp angle.

I've enabled back-light support and lcd support for those unsuccessful
kernels (with all backlight support features disabled kernel boots
ok and backlight shines as bright as always)

Is 2.6.20 broken?  I assume the latest 2.6.20-gitN snapshot are failing..

It would be extremely useful to know which kernel versions you tested
and which ones failed. There are a number of backlight changes which are
just in 2.6.20-mm1 and -mm2 which it would be useful to rule out or
identify as the cause.

i have exactly the same problem with 2.6.21-rc1 on a thinkpad t40
with an ati radeon card. the machine boots up but the backlight never
comes on. 2.6.20 worked okay.

--alex--

-- 
| I believe the moment is at hand when, by a paranoiac and active |
|  advance of the mind, it will be possible (simultaneously with  |
|  automatism and other passive states) to systematize confusion  |
|  and thus to help to discredit completely the world of reality. |

On Wed, 2007-02-21 at 14:18 -0800, Alex Romosan wrote:

Richard Purdie [off-list ref] writes:
i have exactly the same problem with 2.6.21-rc1 on a thinkpad t40
with an ati radeon card. the machine boots up but the backlight never
comes on. 2.6.20 worked okay.

Can you have a look at the differences between the defconfigs for 2.6.20
and 2.6.21-rc1?

If the backlight changes are at fault, I'm guessing James' Kconfig
changes to the video Kconfig would be the culprit since
FB_RADEON_BACKLIGHT only used to be selectable if PMAC_BACKLIGHT was
enabled. On a thinkpad, the backlight is probably under ACPI control.

If FB_RADEON_BACKLIGHT wasn't set for 2.6.20, can you try 2.6.21-rc1
with that option disabled?

An ls /sys/class/backlight under 2.6.20 will tell us which driver it was
using...

Thanks,

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

On Wed, 21 Feb 2007, Richard Purdie wrote:

enabled. On a thinkpad, the backlight is probably under ACPI control.

BIOS+ACPI, actually.  Without ACPI video loaded, the firmware does
everything correctly.  With ACPI video, the firmware does it, then its
changes are clobbered over by ACPI video's.

If ibm-acpi is loaded, a backlight device "ibm" is added.  ibm-acpi's ibm
backlight device can change the display brightness.  It is *not* capable of
turning the backlight on or off, AFAIK.

BTW, some ThinkPads don't have a Radeon, but rather an Intel GM/GMX device.

-- 
  "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

On Wed, 2007-02-21 at 21:17 -0200, Henrique de Moraes Holschuh wrote:

On Wed, 21 Feb 2007, Richard Purdie wrote:

quoted

enabled. On a thinkpad, the backlight is probably under ACPI control.

BIOS+ACPI, actually.  Without ACPI video loaded, the firmware does
everything correctly.  With ACPI video, the firmware does it, then its
changes are clobbered over by ACPI video's.

If ibm-acpi is loaded, a backlight device "ibm" is added.  ibm-acpi's ibm
backlight device can change the display brightness.  It is *not* capable of
turning the backlight on or off, AFAIK.

BTW, some ThinkPads don't have a Radeon, but rather an Intel GM/GMX device.

I have a thinkpad with Intel GM graphics ;-). I need it to work so I try
not to experiment too much on it. I've just tried the ibm-acpi driver
and it doesn't work well :-(.

So far I've noticed:

* 'cat brightness' != 'cat actual_brightness' upon bootup (doesn't have
to be true if there is hardware brightness limiting or level changing in
action but that wouldn't be the case here and this could be fixed
easily).
* 'echo 0 > brightness' lowered the intensity but by a level or two, not
set it to level 0. A couple of more attempts and it did jump from 7 -> 1
and so on, it seems erratic.

actual_brightness always seems to be correct, as does brightness so it
looks like its not updating the hardware correctly.

Regards,

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

On Thu, 22 Feb 2007, Richard Purdie wrote:

I have a thinkpad with Intel GM graphics ;-). I need it to work so I try
not to experiment too much on it. I've just tried the ibm-acpi driver
and it doesn't work well :-(.

2.6.21-rc, or 2.6.20?  If it is in 2.6.21, could you give me a report of how
it fares on 2.6.20?

* 'cat brightness' != 'cat actual_brightness' upon bootup (doesn't have

Hmm, I see this in 2.6.20 too.  And brightness is the one that is buggy.  I
will look into it.

* 'echo 0 > brightness' lowered the intensity but by a level or two, not
set it to level 0. A couple of more attempts and it did jump from 7 -> 1
and so on, it seems erratic.

I know it used to work fine before 2.6.20.  Let me check...  works fine on a
T43, 2.6.20 (Radeon X300).  I need more data to fix it :-)

actual_brightness always seems to be correct, as does brightness so it
looks like its not updating the hardware correctly.

Well, if you have the ACPI video module loaded, unload it.  Does it work
now?

-- 
  "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

On Wed, 21 Feb 2007, Henrique de Moraes Holschuh wrote:

quoted

* 'cat brightness' != 'cat actual_brightness' upon bootup (doesn't have

Hmm, I see this in 2.6.20 too.  And brightness is the one that is buggy.  I
will look into it.

Now, that was trivial to fix, and I will reply with a patch (which will have
Cc's trimmed to just the MLs and Richard).

But really, should not the backlight *class* be doing the initial update of
the brightness?  Looks like something that every device would need to do if
the class doesn't do it, and unlike the "power it off on unregister" thing,
I can't think of a reason not to do it for every backlight class device.

Please ACK the ibm-acpi patch in my next message if you'd like me to submit
it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in
the backlight class core.

-- 
  "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

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

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

NOTE: This patch needs an ACK from Richard Purdie before it can be merged,
as he might want to change the backlight class code instead.

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

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 2429e11..6875421 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c

@@ -1713,6 +1713,13 @@ static struct backlight_properties ibm_backlight_data = {
 
 static int brightness_init(void)
 {
+	int b;
+
+	b = brightness_get(NULL);
+	if (b < 0)
+		return b;
+	ibm_backlight_data.brightness = b;
+
 	ibm_backlight_device = backlight_device_register("ibm", NULL, NULL,
 							 &ibm_backlight_data);
 	if (IS_ERR(ibm_backlight_device)) {

-- 
1.4.4.4

-- 
  "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

On Wed, 2007-02-21 at 23:16 -0200, Henrique de Moraes Holschuh wrote:

NOTE: This patch needs an ACK from Richard Purdie before it can be merged,
as he might want to change the backlight class code instead.

As mentioned elsewhere, we can't do this in the class itself.

This isn't against 2.6.21-rc1 which changed the backlight class a bit.
Basically, you need to set the brightness variable after
backlight_device_register(). It should be simple enough to do and fix
the problem the same way though.

Richard

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>
Cc: Richard Purdie <redacted>
---

Waiting ACK from Richard Purdie, applies on top of 2.6.21-rc1.  Also fixes a
whitespace problem.

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

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 4cc534e..c59ebff 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,8 @@ 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;
 
 	return 0;
 }

-- 
1.4.4.4

-- 
  "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

Here's the final version.  I will request a pull from Len Brown to get it
into acpi-test, from where it will eventually reach mainline at Len's
discretion.

-- 
  "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

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 <redacted>
---
 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;
 }

-- 
1.4.4.4

On Wed, 2007-02-21 at 23:10 -0200, Henrique de Moraes Holschuh wrote:

On Wed, 21 Feb 2007, Henrique de Moraes Holschuh wrote:

quoted

quoted

* 'cat brightness' != 'cat actual_brightness' upon bootup (doesn't have

Hmm, I see this in 2.6.20 too.  And brightness is the one that is buggy.  I
will look into it.

Now, that was trivial to fix, and I will reply with a patch (which will have
Cc's trimmed to just the MLs and Richard).

But really, should not the backlight *class* be doing the initial update of
the brightness?  Looks like something that every device would need to do if
the class doesn't do it, and unlike the "power it off on unregister" thing,
I can't think of a reason not to do it for every backlight class device.

Please ACK the ibm-acpi patch in my next message if you'd like me to submit
it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in
the backlight class core.

We can't change the backlight class code since some hardware can't read
from the device, only write to it. Initialisation in that case is a bit
different.

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

On Thu, 22 Feb 2007, Richard Purdie wrote:

quoted

it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in
the backlight class core.

We can't change the backlight class code since some hardware can't read
from the device, only write to it. Initialisation in that case is a bit
different.

Initializing stuff after registering is also racy as the device is not
locked but we are going to clobber data in its properties struct.  I don't
particularly care about that race, but...

Anyway, you have the 2.6.21-rc patch now, to ACK or NACK.  I still think the
class should be handling this.  If a device is write-only, it should have no
_get ops handler, which means that the class can easily differentiate the
two cases and do the right thing for both.  There's less code duplication
that way.

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

-- 
  "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

On Thu, 2007-02-22 at 12:56 -0200, Henrique de Moraes Holschuh wrote:

On Thu, 22 Feb 2007, Richard Purdie wrote:

quoted

quoted

it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in
the backlight class core.

We can't change the backlight class code since some hardware can't read
from the device, only write to it. Initialisation in that case is a bit
different.

Initializing stuff after registering is also racy as the device is not
locked but we are going to clobber data in its properties struct.  I don't
particularly care about that race, but...

If you really care, add a a call to backlight_update_status() after you
set the brightness attribute like some of the other drivers have. The
only data you're changing are single numbers and as long as
update_status is called afterwards, a consistent state is pushed to the
hardware so there is no race problem.

Anyway, you have the 2.6.21-rc patch now, to ACK or NACK.  I still think the
class should be handling this.  If a device is write-only, it should have no
_get ops handler, which means that the class can easily differentiate the
two cases and do the right thing for both.  There's less code duplication
that way.

Have a look at what corgi_bl does. It can know what state it set the
hardware too as it keeps track itself, it just can't read that state
from the hardware. Note how there is extra code in it to handle a power
limit on the backlight under certain conditions and how this is fed back
through the class through the get_brightness method.

Adding one line of code (admittedly slight more due to error handling),
is hardly that much code duplication.

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

I wasn't aware of this problem. If some devices need bits from both
raedon/whatever and acpi, the current implementations are just plain
wrong. Its not really a backlight class problem and more of an
implementation and interaction problem between acpi and the framebuffer
drivers. They should be presenting and registering *one* backlight class
device, not two. Without knowing more about the circumstances and
how/when to combine which drivers its hard for me to help further...

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

quoted

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

I wasn't aware of this problem. If some devices need bits from both
raedon/whatever and acpi, the current implementations are just plain
wrong. Its not really a backlight class problem and more of an
implementation and interaction problem between acpi and the framebuffer
drivers. They should be presenting and registering *one* backlight class
device, not two. Without knowing more about the circumstances and
how/when to combine which drivers its hard for me to help further...

This is always been a problem. For the fbdev layer you can run vesafb with 
a specific fbdev driver and they both access the same hardware. There is 
no really way around this. The user has to be smart enough to not enable 
bothe drivers.


-------------------------------------------------------------------------
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

On Thu, 22 Feb 2007, Richard Purdie wrote:

If you really care, add a a call to backlight_update_status() after you
set the brightness attribute like some of the other drivers have. The

I will.  Do you ACK the patch, then?

Have a look at what corgi_bl does. It can know what state it set the
hardware too as it keeps track itself, it just can't read that state

You are assuming nothing else is changing the hardware behind the driver's
back.  I am against such assumptions when they can be avoided, but that's a
particular PoV and not much more than that.  IMHO, if you cannot query the
hardware, you shouldn't provide a way to query the current brightness that
will be right only if nobody else messed with the device.

Maybe for corgi, that doesn't hold much strength, but for stuff tied to
ACPI, it does.  And in a ThinkPad's case, where even writes to /dev/nvram
can change the brightness, well, if there weren't a way to ask the EC the
current real brightness, there is NO way I'd be implementing it based on a
memory cache.

from the hardware. Note how there is extra code in it to handle a power
limit on the backlight under certain conditions and how this is fed back
through the class through the get_brightness method.

I will read the corgi driver code, it looks interesting.

Adding one line of code (admittedly slight more due to error handling),
is hardly that much code duplication.

No, it really isn't much trouble.  Which is why I wrote a patch right away.

quoted

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

I wasn't aware of this problem. If some devices need bits from both
raedon/whatever and acpi, the current implementations are just plain
wrong. Its not really a backlight class problem and more of an
implementation and interaction problem between acpi and the framebuffer
drivers. They should be presenting and registering *one* backlight class

I.e. we should add hooks to the framebuffer drivers?  It would work, that's
for sure.

-- 
  "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

On Thu, 2007-02-22 at 14:34 -0200, Henrique de Moraes Holschuh wrote:

On Thu, 22 Feb 2007, Richard Purdie wrote:

quoted

If you really care, add a a call to backlight_update_status() after you
set the brightness attribute like some of the other drivers have. The

I will.  Do you ACK the patch, then?

Yes, it can have an Acked-by: Richard Purdie [off-list ref].

quoted

Have a look at what corgi_bl does. It can know what state it set the
hardware too as it keeps track itself, it just can't read that state

You are assuming nothing else is changing the hardware behind the driver's
back. 

Which it can't in the corgi_bl case (excluding poking things
like /dev/mem). Its safe to assume it has exclusive access.

 I am against such assumptions when they can be avoided, but that's a
particular PoV and not much more than that.  IMHO, if you cannot query the
hardware, you shouldn't provide a way to query the current brightness that
will be right only if nobody else messed with the device.

Maybe for corgi, that doesn't hold much strength, but for stuff tied to
ACPI, it does.  And in a ThinkPad's case, where even writes to /dev/nvram
can change the brightness, well, if there weren't a way to ask the EC the
current real brightness, there is NO way I'd be implementing it based on a
memory cache.

Right, I'd be against such a driver. On the embedded hardware we can
safely assume there is nothing else playing with the brightness settings
though and such a driver is perfectly valid.

quoted

quoted

Howerver, I *do* strongly wish for a way to combine various drivers into a
single backlight device, where radeon/intelfb takes care of some stuff,
ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc.  Also, a
standard naming for the builtin screen(s) would help, calling it "ibm",
"asus", "sony" is not good IMHO.

I wasn't aware of this problem. If some devices need bits from both
raedon/whatever and acpi, the current implementations are just plain
wrong. Its not really a backlight class problem and more of an
implementation and interaction problem between acpi and the framebuffer
drivers. They should be presenting and registering *one* backlight class

I.e. we should add hooks to the framebuffer drivers?  It would work, that's
for sure.

If the backlight controls would then power off the backlight properly
that would be desirable as we've have a more standard interface.

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

On Wed, 2007-02-21 at 22:51 -0200, Henrique de Moraes Holschuh wrote:

On Thu, 22 Feb 2007, Richard Purdie wrote:

quoted

I have a thinkpad with Intel GM graphics ;-). I need it to work so I try
not to experiment too much on it. I've just tried the ibm-acpi driver
and it doesn't work well :-(.

2.6.21-rc, or 2.6.20? 

2.6.21-rc1

If it is in 2.6.21, could you give me a report of how
it fares on 2.6.20?

Maybe next week. I'm away over the weekend and I need it working now ;-).

quoted

* 'echo 0 > brightness' lowered the intensity but by a level or two, not
set it to level 0. A couple of more attempts and it did jump from 7 -> 1
and so on, it seems erratic.

I know it used to work fine before 2.6.20.  Let me check...  works fine on a
T43, 2.6.20 (Radeon X300).  I need more data to fix it :-)

The following sequence is reproducible:

echo 7 > brightness (repeat until actual_brightness reads 7)
echo 0 > brightness (brightness reads 0, actual_brightness reads 4)
echo 0 > brightness (brightness reads 0, actual_brightness reads 0)

I suspect this is interference from software on the machine as it does
show an onscreen display when I change the values in sysfs and the
values on the OSD don't match what's really going on.  The
actual_brightness does match the screen.

I guess this just means we need to get userspace to agree on one method
of access for this kind of thing. It makes me wondering where the
hardware brightness keys fit into things though...

quoted

actual_brightness always seems to be correct, as does brightness so it
looks like its not updating the hardware correctly.

Well, if you have the ACPI video module loaded, unload it.  Does it work
now?

No change if unloaded.

Cheers,

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

On Thu, 22 Feb 2007, Richard Purdie wrote:

The following sequence is reproducible:

echo 7 > brightness (repeat until actual_brightness reads 7)
echo 0 > brightness (brightness reads 0, actual_brightness reads 4)
echo 0 > brightness (brightness reads 0, actual_brightness reads 0)

As I said, it works fine on 2.6.20 on the hardware I have.  I will see if I
can test on 2.6.21, but it is something non-trivial for me to do right now.

I suspect this is interference from software on the machine as it does
show an onscreen display when I change the values in sysfs and the

Yes, I am using something like that here too, and it doesn't break anything.

values on the OSD don't match what's really going on.  The
actual_brightness does match the screen.

So something is screwing with what gets written to the EC, and it does it
AFTER ibm-acpi started processing the brightness change request.

actual_brightness asks the *EC* what the current brightness value is, so it
will never be wrong on a new-enough ThinkPad.

I guess this just means we need to get userspace to agree on one method
of access for this kind of thing. It makes me wondering where the
hardware brightness keys fit into things though...

For ThinkPads:

The Golden Rule: never talk to the EC or write to firmware-command-space in
the CMOS NVRAM in userspace.  If you do, bad things could happen and it is
your fault.

Here's how brightness control works in ThinkPads:

0. Brightness and backlight on/off state are decoupled.  Turning the
backlight on or off is done through DPMS or something else video-card
specific.  Newer models shut the backlight off by EC firmware (or maybe even
on hardware) when the lid is closed, too.

1. The thinkpad does everything brightness-related in firmware (EC+BIOS).
If you keep your hands off, it works correctly on every O.S.

2. The brightness up key exports hotkey events (all models) and a brightness
up ACPI event (*60 and newer thinkpads, with 2.x BIOS).  It is a bad idea to
use those events to change the backlight brightness, because the firmware
will be doing just that too.

3. It doesn't export anything for brightness down.  You find it happened
snooping the CMOS nvram.

4. Ibm-acpi doesn't care about the ACPI events much, it just asks the EC
what the brightness level is when it needs to do something, then it issues
both CMOS commands and EC writes to make damn sure the level is changed.
The CMOS commands are step-style, so to go from 4 to 7, you issue 3x
brightness up.

quoted

Well, if you have the ACPI video module loaded, unload it.  Does it work
now?

No change if unloaded.

I am out of ideas. But blacklist that module on your ThinkPad, it just gets
in the way, even if you are not using 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