Re: PROBLEM: asus_nb_wmi sends KEY_BRIGHTNESSDOWN on pressing CAPS Lock and PrntScrn on Zenbook S 13 UX5304VA

[Hans de Goede <hdegoede@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2576 bytes --]

Hi James,

On 10/18/23 02:17, me@donjajo.com wrote:
> Hi Hans,
> 
> I hope you are feeling better now.
> Thank you so much for your support in resolving this.
> 
>> I assume that the first "BACKLIGHT BUTTON" is the backlight DOWN button ?
> Yes. Correct.
> 
> 
>> 2. Can you please run:
>>
>> sudo evtest and then select the "ACPI video bus" (or something
>> similar) device and see if that reports brightness up/down
>> keypresses?  And then do the same thing for the
>> "Asus WMI hotkeys" device ? I expect the Asus WMI hotkeys
>> device to only report brightness up keypresses (after my
>> hwdb "fix") while I expect brightness-up events to get
>> reported twice, by both the "ACPI video bus" device and
>> the "Asus WMI hotkeys" device.
> Done and attached.
> 
>> Can you confirm this? This also means that brightness
>> up will take bigger steps (2 steps per keypress) then
>> brightness down, right ?
> I am not sure I understand what you mean here. But I have attached the output here

The 2 evtest logs show that each brightness up/down keypress
gets reported twice, once by the "ACPI video bus" device and
once bythe "Asus WMI hotkeys" device.

This means that in e.g. GNOME the brightness will move
up / down by 2 steps for each step, reducing the amount
of steps from 20 to 10, or iow making each step twice
as big. Especially at the low end of the brightness
scale this may be an issue since steeping by 5% there
can already make a big difference and this double
key press reporting now changes this into stepping
by 10% at a time.

> After applying your patch, it seems to have fixed the issue!

Thank you for all the testing and other then the double
keypress issue + the unknown code messages everything
now looks good!

I have applied 2 more patches the first one fixes the
unknown code messages and adds a mapping for the
"Screen Capture" hotkey. The second test filters out
the duplicate (duplicate with the "ACPI video bus")
brightness up/down events.

It would be great if you can add these on top of
the previous 2 patches and then run one last
test for me:

Run evtest on the "Asus WMI hotkeys" device this should now:

1. Show no output for capslock / printscreen

2. Show KEY_SELECTIVE_SCREENSHOT events for the
   "Screen Capture" hotkey.

3. Show no output for brightness up/down,
   yet brightness up/down should still work since
   these are also reported by the "ACPI video bus"

It would be great if you can confirm for each of these
that this behaves as expected with the 2 extra patches
applied on top of the previous patches.

Regards,

Hans

[-- Attachment #2: 0001-platform-x86-asus-wmi-Map-0x2a-code-Ignore-0x2b-and-.patch --]
[-- Type: text/x-patch, Size: 1785 bytes --]

From a2533bd573eb7a762306e83d99f624cbabad7d19 Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Tue, 17 Oct 2023 10:35:14 +0200
Subject: [PATCH 1/2] platform/x86: asus-wmi: Map 0x2a code, Ignore 0x2b and
 0x2c events

Newer Asus laptops send the following new WMI event codes when some
of the F1 - F12 "media" hotkeys are pressed:

0x2a Screen Capture
0x2b PrintScreen
0x2c CapsLock

Map 0x2a to KEY_SELECTIVE_SCREENSHOT mirroring how similar hotkeys
are mapped on other laptops.

PrintScreem and CapsLock are also reported as normal PS/2 keyboard events,
map these event codes to KE_IGNORE to avoid "Unknown key code 0x%x\n" log
messages.

Reported-by: James John <me@donjajo.com>
Closes: https://lore.kernel.org/platform-driver-x86/a2c441fe-457e-44cf-a146-0ecd86b037cf@donjajo.com/
Closes: https://bbs.archlinux.org/viewtopic.php?pid=2123716
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-nb-wmi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index d85d895fee89..df1db54d4e18 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -531,6 +531,9 @@ static void asus_nb_wmi_quirks(struct asus_wmi_driver *driver)
 static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_KEY, ASUS_WMI_BRN_DOWN, { KEY_BRIGHTNESSDOWN } },
 	{ KE_KEY, ASUS_WMI_BRN_UP, { KEY_BRIGHTNESSUP } },
+	{ KE_KEY, 0x2a, { KEY_SELECTIVE_SCREENSHOT } },
+	{ KE_IGNORE, 0x2b, }, /* PrintScreen (also send via PS/2) on newer models */
+	{ KE_IGNORE, 0x2c, }, /* CapsLock (also send via PS/2) on newer models */
 	{ KE_KEY, 0x30, { KEY_VOLUMEUP } },
 	{ KE_KEY, 0x31, { KEY_VOLUMEDOWN } },
 	{ KE_KEY, 0x32, { KEY_MUTE } },
-- 
2.41.0


[-- Attachment #3: 0002-platform-x86-asus-wmi-Do-not-report-brightness-up-do.patch --]
[-- Type: text/x-patch, Size: 2620 bytes --]

From ec01cacaf68fc00c1d5fffd2ce8e0bd03d2753bd Mon Sep 17 00:00:00 2001
From: Hans de Goede <hdegoede@redhat.com>
Date: Wed, 18 Oct 2023 11:47:28 +0200
Subject: [PATCH 2/2] platform/x86: asus-wmi: Do not report brightness up/down
 keys when also reported by acpi_video

For a long time now the acpi_video driver reports evdev brightness up/down
key events for the brightness hotkeys on most (non ancient) laptops.

asus-wmi also reports evdev brightness up/down key events for these
keys leading to each press being reported twice and e.g. GNOME increasing
the brightness by 2 steps instead of 1 step.

Use the acpi_video_handles_brightness_key_presses() helper to detect if
acpi_video is reporting brightness key-presses and if it is then don't
report the same events also from the asus-wmi driver.

Note there is a chance that this may lead to regressions where
the brightness hotkeys stop working because they are not actually
reported by the acpi_video driver. Unfortunately the only way to
find out if this is a problem is to try.

To at least avoid regressions on old hw using the eeepc-wmi driver,
implement this as a key filter in asus-nb-wmi so that the eeepc-wmi
driver is not affected.

Reported-by: James John <me@donjajo.com>
Closes: https://lore.kernel.org/platform-driver-x86/a2c441fe-457e-44cf-a146-0ecd86b037cf@donjajo.com/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-nb-wmi.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index df1db54d4e18..9aa1226e74e6 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -16,6 +16,8 @@
 #include <linux/dmi.h>
 #include <linux/i8042.h>
 
+#include <acpi/video.h>
+
 #include "asus-wmi.h"
 
 #define	ASUS_NB_WMI_FILE	"asus-nb-wmi"
@@ -606,6 +608,19 @@ static const struct key_entry asus_nb_wmi_keymap[] = {
 	{ KE_END, 0},
 };
 
+static void asus_nb_wmi_key_filter(struct asus_wmi_driver *asus_wmi, int *code,
+				   unsigned int *value, bool *autorelease)
+{
+	switch (*code) {
+	case ASUS_WMI_BRN_DOWN:
+	case ASUS_WMI_BRN_UP:
+		if (acpi_video_handles_brightness_key_presses())
+			*code = ASUS_WMI_KEY_IGNORE;
+
+		break;
+	}
+}
+
 static struct asus_wmi_driver asus_nb_wmi_driver = {
 	.name = ASUS_NB_WMI_FILE,
 	.owner = THIS_MODULE,
@@ -614,6 +629,7 @@ static struct asus_wmi_driver asus_nb_wmi_driver = {
 	.input_name = "Asus WMI hotkeys",
 	.input_phys = ASUS_NB_WMI_FILE "/input0",
 	.detect_quirks = asus_nb_wmi_quirks,
+	.key_filter = asus_nb_wmi_key_filter,
 };
 
 
-- 
2.41.0