Re: [PATCH v15 0/6] Merge redmi-wmi into bitland-mifs-wmi

[Mingyou Chen <qby140326@gmail.com>]

On 11/11/26 21:35, Ilpo Järvinen wrote:
> On Wed, 11 Nov 2026, Mingyou Chen wrote:
>
>> On 11/9/26 21:09, Ilpo Järvinen wrote:
>>> On Wed, 28 Oct 2026, Mingyou Chen wrote:
>>>
>>>>> And I've got a question, from sashiko's comment in my v12
>>>>>
>>>>> https://sashiko.dev/#/patchset/20261012133648.132363-1-qby140326%40gmail.com
>>>>>
>>>>>> Will this result in the loss of KEY_PERFORMANCE events on event-only
>>>>>> legacy
>>>>>> devices?
>>>>>> Looking at bitland_mifs_wmi_probe() in
>>>>>> drivers/platform/x86/bitland-mifs-wmi.c,
>>>>>> if dev_type == BITLAND_WMI_EVENT, it returns early after registering
>>>>>> the input
>>>>>> device:
>>>>>> 	if (dev_type == BITLAND_WMI_EVENT) {
>>>>>> 		/* Register input device for hotkeys */
>>>>>> 		drv_data->input_dev =
>>>>>> devm_input_allocate_device(&wdev->dev);
>>>>>> 		...
>>>>>> 		return input_register_device(drv_data->input_dev);
>>>>>> 	}
>>>>>> When the performance key is pressed, bitland_mifs_wmi_notify()
>>>>>> intercepts it,
>>>>>> invokes the bitland_notifier_list, and returns without reporting an
>>>>>> input event:
>>>>>> 	case WMI_EVENT_PERFORMANCE_PLAN:
>>>>>> 	case WMI_EVENT_PERFORMANCE_PLAN_ALT:
>>>>>> 		blocking_notifier_call_chain(&bitland_notifier_list,
>>>>>> 					     BITLAND_NOTIFY_PLATFORM_PROFILE,
>>>>>> 					     NULL);
>>>>>> 		return;
>>>>>> Because the platform profile and notifier were not registered during
>>>>>> probe
>>>>>> for event-only devices, does this mean the events are silently dropped
>>>>>> instead
>>>>>> of emitting KEY_PERFORMANCE like the old driver did?
>>>>> I ackknowledged that KEY_PERFORMANCE (WMI_EVENT_PERFORMANCE_PLAN_ALT in
>>>>> bitland-mifs-wmi) looks like an event in this LKML thread
>>>>>
>>>>>> Since the event reports a change that has already happened,
>>>>>> platform_profile_notify() is the natural way to pass it on, as
>>>>>> suggested in my v6 reply: power-profiles-daemon then re-reads the
>>>>>> profile, which is harmless after the driver's own writes too. If
>>>>>> KEY_PERFORMANCE should stay for on-screen displays (Ibragim asked to
>>>>>> keep these events observable), it may be worth not reporting it for
>>>>>> the
>>>>>> event that echoes the driver's own write, or at least saying in a
>>>>>> comment that it reports a mode change that already happened. On
>>>>>> Ibragim's TM2209 the profile SET is reported not to work [3], so there
>>>>>> 0x16 can only come from Fn+K and the echo does not exist; Ibragim, can
>>>>>> you confirm?
>>>>> https://lore.kernel.org/all/20261008152647.1510152-1-uselessfire@gmail.com/
>>>>>
>>>>> So should I follow sashiko's advice?
>>> Do you find Hans' earlier feedback on this not actionable? Or did you miss
>>> it.
> You did not reply to this question (which is my only remaining concern
> with this series)!?
>
>
>>> Copying long walls of text from sashiko's findings for an older version to
>>> the cover letter might not be the best approach in general. Usually, we
>>> want to get the issues addressed before sending the next version so that
>>> discussion should have occurred in the thread for the previous version.
>>>
>>>>> And some remaining comments from sashiko's review of my v13:
>>>>>
>>>>>>> -	/* Keyboard backlight: Off / Auto / Low / High (new state in
>>>>>>> byte 2) */
>>>>>>> -	{KE_KEY, 0x00000501,	{KEY_KBDILLUMTOGGLE}},
>>>>>>> -	{KE_KEY, 0x00800501,	{KEY_KBDILLUMTOGGLE}},
>>>>>>> -	{KE_KEY, 0x00050501,	{KEY_KBDILLUMTOGGLE}},
>>>>>>> -	{KE_KEY, 0x000a0501,	{KEY_KBDILLUMTOGGLE}},
>>>>>>> -
>>>>>>> -	/* Xiaomi G Command Center */
>>>>>>> -	{KE_KEY, 0x00010a01,	{KEY_VENDOR}},
>>>>>>> -
>>>>>>> -	/* OEM preset power mode: 1=Balanced 2=Silent 3=Turbo 4=Full
>>>>>>> speed */
>>>>>>> -	{KE_KEY, 0x00011601,	{KEY_PERFORMANCE}},
>>>>>>> -	{KE_KEY, 0x00021601,	{KEY_PERFORMANCE}},
>>>>>>> -	{KE_KEY, 0x00031601,	{KEY_PERFORMANCE}},
>>>>>>> -	{KE_KEY, 0x00041601,	{KEY_PERFORMANCE}},
>>>>>> In drivers/platform/x86/redmi-wmi.c, redmi_wmi_notify() reported these
>>>>>> keyboard backlight events (event_id 0x05) as KEY_KBDILLUMTOGGLE and
>>>>>> OEM preset
>>>>>> power mode events (event_id 0x16) as KEY_PERFORMANCE through
>>>>>> sparse_keymap_report_entry().
>>>>>> In drivers/platform/x86/bitland-mifs-wmi.c, bitland_mifs_wmi_keymap
>>>>>> defines
>>>>>> the four WMI_EVENT_KBD_BRIGHTNESS entries for KEY_KBDILLUMTOGGLE:
>>>>>> 	{ KE_KEY, BI_HOTKEY_CODE(WMI_EVENT_KBD_BRIGHTNESS, 0, 0),
>>>>>> {KEY_KBDILLUMTOGGLE} },
>>>>>> 	{ KE_KEY, BI_HOTKEY_CODE(WMI_EVENT_KBD_BRIGHTNESS, 0x80, 0),
>>>>>> {KEY_KBDILLUMTOGGLE} },
>>>>>> 	{ KE_KEY, BI_HOTKEY_CODE(WMI_EVENT_KBD_BRIGHTNESS, 5, 0),
>>>>>> {KEY_KBDILLUMTOGGLE} },
>>>>>> 	{ KE_KEY, BI_HOTKEY_CODE(WMI_EVENT_KBD_BRIGHTNESS, 0x0a, 0),
>>>>>> {KEY_KBDILLUMTOGGLE} },
>>>>>> However, bitland_mifs_wmi_notify() returns early for both
>>>>>> WMI_EVENT_KBD_BRIGHTNESS and WMI_EVENT_PERFORMANCE_PLAN_ALT before
>>>>>> reaching
>>>>>> bitland_mifs_wmi_report_key():
>>>>>> 	switch (event->event_id) {
>>>>>> 	case WMI_EVENT_KBD_BRIGHTNESS:
>>>>>> 		brightness = event->value_low;
>>>>>> 		blocking_notifier_call_chain(&bitland_notifier_list,
>>>>>> 					     BITLAND_NOTIFY_KBD_BRIGHTNESS,
>>>>>> 					     &brightness);
>>>>>> 		return;
>>>>>> 	case WMI_EVENT_PERFORMANCE_PLAN:
>>>>>> 	case WMI_EVENT_PERFORMANCE_PLAN_ALT:
>>>>>> 		blocking_notifier_call_chain(&bitland_notifier_list,
>>>>>> 					     BITLAND_NOTIFY_PLATFORM_PROFILE,
>>>>>> 					     NULL);
>>>>>> 		return;
>>>>>> 	...
>>>>>> 	}
>>>>>> 	payload = get_unaligned_le32(buffer->data);
>>>>>> 	bitland_mifs_wmi_report_key(wdev, payload);
>>>>>> Does this early return leave the WMI_EVENT_KBD_BRIGHTNESS entries in
>>>>>> bitland_mifs_wmi_keymap unreachable and cause a regression on
>>>>>> Redmibook
>>>>>> laptops (especially models that only expose BITLAND_EVENT_GUID without
>>>>>> the
>>>>>> BITLAND_MIFS_GUID control device) when pressing the keyboard backlight
>>>>>> or
>>>>>> power mode hotkeys?
>>>>>> [ ... ]
>>>>> Don't actually agree with sashiko, if we report the key to the
>>>>> userspace, when WMs handle the KEY_KBDILLUMTOGGLE key, the keyboard
>>>>> light will be triggered twice if the firmware already respond to the
>>>>> event,
>>>>> which is not expected.
>>>>>
>>>>> So the question is, does the firmware respond the KBD_BRIGHTNESS event?
>>>>> And should we report this key to userspace?
>>>> >From Chris' comment:
>>>>
>>>>> - Fn+Space (keyboard backlight) sends no WMI event at all, neither 0x05
>>>>>     nor anything else, and does not change the backlight under Linux. So
>>>>>     this model cannot tell whether the firmware handles KBD_BRIGHTNESS.
>>>>>
>>>>> - Fn+K sends event 0x28 with value_low 0x01 on every press (01 28 01
>>>>> 00),
>>>>>     not 0x16. The firmware does not change the mode by itself: the
>>>>>     SystemPerMode value (EC 0x68) stays the same across the presses. If
>>>>>     0x28 is the performance mode key on this model, it is a request
>>>>> rather
>>>>>     than a notification, so platform_profile_cycle() would fit. 0x28 is
>>>>>     currently logged as "Unknown WMI event".
>>>>>
>>>>> - On Windows on the same machine, with Xiaomi PC Manager not running
>>>>>     (it fails to start on this install), neither Fn+K nor Fn+Space does
>>>>>     anything either. So Fn+K looks like a request that Xiaomi's software
>>>>>     normally acts on, not a mode change made by the firmware.
>>>> I think different models have different behaviors on this.
>>>> Should we add a quirk item?
>>> For different behaviors, it may well be possible we need a quirk.
>> This is included in Chris' patch series.
>>
>> https://lore.kernel.org/r/20261106114300.936782-5-chris@miget.com
> I know, so it shouldn't be a concern in the context of _this series_.
>
> We'll be applying Chris' series after this one (and relatively soon after
> it too, I think).
>
>>> But it would be nice to get this refactoring out of the way relatively
>>> soon as it has been blocking lots of other bitland patches and I'm a bit
>>> worried the scope of this work is expanding beyond what an initial
>>> refactor change would need.
>>>
>>> If Chris' device lacks support without his series, this refactoring
>>> shouldn't have to care about that yet as long as it doesn't intentionally
>>> make decisions we need to change right away. New quirks on top of this
>>> refactoring to support a previously unsupported model is not a problem but
>>> too limiting thermal profile mapping would have been problematic as we'd
>>> have immediately had to rework the mapper.
>> I think it's enough for the known cases except this [1].
>>
>> [1]
>> https://lore.kernel.org/all/CAFbNUuLmaoq22pQPzrUidJ+RC2xf2u5Ovxzy5d0LQZ7owH52OA@mail.gmail.com/
>>
>> I'm confused with this bug report, everything looks correctly.
> My point is, that we should focus on getting this refactor series applied
> first.
>
> This again, while nice you're trying to help the report, looks future work
> from the perspective of this refactor series as you again had to add
> another profile table. That is, it is not supported until more work is
> done, which will only happen after this series in terms of applied
> commits.
>
> Also, we don't need to make the rework series perfect, it will take
> forever that way. We might still need to refactor something but I think
> the major problem with the profiles is already dealt with and we're good
> to go in that front.
>
> So the only remaining concern I have is the one I reflagged above. Please
> share your thoughts on that.

Hi,

Sorry for missing your question (I only looked at the bottom, sorry 
about that), here's my opinion:

There are two behaviors on Fn+K as far as I know:

- Hardware report 0x28 and leave the profile unchanged (this is covered 
in Chris' series)

- Hardware change the profile and report 0x16 [1], the driver should 
refresh the platform profile state, and don't report key scancode to 
userspace, I've already done this in my series (in the 
WMI_EVENT_PERFORMANCE_PLAN_ALT case).


[1] 
https://lore.kernel.org/all/20261008152647.1510152-1-uselessfire@gmail.com/