Bug 12249

Created attachment 19362 [details]
original acpidump

Acpidump output attached (SL300 BIOS version 1.16)

Created attachment 19363 [details]
customized DSDT

I also tried a customized DSDT table (modified it in the same way that Zhang Rui did in bug #12037). With the customized DSDT, I get back 2 missing brightness levels.

However, the sysfs brightness file buggy behavior exactly the same, no matter whether I use the customized DSDT or the machine's original BIOS.

Created attachment 19366 [details]
patch: always update the props.brightness

two problems about the backlight sysfs I/F.

1. the "brightness" doesn't reflect the actual brightness value.
2. setting the "brightness" file doesn't change the backlight.

For the first problem, I'd say that it's the "/sys/class/backlight/.../actual_brightness" that reports the current backlight levels.
The "brightness" file just returns a cached value.

For the second problem, could you please try this patch and see if it helps?
Note that please DO NOT touch the "actual_brightness" file during the test.

(In reply to comment #3)
> Created an attachment (id=19366) [details]
> patch: always update the props.brightness

OK, with this patch, I can now set the brightness correctly from sysfs. However, reading the "brightness" sysfs value still gives wrong values; and the brightness shortcut keys still result in brightness "rolling over" from 100% to 25%.

By chance, do you know how hal/g-p-m might be incrementing or decrementing brightness? My guess is that hald is writing some magic value to something in /sys/class/backlight/acpi_video0/ but I am not sure what it is...

(In reply to comment #4)
> (In reply to comment #3)
> > Created an attachment (id=19366) [details] [details]
> > patch: always update the props.brightness
> 
> OK, with this patch, I can now set the brightness correctly from sysfs.
good news. :)

> However, reading the "brightness" sysfs value still gives wrong values;
Never trust the brightness value read from "brightness"
If you want to get the current backlight level, run "cat /sys/class/backlight/acpi_video0/actual_brightness"


> 
> By chance, do you know how hal/g-p-m might be incrementing or decrementing
> brightness?
No. :(

> My guess is that hald is writing some magic value to something in
> /sys/class/backlight/acpi_video0/ but I am not sure what it is...
> 
right.
please kill acpid, and run "cat /proc/acpi/event"
attach the output after pressing the brightness up/down hotkey.
please attach the dmesg output as well.

Created attachment 19368 [details]
dmesg output

(In reply to comment #5)
> Never trust the brightness value read from "brightness"
> If you want to get the current backlight level, run "cat
> /sys/class/backlight/acpi_video0/actual_brightness"

Doesn't work, /sys/class/backlight/acpi_video0/actual_brightness always says 0, no matter what the brightness is.

> please kill acpid, and run "cat /proc/acpi/event"
> attach the output after pressing the brightness up/down hotkey.

Each time I press "brightness down", cat /proc/acpi/event prints
video LCDD 00000087 00000000

Each time I press "brightness up", it prints
video LCDD 00000086 00000000
(even when it rolls over and decreases the brightness again).

> please attach the dmesg output as well.

There is nothing new added to dmesg when I change the screen brightness. For reference, here is the complete dmesg output (the "Lenovo SL series driver" is a driver for the multimedia hotkeys that I am writing at the moment).

please attach the result of  "grep . /proc/acpi/video/*/*/brightness"

Created attachment 19369 [details]
customized DSDT

please try this customized DSDT instead.
and attach the output of "grep . /proc/acpi/video/*/*/brightness" with this DSDT

(In reply to comment #7)
> please attach the result of  "grep . /proc/acpi/video/*/*/brightness"

With my current DSDT:

# grep . /proc/acpi/video/*/*/brightness 
/proc/acpi/video/VGA/CRTD/brightness:<not supported>
/proc/acpi/video/VGA/DVID/brightness:<not supported>
/proc/acpi/video/VGA/LCDD/brightness:levels:  100 80 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
/proc/acpi/video/VGA/LCDD/brightness:current: 100

With your customized DSDT:

# grep . /proc/acpi/video/*/*/brightness
/proc/acpi/video/VGA/CRTD/brightness:<not supported>
/proc/acpi/video/VGA/DVID/brightness:<not supported>
/proc/acpi/video/VGA/LCDD/brightness:levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
/proc/acpi/video/VGA/LCDD/brightness:current: 100

cat /sys/class/backlight/acpi_video0/actual_brightness still reads zero.

Backlight levels no longer roll over when incrementing. Instead, they roll over when decrementing :) In other words, if I decrement all the way down, backlight levels go 100 -> 90 -> 85 -> 80 -> 75 -> 70 -> 65 -> 60 -> 55 -> 50 -> 45 -> 40 -> 35 -> 30 -> 25 -> 20 -> 90 !

Also, could you please post the dsl source for your custom DSDTs so that I can repeat the changes you made on future BIOS versions, I can't get my copy of iasl to disassemble your DSDT.hex...

Created attachment 19371 [details]
source code of the customized DSDT

attached per Alexandre's request. :)

Created attachment 19372 [details]
video debug patch

please apply this patch, and attach the dmesg output after running "cat actual_brightness"

First: it seems that your customized DSDT produces a lot of error messages.

[   32.209855] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FF) is beyond end of object [20080926]
[   32.209874] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[   32.210030] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT
[   32.608059] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FF) is beyond end of object [20080926]
[   32.608066] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[   32.608181] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT
[   32.906924] usbcore: registered new interface driver uvcvideo
[   32.906932] USB Video Class driver (v0.1.0)
[   32.982373] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FF) is beyond end of object [20080926]
[   32.982381] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[   32.982431] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT

and so on for hundreds of lines


Second: after applying video debug patch and running "cat /sys/class/backlight/acpi_video0/actual_brightness", I get the following line in dmesg:

[ 4083.709874] Rui: current level is 255, brightness count is 18

Created attachment 19411 [details]
customized DSDT

please try this DSDT instead. :)

Created attachment 19412 [details]
customized DSDT -- source code

(In reply to comment #14)
> Created an attachment (id=19411) [details]
> customized DSDT
> 
> please try this DSDT instead. :)

Broken.

When incrementing/decrementing brightness, the brightness levels are randomized (even though I am using the brightness level sort patch from bug 12037). Setting a brightness level directly via sysfs or procfs has either no effect or the wrong effect. Information in /proc/acpi/video/VGA/LCDD/brightness does not correspond to reality.

dmesg is full of

[  662.924221] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT
[  663.303518] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FE) is beyond end of object [20080926]
[  663.303538] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[  663.303674] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT
[  663.309624] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FE) is beyond end of object [20080926]
[  663.309644] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[  663.309781] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT
[  812.406381] ACPI Exception (exoparg2-0444): AE_AML_BUFFER_LIMIT, Index (0000000FE) is beyond end of object [20080926]
[  812.406401] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa35e40), AE_AML_BUFFER_LIMIT
[  812.406537] ACPI Error (psparse-0524): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa303e0), AE_AML_BUFFER_LIMIT

Created attachment 19441 [details]
customized DSDT

hah, this one should work for you. please give it a try.

Created attachment 19442 [details]
source code of the customized DSDT

(In reply to comment #17)
> Created an attachment (id=19441) [details]
> customized DSDT
> 
> hah, this one should work for you. please give it a try.

From the user perspective, behaves identically to original BIOS. In other words, no dmesg errors, but on incrementing brightness, rolls over to 25%: ... -> 80 -> 85 -> 90 -> 100 -> 25

When the brightness is at 100% and I run "cat /sys/class/backlight/acpi_video0/actual_brightness", I get a one line message added to dmesg:

[  552.353363] Rui: current level is 15, brightness count is 18

The number after "current level is" seems to correctly reflect the brightness level, going from 0 (for 20% brightness) to 15 (for 100%).

(In reply to comment #19)
> From the user perspective, behaves identically to original BIOS.

Actually, to be precise, it behaves identically to my original customized DSDT. With the system's BIOS, two brightness levels were missing.

(In reply to comment #19)

> When the brightness is at 100% and I run "cat
> /sys/class/backlight/acpi_video0/actual_brightness", I get a one line message
> added to dmesg:
> 
> [  552.353363] Rui: current level is 15, brightness count is 18
> 
what did you get of running "cat
> /sys/class/backlight/acpi_video0/actual_brightness" ?

please "echo 6 > /sys/class/backlight/acpi_video0/brightness"
and "cat
> /sys/class/backlight/acpi_video0/actual_brightness", what do you get in
> dmesg?


> The number after "current level is" seems to correctly reflect the brightness
> level, going from 0 (for 20% brightness) to 15 (for 100%).
> 

Start with brightness at 100%

# cat /sys/class/backlight/acpi_video0/actual_brightness
0
# dmesg | tail
<...>
[ 3578.564570] Rui: current level is 15, brightness count is 18
# echo 6 > /sys/class/backlight/acpi_video0/brightness
/* brightness correctly goes to 50% */
# cat /sys/class/backlight/acpi_video0/actual_brightness
0
# dmesg | tail
<...>
[ 3578.564570] Rui: current level is 15, brightness count is 18
[ 3678.917684] Rui: current level is 6, brightness count is 18

Created attachment 19443 [details]
customized DSDT: fix another BIOS bug

_BQC should return the current backlight levels rather than an index in the PCTG package.

Created attachment 19444 [details]
source code of the customized DSDT

(In reply to comment #23)
> Created an attachment (id=19443) [details]
> customized DSDT: fix another BIOS bug
> 
> _BQC should return the current backlight levels rather than an index in the
> PCTG package.

With this DSDT, incrementing brightness above 100% will flip-flop from 25% to 100%: ... -> 80 -> 85 -> 90 -> 100 -> 25 -> 100 -> 25 -> ...
And same thing when decrementing below 20%.

Start with brightness at 100%

# cat /sys/class/backlight/acpi_video0/actual_brightness
0
# dmesg | tail
<...>
[  893.870775] Rui: current level is 20, brightness count is 18
# echo 6 > /sys/class/backlight/acpi_video0/brightness
/* brightness goes to 50% */
# cat /sys/class/backlight/acpi_video0/actual_brightness
9
# dmesg | tail
<...>
[  893.870775] Rui: current level is 20, brightness count is 18
[  929.443643] Rui: current level is 65, brightness count is 18

/* Note: according to my eyes and /proc/acpi/video/VGA/LCDD/brightness , brightness is now 50%, not 65% */

(In reply to comment #25)
> (In reply to comment #23)
> > Created an attachment (id=19443) [details] [details]
> > customized DSDT: fix another BIOS bug
> > 
> > _BQC should return the current backlight levels rather than an index in the
> > PCTG package.
> 
> With this DSDT, incrementing brightness above 100% will flip-flop from 25% to
> 100%: ... -> 80 -> 85 -> 90 -> 100 -> 25 -> 100 -> 25 -> ...
> And same thing when decrementing below 20%.
weird, please apply the debug patch I attached below
and attach the dmesg output after doing the same test, and you'd better add some comments in the dmesg like this:

...
...
pressing the hotkey
...
...
cat /proc/acpi/video/VGA/LCDD/brightness
...
...
pressing the hotkey
...
...

> 
> Start with brightness at 100%
> 
> # cat /sys/class/backlight/acpi_video0/actual_brightness
> 0
echo 0 > /sys/class/backlight/acpi_video0/brightness at this time,
and cat /sys/class/backlight/acpi_video0/actual_brightness
what does dmesg say?
BTW: do remember to apply the patch in comment #3 when doing this test.

> # dmesg | tail
> <...>
> [  893.870775] Rui: current level is 20, brightness count is 18
> # echo 6 > /sys/class/backlight/acpi_video0/brightness
> /* brightness goes to 50% */
> # cat /sys/class/backlight/acpi_video0/actual_brightness
> 9
> # dmesg | tail
> <...>
> [  893.870775] Rui: current level is 20, brightness count is 18
> [  929.443643] Rui: current level is 65, brightness count is 18
> 
> /* Note: according to my eyes and /proc/acpi/video/VGA/LCDD/brightness ,
> brightness is now 50%, not 65% */
> 
hmm, what does "/proc/acpi/video/VGA/LCDD/brightness" say in this case?
there should be something like "current: 65" I think.

Created attachment 19447 [details]
patch: print more debug info when doing brightness switch

Start with brightness at 85%

# cat /proc/acpi/video/VGA/LCDD/brightness
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 85

/* pressing hotkey brightness up */
# dmesg | tail
< ... >
[ 6013.679783] Rui: event 134
[ 6013.680396] Rui: level_current 30
[ 6013.680402] Rui: level_next 35
# cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 90

/* pressing hotkey brightness up*/
# dmesg | tail
< ... >
[ 6065.811989] Rui: event 134
[ 6065.813037] Rui: level_current 25
[ 6065.813042] Rui: level_next 30
# cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 100

/* pressing hotkey brightness up*/
# dmesg | tail
< ... >
[ 6155.036221] Rui: event 134
[ 6155.037360] Rui: level_current 20
[ 6155.037365] Rui: level_next 25
 # cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 25

/* pressing hotkey brightness up*/
# dmesg | tail
< ... >
[ 6185.974504] Rui: event 134
[ 6185.974817] Rui: level_current 90
[ 6185.974822] Rui: level_next 100
# cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 100

/* pressing hotkey brightness up*/
# dmesg | tail
< ... >
[ 6213.559333] Rui: event 134
[ 6213.560350] Rui: level_current 20
[ 6213.560356] Rui: level_next 25
# cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 25

Start with brightness at 100%

# cat /sys/class/backlight/acpi_video0/actual_brightness
0
# dmesg | tail
< ... >
[ 6408.058894] Rui: current level is 20, brightness count is 18

# echo 0 > /sys/class/backlight/acpi_video0/brightness
/* brightness goes to 20% */
# cat /sys/class/backlight/acpi_video0/actual_brightness
15
# dmesg | tail
< ... >
[ 6483.150674] Rui: current level is 100, brightness count is 18
# cat /proc/acpi/video/VGA/LCDD/brightness
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 20

# echo 6 > /sys/class/backlight/acpi_video0/brightness
/* brightness goes to 50% */
# cat /sys/class/backlight/acpi_video0/actual_brightness
9
# dmesg | tail
<...>
[ 6606.323817] Rui: current level is 65, brightness count is 18
# cat /proc/acpi/video/VGA/LCDD/brightness
levels:  100 90 20 25 30 35 40 45 50 55 60 65 70 75 80 85 90 100
current: 50

Created attachment 19448 [details]
customized DSDT

well, fix another bug in the DSDT.
if it still doesn't work perfectly, please attach the test results just like the ones in the above two comments. :p

(In reply to comment #30)
> Created an attachment (id=19448) [details]
> customized DSDT
> 
> well, fix another bug in the DSDT.
> if it still doesn't work perfectly, please attach the test results just like
> the ones in the above two comments. :p

Thank you very much, this works perfectly! Does not roll over when incrementing, reports correct brightness values in dmesg messages, reports correct information in /sys/class/backlight/acpi_video0/actual_brightness

Could you please post the source for the DSDT?

Also, what do you suggest should be the actual solution to this bug? Contacting Lenovo and telling them to update their BIOS?

Created attachment 19451 [details]
source code of the customized DSDT

(In reply to comment #31)
> 
> Thank you very much, this works perfectly!
hah, good news. :p

> Also, what do you suggest should be the actual solution to this bug?
> Contacting
> Lenovo and telling them to update their BIOS?
> 
exactly.
there are several BIOS problem to me.
1. incorrect _BCL package.
2. wrong value returned by _BQC

Created attachment 19671 [details]
patch: don't use buggy _BCL/_BCM/_BQC methods for backlight control

Alexandre, would you please:
apply this patch on a vanilla kernel, without the customized DSDT and see if the backlight control works? BTW: make sure the thinkpad_acpi driver is built and loaded.

*** Bug 12037 has been marked as a duplicate of this bug. ***

re-open this bug because using the customized DSDT is not what we want to fix this issue.
Anyone who has the same problem please try the patch in comment #34.

*** Bug 12235 has been marked as a duplicate of this bug. ***

We have a couple of bugs which are related to this upstream bug, https://bugs.launchpad.net/ubuntu/+source/linux/+bug/311716 and https://bugs.launchpad.net/ubuntu/+source/linux/+bug/301524.  Testing with the patch in comment #34 has been reported to fix those bugs.  This patch seems sound and help in a lot of cases.  Recommend this for mainline.

Created attachment 19717 [details]
ACPI VIDEO: Setup correct brightness level list sorted and without duplicates

Could you try this patch, pls.
Rui could you give it a review, could this work?

We try to figure out to still make use of the generic video functions. This would be the preferred fix.

Rui: Thanks for the real work, finding out the root cause of all the brightness mess and collecting and parsing all the BIOS data...

well, this can fix the bug in bug 12302,
but can NOT fix the bug 12037 and 12249.
because the current brightness returned by the _BQC method is an index in the _BCL method rather than the value.
you can see the discussion between Matthew and me in the ML.

reopen this bug as we don't have a upstream candidate patch yet.

plus, the level_ac and level_battery are not always the maximum and the second maximum backlight levels.
we should set them to a specific value only if they are not exported by _BCL method.

Btw, I'd like to do a cleanup in video.c and refresh this patch on top of that.

shipped in 2.6.28-git14:

commit 935e5f290ec1eb0f1c15004421f5fd3154380fd5
Author: Zhang Rui <rui.zhang@intel.com>
Date:   Thu Dec 11 16:24:52 2008 -0500

    ACPI: video: Fix reversed brightness behavior on ThinkPad SL series
    
    Section B.6.2 of ACPI 3.0b specification that defines _BCL method
    doesn't require the brightness levels returned to be sorted.
    At least ThinkPad SL300 (and probably all IdeaPads) returns the
    array reversed (i.e. bightest levels have lowest indexes), which
    causes the brightness management behave in completely reversed
    manner on these machines (brightness increases when the laptop is
    idle, while the display dims when used).
    
    Sorting the array by brightness level values after reading the list
    fixes the issue.
    
    http://bugzilla.kernel.org/show_bug.cgi?id=12037
    

(In reply to comment #34)
> Created an attachment (id=19671) [details]
> patch: don't use buggy _BCL/_BCM/_BQC methods for backlight control
> 
> Alexandre, would you please:
> apply this patch on a vanilla kernel, without the customized DSDT and see if
> the backlight control works? BTW: make sure the thinkpad_acpi driver is built
> and loaded.

Applied patch to 2.6.28. Does not work. Backlight controls no longer have any effect at all. /proc/acpi/video/VGA/LCDD/brightness says "<not supported>"

(In reply to comment #34)
> BTW: make sure the thinkpad_acpi driver is built
> and loaded.

Ah, now I see. So your patch simply disables video.ko from controlling the backlight on laptops with buggy BIOS.

Then it's working as intended :)

Unfortunately, thinkpad-acpi is not compatible with the SL series Thinkpads (their BIOS is a strange hybrid of ThinkPad and IdeaPad), so I guess I will need to add backlight controls to the SL series hotkey driver I am intermittently writing...

Thanks a lot. In 2.6.29rc1 kernel backligth work perfect!!!

Asus X51L.

so there is an empty /sys/class/backlight directory after applying this patch, even with thinkpad_acpi loaded, right?

(In reply to comment #46)
> so there is an empty /sys/class/backlight directory after applying this
> patch,
> even with thinkpad_acpi loaded, right?

When patch from comment 34 is applied, there is an empty /sys/class/backlight directory *without* thinkpad_acpi loaded.

I have no idea what happens when thinkpad_acpi is loaded because thinkpad_acpi literally cannot be loaded on a thinkpad SL300 (and for a good reason - the SL series has a very different BIOS from other thinkpads).

Trying to get brightness working.  This custom DSDT crashes my kernel. I have an SL300 with 2Gb and p8400 and ubuntu jaunty.

Is it possible to get functionality of the brightness keys on SL300 by blacklisting video?  I tried it, but I can't use my brightness keys b/c kernel ACPI grabs them.  Is there a way to give back control of the keys to the BIOS without acpi=off? (doesn't boot)  I thought of this idea when a user reported that an old version of ubuntu doesn't have this problem, meaning old kernel and old acpi, 2.6.24 i think.

I prefer to be able to use my keys.  I don't care so much if linux userland can see the interface.

I wrote a driver for controlling the brightness, hotkeys and bluetooth on the SL series ThinkPads.

See http://github.com/tetromino/lenovo-sl-laptop/tree/master

Works nicely with the patch in comment #34

thanks so much! your driver works.  I think it might also be helpful for ideapads as there's similar bugs floating around on ubuntu launchpad for them and I know the firmwares are similar.

Alexandre: Looking at your driver, it looks like it's mostly doing the same than the video.c driver (from brightness perspective), but uses the return value of _BQC (brightness current) as an index of the _BCL (brightness control levels) returned array instead of values in percent.
I wonder whether we could sanity check and handle this in the video driver.
Possibly with dmi matching and go for the index based solution then.
While all other vendor specific brightness drivers have an own interface, this one makes use of the generic ACPI brightness functions and would interfere with the generic video driver in not-solvable way (maybe dmi blacklisting again, but then it's still better to do all the _BQC, _BCL, _BCM processing in the video driver).
I try to get some info and come back in some days. Any comments about the video.c only brightness solution are very welcome.

yes, it would be great if ACPI video driver can handle both indexes and values returned by _BQC, as Matthew pointed out.
I can do it, and in fact, I also have a couple of ACPI video cleanups in hand.
But I'm afraid I can not start until next week.

(In reply to comment #52)
Thomas: on the ThinkPad SL series, both the _BCL and _BQC methods are non-standard (see comments in my code and in the discussion of this bug).

As for a video.c only solution, I agree it would be nice, but I am not sure if it's necessary because in 2.6.28 and higher, there is a good mechanism to prevent video.c and platform drivers from stepping on each others' toes: acpi_video_backlight_support() and related functions, implemented in video_detect.c

You can use it manually with the new kernel parameter acpi_backlight; if acpi_backlight=vendor, then video.c will refrain from touching the backlight, leaving platform drivers to do it.

video_detect.c can also automatically blacklist certain platforms for video.c; Zhang Rui's patch (comment #34) would make it automatically blacklist backlight functionality in laptops with broken _BQC and _BCM methods.

for a BIOS which provides both ACPI video extension and platform specific control methods, ACPI video is preferred.
the patch in comment #34 is a simple way to workaround this issue.
but it's still good to make ACPI video work well on all the laptops with _BCM/_BCL/_BQC methods, even the buggy ones.

But on one Asus laptop(bug12450) the _BCL package is defined as the following:
   > Method (_BCL, 0, NotSerialized)
     {
       Return (Package (0x10)
      {
       0x0F,    0x0E,   0x0D,    0x0C,
       0x0B,    0x0A,   0x09,    0x08,
       0x07,    0x06,   0x05,    0x04,
       0x03,    0x02,   One,     Zero    })
       }

    The index field is returned by the _BQC object.
    In such case how to distinguish the index or the true brightness value?
    Thanks.

1. maximum value doesn't equal 100
2. the brightness levels are sequential numbers start from 0.

Argh, also the ASUS have this?
Then this really should not be in a SL Lenovo driver.
This also makes it very likely that Windows can handle this in a generic way, I doubt ASUS and Lenovo developed a Windows brightness driver together.

> I can do it, and in fact, I also have a couple of ACPI video cleanups in
> hand.
> But I'm afraid I can not start until next week. 
Rui: This would be very much appreciated.
It would be great if you CC me when you post your outcomes or if I should have a look at them. But I do not have such a machine, I hope the guys in this bug can help you out a bit with testing.

> 1. maximum value doesn't equal 100
Good idea, this should work in all cases, unless we hit the next level of brokeness.
> 2. the brightness levels are sequential numbers start from 0.
Not sure whether this is always the case? 1. should be enough?
Another criteria could be: highest value must be lower than amount of brightness levels?

> video_detect.c can also automatically blacklist certain platforms for video.c
Yeah, but this list should be as small as possible. It's also mainly thought for
ACPI vs native drivers, when there are cases where the native ones behaves better. But on your machines it looks like the generic driver can be tweaked
to make things work. We should at least try hard to get it working and only blacklist if we do not see a way to do it generically (otherwise every not yet blacklisted machine will not work if you rely on blacklisting).

When applying the patch from comment #34 to 2.6.28.4 the brightness levels of my Asus B50A laptop work correctly for the first time. 

Without the patch Gnome offers only two (quite unusable) levels.

Created attachment 20479 [details]
patch: video cleanup

please try this patch and see if the problem is fixed.
please attach the content of /proc/acpi/video/*/*/brightness with this patch applied as well.

I'm a bit short on time right now, so I tried to go about the quick and easy way, and compile the ubuntu kernel with this patch applied. The patch applies fine, but there are problems building the kernel

Normally, I don't bother mentioning anything on a bug report here if I've used the ubuntu kernel. But in case it yields anything useful, this is the error.

ERROR: "acpi_ut_warning" [drivers/acpi/video.ko] undefined!
ERROR: "acpi_ut_error" [drivers/acpi/video.ko] undefined!

(more output here: http://paste.ubuntu.com/129215/)

I'll try building a vanilla kernel with this patch applied as soon as I can if I find the time.

I applied the patch from comment #60 against 2.6.28.7 and got exactly the same errors.

Peter, your laptop may be another problem,
please attach the acpidump output, dmesg output after ACPI video driver loaded, and tell me what the laptop's model name is.

Created attachment 20493 [details]
Kernel 2.6.29-rc7 dmesg output

The laptop is an Asus B50A.

Created attachment 20494 [details]
acpidump output of Asus B50A

please attach the output of "grep . /sys/class/backlight/*/*"

both w/ and w/o the patch in comment #60

2.6.29-rc7 (no patches applied):

root@asus:~# grep . /sys/class/backlight/*/*
/sys/class/backlight/acpi_video0/actual_brightness:0
/sys/class/backlight/acpi_video0/bl_power:0
/sys/class/backlight/acpi_video0/brightness:13
/sys/class/backlight/acpi_video0/max_brightness:13

2.6.28.7 (patch from comment #34 applied):

root@asus:~# grep . /sys/class/backlight/*/*
/sys/class/backlight/asus-laptop/actual_brightness:15
/sys/class/backlight/asus-laptop/bl_power:0
/sys/class/backlight/asus-laptop/brightness:15
/sys/class/backlight/asus-laptop/max_brightness:15

2.6.28.7 did not compile when applying the patch from comment #60. I will try applying this patch to 2.6.29-rc7.

Created attachment 20500 [details]
patch: video cleanup

please try this updated patch instead of the previous one.

2.6.29-rc7 (patch from comment #60 applied):

root@asus:~# grep . /sys/class/backlight/*/*
/sys/class/backlight/acpi_video0/actual_brightness:0
/sys/class/backlight/acpi_video0/bl_power:0
/sys/class/backlight/acpi_video0/brightness:15
/sys/class/backlight/acpi_video0/max_brightness:15

The initial brightness in Gnome is usable. However stepping through brightness levels does not work correctly. Setting the brightness levels through /proc gives the correct brightness (10..lowest, 100..highest, total of 16 levels).

root@asus:~# cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
current: 100

In comparison to 2.6.29-rc7 without any patches applied (14 brightness levels)

root@asus:~$ cat /proc/acpi/video/VGA/LCDD/brightness 
levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87
current: 87

2.6.29-rc7 (patch from comment #69 applied):

Directory /sys/class/backlight is empty.

root@asus:~# cat /proc/acpi/video/VGA/LCDD/brightness 
<not supported>

Backlight control is completely gone.

Peter, please attach the dmesg output with the kernel in comment #71. thanks

Created attachment 20513 [details]
Kernel 2.6.27-rc7 dmesg output 

Patch from comment #69 applied. See results in comment #71.

Sorry, I meant 2.6.29-rc7, not 2.6.27-rc7.

Created attachment 20536 [details]
debug patch

please apply this debug patch on top of the previous patch and attach the dmesg output after boot.

Created attachment 20553 [details]
dmesg output (2.6.29-rc7 with debug patch applied)

Patches from comment #69 and comment #75 applied

Created attachment 20559 [details]
customized DSDT

please
1.set CONFIG_ACPI_DEBUG,
2.apply this customized DSDT
3.rebuild
4.boot with "acpi.debug_level=0x07"
5.attach the dmesg output after boot.

Created attachment 20572 [details]
2.6.29-rc7 dmesg output (DSDT + both patches)

Created attachment 20573 [details]
2.6.29-rc7 dmesg output (only DSDT applied)

weird, I didn't get the info I want.
could you please redo the test in comment #78 and attach the output of "cat /sys/module/acpi/parameters/debug_level" after you get the dmesg.

Created attachment 20574 [details]
customized DSDT

please use this DSDT instead. :)

patch already available.
http://patchwork.kernel.org/patch/12770/
http://patchwork.kernel.org/patch/12771/
http://patchwork.kernel.org/patch/12772/
http://patchwork.kernel.org/patch/12773/
http://patchwork.kernel.org/patch/12774/
http://patchwork.kernel.org/patch/12775/

Created attachment 20585 [details]
2.6.29-rc7 dmesg output (DSDT (comment #81) + both patches)

cat /sys/module/acpi/parameters/debug_level reports:

debug_level = 0x00000007

9be1df98bca44dbe3769cd22f4ab8122b76c5313
http://patchwork.kernel.org/patch/1309/
(bd->props.brightness doesn't reflect the actual backlight level.)
shipped in Linux-2.6.29-rc1

Created attachment 20728 [details]
patch:video cleanup 7

please attach the patch on top of the previous patches and see if it helps.

Zhang: There are so many patches attached to this bug report, I'm a bit at a loss which ones to try.

So far, I've tried some patches from here a few times, the last time I tried all patches from comment #82. As of yet, I've had no success in having a properly working backlight control.

Since I haven't been sure I'm using the correct patches, I haven't given any feedback. So, could you please give a list of all patches that one should try at this point? Thanks.

AFAIK most of Rui's patches went into Len's test tree:
git clone git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6 linux-acpi
git branch --track origin/test test
git checkout test
should give you that one with the latest brightness patches from Rui that should go into 2.6.30.
Then adding additional ones (and testing others) should be easier.
Rui: I did not made it adding them to our latest tree. I better wait, if they are in Len's tree we get them in .30 soon anyway and I make affected people test on .30 based kernel then.

Created attachment 20747 [details]
dmesg output (2.6.29-rc7 with DSDT and all 3 patches applied)

Backlight control is back again.

root@asus:~# grep . /sys/class/backlight/*/*
/sys/class/backlight/acpi_video0/actual_brightness:0
/sys/class/backlight/acpi_video0/bl_power:0
/sys/class/backlight/acpi_video0/brightness:15
/sys/class/backlight/acpi_video0/max_brightness:15

root@asus:~# cat /proc/acpi/video/VGA/LCDD/brightness
levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
current: 15

Gnome offers two backlight levels:

dark: /sys/class/backlight/acpi_video0/brightness:1
bright: /sys/class/backlight/acpi_video0/brightness:14

This seems to result in maximum brightness:
echo 100 > /proc/acpi/video/VGA/LCDD/brightness

-> /sys/class/backlight/acpi_video0/brightness:15

Interestingly I obtain an error when trying to set the brightness to 10:

root@asus:~# echo 10 > /proc/acpi/video/VGA/LCDD/brightness
-su: echo: write error: Invalid argument

Hi, Peter,

thanks for your test, it seems that there are still two problems here.

1.
root@asus:~# cat /proc/acpi/video/VGA/LCDD/brightness
levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
current: 15
here 15 is an index value which I failed to translate it to the percentage value.

2.
root@asus:~# echo 10 > /proc/acpi/video/VGA/LCDD/brightness
this is not clear yet. But may be the same problem as the above one.

Patch will be attached later.

thanks,
rui

Created attachment 20749 [details]
patch: video cleanup -8

hi, Peter,

please get a clean 2.6.29 kernel,
1.apply the customized DSDT in comment #81,
2.apply the patch set in comment #82,
3.apply the patch in comment #85.
4.apply the patch in comment #90.
5.set CONFIG_ACPI_DEBUG and rebuild.
6.boot with acpi.debug_layer=0xffffffff acpi.debug_level=0x2007
7.attach the dmesg output after boot.

> root@asus:~# echo 10 > /proc/acpi/video/VGA/LCDD/brightness
> -su: echo: write error: Invalid argument

this only happens when setting the brightness to the minimum value, right?

Yes, only value 10 shows this error. All other values (16-100) work correctly.

Created attachment 20765 [details]
/var/log/messages output as requested in comment #91

I had to take /var/log/messages instead of dmesg since the startup output exceeded the ring buffer size.

this seems to be another BIOS/AML code problem.
when set the backlight to 10 (the minimum brightness value),
there is a failure when evaluating STBR method.
please do this test in the comment #91 kernel,
1. echo 0xffffffff > /sys/module/acpi/parameters/trace_debug_layer
2. echo 0xffffffff > /sys/module/acpi/parameters/trace_debug_level
3. echo STBR > /sys/module/acpi/parameters/trace_method_name
4. echo 1 > /sys/module/acpi/parameters/trace_state
5. echo 0 >/sys/class/backlight/acpi_video0/brightness
5. echo 10 >/sys/class/backlight/acpi_video0/brightness
6. attach the dmesg output

Created attachment 20775 [details]
Difference in dmesg output

I executed this script and then diffed both dmesg files:

dmesg > dmesg-$(uname -r).before.txt
echo 0xffffffff > /sys/module/acpi/parameters/trace_debug_layer
echo 0xffffffff > /sys/module/acpi/parameters/trace_debug_level
echo STBR > /sys/module/acpi/parameters/trace_method_name
echo 1 > /sys/module/acpi/parameters/trace_state
echo 0 >/sys/class/backlight/acpi_video0/brightness
echo 10 >/sys/class/backlight/acpi_video0/brightness
dmesg > dmesg-$(uname -r).after.txt

weird, the dmesg doesn't show the info I need.
please retry this test, but
1. don't use boot options "acpi.debug_layer=0xffffffff acpi.debug_level=0x2007"
2. echo enable > /sys/module/acpi/parameters/trace_state instead of "echo 1 > ..."

Created attachment 20776 [details]
customized DSDT

BTW: please use this customized DSDT instead.

Do these lines look better?

[  134.721751] ACPI Exception (exoparg2-0445): AE_AML_BUFFER_LIMIT, Index (0000000FF) is beyond end of object [20081204]
[  134.721770] ACPI Error (psparse-0537): Method parse/execution failed [\_SB_.PCI0.SBRG.EC0_.STBR] (Node ffff88013fa36ac0), AE_AML_BUFFER_LIMIT
[  134.721931] ACPI Error (psparse-0537): Method parse/execution failed [\_SB_.PCI0.VGA_.LCDD._BCM] (Node ffff88013fa30c60), AE_AML_BUFFER_LIMIT
[  134.722096] ACPI Error (video-0505): Evaluating _BCM failed [20081204]

no, :(
please attach the full dmesg output with boot parameters "acpi.debug_layer=0xffffffff acpi.debug_level=0x2007"

Created attachment 20786 [details]
/var/log/messages output as requested in comment #99

Again the dmesg output exceeds the ring buffer size, so I had to attach /var/log/messages.

For some unknown reason /var/log/messages seems to be incomplete as well. The usual startup message of the kernel including its version number and compiler is missing.

Output starts at time index 1.729114 instead of 0.000000.

Hopefully all relevant ACPI debug output is contained.

Created attachment 20787 [details]
customized DSDT

I think I found the problem.
It's still a AML code problem, please apply this customized DSDT and attach the dmesg output like the previous one.

> Again the dmesg output exceeds the ring buffer size...
Yes, that happens easily with increased ACPI debug output, you may want to use:
log_buf_len=
boot param. E.g. log_buf_len=16777216 increases the ring buffer to 16MB.

Created attachment 20797 [details]
dmesg output as requested in comment #101

Finally I can provide an untruncated dmesg output (thanks Thomas for suggesting to use log_buf_len).

Created attachment 20844 [details]
debug patch

please apply this patch on top and re-attach the dmesg output. thanks.

commit d32f69470c2081ffdfd82740ac19f940790f9e93
Author: Zhang Rui <rui.zhang@intel.com>
Date:   Wed Mar 18 16:27:12 2009 +0800

    ACPI video: support _BCL packages that don't export brightness levels when machine is on AC/Battery


shipped in 2.6.30 merge window (2.6.29-git14)

Created attachment 20850 [details]
dmesg output as requested in comment #104

I switched to kernel 2.6.29.1.

From the dmesg output in comment #106,
it seems that the ACPI brightness control is working perfectly on your box, doesn't it?

From a kernel point of view this may be true. However Gnome shows quite erratic behavior.

"echo 10 > /proc/acpi/video/VGA/LCD/brightness" no longer shows an error message which is definitely better than before.

Iterating through all values in /proc/acpi/video/VGA/LCD/brightness works perfectly and changes the brightness as expected (10=lowest, 100=highest).

This scenario however shows odd behavior:

* Set maximum brightness: echo 100 > /proc/acpi/video/VGA/LCD/brightness
  * root@asus:/proc/acpi/video/VGA/LCDD# cat brightness 
    levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
    current: 100
  * root@asus:/proc/acpi/video/VGA/LCDD# grep . /sys/class/backlight/*/*
    /sys/class/backlight/acpi_video0/actual_brightness:0
    /sys/class/backlight/acpi_video0/bl_power:0
    /sys/class/backlight/acpi_video0/brightness:15
    /sys/class/backlight/acpi_video0/max_brightness:15

* Reducing the brightness level by one using the corresponding brightness key
  * The brightness stays very high (I would assume 93)
  * root@asus:/proc/acpi/video/VGA/LCDD# cat brightness 
    levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
    current: 16
  * root@asus:/proc/acpi/video/VGA/LCDD# grep . /sys/class/backlight/*/*
    /sys/class/backlight/acpi_video0/actual_brightness:1
    /sys/class/backlight/acpi_video0/bl_power:0
    /sys/class/backlight/acpi_video0/brightness:14
    /sys/class/backlight/acpi_video0/max_brightness:15

* Pressing the brightness key a few more times does not change anything, but then abruptly minimum brightness is set
  * root@asus:/proc/acpi/video/VGA/LCDD# cat brightness 
    levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
    current: 10
  * root@asus:/proc/acpi/video/VGA/LCDD# grep . /sys/class/backlight/*/*
    /sys/class/backlight/acpi_video0/actual_brightness:15
    /sys/class/backlight/acpi_video0/bl_power:0
    /sys/class/backlight/acpi_video0/brightness:0
    /sys/class/backlight/acpi_video0/max_brightness:15

Do these values look sane to you?

It seems strange that manually executing "echo 16 > /proc/acpi/video/VGA/LCD/brightness" results in a dark screen whereas setting brightness with Gnome results in a very bright screen.

In both cases /proc/acpi/video/VGA/LCD/brightness contains "current: 16".

Is /proc/acpi/video/VGA/LCD/brightness a reliable source for the current brightness value or should /sys/class/backlight/acpi_video0/brightness be used instead?

Created attachment 20874 [details]
debug patch v2

well. this is another BIOS problem, sigh...
please apply this debug patch instead of the previous one and see if it helps.

The final patch from comment #109 made it work perfectly!

Thanks a lot Zhang for all your efforts!

root@asus:/proc/acpi/video/VGA/LCDD# cat brightness 
levels:  10 16 21 27 33 39 45 51 57 63 69 74 81 87 93 100
current: 100

Using "echo xx > brightness" works for all listed values.

Using the brightness keys in Gnome gives this mapping between values in /proc/acpi/video/VGA/LCDD/brightness and /sys/class/backlight/acpi_video0/brightness.

100/15, 87/13, 74/11, 63/9, 51/7, 39/5, 27/3, 16/1, 10/0

The value of /sys/class/backlight/acpi_video0/brightness always matches the value of /sys/class/backlight/acpi_video0/actual_brightness.

The visible screen brightness matches this values in all cases. Brightness can now smoothly be changed from low to high using the brightness keys.

This leads to two more questions:

* When will these patches go into mainline (I hope they will)?
* Do I always need the customized DSDT or was it only necessary for debugging purposes?

Created attachment 20894 [details]
dmesg output corresponding to patch in comment #109

1. When will these patches go into mainline (I hope they will)?
Bad news is that all of them are going to upsteam except the last one.
Good news is that I can rewrite a DSDT that can work for you w/o this patch. :)

2. Do I always need the customized DSDT
yes, there are several AML code errors that can not be fixed by kernel code.

close the bug as patches are already available.

patches above shipping in linux-2.6.30-rc3-git4
please re-open if they don't work,
or open a new bug if there are any new problems.

closed.