Bug 13210 - General issues with acpi brightness keys on aspire 5720
Summary: General issues with acpi brightness keys on aspire 5720
Status: CLOSED INVALID
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: All Linux
: P1 low
Assignee: Zhang Rui
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-04-29 16:33 UTC by Maxim Levitsky
Modified: 2009-05-16 02:53 UTC (History)
6 users (show)

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


Attachments

Description Maxim Levitsky 2009-04-29 16:33:04 UTC
In general that notebook has very broken implementation of acpi video interface.

Thus probably it is best just to use acer-wmi for it, and it does work perfectly.

But I once noticed that acer-wmi won't load and demand to use generic acpi
video interface. I eventually discovered that, if you don't include acpi video in the kernel (disabled in config), then it works.


Anyway, I would like to discuss, and probably improve the acpi video driver, for less broken notebooks (and this probably will involve fixing gnome-power-manager/hal as well)

Anyway here is the mess I have:


* Hardware controls brightness internally, probably using EC code.
* Yet, it reports brightness keys as normal keyboard keys
* It also reports brightness up/down acpi events, it does so only when acpi 'video' driver is loaded - tested by doing cat /proc/acpi/event.

* Acpi events cause video driver to set brightness, and also report 'em on separate event interface.


Thus a single (say brightness up) event results in brightness going 4 levels up:

1 due to hardware change, 1 for acpi video driver automatic chager, 1 by gpm listening to acpi event device, and finally 1 due to normal keypress.
Comment 1 Zhang Rui 2009-04-30 01:28:19 UTC
please attach the output of "grep . /sys/firmware/acpi/interrupts/*" before and after pressing the hotkey.
Comment 2 ykzhao 2009-04-30 02:09:38 UTC
Will you please attach the output of acpidump?
Will you please try the boot option of "acpi_backlight=vendor" and see whether it is helpful?
    Thanks.
Comment 3 Zhang Rui 2009-04-30 02:50:34 UTC
Yakui,
acpidump can be gotten from bug #13121
and your suggestion is not helpful for this bug.

Now, I think we should make clear
1. if the brightness is changed by BIOS or by AML code.
2. what does gpm use for brightness control, /sys/class/backlight/.../brightness vs actual_brightness

(In reply to comment #0)
> 
> Thus a single (say brightness up) event results in brightness going 4 levels
> up:
> 
> 1 due to hardware change,
agree

> 1 for acpi video driver automatic chager,
agree, this can be enabled/disable via brightness_switch_enable.

> 1 by gpm listening to acpi event device, and finally 1 due to normal
> keypress.

I'm confused here.

the "finally 1 due to normal keypress", you mean the input event for the keypress, right?
how does gpm catch acpi event? or catch what kind of events?
I think these two are the same.

IMO, there are three parts in all, when pressing a hotkey
1. an ACPI interrupt is generated
2. an ACPI notification is send to ACPI video driver as a result of the ACPI interrupt
3. ACPI video driver exports an input event to userspace upon the notification

Backlight can be changed in either 1(BIOS/AML), 2(ACPI video driver), or 3(userspace app).
The problem here is that if it's done in step 1, which is out of OS's control, we should be able to prevent the actions in step 2 and step 3.
And it seems that brightness_switch_enabled and with hal laptop_panel.brightness_in_hardware flags are just for this purpose.

If I'm right, the only problem left is that OSD shows incorrect brightness level, right?
IMO, there could be two reasons.
1. OSD uses /sys/.../brightness, like you said
2. when hal laptop_panel.brightness_in_hardware is set, hal just ignore the event totally, i.e. even not re-query the current brightness level upon this event.

this is gotten from your previous email,
maxim@maxim-laptop:~$ grep . /sys/class/backlight/acpi_video0/*
/sys/class/backlight/acpi_video0/actual_brightness:2
/sys/class/backlight/acpi_video0/bl_power:0
/sys/class/backlight/acpi_video0/brightness:7
/sys/class/backlight/acpi_video0/max_brightness:9

what does OSD show in this case? a level of 7?
please change the backlight via this sysfs I/F, e.g. echo 4 > /sys/class/backlight/acpi_video0/brightness, then press the brightness up hotkey, what does OSD show now? a level of 5 or still 7?
Comment 4 ykzhao 2009-04-30 04:04:42 UTC
Hi, Maxim
    Will you please apply the patch in http://bugzilla.kernel.org/show_bug.cgi?id=13121#C18 and do the following test on your box?
    a. kill the process using the /proc/acpi/event(the command of "lsof /proc/acpi/event" can get the process ID)
    b. cat /proc/acpi/event >acpi_event
    c. press the brightness hotkey and see whether the brightness can be changed.
    d. please use /sys/class/backlight/*/brightness I/F and see whether the brightness can be changed.

    It will be great if you can do the above test after adding the boot option of "acpi_backlight=vendor".
    Note: There is no /sys/class/backlight/* I/F when adding the boot option of "acpi_backlight=vendor". And it is unnecessary to try the /sys/class/baclight/ I/F.
    
    Thanks.
Comment 5 Maxim Levitsky 2009-04-30 15:41:34 UTC
(In reply to comment #3)
> Yakui,
> acpidump can be gotten from bug #13121
> and your suggestion is not helpful for this bug.
> 
> Now, I think we should make clear
> 1. if the brightness is changed by BIOS or by AML code.
> 2. what does gpm use for brightness control,
> /sys/class/backlight/.../brightness vs actual_brightness
> 
> (In reply to comment #0)
> > 
> > Thus a single (say brightness up) event results in brightness going 4
> levels
> > up:
> > 
> > 1 due to hardware change,
> agree
> 
> > 1 for acpi video driver automatic chager,
> agree, this can be enabled/disable via brightness_switch_enable.
> 
> > 1 by gpm listening to acpi event device, and finally 1 due to normal
> keypress.
> 
> I'm confused here.
> 
> the "finally 1 due to normal keypress", you mean the input event for the
> keypress, right?
> how does gpm catch acpi event? or catch what kind of events?
> I think these two are the same.
I think it uses hal, and it reads directly from /dev/input/eventX interfaces

> 
> IMO, there are three parts in all, when pressing a hotkey
> 1. an ACPI interrupt is generated
> 2. an ACPI notification is send to ACPI video driver as a result of the ACPI
> interrupt
> 3. ACPI video driver exports an input event to userspace upon the
> notification
> 
> Backlight can be changed in either 1(BIOS/AML), 2(ACPI video driver), or
> 3(userspace app).
> The problem here is that if it's done in step 1, which is out of OS's
> control,
> we should be able to prevent the actions in step 2 and step 3.
> And it seems that brightness_switch_enabled and with hal
> laptop_panel.brightness_in_hardware flags are just for this purpose.
> 
> If I'm right, the only problem left is that OSD shows incorrect brightness
> level, right?
> IMO, there could be two reasons.
> 1. OSD uses /sys/.../brightness, like you said
> 2. when hal laptop_panel.brightness_in_hardware is set, hal just ignore the
> event totally, i.e. even not re-query the current brightness level upon this
> event.
> 
> this is gotten from your previous email,
> maxim@maxim-laptop:~$ grep . /sys/class/backlight/acpi_video0/*
> /sys/class/backlight/acpi_video0/actual_brightness:2
> /sys/class/backlight/acpi_video0/bl_power:0
> /sys/class/backlight/acpi_video0/brightness:7
> /sys/class/backlight/acpi_video0/max_brightness:9
> 
> what does OSD show in this case? a level of 7?
> please change the backlight via this sysfs I/F, e.g. echo 4 >
> /sys/class/backlight/acpi_video0/brightness, then press the brightness up
> hotkey, what does OSD show now? a level of 5 or still 7?


I already did that test for myself.
OSD shows /sys/class/backlight/acpi_video0/brightness
Comment 6 Maxim Levitsky 2009-04-30 16:19:03 UTC
No need to apply http://bugzilla.kernel.org/show_bug.cgi?id=13121#C18
I am the same Maxim that reported that issue, so I use that patch.


/sys/firmware/acpi/interrupts/ shows same picture, only GPE #2 and #1C increase.

#2 is thremal event which is signaled from SMI, as according to ICH8 datesheet, this is user generated event, by writting to a register.

#1C is the EC interrupt, and it ether steadily increases qute fast (when the 'ec interrupt storm is detected', or otherwise increases rarely.

Brightness events are reported via #1C, using this ugly AML code:


                   Method (_Q11, 0, NotSerialized)
                    {

                        /* this I think checks for vista */
                        If (LGreaterEqual (OSYS, 0x07D6))
                        {
                            /* this checks if onboard graphics are enabled*/
                            /* OBV is first pci confif register on device#0 
                            function  #1*/

                            If (LEqual (OBV, 0xFF))
                            {
                                Notify (\_SB.PCI0.PEGP.VGA.LCD, 0x87)
                            }
                            Else
                            {
                                Notify (\_SB.PCI0.OVGA.DD03, 0x87)
                            }
                        }
                        Else
                        {
                            
                            /* this seems to be vendor specific reading 
                              of brightness */
                            If (LEqual (\_SB.PCI0.WMID.BAEF, One))
                            {
                                /* this reads current brightness*/
                                Store (BRTS, Local1)
                                Store (\_SB.PCI0.WMID.LBL0, Local2)
                                Add (Local2, Local1, Local2)

                                /* NTDC I think is software variable*/
                                Store (Local2, \_SB.PCI0.WMID.NTDC)
                                Notify (WMID, 0x80)
                            }
                        }
                    }


Notice the check for windows vista? (not sure, but I think so)
Comment 7 Maxim Levitsky 2009-04-30 16:20:30 UTC
I am sure brightness is changed by the EC
Comment 8 Maxim Levitsky 2009-04-30 16:21:25 UTC
I'll check acpi_backlight=vendor now
Comment 9 Maxim Levitsky 2009-04-30 16:31:52 UTC
As I now understand acpi_backlight=vendor cause acer-wmi to be prefeered.
it works fine with acer-wmi as it did before.

Thus I can use this option to switch between drivers without recompiling.
Comment 10 Zhang Rui 2009-05-04 07:03:53 UTC
So far, I think the problem is that gpm uses /sys/class/backlight/acpi_video0/brightness instead of actual_brightness, thus it shows incorrect brightness levels when backlight is changed via hotkeys... right?

I checked the gpm source code, and couldn't find anything useful.
cc Richard, who is an expert on this.

plus, the original post can be found at
http://marc.info/?l=linux-acpi&m=124085511126969&w=2
Comment 11 Zhang Rui 2009-05-11 06:34:14 UTC
hmm, as this seems to be a gpm problem, please file a new bug report at
http://bugzilla.gnome.org/simple-bug-guide.cgi

Close it as I don't think there is anything that we should improve in Linux kernel.

Maxim,
please cc me in the new bugzilla report.
please add the link of this page in the new bug report.

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