Bug 60680 - Lenovo x230: i915 Backlight not controllable over Fx-F{8,9} buttons
Summary: Lenovo x230: i915 Backlight not controllable over Fx-F{8,9} buttons
Status: CLOSED DOCUMENTED
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: x86-64 Linux
: P1 normal
Assignee: Aaron Lu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-02 08:32 UTC by Borislav Petkov
Modified: 2013-08-12 01:27 UTC (History)
3 users (show)

See Also:
Kernel Version: 3.11-c3
Subsystem:
Regression: No
Bisected commit-id:


Attachments
acpidump by hand (115.88 KB, application/gzip)
2013-08-02 08:32 UTC, Borislav Petkov
Details
Add debug statement in backlight related function (3.25 KB, patch)
2013-08-02 08:51 UTC, Aaron Lu
Details | Diff
dmesg from debug patch (2.13 KB, application/octet-stream)
2013-08-02 16:42 UTC, Borislav Petkov
Details

Description Borislav Petkov 2013-08-02 08:32:16 UTC
Created attachment 107078 [details]
acpidump by hand

Since the 3.11 cycle started, I cannot control the
backlight on my laptop using the Fx-F{8,9} keys. It
still works though when I write values over sysfs's node
/sys/class/backlight/intel_backlight/brightness.

acpidump is attached.

Well, almost - it doesn't work the normal way, it says here:

# acpidump
Linux version not implemented yet
Could not get ACPI tables, AE_SUPPORT

so I read them out from sysfs and hexdump-ed them. Let me know if this
is not ok but it looks ok to me.

Thanks.
Comment 1 Aaron Lu 2013-08-02 08:51:26 UTC
Created attachment 107079 [details]
Add debug statement in backlight related function

Apply on top of today's Linus' tree.

After boot to GUI, use hotkey to adjust brightness. Take a small delay between hotkey press, so that it's easy to see which message is for which action :-)

Also adjust brightness through sysfs, see if the message would be different with the those from hotkey.
Comment 2 Aaron Lu 2013-08-02 08:53:31 UTC
BTW, the acpidump.c is in kernel tree: tools/power/acpi
It seems the new shipped acpidump tool from ACPICA site has problem, but the one in kernel tree is OK.
Comment 3 Borislav Petkov 2013-08-02 16:41:37 UTC
Ok, here it is.

I've attached two files: dmesg.log and dmesg-vendor.log, the last being the
machine booted with "acpi_backlight=vendor".

In both cases, the Fn-Fx buttons didn't work but the debug output you
see is from the sysfs writes I'm doing with my script.

HTH.
Comment 4 Borislav Petkov 2013-08-02 16:42:25 UTC
Created attachment 107082 [details]
dmesg from debug patch
Comment 5 Robert Moore 2013-08-02 23:46:12 UTC
> # acpidump
> Linux version not implemented yet
> Could not get ACPI tables, AE_SUPPORT

This was a temporary placeholder version of acpidump that should not have made it into the linux tree.

The current version is fully implemented for Linux and should work properly.
Comment 6 Aaron Lu 2013-08-03 14:49:26 UTC
(In reply to Borislav Petkov from comment #3)
> Ok, here it is.
> 
> I've attached two files: dmesg.log and dmesg-vendor.log, the last being the
> machine booted with "acpi_backlight=vendor".
> 
> In both cases, the Fn-Fx buttons didn't work but the debug output you
> see is from the sysfs writes I'm doing with my script.

I don't quite understand this...
So when you press the hotkey, is there any kernel message emitted? Is your script called in response to the hotkey automatically?
Comment 7 Borislav Petkov 2013-08-04 10:32:11 UTC
(In reply to Aaron Lu from comment #6)
> I don't quite understand this...
> So when you press the hotkey, is there any kernel message emitted? Is your
> script called in response to the hotkey automatically?

Ah, sorry. So the way it looks like right now is that the hotkey events
don't reach the backlight control.

IOW, acpi_listen says that it gets the events:

video/brightnessdown BRTDN 00000087 00000000 K
video/brightnessdown BRTDN 00000087 00000000
video/brightnessup BRTUP 00000086 00000000 K
video/brightnessup BRTUP 00000086 00000000
video/brightnessdown BRTDN 00000087 00000000 K
video/brightnessdown BRTDN 00000087 00000000
video/brightnessup BRTUP 00000086 00000000 K
video/brightnessup BRTUP 00000086 00000000
video/brightnessdown BRTDN 00000087 00000000 K
video/brightnessdown BRTDN 00000087 00000000

but there's no output from the debug patch. If I do, say increase the
brightness by hand:

echo $(( $(cat /sys/class/backlight/intel_backlight/brightness) + 100 )) > /sys/class/backlight/intel_backlight/brightness

I do get the debugging output:

[ 5527.193263] backlight intel_backlight: backlight_store_brightness: current_brightness=1235, max_brightness=4437, to_be_set_brightness=1335
[ 5527.193273] backlight: set brightness to 1335
[ 5527.193286] [drm:intel_panel_get_max_backlight], max backlight PWM = 4437
[ 5527.193295] i915 0000:00:02.0: intel_panel_set_backlight: level=1335, max=4437, freq=4437
[ 5527.193299] i915 0000:00:02.0: intel_panel_set_backlight: after scale, level becomes 1335
[ 5527.193302] i915 0000:00:02.0: intel_panel_actually_set_backlight: set backlight level=1335
[ 5527.193305] [drm:intel_panel_actually_set_backlight], set backlight PWM = 1335
[ 5528.097711] backlight intel_backlight: backlight_store_brightness: current_brightness=1335, max_brightness=4437, to_be_set_brightness=1435
[ 5528.097720] backlight: set brightness to 1435
[ 5528.097733] [drm:intel_panel_get_max_backlight], max backlight PWM = 4437
[ 5528.097742] i915 0000:00:02.0: intel_panel_set_backlight: level=1435, max=4437, freq=4437
[ 5528.097746] i915 0000:00:02.0: intel_panel_set_backlight: after scale, level becomes 1435
[ 5528.097750] i915 0000:00:02.0: intel_panel_actually_set_backlight: set backlight level=1435
[ 5528.097753] [drm:intel_panel_actually_set_backlight], set backlight PWM = 1435

So, from what I can see here, what's missing is the hotkey events
reaching intel_backlight...

Thanks.
Comment 8 Aaron Lu 2013-08-05 01:25:59 UTC
Right, so I would thinks this is a GUI problem. What GUI are you using, or you are on console?
Comment 9 Borislav Petkov 2013-08-05 09:37:59 UTC
Right, maybe. The GUI I use is Fluxbox.

Here's some more pondering on the whole issue:

Until 3.10 (rc7+ but this should be close to 3.10 in that respect) I
used to boot with acpi_backlight=vendor so that thinkpad_acpi was taking
over this and hotkeys were working. It would still scream like this
though:

[   62.380979] thinkpad_acpi: unknown possible thermal alarm or keyboard event received
[   62.380990] thinkpad_acpi: unhandled HKEY event 0x6050
[   62.380995] thinkpad_acpi: please report the conditions when this event happened to ibm-acpi-devel@lists.sourceforge.net

Now, with 3.11, I'm guessing thinkpad_acpi doesn't get those events
anymore and acpi_backlight=vendor doesn't make any difference.

When I run 'xev', I do get the following events when pressing the
hotkeys:

KeyPress event, serial 36, synthetic NO, window 0x1600001,
    root 0xb3, subw 0x0, time 297345, (982,103), root:(1029,172),
    state 0x0, keycode 232 (keysym 0x1008ff03, XF86MonBrightnessDown), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 36, synthetic NO, window 0x1600001,
    root 0xb3, subw 0x0, time 297345, (982,103), root:(1029,172),
    state 0x0, keycode 232 (keysym 0x1008ff03, XF86MonBrightnessDown), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyPress event, serial 38, synthetic NO, window 0x1600001,
    root 0xb3, subw 0x0, time 300587, (982,103), root:(1029,172),
    state 0x0, keycode 233 (keysym 0x1008ff02, XF86MonBrightnessUp), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 38, synthetic NO, window 0x1600001,
    root 0xb3, subw 0x0, time 300587, (982,103), root:(1029,172),
    state 0x0, keycode 233 (keysym 0x1008ff02, XF86MonBrightnessUp), same_screen YES,
    XLookupString gives 0 bytes:
    XFilterEvent returns: False


So, basically the fix is to connect them to intel_backlight. And I've done this:

.fluxbox/keys:
XF86MonBrightnessUp    :Execute sudo /usr/sbin/brightness.sh i
XF86MonBrightnessDown  :Execute sudo /usr/sbin/brightness.sh d

The syntax should be clear, when getting a XF86MonBrightnessDown/Up
event, execute this script which writes to
/sys/class/backlight/intel_backlight/brightness

And it works ok so I guess we can leave it at that.

It would be better though if those events didn't leave the kernel and
went straight to intel_backlight... Oh well, I'm not sure this even
makes sense.

Thanks.
Comment 10 Aaron Lu 2013-08-06 09:07:25 UTC
OK, so I think you have analyzed pretty clear and I agree with you. I think Fluxbox doesn't provide such functionality to handle hotkeys but your own script does the job.

As for the in kernel processing, I think many people would agree with you. Those who don't use big GUIs like gnome/kde and those who prefer to work under console. But the kernel lacks such functionality now, it doesn't know which interface this system should use. Individual backlight provider module provides control interface through sysfs but they don't have an idea of things like priority. Some provider module does the decision making based on ACPI video: if ACPI video creates backlight interface, they will not(some platform driver like thinkpad_acpi here and the GPU driver nouveau also does this). It's kind of a mess to me.
Comment 11 Borislav Petkov 2013-08-06 09:40:28 UTC
Sounds to me like an RFC mail to lkml should be in order. I don't like
the idea of depending on a userspace element to control such basic
functionality which should be simply done in the kernel.

And since you know the code better, it'll probably be better if you
wrote that mail so that we can have a nice discussion :) (If you rather
don't, I could give it a try with my basic skills).

We should probably put assorted GPU and ACPI people on CC so that we can
hear from all. What do you think?

Thanks Aaron.
Comment 12 Aaron Lu 2013-08-07 00:59:06 UTC
(In reply to Borislav Petkov from comment #11)
> Sounds to me like an RFC mail to lkml should be in order. I don't like
> the idea of depending on a userspace element to control such basic
> functionality which should be simply done in the kernel.

Do you mean the handling of backlight event or more than this?

For backlight event handling, what do you think if systemd control this by listening backlight hotkey events and then do the adjustment?
Peter has this idea:
https://bugzilla.kernel.org/show_bug.cgi?id=51231#c108
https://bugs.freedesktop.org/show_bug.cgi?id=66367
And I think it is also feasible.
Comment 13 Borislav Petkov 2013-08-07 07:37:13 UTC
Actually, I would not rely on any userspace agent if I could be doing the backlight control solely in the kernel. It is much more robust and configuration-free this way because it just works. :)

Thanks.
Comment 14 Borislav Petkov 2013-08-09 21:19:55 UTC
Closing.

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