Bug 9262 - Brightness hotkeys broken with Toshiba laptop
Summary: Brightness hotkeys broken with Toshiba laptop
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Alexey Starikovskiy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-10-30 05:33 UTC by Danny Baumann
Modified: 2008-09-16 00:11 UTC (History)
2 users (show)

See Also:
Kernel Version: 2.6.23.1
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
acpidump output (171.31 KB, text/plain)
2007-10-30 05:44 UTC, Danny Baumann
Details
Test patch by Alexey Starikovskiy that restores brightness buttons functionality. (327 bytes, patch)
2007-10-30 10:28 UTC, Danny Baumann
Details | Diff
Output in /var/log/messages when pressing brightness buttons (1.95 KB, text/plain)
2007-10-30 10:52 UTC, Danny Baumann
Details
Full dmesg output (24.13 KB, text/plain)
2007-10-30 11:04 UTC, Danny Baumann
Details
Do early init only for fake _INI being present (1.29 KB, patch)
2007-10-30 13:19 UTC, Alexey Starikovskiy
Details | Diff
typo fixed (1.35 KB, patch)
2007-11-14 10:08 UTC, Alexey Starikovskiy
Details | Diff
try the debug patch to see whether the system will be affected by this patch (3.32 KB, patch)
2008-08-28 02:06 UTC, ykzhao
Details | Diff

Description Danny Baumann 2007-10-30 05:33:05 UTC
Most recent kernel where this bug did not occur: 2.6.23-rc2
Distribution: Fedora 8
Hardware Environment: Toshiba Satellite Pro A100 laptop (C2D T5500, NV7600Go, ...)
Problem Description:
From kernel 2.6.23-rc3 onwards, I am no longer able to adjust the brightness of my laptop screen with the Fn+F6/7 keys. 2.6.23-rc3 will just do nothing when pressing them, 2.6.23-rc4 onwards shows up F6/F7 key events when pressing those hotkeys.

Manually adjusting the brightness by writing to /proc/acpi/video/VGA/LCD/brightness still works.

Manually reverting the embedded controller driver (ec.c) to 2.6.23-rc2 state makes backlight control work again, although key events still are sent. Using git head ec.c doesn't work.

Please tell me if any additional information is needed.
Comment 1 Danny Baumann 2007-10-30 05:44:34 UTC
Created attachment 13335 [details]
acpidump output
Comment 2 Danny Baumann 2007-10-30 10:28:45 UTC
Created attachment 13347 [details]
Test patch by Alexey Starikovskiy that restores brightness buttons functionality.
Comment 3 Alexey Starikovskiy 2007-10-30 10:39:14 UTC
Could you please check 2.6.24-rc1 version?
>> Please check if attached patch changes situation.

> Yes, it does. With that patch, everything works as intended.
> Gnome-power-manager doesn't seem to work properly with it, but that's a
> different story  ;-) 
I fear it's related. dmesg would be helpful.
Comment 4 Danny Baumann 2007-10-30 10:49:55 UTC
(In reply to comment #3)
> Could you please check 2.6.24-rc1 version?
> >> Please check if attached patch changes situation.
> 
> > Yes, it does. With that patch, everything works as intended.
> > Gnome-power-manager doesn't seem to work properly with it, but that's a
> > different story  ;-) 
> I fear it's related. dmesg would be helpful.

Maybe my statement was confusing: gpm doesn't work properly with both the -rc2 kernel and your patch. If shows that pretty OSD-like thingy, but won't allow me to adjust the brightness to more than 50%. Trying to increase the brightness further will only result in a short flicker. I will try to build gpm with debug output to see what's going on.

dmesg shows nothing while pressing brightness hotkeys (or do you want the full dmesg output since kernel startup?), will attach the output in /var/log/messages.
Comment 5 Danny Baumann 2007-10-30 10:52:17 UTC
Created attachment 13348 [details]
Output in /var/log/messages when pressing brightness buttons

This is the output in /var/log/messages for a single Brightness-Down press followed by a single Brightness-Up press.
Obviously (incorrectly?) two ACPI events are sent per button press.
Comment 6 Danny Baumann 2007-10-30 11:02:05 UTC
(In reply to comment #3)
> Could you please check 2.6.24-rc1 version?

Of ec.c or the full kernel? For the former, I am already trying the Git HEAD version. For the latter, I will try ASAP; which probably will be after F8 release.   If you could tell me which subset also needs updates, I would really appreciate that.
Comment 7 Danny Baumann 2007-10-30 11:04:29 UTC
Created attachment 13349 [details]
Full dmesg output

Full dmesg output after start.

About the Nvidia driver: The situation is completely unchanged when unloading the Nvidia module and using the nv X driver.
Comment 8 Alexey Starikovskiy 2007-10-30 11:28:00 UTC
ok, for gpm issue this patch might help:
commit 63f0edfc0b7f8058f9d3f9b572615ec97ae011ba
Author: Alexey Starikovskiy <astarikovskiy@suse.de>
Date:   Mon Sep 3 16:30:08 2007 +0400

    ACPI: VIDEO: Adjust current level to closest available one.

    Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
    Signed-off-by: Len Brown <len.brown@intel.com>
Comment 9 Alexey Starikovskiy 2007-10-30 12:09:26 UTC
rc3 was kind of broken for handling _Qxx events, see bug 8886, patch went into rc4, so the current state is: Fn-F6/F7 brightness events turned into F6/F7 keyboard events between rc2 and rc4, not doing early init of EC helps. 
Comment 10 Alexey Starikovskiy 2007-10-30 13:19:59 UTC
Created attachment 13354 [details]
Do early init only for fake _INI being present

Please check this patch
Comment 11 Danny Baumann 2007-10-31 02:39:34 UTC
(In reply to comment #10)
> Created an attachment (id=13354) [details]
> Do early init only for fake _INI being present
> 
> Please check this patch

It works fine.

The patch from comment #8 doesn't help for gpm, though. It seems like the gpm issue is related to two ACPI events being sent per key press. Gpm tries to set the brightness itself on the second event (it doesn't act on the first one) to a value not listed in the valid value table (_BCL) which causes the brightness to drop to the lowest level (driven by the firmware I guess). Maybe the new brightness should be validated in acpi_video_device_set_level (so the validation should be moved from acpi_video_device_write_brightness to this function)? If desired, I can provide a patch for that - I will play around with that anyway.
Comment 12 Danny Baumann 2007-10-31 02:40:57 UTC
Additional question: The brightness change notify events are driven by the firmware, right? So there's no chance to fix the duplicate events besides a custom DSDT, is it?
Comment 13 Danny Baumann 2007-11-02 06:24:22 UTC
(In reply to comment #11)
> The patch from comment #8 doesn't help for gpm, though. It seems like the gpm
> issue is related to two ACPI events being sent per key press. Gpm tries to
> set
> the brightness itself on the second event (it doesn't act on the first one)
> to
> a value not listed in the valid value table (_BCL) which causes the
> brightness
> to drop to the lowest level (driven by the firmware I guess). Maybe the new
> brightness should be validated in acpi_video_device_set_level (so the
> validation should be moved from acpi_video_device_write_brightness to this
> function)? If desired, I can provide a patch for that - I will play around
> with
> that anyway.

I posted a patch for that issue to bug 9277: http://bugzilla.kernel.org/show_bug.cgi?id=9277
Comment 14 Zhang Rui 2007-11-12 22:55:58 UTC
Re comment#12, the duplicate events is caused by duplicate video buses in your DSDT. Event is sent to each of the video bus and that's why you got two acpi notifications for every hotkey pressing.

We are still looking for a solution of the duplicate video bus device issue.
Comment 15 Alexey Starikovskiy 2007-11-14 10:08:22 UTC
Created attachment 13546 [details]
typo fixed

Len, please send this one up.
Comment 16 Len Brown 2007-11-18 22:47:58 UTC
27792f3af1f67057fc17e4bd3b28d0ce8fe1d15c
(ACPI: EC: Don't init EC early if it has no _INI)
applied to acpi test branch
Comment 17 Len Brown 2007-12-03 22:39:44 UTC
shipped in linux-2.6.24-rc4
closed.
Comment 18 ykzhao 2008-08-28 02:06:16 UTC
Created attachment 17499 [details]
try the debug patch to see whether the system will be affected by this patch

Hi, Danny
  Will you please try the attached patch and see whether the system will be affected by it?
  Thanks.
Comment 19 Danny Baumann 2008-09-03 23:12:11 UTC
(In reply to comment #18)
> Created an attachment (id=17499) [details]
> try the debug patch to see whether the system will be affected by this patch
> 
> Hi, Danny
>   Will you please try the attached patch and see whether the system will be
> affected by it?
>   Thanks.

Will do ASAP. Which tree (and additional patches) do I need to test this one?
Comment 20 Danny Baumann 2008-09-08 08:09:15 UTC
(In reply to comment #19)
> (In reply to comment #18)
> > Created an attachment (id=17499) [details] [details]
> > try the debug patch to see whether the system will be affected by this
> patch
> > 
> > Hi, Danny
> >   Will you please try the attached patch and see whether the system will be
> > affected by it?
> >   Thanks.
> 
> Will do ASAP. Which tree (and additional patches) do I need to test this one?

I tried the patch on a snapshot of 2.6.27-rc5 in Linus' tree (commit d210baf53b699fc61aa891c177b71d7082d3b957). Brightness hotkeys continue to work just fine.
Comment 21 ykzhao 2008-09-16 00:11:00 UTC
Hi, Danny
   thanks for the test.
   It is very lucky that your system won't be affected by the patch in comment #18.
   Thanks again.

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