On Mon May 12, 2025 at 7:16 AM -03, Antheas Kapenekakis wrote: > On Mon, 12 May 2025 at 01:30, Kurt Borja <kuurtb@xxxxxxxxx> wrote: >> >> Hi Antheas, >> >> On Sun May 11, 2025 at 5:44 PM -03, Antheas Kapenekakis wrote: >> > This draft patch series brings into parity the msi-wmi-platform driver with >> > the MSI Center M Windows application for the MSI Claw (all models). >> > Unfortunately, MSI Center M and this interface do not have a discovery API, >> > necessitating the introduction of a quirk system. >> > >> > While this patch series is fully functional and tested, there are still >> > some issues that need to be addressed: >> > - Armin notes we need to disable fan curve support by default and quirk >> > it as well, as it is not supported on all models. However, the way >> > PWM enable ops work, this makes it a bit difficult, so I would like >> > some suggestions on how to rework this. >> >> If I understood the question correctly, then you should control the >> visibility of all "curve" related attributes with the quirk. > > Yep, this is what I was wondering. I will investigate this. It would > be good to get some comments on the quirk naming as well. You can check [1] for an example of the hwmon visibility. It's a similar problem because some models have 2 fans and others have 4. In the alienware-wmi driver we also have custom hwmon attributes, see [2] for how to handle visibility with those. I would personally just name it fan_curve or has_fan_curve. > >> The custom hwmon attribute_group has an is_visible callback, and so do >> the hwmon_ops. >> >> > - It turns out that to fully disable the fan curve, we have to restore >> > the default fan values. This is also what is done on the OEM software. >> > For this, the last patch in the series is used, which is a bit dirty. >> >> I have a couple questions about this. >> >> * What are the default fan curves? Can these be statically defined? >> * Are user-defined fan curves persistent between reboots? >> >> I have some doubts about the approach you took on the last patch, but I >> want to understand how the platform works first. > > So do I. Essentially here is how the Windows software works: when it > first opens, it saves the current curve in Windows registry. Then, > when the user sets a fan curve, it applies it in the same way we do > here and sets a bit in AP. When the custom curve is removed, it unsets > that bit and restores the original curve in WMI. > > The logical reasoning would be that that bit controls the fan curve. > This is how it is named in the software. However, when setting that > bit on its own, it seems to only partially affect the fan curve. E.g., > when the fan curve is 100% in all points, unsetting that bit makes it > go down to 50% when no load occurs. When using the default fan curve, > it goes to 0%. Therefore, it seems like that bit makes the fan curve > semi-autonomous? > > The fan curve seems to be hardware specific and resets after reboots. > So a straightforward way to get it is to grab it on a fresh boot. Oh - this is interesting. Then I think it is the right approach. I'll add a couple more comments. > > Antheas > >> > >> > Sleep was tested with all values being preserved during S0iX (platform >> > profile, fan curve, PL1/PL2), so we do not need suspend/resume hooks, at >> > least for the Claw devices. >> > >> > For PL1/PL2, we use firmware-attributes. So for that I +cc Kurt since if >> > his new high level interface is merged beforehand, we can use that instead. >> >> Hopefully! >> >> -- >> ~ Kurt >> >> > >> > Antheas Kapenekakis (8): >> > platform/x86: msi-wmi-platform: Add unlocked msi_wmi_platform_query >> > platform/x86: msi-wmi-platform: Add quirk system >> > platform/x86: msi-wmi-platform: Add platform profile through shift >> > mode >> > platform/x86: msi-wmi-platform: Add PL1/PL2 support via firmware >> > attributes >> > platform/x86: msi-wmi-platform: Add charge_threshold support >> > platform/x86: msi-wmi-platform: Drop excess fans in dual fan devices >> > platform/x86: msi-wmi-platform: Update header text >> > platform/x86: msi-wmi-platform: Restore fan curves on PWM disable and >> > unload >> > >> > Armin Wolf (2): >> > platform/x86: msi-wmi-platform: Use input buffer for returning result >> > platform/x86: msi-wmi-platform: Add support for fan control >> > >> > .../wmi/devices/msi-wmi-platform.rst | 26 + >> > drivers/platform/x86/Kconfig | 3 + >> > drivers/platform/x86/msi-wmi-platform.c | 1181 ++++++++++++++++- >> > 3 files changed, 1156 insertions(+), 54 deletions(-) >> > >> > >> > base-commit: 62b1dcf2e7af3dc2879d1a39bf6823c99486a8c2 >> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/tree/drivers/platform/x86/dell/alienware-wmi-wmax.c?h=for-next#n742 [2] https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/tree/drivers/platform/x86/dell/alienware-wmi-wmax.c?h=for-next#n942
Attachment:
signature.asc
Description: PGP signature
