[RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature

Werner Sembach posted 3 patches 1 month, 3 weeks ago
drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
1 file changed, 182 insertions(+), 13 deletions(-)
[RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature
Posted by Werner Sembach 1 month, 3 weeks ago
Hi,

This series is based on another not yet accepted series
https://lore.kernel.org/all/20260417050912.5582-1-W_Armin@gmx.de/

RFC because of that, because the third patch is not yet tested, and because
I have a question regarding the 3rd patch:

Should I abstract the call to wmi_evaluate_method away in a wrapper
function in uniwill-wmi somehow, or is it ok to have it straight in the
uniwill-laptop code like that? It is quite self contained.

Best regards,

Werner

Werner Sembach (3):
  platform/x86: uniwill-laptop: Make super key init lineup with other
    inits
  platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)
  platform/x86: uniwill-laptop: Offer support to activate local dimming

 drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
 1 file changed, 182 insertions(+), 13 deletions(-)

-- 
2.43.0
Re: [RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature
Posted by Armin Wolf 1 month, 3 weeks ago
Am 21.04.26 um 22:01 schrieb Werner Sembach:
> Hi,
> 
> This series is based on another not yet accepted series
> https://lore.kernel.org/all/20260417050912.5582-1-W_Armin@gmx.de/
> 
> RFC because of that, because the third patch is not yet tested, and because
> I have a question regarding the 3rd patch:
> 
> Should I abstract the call to wmi_evaluate_method away in a wrapper
> function in uniwill-wmi somehow, or is it ok to have it straight in the
> uniwill-laptop code like that? It is quite self contained.
> 
> Best regards,
> 
> Werner

Hi,

i prefer having the code for this WMI interface inside a separate file. 
I think we have to first rename uniwill-wmi* to uniwill-wmi-event*, then 
the new code can live in uniwill-wmi.c

However we also need some synchronization mechanism between uniwill-acpi 
and the new uniwill-wmi because some code path need to check EC 
registers before enabling support for local dimming. I suggest that you 
use the component framework for that. Basically the new WMI driver 
registers a component during probing, while the EC acts as a component 
master _if_ local dimming support is present. When the component and the 
component master match a sysfs attribute is registered to allow 
userspace application to control this feature.

Thanks,
Armin Wolf

> 
> Werner Sembach (3):
>    platform/x86: uniwill-laptop: Make super key init lineup with other
>      inits
>    platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)
>    platform/x86: uniwill-laptop: Offer support to activate local dimming
> 
>   drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
>   1 file changed, 182 insertions(+), 13 deletions(-)
> 
Re: [RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature
Posted by Werner Sembach 1 month, 3 weeks ago
Am 22.04.26 um 13:28 schrieb Armin Wolf:
> Am 21.04.26 um 22:01 schrieb Werner Sembach:
>> Hi,
>>
>> This series is based on another not yet accepted series
>> https://lore.kernel.org/all/20260417050912.5582-1-W_Armin@gmx.de/
>>
>> RFC because of that, because the third patch is not yet tested, and because
>> I have a question regarding the 3rd patch:
>>
>> Should I abstract the call to wmi_evaluate_method away in a wrapper
>> function in uniwill-wmi somehow, or is it ok to have it straight in the
>> uniwill-laptop code like that? It is quite self contained.
>>
>> Best regards,
>>
>> Werner
>
> Hi,
>
> i prefer having the code for this WMI interface inside a separate file. I 
> think we have to first rename uniwill-wmi* to uniwill-wmi-event*, then the new 
> code can live in uniwill-wmi.c
>
> However we also need some synchronization mechanism between uniwill-acpi and 
> the new uniwill-wmi because some code path need to check EC registers before 
> enabling support for local dimming. I suggest that you use the component 
> framework for that. Basically the new WMI driver registers a component during 
> probing, while the EC acts as a component master _if_ local dimming support is 
> present. When the component and the component master match a sysfs attribute 
> is registered to allow userspace application to control this feature.

Calling the old uniwill-wmi uniwill-wmi-event and the new driver uniwill-wmi 
from here on:

I haven't yet worked with the component framework, but i wonder: since the same 
manual register call structure that we already have for uniwill-wmi-event will 
be also required for uniwill-wmi, can't this call not also be used to exchange 
callbacks like for uniwill-wmi-event and the notifier block? just the other way 
around with uniwill-acpi receiving a callback instead of providing one.

ofc these callbacks must be guarded in some way to not accidentally call 
uninitialized code, if uniwill-wmi fails to init or probe

>
> Thanks,
> Armin Wolf
>
>>
>> Werner Sembach (3):
>>    platform/x86: uniwill-laptop: Make super key init lineup with other
>>      inits
>>    platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)
>>    platform/x86: uniwill-laptop: Offer support to activate local dimming
>>
>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
>>   1 file changed, 182 insertions(+), 13 deletions(-)
>>
>
Re: [RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature
Posted by Armin Wolf 1 month, 3 weeks ago
Am 22.04.26 um 17:30 schrieb Werner Sembach:
> 
> Am 22.04.26 um 13:28 schrieb Armin Wolf:
>> Am 21.04.26 um 22:01 schrieb Werner Sembach:
>>> Hi,
>>>
>>> This series is based on another not yet accepted series
>>> https://lore.kernel.org/all/20260417050912.5582-1-W_Armin@gmx.de/
>>>
>>> RFC because of that, because the third patch is not yet tested, and 
>>> because
>>> I have a question regarding the 3rd patch:
>>>
>>> Should I abstract the call to wmi_evaluate_method away in a wrapper
>>> function in uniwill-wmi somehow, or is it ok to have it straight in the
>>> uniwill-laptop code like that? It is quite self contained.
>>>
>>> Best regards,
>>>
>>> Werner
>>
>> Hi,
>>
>> i prefer having the code for this WMI interface inside a separate 
>> file. I think we have to first rename uniwill-wmi* to uniwill-wmi- 
>> event*, then the new code can live in uniwill-wmi.c
>>
>> However we also need some synchronization mechanism between uniwill- 
>> acpi and the new uniwill-wmi because some code path need to check EC 
>> registers before enabling support for local dimming. I suggest that 
>> you use the component framework for that. Basically the new WMI driver 
>> registers a component during probing, while the EC acts as a component 
>> master _if_ local dimming support is present. When the component and 
>> the component master match a sysfs attribute is registered to allow 
>> userspace application to control this feature.
> 
> Calling the old uniwill-wmi uniwill-wmi-event and the new driver 
> uniwill-wmi from here on:
> 
> I haven't yet worked with the component framework, but i wonder: since 
> the same manual register call structure that we already have for 
> uniwill-wmi-event will be also required for uniwill-wmi, can't this call 
> not also be used to exchange callbacks like for uniwill-wmi-event and 
> the notifier block? just the other way around with uniwill-acpi 
> receiving a callback instead of providing one.
> 
> ofc these callbacks must be guarded in some way to not accidentally call 
> uninitialized code, if uniwill-wmi fails to init or probe

This would indeed be possible, but where would be no mechanism for 
detecting when the WMI driver has finished probing.

I was thinking that the WMI driver uses the component callbacks to pass 
a pointer to its data struct to the component master. Said data struct 
can then by used to change the local dimming status.

I am still not sure when the associated sysfs files should be created. 
We can create them during probing of the EC, but then there will be 
problems should the WMI device take too long/fail to probe. 
Alternatively we can create the sysfs file when the component master 
binds the WMI component, so that the sysfs file will only become visible
once it is operational. We can then issue a uevent towards userspace to 
signal userspace applications that the sysfs attributes of our device 
have changed.

Thanks,
Armin Wolf

> 
>>
>> Thanks,
>> Armin Wolf
>>
>>>
>>> Werner Sembach (3):
>>>    platform/x86: uniwill-laptop: Make super key init lineup with other
>>>      inits
>>>    platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)
>>>    platform/x86: uniwill-laptop: Offer support to activate local dimming
>>>
>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
>>>   1 file changed, 182 insertions(+), 13 deletions(-)
>>>
>>
> 
Re: [RFC PATCH 0/3] platform/x86: uniwill-laptop: Another improvement and another feature
Posted by Werner Sembach 1 month, 3 weeks ago
Am 22.04.26 um 18:12 schrieb Armin Wolf:
> Am 22.04.26 um 17:30 schrieb Werner Sembach:
>>
>> Am 22.04.26 um 13:28 schrieb Armin Wolf:
>>> Am 21.04.26 um 22:01 schrieb Werner Sembach:
>>>> Hi,
>>>>
>>>> This series is based on another not yet accepted series
>>>> https://lore.kernel.org/all/20260417050912.5582-1-W_Armin@gmx.de/
>>>>
>>>> RFC because of that, because the third patch is not yet tested, and because
>>>> I have a question regarding the 3rd patch:
>>>>
>>>> Should I abstract the call to wmi_evaluate_method away in a wrapper
>>>> function in uniwill-wmi somehow, or is it ok to have it straight in the
>>>> uniwill-laptop code like that? It is quite self contained.
>>>>
>>>> Best regards,
>>>>
>>>> Werner
>>>
>>> Hi,
>>>
>>> i prefer having the code for this WMI interface inside a separate file. I 
>>> think we have to first rename uniwill-wmi* to uniwill-wmi- event*, then the 
>>> new code can live in uniwill-wmi.c
>>>
>>> However we also need some synchronization mechanism between uniwill- acpi 
>>> and the new uniwill-wmi because some code path need to check EC registers 
>>> before enabling support for local dimming. I suggest that you use the 
>>> component framework for that. Basically the new WMI driver registers a 
>>> component during probing, while the EC acts as a component master _if_ local 
>>> dimming support is present. When the component and the component master 
>>> match a sysfs attribute is registered to allow userspace application to 
>>> control this feature.
>>
>> Calling the old uniwill-wmi uniwill-wmi-event and the new driver uniwill-wmi 
>> from here on:
>>
>> I haven't yet worked with the component framework, but i wonder: since the 
>> same manual register call structure that we already have for 
>> uniwill-wmi-event will be also required for uniwill-wmi, can't this call not 
>> also be used to exchange callbacks like for uniwill-wmi-event and the 
>> notifier block? just the other way around with uniwill-acpi receiving a 
>> callback instead of providing one.
>>
>> ofc these callbacks must be guarded in some way to not accidentally call 
>> uninitialized code, if uniwill-wmi fails to init or probe
>
> This would indeed be possible, but where would be no mechanism for detecting 
> when the WMI driver has finished probing.
>
> I was thinking that the WMI driver uses the component callbacks to pass a 
> pointer to its data struct to the component master. Said data struct can then 
> by used to change the local dimming status.

ok i will look into this, but first a week of hollyday ^^

best regards,

Werner

>
> I am still not sure when the associated sysfs files should be created. We can 
> create them during probing of the EC, but then there will be problems should 
> the WMI device take too long/fail to probe. Alternatively we can create the 
> sysfs file when the component master binds the WMI component, so that the 
> sysfs file will only become visible
> once it is operational. We can then issue a uevent towards userspace to signal 
> userspace applications that the sysfs attributes of our device have changed.
>
> Thanks,
> Armin Wolf
>
>>
>>>
>>> Thanks,
>>> Armin Wolf
>>>
>>>>
>>>> Werner Sembach (3):
>>>>    platform/x86: uniwill-laptop: Make super key init lineup with other
>>>>      inits
>>>>    platform/x86: uniwill-laptop: Implement lightbar for XMG Fusion (L19)
>>>>    platform/x86: uniwill-laptop: Offer support to activate local dimming
>>>>
>>>>   drivers/platform/x86/uniwill/uniwill-acpi.c | 195 ++++++++++++++++++--
>>>>   1 file changed, 182 insertions(+), 13 deletions(-)
>>>>
>>>
>>
>