Bug 214899 - ideapad-laptop: brightness hotkeys not working
Summary: ideapad-laptop: brightness hotkeys not working
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Platform_x86 (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_platform_x86@kernel-bugs.osdl.org
URL:
Keywords:
: 216933 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-10-31 19:28 UTC by Johannes Penßel
Modified: 2023-01-24 09:32 UTC (History)
9 users (show)

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


Attachments
DSDT.dsl (1.78 MB, text/x-csrc)
2021-10-31 19:28 UTC, Johannes Penßel
Details
dmesg (64.91 KB, text/plain)
2021-10-31 19:31 UTC, Johannes Penßel
Details
acpidump (1.80 MB, text/plain)
2021-11-01 20:34 UTC, Johannes Penßel
Details
ideapad-laptop event debug patch. (1.06 KB, patch)
2021-11-02 08:13 UTC, Hans de Goede
Details | Diff
acpi-video event debug patch (993 bytes, patch)
2021-11-02 14:53 UTC, Hans de Goede
Details | Diff
acpidump BIOS version FHCN64WW, GPU enabled (1.83 MB, text/plain)
2022-04-26 12:56 UTC, Johannes Penßel
Details
ACPICA: eval \_SB.PC00._INI and \_SB.PCI0._INI before running _REG methods (3.94 KB, patch)
2022-06-13 13:17 UTC, Hans de Goede
Details | Diff
Attempt 2 at fixing this (3 patches in a tarbal) (4.74 KB, application/x-compressed-tar)
2022-06-14 18:48 UTC, Hans de Goede
Details
Attempt 3 at fixing this (4 patches in a tarbal) (7.84 KB, application/x-compressed-tar)
2022-06-15 16:36 UTC, Hans de Goede
Details
[PATCH 1/2] ACPICA: Make address-space-handler install and _REG execution 2 separate steps (6.52 KB, patch)
2022-07-04 10:12 UTC, Hans de Goede
Details | Diff
[PATCH 2/2] ACPI: EC: fix ECDT probe ordering issues (5.83 KB, patch)
2022-07-04 10:12 UTC, Hans de Goede
Details | Diff
[PATCH 1/4] ACPICA: include/acpi/acpixf.h: Fix indentation (5.49 KB, patch)
2022-10-03 14:32 UTC, Hans de Goede
Details | Diff
[PATCH 2/4] ACPICA: Allow address_space_handler Install and _REG execution as 2 separate steps (7.45 KB, patch)
2022-10-03 14:32 UTC, Hans de Goede
Details | Diff
[PATCH 3/4] ACPI: EC: Fix EC address space handler unregistration (2.01 KB, patch)
2022-10-03 14:33 UTC, Hans de Goede
Details | Diff
[PATCH 4/4] ACPI: EC: fix ECDT probe ordering issues (5.81 KB, patch)
2022-10-03 14:33 UTC, Hans de Goede
Details | Diff

Description Johannes Penßel 2021-10-31 19:28:04 UTC
Created attachment 299379 [details]
DSDT.dsl

Hardware: Lenovo IdeaPad 5 15ITL05, latest BIOS version
Issue: Platform backlight device provided by ideapad-laptop has no effect on actual brightness.
Pressing brightness keys on my keyboard causes changes in /sys/class/backlight/ideapad/actual_brightness, but no uevent is generated (as outlined in drivers/video/backlight.c source code comments), and /sys/class/backlight/ideapad/brightness is unaffected. 
When manipulating /sys/class/backlight/ideapad/brightness directly, uevents are being generated, but no change in real screen brightness occurs. 
The only way that screen brightness can actually be changed is by using i915's intel_backlight sysfs interface, which does not respond to brightness keyboard keys.

attached is my decompiled DSDT and dmesg.

ls -la /sys/class/backlight/ideapad/
total 0
drwxr-xr-x 3 root root     0 Oct 31 19:40 .
drwxr-xr-x 3 root root     0 Oct 31 19:40 ..
-r--r--r-- 1 root root  4096 Oct 31 19:40 actual_brightness
-rw-r--r-- 1 root root  4096 Oct 31 19:40 bl_power
-rw-rw-r-- 1 root video 4096 Oct 31 19:43 brightness
lrwxrwxrwx 1 root root     0 Oct 31 19:40 device -> ../../../VPC2004:00
-r--r--r-- 1 root root  4096 Oct 31 19:40 max_brightness
drwxr-xr-x 2 root root     0 Oct 31 19:40 power
-r--r--r-- 1 root root  4096 Oct 31 19:40 scale
lrwxrwxrwx 1 root root     0 Oct 31 19:40 subsystem -> ../../../../../../../class/backlight
-r--r--r-- 1 root root  4096 Oct 31 19:40 type
-rw-r--r-- 1 root root  4096 Oct 31 19:40 uevent
Comment 1 Johannes Penßel 2021-10-31 19:31:22 UTC
Created attachment 299381 [details]
dmesg
Comment 2 Hans de Goede 2021-11-01 19:35:56 UTC
What is your kernel commandline?  According to your attached dmesg it is empty, but that seems unlikely... ?

The ideapad-laptop code has this:

        if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
                err = ideapad_backlight_init(priv);
                if (err && err != -ENODEV)
                        goto backlight_failed;
        }

And acpi_video_get_backlight_type() is:

enum acpi_backlight_type acpi_video_get_backlight_type(void)
{
...
        if (acpi_backlight_cmdline != acpi_backlight_undef)
                return acpi_backlight_cmdline;

        if (acpi_backlight_dmi != acpi_backlight_undef)
                return acpi_backlight_dmi;

        if (!(video_caps & ACPI_VIDEO_BACKLIGHT))
                return acpi_backlight_vendor;

        if (acpi_osi_is_win8() && backlight_device_get_by_type(BACKLIGHT_RAW))
                return acpi_backlight_native;

        return acpi_backlight_video;
}

So the only way that the ideapad backlight can get registered is if:

1. You have acpi_backlight=vendor on your kernel cmdline
2. There is a DMI quirk setting for your model in the kernel which there is not.
3. Your ACPI tables do not implement the ACPI_VIDEO_BACKLIGHT interface, but your dmesg says:

[    2.237329] ACPI: video: Video Device [GFX0] (multi-head: yes  rom: no  post: no)

So this is unlikely, it is still possible though that your DSDT implements the ACPI video interface but not the backlight part (that would be a first). I cannot tell since the ACPI video interface is not in the DSDT, it is likely defined in one of the SSDT tables. Please attach a full acpidump.

Can you try specifying: "acpi_backlight=video" on the kernel commandline and then do:

ls /sys/class/backlight

And copy and paste the output here. This cmdline option not only helps me debug this, but it should also workaround your backlight issue :)
Comment 3 Johannes Penßel 2021-11-01 20:33:53 UTC
I'm specifying my command line via kernel config, so for some reason, it shows up in a separate message somewhat late in dmesg:

[    0.017237] Kernel command line: crypt_root=UUID=2cf18008-b55c-4f5f-926b-5f92a588348a root=UUID=c890d077-9357-4996-94a3-811d6f477c89 i915.enable_guc=2 iwlwifi.power_save=1 iwlmvm.power_scheme=3 snd-hda-intel.power_save_controller=1 acpi_backlight=vendor

Possibly of note as well is that I am able to write custom values into /sys/class/backlight/ideapad/actual_brightness, which none of the other interfaces allows. 

ls -la /sys/class/backlight (with acpi_backlight=video)
total 0
drwxr-xr-x  2 root root 0 Nov  1 21:28 .
drwxr-xr-x 79 root root 0 Nov  1 21:28 ..
lrwxrwxrwx  1 root root 0 Nov  1 21:28 acpi_video0 -> ../../devices/pci0000:00/0000:00:02.0/backlight/acpi_video0
lrwxrwxrwx  1 root root 0 Nov  1 21:28 acpi_video1 -> ../../devices/pci0000:00/0000:00:02.0/backlight/acpi_video1
lrwxrwxrwx  1 root root 0 Nov  1 21:28 intel_backlight -> ../../devices/pci0000:00/0000:00:02.0/drm/card0/card0-eDP-1/intel_backlight

I am afraid to report that booting with acpi_backlight=video did not help with the brightness keys :(
Comment 4 Johannes Penßel 2021-11-01 20:34:52 UTC
Created attachment 299389 [details]
acpidump
Comment 5 Hans de Goede 2021-11-01 21:22:57 UTC
Ah, ok so you indeed have "acpi_backlight=vendor" on the kernel commandline, please don't do that, the old Ideapad specific backlight interface enabled by that is only for very old laptop models.

Where as your laptop is very new! If you omit this, then you should only have the intel_backlight under /sys/class/backlight and brightness control to e.g. the slider in the GNOME system (top right) menu should work.

The brightness-keys directly changing the brightness is something from the same era as the Ideapad specific backlight interface. Now a days almost without exception the brightness keys will simply generate key-press events which are processed by userspace to actually change the brightness. This allows userspace to apply various policies to the brightness setting, for e.g. interaction with auto-brightness control on devices with an ambient-light-sensor (ALS).

Almost all "integrated" desktop environments like GNOME, KDE, XFCE, MATE, etc. will automatically do the right thing with the brightness-key events. If you are however using e.g. i3 as window-manager then you will need to script this yourself, there are plenty of examples how to do this.

###

So as said if you omit "acpi_backlight=vendor" on the kernel commandline, you should only get the intel_backlight in /sys/class/backlight, which you have already indicated works as it should.

All that remains then is to check that key-press events are properly generated and if they are then this is "not a kernel bug". Please install evemu or evtest and then run (as root) evemu-record or evtest and test for each input-device if it perhaps sends brightness key-press events when pressing the brightness hotkeys.

The following devices are a good starting point to test:
"AT Translated Set 2 keyboard"
"Video Bus"

But if neither of those works then please test them all 1 by 1. Note it is possible that the "AT Translated Set 2 keyboard" generates only EV_MSC events or the wrong EV_KEY codes for the brightness keys (so not KEY_BRIGHTNESS[DOWN|UP]). In this case we need to add a mapping for your laptop to fix this.
Comment 6 Johannes Penßel 2021-11-01 22:17:29 UTC
Getting those brightness keys to actually produce key events is something I have tried unsuccessfully for quite a while now. Having just re-checked all 14 available input devices for good measure with evtest, there is still nothing to report. Same goes for ACPI and WMI events as far as I can tell. Under Windows, the brightness keys work without issues.

I have noticed that this is actually very similar to how the microphone mute key behaves without the ideapad-laptop driver loaded. (i.e. it is virtually undetectable) The driver exposes this key via the keymap it creates for the "Ideapad extra buttons" input device. Maybe my brightness keys need to be added to this keymap as well?
Comment 7 Hans de Goede 2021-11-01 22:34:23 UTC
The ideapad-laptop code seems to log to dmesg on most unknown events, I assume nothing shows up in dmesg?

I think based on your other experiments that it would be interesting to log the value of now in this function in ideapad-laptop.c:


static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
{
        unsigned long now;

        /* if we control brightness via acpi video driver */
        if (!priv->blightdev)
                read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now);
        else
                backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
}

Changing it to:

static void ideapad_backlight_notify_brightness(struct ideapad_private *priv)
{
        unsigned long now;

        /* if we control brightness via acpi video driver */
        if (!priv->blightdev) {
                read_ec_data(priv->adev->handle, VPCCMD_R_BL, &now);
                pr_info("EC's LCD backlight value is now: %ld\n", now);
        } else
                backlight_force_update(priv->blightdev, BACKLIGHT_UPDATE_HOTKEY);
}


I suspect that we will see that value change (note this assumes you do not specify an acpi_backlight= kernel cmdline option).
Comment 8 Hans de Goede 2021-11-01 22:44:08 UTC
Note to self, find _DOS ACPI video method, see what setting bit 2 does (this should disable automatic brightness adjustment by the EC).
Comment 9 Johannes Penßel 2021-11-01 23:42:13 UTC
Yes, nothing about unknown events in dmesg. 

Now this is weird. The source code change you suggested seems to have no effect. (I double-checked my command line and printk log level)
Comment 10 Hans de Goede 2021-11-02 08:13:33 UTC
Created attachment 299401 [details]
ideapad-laptop event debug patch.

Can you give this patch a try please; also make sure you have CONFIG_ACPI_WMI enabled please.

With this patch pressing your micmute button should definitely show an event in the "dmesg -w" output; if the micmute does not show events your not running the patched code.

Once you have verified that the micmute button logs events in dmesg, please try the brightness buttons.
Comment 11 Johannes Penßel 2021-11-02 14:15:51 UTC
[  150.562836] ideapad_laptop: ideapad_acpi_notify vpc: 100
[  152.094907] ideapad_laptop: ideapad_acpi_notify vpc: 2000
[  154.742909] ideapad_laptop: ideapad_wmi_notify value: d0

These are the events picked up with your ideapad-laptop patch.

ideapad_acpi_notify vpc: 100 
this is the mic mute key

ideapad_acpi_notify vpc: 2000 
this is the rfkill/airplane mode key

ideapad_wmi_notify value: d0
this is a custom "open app" key. Although it generates no key code, it produces this ACPI event: (picked up with acpi_listen)
8FC0DE0C-B4E4- 000000d0 00000000

Still nothing but grave silence from the brightness keys though.
Comment 12 Andy Shevchenko 2021-11-02 14:20:53 UTC
(In reply to Johannes P from comment #11)
> Still nothing but grave silence from the brightness keys though.

Which might be a sign that the key presses are handled by some microcontroller (EC?) with a custom firmware and being delivered to the OS in different way. It's also possible that the WMI can be used but these events have to be enabled somehow (obviously with ACPI methods or so).
Comment 13 Hans de Goede 2021-11-02 14:30:17 UTC
> Still nothing but grave silence from the brightness keys though.

Ok, I suspect that we need to enable reporting of events from them by some ACPI video call; and that we will then get events for them on the "Video Bus" input device.

I need to do a deep dive into your acpidump to see if I can find anything there. Mondays are my "look into odd bugs which need more then a quick 5 minute bugzilla reply" day, so I hope to make some time to look into this next Monday.

In the mean time I have one last 5 minutes idea :)  Let me attach another patch with some video-bus debugging added.
Comment 14 Hans de Goede 2021-11-02 14:53:32 UTC
Created attachment 299405 [details]
acpi-video event debug patch

Please give this a test spin and see if any events are printed to dmesg when pressing the brightness keys.

p.s.

We should really also hookup that WMI 0xd0 event to e.g. KEY_OPEN. Johannes, that would potentially make a good first kernel patch for you (assuming you haven't written kernel patches before).
Comment 15 Johannes Penßel 2021-11-02 22:06:46 UTC
Unfortunately, your latest patch did not yield any results either. :(

In the meantime, I have discovered another peculiarity: the ACPI/WMI events generated by this "open app" key are identical to the ones created by pressing Fn + ESC. (i.e. toggling Fn lock) In fact, the WMI 0xd0 event is already handled by ideapad driver in the ideapad_wmi_notify function. The only apparent difference between pressing "open app" and pressing Fn + ESC is that the latter also toggles the LED on the ESC key. This might be a firmware issue however, as this key does nothing in Windows either.

Thanks a lot for your support!
Comment 16 Johannes Penßel 2021-11-02 22:15:20 UTC
(In reply to Andy Shevchenko from comment #12)
> (In reply to Johannes P from comment #11)
> > Still nothing but grave silence from the brightness keys though.
> 
> Which might be a sign that the key presses are handled by some
> microcontroller (EC?) with a custom firmware and being delivered to the OS
> in different way. It's also possible that the WMI can be used but these
> events have to be enabled somehow (obviously with ACPI methods or so).

This is something that came to my mind as well. The keyboard backlight on this machine works in a similar way: It seems to be entirely handled by the EC, because it is in no way exposed to the OS, but can be toggled at any time, even during boot/shutdown/restart.
Comment 17 Johannes Penßel 2021-11-04 21:22:53 UTC
Looks like the mic mute key and the brightness keys do not behave identically with the ideapad-laptop module unloaded after all.

While playing around with ACPI EC debugging today, I was able to produce output for everything (including the otherwise "invisible" KB backlight toggle) except the brightness keys. This included the mic mute key and other keys reliant on ideapad-laptop. Without the module, pressing these keys still triggered EC debug output.

Just as a correction to my above claims.
Comment 18 Hans de Goede 2021-11-15 14:28:24 UTC
Just a quick update that this is still on my radar, but unfortunately so far I've not found the time to do a deep dive into the DSDT.
Comment 19 Johannes Penßel 2022-02-17 01:20:54 UTC
After doing some further testing, I am pretty sure that these keys are not handled by ACPI at all. Setting acpi.debug_level to maximum and going through the debug layers one by one, I was not able to produce any sort of response from them. I would really like to know what is going on here. 

Curiously, uninstalling the Lenovo hotkey driver in Windows does not break them there, so I guess they nonetheless must be using some kind of standardized method to communicate with the OS?
Comment 20 Hans de Goede 2022-04-12 18:45:35 UTC
So I just hit a similar problem on another laptop what helped there (it got the keys to send events on the "AT Translated Set 2 keyboard" device) is adding i8042.dritek=Y to the kernel commandline.

I think doing this is worth a try on your ideapad too.
Comment 21 Johannes Penßel 2022-04-12 21:46:15 UTC
Thank you for your advice, I really appreciate that you still keep my issue in mind. Unfortunately, still no success with i8042.dritek=1. 

After digging through Windows Event Viewer, I was able to figure out that the keys actually do trigger ACPI events there. When pressing brightness up/down, the corresponding EC query methods _Q12/_Q11 are executed, resulting in an ACPI brightness event.

            Method (_Q11, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
            {
                P80B = 0x11
                Notify (^^^GFX0.DD1F, 0x87) // Device-Specific
                Notify (VPC0, 0x80) // Status Change
            }

            Method (_Q12, 0, NotSerialized)  // _Qxx: EC Query, xx=0x00-0xFF
            {
                P80B = 0x12
                Notify (^^^GFX0.DD1F, 0x86) // Device-Specific
                Notify (VPC0, 0x80) // Status Change
            }

When calling the methods on Linux through the ACPI kernel debugger, the same thing happens. A brightness event is generated and the EC debug module prints this:

[ 1101.374238] ACPI: EC: 2: Increase command
[ 1101.374244] ACPI: EC: Command(WR_EC) started
[ 1101.374247] ACPI: EC: TASK (0)
[ 1101.374271] ACPI: EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1101.374275] ACPI: EC: EC_SC(W) = 0x81
[ 1101.374497] ACPI: EC: IRQ (1)
[ 1101.374532] ACPI: EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1101.374537] ACPI: EC: EC_DATA(W) = 0x00
[ 1101.374837] ACPI: EC: IRQ (1)
[ 1101.374872] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.374876] ACPI: EC: EC_DATA(W) = 0x10
[ 1101.375113] ACPI: EC: IRQ (1)
[ 1101.375145] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.375163] ACPI: EC: Command(WR_EC) stopped
[ 1101.375169] ACPI: EC: 1: Decrease command
[ 1101.375301] ACPI: EC: 2: Increase command
[ 1101.375304] ACPI: EC: Command(RD_EC) started
[ 1101.375307] ACPI: EC: TASK (0)
[ 1101.375331] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.375337] ACPI: EC: EC_SC(W) = 0x80
[ 1101.375564] ACPI: EC: IRQ (1)
[ 1101.375599] ACPI: EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1101.375603] ACPI: EC: EC_DATA(W) = 0x00
[ 1101.375853] ACPI: EC: IRQ (1)
[ 1101.375888] ACPI: EC: EC_SC(R) = 0x01 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=1
[ 1101.375908] ACPI: EC: EC_DATA(R) = 0x00
[ 1101.375922] ACPI: EC: Command(RD_EC) stopped
[ 1101.375926] ACPI: EC: 1: Decrease command
[ 1101.376178] ACPI: EC: 2: Increase command
[ 1101.376182] ACPI: EC: Command(WR_EC) started
[ 1101.376185] ACPI: EC: TASK (0)
[ 1101.376209] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.376214] ACPI: EC: EC_SC(W) = 0x81
[ 1101.376601] ACPI: EC: IRQ (1)
[ 1101.376636] ACPI: EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1101.376641] ACPI: EC: EC_DATA(W) = 0x00
[ 1101.377350] ACPI: EC: IRQ (1)
[ 1101.377385] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.377389] ACPI: EC: EC_DATA(W) = 0x1a
[ 1101.377671] ACPI: EC: IRQ (1)
[ 1101.377697] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.377712] ACPI: EC: Command(WR_EC) stopped
[ 1101.377715] ACPI: EC: 1: Decrease command
[ 1101.377802] ACPI: EC: 2: Increase command
[ 1101.377805] ACPI: EC: Command(RD_EC) started
[ 1101.377808] ACPI: EC: TASK (0)
[ 1101.377831] ACPI: EC: EC_SC(R) = 0x00 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=0
[ 1101.377836] ACPI: EC: EC_SC(W) = 0x80
[ 1101.378089] ACPI: EC: IRQ (1)
[ 1101.378124] ACPI: EC: EC_SC(R) = 0x08 SCI_EVT=0 BURST=0 CMD=1 IBF=0 OBF=0
[ 1101.378128] ACPI: EC: EC_DATA(W) = 0x00
[ 1101.378445] ACPI: EC: IRQ (1)
[ 1101.378469] ACPI: EC: EC_SC(R) = 0x01 SCI_EVT=0 BURST=0 CMD=0 IBF=0 OBF=1
[ 1101.378489] ACPI: EC: EC_DATA(R) = 0x00
[ 1101.378498] ACPI: EC: Command(RD_EC) stopped
[ 1101.378501] ACPI: EC: 1: Decrease command

But why doesn't this happen when pressing the actual key like on Windows?

As far as I can tell, Windows registers keycodes on the exact same keys as evtest, i.e. there are no codes registered when I press the brightness keys. (this also applies to mic mute, rfkill, etc...)
Comment 22 Hans de Goede 2022-04-25 16:30:13 UTC
Sorry for the slow reply (note I'm afraid my next reply will likely also be a bit slow).

Good find on the methods, so this means that the key-press events are supposed to come from the "Video Bus" device and as you point out for some reason this is not happening. I guess that the drivers/acpi/acpi_video.c is not making some call to enable the events which your laptop requires; or it is making a call which it should not make which disables the events...

One thing which I just noticed is that when passing acpi_backlight=video you get 2 acpi_video# backlight devices. Given that AFAICT your laptop has only a single GPU,  may be an indication of a problem.

I wonder what happens when you pass:

video.only_lcd=1 acpi_backlight=video

on the kernel cmdline, I would expect there to only be 1 acpi_video# backlight device under /sys/class/backlight then ?    And maybe this also fixes the brightness keys?

I assume that without any "acpi_backlight=xxx" on the kernel cmdline you only get the intel_backlight device, right? You could also check to see if adding just "video.only_lcd=1" to the kernel cmdline helps.

If that does not help, you may want to play with the acpi_video_bus_DOS() call from 
drivers/acpi/acpi_video.c .

This gets called from the acpi_video_bus_start_devices() function:

static int acpi_video_bus_start_devices(struct acpi_video_bus *video)
{
        return acpi_video_bus_DOS(video, 0,
                                  acpi_osi_is_win8() ? 1 : 0);
}

You may want to modify this trying the following:

        return acpi_video_bus_DOS(video, 3,
                                  acpi_osi_is_win8() ? 1 : 0);

Looking at your acpidump it seems the second parameter is ignored, so changing that will likely not matter.
Comment 23 Johannes Penßel 2022-04-26 12:54:51 UTC
No problem, this isn't exactly the most pressing of issues.

Looks like neither any combination of kernel parameters, nor the code change you suggested, leads to any difference in behaviour.

My laptop does have a 2nd GPU actually, a GeForce MX450 which is for all practical purposes disabled at runtime by the nouveau driver. But no matter if enabled or disabled in BIOS, there is only the intel_backlight device in sysfs without any kernel parameters, as expected. By the way, enabling the GPU results in this warning message at boot:

[    3.894320] ACPI: video: [Firmware Bug]: ACPI(PEGP) defines _DOD but not _DOS

Interestingly, I have noticed that booting with acpi_backlight=video results in just one acpi_video device appearing in sysfs now. (regardless if the GPU is activated or not) I'm not sure if this is due to a change in BIOS or the kernel, so just to be sure, I am uploading an acpi dump from the most recent BIOS release. 

Here is some additional info about the input devices registered by acpi video that might be helpful:

/proc/bus/input/devices

I: Bus=0019 Vendor=0000 Product=0006 Version=0000
N: Name="Video Bus"
P: Phys=LNXVIDEO/video/input0
S: Sysfs=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:00/LNXVIDEO:00/input/input8
U: Uniq=
H: Handlers=kbd event6 
B: PROP=0
B: EV=3
B: KEY=3e000b00000000 0 0 0

I: Bus=0019 Vendor=0000 Product=0006 Version=0000
N: Name="Video Bus"
P: Phys=LNXVIDEO/video/input0
S: Sysfs=/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/LNXVIDEO:01/input/input9
U: Uniq=
H: Handlers=kbd event7 
B: PROP=0
B: EV=3
B: KEY=3e000b00000000 0 0 0

Not sure if relevant, but something else I have noticed is that the brightness value recorded by the ideapad backlight device, which as mentioned does change when the brightness keys are pressed, corresponds to value B9 on the register map dumped by the kernel ec tool:

     00  01  02  03  04  05  06  07  08  09  0A  0B  0C  0D  0E  0F
00:  00  01  00  00  00  00  00  01  00  00  10  00  00  00  29  00 
10:  00  00  80  00  01  00  65  04  00  01  9a  20  00  02  00  00 
20:  02  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00 
30:  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00  00 
40:  00  00  00  00  00  00  00  00  00  00  00  00  8d  52  00  00 
50:  00  00  00  33  00  00  2d  00  00  00  00  00  00  00  00  00 
60:  00  80  02  03  02  00  00  08  ef  17  85  43  01  00  20  01 
70:  00  02  10  01  00  00  00  00  00  00  00  00  00  00  00  00 
80:  00  00  00  00  08  00  00  00  c0  00  93  00  20  01  00  4c 
90:  47  43  00  32  30  31  39  00  4c  31  39  4c  34  50  46  31 
A0:  00  01  00  80  00  00  c0  00  00  00  00  00  00  15  00  00 
B0:  2d  2f  00  2a  00  24  00  28  02  11  04  00  00  00  01  00 
C0:  30  00  ce  14  33  05  97  3e  10  3b  58  1b  9b  1b  4b  00 
D0:  00  00  00  00  00  00  00  23  2f  0c  00  00  00  00  00  00 
E0:  00  00  00  00  00  00  00  00  00  00  00  00  01  00  43  00 
F0:  01  00  00  40  90  51  80  43  00  00  00  00  00  00  00  00

This value persists across reboots, but does not change when pressing the brightness keys in Windows.
Comment 24 Johannes Penßel 2022-04-26 12:56:47 UTC
Created attachment 300805 [details]
acpidump BIOS version FHCN64WW, GPU enabled
Comment 25 Hans de Goede 2022-04-27 12:03:02 UTC
Hmm, your last comments make me wonder if that maybe the ideapad-laptop module is actually causing this problem and if iy t maybe somehow causes the EC to stop sending the events needed for the "Video bus" input device.

Can you try blacklisting the ideapad-laptop module and see if that makes a change? I guess you may loose functionality of some other hotkeys then, but lets worry about that latter and for now just see if this makes a change?
Comment 26 Johannes Penßel 2022-04-29 15:04:52 UTC
Blacklisting ideapad-laptop seems to have no impact on the brightness keys. Pressing them still results in the same register change outlined above, and nothing else. 

Note for sanity:
For debugging purposes, I am running video.ko with report_key_events=3. Please let me know in case that may be a source of trouble.
Comment 27 Hans de Goede 2022-04-29 16:29:50 UTC
(In reply to Johannes P from comment #26)
> Blacklisting ideapad-laptop seems to have no impact on the brightness keys.
> Pressing them still results in the same register change outlined above, and
> nothing else. 

Bummer, I'm afraid I'm all out of ideas then.
Comment 28 Johannes Penßel 2022-06-10 20:07:21 UTC
Finally, I have managed to get the keys working. 

Using ec-dump.exe, I figured out which EC registers hold different values on Windows compared to Linux. After writing 0x86 to offset 0xA3, (which is 0x80 on Linux by default) the keys started working instantly, with proper ACPI events and everything.
Comment 29 Hans de Goede 2022-06-12 15:25:04 UTC
(In reply to Johannes Penßel from comment #28)
> Finally, I have managed to get the keys working. 
> 
> Using ec-dump.exe, I figured out which EC registers hold different values on
> Windows compared to Linux. After writing 0x86 to offset 0xA3, (which is 0x80
> on Linux by default) the keys started working instantly, with proper ACPI
> events and everything.

That is some good detective work on your side!

May I ask how you are writing 0x86 to offset 0xA3 under Linux ?

I think this part of the DSDT is interesting:

        Device (EC0)
        {
            ...

            OperationRegion (ERAX, SystemMemory, 0xFE0B0400, 0xFF)
            Field (ERAX, ByteAcc, Lock, Preserve)
            {
                ...
                Offset (0xA3), 
                OSTY,   3, 
                    ,   1, 
                ADPI,   2, 
                    ,   1, 
                ADPT,   1, 
                ...
            }
            ...

            Method (_REG, 2, NotSerialized)  // _REG: Region Availability
            {
                If ((Arg0 == 0x03))
                {
                    ECAV = Arg1
                }

                If (((Arg0 == 0x03) && (Arg1 == One)))
                {
                    ...
                    If ((OSYS == 0x07DF))
                    {
                        Local0 = 0x06
                    }

                    If ((Acquire (LFCM, 0xA000) == Zero))
                    {
                        OSTY = Local0


Under Linux OSYS should be 0x06. So this in essence writes 0x06 to the lower 4 bits of the byte at offset 0xA3 of the MMIO region at 0xFE0B0400...

So maybe this is the path which we need to hit, but which for some reason is not being hit under Linux ... ?

We could try to confirm this with a DSDT override. where we add an extra
unconditional (sow without all the if-s):


                       OSTY = 0x06 

To the top of the _REG method above and then build a new DSDT and use the initrd override method to inject this. See:

https://docs.kernel.org/admin-guide/acpi/initrd_table_override.html

If you can give this a try that would be great. If the issue turns out to be that the _REG code is somehow not hitting the desired code-path then that likely is a generic issue caused by subtle differences between how Linux runs ACPI code vs how Windows does it. And figuring that out and fixing it will likely also help on other devices.
Comment 30 Hans de Goede 2022-06-12 15:32:24 UTC
If it turns out that the issue is that for some reason the "Local0 = 0x06" part of:

                    If ((OSYS == 0x07DF))
                    {
                        Local0 = 0x06
                    }

Does not execute then this is similar to:
https://bugzilla.redhat.com/show_bug.cgi?id=1842039

Which is also caused by an OSYS check for some reason not working. Note OSYS gets set to 0x07DF this bit of DSDT code:

                If (_OSI ("Windows 2015"))
                {
                    OSYS = 0x07DF
                }

Which should run early on, but on the 1842039 this for some reason is not working.

For some more background info, Linux will answer true for _OSI ("Windows 2015") which is asking the OS if it is "Windows 10", or IOW if it can handle modern ACPI features, which Linux can.

So assuming that this is the cause of 0xA3 is not being set to 0x86 (*), then getting to the bottom of this is definitely interesting.

*) Note I'm not even sure that the OSTY reference to address 0xFE0B04A3 is the A3 we are looking for ...
Comment 31 Johannes Penßel 2022-06-13 09:18:53 UTC
Thank you for your help!

The kernel ships with a tool to read / write to the EC. It is built by running "make ec" inside tools/power/acpi and relies on the module ec_sys (enabled by CONFIG_ACPI_EC_DEBUGFS). To enable EC write support, this module must be loaded with the parameter "write_support=1". Writing to a register works by running "./ec -w [offset] -v [value]".

Replacing all the OSYS if-conditionals inside _REG with just "Local0 = 0x06" indeed fixes the issue. Looks like _REG somehow does not receive a correct value for OSYS. I've tried changing _SB.PC00._INI so it unconditionally sets OSYS to 0x07DF, but this does not fix _REG. When executed in step-by-step mode with the kernel AML debugger, _SB.PC00._INI seems to function correctly by default anyway.

Here comes the interesting part: When running _REG (with arguments 0x03 and 0x01) inside the kernel AML debugger with unmodified ACPI tables, the method executes correctly and the keys start working.

My understanding of ACPI is limited, but my guess would be that for some reason, \_SB.PC00.LPCB.EC0._REG gets evaluated before _SB.PC00._INI, thus leaving the former with an incorrect value for OSYS. AFAICT, this does not look too dissimilar from the issue in the bugzilla thread you linked.
Comment 32 Hans de Goede 2022-06-13 10:30:41 UTC
(In reply to Johannes Penßel from comment #31)
> Thank you for your help!

You're welcome and thank you for your work on this, your APCI debugging skills are impressive!

> My understanding of ACPI is limited, but my guess would be that for some
> reason, \_SB.PC00.LPCB.EC0._REG gets evaluated before _SB.PC00._INI, thus
> leaving the former with an incorrect value for OSYS. AFAICT, this does not
> look too dissimilar from the issue in the bugzilla thread you linked.

Yes, I believe that that is what is happening too.

Looking at the code I have an idea how to fix this, I'm testing a patch for this now (to make sure it does not regress things on other hw, or at least on my laptop).
Comment 33 Hans de Goede 2022-06-13 13:17:38 UTC
Created attachment 301163 [details]
ACPICA: eval \_SB.PC00._INI and \_SB.PCI0._INI before running  _REG methods

As promised here is a kernel patch which will hopefully fix this, please give this a try.

If this works, please let me know if it is ok to add the following to the commit msg:

Reported-and-tested-by: Johannes Penßel <johannes.xxxxxxx@domain.tld>

But then when your real email address. Note this will expose your email address to the entire world (where as here in bugzilla it is only visible to users with a bugzilla account).

Alternatively I can also give you credit using just your name:

Reported-and-tested-by: Johannes Penßel
Comment 34 Johannes Penßel 2022-06-13 21:23:47 UTC
I am afraid to report that the patch does not work on my system, even after a full kernel rebuild:(

I have also tried simply replacing \\_SB with \\_SB.PC00 at line 166 of nsinit.c:

		status = acpi_get_handle(NULL, "\\_SB", &handle);
		if (ACPI_SUCCESS(status)) {
			memset(info.evaluate_info, 0,
			       sizeof(struct acpi_evaluate_info));

But that did not work either. Neither did removing the code block that evaluates all _REG methods.



Glad I can be of help! Regarding the mail address, I'd prefer <johannespenssel@posteo.net> :)
Comment 35 Hans de Goede 2022-06-14 13:41:09 UTC
I think I know what is going on here:

1. acpi_bus_init() calls acpi_ec_ecdt_probe()
2. acpi_ec_ecdt_probe() calls acpi_ec_setup()
3. acpi_ec_setup() calls ec_install_handlers()
4. ec_install_handlers() calls acpi_install_address_space_handler()
5. acpi_install_address_space_handler() calls _REG for Device (EC0)
6. acpi_bus_init() calls acpi_initialize_objects(), but _REG has already run

I've a plan for fixing this. I hope to have a set of patches for you to test in a couple of hours.
Comment 36 Hans de Goede 2022-06-14 18:48:06 UTC
Created attachment 301177 [details]
Attempt 2 at fixing this (3 patches in a tarbal)

Here is attempt 2 at fixing this. I just noticed looking at the tables of my X1C8 that this series will not work entirely correct here, since those depend on having the EC OpRegion working from \_SB.PCI0._INI. So this needs more work.

Still if you've time it would be good if you can give this a shot to see if this early version does fix things for you.
Comment 37 Johannes Penßel 2022-06-14 19:14:39 UTC
Works like a charm! Kernel log looks normal as well.
Comment 38 Andy Shevchenko 2022-06-15 09:39:16 UTC
(In reply to Hans de Goede from comment #36)
> Created attachment 301177 [details]
> Attempt 2 at fixing this (3 patches in a tarbal)

I would propose, if it's feasible, to evaluate _HID/_CID (to be PCI host bridge compatible) of the first level of \_SB and evaluate _INI of all of them. The listing of the names seems a bit fragile, while I don't believe there will be many new entries. That said, IIRC, \_SB.PCI is also present in some DSDTs.
Comment 39 Hans de Goede 2022-06-15 09:46:44 UTC
(In reply to Andy Shevchenko from comment #38)
> (In reply to Hans de Goede from comment #36)
> > Created attachment 301177 [details]
> > Attempt 2 at fixing this (3 patches in a tarbal)
> 
> I would propose, if it's feasible, to evaluate _HID/_CID (to be PCI host
> bridge compatible) of the first level of \_SB and evaluate _INI of all of
> them. The listing of the names seems a bit fragile, while I don't believe
> there will be many new entries. That said, IIRC, \_SB.PCI is also present in
> some DSDTs.

That is an interesting proposal, but this happens really early on and for some devices _HID / _CID is not just an ACPI Name() field but an actual Method which does a bunch of things, like e.g. checking GPIOs to see which touchscreen/pad model is installed. 

Although it is unlikely to encounter such _CID / _HID methods on devices directly under _SB, it cannot be ruled out, so calling _CID + _HID for all devices under _SB seems unwise.

Also note that these patches still need to change a bit, as I mentioned they cause trouble on a X1C8. I've a plan for this which I plan to implement today and then I'll send out a RFC series to the linux-acpi list (and also attach it here for Johannes to test the new version).
Comment 40 Hans de Goede 2022-06-15 16:36:15 UTC
Created attachment 301181 [details]
Attempt 3 at fixing this (4 patches in a tarbal)

Ok, this patch-set should still fix things on your IdeaPad while also not causing any ACPI errors on my X1C8.

So this should be it, assuming it passes review.

Please give this a try and let me know how it goes.

I'll also submit this upstream as a RFC to get some discussion going about the chosen approach, I'll put you in the Cc of that.
Comment 41 Johannes Penßel 2022-06-15 20:27:03 UTC
I'm happy to report that on my end, there are still zero issues and working brightness keys with the latest patch set.
Comment 42 tinozzo123 2022-06-26 15:38:58 UTC
I have zero issues on an IdeaPad 5 14ARE05.
Comment 43 Philipp Jungkamp 2022-07-03 20:44:32 UTC
I can report that this also fixes the brightness keys on the 'Lenovo Yoga 9 14IAP7'.

I was just trying to trace the ACPI events on Windows when I found this thread while browsing the Mailing list in the train.

(In reply to Johannes Penßel from comment #15)
> Unfortunately, your latest patch did not yield any results either. :(
> 
> In the meantime, I have discovered another peculiarity: the ACPI/WMI events
> generated by this "open app" key are identical to the ones created by
> pressing Fn + ESC. (i.e. toggling Fn lock) In fact, the WMI 0xd0 event is
> already handled by ideapad driver in the ideapad_wmi_notify function. The
> only apparent difference between pressing "open app" and pressing Fn + ESC
> is that the latter also toggles the LED on the ESC key. This might be a
> firmware issue however, as this key does nothing in Windows either.
> 
> Thanks a lot for your support!

My device has multiple of these open app keys (Support, Favorite App, Virtual Background, Sound Profile, Dark Mode Toggle), I just wrote a short WMI driver to get them working. Have you figured out how to get these working for yourself? I presume the open application key you were talking about is the "star with an S inside", which is also present on my device.
Comment 44 Hans de Goede 2022-07-04 10:12:08 UTC
Created attachment 301329 [details]
[PATCH 1/2] ACPICA: Make address-space-handler install and _REG execution 2 separate steps
Comment 45 Hans de Goede 2022-07-04 10:12:33 UTC
Created attachment 301330 [details]
[PATCH 2/2] ACPI: EC: fix ECDT probe ordering issues
Comment 46 Hans de Goede 2022-07-04 10:14:06 UTC
The Linux kernel ACPI subsystem maintainer has requested to solve this in a different way.

Johannes, can you please test the 2 patches which I've just attached?  (these replace the 4 previous patches).

Philipp, if you can test these 2 patches too that would be great.
Comment 47 Hans de Goede 2022-07-04 10:34:04 UTC
(In reply to Philipp Jungkamp from comment #43)
> I can report that this also fixes the brightness keys on the 'Lenovo Yoga 9
> 14IAP7'.

That is great news.

> My device has multiple of these open app keys (Support, Favorite App,
> Virtual Background, Sound Profile, Dark Mode Toggle), I just wrote a short
> WMI driver to get them working.

Cool, please submit the new driver upstream to: platform-driver-x86@vger.kernel.org
and then I can review it and once it passes review merge it into the mainline kernel.
Comment 48 Johannes Penßel 2022-07-05 07:59:28 UTC
(In reply to Hans de Goede from comment #46)
> Johannes, can you please test the 2 patches which I've just attached? 
> (these replace the 4 previous patches).

On my system, the two new patches are working just as well as the four older ones did.
Comment 49 Hans de Goede 2022-07-10 15:30:26 UTC
(In reply to Johannes Penßel from comment #48)
> (In reply to Hans de Goede from comment #46)
> > Johannes, can you please test the 2 patches which I've just attached? 
> > (these replace the 4 previous patches).
> 
> On my system, the two new patches are working just as well as the four older
> ones did.

Thank you for testing we are discussing how to move forward with this upstream.
Comment 50 Philipp Jungkamp 2022-07-30 19:04:25 UTC
A late response from me: Your new patches are also working wonderfully, I just applied and tested them!
Comment 51 cris223 2022-07-31 01:46:29 UTC
I tested the 2 patches and i can confirm works fine on my Lenovo Yoga 7i 16IAP7
Philipp's patch for favorites keys works good too!
Thank you guys.
Comment 52 cris223 2022-07-31 03:41:25 UTC
I've compiled the new ver 5.18.5 linux-xanmod-edge with the patches given by Phillip's github
https://github.com/PJungkamp/yoga9-linux/tree/main/kernel-patches

on my github
https://github.com/322sirc/linux-xanmod-edge
Comment 53 Hans de Goede 2022-10-03 14:32:05 UTC
Created attachment 302925 [details]
[PATCH 1/4] ACPICA: include/acpi/acpixf.h: Fix indentation

Sorry for the long silence, I'm afraid my workload is causing me to sometimes be a bit slow with following up on some bugs.

I have prepared a new set of patches (with only minor changes) based on upstream feedback which I will send upstream soon.

If possible please give this new patch series a try. It is based on top of the 6.0 kernel.
Comment 54 Hans de Goede 2022-10-03 14:32:48 UTC
Created attachment 302926 [details]
[PATCH 2/4] ACPICA: Allow address_space_handler Install and _REG execution as 2 separate steps
Comment 55 Hans de Goede 2022-10-03 14:33:19 UTC
Created attachment 302927 [details]
[PATCH 3/4] ACPI: EC: Fix EC address space handler unregistration
Comment 56 Hans de Goede 2022-10-03 14:33:56 UTC
Created attachment 302928 [details]
[PATCH 4/4] ACPI: EC: fix ECDT probe ordering issues
Comment 57 Johannes Penßel 2022-10-05 23:21:38 UTC
On my machine, the new patch set is working flawlessly with Linux 6.0.

No problem, thank you very much for your time and effort working on this!
Comment 58 Hans de Goede 2022-10-06 07:38:23 UTC
(In reply to Johannes Penßel from comment #57)
> On my machine, the new patch set is working flawlessly with Linux 6.0.

Great, thank you for testing!
Comment 59 Gerald Nunn 2022-10-29 02:48:10 UTC
Tested the patches on a Lenovo Slim 7 (i7-1260p) which is the updated version of the Ideapad Slim 7 Pro and it works great. Using Arch with kernel 6.0.5
Comment 60 Hans de Goede 2022-10-30 09:54:47 UTC
(In reply to Gerald Nunn from comment #59)
> Tested the patches on a Lenovo Slim 7 (i7-1260p) which is the updated
> version of the Ideapad Slim 7 Pro and it works great. Using Arch with kernel
> 6.0.5

That is good to hear, we are waiting for:

https://github.com/acpica/acpica/pull/786

to get merged and the fixes should land in the mainline kernel pretty quickly after that.
Comment 61 Takashi Iwai 2023-01-24 08:20:57 UTC
*** Bug 216933 has been marked as a duplicate of this bug. ***
Comment 62 Hans de Goede 2023-01-24 09:32:38 UTC
(In reply to Hans de Goede from comment #60)
> That is good to hear, we are waiting for:
> 
> https://github.com/acpica/acpica/pull/786
> 
> to get merged and the fixes should land in the mainline kernel pretty
> quickly after that.

The fixes for this have been merged into the 6.2 series, closing.

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