Message-ID: <73fbfe55-237b-48d7-8e6e-9bf1b0e71036@tuxedocomputers.com>
Date: Mon, 24 Nov 2025 18:43:48 +0100
From: Werner Sembach <wse@...edocomputers.com>
To: Armin Wolf <W_Armin@....de>, hansg@...nel.org,
 ilpo.jarvinen@...ux.intel.com
Cc: platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] platform/x86/uniwill: Handle more WMI events required
 for TUXEDO devices


Am 23.11.25 um 00:54 schrieb Armin Wolf:
> Am 20.11.25 um 23:06 schrieb Werner Sembach:
>
>>
>> Am 20.11.25 um 14:40 schrieb Armin Wolf:
>>> Am 20.11.25 um 11:42 schrieb Werner Sembach:
>>>
>>>>
>>>> Am 20.11.25 um 01:53 schrieb Armin Wolf:
>>>>> Am 18.11.25 um 16:05 schrieb Werner Sembach:
>>>>>
>>>>>>
>>>>>> Am 18.11.25 um 15:41 schrieb Armin Wolf:
>>>>>>> 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@...edocomputers.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.
>>>>>> But the platform profiles are a fixed number? TCC currently allows an 
>>>>>> arbitrary amount of profiles being created.
>>>>>
>>>>> With "poll the platform profile sysfs interface" i meant that you could 
>>>>> use poll() (https://linux.die.net/man/2/poll)
>>>>> or epoll() on the sysfs file containing the current platform profile.
>>>> Sorry i think i still don't completely get what you mean with platform 
>>>> profile. I assume you have a poc on github? If not can you give me a short 
>>>> overview?
>>>
>>> Example code, might not work:
>>>
>>> from select import poll, POLLPRI
>>>
>>> fd = open("|/sys/firmware/acpi/platform_profile", "r") p = poll() 
>>> p.register(fd.fileno(), POLLPRI) # Wait till platform profile changes 
>>> p.poll() print("Platform profile changed") This however comes with the 
>>> drawback that you cannot prevent the platform profile from cycling. If you 
>>> want to do that manually depending on the settings inside your custom 
>>> profiles, then maybe we can keep the F14 hack for now. I will then add a 
>>> module option when adding platform profile support to select between 
>>> platform_profile_cycle() and the F14 keycode. Does this sound OK?|
>>
>> a sorry i was imprecise, i wanted to know the kernelspace implementation.
>>
> Take a look at 
> https://github.com/Wer-Wolf/uniwill-laptop/tree/platform_profile for the 
> prototype.
thanks
>
> The function platform_profile_cycle() is defined inside 
> drivers/acpi/platform_profile.
>
>> But let me sum up what i think you mean:
>>
>> Platform profiles are in driver predefined profiles like: Power Save, 
>> Balanced, Performance, and Custom.
>>
>> When you press the button you want to cycle through the profiles (except 
>> custom I guess?).
>>
>> Only in Custom things like cTGP can be directly controlled by userspace via 
>> sysfs (otherwise the sysfs value is ignored?)
>>
> Correct.
>
>> Maybe an elegant solution would be that upon boot for example "Balanced" is 
>> selected and when being in one of the predefined profiles the button cycles 
>> them. But once Custom get selected via sysfs the button starts sending a 
>> button press as the driver now expects everything to be handled by userspace. 
>> Bonus points if userspace can read out what the predefined profiles actually 
>> set to, for example, use that as initialization for custom profiles.
>>
> Agreed, i will implement this behavior once my testers give me feedback.
thanks
>
> Thanks,
> Armin Wolf
>
>>>
>>>>>
>>>>> Anyway, i attached the patch with the device descriptor infrastructure. 
>>>>> The callback called during probe cannot modify
>>>>> the feature bitmap anymore, but i suggest that you simply set the limit 
>>>>> for cTGP to zero. The code responsible for
>>>>> initializing cTGP support can then check if the cTGP limit is zero and 
>>>>> return early.
>>>>
>>>> I wonder if we should directly put that into a formal quirk list. Opinions?
>>>>
>>>> Best regards,
>>>>
>>>> Werner
>>>>
>>> The problem is that the quirk list will become RO before the driver can 
>>> access the EC, so we have to use uniwill_data
>>> for storing this information.
>>>
>>> Thanks,
>>> Armin Wolf
>>>
>>>>>
>>>>> Thanks,
>>>>> Armin Wolf
>>>>>
>>>>>>>
>>>>>>> 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;
>>>>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>

Powered by blists - more mailing lists