Bug 51231 - Backlight keys stopped working, unless acpi_osi="!Windows 2012" - Thinkpad t430s - BISECTED
Backlight keys stopped working, unless acpi_osi="!Windows 2012" - Thinkpad t4...
Status: CLOSED CODE_FIX
Product: ACPI
Classification: Unclassified
Component: Power-Video
All Linux
: P1 normal
Assigned To: Aaron Lu
:
: 57131 62941 63811 67031 68751 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-12-02 16:52 UTC by Peter Weber
Modified: 2015-04-25 13:27 UTC (History)
33 users (show)

See Also:
Kernel Version: 3.7.0-rc7
Tree: Mainline
Regression: Yes


Attachments
config of kernel 3.7.0-rc7 (77.78 KB, application/octet-stream)
2012-12-02 16:52 UTC, Peter Weber
Details
dmesg output (66.31 KB, text/plain)
2012-12-02 16:52 UTC, Peter Weber
Details
output of dmidecode on a thinkpad t430s, uefi only mode (16.72 KB, text/plain)
2012-12-02 18:03 UTC, Peter Weber
Details
not working modified backlist.c (9.07 KB, application/octet-stream)
2012-12-30 18:13 UTC, Peter Weber
Details
acpidump output for ThinkPad X230 (367.05 KB, text/plain)
2013-01-08 05:29 UTC, Daniel Kao
Details
acpidump of thinkpad t430s with uefi 2.05 in uefi-only mode, without csm (378.86 KB, application/octet-stream)
2013-01-12 19:11 UTC, Peter Weber
Details
Disable Windows 8 compatibility for ThinkPad T430s and X230 (2.45 KB, patch)
2013-01-14 16:05 UTC, Seth Forshee
Details | Diff
t530 acpidump uefi only (363.45 KB, text/plain)
2013-01-16 09:40 UTC, vekin
Details
Disable Windows 8 compatibility for some Lenovo Thinkpads (v2) (2.65 KB, patch)
2013-01-16 15:05 UTC, Seth Forshee
Details | Diff
Disable Windows 8 compatibility for some Lenovo Thinkpads (v3) (2.87 KB, patch)
2013-02-08 19:32 UTC, Seth Forshee
Details | Diff
dmidecode on a ThinkPad L430 (15.05 KB, text/plain)
2013-02-08 21:08 UTC, Michael Laß
Details
Disable Windows 8 compatibility for some Lenovo Thinkpads (v4) (3.34 KB, patch)
2013-02-08 22:43 UTC, Seth Forshee
Details | Diff
dmidecode on thinkpad x201 (18.98 KB, text/plain)
2013-04-03 10:41 UTC, Alice Ferrazzi
Details
acpidump on x201 (295.76 KB, text/plain)
2013-04-03 10:50 UTC, Alice Ferrazzi
Details
Disable ACPI backlight for broken ThinkPads (6.36 KB, patch)
2013-04-22 17:18 UTC, Seth Forshee
Details | Diff
acpidump from thinkpad x230 (2320-5NG) (366.17 KB, application/octet-stream)
2013-04-29 19:13 UTC, Igor Gnatenko
Details
dmesg with vanilla 3.9.7 (60.16 KB, application/octet-stream)
2013-06-21 09:56 UTC, Yves-Alexis Perez
Details
config for 3.9.7 (94.33 KB, application/octet-stream)
2013-06-21 09:57 UTC, Yves-Alexis Perez
Details
ACPI / video: Remove brightness object if we're not going to use it (538 bytes, patch)
2013-07-17 10:59 UTC, Rafael J. Wysocki
Details | Diff
ACPI video backlight win8 patches (7.85 KB, application/gzip)
2013-10-16 02:00 UTC, Aaron Lu
Details
dmidecode video.use_native_backlight=1 (16.13 KB, text/plain)
2013-10-19 15:44 UTC, Peter Weber
Details
dmidecode video.use_native_backlight -> not set (16.13 KB, text/plain)
2013-10-19 15:45 UTC, Peter Weber
Details
kernel config, kernel 3.12.0-rc5 with patch_set_v5 (81.91 KB, application/octet-stream)
2013-10-19 15:47 UTC, Peter Weber
Details
dmesg with complain from thinkpad_acpi about brightness interface (61.59 KB, text/plain)
2013-10-19 16:00 UTC, Peter Weber
Details
Add Thinkpad T430s into the list (1.55 KB, patch)
2013-10-21 01:34 UTC, Aaron Lu
Details | Diff
fixed typo, ThinkPad is written in camelCase, Thinkpad -> ThinkPad (1.55 KB, application/octet-stream)
2013-10-23 22:55 UTC, Peter Weber
Details
fixed typo, Thinkpad -> ThinkPad (1.55 KB, patch)
2013-10-23 22:59 UTC, Peter Weber
Details | Diff
acpidump T430 (364.03 KB, text/plain)
2014-03-04 10:28 UTC, edm
Details
acpi dump for ThinkPad Helix (UEFI) (358.07 KB, text/plain)
2014-04-08 12:24 UTC, Stephen Chandler Paul
Details
dmidecode dump for ThinkPad Helix (16.22 KB, text/plain)
2014-04-08 12:26 UTC, Stephen Chandler Paul
Details
Dell XPS13 9333 acpidump (458.08 KB, application/octet-stream)
2014-06-01 13:42 UTC, Gabriele Mazzotta
Details
ThinkPad T530 use native backlight patch (555 bytes, patch)
2014-06-22 18:18 UTC, Robert Zavalczki
Details | Diff

Description Peter Weber 2012-12-02 16:52:02 UTC
Created attachment 88171 [details]
config of kernel 3.7.0-rc7

Hello!

The backlight keys (Fn+F8, Fn+F9) stopped working with Kernel 3.7.0-rc7. It works with the current stable Kernel 3.6.8. I can still change the backlight brightness via "System Settings" > "Brightness & Lock" in GNOME. I can't see any message about not working keys in dmesg or the syslog.

I should note, that the brightness-indicator of GNOME shows if I press one of the keys and it moves the progress-bar a  ittle bit. After I changedthe brigthness via "System Settings" it is possible to use the keys to change the brightness over a wider range, but not the complete range. Seems to be some side effect.

Maximal movement of of brightness, visuallay:
Default:
[ x x x x x x x - ]
After I changed the brightness to the middle, via "System Settings":
[ x x - - - - x x ]

Sorry for the weird report.
Thank you.
Comment 1 Peter Weber 2012-12-02 16:52:51 UTC
Created attachment 88181 [details]
dmesg output
Comment 2 Peter Weber 2012-12-02 16:54:22 UTC
Hardware is a ThinkPad t430s in UEFI-Mode, theCompatibility Support Module is switched off.
Comment 3 Zhang Rui 2012-12-02 17:44:19 UTC
well, I guess the problem also exists in 3.3 kernel, right?

it seems that this patch introduces the "regression".
commit fba4e087361605d1eed63343bb08811f097c83ee
Author: Igor Murzov <e-mail@date.by>
Date:   Sat Oct 13 04:41:25 2012 +0400

    ACPI video: Ignore errors after _DOD evaluation.
    
    There are systems where video module known to work fine regardless
    of broken _DOD and ignoring returned value here doesn't cause
    any issues later. This should fix brightness controls on some laptops.
    
    Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=47861
    
    Signed-off-by: Igor Murzov <e-mail@date.by>
    Reviewed-by: Sergey V <sftp.mtuci@gmail.com>
    Signed-off-by: Zhang Rui <rui.zhang@intel.com>

But IMO, the ACPI backlight control on your laptop is always broken,
but this commit
commit ea9f8856bd6d4ed45885b06a338f7362cd6c60e5
Author: Igor Murzov <intergalactic.anonymous@gmail.com>
Date:   Fri Mar 30 21:32:08 2012 +0400

    ACPI video: Harden video bus adding.
    
    It is always better to check return values, so add some new checks and
    correct existing ones.
    
    v2: Be consistent and don't mix errors from -E* and AE_* namespaces.
    
    Signed-off-by: Igor Murzov <e-mail@date.by>
    Signed-off-by: Len Brown <len.brown@intel.com>
happens to make your laptop stop using ACPI backlight control. :)

can you please attach the dmidecode output of your laptop?
so that I can backlist your laptop from ACPI Backlight control.
Comment 4 Peter Weber 2012-12-02 18:02:53 UTC
Thank you. I add the output of dmidecode in a few seconds.
You should take also a look at this (Daniel Vetter and someone with a Dell Laptop):
http://www.spinics.net/lists/kernel/msg1446647.html
Comment 5 Peter Weber 2012-12-02 18:03:31 UTC
Created attachment 88191 [details]
output of dmidecode on a thinkpad t430s, uefi only mode
Comment 6 Peter Weber 2012-12-02 18:11:35 UTC
Adding the parameter "acpi_backlight=vendor" as kernel option to the bootloader makes the Fn-Keys working again, like assumed by Daniel Vetter. But than the UI of GNOME is broken.
Comment 7 Peter Weber 2012-12-03 12:05:16 UTC
I can't confirm that the problem exists in Kernel 3.3 or not, because my first Kernel on that device was a Kernel of the 3.5 series. And with Kernel 3.5 and 3.6 everything worked.
Comment 8 Peter Weber 2012-12-30 18:12:52 UTC
I tried the change blacklist.c, but it doesn't work for me. See attachment. I also wonder about the last empty curly braces in the struct (not added by me)?
Comment 9 Peter Weber 2012-12-30 18:13:51 UTC
Created attachment 89941 [details]
not working modified backlist.c
Comment 10 Peter Weber 2012-12-30 18:16:59 UTC
Question:
$ ls -l
total 0
lrwxrwxrwx 1 root root 0 Dec 30 19:02 acpi_video0 -> ../../devices/pci0000:00/0000:00:02.0/backlight/acpi_video0
lrwxrwxrwx 1 root root 0 Dec 30 19:02 intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-LVDS-1/intel_backlight

If I understand your noted patches above, it is *okay* to see both directorie on kernel 3.6 and 3.7. The acpi_video0 just doesn't block intel_backlight on kernel 3.6?
Comment 11 Peter Weber 2012-12-30 20:33:06 UTC
Adding following parameters to boot command line seems to be a mostly working workaround:

# first switches of thinkpad_acpi, seconds switches of acpi_video
thinkpad-acpi.brightness_enable=0 acpi_backlight=vendor

* Fn+BrightnessDown, Fn+BrighnessUp works
* GNOME Power-Widget displays change of value through Fn-Keys
* GNOME > System Settings > Brighness works
* ls -l /sys/class/backlight only lists intel_backlight (not acpi or thinkpad)

Only automatic display-dimming through GNOME seems not to work.
Comment 12 Daniel Kao 2012-12-31 08:39:29 UTC
I have done a git bisect and found the offending commit:
commit a57f7f9175b8ccbc9df83ac13860488913115de4
Author: Bob Moore <robert.moore@intel.com>
Date:   Fri Aug 17 10:55:02 2012 +0800

    ACPICA: Add Windows8/Server2012 string for _OSI method.

    This change adds a new _OSI string, "Windows 2012" for both Windows 8
    and Windows Server 2012.

    From Microsoft document "How to Identify the Windows Version in ACPI
    by Using _OSI", July 13, 2012.

    Signed-off-by: Bob Moore <robert.moore@intel.com>
    Signed-off-by: Feng Tang <feng.tang@intel.com>
    Signed-off-by: Len Brown <len.brown@intel.com>

Reverting this fixes the issue for me on ThinkPad X230.
Comment 13 Zhang Rui 2013-01-08 02:34:50 UTC
oh, please attach the acpidump output of your laptop.
Comment 14 Zhang Rui 2013-01-08 02:47:52 UTC
BTW, does adding kernel parameter acpi_osi="!Windows 2012" help?
Comment 15 Daniel Kao 2013-01-08 05:29:59 UTC
Created attachment 90681 [details]
acpidump output for ThinkPad X230
Comment 16 Daniel Kao 2013-01-08 05:31:12 UTC
acpi_osi="!Windows 2012" works, I've been using that instead of the modified kernel.
Thanks.
Comment 17 Peter Weber 2013-01-08 10:06:20 UTC
Thats weird! I will test that also and tell you if it works.

Am I right, that this will tell the ACPI based functions of the kernel that the BIOS/UEFI doesn't support Windows 2012?
Comment 18 Peter Weber 2013-01-08 21:33:26 UTC
I can confirm that this works!

ls -l /sys/class/backlight/
total 0
lrwxrwxrwx 1 root root 0 Jan  8 22:20 acpi_video0 -> ../../devices/pci0000:00/0000:00:02.0/backlight/acpi_video0
lrwxrwxrwx 1 root root 0 Jan  8 22:20 intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-LVDS-1/intel_backlight

Booth seem to work, again, adjacent to another.
But the automatic display dimming via GNOME still seems not to work. I wonder why, but it is not really important.
Comment 19 Seth Forshee 2013-01-10 16:41:26 UTC
I took a look at the ACPI tables from an x230. If the OS says it's Windows 2012 then _BCL gives an alternate table of brightness values to meet an arbitrary requirement of Win8 to have 101 brightness levels [1]. _BCM tries to find a match for the brightness level it's given in the smaller table it uses for non-Win8 OSes. If it doesn't find an exact match then it just returns without applying any brightness change, so of the 101 values that _BCL claims to support only a much smaller subset (16) actually affect a change in the brightness.

[1] http://msdn.microsoft.com/en-us/library/windows/hardware/jj128256.aspx
Comment 20 Seth Forshee 2013-01-10 17:46:56 UTC
I'm not seeing any way to fix this other than doing some sort of quirking for the affected machines.

One option add a quirk to keep the kernel from claiming to be Windows 8. There are other places in the ACPI tables where behavior will be affected, but since folks have been running older kernels successfully on these machines I suspect everything should work okay.

The other option I can think of is to quirk acpi-video so that it will only use the brightness values that actually work. I don't see any way to do this without hard coding firmware implementation details into the driver though, which is ugly.

Any other ideas?
Comment 21 Daniel Kao 2013-01-10 22:28:56 UTC
I see that _BCM and _BQC only work with the 16 step table (\_SB.PCI0.LPC.EC.BRTW).
When interacting with the backlight through sysfs, brightness node can be set to any value from _BCL, but actual_brightness reports its current value from _BQC. This causes a discrepancy between brightness and actual_brightness where brightness could be 75 but actual_brightness remains at 70.

Gnome uses RandR to change backlight, which reports actual_brightness when available. When Gnome tries to change brightness in reaction to media keys, it tries to do so in 5% increments based on actual_brightness. Now, since 75% is not supported in the BRTW table, stepping up from 70% would never succeed and so Gnome's brightness OSD gets stuck at 75% and cannot go any higher. Same thing happens when trying to step down from 20%.

I have created a test kernel by commenting out the get_brightness entry in acpi_backlight_ops. RandR seems to fallback to the brightness node and everything works fine in Gnome.

Brightness adjustment with media keys in virtual terminals still has no effect. But I have no idea how the brightness keys are handled in VT so I can't comment on that.
Comment 22 Daniel Kao 2013-01-10 23:34:17 UTC
Sorry, made a mistake in the previous comment:
RandR backlight control doesn't work without a valid actual_brightness node so Gnome falls back to its own backlight helper which uses the brightness node.
Comment 23 Seth Forshee 2013-01-11 16:00:06 UTC
(In reply to comment #21)
> I see that _BCM and _BQC only work with the 16 step table
> (\_SB.PCI0.LPC.EC.BRTW).
> When interacting with the backlight through sysfs, brightness node can be set
> to any value from _BCL, but actual_brightness reports its current value from
> _BQC. This causes a discrepancy between brightness and actual_brightness where
> brightness could be 75 but actual_brightness remains at 70.

Yes, since _BCM only discards the value and doesn't return an error, acpi_video thinks the brightness change was successful and thus updates the cached brightness value. actual_brightness shows that the firmware discarded the value.

> Gnome uses RandR to change backlight, which reports actual_brightness when
> available. When Gnome tries to change brightness in reaction to media keys, it
> tries to do so in 5% increments based on actual_brightness. Now, since 75% is
> not supported in the BRTW table, stepping up from 70% would never succeed and
> so Gnome's brightness OSD gets stuck at 75% and cannot go any higher. Same
> thing happens when trying to step down from 20%.
> 
> I have created a test kernel by commenting out the get_brightness entry in
> acpi_backlight_ops. RandR seems to fallback to the brightness node and
> everything works fine in Gnome.

We shouldn't expose the backlight to userspace if it isn't going to work properly. So the possible solutions I can think of are quirking to use "!Windows 2012" by default, quirking to disable acpi_video backlight, or quirking somehow to make acpi_video only use the values that actually work.

> Brightness adjustment with media keys in virtual terminals still has no effect.
> But I have no idea how the brightness keys are handled in VT so I can't comment
> on that.

This depends to some degree on how the firmware behaves. Most of the time a keycode is emitted when you press the brightness keys which userspace needs to respond to. When you're in a VT there's probably no software responding to the brightness keycodes, thus no change in brightness.
Comment 24 Peter Weber 2013-01-12 19:11:51 UTC
Created attachment 91091 [details]
acpidump of thinkpad t430s with uefi 2.05 in uefi-only mode, without csm

acpidump in uefi-only mode, compatibility-support-module is off
Comment 25 Seth Forshee 2013-01-14 16:05:38 UTC
Created attachment 91311 [details]
Disable Windows 8 compatibility for ThinkPad T430s and X230

Here's a patch (based on 3.8-rc3) which disables Windows 8 compatibility for the T430s and X230 in ACPI. Please give it a try.
Comment 26 Seth Forshee 2013-01-14 19:52:55 UTC
Just a note that I've received a positive verification of the patch on the X230. I'd still like to get someone to verify it works on the T430s.
Comment 27 Peter Weber 2013-01-15 08:41:52 UTC
I can't promise it, but I looking forward to test the patch today evening (20.00 o'clock CET).
Comment 28 Peter Weber 2013-01-15 22:03:38 UTC
I'm late. But I'm always late ;-)

The patch works! Thank you!
Also your patch showed me, what was wrong with my own attempt.
Comment 29 vekin 2013-01-16 09:40:44 UTC
Created attachment 91381 [details]
t530 acpidump uefi only

t530 is also affected.

acpi_osi="!Windows 2012" workaround works.

Here is my acpi dump while in uefi only mode.
Comment 30 Seth Forshee 2013-01-16 15:05:29 UTC
Created attachment 91391 [details]
Disable Windows 8 compatibility for some Lenovo Thinkpads (v2)

Sigh. I wonder how many machines are affected in total. I wonder if we ought to resort to just disabling the acpi_video backlight on these machines instead, but that assumes that every machine with this implementation has another working backlight available.

Anyway, for now here's an updated patch including a quirk for the T530. Please test.
Comment 31 Peter Weber 2013-01-16 22:57:44 UTC
I'm afraid all current ThinkPads from 2012. Pretty sure we can safely add add x230t (this time with t), t430(this time without s) and x1 also. I think the best solution is to talk to the people at Lenovo, because I've good experience with the own forums, I decided to open a thread there. Writing to "official" emails addresses seem like a bad idea.

http://forums.lenovo.com/t5/Linux-Discussion/Contact-to-engineers-developers-for-fixing-ACPI-Backlight-broken/td-p/992621

I asked them for a list of models with this "mapping", and as bonus, for how the mapping can be used. Maybe we can profit from that knowledge in future.

btw. To you think Linus will accept the quirks for the upcoming rcs of 3.8? They are small, improve the situation and couldn't harm.
Comment 32 Seth Forshee 2013-01-17 01:53:24 UTC
I'm already trying to get this information to Lenovo. However at this point it's in the wild and there will always be broken machines, so it would be nice if we had a way to deal with it. Quirking that many machines might not be well received, though any other solution I can think of seems even worse. Unless all the broken machines have some other attribute in common, like the firmware version, that we can use for quirking instead. That would be good.

Fwiw, I suspect the reason it works under Windows is that it must be trying to do a smooth backlight adjustment by writing every brightness value between current_brightness and new_brightness, so you at least end up with the last valid brightness value written.
Comment 33 Peter Weber 2013-01-17 12:20:44 UTC
As far as I know, the firmware version 2.0 was introduced to support Windows 8.

# this dmesg is a bit old, my current firmware is 2.05
DMI: LENOVO 2353CTO/2353CTO, BIOS G7ET60WW (2.02 ) 09/11/2012
Comment 34 fmedinamm 2013-01-18 00:33:12 UTC
Running Kernel 3.7.2 on ThinkPad X1 Carbon with Ubuntu 12.10, can confirm this exact bug.  

xev shows this when hitting Fn+Brightness Key:

RRNotify event, serial 71, synthetic NO, window 0x4a00001,
    subtype XRROutputPropertyChangeNotifyEvent
    output LVDS1, property BACKLIGHT, timestamp 1441820, state NewValue
Comment 35 zap_roland 2013-01-23 10:11:58 UTC
I can confirm this bug on ThinkPad W530 running Arch Linux kernel 3.7.3.
Comment 36 Keshav Kini 2013-02-08 19:28:01 UTC
I can confirm this bug on ThinkPad X230 running pf-sources 3.7.4 (for whatever that's worth).
Comment 37 Seth Forshee 2013-02-08 19:32:33 UTC
Created attachment 92761 [details]
Disable Windows 8 compatibility for some Lenovo Thinkpads (v3)

Sorry for not following up sooner. I was hoping to be able to get some information from Lenovo, but so far that hasn't happened.

I also took a look at trying to quirk based on some other criteria like firmware version. I collected dmi information from Launchpad for a bunch of different thinkpad models, and unfortunately the way the bios is versioned across different models doesn't make this a viable option.

So anyway, I'm attaching an updated patch which contains quirks for the four models which have been confirmed as being affected by this bug.
Comment 38 Keshav Kini 2013-02-08 19:38:15 UTC
Thanks for the quick response! Forgive me if I've misunderstood something, but it seems that the X230 model is not included among the four in your patch. Does this mean that your patch would not fix the issue on an X230 even if you did include a match for the X230?

BTW, minor typo in your code comments - s/behavoir/behavior/ ;)
Comment 39 Seth Forshee 2013-02-08 19:59:02 UTC
(In reply to comment #38)
> Thanks for the quick response! Forgive me if I've misunderstood something, but
> it seems that the X230 model is not included among the four in your patch. Does
> this mean that your patch would not fix the issue on an X230 even if you did
> include a match for the X230?

My quick response was a lucky coincidence -- I was already getting ready to post the new patch.

I did somehow end up dropping the quirk for the x230 (it was in the first two versions). Not sure how that happened. Thanks for catching it; I'll add it back.

> BTW, minor typo in your code comments - s/behavoir/behavior/ ;)

Thanks!
Comment 40 Michael Laß 2013-02-08 20:07:49 UTC
Too bad that Lenovo didn't give you more information on that.
I have a "ThinkPad L430" which is also affected by this problem. Would be nice if this model also finds it way into the patch.
Comment 41 Seth Forshee 2013-02-08 20:15:43 UTC
(In reply to comment #40)
> Too bad that Lenovo didn't give you more information on that.
> I have a "ThinkPad L430" which is also affected by this problem. Would be nice
> if this model also finds it way into the patch.

I'll need dmi information to add your machine. Please give me the output from running dmidecode on your machine.
Comment 42 Michael Laß 2013-02-08 21:08:13 UTC
Created attachment 92771 [details]
dmidecode on a ThinkPad L430
Comment 43 Seth Forshee 2013-02-08 22:43:59 UTC
Created attachment 92781 [details]
Disable Windows 8 compatibility for some Lenovo Thinkpads (v4)

Another patch, this time re-adding the X230 and adding the L430 as well. Please test.
Comment 44 Michael Laß 2013-02-09 11:21:41 UTC
I can confirm that this patch solves the problem on the L430 running Linux 3.7.6.
Comment 45 Peter Weber 2013-02-09 17:56:49 UTC
This fourth release of this patch works (of course) still with kernel 3.8-rc7.
Comment 46 fmedinamm 2013-02-14 04:43:54 UTC
This patch works on kernel 3.7.7 on ThinkPad X1 Carbon as well.

Great job!
Comment 47 Keshav Kini 2013-02-15 22:42:15 UTC
Hmm. I can't seem to find any difference between the behavior before and after the patch. Brightness keys still do not work; /sys/class/backlight/acpi_video0 and /sys/class/backlight/intel_backlight function the same before and after the patch. However, I'm running a heavily patched kernel, so let me try this with a vanilla kernel and report back.
Comment 48 Keshav Kini 2013-02-15 23:58:12 UTC
Ugh, never mind. The patch works just fine, I just failed to copy the right kernel image to /boot when testing it... sorry for the noise!
Comment 49 Peter Weber 2013-02-16 14:58:21 UTC
Lenovo released a new BIOS/UEFI, at least for ThinkPad t430s:
http://download.lenovo.com/ibmdl/pub/pc/pccbbs/mobiles/g7uj06uc.txt

Quote:
"- Fixed an issue where the display was corrupted on Linux."

Doesn't sound like it fits to this issue. But I will install it anyway.
Comment 50 Peter Weber 2013-02-17 11:22:54 UTC
I've installed the new BIOS/UEFI. Nothing changed, with and without the patch.

Seems like Lenovo added four addidional questions "Do you want really upgrade?" to the installer. I'm not kidding. The installer also ask to keep the "UEFI boot image", sounds like the UEFI bootloader-entries.
Comment 51 Theodore Tso 2013-02-18 19:14:55 UTC
For what it's worth, I have an X230, with the 2.06 version of the BIOS and the 1.10 version of the firmware, running a 3.8-rc6 kernel plus ext4 development patches (3.8.0-rc6-00041-gf3a1e5d).  i am using Xfce4 (none of that GNOME crap for me :-), and the brightness keys (Fn-F8/Fn-F9) work just _fine_.
Comment 52 Keshav Kini 2013-02-19 00:20:39 UTC
I have an X230 with BIOS 2.02 and firmware 1.9, and I'm also not using GNOME (just xmonad on bare X). The backlight keys weren't working for me on 3.7 + postfactum's patchset. So I guess it wasn't a GNOME problem, but a kernel problem. (Well, possibly a problem in the patchset, but I doubt it since multiple people presumably not using that patchset were able to duplicate the issue.)

I'm glad to hear it's fixed in 3.8-rc6, though :)
Comment 53 Yves-Alexis Perez 2013-02-19 09:32:54 UTC
(In reply to comment #52)
> I have an X230 with BIOS 2.02 and firmware 1.9, and I'm also not using GNOME
> (just xmonad on bare X). The backlight keys weren't working for me on 3.7 +
> postfactum's patchset. So I guess it wasn't a GNOME problem, but a kernel
> problem. (Well, possibly a problem in the patchset, but I doubt it since
> multiple people presumably not using that patchset were able to duplicate the
> issue.)
> 
> I'm glad to hear it's fixed in 3.8-rc6, though :)

Actually I don't think it's fixed. I just tried on 3.8.0 and it doesn't work. Afair Theodore's x230 doesn't use UEFI mode, and afaict everyone else here does boot using “pure” UEFI and no CSM (but a confirmation would help).

My grub-efi doesn't boot when using CSM (indeed) so I can't tell if it'd work on 3.8+ with CSM but I think it's at least related.
Comment 54 Seth Forshee 2013-02-19 14:26:29 UTC
(In reply to comment #51)
> For what it's worth, I have an X230, with the 2.06 version of the BIOS and the
> 1.10 version of the firmware, running a 3.8-rc6 kernel plus ext4 development
> patches (3.8.0-rc6-00041-gf3a1e5d).  i am using Xfce4 (none of that GNOME crap
> for me :-), and the brightness keys (Fn-F8/Fn-F9) work just _fine_.

Odd, this bios version isn't listed on Lenovo's website. It is newer than the x230 bios version that I know to have the bug, so perhaps Lenovo fixed the problem. Care to attach your ACPI tables?

It's also possible that it works because you're using a different window manager, so you most likely wouldn't be using gnome-settings-daemon. You could check /sys/class/backlight/*/{brightness,actual_brightness} and see how the values are changing in response to the brightness keys.
Comment 55 Michael Laß 2013-02-19 16:12:00 UTC
(In reply to comment #53)
> Actually I don't think it's fixed. I just tried on 3.8.0 and it doesn't work.
> Afair Theodore's x230 doesn't use UEFI mode, and afaict everyone else here does
> boot using “pure” UEFI and no CSM (but a confirmation would help).

The L430 is also affected when booting in legacy mode (not UEFI), so I don't think that the boot mode has any influence on this problem.
Comment 56 Yves-Alexis Perez 2013-02-19 17:47:29 UTC
(In reply to comment #54)
> (In reply to comment #51)
> > For what it's worth, I have an X230, with the 2.06 version of the BIOS and the
> > 1.10 version of the firmware, running a 3.8-rc6 kernel plus ext4 development
> > patches (3.8.0-rc6-00041-gf3a1e5d).  i am using Xfce4 (none of that GNOME crap
> > for me :-), and the brightness keys (Fn-F8/Fn-F9) work just _fine_.
> 
> Odd, this bios version isn't listed on Lenovo's website. 

Sure it is: http://download.lenovo.com/ibmdl/pub/pc/pccbbs/mobiles/g2uj10us.txt
Latest is 2.51 and:

2.06  (G2UJ08US)  2.06       (G2ET86WW)  1.10  (G2HT31WW)    01    2012/11/27



> It is newer than the
> x230 bios version that I know to have the bug, so perhaps Lenovo fixed the
> problem. Care to attach your ACPI tables?

I do have the problem with 2.51 (latest version available).
> 
> It's also possible that it works because you're using a different window
> manager, so you most likely wouldn't be using gnome-settings-daemon. You could
> check /sys/class/backlight/*/{brightness,actual_brightness} and see how the
> values are changing in response to the brightness keys.

I'm using Xfce too, so I'll try to explain how it works to hopefully unconfuse people.

On x230 (and all the 2012 Thinkpads, it seems), before 3.7 brightness keys work just fine independently of any userspace daemon. It works in single user, it works in vt, it works in X at login screen, it just works. However, Xfce needs a bit of tuning in order to indicate xfce4-power-manager it doesn't have to handle brightness keys (KEY_BRIGHTNESS{UP,DOWN}) itself (since it's already handled, I guess by video.ko) but that's all.

On 3.7+ and due to the _OSI("Windows 2012") handling, video.ko seems to not handle those anymore, but KEY_BRIGHTNESS{UP,DOWN} are still correctly emitted (by /dev/input/even4 which is "Video Bus"). So if xfce4-power-manager is configured to handle the keys itself (which is the case by default) it'll happily do it (however you'll only have 8 or 9 brightness level instead of 15, it seems).

So in case it's still confusing, I think it's better to test either in a vt or in X with nothing else running (just running X from vt for example).
Comment 57 Seth Forshee 2013-02-19 18:28:43 UTC
(In reply to comment #56)
> (In reply to comment #54)
> > (In reply to comment #51)
> > > For what it's worth, I have an X230, with the 2.06 version of the BIOS and the
> > > 1.10 version of the firmware, running a 3.8-rc6 kernel plus ext4 development
> > > patches (3.8.0-rc6-00041-gf3a1e5d).  i am using Xfce4 (none of that GNOME crap
> > > for me :-), and the brightness keys (Fn-F8/Fn-F9) work just _fine_.
> > 
> > Odd, this bios version isn't listed on Lenovo's website. 
> 
> Sure it is: http://download.lenovo.com/ibmdl/pub/pc/pccbbs/mobiles/g2uj10us.txt
> Latest is 2.51 and:
> 
> 2.06  (G2UJ08US)  2.06       (G2ET86WW)  1.10  (G2HT31WW)    01    2012/11/27

It wasn't listed at whatever page I was looking at :-/

> > It is newer than the
> > x230 bios version that I know to have the bug, so perhaps Lenovo fixed the
> > problem. Care to attach your ACPI tables?
> 
> I do have the problem with 2.51 (latest version available).

Good to know, thanks.

> > It's also possible that it works because you're using a different window
> > manager, so you most likely wouldn't be using gnome-settings-daemon. You could
> > check /sys/class/backlight/*/{brightness,actual_brightness} and see how the
> > values are changing in response to the brightness keys.
> 
> I'm using Xfce too, so I'll try to explain how it works to hopefully unconfuse
> people.
> 
> On x230 (and all the 2012 Thinkpads, it seems), before 3.7 brightness keys work
> just fine independently of any userspace daemon. It works in single user, it
> works in vt, it works in X at login screen, it just works. However, Xfce needs
> a bit of tuning in order to indicate xfce4-power-manager it doesn't have to
> handle brightness keys (KEY_BRIGHTNESS{UP,DOWN}) itself (since it's already
> handled, I guess by video.ko) but that's all.
> 
> On 3.7+ and due to the _OSI("Windows 2012") handling, video.ko seems to not
> handle those anymore, but KEY_BRIGHTNESS{UP,DOWN} are still correctly emitted
> (by /dev/input/even4 which is "Video Bus"). So if xfce4-power-manager is
> configured to handle the keys itself (which is the case by default) it'll
> happily do it (however you'll only have 8 or 9 brightness level instead of 15,
> it seems).

To clarify, video.ko does still handle the keys (whether or not it should be doing so is debatable). What happens is that it receives a brightness up/down notification, reads the current brightness via _BQC, selects the next higher/lower brightness level, and writes it via the _BCM. The adjacent levels do not change the brightness though, not even the internal value of the brightness returned by _BQC. So the next time the brightness up/down key is pressed it reads the exact same value as the first time. So no matter how many times you hit the hotkeys you'll never see any change in brightness.

> So in case it's still confusing, I think it's better to test either in a vt or
> in X with nothing else running (just running X from vt for example).

That would take X and the desktop out of the picture. What I thought might be  different about xfce is that it could be responding to the notifications by changing the brightness via the GPU's backlight device instead of acpi_video's. But you say that isn't happening for you, so it makes me wonder what's different about Ted's situation.

Also I should probably give an update. The short version is that the maintainers have requested that I fix the issue by gradually changing the brightness instead of using a quirk. Going that route doesn't fix the problem I just described with the hotkeys, so the approach I've taken is to 1) read back the brightness after writing and disable use of _BQC if the value doesn't match, and 2) by default disable handling brightness notifications within the video driver and letting userspace take care of it.

I've got patches (along with a test build for Ubuntu), which I'll link to below. Testing is appreciated. One warning though: a colleague of mine had his machine get bricked while testing an earlier revision of these patches. I really don't _think_ there's anything in them which should brick a machine, however I can't say for sure as we don't know how his machine got bricked. We do know that he had a kernel panic (not where the panic occurred though) and that the machine got very hot before he noticed, and either of those could also be related to the bricking.

That said, here's the link to the patches for those willing to assume the risk.

http://people.canonical.com/~sforshee/lp1098216/linux-3.8.0-6.12~lp1098216v201302141715/
Comment 58 Yves-Alexis Perez 2013-02-19 19:47:49 UTC
(In reply to comment #57)
> To clarify, video.ko does still handle the keys (whether or not it should be
> doing so is debatable). What happens is that it receives a brightness up/down
> notification, reads the current brightness via _BQC, selects the next
> higher/lower brightness level, and writes it via the _BCM. The adjacent levels
> do not change the brightness though, not even the internal value of the
> brightness returned by _BQC. So the next time the brightness up/down key is
> pressed it reads the exact same value as the first time. So no matter how many
> times you hit the hotkeys you'll never see any change in brightness.

Thanks for the clarification.
> 
> > So in case it's still confusing, I think it's better to test either in a vt or
> > in X with nothing else running (just running X from vt for example).
> 
> That would take X and the desktop out of the picture. What I thought might be 
> different about xfce is that it could be responding to the notifications by
> changing the brightness via the GPU's backlight device instead of acpi_video's.
> But you say that isn't happening for you, so it makes me wonder what's
> different about Ted's situation.

I think that Ted might not have changed anything to xfce4-power-manager default setup, so maybe 3.7 actually improved his experience, due to shortcomings in xfpm.
> 
> Also I should probably give an update. The short version is that the
> maintainers have requested that I fix the issue by gradually changing the
> brightness instead of using a quirk. Going that route doesn't fix the problem I
> just described with the hotkeys, so the approach I've taken is to 1) read back
> the brightness after writing and disable use of _BQC if the value doesn't
> match, and 2) by default disable handling brightness notifications within the
> video driver and letting userspace take care of it.

Hmh, that looks like a huge change to me. On a personal note, I prefer this kind of things to be handled in the kernel, so it works just fine wether you're in X, on a vte, with the screen locked or not etc. I know some daemon don't handle this really fine (at least xfpm is a bit confused since it doesn't really detect automatically if he should hands off the brightness), but at least that means a daemon is *not* needed to handle something as close to metal as the brightness handling is.

> That said, here's the link to the patches for those willing to assume the risk.

I'll try to test them tomorrow on X230 and report back.
Comment 59 Yves-Alexis Perez 2013-03-06 15:59:06 UTC
It took me a while, but I just tested the patches on top on 3.8.2.

- brightness keys don't work in a tty or when no userspace daemon handles brightness
- evtest /dev/input/event4 (Video Bus) sees KEY_BRIGHTNESS{UP,DOWN}
- on Xfce with xfce4-power-manager default config 
  - brightness is correctly changed when brightness keys are used, 
  - 16 levels (with no smoothing) and OSD is behaving correctly

For comparison, on a 3.8.2 booted with acpi_osi="!Windows 2012" I get:

- brightness keys work in a tty or when no userspace daemon is running (16 levels, no smoothing)
- evtest /dev/input/event4 (Video Bus) sees KEY_BRIGHTNESS{UP,DOWN}
- on Xfce with xfce4-power-manager default config
  - brightness is correctly changed when brightness keys are used
  - I only get 9 levels, since brightness is changed both by the kernel and by xfpm
  - when disabling xfpm brightness keys handling I get 16 levels back (and I lose the OSD report)

In my opinion the second behavior is better since that means brightness doesn't depend on userspace. And I wonder if KEY_BRIGHTNESS{UP,DOWN} should be sent at all, since job has already been done.
Comment 60 Aaron Lu 2013-03-07 06:32:34 UTC
(In reply to comment #57)
> To clarify, video.ko does still handle the keys (whether or not it should be

Indeed, but it handles the key only when requested, i.e. a user space program did a sysfs write to the brightness file, then acpi video driver will update the backlight level accordingly; not that it handles the key automatically. So if there is no app reacting to the key, acpi video driver will do nothing.

> doing so is debatable). What happens is that it receives a brightness up/down
> notification, reads the current brightness via _BQC, selects the next
> higher/lower brightness level, and writes it via the _BCM. The adjacent levels

I don't think it is working this way.
When user press the Fn+brightness key, usually, the scan code will be sent to user space as an input event; user space apps like g-s-d or acpid will listen for such evens and react accordingly: choosing a level and write it to the sysfs file to change the brightness level. How does the app choose the next level is beyond of my knowledge, and I would very much like to know.

> do not change the brightness though, not even the internal value of the
> brightness returned by _BQC. So the next time the brightness up/down key is
> pressed it reads the exact same value as the first time. So no matter how many
> times you hit the hotkeys you'll never see any change in brightness.

Thanks for the information, it helped to understand what is happening here.

> 
> > So in case it's still confusing, I think it's better to test either in a vt or
> > in X with nothing else running (just running X from vt for example).
> 
> That would take X and the desktop out of the picture. What I thought might be 
> different about xfce is that it could be responding to the notifications by
> changing the brightness via the GPU's backlight device instead of acpi_video's.

That's possible, it all depends on user space app. AFAIK, g-s-d will prefer to use acpi_video's backlight control file, but I don't know how xfce's power management component works.

> But you say that isn't happening for you, so it makes me wonder what's
> different about Ted's situation.
> 
> Also I should probably give an update. The short version is that the
> maintainers have requested that I fix the issue by gradually changing the
> brightness instead of using a quirk. Going that route doesn't fix the problem I
> just described with the hotkeys, so the approach I've taken is to 1) read back
> the brightness after writing and disable use of _BQC if the value doesn't

This might be more complex: current code already handle some broken BIOSes that does not report correct value with _BQC and we would assume the _BQC is retuning an index instead of level value.

> match, and 2) by default disable handling brightness notifications within the
> video driver and letting userspace take care of it.

As I said above, acpi video driver does not handle it automatically. For the normal case, it even doesn't know such a Fn key is pressed; for some other cases, it will get a notification from some EC firmware code and then it will map those notification to standard video brightness Fn keys and send them to user space through an input device it registered, for user space's awareness.

> 
> I've got patches (along with a test build for Ubuntu), which I'll link to
> below. Testing is appreciated. One warning though: a colleague of mine had his
> machine get bricked while testing an earlier revision of these patches. I
> really don't _think_ there's anything in them which should brick a machine,
> however I can't say for sure as we don't know how his machine got bricked. We
> do know that he had a kernel panic (not where the panic occurred though) and
> that the machine got very hot before he noticed, and either of those could also
> be related to the bricking.
> 
> That said, here's the link to the patches for those willing to assume the risk.
> 
> http://people.canonical.com/~sforshee/lp1098216/linux-3.8.0-6.12~lp1098216v201302141715/

May I know which patch is included in this kernel?

Thanks.
Comment 61 Aaron Lu 2013-03-07 06:51:32 UTC
Hi Seth,

I'm sorry I think I missed some code in video.c...

Apparently, for the notification case, the driver indeed changed the brightness level. I don't think this is correct, since it will also report this event to user space, like the no notification case. So we should definitely set brightness_switch_enabled to 0 by default, to give user space a consistent behaviour.

BTW, I've just checked the link you gave, there are several patches. Thanks for the patches.
Comment 62 Yves-Alexis Perez 2013-03-07 07:09:05 UTC
(In reply to comment #60)
> (In reply to comment #57)
> > To clarify, video.ko does still handle the keys (whether or not it should be
> 
> Indeed, but it handles the key only when requested, i.e. a user space program
> did a sysfs write to the brightness file, then acpi video driver will update
> the backlight level accordingly; not that it handles the key automatically. So
> if there is no app reacting to the key, acpi video driver will do nothing.

Well, I'm not sure how generic this comment is, but I kind-of disagree. I can't test without video.ko since nowadays i915 needs it and KMS needs i915, but on my x201s and my x230 pre 3.7 or 3.7+ with acpi_osi="!Windows 2012" I don't need any userspace daemon to handle brightness keys.

(In reply to comment #61)
> Hi Seth,
> 
> I'm sorry I think I missed some code in video.c...
> 
> Apparently, for the notification case, the driver indeed changed the brightness
> level. I don't think this is correct, since it will also report this event to
> user space, like the no notification case. So we should definitely set
> brightness_switch_enabled to 0 by default, to give user space a consistent
> behaviour.

If that means brightness keys won't work anymore if no userspace is running (or if the screen is locked, or noone is logged in, or anything userspace related), I again think this is a bad idea.
Comment 63 Aaron Lu 2013-03-07 07:20:46 UTC
(In reply to comment #62)
> (In reply to comment #60)
> > (In reply to comment #57)
> > > To clarify, video.ko does still handle the keys (whether or not it should be
> > 
> > Indeed, but it handles the key only when requested, i.e. a user space program
> > did a sysfs write to the brightness file, then acpi video driver will update
> > the backlight level accordingly; not that it handles the key automatically. So
> > if there is no app reacting to the key, acpi video driver will do nothing.
> 
> Well, I'm not sure how generic this comment is, but I kind-of disagree. I can't
> test without video.ko since nowadays i915 needs it and KMS needs i915, but on

The acpi video driver can do some things, and backlight control is one of them. I'm talking about backlight control alone here.

> my x201s and my x230 pre 3.7 or 3.7+ with acpi_osi="!Windows 2012" I don't need
> any userspace daemon to handle brightness keys.

Right, that is because for your laptop, there will be an acpi notification once the function key is pressed. But this is not the case for all laptops.

> 
> (In reply to comment #61)
> > Hi Seth,
> > 
> > I'm sorry I think I missed some code in video.c...
> > 
> > Apparently, for the notification case, the driver indeed changed the brightness
> > level. I don't think this is correct, since it will also report this event to
> > user space, like the no notification case. So we should definitely set
> > brightness_switch_enabled to 0 by default, to give user space a consistent
> > behaviour.
> 
> If that means brightness keys won't work anymore if no userspace is running (or
> if the screen is locked, or noone is logged in, or anything userspace related),
> I again think this is a bad idea.

OK, that is debatable :-)
Comment 64 Seth Forshee 2013-03-07 13:45:01 UTC
(In reply to comment #59)
> It took me a while, but I just tested the patches on top on 3.8.2.

Thanks for testing.

> - brightness keys don't work in a tty or when no userspace daemon handles
> brightness
> - evtest /dev/input/event4 (Video Bus) sees KEY_BRIGHTNESS{UP,DOWN}
> - on Xfce with xfce4-power-manager default config 
>   - brightness is correctly changed when brightness keys are used, 
>   - 16 levels (with no smoothing) and OSD is behaving correctly

Sounds like it's working as expected.

> For comparison, on a 3.8.2 booted with acpi_osi="!Windows 2012" I get:
> 
> - brightness keys work in a tty or when no userspace daemon is running (16
> levels, no smoothing)
> - evtest /dev/input/event4 (Video Bus) sees KEY_BRIGHTNESS{UP,DOWN}
> - on Xfce with xfce4-power-manager default config
>   - brightness is correctly changed when brightness keys are used
>   - I only get 9 levels, since brightness is changed both by the kernel and by
> xfpm
>   - when disabling xfpm brightness keys handling I get 16 levels back (and I
> lose the OSD report)
> 
> In my opinion the second behavior is better since that means brightness doesn't
> depend on userspace. And I wonder if KEY_BRIGHTNESS{UP,DOWN} should be sent at
> all, since job has already been done.

The upstream maintainers requested the first behavior and probably aren't going to take the patch to quirk these machines with ascpi_osi="!Windows 2012". However, nothing prevents you from continuing to boot with this option to get the second behavior.
Comment 65 Yves-Alexis Perez 2013-03-07 18:50:20 UTC
(In reply to comment #64)
> The upstream maintainers requested the first behavior

Out of curiosity, who are the “upstream maintainers” (and why aren't they commenting here, since we're on the kernel bugzilla)

> and probably aren't going
> to take the patch to quirk these machines with ascpi_osi="!Windows 2012".
> However, nothing prevents you from continuing to boot with this option to get
> the second behavior.

Well, I can't test right now but are you sure I'll get that behavior with your patches applied? On top of that, acpi_osi might have other side effects, so it's not really a durable workaround I think.
Comment 66 Seth Forshee 2013-03-07 19:29:06 UTC
(In reply to comment #65)
> (In reply to comment #64)
> > The upstream maintainers requested the first behavior
> 
> Out of curiosity, who are the “upstream maintainers” (and why aren't they
> commenting here, since we're on the kernel bugzilla)

Here's the thread:

http://www.spinics.net/lists/linux-acpi/msg42180.html

And yeah, the term "upstream" was probably misused since we're on b.k.o ;-)

> > and probably aren't going
> > to take the patch to quirk these machines with ascpi_osi="!Windows 2012".
> > However, nothing prevents you from continuing to boot with this option to get
> > the second behavior.
> 
> Well, I can't test right now but are you sure I'll get that behavior with your
> patches applied? On top of that, acpi_osi might have other side effects, so
> it's not really a durable workaround I think.

Actually you also have to pass video.brightness_switch_enabled=1.
Comment 67 Yves-Alexis Perez 2013-03-07 20:12:57 UTC
(In reply to comment #66)
> (In reply to comment #65)
> > (In reply to comment #64)
> > > The upstream maintainers requested the first behavior
> > 
> > Out of curiosity, who are the “upstream maintainers” (and why aren't they
> > commenting here, since we're on the kernel bugzilla)
> 
> Here's the thread:
> 
> http://www.spinics.net/lists/linux-acpi/msg42180.html
> 
> And yeah, the term "upstream" was probably misused since we're on b.k.o ;-)

Thanks for the link. It's somehow weird that Henrique de Moraes Holschuh was not in the loop (I think he's subscribed to linux-acpi, but maybe not). He has a deep knowledge of Thinkpads.

Note that Matthew Garrett is wrong when he says:

> So the problem is that userspace is writing values that don't happen to 
> be aligned with the values the hardware reacts to, and so nothing gets 
> changed?

Here the value is not written by userspace but by the kernel. Not sure if it matters though.

> 
> > > and probably aren't going
> > > to take the patch to quirk these machines with ascpi_osi="!Windows 2012".
> > > However, nothing prevents you from continuing to boot with this option to get
> > > the second behavior.
> > 
> > Well, I can't test right now but are you sure I'll get that behavior with your
> > patches applied? On top of that, acpi_osi might have other side effects, so
> > it's not really a durable workaround I think.
> 
> Actually you also have to pass video.brightness_switch_enabled=1.

Good point.

The thing is, every userspace daemon does this differently, and not everyone uses a userspace daemon (nor want to). And userspace daemon only works when it's running. All in all, deffering this to userspace seems to me like a bad workaround. I think the kernel should handle this as much as possible in order for people to get a consistent behavior.

I'm not too sure if Matthew Garrett, Ben Jencks and other people involved in the thread do read this bug, but I'm open to writing this on linux-acpi if needed (I'm not subscribed but I can get a copy of the initial mail if needed).
Comment 68 Peter Weber 2013-03-09 10:20:28 UTC
I'm not competent enough to decide is technial better, but I'm working a lot on the plain terminal. Also other users may use just a Window-manager or something different.

If their is a generic way to change the brigthness (i.e. on top of Systemd, Upstart, SysV-Init) in the userspace it would be fine. At least Systemd already handles such things like LID-Switch-Behaviour, see "/etc/systemd/logind.conf". If not, I would prefer the solution which works straight on top of the kernel.

With a stack from ACPI, Kernelspace, Init-Daemon, X11 to a random choosen Desktop-Enviroment we get a lot of thinks which could break. I remember the broken hot-keys with XINPUT2 in GNOME 3.4 or the missing support for "Inhibit" the power-functions in Systemd till the recent releases of GNOME and KDE. Also X11 and a desktop are not mandatory.

Long story, short conclusion:
It should just works always.
Comment 69 Aaron Lu 2013-03-12 05:24:49 UTC
*** Bug 55071 has been marked as a duplicate of this bug. ***
Comment 70 Danny Baumann 2013-03-12 08:32:06 UTC
While my report #55071 was duplicated into this one, I'd like to state that my backlight issue is of different nature:

- The device is a Dell Inspiron 15R SE (7520) in UEFI mode.
- _BCL reports 102 levels (100, 30, and 1-100).
- I now get 2 acpi_video devices instead of previously 1.
- No matter what I set via writing to 'brightness' in either acpi_video device, reading it always yields '99'. This is - unlike the Thinkpads - also the case for values contained in the _BCL table for the non-Windows 8 case.

I can upload acpidump output again if desired.
Comment 71 Aaron Lu 2013-03-12 08:51:38 UTC
(In reply to comment #70)
> While my report #55071 was duplicated into this one, I'd like to state that my
> backlight issue is of different nature:

OK, understand.
Please feel free to describe your specific problem in the original bug page, and I hope we can have a central place to track all acpi video backlight issues related to win8 here. Thanks.
Comment 72 Alice Ferrazzi 2013-04-03 10:41:26 UTC
Created attachment 97201 [details]
dmidecode on thinkpad x201
Comment 73 Alice Ferrazzi 2013-04-03 10:50:50 UTC
Created attachment 97211 [details]
acpidump on x201
Comment 74 Aaron Lu 2013-04-03 11:41:55 UTC
Hi Alice,

Can you please let us know what is your problem? From your acpidump, I don't see you have the same problem(no win8 OSI and BCL does not claim 102 levels). Thanks.
Comment 75 Alice Ferrazzi 2013-04-04 05:26:13 UTC
yes i was using the thinkpad x201 by remote and i tried it only after attaching and actually there is no problem with brightness so is just for give more information
Comment 76 Seth Forshee 2013-04-22 17:18:24 UTC
Created attachment 99751 [details]
Disable ACPI backlight for broken ThinkPads

I'm attaching a new patch with a different take on this problem. It just disables the ACPI backlight control on these machines whenever the kernel has indicated Windows 8 compatibility via _OSI.

Most machines should fall back to using a secondary backlight control, but it's possible that a few machines won't have another control. It's also likely that brightness changes on VTs using the hotkeys will be broken. In either of these cases it will be necessary to use acpi_osi="!Windows 2012", and I'd appreciate it if someone could test this case to verify the acpi_video backlight works as before with the new patch.

Please give it a whirl and let me know your results. I've included quirks for every machine I'm aware of which is broken, but if I've missed any please let me know and include DMI information.
Comment 77 Ben Jencks 2013-04-22 19:46:43 UTC
Re: #76 patch:

Unfortunately, just setting acpi_backlight=vendor has too many side effects to completely solve the problem:
1. It causes _BCL not to be called, so the firmware won't generate ACPI events for the keypresses. Instead, the firmware will handle them entirely internally, changing the brightness. (See [1])
2. thinkpad_acpi will not mask the brightness keys, so we get hotkey events through that, in addition to the firmware's internal brightness changes.
3. thinkpad_acpi creates its "thinkpad_screen" sysfs device, which userspace prefers to the intel_backlight device that we want to be using.

I hacked up my kernel to do what I want on my machine, and I had to have acpi_video_backlight_support continue to return true, explicitly call _BCL, and return from acpi_video_device_find_cap before the acpi_video0 device was created.

Another alternative would be to allow acpi_video0 to load normally, but prefer intel_backlight from userspace (Intel's xorg driver, xf86-video-intel/src/intel_display.c, backlight_interfaces) . That doesn't sound likely, though, from some discussions I read about libbacklight preferring firmware interfaces to raw interfaces.

[1] http://sourceforge.net/mailarchive/forum.php?thread_name=51758EA5.6030405%40bjencks.net&forum_name=ibm-acpi-devel
Comment 78 Aaron Lu 2013-04-23 12:51:17 UTC
So for optimal experience, intel_backlight interface + acpi notification event handling should be used. I need to think about this.
Comment 79 Aaron Lu 2013-04-25 14:17:12 UTC
(In reply to comment #77)
> Unfortunately, just setting acpi_backlight=vendor has too many side effects to
> completely solve the problem:
> 1. It causes _BCL not to be called, so the firmware won't generate ACPI events
> for the keypresses. Instead, the firmware will handle them entirely internally,
> changing the brightness. (See [1])

Is it possible for thinkpad-acpi module to control this behaviour? I mean, on hotkey press, do not handle backlight internally, just send out event.

> 2. thinkpad_acpi will not mask the brightness keys, so we get hotkey events
> through that, in addition to the firmware's internal brightness changes.

I don't see anywhere backlight gets changed internally. Is it done by some HKEY function?

> 3. thinkpad_acpi creates its "thinkpad_screen" sysfs device, which userspace
> prefers to the intel_backlight device that we want to be using.

Does that interface work or not?
Thanks.
Comment 80 Ben Jencks 2013-04-26 07:18:52 UTC
(In reply to comment #79)
> (In reply to comment #77)
> > Unfortunately, just setting acpi_backlight=vendor has too many side effects to
> > completely solve the problem:
> > 1. It causes _BCL not to be called, so the firmware won't generate ACPI events
> > for the keypresses. Instead, the firmware will handle them entirely internally,
> > changing the brightness. (See [1])
> 
> Is it possible for thinkpad-acpi module to control this behaviour? I mean, on
> hotkey press, do not handle backlight internally, just send out event.

I see a call to _BCL in thinkpad_acpi, but it doesn't seem to be getting called for some reason when acpi_backlight=vendor. That's the only way to disable the firmware handling the backlight internally.

> > 2. thinkpad_acpi will not mask the brightness keys, so we get hotkey events
> > through that, in addition to the firmware's internal brightness changes.
> 
> I don't see anywhere backlight gets changed internally. Is it done by some HKEY
> function?

It's in \_SB.PCI0.LPC.EC._Q14 and _Q15 on my T530 firmware.

> > 3. thinkpad_acpi creates its "thinkpad_screen" sysfs device, which userspace
> > prefers to the intel_backlight device that we want to be using.
> 
> Does that interface work or not?

It doesn't appear to, for either reading or writing. I booted an unmodified kernel with acpi_backlight=vendor. thinkpad_screen/actual_brightness always returns 0. You can write any value to thinkpad_screen/brightness and it will save it and return it, but it has no effect.

The brightness keys do work through the firmware, but the actual brightness has no effect on the values thinkpad_screen returns. By default, the hotkeys are unmasked, so I get a useless on-screen-display of brightness failing to change. Masking the hotkeys takes both kernel and userspace out of the picture, so there's no OSD.
Comment 81 Igor Gnatenko 2013-04-29 08:51:19 UTC
*** Bug 57131 has been marked as a duplicate of this bug. ***
Comment 82 Igor Gnatenko 2013-04-29 17:54:56 UTC
(In reply to comment #76)
> Created an attachment (id=99751) [details]
> Disable ACPI backlight for broken ThinkPads
> 
> I'm attaching a new patch with a different take on this problem. It just
> disables the ACPI backlight control on these machines whenever the kernel has
> indicated Windows 8 compatibility via _OSI.
> 
> Most machines should fall back to using a secondary backlight control, but it's
> possible that a few machines won't have another control. It's also likely that
> brightness changes on VTs using the hotkeys will be broken. In either of these
> cases it will be necessary to use acpi_osi="!Windows 2012", and I'd appreciate
> it if someone could test this case to verify the acpi_video backlight works as
> before with the new patch.
> 
> Please give it a whirl and let me know your results. I've included quirks for
> every machine I'm aware of which is broken, but if I've missed any please let
> me know and include DMI information.
I test you patch on ThinkPad X230 (2320-5NG) and nothing changes
Comment 83 Igor Gnatenko 2013-04-29 19:13:14 UTC
Created attachment 100281 [details]
acpidump from thinkpad x230 (2320-5NG)
Comment 84 Igor Gnatenko 2013-05-06 05:24:33 UTC
I try to add acpi_osi="!Windows 2012", but backlight not working correctly.
Add acpi_osi=Linux acpi_backlight=vendor thinkpad_acpi.brightness_enable=0 works..
Comment 85 Peter Weber 2013-05-06 16:54:06 UTC
Did you removed the patch #76? The option acpi_osi="!Windows 2012" should work fine.

Anyway I just stayed with the patch "v4", which does the same.
Comment 86 Igor Gnatenko 2013-05-06 17:29:45 UTC
(In reply to comment #85)
> Did you removed the patch #76? The option acpi_osi="!Windows 2012" should work
> fine.
> 
> Anyway I just stayed with the patch "v4", which does the same.
I tested option in vanilla kernel...
Patch v4 I will try to test tomorrow.
Comment 87 Igor Gnatenko 2013-05-10 04:25:55 UTC
(In reply to comment #76)
> Created an attachment (id=99751) [details]
> Disable ACPI backlight for broken ThinkPads
> 
> I'm attaching a new patch with a different take on this problem. It just
> disables the ACPI backlight control on these machines whenever the kernel has
> indicated Windows 8 compatibility via _OSI.
> 
> Most machines should fall back to using a secondary backlight control, but it's
> possible that a few machines won't have another control. It's also likely that
> brightness changes on VTs using the hotkeys will be broken. In either of these
> cases it will be necessary to use acpi_osi="!Windows 2012", and I'd appreciate
> it if someone could test this case to verify the acpi_video backlight works as
> before with the new patch.
> 
> Please give it a whirl and let me know your results. I've included quirks for
> every machine I'm aware of which is broken, but if I've missed any please let
> me know and include DMI information.
Patch works to regulate backlight, but xbacklight always return 0.
# tree /sys/class/backlight/
/sys/class/backlight/
|-- intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-LVDS-1/intel_backlight
`-- thinkpad_screen -> ../../devices/virtual/backlight/thinkpad_screen
# xbacklight 
0.000000
# cat /sys/class/backlight/thinkpad_screen/max_brightness 
7
# cat /sys/class/backlight/thinkpad_screen/brightness 
0
But real backlight is 100%.
Comment 88 Aaron Lu 2013-06-17 02:52:27 UTC
Hi All,

Matthew has proposed a patchset to solve this problem:
https://patchwork.kernel.org/patch/2695411/
https://patchwork.kernel.org/patch/2695391/
https://patchwork.kernel.org/patch/2695401/

Please give it a test, thanks.
Comment 89 Igor Gnatenko 2013-06-17 05:28:22 UTC
(In reply to comment #88)
> Hi All,
> 
> Matthew has proposed a patchset to solve this problem:
> https://patchwork.kernel.org/patch/2695411/
> https://patchwork.kernel.org/patch/2695391/
> https://patchwork.kernel.org/patch/2695401/
> 
> Please give it a test, thanks.
Yep. This patches fixes my problem.
Comment 90 Yves-Alexis Perez 2013-06-17 10:50:59 UTC
I've tried to apply the patchset to git master and it doesn't seem to completely fine on my ThinkPad x230.

Brightness keys still don't work on console on pure X. If a power manager is running (xfce4-power-manager) it will work but it already worked on 3.9 so I'm not sure it's really an improvement. One things changes though, is that the lower brightness level really shuts the screen down.

Here's some more info:

root@balvenie:~# dmesg |grep backlight
[   32.283005] thinkpad_acpi: This ThinkPad has standard ACPI backlight brightness control, supported by the ACPI video driver
[   32.288788] thinkpad_acpi: Standard ACPI backlight interface available, not loading native one

root@balvenie:~# dmesg |grep i915
[    0.625098] i915 0000:00:02.0: enabling device (0006 -> 0007)
[    0.625750] i915 0000:00:02.0: setting latency timer to 64
[    0.638679] i915 0000:00:02.0: irq 43 for MSI/MSI-X
[    0.738217] [drm] GMBUS [i915 gmbus dpb] timed out, falling back to bit banging on pin 5
[    1.590831] i915 0000:00:02.0: fb0: inteldrmfb frame buffer device
[    1.590833] i915 0000:00:02.0: registered panic notifier
[    1.616220] [drm] Initialized i915 1.6.0 20080730 for 0000:00:02.0 on minor 0

tree /sys/class/backlight/
/sys/class/backlight/
`-- intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-LVDS-1/intel_backlight

root@balvenie:~# cat /sys/class/backlight/intel_backlight/max_brightness 
4438
Comment 91 Danny Baumann 2013-06-17 11:02:46 UTC
(In reply to comment #90)
> One things changes though, is that the
> lower brightness level really shuts the screen down.

That's because ACPI and the intel driver have different understandings what exactly 0 brightness is supposed to mean.

See the mail thread in [1] for some discussion around this. As KDE dims down to 0, it's probably even more noticable for KDE users. I sent a patch to them as well ([2]), but interest in merging it seems low.

[1] http://lists.freedesktop.org/archives/intel-gfx/2013-March/026016.html
[2] https://git.reviewboard.kde.org/r/109792/
Comment 92 Aaron Lu 2013-06-18 14:45:20 UTC
(In reply to comment #90)
> I've tried to apply the patchset to git master and it doesn't seem to
> completely fine on my ThinkPad x230.
> 
> Brightness keys still don't work on console on pure X. If a power manager is

Right, we have no way to make it work I'm afraid. The current mechanism is to let intel_backlight handle the brightness change and ACPI video handle hotkey event delivery. So when the event is sent out, if no user space tool reacts to it, then nothing would happen.

> running (xfce4-power-manager) it will work but it already worked on 3.9 so I'm

I don't understand how v3.9 could work? Are you already using some xorg.conf to specify which backlight interface to use(i.e. intel_backlight in this case)?

> not sure it's really an improvement. One things changes though, is that the
> lower brightness level really shuts the screen down.

As Danny explained, this is the behavior of intel_backlight...

Thanks for testing out.
Comment 93 Yves-Alexis Perez 2013-06-18 22:02:33 UTC
(In reply to comment #92)
> (In reply to comment #90)
> > I've tried to apply the patchset to git master and it doesn't seem to
> > completely fine on my ThinkPad x230.
> > 
> > Brightness keys still don't work on console on pure X. If a power manager is
> 
> Right, we have no way to make it work I'm afraid. The current mechanism is to
> let intel_backlight handle the brightness change and ACPI video handle hotkey
> event delivery. So when the event is sent out, if no user space tool reacts to
> it, then nothing would happen.

Can't the acpi part handling the hotkey even pass the message to intel_backlight?
> 
> > running (xfce4-power-manager) it will work but it already worked on 3.9 so I'm
> 
> I don't understand how v3.9 could work? Are you already using some xorg.conf to
> specify which backlight interface to use(i.e. intel_backlight in this case)?

I'm not. I need to double check I'm not doing anything fancy. I tested using the Debian kernel so maybe there are some patches (although it seems unlikely).
Comment 94 Aaron Lu 2013-06-19 00:44:36 UTC
(In reply to comment #93)
> (In reply to comment #92)
> > (In reply to comment #90)
> > > I've tried to apply the patchset to git master and it doesn't seem to
> > > completely fine on my ThinkPad x230.
> > > 
> > > Brightness keys still don't work on console on pure X. If a power manager is
> > 
> > Right, we have no way to make it work I'm afraid. The current mechanism is to
> > let intel_backlight handle the brightness change and ACPI video handle hotkey
> > event delivery. So when the event is sent out, if no user space tool reacts to
> > it, then nothing would happen.
> 
> Can't the acpi part handling the hotkey even pass the message to
> intel_backlight?

I'll not do this decision making thing, and you are welcome to give your ideas in the mailing list on this topic:
https://lkml.org/lkml/2013/6/9/161

> > 
> > > running (xfce4-power-manager) it will work but it already worked on 3.9 so I'm
> > 
> > I don't understand how v3.9 could work? Are you already using some xorg.conf to
> > specify which backlight interface to use(i.e. intel_backlight in this case)?
> 
> I'm not. I need to double check I'm not doing anything fancy. I tested using
> the Debian kernel so maybe there are some patches (although it seems unlikely).

If no changes are made, the ACPI video backlight interface will be picked up by user space and since it is broken, I don't see why 3.9 would work...Did you add any kernel command line like the !Windows 2012?
Comment 95 Yves-Alexis Perez 2013-06-21 09:54:58 UTC
(In reply to comment #94)
> (In reply to comment #93)
> > Can't the acpi part handling the hotkey even pass the message to
> > intel_backlight?
> 
> I'll not do this decision making thing, and you are welcome to give your ideas
> in the mailing list on this topic:
> https://lkml.org/lkml/2013/6/9/161

Ok, will try to write to the thread.

> If no changes are made, the ACPI video backlight interface will be picked up by
> user space and since it is broken, I don't see why 3.9 would work...Did you add
> any kernel command line like the !Windows 2012?

Ok, I've just tried on vanilla 3.9.7 (will attach dmesg and config just in case). Using /sys/class/backlight/acpi_video0/brightness *does* work. /s/c/b/intel_backlight/brightness works too.

Not to sure what xfpm uses, I can check if needed.
Comment 96 Yves-Alexis Perez 2013-06-21 09:56:33 UTC
Created attachment 105621 [details]
dmesg with vanilla 3.9.7
Comment 97 Yves-Alexis Perez 2013-06-21 09:57:22 UTC
Created attachment 105631 [details]
config for 3.9.7
Comment 98 Igor Gnatenko 2013-06-21 10:09:35 UTC
(In reply to comment #95)
> (In reply to comment #94)
> > (In reply to comment #93)
> > > Can't the acpi part handling the hotkey even pass the message to
> > > intel_backlight?
> > 
> > I'll not do this decision making thing, and you are welcome to give your ideas
> > in the mailing list on this topic:
> > https://lkml.org/lkml/2013/6/9/161
> 
> Ok, will try to write to the thread.
> 
> > If no changes are made, the ACPI video backlight interface will be picked up by
> > user space and since it is broken, I don't see why 3.9 would work...Did you add
> > any kernel command line like the !Windows 2012?
> 
> Ok, I've just tried on vanilla 3.9.7 (will attach dmesg and config just in
> case). Using /sys/class/backlight/acpi_video0/brightness *does* work.
> /s/c/b/intel_backlight/brightness works too.
> 
> Not to sure what xfpm uses, I can check if needed.
1. Need 3.10 kernel
2. /sys/class/backlight/acpi_video0/ shouldn't exist after patching
Comment 99 Yves-Alexis Perez 2013-06-21 10:13:42 UTC
(In reply to comment #98)
> 1. Need 3.10 kernel
> 2. /sys/class/backlight/acpi_video0/ shouldn't exist after patching

I think you missed something. The point was to say, using 3.9.7, acpi_video *does* work on my x230.
Comment 100 Igor Gnatenko 2013-06-21 10:25:37 UTC
(In reply to comment #99)
> (In reply to comment #98)
> > 1. Need 3.10 kernel
> > 2. /sys/class/backlight/acpi_video0/ shouldn't exist after patching
> 
> I think you missed something. The point was to say, using 3.9.7, acpi_video
> *does* work on my x230.
Provide
# cat /sys/class/backlight/acpi_video0/max_brightness
Comment 101 Yves-Alexis Perez 2013-06-21 10:34:26 UTC
grep . /sys/class/backlight/acpi_video0/*
/sys/class/backlight/acpi_video0/actual_brightness:100
/sys/class/backlight/acpi_video0/bl_power:0
/sys/class/backlight/acpi_video0/brightness:100
egrep: /sys/class/backlight/acpi_video0/device: Is a directory
/sys/class/backlight/acpi_video0/max_brightness:100
egrep: /sys/class/backlight/acpi_video0/power: Is a directory
egrep: /sys/class/backlight/acpi_video0/subsystem: Is a directory
/sys/class/backlight/acpi_video0/type:firmware
Comment 102 Igor Gnatenko 2013-06-21 10:41:51 UTC
(In reply to comment #101)
> grep . /sys/class/backlight/acpi_video0/*
> /sys/class/backlight/acpi_video0/actual_brightness:100
> /sys/class/backlight/acpi_video0/bl_power:0
> /sys/class/backlight/acpi_video0/brightness:100
> egrep: /sys/class/backlight/acpi_video0/device: Is a directory
> /sys/class/backlight/acpi_video0/max_brightness:100
> egrep: /sys/class/backlight/acpi_video0/power: Is a directory
> egrep: /sys/class/backlight/acpi_video0/subsystem: Is a directory
> /sys/class/backlight/acpi_video0/type:firmware
yep. you have this bug.
>max_brightness:100
not all values work. I'm right ?
Comment 103 Yves-Alexis Perez 2013-06-21 14:09:32 UTC
(In reply to comment #102)
> (In reply to comment #101)
> > grep . /sys/class/backlight/acpi_video0/*
> > /sys/class/backlight/acpi_video0/actual_brightness:100
> > /sys/class/backlight/acpi_video0/bl_power:0
> > /sys/class/backlight/acpi_video0/brightness:100
> > egrep: /sys/class/backlight/acpi_video0/device: Is a directory
> > /sys/class/backlight/acpi_video0/max_brightness:100
> > egrep: /sys/class/backlight/acpi_video0/power: Is a directory
> > egrep: /sys/class/backlight/acpi_video0/subsystem: Is a directory
> > /sys/class/backlight/acpi_video0/type:firmware
> yep. you have this bug.
> >max_brightness:100
> not all values work. I'm right ?

Yes. Working values are:

- every 5 from 5 to 70
- every 10 from 70 to 100
Comment 104 Aaron Lu 2013-06-21 15:53:06 UTC
(In reply to comment #103)
> (In reply to comment #102)
> > (In reply to comment #101)
> > > grep . /sys/class/backlight/acpi_video0/*
> > > /sys/class/backlight/acpi_video0/actual_brightness:100
> > > /sys/class/backlight/acpi_video0/bl_power:0
> > > /sys/class/backlight/acpi_video0/brightness:100
> > > egrep: /sys/class/backlight/acpi_video0/device: Is a directory
> > > /sys/class/backlight/acpi_video0/max_brightness:100
> > > egrep: /sys/class/backlight/acpi_video0/power: Is a directory
> > > egrep: /sys/class/backlight/acpi_video0/subsystem: Is a directory
> > > /sys/class/backlight/acpi_video0/type:firmware
> > yep. you have this bug.
> > >max_brightness:100
> > not all values work. I'm right ?
> 
> Yes. Working values are:
> 
> - every 5 from 5 to 70
> - every 10 from 70 to 100

Then this is a problem for people relying on GUI to do the automatic brightness adjustment, it can prevent brightness getting downwards.
Comment 105 Yves-Alexis Perez 2013-06-21 16:06:46 UTC
(In reply to comment #104)
> Then this is a problem for people relying on GUI to do the automatic brightness
> adjustment, it can prevent brightness getting downwards.

Well, that's why my position is that the kernel should handle everything itself (but I need to raise that on the mailing list)
Comment 106 Peter Weber 2013-06-28 12:58:15 UTC
I'm a little bit out-of-date, do I sume up correctly?

The past:
ACPI in-kernel handles hotkeys and passes it events to ACPI_BACKLIGHT (other drivers didn't work in this way) or userspace
The future:
ACPI in-kernel handles hotkeys and passes it events to userspace, which passes it to $RANDOM_DRIVER

If that is correct: Could probably Systemd provide a hook for brightness, similiar to HANDLELIDSWITCH?
see here: https://wiki.archlinux.org/index.php/Systemd#ACPI_power_management

NAME could be:
HANDLEBRIGHTNESS (without the extension KEY or KEYS, maybe it is a sensor or whatever...)

This would allow for a environment agnositic approach, i.e. it works out-of-the box with the TTY, X11, Wayland and the different desktops. The backlight brigthness would be modifiable with the hotkeys always. If GNOME, KDE or XFCE handles the hotkeys itself, it could "inhibit" the HANDLE and uses it own facilities for power-management.
Comment 107 Aaron Lu 2013-06-29 13:11:55 UTC
Hi Peter,

(In reply to comment #106)
> I'm a little bit out-of-date, do I sume up correctly?

Yes, almost :-)

> 
> The past:
> ACPI in-kernel handles hotkeys and passes it events to ACPI_BACKLIGHT (other
> drivers didn't work in this way) or userspace

The event is always sent to user space no matter if ACPI video driver will handle the event or not. So if ACPI video driver handles this event and user space also handles this event, the brightness level will go more steps on one hotkey press.

> The future:
> ACPI in-kernel handles hotkeys and passes it events to userspace, which passes
> it to $RANDOM_DRIVER

Yes, at least this is the case here. Since ACPI video's backlight interface is removed, the video driver will not handle brightness change on receiving an event.

> 
> If that is correct: Could probably Systemd provide a hook for brightness,
> similiar to HANDLELIDSWITCH?
> see here: https://wiki.archlinux.org/index.php/Systemd#ACPI_power_management
> 
> NAME could be:
> HANDLEBRIGHTNESS (without the extension KEY or KEYS, maybe it is a sensor or
> whatever...)
> 
> This would allow for a environment agnositic approach, i.e. it works out-of-the
> box with the TTY, X11, Wayland and the different desktops. The backlight
> brigthness would be modifiable with the hotkeys always. If GNOME, KDE or XFCE
> handles the hotkeys itself, it could "inhibit" the HANDLE and uses it own
> facilities for power-management.

I think this is a good idea.
Comment 108 Peter Weber 2013-06-29 15:37:58 UTC
Hello Aaron!
Thank you for your help and clarification!

I've proposed this to Systemd:
https://bugs.freedesktop.org/show_bug.cgi?id=66367
Comment 109 Aaron Lu 2013-07-16 06:07:39 UTC
Hi,

Rafael has revised the patches and now the following two patches are available:
https://patchwork.kernel.org/patch/2812951/ (expose OSI version)
https://patchwork.kernel.org/patch/2827793/ (win8 video quirk)

Please give it a test, thanks. Hopefully, this is the last update to this problem.
Comment 110 Yves-Alexis Perez 2013-07-16 09:15:42 UTC
Should we apply against 3.10 or mainline?
Comment 111 Rafael J. Wysocki 2013-07-16 11:29:28 UTC
3.10 should be fine although they are against the mainline technically.
Comment 112 Yves-Alexis Perez 2013-07-16 11:55:38 UTC
(In reply to Aaron Lu from comment #109)
> Hi,
> 
> Rafael has revised the patches and now the following two patches are
> available:
> https://patchwork.kernel.org/patch/2812951/ (expose OSI version)
> https://patchwork.kernel.org/patch/2827793/ (win8 video quirk)
> 
> Please give it a test, thanks. Hopefully, this is the last update to this
> problem.

Ok, so I've tried those patches on top of 3.10.1 and it seems to work just fine for me. It seems that I have brightness keys working as soon as i915 is loaded (so even before X is even started, not to mention any desktop environment power management daemon).

In /sys/class/backlight there's only intel_backlight with:

grep -r . /sys/class/backlight/intel_backlight
/sys/class/backlight/intel_backlight/type:raw
/sys/class/backlight/intel_backlight/brightness:3219
/sys/class/backlight/intel_backlight/power/control:auto
/sys/class/backlight/intel_backlight/power/async:disabled
/sys/class/backlight/intel_backlight/power/runtime_enabled:disabled
/sys/class/backlight/intel_backlight/power/runtime_active_kids:0
/sys/class/backlight/intel_backlight/power/runtime_active_time:0
grep: /sys/class/backlight/intel_backlight/power/autosuspend_delay_ms: Input/output error
/sys/class/backlight/intel_backlight/power/runtime_status:unsupported
/sys/class/backlight/intel_backlight/power/runtime_usage:0
/sys/class/backlight/intel_backlight/power/runtime_suspended_time:0
/sys/class/backlight/intel_backlight/bl_power:0
/sys/class/backlight/intel_backlight/max_brightness:4438
/sys/class/backlight/intel_backlight/actual_brightness:3219

Although I now have warnings from thinkpad_acpi about unhandled key events:

Jul 16 13:54:36 balvenie kernel: [  419.375753] thinkpad_acpi: unknown possible thermal alarm or keyboard event received
Jul 16 13:54:36 balvenie kernel: [  419.375756] thinkpad_acpi: unhandled HKEY event 0x6050
Jul 16 13:54:36 balvenie kernel: [  419.375758] thinkpad_acpi: please report the conditions when this event happened to ibm-acpi-devel@lists.sourceforge.net

each time I press one button. As far as I can tell they're harmless but Henrique would be better placed than me to assess that.
Comment 113 Rafael J. Wysocki 2013-07-16 12:06:43 UTC
Gosh, I wonder what's going on here.  Henrique?
Comment 114 Aaron Lu 2013-07-16 14:07:29 UTC
So no "unhanded key events" messages with 3.10.1 but after applying the two patches on top of v3.10.1, they appeared, right?
Comment 115 Ben Jencks 2013-07-16 14:19:58 UTC
That means _BCL was never called, so the firmware is handling the brightness changes totally internally, and not reporting the keypress events to acpi video at all. See the message linked in Comment #77, excerpted here:

> When a brightness key is pressed:
>   If the key is unmasked:
>     Report the key to thinkpad-acpi
>   If _BCL has been called:
>     Report the key (video event) to acpi video
>   Else:
>     Adjust the brightness
>     Report event 0x6050 to thinkpad-acpi
Comment 116 Yves-Alexis Perez 2013-07-16 14:31:05 UTC
(In reply to Aaron Lu from comment #114)
> So no "unhanded key events" messages with 3.10.1 but after applying the two
> patches on top of v3.10.1, they appeared, right?

I didn't try with 3.10.1 without patch yet, i'll do that now

(In reply to Ben Jencks from comment #115)
> That means _BCL was never called, so the firmware is handling the brightness
> changes totally internally, and not reporting the keypress events to acpi
> video at all. See the message linked in Comment #77, excerpted here:
> 
> > When a brightness key is pressed:
> >   If the key is unmasked:
> >     Report the key to thinkpad-acpi
> >   If _BCL has been called:
> >     Report the key (video event) to acpi video
> >   Else:
> >     Adjust the brightness
> >     Report event 0x6050 to thinkpad-acpi

Oh, then I guess it's not good for you guys? Having it handled by the firmware is fine by me though :)
Comment 117 Rafael J. Wysocki 2013-07-16 18:57:43 UTC
(In reply to Yves-Alexis Perez from comment #112)
> (In reply to Aaron Lu from comment #109)

> Although I now have warnings from thinkpad_acpi about unhandled key events:
> 
> Jul 16 13:54:36 balvenie kernel: [  419.375753] thinkpad_acpi: unknown
> possible thermal alarm or keyboard event received
> Jul 16 13:54:36 balvenie kernel: [  419.375756] thinkpad_acpi: unhandled
> HKEY event 0x6050
> Jul 16 13:54:36 balvenie kernel: [  419.375758] thinkpad_acpi: please report
> the conditions when this event happened to
> ibm-acpi-devel@lists.sourceforge.net
> 
> each time I press one button. As far as I can tell they're harmless but
> Henrique would be better placed than me to assess that.

Does this additional patch from Matthew Garrett help:

http://marc.info/?l=linux-acpi&m=137399452905372&w=2

?
Comment 118 Henrique de Moraes Holschuh 2013-07-16 23:03:30 UTC
On Tue, 16 Jul 2013, bugzilla-daemon@bugzilla.kernel.org wrote:
> Although I now have warnings from thinkpad_acpi about unhandled key events:
> 
> Jul 16 13:54:36 balvenie kernel: [  419.375753] thinkpad_acpi: unknown possible
> thermal alarm or keyboard event received
> Jul 16 13:54:36 balvenie kernel: [  419.375756] thinkpad_acpi: unhandled HKEY
> event 0x6050
> Jul 16 13:54:36 balvenie kernel: [  419.375758] thinkpad_acpi: please report
> the conditions when this event happened to ibm-acpi-devel@lists.sourceforge.net
> 
> each time I press one button. As far as I can tell they're harmless but
> Henrique would be better placed than me to assess that.

They're utterly harmless.
Comment 119 Rafael J. Wysocki 2013-07-17 00:33:07 UTC
OK, good to know, thanks.
Comment 120 Aaron Lu 2013-07-17 00:45:13 UTC
(In reply to Ben Jencks from comment #115)
> That means _BCL was never called, so the firmware is handling the brightness
> changes totally internally, and not reporting the keypress events to acpi
> video at all.

I don't get this, isn't the following code path has a call to _BCL?
thinkpad_acpi_module_init
  -> tpacpi_detect_brightness_capabilities
    -> tpacpi_check_std_acpi_brightness_support
      -> tpacpi_query_bcl_levels
Comment 121 Ben Jencks 2013-07-17 07:15:21 UTC
The tpacpi_acpi_handle_locate call in tpacpi_check_std_acpi_brightness_support is returning NULL.
Comment 122 Ben Jencks 2013-07-17 07:16:57 UTC
Oh, and I can confirm proper behavior (intel_backlight present, acpi_video absent, hotkeys handled by userspace through intel_backlight) with the two patchwork links above plus Matthew's additional patch.
Comment 123 Yves-Alexis Perez 2013-07-17 08:28:45 UTC
(In reply to Rafael J. Wysocki from comment #117)
> (In reply to Yves-Alexis Perez from comment #112)
> > (In reply to Aaron Lu from comment #109)
> 
> > Although I now have warnings from thinkpad_acpi about unhandled key events:
> > 
> > Jul 16 13:54:36 balvenie kernel: [  419.375753] thinkpad_acpi: unknown
> > possible thermal alarm or keyboard event received
> > Jul 16 13:54:36 balvenie kernel: [  419.375756] thinkpad_acpi: unhandled
> > HKEY event 0x6050
> > Jul 16 13:54:36 balvenie kernel: [  419.375758] thinkpad_acpi: please report
> > the conditions when this event happened to
> > ibm-acpi-devel@lists.sourceforge.net
> > 
> > each time I press one button. As far as I can tell they're harmless but
> > Henrique would be better placed than me to assess that.
> 
> Does this additional patch from Matthew Garrett help:
> 
> http://marc.info/?l=linux-acpi&m=137399452905372&w=2
> 
> ?
Ok, besides the fact that I guess it'll break (as intended) backlight keys without userspace daemon, it actually breaks more than that. It's not a panic as far as I can tell, however the system is locked up and I can't do anything. It can't save the logs in syslog so I can't copy/paste. I've tried pstore but nothing shows up there either. I've taken a snapshot with my phone but I'm not sure it's really readable. As I can't upload the file (even though it's under size limit), it's available there: http://molly.corsac.net/~corsac/13070010.jpg

After that screen there's some more logs printed but I can't really take everything.
Comment 124 Aaron Lu 2013-07-17 09:13:53 UTC
Please try the following patch. After Matthew's patch, the device->brightness is not NULL anymore, so a NULL pointer access occured in acpi_video_switch_brightness for device->backlight.

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index 95fa76e..4a1bb47 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -1332,7 +1332,7 @@ acpi_video_switch_brightness(struct acpi_video_device *device, int event)
        int result = -EINVAL;
 
        /* no warning message if acpi_backlight=vendor is used */
-       if (!acpi_video_backlight_support())
+       if (!acpi_video_verify_backlight_support())
                return 0;
 
        if (!device->brightness)
Comment 125 Rafael J. Wysocki 2013-07-17 10:19:01 UTC
OK, so we need to rework this again.

I think acpi_video_device_find_cap() should remove device->brightness if acpi_video_verify_backlight_support() returns false.
Comment 126 Aaron Lu 2013-07-17 10:52:11 UTC
(In reply to Ben Jencks from comment #121)
> The tpacpi_acpi_handle_locate call in
> tpacpi_check_std_acpi_brightness_support is returning NULL.

I don't see why it failed...There are at least two ACPI devices that should have VIDEO HID: \_SB.PCI0.VID and \_SB.PCI0.PEG.VID. Don't know what's going wrong.
Comment 127 Rafael J. Wysocki 2013-07-17 10:59:16 UTC
Created attachment 106901 [details]
ACPI / video: Remove brightness object if we're not going to use it

Yves-Alexis, can you please try this patch on top of the Matthew's one?
Comment 128 Yves-Alexis Perez 2013-07-17 12:44:11 UTC
(In reply to Rafael J. Wysocki from comment #127)
> Created attachment 106901 [details]
> ACPI / video: Remove brightness object if we're not going to use it
> 
> Yves-Alexis, can you please try this patch on top of the Matthew's one?

I'm not too sure here. Should I try:

- 3.10
+ two patches from comment #109
+ patch from comment #124
+ patch from comment #127

?
Comment 129 Aaron Lu 2013-07-17 12:52:05 UTC
I think Rafael means:
- 3.10
+ two patches from comment #109
+ patch from Matthew: http://marc.info/?l=linux-acpi&m=137399452905372&w=2
+ patch from comment #127
Thanks.
Comment 130 Yves-Alexis Perez 2013-07-17 13:19:37 UTC
(In reply to Aaron Lu from comment #129)
> I think Rafael means:
> - 3.10
> + two patches from comment #109
> + patch from Matthew: http://marc.info/?l=linux-acpi&m=137399452905372&w=2
> + patch from comment #127
> Thanks.

Ok, with those, I don't have the recursive fault nor the thinkpad-acpi warnings.

I get the “crippled” behavior: no brightness keys without userspace daemon. With a userspace daemon (xfce4-power-manager) it works fine (I get only 10 brightness levels but that's will indeed depend on what daemon you use).

I still get a warning in the logs though:

Jul 17 15:16:51 balvenie kernel: [  182.606930] ACPI: Failed to switch the brightness

which I guess is expected since ACPI doesn't handle the brightness.
Comment 131 Aaron Lu 2013-07-17 14:23:29 UTC
With an additional patch in comment #124, I think that warning will go away. Possible to give it a test?

Rafael, what do you think of the patch in comment #124? If you think it is worth, it can be merged into your patch which introduced the acpi_video_verify_backlight_support function.
Comment 132 Rafael J. Wysocki 2013-07-17 22:38:52 UTC
Yeah, makes sense.  I'll fold in into https://patchwork.kernel.org/patch/2827793/ .
Comment 133 Aaron Lu 2013-07-22 01:13:06 UTC
Patches in Linus' tree.

commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Date:   Thu Jul 18 02:08:06 2013 +0200

    ACPI / video / i915: No ACPI backlight if firmware expects Windows 8

commit c04c697cf1fe8f0962ccd3c2392a9b637a5307aa
Author: Matthew Garrett <matthew.garrett@nebula.com>
Date:   Tue Jul 16 17:08:16 2013 +0000

    ACPI / video: Always call acpi_video_init_brightness() on init

commit 242b2287cd7f27521c8b54a4101d569e53e7a0ca
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Jul 2 21:59:10 2013 +0800

    ACPICA: expose OSI version
Comment 134 Aaron Lu 2013-07-26 06:29:27 UTC
Alternatively, I suppose by using the Xorg.conf to specify backlight interface should also work(for GUI environment only)?

$ cat /etc/Xorg/Xorg.conf
Section "Device"
        Option     "Backlight"	"intel_backlight"
	Identifier  "Card0"
	Driver      "intel"
	BusID       "PCI:0:2:0"
EndSection
Comment 135 Rafael J. Wysocki 2013-07-27 14:39:00 UTC
Commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255 had to be reverted, because it introduced backlight control regressions on multiple systems.
Comment 136 Felipe Contreras 2013-08-02 06:55:40 UTC
I think it's pretty clear we need to go for the initial approach right now (blacklisting machines to boot with !Windows 2012).

It has been several months that backlight is broken, and there's no point in waiting many more for the "perfect" solution.
Comment 137 Felipe Contreras 2013-08-02 18:47:15 UTC
I created a new bug report (bug 60682) with a blacklist patch, not only Thinkpads are affected by this.

This is the list I've gathered so far:

* ASUS Zenbook Prime UX31A
* ASUS N56VZ
* Lenovo ThinkPad L430
* Lenovo ThinkPad T430s
* Lenovo ThinkPad T530
* Lenovo ThinkPad W530
* Lenovo ThinkPad X1 Carbon
* Lenovo ThinkPad X230
* Lenovo ThinkPad Edge E330

If your machine works correctly with acpi_osi="!Windows 2012", and doesn't otherwise, comment with the name of the laptop, plus the output of this command:

% dmesg | grep DMI

Add a comment in bug 60682.
Comment 138 Aaron Lu 2013-09-25 06:24:01 UTC
FYI, patches posted to:
https://lkml.org/lkml/2013/9/24/157

I have a branch for it:
https://github.com/aaronlu/linux acpi_video_win8
Comment 139 Peter Weber 2013-09-29 09:59:29 UTC
Maybe I can test the patches within the next days. But I can't promise. Is it possible, that we can see this patches in kernel 3.13? Merge window for 3.12 already passed.
Comment 140 Aaron Lu 2013-10-08 02:55:23 UTC
(In reply to Peter Weber from comment #139)
> Maybe I can test the patches within the next days. But I can't promise. Is
> it possible, that we can see this patches in kernel 3.13? Merge window for
> 3.12 already passed.

That depends on if it causes regressions for other working systems.
Comment 141 Peter Weber 2013-10-09 14:48:40 UTC
Thanks. Can you add the patches to this bug as attachment?
Comment 142 Aaron Lu 2013-10-16 02:00:51 UTC
Created attachment 111191 [details]
ACPI video backlight win8 patches

v5 of the patchset that is sent to LKML:
http://lkml.org/lkml/2013/10/11/409

Please give it a test and if the video.use_native_backlight=1 works for you, leave your DMI info here, thanks.
Comment 143 Peter Weber 2013-10-19 15:44:58 UTC
Created attachment 111581 [details]
dmidecode video.use_native_backlight=1

kernel 3.12.0-rc5 with patch_set_v5
Comment 144 Peter Weber 2013-10-19 15:45:38 UTC
Created attachment 111591 [details]
dmidecode video.use_native_backlight -> not set

kernel 3.12.0-rc5 with patch_set_v5
Comment 145 Peter Weber 2013-10-19 15:47:07 UTC
Created attachment 111601 [details]
kernel config, kernel 3.12.0-rc5 with patch_set_v5
Comment 146 Peter Weber 2013-10-19 15:58:00 UTC
Thanks for your work on this Aaron! I've upgraded to kernel 3.12.0-rc5 and applied the patches. When I set video.use_native_backlight=1:

* everything works fine!
* /sys/class/backlight contains only intel_backlight
* there seem to be 21 brightness-levels available with gnome-power-manager
* the lowest level turns the backlight completely off (this doesn't happend with acpi_osi="!Windows 2012"), is this intentional?
* as already expected it is not longer possible to tune the brightness-level with the Fn+Keys, while working on a virtual-terminal
* thinkpad_acpi seem unhappy about the brightness-interface (see dmesg)

> thinkpad_acpi: http://ibm-acpi.sf.net/
> thinkpad_acpi: Unsupported brightness interface, please contact ibm-acpi-devel@lists.sourceforge.net


Looks good.
Comment 147 Peter Weber 2013-10-19 16:00:49 UTC
Created attachment 111611 [details]
dmesg with complain from thinkpad_acpi about brightness interface

unsupported brightness interface
Comment 148 Tom Wijsman 2013-10-19 16:10:25 UTC
Peter: Can you change this bug from NEEDINFO to UNCONFIRMED or CONFIRMED? I don't know for sure; but, due to the high traffic, the assigned person might miss your responses if the bug doesn't appear back in his list. It might be the possibility that only the assigned person can change, in which case never mind...
Comment 149 Peter Weber 2013-10-19 18:24:40 UTC
Ohhhh. I thought the active developers change that if appropriate. I changed the status to VERIFIED, because there is no CONFIRMED available.
Comment 150 Tom Wijsman 2013-10-19 18:33:26 UTC
Eh, VERIFIED is usually a status to mark it resolved and verified by QA; you'll not want to pick that one, see https://bugzilla.kernel.org/page.cgi?id=fields.html#status for details on the various statuses.
Comment 151 Peter Weber 2013-10-19 20:18:05 UTC
The naming is pretty weird, isn't it? I don't have the canconfirm-permission. I hesitate to set this bug to "RESOLVED" but the description fits. Maybe Aaron would set this to IN_PROGRESS.
Comment 152 Aaron Lu 2013-10-21 00:29:26 UTC
Thanks Peter for the test. If you show me your dmi info, I can add your system into a list so that the mentioned param will be automatically set for you.
Comment 153 Theodore Tso 2013-10-21 00:44:20 UTC
I can confirm that I also need video.use_native_backlight=1 plus this patch (applied to 3.12-rc5) in order to make things work on my T430s laptop:

Handle 0x000F, DMI type 1, 27 bytes
System Information
        Manufacturer: LENOVO
        Product Name: 2352CTO
        Version: ThinkPad T430s
        Wake-up Type: Power Switch
        SKU Number: LENOVO_MT_2352
        Family: ThinkPad T430s
Comment 154 Aaron Lu 2013-10-21 01:34:26 UTC
Created attachment 111821 [details]
Add Thinkpad T430s into the list

For Thinkpad T430s, apply on top of the existing v5 patchset.
Comment 155 Peter Weber 2013-10-23 08:02:40 UTC
(In reply to Aaron Lu from comment #152)
> Thanks Peter for the test. If you show me your dmi info, I can add your
> system into a list so that the mentioned param will be automatically set for
> you.

Theodore posted it in meanwhile :-)
You can find the complete output in comment 143.  I will add the patch on top of v5 within the next days, but doesn't expect any surprise.
Comment 156 Peter Weber 2013-10-23 22:55:46 UTC
Created attachment 112141 [details]
fixed typo, ThinkPad is written in camelCase, Thinkpad -> ThinkPad
Comment 157 Peter Weber 2013-10-23 22:59:02 UTC
Created attachment 112151 [details]
fixed typo, Thinkpad -> ThinkPad

ThinkPad brand name is written in CamelCaseas, as reported by "dmidecode -t 1".
Comment 158 Peter Weber 2013-10-23 23:07:10 UTC
Great! I'm myself not able to type CamelCase without a typo. To be clear, is necessary to change it to "ThinkPad" to make it working!

By the way, let me be pedantic :-)
0003-ACPI-video-Do-not-register-backlight-if-win8-and-nat.patch
Line 54:
- * For Windows 8 systems: if set ture and the GPU driver has
+ * For Windows 8 systems: if set true and the GPU driver has


Thank you! Good night!
Comment 159 Kevin Smith 2013-10-27 18:15:35 UTC
I tried the v5 patchset on 3.12.0-rc5 on my Lenovo Yoga 13, and video.use_native_backlight=1 also fixes the issue on this machine.  I'm glad to help with any future testing needed to get this patch merged.  Thanks for fixing this!
Comment 160 Aaron Lu 2013-10-28 02:18:24 UTC
*** Bug 63811 has been marked as a duplicate of this bug. ***
Comment 161 Aaron Lu 2013-12-20 00:57:00 UTC
*** Bug 67031 has been marked as a duplicate of this bug. ***
Comment 162 Aaron Lu 2014-01-28 00:39:45 UTC
*** Bug 68751 has been marked as a duplicate of this bug. ***
Comment 163 Aaron Lu 2014-01-28 07:10:38 UTC
*** Bug 62941 has been marked as a duplicate of this bug. ***
Comment 164 Aaron Lu 2014-02-19 01:22:42 UTC
commit 1811fcb029fa3eca5bd1d25eab386b0c4b80fb93
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Feb 18 13:54:20 2014 +0800

    ACPI / video: Add systems that should favour native backlight interface

has entered Rafael's linux-next tree.
Comment 165 edm 2014-02-22 20:48:29 UTC
Hi,
Thinkpad T430 here (without the s), I'm using linux 3.13.4 and systemd 208 from arch linux booting in bios mode with the last bios update (2.62).

The bug seems the same as for T430s and I can "fix" it with acpi_osi='!Windows 2012' or using video.use_native_backlight=1 as kernel command line in grub.

It works kinda well with video.use_native_backlight=1 except that I have now 4437 level of brightness in /sys/class/backlight/intel_backlight and as reported by Weber, systemd complains about this:

[    9.226063] thinkpad_acpi: Unsupported brightness interface, please contact ibm-acpi-devel@lists.sourceforge.net


System Information
        Manufacturer: LENOVO
        Product Name: 2349KDG
        Version: ThinkPad T430
        (...)
        Wake-up Type: Power Switch
        SKU Number: LENOVO_MT_2349
        Family: ThinkPad T430

It is because of so many levels of backlight that I have in KDE this behavior switching brightness with the keys:
from 0 to 100: 0-10-20-30-40-50-60-70-80-90-100
from 100 to 0: 100-89-79-69...etc.
Comment 166 Aaron Lu 2014-03-04 02:58:15 UTC
(In reply to edm from comment #165)
> Hi,
> Thinkpad T430 here (without the s), I'm using linux 3.13.4 and systemd 208
> from arch linux booting in bios mode with the last bios update (2.62).
> 
> The bug seems the same as for T430s and I can "fix" it with
> acpi_osi='!Windows 2012' or using video.use_native_backlight=1 as kernel
> command line in grub.
> 
> It works kinda well with video.use_native_backlight=1 except that I have now
> 4437 level of brightness in /sys/class/backlight/intel_backlight and as
> reported by Weber, systemd complains about this:
> 
> [    9.226063] thinkpad_acpi: Unsupported brightness interface, please
> contact ibm-acpi-devel@lists.sourceforge.net

This is harmless, I'll send a patch to fix this.

> 
> 
> System Information
>         Manufacturer: LENOVO
>         Product Name: 2349KDG
>         Version: ThinkPad T430
>         (...)
>         Wake-up Type: Power Switch
>         SKU Number: LENOVO_MT_2349
>         Family: ThinkPad T430
> 
> It is because of so many levels of backlight that I have in KDE this
> behavior switching brightness with the keys:
> from 0 to 100: 0-10-20-30-40-50-60-70-80-90-100
> from 100 to 0: 100-89-79-69...etc.

I'll prepare a new patch for your laptop later(together with some other model once that model is confirmed to work well with video.use_native_backlight=1).
Comment 167 Aaron Lu 2014-03-04 03:00:39 UTC
commit 0e9f81d3b7cd0649a3bc437391b6a0650f98f844
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Feb 18 13:54:20 2014 +0800

    ACPI / video: Add systems that should favour native backlight interface

entered Linus' tree as v3.14 material.
Comment 168 Aaron Lu 2014-03-04 03:01:14 UTC
BTW, edm, please also attach your acpidump here, thanks.
Comment 169 edm 2014-03-04 10:28:17 UTC
Created attachment 127991 [details]
acpidump T430

Output of 
# acpidump > acpidump-T430.txt
Comment 170 edm 2014-03-04 10:31:43 UTC
Just a stupid question: does it make any difference doing "acpidump" with or without acpi_osi="!Windows 2012" as boot option?
Comment 171 Aaron Lu 2014-04-08 02:58:59 UTC
(In reply to edm from comment #170)
> Just a stupid question: does it make any difference doing "acpidump" with or
> without acpi_osi="!Windows 2012" as boot option?

No difference.
I'll add your system into the table shortly, sorry for the long delay.
Comment 172 Stephen Chandler Paul 2014-04-08 12:24:07 UTC
Created attachment 131691 [details]
acpi dump for ThinkPad Helix (UEFI)
Comment 173 Stephen Chandler Paul 2014-04-08 12:26:09 UTC
Created attachment 131701 [details]
dmidecode dump for ThinkPad Helix

That should be all the information you need on the Helix regarding the patch I submitted ( https://patchwork.kernel.org/patch/3938911/ ), let me know if you need anything else
Comment 174 Aaron Lu 2014-04-28 03:25:50 UTC
(In reply to Aaron Lu from comment #171)
> (In reply to edm from comment #170)
> > Just a stupid question: does it make any difference doing "acpidump" with or
> > without acpi_osi="!Windows 2012" as boot option?
> 
> No difference.
> I'll add your system into the table shortly, sorry for the long delay.

Matthew sent out a patch to enable native backlight control interface by default for win8 systems so that we do not need to add more systems into the DMI table:
http://www.spinics.net/lists/dri-devel/msg57228.html
Comment 175 Gabriele Mazzotta 2014-06-01 13:42:52 UTC
Created attachment 137791 [details]
Dell XPS13 9333 acpidump

I attached the acpidump of my Dell XPS13 9333.

If video.use_native_backlight=1 is not added to the kernel cmdline, the Fn keys are buggy: the brightness increases or decreases by one unit (max_brigthness is 937, so a variation of 1 is hardly noticeable) and I get not DE OSD.

With video.use_native_backlight=1, I get the OSD and a brightness variations of 10% as expected.



System Information
        Manufacturer: Dell Inc.
        Product Name: XPS13 9333
Comment 176 Robert Zavalczki 2014-06-22 18:18:57 UTC
Created attachment 140631 [details]
ThinkPad T530 use native backlight patch
Comment 177 Robert Zavalczki 2014-06-22 18:22:43 UTC
The backlight keys do not work properly with kernel 3.15.1 and ThinkPad T530 (with Intel integrated graphics). Only 2 brightness levels are available.

I noticed that using native backlight option for Thinkpad T530 fixes the issue:

see attachment 140631 [details]

After applying the above patch 11 brightness levels + blank can be set (I am using an Xfce 4.10 desktop).

Weirdly, in pre 3.15 kernel versions adding the use native backlight option caused my monitor to become unresponsive (stay blank) after a screen lock followed by a suspend screen. I no longer get that behavior with kernel version 3.15.1. Should you need any more information I can happily provide it.
Comment 178 Yves-Alexis Perez 2014-09-10 15:28:28 UTC
So I just got the 3.16 kernel with the changes for all Windows 8 laptops.

And while I bought this laptop (Thinkpad x201s) in 2010, it seems a BIOS update added the “support” for Windows 8 (at least that's a guess from http://paste.debian.net/120230/ , I can provide full acpidump).

So this laptop where brightness keys where working perfectly using ACPI, has now the same issue, and I need to have something in userspace doing the job.

That's really a regression for me.
Comment 179 Aaron Lu 2014-09-11 05:53:04 UTC
Hi Perez,

I believe commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d is meant to fix such problems:

commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d
Author: Hans de Goede <hdegoede@redhat.com>
Date:   Thu Aug 28 10:20:46 2014 +0200

    ACPI / video: Add a disable_native_backlight quirk
    
    Some laptops have a working acpi_video backlight control, and using native
    backlight on these causes a regression where backlight control does not work
    when userspace is not handling brightness key events. Disable native_backlight
    on these to fix this.
    
    Link: https://bugzilla.kernel.org/show_bug.cgi?id=81691
    Reported-and-tested-by: Andre Müller <andre.muller@web.de>
    Cc: 3.16+ <stable@vger.kernel.org> # 3.16+
    Signed-off-by: Hans de Goede <hdegoede@redhat.com>
    Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Feel free to submit a patch to add your model into the video_dmi_table to disable native backlight control.
Comment 180 Yves-Alexis Perez 2014-09-11 06:05:51 UTC
(In reply to Aaron Lu from comment #179)
> Hi Perez,
> 
> I believe commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d is meant to fix
> such problems:
> 
> commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d
> Author: Hans de Goede <hdegoede@redhat.com>
> Date:   Thu Aug 28 10:20:46 2014 +0200
> 
>     ACPI / video: Add a disable_native_backlight quirk
>     
>     Some laptops have a working acpi_video backlight control, and using
> native
>     backlight on these causes a regression where backlight control does not
> work
>     when userspace is not handling brightness key events. Disable
> native_backlight
>     on these to fix this.
>     
>     Link: https://bugzilla.kernel.org/show_bug.cgi?id=81691
>     Reported-and-tested-by: Andre Müller <andre.muller@web.de>
>     Cc: 3.16+ <stable@vger.kernel.org> # 3.16+
>     Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Feel free to submit a patch to add your model into the video_dmi_table to
> disable native backlight control.

Thanks. So the “DMI table” approach was considered bad because not sustainable for this bug, but ok for bug #81691 ?

Anyway, will provide the correct DMI information on that bug.
Comment 181 Aaron Lu 2014-09-11 07:40:59 UTC
(In reply to Yves-Alexis Perez from comment #180)
> (In reply to Aaron Lu from comment #179)
> > Hi Perez,
> > 
> > I believe commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d is meant to fix
> > such problems:
> > 
> > commit 5f24079b021cd3147c8d24ba65833f7a0df7e80d
> > Author: Hans de Goede <hdegoede@redhat.com>
> > Date:   Thu Aug 28 10:20:46 2014 +0200
> > 
> >     ACPI / video: Add a disable_native_backlight quirk
> >     
> >     Some laptops have a working acpi_video backlight control, and using
> > native
> >     backlight on these causes a regression where backlight control does not
> > work
> >     when userspace is not handling brightness key events. Disable
> > native_backlight
> >     on these to fix this.
> >     
> >     Link: https://bugzilla.kernel.org/show_bug.cgi?id=81691
> >     Reported-and-tested-by: Andre Müller <andre.muller@web.de>
> >     Cc: 3.16+ <stable@vger.kernel.org> # 3.16+
> >     Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> >     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > Feel free to submit a patch to add your model into the video_dmi_table to
> > disable native backlight control.
> 
> Thanks. So the “DMI table” approach was considered bad because not
> sustainable for this bug, but ok for bug #81691 ?

The systems that claim to be Win8 but have a working ACPI video interface are supposed to be not common(hopefully this is true), so should be OK to keep a DMI table for them. While the systems that claim to be Win8 and don't have a working ACPI video interface are supposed to be the common case, so keeping a DMI table for all of them is not feasible.

> 
> Anyway, will provide the correct DMI information on that bug.

OK, thanks.

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