Bug 51051 - Acer Aspire S7 Brightness changes but hotkeys do not work
Summary: Acer Aspire S7 Brightness changes but hotkeys do not work
Status: CLOSED CODE_FIX
Alias: None
Product: ACPI
Classification: Unclassified
Component: Power-Video (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Aaron Lu
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-27 23:03 UTC by Kevin Anderson
Modified: 2013-07-22 01:05 UTC (History)
3 users (show)

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


Attachments
acpidump of Acer Aspire S7-391 (272.53 KB, application/octet-stream)
2012-11-30 01:59 UTC, Kevin Anderson
Details
custom DSDT (439.25 KB, application/octet-stream)
2012-12-02 02:36 UTC, Zhang Rui
Details
ACPI Debug output with custom DSDT (90.72 KB, text/plain)
2012-12-03 01:42 UTC, Kevin Anderson
Details
DMIDECODE Output (8.38 KB, text/plain)
2013-03-26 22:06 UTC, Kevin Anderson
Details
Test patch to remove acpi_video_unregister call (3.16 KB, patch)
2013-03-28 07:01 UTC, Aaron Lu
Details | Diff
Debug DSDT to dump some variable (46.81 KB, application/octet-stream)
2013-03-29 02:54 UTC, Aaron Lu
Details
Compilation Errors (43.01 KB, text/plain)
2013-04-03 03:05 UTC, Kevin Anderson
Details
0001-acer-wmi-add-Acer-Aspire-S7-391-to-video-vendor-list.patch (2.06 KB, patch)
2013-04-03 03:32 UTC, Lee, Chun-Yi
Details | Diff
Custom DSDT table ACPI Debug Output (85.39 KB, text/plain)
2013-04-03 16:30 UTC, Kevin Anderson
Details
Add Acer Aspire-S7 into video_detect_dmi_table (1.15 KB, patch)
2013-04-11 03:10 UTC, Aaron Lu
Details | Diff

Description Kevin Anderson 2012-11-27 23:03:52 UTC
Acer Aspire S7-391
Fedora 17
Intel 4000HD Onboard Graphics

With no acpi options specified for the kernel two devices are shown under /sys/class/backlight. They are intel_backlight and something along the line of acpi_video. Adding acpi_osi=Linux acpi_backlight=vendor to the boot arguments gets rid of the acpi entry under backlights and leave only the intel_backlight. This allows tools such as gnome power manager to adjust the backlight successfully. Adding the boot options has the side effect of disabling the brightness hot keys completely. Using xev and showkey show no key presses being registered. If the boot options are removed then the hot keys function but do not actually change the backlight. Also I can sucessfully echo a brightness value in to /sys/class/backlight/intel_backlight/brightness at any time with any boot options and have it take effect.

Steps to reproduce:
1. Add acpi_osi=Linux acpi_backlight=vendor to kernel command line
2. See only intel_backlight under /sys/class/backlight
3. Try to use brightness keys and see they don't register

Actual results:
1. Function keys to change brightness do not actually register but brightness can be changed through gnome power applets

Expected results:
1. Brightness function keys should register and brightness should be changed with them
Comment 1 Zhang Rui 2012-11-30 01:16:37 UTC
please attach the acpidump output of your laptop.
Comment 2 Kevin Anderson 2012-11-30 01:59:09 UTC
Created attachment 87881 [details]
acpidump of Acer Aspire S7-391
Comment 3 Zhang Rui 2012-11-30 03:31:19 UTC
    Scope (_SB.PCI0.GFX0.DD02)
    {
        Method (_BCL, 0, NotSerialized)  // _BCL: Brightness Control Levels
        {
            Return (\BCLP)
        }

        Name (CBNL, Zero)
        Method (_BCM, 1, NotSerialized)  // _BCM: Brightness Control Method
        {
            Store (Arg0, CBNL)
        }

        Method (_BQC, 0, NotSerialized)  // _BQC: Brightness Query Current
        {
            Return (CBNL)
        }
    }
this shows that the ACPI backlight control for this laptop is totally fake.
so you always need acpi_backlight=vendor whenever you want to control the backlight.

Q1. does /sys/class/backlight/intel_backlight/ always works no matter if the parameter is added?
Q2. does the hotkey work if you boot with acpi_osi="!Windows 2012" acpi_osi="!Windows 2009" acpi_osi="!Windows 2006" altogether?
Comment 4 Kevin Anderson 2012-11-30 03:48:09 UTC
1. Yes as far as I can tell intel_backlight always works by manually echoing a value in to /sys/class/backlight/intel_backlight/brightness. 

2. Booting with all of those options allows the hotkeys to function although it brings back the acpi_video0 interface so even though they are registered they do not actually control the backlight as expected.
Comment 5 Zhang Rui 2012-11-30 03:55:12 UTC
(In reply to comment #4)
> 2. Booting with all of those options allows the hotkeys to function although
> it
> brings back the acpi_video0 interface so even though they are registered they
> do not actually control the backlight as expected.

oh, as I mentioned in comment #3, you always need acpi_backlight=vendor added.
please try with these four parameters altogether.
Comment 6 Kevin Anderson 2012-11-30 23:59:25 UTC
So I tried with all of the boot options and acpi_backlight=vendor so only the intel backlight shows up but so far the backlight keys are still not functioning.
Comment 7 Zhang Rui 2012-12-01 03:41:18 UTC
(In reply to comment #0)
> If the boot options are removed then the hot keys function but do
> not actually change the backlight.

then how do you know the hot key works?
can you get input event successfully?
please attach the output of acpi_listen when you press the hotkey in kernels both with and without acpi_osi="!Windows 2012"
acpi_osi="!Windows 2009" acpi_osi="!Windows 2006" and acpi_backlight=vendor.
Comment 8 Kevin Anderson 2012-12-01 14:34:12 UTC
I know the hotkeys work because without any boot options you can see the Gnome Brightness dialogue attempt to change the brightness. The output of acpi_listen without any acpi boot options is below:

Boot line: kernel /vmlinuz-3.6.7-4.fc17.x86_64 rd.lvm=0 rd.dm=0 rd.md.uuid=fd734d6e:c3705bec:0b9e4b74:93046508  KEYTABLE=us rd.md.uuid=39dae6e2:da0f271e:51a7cc88:8de15c75 rd.luks=0 LANG=en_US.UTF-8 ro root=UUID=0ff0b367-4a2a-4a13-84be-49f3877b76c2 rhgb quiet i915.i915_enable_rc6=1 raid=noautodetect pcie_aspm=force

[root@slim ~]# acpi_listen 
video/brightnessdown BRTDN 00000087 00000000
video/brightnessup BRTUP 00000086 00000000
video/brightnessup BRTUP 00000086 00000000
video/brightnessdown BRTDN 00000087 00000000


With the following boot line no acpi events are seen for the two brightness keys.

Boot line: kernel /vmlinuz-3.6.7-4.fc17.x86_64 rd.lvm=0 rd.dm=0 rd.md.uuid=fd734d6e:c3705bec:0b9e4b74:93046508  KEYTABLE=us rd.md.uuid=39dae6e2:da0f271e:51a7cc88:8de15c75 rd.luks=0 LANG=en_US.UTF-8 ro root=UUID=0ff0b367-4a2a-4a13-84be-49f3877b76c2 rhgb quiet acpi_osi=Linux acpi_backlight=vendor i915.i915_enable_rc6=1 raid=noautodetect pcie_aspm=force acpi_osi="!Windows 2012" acpi_osi="!Windows 2009" acpi_osi="!Windows 2006"

[root@slim ~]# acpi_listen 
^C
[root@slim ~]#
Comment 9 Zhang Rui 2012-12-02 02:36:27 UTC
Created attachment 88131 [details]
custom DSDT

please apply this customized DSDT, and reboot with kernel parameter "acpi.aml_debug_output=1".
and then press the hotkey and see if it works.
please attach the dmesg output after you press the hotkey as well.
Comment 10 Kevin Anderson 2012-12-03 01:41:55 UTC
Hi,
   I've applied the custom DSDT and attached the full dmesg output. The output for the brightness up and down key presses are below as well.

[  759.489985] [ACPI Debug]  String [0x07] "In _Q87"
[  759.490008] [ACPI Debug]  String [0x04] "DSEN"
[  759.490052] [ACPI Debug]  Integer 0x00000004

[ 1064.549723] [ACPI Debug]  String [0x07] "In _Q86"
[ 1064.549746] [ACPI Debug]  String [0x04] "DSEN"
[ 1064.549792] [ACPI Debug]  Integer 0x00000004
Comment 11 Kevin Anderson 2012-12-03 01:42:43 UTC
Created attachment 88241 [details]
ACPI Debug output with custom DSDT

Requested output with the custom DSDT applied
Comment 12 Kevin Anderson 2013-01-02 03:52:15 UTC
I have been looking at this and trying to learn as I go along. With that being said before I started looking in to this I never really looked at the kernel source code so I may be incorrect in some of my statements.

I believe without acpi_backlight=vendor as a kernel parameter then the hotkeys are being captured by the acpi video driver but they are being passed to the acpi video driver instead of the Intel driver. With acpi_backlight=vendor set though there is nothing to capture the hotkey presses.

My normal distribution is Fedora but I decided to try Ubuntu to see if the hotkeys would work there with acpi_backlight=vendor set and it turns out that the hotkeys register and adjust the brightness for the Intel driver appropriately. /sys/class/backlight also only shows intel_backlight. I believe it is due to the fact that Ubuntu uses the 3.5 kernel instead of 3.6 or 3.7. 

I believe the answer may be in commit a60b21763cce01c64cc537869662b41429c62e5f in the acer-wmi.c file. It looks like before that commit acpi_video_backlight_support() equaled 0 so acpi_video_unregister is never called. After that commit acpi_video_backlight_support() still equals 0 but the else clause calls acpi_video_unregister which unloads the acpi video driver and that takes the hotkeys with it.

I'm not entirely sure if any of this helps but hopefully it will. :)
Comment 13 Aaron Lu 2013-03-12 03:07:05 UTC
Hi Kevin,

Thanks for the finding, it definitely helps.

Can you please test the latest upstream kernel first? If it still has this problem, I think we can add Corentin Chary, who is the author of the commit, to take a look at this problem, thanks.
Comment 14 Kevin Anderson 2013-03-18 15:54:05 UTC
Hi Aaron,

I just tried the 3.9-rc2 kernel Saturday and the hotkeys do not work with that version either.
Comment 15 Aaron Lu 2013-03-19 05:43:42 UTC
Thanks Kevin.

Hi Corentin,
It seems your commit a60b21763cce01c64cc537869662b41429c62e5f(drivers-platform-x86: use acpi_video_dmi_promote_vendor()) breaks the hotkey on Acer Aspire S7-391 as spotted by Kevin.

It looks like, for this system, when hotkey for brightness is pressed, there will be no keyboard scancode emitted, instead, EC will send out a notification. unregister acpi video driver would make such notification lost. So perhaps we need to keep acpi video driver's notification handling.

I'm not aware of the reason why acpi_video_unregister is added if acpi video driver will not take care of backlight control, I think it has other uses besides that, like output device switch, etc.
Comment 16 Corentin Chary 2013-03-19 17:51:01 UTC
Well, the current code looks good to me.

	if (dmi_check_system(video_vendor_dmi_table))
		acpi_video_dmi_promote_vendor();
	if (acpi_video_backlight_support()) {
		interface->capability &= ~ACER_CAP_BRIGHTNESS;
		pr_info("Brightness must be controlled by acpi video driver\n");
	} else {
		pr_info("Disabling ACPI video driver\n");
		acpi_video_unregister();
	}


If the model is in video_vendor_dmi_table, or if the user ask for it, user the vendor driver for backlight instead of acpi/video.

If this model is only working with the generic video driver, then maybe it should not be in video_vendor_dmi_table ?
Comment 17 Aaron Lu 2013-03-20 00:48:18 UTC
(In reply to comment #16)
> Well, the current code looks good to me.
> 
>     if (dmi_check_system(video_vendor_dmi_table))
>         acpi_video_dmi_promote_vendor();
>     if (acpi_video_backlight_support()) {
>         interface->capability &= ~ACER_CAP_BRIGHTNESS;
>         pr_info("Brightness must be controlled by acpi video driver\n");
>     } else {
>         pr_info("Disabling ACPI video driver\n");
>         acpi_video_unregister();
>     }
> 
> 
> If the model is in video_vendor_dmi_table, or if the user ask for it, user
> the
> vendor driver for backlight instead of acpi/video.
> 
> If this model is only working with the generic video driver, then maybe it
> should not be in video_vendor_dmi_table ?

This system is kind of interesting: acpi video driver can't handle backlight, while vendor driver can't handle hotkey... and to make it work, both drivers will be needed. Right?
Comment 18 Corentin Chary 2013-03-20 11:42:32 UTC
Then maybe something smarter need to be added to load both drivers. But this is usually a very bad idea since userspace will try to use the generic one first, and it won't work.

Is there a BIOS update somewhere that could fix that ? Or a specific acpi_osi setting ?
Comment 19 Aaron Lu 2013-03-21 02:48:16 UTC
Or what about if vendor driver is to be used to control backlight level, we do not unregister acpi video driver, so that it can still handle notification. As long as acpi_video_backlight_support returns false, acpi video driver will not create any sysfs backlight interface, so vendor driver doesn't need to do anything more I think.

I can understand the reason to unregister acpi video driver in the code, but unfortunately, vendor driver can't handle hotkey in this case.

(In reply to comment #18)
> Is there a BIOS update somewhere that could fix that ?
I don't know about this, but the chances are low I believe.

> Or a specific acpi_osi setting ?
The standard interface to control backlight level in this BIOS table is totally fake(I don't understand why they even bother to create a bogus one :-( ), it doesn't do anything real, so it's impossible for acpi video driver to control backlight, as illustrated in comment #3 by Rui, no matter what acpi_osi is given.
Comment 20 Corentin Chary 2013-03-21 09:57:55 UTC
Oh .. in this case the acpi_video driver doesn't create a sysfs interface but still handle notifications ? If that's true, we could add a specific DMI match for this model, and do not undregister it in this specific case. It's still necessary to unregister it in other cases.

I won't have time to write such a patch in the near future, but I will be happy to review it.
Comment 21 Aaron Lu 2013-03-22 12:46:17 UTC
(In reply to comment #20)
> Oh .. in this case the acpi_video driver doesn't create a sysfs interface but
> still handle notifications ? 

Yes. It will need to handle notifications, since the hotkey will not send out any scancode, but an acpi event. At least, this seems to be the case based on Kevin's description.

> If that's true, we could add a specific DMI match
> for this model, and do not undregister it in this specific case. It's still
> necessary to unregister it in other cases.

For other cases, is it because both scancode and acpi event will be emitted when hotkey is pressed?

> 
> I won't have time to write such a patch in the near future, but I will be
> happy
> to review it.

OK, I'll write a patch to handle this, thanks for the review.
Comment 22 Aaron Lu 2013-03-26 05:32:32 UTC
Hi Kevin,

You dmidecode output please, thanks.
Comment 23 Kevin Anderson 2013-03-26 22:06:13 UTC
Created attachment 96291 [details]
DMIDECODE Output
Comment 24 Kevin Anderson 2013-03-26 22:08:38 UTC
Aaron I have attached the dmidecode output as requested. Thanks.
Comment 25 Aaron Lu 2013-03-28 07:01:46 UTC
Created attachment 96451 [details]
Test patch to remove acpi_video_unregister call

Hi,

Please kindly test this patch, it is on top of 3.8.4, also applies to latest
Linus' git tree. You do not need to add any kernel command line parameter with this patch.

Please list /sys/class/backlight after test, thanks.
Comment 26 Kevin Anderson 2013-03-28 15:32:39 UTC
Hi Aaron,
   I was able to test the patch against 3.9.0-rc4. After applying the patch I can adjust the brightness successfully but there are some caveats. Also the only thing showing under /sys/class/backlight is the intel_backlight with and without acpi_backlight=vendor.

While the backlight does change with the hotkeys the key presses do not show up under showkey or acpi_listen. The other thing is that the Gnome On Screen Dialog does not register the key presses when changing backlight brightness (I think it may be because the keys are not being registered how Gnome expects them to be or because of the next point). One final thing is that if a brightness is echoed to /sys/class/backlight/intel_backlight/brightness then the brightness is not changed.

Thanks!
Comment 27 Aaron Lu 2013-03-29 02:50:12 UTC
(In reply to comment #26)
> Hi Aaron,
>    I was able to test the patch against 3.9.0-rc4. After applying the patch I
> can adjust the brightness successfully but there are some caveats. Also the
> only thing showing under /sys/class/backlight is the intel_backlight with and
> without acpi_backlight=vendor.

This is expected, neither acpi video driver nor platform vendor driver can handle this system's backlight, only intel gpu driver can, so only that interface exists.

> 
> While the backlight does change with the hotkeys the key presses do not show
> up
> under showkey or acpi_listen.

Weird, if the hotkey doesn't make its way to user space, how can the backlight change?

> The other thing is that the Gnome On Screen
> Dialog does not register the key presses when changing backlight brightness
> (I
> think it may be because the keys are not being registered how Gnome expects
> them to be or because of the next point). One final thing is that if a
> brightness is echoed to /sys/class/backlight/intel_backlight/brightness then
> the brightness is not changed.

This suggests that the intel_backlight does not function. This patch shouldn't affect intel gpu driver, but to be safe, plase also test 3.9.0-rc4 without applying this patch, see if echo to intel_backlight/brightness works or not.
Comment 28 Aaron Lu 2013-03-29 02:54:47 UTC
Created attachment 96531 [details]
Debug DSDT to dump some variable

Hi Kevin,

Please do one more test by using this DSDT table together with the patch, and attach the dmesg after you've pressed the brightness hotkey. Thanks. And you will need the acpi.aml_debug_output=1 kernel parameter for the debug output, no other acpi related parameter is needed.
Comment 29 Corentin Chary 2013-03-29 11:42:08 UTC
(In reply to comment #25)
> Created an attachment (id=96451) [details]
> Test patch to remove acpi_video_unregister call
> 
> Hi,
> 
> Please kindly test this patch, it is on top of 3.8.4, also applies to latest
> Linus' git tree. You do not need to add any kernel command line parameter
> with
> this patch.
> 
> Please list /sys/class/backlight after test, thanks.


Why are you moving this to acpi directly ? Does it really need to be here and not in the vendor driver ? (this can happen if registering/unregistering the video driver confuse the DSDT, but I don't think it's the case for *every* model in this list).
Comment 30 Corentin Chary 2013-03-29 11:44:28 UTC
 
> While the backlight does change with the hotkeys the key presses do not show
> up
> under showkey or acpi_listen. The other thing is that the Gnome On Screen
> Dialog does not register the key presses when changing backlight brightness
> (I
> think it may be because the keys are not being registered how Gnome expects
> them to be or because of the next point).

No, gnome is broken and listen to key press instead of uevent to display brightness changes: https://bugzilla.gnome.org/show_bug.cgi?id=672380
Comment 31 Aaron Lu 2013-03-29 14:33:17 UTC
(In reply to comment #29)
> Why are you moving this to acpi directly ? Does it really need to be here and
> not in the vendor driver ? (this can happen if registering/unregistering the
> video driver confuse the DSDT, but I don't think it's the case for *every*
> model in this list).

I understand the reason to keep those dmi entries in vendor driver is to avoid a big dmi table for acpi video driver, which is indeed best to avoid, but in this case, I don't see how we can overcome this without moving this to acpi directly.

We can't simply not call acpi_video_unregister for these systems, as we really need to make sure acpi video driver does not create backlight device for them, or user will need to pass the acpi_backlight=vendor command line param.

An alternative would be, introduce a new interface, something like acpi_video_unregister_backlight, which does not unload the whole acpi video driver, but just unregister the backlight device it has created. So that its notification handler is still there.

But I would like to avoid this, it is adding yet another quirk on top of an existing quirk, it makes the code difficult to understand.

And moving all the DMI entries to acpi video also has the benefit of no unnecessary dependency between vendor driver and video driver.

But if you insist, I can do that. Just let me know your opinion, thanks.
Comment 32 Aaron Lu 2013-04-02 05:12:09 UTC
Hi Kevin,

My understanding of your system backlight control is:
for brightness level change, you are using intel_backlight, which is provided by the gpu driver;
for hotkey notification, you are using acpi video driver.

But your comment #26 made me feel that this is not the case, or you should receive key press event and the intel_backlight interface should work using the echo command. So can you please verify if unload acer-wmi module will make a difference, thanks.
Comment 33 Aaron Lu 2013-04-02 05:14:42 UTC
BTW, bug #35622 is a similar one.
Comment 34 Kevin Anderson 2013-04-02 18:33:15 UTC
Aaron,
   I have not had a chance to test 3.9 without the provided patches. I have tried it with the modified DSDT but it did not output anything in dmesg.

I unloaded acer-wmi with modprobe -r acer-wmi and while the on screen dialogue does not work the following occurs:

echo'ing a value to brightness updates the screen brightness as well as the actual_brightness value. Using the hotkeys only updates that actual_brightness value while leaving the brightness value untouched.
Comment 35 Aaron Lu 2013-04-03 01:27:56 UTC
Kevin,

This really beats me... :-)
I would expect some similar output as in comment #10, I don't see why we lose those messages. Did you add acpi.aml_debug_output=1 to kernel command line?
Comment 36 Kevin Anderson 2013-04-03 03:05:31 UTC
Created attachment 97111 [details]
Compilation Errors
Comment 37 Kevin Anderson 2013-04-03 03:09:09 UTC
Aaron,
   It looks like I missed a few options that were necessary for adding the custom DSDT. I added the required options but when I try to compile I get the errors in the attachment to comment #36. Should this be expected? Also I apologize for the confusion. Looks like I should have drank some coffee first. :)
Comment 38 Lee, Chun-Yi 2013-04-03 03:32:56 UTC
Created attachment 97141 [details]
0001-acer-wmi-add-Acer-Aspire-S7-391-to-video-vendor-list.patch

Similar with bko#35622, create this testing patch.

Hi Kevin, 

Please help to test this patch, it's still add Aspire-S7 to table but this time I add a new capability for avoid unregister acpi/video driver on your machine.

Please try this patch WITHOUT "acpi_backlight=vendor" and "acpi_osi=Linux" and other kernel parameter, I think this time acpi/video will keep there for emit keycode.

Please also use "lsmod | grep video" to check if video driver still there.

Thanks for your help!
Comment 39 Aaron Lu 2013-04-03 05:51:29 UTC
(In reply to comment #37)
> Aaron,
>    It looks like I missed a few options that were necessary for adding the
> custom DSDT. I added the required options but when I try to compile I get the
> errors in the attachment to comment #36. Should this be expected? Also I
> apologize for the confusion. Looks like I should have drank some coffee
> first.
> :)

Hi Kevin,

Please follow this document: Documentation/acpi/initrd_table_override.txt, no need to recompile kernel for the DSDT, we can replace the DSDT in initrd.
Comment 40 Kevin Anderson 2013-04-03 16:27:26 UTC
Hi Aaron,
  Here are the debug messages that were printed. The first three lines occur when pressing the brightness down hotkey key and the last three were brightness up hotkey. I will also attach the full output of dmesg.

Brightness Down Outputs:
[  591.986654] [ACPI Debug]  String [0x04] "_Q86"
[  591.987156] [ACPI Debug]  Integer 0x00000001
[  591.988751] [ACPI Debug]  Integer 0x00000000

Brightness Up Outputs:
[  593.356288] [ACPI Debug]  String [0x04] "_Q87"
[  593.356789] [ACPI Debug]  Integer 0x00000001
[  593.358288] [ACPI Debug]  Integer 0x00000000

Thanks!
Comment 41 Kevin Anderson 2013-04-03 16:30:09 UTC
Created attachment 97221 [details]
Custom DSDT table ACPI Debug Output

Full output of dmesg with acpi debug dsdt.
Comment 42 Kevin Anderson 2013-04-03 16:41:53 UTC
(In reply to comment #38)
> Created an attachment (id=97141) [details]
> 0001-acer-wmi-add-Acer-Aspire-S7-391-to-video-vendor-list.patch
> 
> Similar with bko#35622, create this testing patch.
> 
> Hi Kevin, 
> 
> Please help to test this patch, it's still add Aspire-S7 to table but this
> time
> I add a new capability for avoid unregister acpi/video driver on your
> machine.
> 
> Please try this patch WITHOUT "acpi_backlight=vendor" and "acpi_osi=Linux"
> and
> other kernel parameter, I think this time acpi/video will keep there for emit
> keycode.
> 
> Please also use "lsmod | grep video" to check if video driver still there.
> 
> Thanks for your help!

Hi,
   I tested the patch and without acpi_backlight=vendor both intel and acpi_backlight show under /sys/class/backlight. The hotkeys do work but no keycode is emitted for X/Gnome Brightness to catch.
Comment 43 Aaron Lu 2013-04-04 10:53:40 UTC
(In reply to comment #16)
> Well, the current code looks good to me.
> 
>     if (dmi_check_system(video_vendor_dmi_table))
>         acpi_video_dmi_promote_vendor();
>     if (acpi_video_backlight_support()) {
>         interface->capability &= ~ACER_CAP_BRIGHTNESS;
>         pr_info("Brightness must be controlled by acpi video driver\n");
>     } else {
>         pr_info("Disabling ACPI video driver\n");
>         acpi_video_unregister();
>     }
> 
> 
> If the model is in video_vendor_dmi_table, or if the user ask for it, user
> the
> vendor driver for backlight instead of acpi/video.
> 
> If this model is only working with the generic video driver, then maybe it
> should not be in video_vendor_dmi_table ?

Hi Corentin,

I misunderstand your words "generic video driver", I thought you are saying vendor driver.

So yes, this model only works with the generic video driver(i.e. the gpu driver), and I agree that it shouldn't be in video_vendor_dmi_table.
Comment 44 Aaron Lu 2013-04-04 11:01:34 UTC
(In reply to comment #40)
> Brightness Up Outputs:
> [  593.356288] [ACPI Debug]  String [0x04] "_Q87"
> [  593.356789] [ACPI Debug]  Integer 0x00000001
1 here is for variable IGDS
> [  593.358288] [ACPI Debug]  Integer 0x00000000
0 here is for DSEN

It is interesting to see you have different values in comment #10(the DSEN is 4 there and 0 here). I don't why this is the case, but it has the consequences of hotkey directly changing backlight instead of sending out notification.
Comment 45 Aaron Lu 2013-04-08 09:22:10 UTC
Hi Rui,

In the dmesg Kevin attached in comment #11, I saw:
...
[    5.351340] [ACPI Debug]  String [0x06] "In DOS"
[    5.351355] [ACPI Debug]  String [0x04] "DSEN"
[    5.351388] [ACPI Debug]  Integer 0x00000000
[    5.351398] [ACPI Debug]  String [0x04] "Arg0"
[    5.351407] [ACPI Debug]  Integer 0x00000004
[    5.351470] ACPI: Video Device [GFX0] (multi-head: yes  rom: no  post: no)
[    5.351589] input: Video Bus as /devices/LNXSYSTM:00/device:00/PNP0A08:00/LNXVIDEO:00/input/input6
...

The arg0 will be 4 only when we call acpi_video_bus_stop_devices, but obviously this dump here is from acpi_video_bus_start_devices. So I don't see why the arg0 will be 4 here. Do you have any idea? Thanks.
Comment 46 Aaron Lu 2013-04-09 06:36:02 UTC
Hello Lee,

Do you know if acer-wmi module can deliver hotkey notification for this system? Thanks.
Comment 47 Aaron Lu 2013-04-11 03:10:32 UTC
Created attachment 98041 [details]
Add Acer Aspire-S7 into video_detect_dmi_table

Hi Kevin,

I have prepared two patches for you to test, patch 1 is attached here, and patch 2 is here:
https://bugzilla.kernel.org/attachment.cgi?id=98031

Please apply patch 1 and then patch 2, on top of v3.8 or Linus' git tree.

Patch 1 adds this system into video_detect_dmi_table without touching any platform code(hopefully platform people will be happy), and patch 2 will turn on the flag for firmware to report notifications.
Comment 48 Aaron Lu 2013-04-11 03:30:48 UTC
BTW, when building the kernel, please do not select acer-wmi for this test, thanks.

A more general solution than this will be required(as distros will always build those modules), but I want to know if this works or not.
Comment 49 Kevin Anderson 2013-04-11 20:57:27 UTC
Hi Aaron,
   If I do not build the acer-wmi module and apply the patches I am able to boot without the acpi_backlight=vendor option and everything works as expected. The hotkeys function properly and the key presses are passed to Gnome.
Comment 50 Aaron Lu 2013-06-17 05:25:14 UTC
Hi Kevin,

Matthew submitted a patchset to remove ACPI video's backlight interface if the system claims support of Win8, as is the case of your system.

Can you please give it a try? They are here:
https://patchwork.kernel.org/patch/2695411/
https://patchwork.kernel.org/patch/2695391/
https://patchwork.kernel.org/patch/2695401/

And in addition to that, the mentioned patch 2 in comment #47 is still required to fix the notification delivery problem:
https://bugzilla.kernel.org/attachment.cgi?id=98031

So with the four patches, the intel_backlight will be the only interface left, while the notification delivery provided by ACPI video is fixed and usable. Let me know your test result, thanks.
Comment 51 Kevin Anderson 2013-06-21 03:06:16 UTC
Hi Aaron,
   I tested the 4 patches against 3.10.0 RC6. I booted with the following kernel command line: Command line: BOOT_IMAGE=/vmlinuz-3.10.0-rc6-kevin+ root=UUID=0d6d2e9b-811c-4851-8b57-bac3ec9010df ro rd.lvm=0 rd.dm=0 rd.md.uuid=fd734d6e:c3705bec:0b9e4b74:93046508 rd.md.uuid=39dae6e2:da0f271e:51a7cc88:8de15c75 rd.luks=0 rhgb quiet raid=noautodetect libahci.ignore_sss=1 elevator=noop LANG=en_US.UTF-8

With the patches everything worked as expected. The only backlight present was the intel_backlight and the hotkey notifications worked as well with GNOME.
Comment 52 Aaron Lu 2013-07-22 01:01:28 UTC
Patches in Rafael's acpi-video branch.

https://patchwork.kernel.org/patch/2829342/  (expose OSI version)
https://patchwork.kernel.org/patch/2829344/  (Always call acpi_video_init_brightness() on init)
https://patchwork.kernel.org/patch/2829339/  (No ACPI backlight if firmware expects Windows 8)
https://patchwork.kernel.org/patch/2827878/  (no automatic brightness changes by firmware)

http://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git acpi-video
Comment 53 Aaron Lu 2013-07-22 01:05:10 UTC
Patches in Linus' tree.

commit efaa14c7e981bdf8d3c8d39d3ed12bdc60faabb8
Author: Aaron Lu <aaron.lu@intel.com>
Date:   Tue Jul 16 13:08:05 2013 +0800

    ACPI / video: no automatic brightness changes by win8-compatible firmware

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

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