MSI software is a bit weird in that even when the manual fan curve is
disabled, the fan speed is still somewhat affected by the curve. So
we have to restore the fan curves on unload and PWM disable, as it
is done in Windows.
Suggested-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
---
drivers/platform/x86/msi-wmi-platform.c | 123 +++++++++++++++++++++++-
1 file changed, 122 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
index 7dafe17d4d6be..a917db0300c06 100644
--- a/drivers/platform/x86/msi-wmi-platform.c
+++ b/drivers/platform/x86/msi-wmi-platform.c
@@ -123,16 +123,25 @@ struct msi_wmi_platform_quirk {
bool shift_mode; /* Shift mode is supported */
bool charge_threshold; /* Charge threshold is supported */
bool dual_fans; /* For devices with two hwmon fans */
+ bool restore_curves; /* Restore factory curves on unload */
int pl_min; /* Minimum PLx value */
int pl1_max; /* Maximum PL1 value */
int pl2_max; /* Maximum PL2 value */
};
+struct msi_wmi_platform_factory_curves {
+ u8 cpu_fan_table[32];
+ u8 gpu_fan_table[32];
+ u8 cpu_temp_table[32];
+ u8 gpu_temp_table[32];
+};
+
struct msi_wmi_platform_data {
struct wmi_device *wdev;
struct msi_wmi_platform_quirk *quirks;
struct mutex wmi_lock; /* Necessary when calling WMI methods */
struct device *ppdev;
+ struct msi_wmi_platform_factory_curves factory_curves;
struct acpi_battery_hook battery_hook;
struct device *fw_attrs_dev;
struct kset *fw_attrs_kset;
@@ -219,6 +228,7 @@ static struct msi_wmi_platform_quirk quirk_gen1 = {
.shift_mode = true,
.charge_threshold = true,
.dual_fans = true,
+ .restore_curves = true,
.pl_min = 8,
.pl1_max = 43,
.pl2_max = 45
@@ -227,6 +237,7 @@ static struct msi_wmi_platform_quirk quirk_gen2 = {
.shift_mode = true,
.charge_threshold = true,
.dual_fans = true,
+ .restore_curves = true,
.pl_min = 8,
.pl1_max = 30,
.pl2_max = 37
@@ -507,6 +518,94 @@ static struct attribute *msi_wmi_platform_hwmon_attrs[] = {
};
ATTRIBUTE_GROUPS(msi_wmi_platform_hwmon);
+static int msi_wmi_platform_curves_save(struct msi_wmi_platform_data *data)
+{
+ int ret;
+
+ data->factory_curves.cpu_fan_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_CPU_FAN_TABLE;
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_GET_FAN,
+ data->factory_curves.cpu_fan_table,
+ sizeof(data->factory_curves.cpu_fan_table));
+ if (ret < 0)
+ return ret;
+ data->factory_curves.cpu_fan_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_CPU_FAN_TABLE;
+
+ data->factory_curves.gpu_fan_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_GPU_FAN_TABLE;
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_GET_FAN,
+ data->factory_curves.gpu_fan_table,
+ sizeof(data->factory_curves.gpu_fan_table));
+ if (ret < 0)
+ return ret;
+ data->factory_curves.gpu_fan_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_GPU_FAN_TABLE;
+
+ data->factory_curves.cpu_temp_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_CPU_TEMP_TABLE;
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_GET_TEMPERATURE,
+ data->factory_curves.cpu_temp_table,
+ sizeof(data->factory_curves.cpu_temp_table));
+ if (ret < 0)
+ return ret;
+ data->factory_curves.cpu_temp_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_CPU_TEMP_TABLE;
+
+ data->factory_curves.gpu_temp_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_GPU_TEMP_TABLE;
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_GET_TEMPERATURE,
+ data->factory_curves.gpu_temp_table,
+ sizeof(data->factory_curves.gpu_temp_table));
+ if (ret < 0)
+ return ret;
+ data->factory_curves.gpu_temp_table[0] =
+ MSI_PLATFORM_FAN_SUBFEATURE_GPU_TEMP_TABLE;
+
+ return 0;
+}
+
+static int msi_wmi_platform_curves_load(struct msi_wmi_platform_data *data)
+{
+ u8 buffer[32] = { };
+ int ret;
+
+ memcpy(buffer, data->factory_curves.cpu_fan_table,
+ sizeof(data->factory_curves.cpu_fan_table));
+ ret = msi_wmi_platform_query_unlocked(data, MSI_PLATFORM_SET_FAN,
+ buffer, sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ memcpy(buffer, data->factory_curves.gpu_fan_table,
+ sizeof(data->factory_curves.gpu_fan_table));
+ ret = msi_wmi_platform_query_unlocked(data, MSI_PLATFORM_SET_FAN,
+ buffer, sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ memcpy(buffer, data->factory_curves.cpu_temp_table,
+ sizeof(data->factory_curves.cpu_temp_table));
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_SET_TEMPERATURE, buffer, sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ memcpy(buffer, data->factory_curves.gpu_temp_table,
+ sizeof(data->factory_curves.gpu_temp_table));
+ ret = msi_wmi_platform_query_unlocked(
+ data, MSI_PLATFORM_SET_TEMPERATURE, buffer, sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+
static umode_t msi_wmi_platform_is_visible(const void *drvdata, enum hwmon_sensor_types type,
u32 attr, int channel)
{
@@ -603,9 +702,19 @@ static int msi_wmi_platform_write(struct device *dev, enum hwmon_sensor_types ty
return -EINVAL;
}
- return msi_wmi_platform_query_unlocked(
+ ret = msi_wmi_platform_query_unlocked(
data, MSI_PLATFORM_SET_AP, buffer,
sizeof(buffer));
+ if (ret < 0)
+ return ret;
+
+ if (val == 2 && data->quirks->restore_curves) {
+ ret = msi_wmi_platform_curves_load(data);
+ if (ret < 0)
+ return ret;
+ }
+
+ return 0;
default:
return -EOPNOTSUPP;
}
@@ -1373,6 +1482,13 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
msi_wmi_platform_profile_setup(data);
+ if (data->quirks->restore_curves) {
+ guard(mutex)(&data->wmi_lock);
+ ret = msi_wmi_platform_curves_save(data);
+ if (ret < 0)
+ return ret;
+ }
+
return msi_wmi_platform_hwmon_init(data);
}
@@ -1382,6 +1498,11 @@ static void msi_wmi_platform_remove(struct wmi_device *wdev)
if (data->quirks->charge_threshold)
battery_hook_unregister(&data->battery_hook);
+
+ if (data->quirks->restore_curves) {
+ guard(mutex)(&data->wmi_lock);
+ msi_wmi_platform_curves_load(data);
+ }
}
static const struct wmi_device_id msi_wmi_platform_id_table[] = {
--
2.49.0On Sun May 11, 2025 at 5:44 PM -03, Antheas Kapenekakis wrote:
> MSI software is a bit weird in that even when the manual fan curve is
> disabled, the fan speed is still somewhat affected by the curve. So
> we have to restore the fan curves on unload and PWM disable, as it
> is done in Windows.
>
> Suggested-by: Armin Wolf <W_Armin@gmx.de>
> Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
> ---
> drivers/platform/x86/msi-wmi-platform.c | 123 +++++++++++++++++++++++-
> 1 file changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/msi-wmi-platform.c b/drivers/platform/x86/msi-wmi-platform.c
> index 7dafe17d4d6be..a917db0300c06 100644
> --- a/drivers/platform/x86/msi-wmi-platform.c
> +++ b/drivers/platform/x86/msi-wmi-platform.c
> @@ -123,16 +123,25 @@ struct msi_wmi_platform_quirk {
> bool shift_mode; /* Shift mode is supported */
> bool charge_threshold; /* Charge threshold is supported */
> bool dual_fans; /* For devices with two hwmon fans */
> + bool restore_curves; /* Restore factory curves on unload */
> int pl_min; /* Minimum PLx value */
> int pl1_max; /* Maximum PL1 value */
> int pl2_max; /* Maximum PL2 value */
> };
>
> +struct msi_wmi_platform_factory_curves {
> + u8 cpu_fan_table[32];
> + u8 gpu_fan_table[32];
> + u8 cpu_temp_table[32];
> + u8 gpu_temp_table[32];
> +};
> +
> struct msi_wmi_platform_data {
> struct wmi_device *wdev;
> struct msi_wmi_platform_quirk *quirks;
> struct mutex wmi_lock; /* Necessary when calling WMI methods */
> struct device *ppdev;
> + struct msi_wmi_platform_factory_curves factory_curves;
> struct acpi_battery_hook battery_hook;
> struct device *fw_attrs_dev;
> struct kset *fw_attrs_kset;
> @@ -219,6 +228,7 @@ static struct msi_wmi_platform_quirk quirk_gen1 = {
> .shift_mode = true,
> .charge_threshold = true,
> .dual_fans = true,
> + .restore_curves = true,
> .pl_min = 8,
> .pl1_max = 43,
> .pl2_max = 45
> @@ -227,6 +237,7 @@ static struct msi_wmi_platform_quirk quirk_gen2 = {
> .shift_mode = true,
> .charge_threshold = true,
> .dual_fans = true,
> + .restore_curves = true,
> .pl_min = 8,
> .pl1_max = 30,
> .pl2_max = 37
> @@ -507,6 +518,94 @@ static struct attribute *msi_wmi_platform_hwmon_attrs[] = {
> };
> ATTRIBUTE_GROUPS(msi_wmi_platform_hwmon);
>
> +static int msi_wmi_platform_curves_save(struct msi_wmi_platform_data *data)
> +{
> + int ret;
> +
> + data->factory_curves.cpu_fan_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_CPU_FAN_TABLE;
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_GET_FAN,
> + data->factory_curves.cpu_fan_table,
> + sizeof(data->factory_curves.cpu_fan_table));
> + if (ret < 0)
> + return ret;
> + data->factory_curves.cpu_fan_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_CPU_FAN_TABLE;
Is it necessary to set the subfeature again here (and bellow)?
Also there is a lot of code repetition here, I would suggest a helper
function. It will be optimized/inlined by the compiler anyway.
> +
> + data->factory_curves.gpu_fan_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_GPU_FAN_TABLE;
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_GET_FAN,
> + data->factory_curves.gpu_fan_table,
> + sizeof(data->factory_curves.gpu_fan_table));
> + if (ret < 0)
> + return ret;
> + data->factory_curves.gpu_fan_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_GPU_FAN_TABLE;
> +
> + data->factory_curves.cpu_temp_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_CPU_TEMP_TABLE;
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_GET_TEMPERATURE,
> + data->factory_curves.cpu_temp_table,
> + sizeof(data->factory_curves.cpu_temp_table));
> + if (ret < 0)
> + return ret;
> + data->factory_curves.cpu_temp_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_CPU_TEMP_TABLE;
> +
> + data->factory_curves.gpu_temp_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_GPU_TEMP_TABLE;
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_GET_TEMPERATURE,
> + data->factory_curves.gpu_temp_table,
> + sizeof(data->factory_curves.gpu_temp_table));
> + if (ret < 0)
> + return ret;
> + data->factory_curves.gpu_temp_table[0] =
> + MSI_PLATFORM_FAN_SUBFEATURE_GPU_TEMP_TABLE;
> +
> + return 0;
> +}
> +
> +static int msi_wmi_platform_curves_load(struct msi_wmi_platform_data *data)
> +{
> + u8 buffer[32] = { };
> + int ret;
> +
> + memcpy(buffer, data->factory_curves.cpu_fan_table,
> + sizeof(data->factory_curves.cpu_fan_table));
> + ret = msi_wmi_platform_query_unlocked(data, MSI_PLATFORM_SET_FAN,
> + buffer, sizeof(buffer));
> + if (ret < 0)
> + return ret;
A helper for this operation would be nice too.
> +
> + memcpy(buffer, data->factory_curves.gpu_fan_table,
> + sizeof(data->factory_curves.gpu_fan_table));
> + ret = msi_wmi_platform_query_unlocked(data, MSI_PLATFORM_SET_FAN,
> + buffer, sizeof(buffer));
> + if (ret < 0)
> + return ret;
> +
> + memcpy(buffer, data->factory_curves.cpu_temp_table,
> + sizeof(data->factory_curves.cpu_temp_table));
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_SET_TEMPERATURE, buffer, sizeof(buffer));
> + if (ret < 0)
> + return ret;
> +
> + memcpy(buffer, data->factory_curves.gpu_temp_table,
> + sizeof(data->factory_curves.gpu_temp_table));
> + ret = msi_wmi_platform_query_unlocked(
> + data, MSI_PLATFORM_SET_TEMPERATURE, buffer, sizeof(buffer));
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +}
> +
> +
> static umode_t msi_wmi_platform_is_visible(const void *drvdata, enum hwmon_sensor_types type,
> u32 attr, int channel)
> {
> @@ -603,9 +702,19 @@ static int msi_wmi_platform_write(struct device *dev, enum hwmon_sensor_types ty
> return -EINVAL;
> }
>
> - return msi_wmi_platform_query_unlocked(
> + ret = msi_wmi_platform_query_unlocked(
> data, MSI_PLATFORM_SET_AP, buffer,
> sizeof(buffer));
> + if (ret < 0)
> + return ret;
> +
> + if (val == 2 && data->quirks->restore_curves) {
> + ret = msi_wmi_platform_curves_load(data);
> + if (ret < 0)
> + return ret;
> + }
> +
> + return 0;
> default:
> return -EOPNOTSUPP;
> }
> @@ -1373,6 +1482,13 @@ static int msi_wmi_platform_probe(struct wmi_device *wdev, const void *context)
>
> msi_wmi_platform_profile_setup(data);
>
> + if (data->quirks->restore_curves) {
> + guard(mutex)(&data->wmi_lock);
We don't need locking here. data->factory_curves is not shared until you
register the hwmon device.
> + ret = msi_wmi_platform_curves_save(data);
> + if (ret < 0)
> + return ret;
> + }
> +
> return msi_wmi_platform_hwmon_init(data);
> }
>
> @@ -1382,6 +1498,11 @@ static void msi_wmi_platform_remove(struct wmi_device *wdev)
>
> if (data->quirks->charge_threshold)
> battery_hook_unregister(&data->battery_hook);
> +
> + if (data->quirks->restore_curves) {
> + guard(mutex)(&data->wmi_lock);
We can avoid locking here by adding a devm action that restores the
curves. devm resources are unloaded in LIFO order.
Please, also check my comments on [Patch 2]. I don't think that patch is
needed.
--
~ Kurt
> + msi_wmi_platform_curves_load(data);
> + }
> }
>
> static const struct wmi_device_id msi_wmi_platform_id_table[] = {
© 2016 - 2025 Red Hat, Inc.