Re: [PATCH 2/6] platform/x86/uniwill: Handle more WMI events required for TUXEDO devices

[Armin Wolf <W_Armin@gmx.de>]

Am 18.11.25 um 15:27 schrieb Werner Sembach:

>
> Am 18.11.25 um 14:48 schrieb Armin Wolf:
>> Am 18.11.25 um 14:29 schrieb Werner Sembach:
>>
>>>
>>> Am 18.11.25 um 14:12 schrieb Armin Wolf:
>>>> Am 18.11.25 um 13:45 schrieb Werner Sembach:
>>>>
>>>>>
>>>>> Am 18.11.25 um 12:08 schrieb Armin Wolf:
>>>>>> Am 17.11.25 um 14:23 schrieb Werner Sembach:
>>>>>>
>>>>>>> Handle some more WMI events that are triggered on TUXEDO devices.
>>>>>>>
>>>>>>> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
>>>>>>> ---
>>>>>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 19 
>>>>>>> ++++++++++++++++++-
>>>>>>>   drivers/platform/x86/uniwill/uniwill-wmi.h  |  2 ++
>>>>>>>   2 files changed, 20 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/platform/x86/uniwill/uniwill-acpi.c 
>>>>>>> b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>>>>>> index 29bb3709bfcc8..0cb86a701b2e1 100644
>>>>>>> --- a/drivers/platform/x86/uniwill/uniwill-acpi.c
>>>>>>> +++ b/drivers/platform/x86/uniwill/uniwill-acpi.c
>>>>>>> @@ -371,9 +371,11 @@ static const struct key_entry 
>>>>>>> uniwill_keymap[] = {
>>>>>>>         /* Reported in manual mode when toggling the airplane 
>>>>>>> mode status */
>>>>>>>       { KE_KEY,       UNIWILL_OSD_RFKILL, { KEY_RFKILL }},
>>>>>>> +    { KE_IGNORE,    UNIWILL_OSD_RADIOON, { KEY_UNKNOWN }},
>>>>>>> +    { KE_IGNORE,    UNIWILL_OSD_RADIOOFF, { KEY_UNKNOWN }},
>>>>>>>         /* Reported when user wants to cycle the platform 
>>>>>>> profile */
>>>>>>> -    { KE_IGNORE, UNIWILL_OSD_PERFORMANCE_MODE_TOGGLE, { 
>>>>>>> KEY_UNKNOWN }},
>>>>>>> +    { KE_KEY, UNIWILL_OSD_PERFORMANCE_MODE_TOGGLE, { KEY_F14 }},
>>>>>>
>>>>>> I am currently working a patch adding platform profile support, 
>>>>>> so this event would
>>>>>> be handled inside the kernel on models with platform profile 
>>>>>> support.
>>>>>
>>>>> For tuxedo devices we have profiles managed in userspace that do 
>>>>> additional things. So we need a way to handle this in userspace.
>>>>>
>>>> Do these things have something to do with the uniwill EC? If so 
>>>> then we should implement those inside the driver
>>>> itself. The control center can then poll the platform profile sysfs 
>>>> file to get notified when platform_profile_cycle()
>>>> is executed to perform additional actions.
>>> Not exclusively, e.g. one thing is display brightness.
>>
>> And you cannot poll the sysfs interface?
> I can't follow you atm?

I meant to ask whether or not your application could poll the platform profile sysfs interface for changes instead of
listing for the F14 key.

Thanks,
Armin Wolf

>>
>>>>
>>>>> The 2 things I can spontaneously think of would be a sysfs toggle 
>>>>> or 2 different UNIWILL_FEATURE_* defines.
>>>>>
>>>> TPH i would love to have an ordinary keycode allocated for that if 
>>>> the above does not work for you. There already
>>>> exists KEY_PERFORMANCE, so adding something like 
>>>> KEY_PERFORMANCE_CYCLE should be possible.
>>>
>>> New keycodes won't work on X11, I don't know the reason, but X11 
>>> only supports a max of 248 keycodes
>>>
>>> That's why for example touchpad toggle is bound to F21 e.g. here 
>>> https://elixir.bootlin.com/linux/v6.17.8/source/drivers/platform/x86/lg-laptop.c#L106 
>>> .
>>>
>> Oh no. In this case using F14 is fine.
>>
>>
>> Thanks,
>> Armin Wolf
>>
>>>>
>>>>>>
>>>>>>>         /* Reported when the user wants to adjust the brightness 
>>>>>>> of the keyboard */
>>>>>>>       { KE_KEY,       UNIWILL_OSD_KBDILLUMDOWN, { 
>>>>>>> KEY_KBDILLUMDOWN }},
>>>>>>> @@ -382,11 +384,19 @@ static const struct key_entry 
>>>>>>> uniwill_keymap[] = {
>>>>>>>       /* Reported when the user wants to toggle the microphone 
>>>>>>> mute status */
>>>>>>>       { KE_KEY,       UNIWILL_OSD_MIC_MUTE, { KEY_MICMUTE }},
>>>>>>>   +    /* Reported when the user wants to toggle the mute status */
>>>>>>> +    { KE_IGNORE,    UNIWILL_OSD_MUTE, { KEY_MUTE }},
>>>>>>
>>>>>> Why is this event being ignored?
>>>>> Because the UNIWILL_OSD_MUTE event is sent in addition to the mute 
>>>>> key event, so not ignoring it here would result in a double trigger.
>>>>
>>>> I understand.
>>>>
>>>>>>
>>>>>>> +
>>>>>>>       /* Reported when the user locks/unlocks the Fn key */
>>>>>>>       { KE_IGNORE,    UNIWILL_OSD_FN_LOCK, { KEY_FN_ESC }},
>>>>>>>         /* Reported when the user wants to toggle the brightness 
>>>>>>> of the keyboard */
>>>>>>>       { KE_KEY,       UNIWILL_OSD_KBDILLUMTOGGLE, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>> +    { KE_KEY,       UNIWILL_OSD_KB_LED_LEVEL0, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>> +    { KE_KEY,       UNIWILL_OSD_KB_LED_LEVEL1, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>> +    { KE_KEY,       UNIWILL_OSD_KB_LED_LEVEL2, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>> +    { KE_KEY,       UNIWILL_OSD_KB_LED_LEVEL3, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>> +    { KE_KEY,       UNIWILL_OSD_KB_LED_LEVEL4, { 
>>>>>>> KEY_KBDILLUMTOGGLE }},
>>>>>>>         /* FIXME: find out the exact meaning of those events */
>>>>>>>       { KE_IGNORE, UNIWILL_OSD_BAT_CHARGE_FULL_24_H, { 
>>>>>>> KEY_UNKNOWN }},
>>>>>>> @@ -395,6 +405,9 @@ static const struct key_entry 
>>>>>>> uniwill_keymap[] = {
>>>>>>>       /* Reported when the user wants to toggle the benchmark 
>>>>>>> mode status */
>>>>>>>       { KE_IGNORE, UNIWILL_OSD_BENCHMARK_MODE_TOGGLE, { 
>>>>>>> KEY_UNKNOWN }},
>>>>>>>   +    /* Reported when the user wants to toggle the webcam */
>>>>>>> +    { KE_IGNORE,    UNIWILL_OSD_WEBCAM_TOGGLE, { KEY_UNKNOWN }},
>>>>>>
>>>>>> Same as above.
>>>>>
>>>>> Same as above ;)
>>>>>
>>>>> At least iirc, would have to double check
>>>>>
>>>> Ok.
>>>>
>>>> Thanks,
>>>> Armin Wolf
>>>>
>>>>>>
>>>>>>> +
>>>>>>>       { KE_END }
>>>>>>>   };
>>>>>>>   @@ -1247,6 +1260,10 @@ static int uniwill_notifier_call(struct 
>>>>>>> notifier_block *nb, unsigned long action
>>>>>>>           }
>>>>>>>           mutex_unlock(&data->battery_lock);
>>>>>>>   +        return NOTIFY_OK;
>>>>>>> +    case UNIWILL_OSD_DC_ADAPTER_CHANGED:
>>>>>>> +        // noop for the time being
>>>>>>
>>>>>> Wrong comment style, please use /* */.
>>>>> ack
>>>>>>
>>>>>> Thanks,
>>>>>> Armin Wolf
>>>>>>
>>>>>>> +
>>>>>>>           return NOTIFY_OK;
>>>>>>>       default:
>>>>>>>           mutex_lock(&data->input_lock);
>>>>>>> diff --git a/drivers/platform/x86/uniwill/uniwill-wmi.h 
>>>>>>> b/drivers/platform/x86/uniwill/uniwill-wmi.h
>>>>>>> index 2bf69f2d80381..48783b2e9ffb9 100644
>>>>>>> --- a/drivers/platform/x86/uniwill/uniwill-wmi.h
>>>>>>> +++ b/drivers/platform/x86/uniwill/uniwill-wmi.h
>>>>>>> @@ -113,6 +113,8 @@
>>>>>>>     #define UNIWILL_OSD_BENCHMARK_MODE_TOGGLE    0xC0
>>>>>>>   +#define UNIWILL_OSD_WEBCAM_TOGGLE        0xCF
>>>>>>> +
>>>>>>>   #define UNIWILL_OSD_KBD_BACKLIGHT_CHANGED    0xF0
>>>>>>>     struct device;
>>>>>
>