Am 26.12.24 um 01:18 schrieb John Martens:
I guess the most important task is to get following points right because they are hard to fix later. 1. Should there be a unifrom sysfs interface for different access methods? Depending on the model different methods must be used to control the same feature, e.g. the powermode, fan table, dust-cleaning-mode. The access methods could be a different WMI method (newer model), direct ACPI without WMI, or port mapped IO (outb/inb). I suggest that regardless of the access methods it should be produce the same sysfs entry. Example: there is a fan-fullspeed-methods/dustcleaning-mode that sets the fans to the maximal possible speed. I suggest that regardless of the used access method there should be the one file: /sys/class/firmware-attributes/*/attributes/fanfullspeed/current_value Alternatively, one could use the less elegant approach: /sys/class/firmware-attributes/*/attributes/wmi-other-fanfullspeed/current_value /sys/class/firmware-attributes/*/attributes/wmi-gamezone-fanfullspeed/current_value /sys/class/firmware-attributes/*/attributes/acpi1-fanfullspeed/current_value
The firmware-attribute class interface is only intended for attributes which are persistent and cannot be exposed over other subsystem interfaces. The "current_value" attribute can be exposed using the hwmon subsystem. Also i assume that the setting is not persistent across reboots (correct me if i am wrong). Also depending on the driver the path will be: /sys/class/firmware-attributes/<name>/attributes/<attr name>/current_value Because of this i think having a separate interface for each driver is better. We can of course harmonize the naming of the attributes where it makes sense.
2. Naming and file structure: As mentioned above, there different methods - including non-WMI methods - are used. Hence, it might not be optimal name the driver "legion-wmi". One idea would be to name the folder/driver "legion" and then seperate into multiple files by access methods (WMI by GUID, ACPI, port mapped IO). 3. Driver Structure, selection of access method and probing: The right access method (WMI, ACPI, ...) has to be chosen for each model. Some of them can be automatically probed, some of them have to be hard coded (c.f. also Window tools) by the letter-only prefix of the BIOS version. Depending on the driver structure there are multiple ideas how to manage this i nformation: a: global-access-into-driver-decide-by-enum: initially the driver can store the method of access (WMI, ACPI, ...) for e.g. modifying fanfullspeed as an enum/bitfield/... globally. The value can be decided on by probing and some hard coded rules. There is one "glovbal" c-file that acts as an entrypoint into the driver and adds all the show/store functions. When the show-function is called it is decided e.g. by a switch statement which function in one of the different files (WMI, ACPI, ...) is called. The upside of this method is that if there are not warnings in the code, then every case is totally covered. The downside is a lot of boilerplate code. b: global-access-into-driver-decide-by-function-pointer: Same as above in case a, but direclty use function pointers instead of enum/bits. There is one function pointers for each attribute in a "global" struct. When the driver is loaded initially, it sets each function pointers to modify an attribute the right function for the model. The upside is less boilerplate. The downside is that it might get a little less safe working with the function pointers. c: independent-access-in-independent-driver-parts: the driver is split into totally independent parts for each method (WMI, ACPI, ...) and GUID. Each driver part is responsible for creating the sysfs entries. To prevent conflicts each part has to use a unique name (see 1) for the attribute. Alternatively, the choice of access has to be propagated down to each part to prevent creating the same sysfs attribute multiple times. The upside is the elegance and easy extension. The downside is the weird sysfs user-interface and the weird coupling between the different driver parts. d: totally independent drivers: make a totally independent driver (module) for each access method.
I would prefer approach d. Ideally each driver would use standard subsystem interfaces (hwmon, backlight, firmware-attributes, platform-profile, etc) so that userspace software can be written in a driver-agnostic way.
Some more remarks: - I would never make one attribute depend on another attribute, e.g. when changing some power parameters of GPU/CPU it should not change the power mode (e.g. going to custom mode). Initially, I did the same but it turned out to be not a good idea. However, if one changes some power settings and is not custom-powermode some sometimes some weird things happen. Sometimes also all changes are ignored by the firmware as seen in the ACPI dissassembly. I guess it is best to just manage this in user space.
I agree.
- When trying to find out what access method to choose one cannot rely on the ACPI/WMI interface. From disassembling the ACPI, one can see that sometimes/often even if the function is not implemented it will return without error. Moreover, there are some WMI methods with name "*IsSupported" (or similar) but they often do not tell the truth.
Oh no. I hope that we just misinterpret the result of those methods. Otherwise this would indeed be very frustrating. Maybe some help from Lenovo can solve this issue. Thanks, Armin Wolf
- Using just one WMI interface is simple — my grandmother could do it. However, when juggling and organizing the various access methods, your guidance is needed to set the driver on the right path from the beginning. So I defenitely, appreciate your input on the different options.
