Bug 12249 - Lenovo ThinkPad SL series: LCD brightness works in procfs, buggy in sysfs
Summary: Lenovo ThinkPad SL series: LCD brightness works in procfs, buggy in sysfs
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Zhang Rui
URL:
Keywords:
: 12037 12235 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-12-18 12:47 UTC by Alexandre Rostovtsev
Modified: 2009-04-28 15:52 UTC (History)
9 users (show)

See Also:
Kernel Version: 2.6.28-rc8
Subsystem:
Regression: No
Bisected commit-id:


Attachments
original acpidump (417.93 KB, text/plain)
2008-12-18 13:12 UTC, Alexandre Rostovtsev
Details
customized DSDT (444.19 KB, text/plain)
2008-12-18 13:19 UTC, Alexandre Rostovtsev
Details
patch: always update the props.brightness (695 bytes, patch)
2008-12-18 18:09 UTC, Zhang Rui
Details | Diff
dmesg output (68.14 KB, text/plain)
2008-12-18 19:52 UTC, Alexandre Rostovtsev
Details
customized DSDT (444.13 KB, application/octet-stream)
2008-12-18 21:41 UTC, Zhang Rui
Details
source code of the customized DSDT (377.33 KB, text/plain)
2008-12-18 23:35 UTC, Zhang Rui
Details
video debug patch (872 bytes, patch)
2008-12-18 23:55 UTC, Zhang Rui
Details | Diff
customized DSDT (444.13 KB, application/octet-stream)
2008-12-21 18:43 UTC, Zhang Rui
Details
customized DSDT -- source code (377.33 KB, text/plain)
2008-12-21 18:44 UTC, Zhang Rui
Details
customized DSDT (444.86 KB, application/octet-stream)
2008-12-22 19:50 UTC, Zhang Rui
Details
source code of the customized DSDT (378.32 KB, text/plain)
2008-12-22 19:51 UTC, Zhang Rui
Details
customized DSDT: fix another BIOS bug (445.05 KB, application/octet-stream)
2008-12-22 22:00 UTC, Zhang Rui
Details
source code of the customized DSDT (378.40 KB, text/plain)
2008-12-22 22:01 UTC, Zhang Rui
Details
patch: print more debug info when doing brightness switch (1.37 KB, patch)
2008-12-22 23:40 UTC, Zhang Rui
Details | Diff
customized DSDT (445.13 KB, application/octet-stream)
2008-12-23 00:19 UTC, Zhang Rui
Details
source code of the customized DSDT (378.46 KB, text/plain)
2008-12-23 00:47 UTC, Zhang Rui
Details
patch: don't use buggy _BCL/_BCM/_BQC methods for backlight control (3.12 KB, patch)
2009-01-05 22:32 UTC, Zhang Rui
Details | Diff
ACPI VIDEO: Setup correct brightness level list sorted and without duplicates (3.27 KB, patch)
2009-01-08 07:24 UTC, Thomas Renninger
Details | Diff
patch: video cleanup (10.21 KB, patch)
2009-03-09 23:08 UTC, Zhang Rui
Details | Diff
Kernel 2.6.29-rc7 dmesg output (51.17 KB, text/plain)
2009-03-11 10:13 UTC, Peter Klotz
Details
acpidump output of Asus B50A (451.67 KB, application/octet-stream)
2009-03-11 10:14 UTC, Peter Klotz
Details
patch: video cleanup (13.36 KB, patch)
2009-03-12 01:39 UTC, Zhang Rui
Details | Diff
Kernel 2.6.27-rc7 dmesg output (51.80 KB, text/plain)
2009-03-12 23:00 UTC, Peter Klotz
Details
debug patch (666 bytes, patch)
2009-03-15 19:48 UTC, Zhang Rui
Details | Diff
dmesg output (2.6.29-rc7 with debug patch applied) (52.31 KB, text/plain)
2009-03-16 10:20 UTC, Peter Klotz
Details
customized DSDT (509.93 KB, application/octet-stream)
2009-03-16 20:49 UTC, Zhang Rui
Details
2.6.29-rc7 dmesg output (DSDT + both patches) (52.69 KB, text/plain)
2009-03-17 14:41 UTC, Peter Klotz
Details
2.6.29-rc7 dmesg output (only DSDT applied) (51.50 KB, text/plain)
2009-03-17 14:42 UTC, Peter Klotz
Details
customized DSDT (509.87 KB, application/octet-stream)
2009-03-17 18:07 UTC, Zhang Rui
Details
2.6.29-rc7 dmesg output (DSDT (comment #81) + both patches) (52.20 KB, text/plain)
2009-03-18 13:36 UTC, Peter Klotz
Details
patch:video cleanup 7 (2.96 KB, patch)
2009-03-30 03:18 UTC, Zhang Rui
Details | Diff
dmesg output (2.6.29-rc7 with DSDT and all 3 patches applied) (54.52 KB, text/plain)
2009-03-30 20:40 UTC, Peter Klotz
Details
patch: video cleanup -8 (2.56 KB, patch)
2009-03-31 02:56 UTC, Zhang Rui
Details | Diff
/var/log/messages output as requested in comment #91 (353.80 KB, text/plain)
2009-03-31 20:08 UTC, Peter Klotz
Details
Difference in dmesg output (11.04 KB, text/plain)
2009-04-02 05:20 UTC, Peter Klotz
Details
customized DSDT (510.10 KB, application/octet-stream)
2009-04-02 05:45 UTC, Zhang Rui
Details
/var/log/messages output as requested in comment #99 (249.95 KB, text/plain)
2009-04-03 05:24 UTC, Peter Klotz
Details
customized DSDT (510.18 KB, application/octet-stream)
2009-04-03 06:10 UTC, Zhang Rui
Details
dmesg output as requested in comment #101 (965.98 KB, text/plain)
2009-04-03 19:48 UTC, Peter Klotz
Details
debug patch (1.12 KB, patch)
2009-04-07 02:57 UTC, Zhang Rui
Details | Diff
dmesg output as requested in comment #104 (963.70 KB, text/plain)
2009-04-07 06:00 UTC, Peter Klotz
Details
debug patch v2 (1.62 KB, patch)
2009-04-08 05:49 UTC, Zhang Rui
Details | Diff
dmesg output corresponding to patch in comment #109 (861.95 KB, text/plain)
2009-04-09 06:37 UTC, Peter Klotz
Details

Description Alexandre Rostovtsev 2008-12-18 12:47:14 UTC
Latest working kernel version: unknown
Earliest failing kernel version: 2.6.27
Distribution: Gentoo
Hardware Environment: Lenovo ThinkPad SL300, Intel G45 video
Software Environment:
Problem Description: Reversed order of brightness levels is fixed by applying patch and DSDT in bug #12037 but several more problems with LCD brightness remain.

Problem 1: brightness controls in /proc and /sys are inconsistent.

First, /sys/class/backlight/acpi_video0/actual_brightness always reads 0, no matter what the actual brightness is. /sys/class/backlight/acpi_video0/brightness *sometimes* displays the correct brightness value. /proc/acpi/video/VGA/LCDD/brightness is always correct.

Second, setting the brightness via procfs always works (but the change is never reflected in sysfs). Setting the brightness in sysfs to any value different from what sysfs thinks it currently is always works (and is immediately reflected in procfs). Setting the brightness in sysfs to a value equal to what it thinks the brightness is (but different from what it actually is) has no effect.

Here is a sequence of commands to illustrate this issue:


# # Start: LCD is at 100% brightness
# cat /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
current: 100
# cat /sys/class/backlight/acpi_video0/brightness 
15
# # 15 is the max increment, 0 is lowest

# echo 50 > /proc/acpi/video/VGA/LCDD/brightness
# # LCD is at 50% brightness
# cat /proc/acpi/video/VGA/LCDD/brightness | grep current
current: 50
# cat /sys/class/backlight/acpi_video0/brightness 
15
# # sysfs is lying

# echo 15 > /sys/class/backlight/acpi_video0/brightness
# # nothing happens
# cat /proc/acpi/video/VGA/LCDD/brightness | grep current
current: 50

# echo 70 > /proc/acpi/video/VGA/LCDD/brightness
# # screen goes to 70% brightness
# cat /proc/acpi/video/VGA/LCDD/brightness | grep current
current: 70
# cat /sys/class/backlight/acpi_video0/brightness 
15

# echo 3 > /sys/class/backlight/acpi_video0/brightness
# # screen goes to 35% brightness
# cat /proc/acpi/video/VGA/LCDD/brightness | grep current
current: 90
# cat /sys/class/backlight/acpi_video0/brightness 
3

# echo 15 > /sys/class/backlight/acpi_video0/brightness
# # screen goes back to 100% brightness
# cat /proc/acpi/video/VGA/LCDD/brightness | grep current
current: 100


Problem 2: in Gnome, incrementing brightness above the maximum (with the keyboard shortcut) results in 25% brightness. When the screen turns on after going off due to power saving, the brightness is also at 25%, no matter where it was originally. Using keyboard shortcuts, the only way to bring it back to 100% is to decrement brightness one step and then increment it one step. I suspect this is also caused by sysfs brightness file's buggy behavior.
Comment 1 Alexandre Rostovtsev 2008-12-18 13:12:56 UTC
Created attachment 19362 [details]
original acpidump

Acpidump output attached (SL300 BIOS version 1.16)
Comment 2 Alexandre Rostovtsev 2008-12-18 13:19:53 UTC
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.
Comment 3 Zhang Rui 2008-12-18 18:09:42 UTC
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.
Comment 4 Alexandre Rostovtsev 2008-12-18 19:07:36 UTC
(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...
Comment 5 Zhang Rui 2008-12-18 19:32:13 UTC
(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.
Comment 6 Alexandre Rostovtsev 2008-12-18 19:52:05 UTC
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).
Comment 7 Zhang Rui 2008-12-18 21:40:35 UTC
please attach the result of  "grep . /proc/acpi/video/*/*/brightness"
Comment 8 Zhang Rui 2008-12-18 21:41:56 UTC
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
Comment 9 Alexandre Rostovtsev 2008-12-18 22:51:04 UTC
(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
Comment 10 Alexandre Rostovtsev 2008-12-18 23:08:57 UTC
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...
Comment 11 Zhang Rui 2008-12-18 23:35:24 UTC
Created attachment 19371 [details]
source code of the customized DSDT

attached per Alexandre's request. :)
Comment 12 Zhang Rui 2008-12-18 23:55:29 UTC
Created attachment 19372 [details]
video debug patch

please apply this patch, and attach the dmesg output after running "cat actual_brightness"
Comment 13 Alexandre Rostovtsev 2008-12-19 00:06:12 UTC
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
Comment 14 Zhang Rui 2008-12-21 18:43:43 UTC
Created attachment 19411 [details]
customized DSDT

please try this DSDT instead. :)
Comment 15 Zhang Rui 2008-12-21 18:44:38 UTC
Created attachment 19412 [details]
customized DSDT -- source code
Comment 16 Alexandre Rostovtsev 2008-12-22 18:49:58 UTC
(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
Comment 17 Zhang Rui 2008-12-22 19:50:39 UTC
Created attachment 19441 [details]
customized DSDT

hah, this one should work for you. please give it a try.
Comment 18 Zhang Rui 2008-12-22 19:51:25 UTC
Created attachment 19442 [details]
source code of the customized DSDT
Comment 19 Alexandre Rostovtsev 2008-12-22 20:57:51 UTC
(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%).
Comment 20 Alexandre Rostovtsev 2008-12-22 21:20:56 UTC
(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.
Comment 21 Zhang Rui 2008-12-22 21:31:30 UTC
(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%).
> 
Comment 22 Alexandre Rostovtsev 2008-12-22 21:46:59 UTC
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
Comment 23 Zhang Rui 2008-12-22 22:00:18 UTC
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.
Comment 24 Zhang Rui 2008-12-22 22:01:10 UTC
Created attachment 19444 [details]
source code of the customized DSDT
Comment 25 Alexandre Rostovtsev 2008-12-22 22:32:09 UTC
(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% */
Comment 26 Zhang Rui 2008-12-22 23:40:09 UTC
(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.
Comment 27 Zhang Rui 2008-12-22 23:40:53 UTC
Created attachment 19447 [details]
patch: print more debug info when doing brightness switch
Comment 28 Alexandre Rostovtsev 2008-12-22 23:58:23 UTC
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
Comment 29 Alexandre Rostovtsev 2008-12-23 00:05:22 UTC
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
Comment 30 Zhang Rui 2008-12-23 00:19:34 UTC
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
Comment 31 Alexandre Rostovtsev 2008-12-23 00:45:29 UTC
(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?
Comment 32 Zhang Rui 2008-12-23 00:47:06 UTC
Created attachment 19451 [details]
source code of the customized DSDT
Comment 33 Zhang Rui 2008-12-23 00:49:48 UTC
(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
Comment 34 Zhang Rui 2009-01-05 22:32:01 UTC
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.
Comment 35 Zhang Rui 2009-01-05 22:34:48 UTC
*** Bug 12037 has been marked as a duplicate of this bug. ***
Comment 36 Zhang Rui 2009-01-05 22:36:46 UTC
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.
Comment 37 Zhang Rui 2009-01-07 18:23:13 UTC
*** Bug 12235 has been marked as a duplicate of this bug. ***
Comment 38 Andy Whitcroft 2009-01-07 18:29:27 UTC
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.
Comment 39 Thomas Renninger 2009-01-08 07:24:00 UTC
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...
Comment 40 Zhang Rui 2009-01-08 18:15:37 UTC
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.
Comment 41 Zhang Rui 2009-01-08 18:19:40 UTC
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.
Comment 42 Len Brown 2009-01-09 14:07:54 UTC
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
    
Comment 43 Alexandre Rostovtsev 2009-01-10 15:13:23 UTC
(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>"
Comment 44 Alexandre Rostovtsev 2009-01-10 15:23:23 UTC
(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...
Comment 45 Evgeniy Manachkin 2009-01-10 21:53:43 UTC
Thanks a lot. In 2.6.29rc1 kernel backligth work perfect!!!

Asus X51L.
Comment 46 Zhang Rui 2009-01-11 17:56:27 UTC
so there is an empty /sys/class/backlight directory after applying this patch, even with thinkpad_acpi loaded, right?
Comment 47 Alexandre Rostovtsev 2009-01-11 18:02:49 UTC
(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).
Comment 48 Gary Trakhman 2009-01-26 06:55:16 UTC
Trying to get brightness working.  This custom DSDT crashes my kernel. I have an SL300 with 2Gb and p8400 and ubuntu jaunty.
Comment 49 Gary Trakhman 2009-02-08 11:04:04 UTC
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.
Comment 50 Alexandre Rostovtsev 2009-02-08 11:43:55 UTC
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
Comment 51 Gary Trakhman 2009-02-08 12:48:40 UTC
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.
Comment 52 Thomas Renninger 2009-02-08 15:39:46 UTC
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.
Comment 53 Zhang Rui 2009-02-08 18:03:36 UTC
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.
Comment 54 Alexandre Rostovtsev 2009-02-08 18:18:07 UTC
(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.
Comment 55 Zhang Rui 2009-02-08 18:27:58 UTC
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.
Comment 56 ykzhao 2009-02-08 20:52:57 UTC
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.
Comment 57 Zhang Rui 2009-02-08 21:13:29 UTC
1. maximum value doesn't equal 100
2. the brightness levels are sequential numbers start from 0.
Comment 58 Thomas Renninger 2009-02-09 01:00:17 UTC
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).
Comment 59 Peter Klotz 2009-02-09 08:45:35 UTC
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.
Comment 60 Zhang Rui 2009-03-09 23:08:32 UTC
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.
Comment 61 Khashayar Naderehvandi 2009-03-10 07:44:19 UTC
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.
Comment 62 Peter Klotz 2009-03-10 10:25:31 UTC
I applied the patch from comment #60 against 2.6.28.7 and got exactly the same errors.
Comment 63 Zhang Rui 2009-03-10 19:08:09 UTC
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.
Comment 64 Peter Klotz 2009-03-11 10:13:56 UTC
Created attachment 20493 [details]
Kernel 2.6.29-rc7 dmesg output

The laptop is an Asus B50A.
Comment 65 Peter Klotz 2009-03-11 10:14:55 UTC
Created attachment 20494 [details]
acpidump output of Asus B50A
Comment 66 Zhang Rui 2009-03-11 20:14:24 UTC
please attach the output of "grep . /sys/class/backlight/*/*"
Comment 67 Zhang Rui 2009-03-11 20:15:11 UTC
both w/ and w/o the patch in comment #60
Comment 68 Peter Klotz 2009-03-11 22:59:49 UTC
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.
Comment 69 Zhang Rui 2009-03-12 01:39:07 UTC
Created attachment 20500 [details]
patch: video cleanup

please try this updated patch instead of the previous one.
Comment 70 Peter Klotz 2009-03-12 13:46:18 UTC
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
Comment 71 Peter Klotz 2009-03-12 14:57:22 UTC
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.
Comment 72 Zhang Rui 2009-03-12 17:55:25 UTC
Peter, please attach the dmesg output with the kernel in comment #71. thanks
Comment 73 Peter Klotz 2009-03-12 23:00:31 UTC
Created attachment 20513 [details]
Kernel 2.6.27-rc7 dmesg output 

Patch from comment #69 applied. See results in comment #71.
Comment 74 Peter Klotz 2009-03-12 23:02:43 UTC
Sorry, I meant 2.6.29-rc7, not 2.6.27-rc7.
Comment 75 Zhang Rui 2009-03-15 19:48:06 UTC
Created attachment 20536 [details]
debug patch

please apply this debug patch on top of the previous patch and attach the dmesg output after boot.
Comment 76 Peter Klotz 2009-03-16 10:20:26 UTC
Created attachment 20553 [details]
dmesg output (2.6.29-rc7 with debug patch applied)

Patches from comment #69 and comment #75 applied
Comment 77 Zhang Rui 2009-03-16 20:49:31 UTC
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.
Comment 78 Peter Klotz 2009-03-17 14:41:34 UTC
Created attachment 20572 [details]
2.6.29-rc7 dmesg output (DSDT + both patches)
Comment 79 Peter Klotz 2009-03-17 14:42:23 UTC
Created attachment 20573 [details]
2.6.29-rc7 dmesg output (only DSDT applied)
Comment 80 Zhang Rui 2009-03-17 18:06:08 UTC
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.
Comment 81 Zhang Rui 2009-03-17 18:07:15 UTC
Created attachment 20574 [details]
customized DSDT

please use this DSDT instead. :)
Comment 83 Peter Klotz 2009-03-18 13:36:00 UTC
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
Comment 84 Len Brown 2009-03-28 04:56:09 UTC
9be1df98bca44dbe3769cd22f4ab8122b76c5313
http://patchwork.kernel.org/patch/1309/
(bd->props.brightness doesn't reflect the actual backlight level.)
shipped in Linux-2.6.29-rc1
Comment 85 Zhang Rui 2009-03-30 03:18:40 UTC
Created attachment 20728 [details]
patch:video cleanup 7

please attach the patch on top of the previous patches and see if it helps.
Comment 86 Khashayar Naderehvandi 2009-03-30 09:01:41 UTC
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.
Comment 87 Thomas Renninger 2009-03-30 14:50:21 UTC
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.
Comment 88 Peter Klotz 2009-03-30 20:40:41 UTC
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
Comment 89 Zhang Rui 2009-03-31 01:44:15 UTC
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
Comment 90 Zhang Rui 2009-03-31 02:56:41 UTC
Created attachment 20749 [details]
patch: video cleanup -8
Comment 91 Zhang Rui 2009-03-31 03:06:44 UTC
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?
Comment 92 Peter Klotz 2009-03-31 04:59:58 UTC
Yes, only value 10 shows this error. All other values (16-100) work correctly.
Comment 93 Peter Klotz 2009-03-31 20:08:12 UTC
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.
Comment 94 Zhang Rui 2009-04-02 02:14:55 UTC
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
Comment 95 Peter Klotz 2009-04-02 05:20:54 UTC
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
Comment 96 Zhang Rui 2009-04-02 05:42:46 UTC
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 > ..."
Comment 97 Zhang Rui 2009-04-02 05:45:48 UTC
Created attachment 20776 [details]
customized DSDT

BTW: please use this customized DSDT instead.
Comment 98 Peter Klotz 2009-04-02 20:17:47 UTC
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]
Comment 99 Zhang Rui 2009-04-03 00:47:47 UTC
no, :(
please attach the full dmesg output with boot parameters "acpi.debug_layer=0xffffffff acpi.debug_level=0x2007"
Comment 100 Peter Klotz 2009-04-03 05:24:32 UTC
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.
Comment 101 Zhang Rui 2009-04-03 06:10:34 UTC
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.
Comment 102 Thomas Renninger 2009-04-03 09:42:43 UTC
> 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.
Comment 103 Peter Klotz 2009-04-03 19:48:25 UTC
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).
Comment 104 Zhang Rui 2009-04-07 02:57:05 UTC
Created attachment 20844 [details]
debug patch

please apply this patch on top and re-attach the dmesg output. thanks.
Comment 105 Len Brown 2009-04-07 03:05:34 UTC
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)
Comment 106 Peter Klotz 2009-04-07 06:00:01 UTC
Created attachment 20850 [details]
dmesg output as requested in comment #104

I switched to kernel 2.6.29.1.
Comment 107 Zhang Rui 2009-04-07 06:47:56 UTC
From the dmesg output in comment #106,
it seems that the ACPI brightness control is working perfectly on your box, doesn't it?
Comment 108 Peter Klotz 2009-04-07 19:55:23 UTC
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?
Comment 109 Zhang Rui 2009-04-08 05:49:12 UTC
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.
Comment 110 Peter Klotz 2009-04-09 06:35:16 UTC
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?
Comment 111 Peter Klotz 2009-04-09 06:37:24 UTC
Created attachment 20894 [details]
dmesg output corresponding to patch in comment #109
Comment 112 Zhang Rui 2009-04-09 08:04:09 UTC
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.
Comment 113 Zhang Rui 2009-04-14 05:26:49 UTC
close the bug as patches are already available.
Comment 114 Len Brown 2009-04-28 15:52:07 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.