Message-ID: <d452fc76-26a1-eb08-d855-5b9d5fabb039@redhat.com>
Date:   Sat, 21 Oct 2023 11:46:28 +0200
From:   Hans de Goede <hdegoede@...hat.com>
To:     James John <me@...jajo.com>
Cc:     Corentin Chary <corentin.chary@...il.com>,
        Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        Mark Gross <markgross@...nel.org>,
        platform-driver-x86@...r.kernel.org,
        acpi4asus-user@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: PROBLEM: asus_nb_wmi sends KEY_BRIGHTNESSDOWN on pressing CAPS
 Lock and PrntScrn on Zenbook S 13 UX5304VA

Hi James,

On 10/20/23 01:22, James John wrote:
> Hello Hans,
> 
> Thank you for your support so far. I really appreciate this.
> 
> I have always wanted to contribute to the kernel, so, this is fun for me! :)

That is great and thank you for all your help with solving this.

> 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.
> 
> I do not think these are multiple events. There are different. One has the value of 0, the other has value of 1.
> I am not sure what they mean. I initially thought it could be keydown and keyup events, but it is not, because
> on pressing the keydown, they are still both reported. I also think the desktops handle this, maybe by filtering out
> 0 values. I use KDE Plasma, and I still have 5% step despite evtest reporting these 2 events.

The 1 / 0 events are indeed press / release events that is
not the problem, the problem is that a single keypress reports
these events on 2 different /dev/input/event# nodes.

Interesting that this is not a problem for KDE, I know it is
a problem for GNOME. I guess KDE may do some filtering of
the duplicate events itself.

> I have applied the last 2 patches.
> 
> 1. Show no output for capslock / printscreen
> 
> Correct. These keys are no longer captured by Asus WMI hotkeys
> 
> 2. Show KEY_SELECTIVE_SCREENSHOT events for the
>    "Screen Capture" hotkey.
> 
> I am not sure I am getting KEY_SELECTIVE_SCREENSHOT event for the "Screen Capture" hotkey. This is what I get:
> Event: time 1697757579.588239, type 4 (EV_MSC), code 4 (MSC_SCAN), value 2a
> Event: time 1697757579.588239, type 1 (EV_KEY), code 634 (?), value 1
> Event: time 1697757579.588239, -------------- SYN_REPORT ------------
> Event: time 1697757579.588244, type 1 (EV_KEY), code 634 (?), value 0
> Event: time 1697757579.588244, -------------- SYN_REPORT ------------

This is actually the correct output, 634 is 0x27a hexadecimal and:

/usr/include/linux/input-event-codes.h :

/* Select an area of screen to be copied */
#define KEY_SELECTIVE_SCREENSHOT        0x27a

This is a somewhat (but not really) recent addition to the list
of KEY_foo defines, so I guess you are just using a somewhat old
evtest which does not know this code yet.

> 
> And this is what I get for "Screen Capture" hotkey, from the debug you placed
> [ 1096.691389] asus_wmi: raw event code 0x2a
> [ 1096.691446] asus_wmi: raw event code 0xffffffffffffffff
> [ 1097.982976] asus_wmi: raw event code 0x2a
> [ 1097.983032] asus_wmi: raw event code 0xffffffffffffffff
> 
> 
> 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"
> 
> Yes, correct. No output from Asus WMI hotkeys, but there an output from Video bus

Great, that means that everything works as it should now, thank you.

Regards,

Hans






> On 18/10/2023 19:35, Hans de Goede wrote:
>> Hi James,
>>
>> On 10/18/23 02:17, me@...jajo.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
> 

Powered by blists - more mailing lists