Re: [PATCH 08/10] platform/x86: alienware-wmi-wmax: Add support for manual fan control

[Armin Wolf <W_Armin@xxxxxx>]

Am 08.02.25 um 06:16 schrieb Kurt Borja:

> All models with the "AWCC" WMAX device support a way of manually
> controlling fans.
>
> The PWM duty cycle of a fan can't be controlled directly. Instead the
> AWCC interface let's us tune a PWM `boost` value, which has the
> following empirically discovered behavior over the PWM value:
>
> 	pwm = pwm_base + (pwm_boost / 255) * (pwm_max - pwm_base)
>
> Where the pwm_base is the locked PWM value controlled by the EC and
> pwm_boost is a value between 0 and 255.
>
> This pwm_boost knob is exposed as a standard `pwm` attribute.

I am not sure if exposing this over the standard "pwm" attribute is correct here,
since userspace applications expect to have full access to the fan when using the
"pwm" attribute.

Maybe using a custom attribute like "fanX_boost" would make sense here? Either way
documenting this special behavior would be nice for future users, maybe you can write
this down under Documentation/admin-guide/laptops?

Thanks,
Armin Wolf

> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Signed-off-by: Kurt Borja <kuurtb@xxxxxxxxx>
> ---
>   .../platform/x86/dell/alienware-wmi-wmax.c    | 55 +++++++++++++++++--
>   1 file changed, 49 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/platform/x86/dell/alienware-wmi-wmax.c b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> index 5f02da7ff25f..06d6f88ea54b 100644
> --- a/drivers/platform/x86/dell/alienware-wmi-wmax.c
> +++ b/drivers/platform/x86/dell/alienware-wmi-wmax.c
> @@ -13,6 +13,7 @@
>   #include <linux/dmi.h>
>   #include <linux/hwmon.h>
>   #include <linux/jiffies.h>
> +#include <linux/minmax.h>
>   #include <linux/moduleparam.h>
>   #include <linux/mutex.h>
>   #include <linux/overflow.h>
> @@ -176,10 +177,12 @@ enum AWCC_THERMAL_INFORMATION_OPERATIONS {
>   	AWCC_OP_GET_MIN_RPM			= 0x08,
>   	AWCC_OP_GET_MAX_RPM			= 0x09,
>   	AWCC_OP_GET_CURRENT_PROFILE		= 0x0B,
> +	AWCC_OP_GET_FAN_BOOST			= 0x0C,
>   };
>
>   enum AWCC_THERMAL_CONTROL_OPERATIONS {
>   	AWCC_OP_ACTIVATE_PROFILE		= 0x01,
> +	AWCC_OP_SET_FAN_BOOST			= 0x02,
>   };
>
>   enum AWCC_GAME_SHIFT_STATUS_OPERATIONS {
> @@ -563,12 +566,13 @@ static inline int awcc_thermal_information(struct wmi_device *wdev, u8 operation
>   	return __awcc_wmi_command(wdev, AWCC_METHOD_THERMAL_INFORMATION, &args, out);
>   }
>
> -static inline int awcc_thermal_control(struct wmi_device *wdev, u8 profile)
> +static inline int awcc_thermal_control(struct wmi_device *wdev, u8 operation,
> +				       u8 arg1, u8 arg2)
>   {
>   	struct wmax_u32_args args = {
> -		.operation = AWCC_OP_ACTIVATE_PROFILE,
> -		.arg1 = profile,
> -		.arg2 = 0,
> +		.operation = operation,
> +		.arg1 = arg1,
> +		.arg2 = arg2,
>   		.arg3 = 0,
>   	};
>   	u32 out;
> @@ -684,6 +688,11 @@ static umode_t awcc_hwmon_is_visible(const void *drvdata, enum hwmon_sensor_type
>   		if (channel < priv->fan_count)
>   			return 0444;
>
> +		break;
> +	case hwmon_pwm:
> +		if (channel < priv->fan_count)
> +			return 0644;
> +
>   		break;
>   	default:
>   		break;
> @@ -698,6 +707,7 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>   	struct awcc_priv *priv = dev_get_drvdata(dev);
>   	struct awcc_temp_channel_data *temp;
>   	struct awcc_fan_channel_data *fan;
> +	u32 fan_boost;
>   	int ret;
>
>   	switch (type) {
> @@ -742,6 +752,16 @@ static int awcc_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>   			return -EOPNOTSUPP;
>   		}
>
> +		break;
> +	case hwmon_pwm:
> +		fan = &priv->fan_data[channel];
> +
> +		ret = awcc_thermal_information(priv->wdev, AWCC_OP_GET_FAN_BOOST,
> +					       fan->id, &fan_boost);
> +		if (ret)
> +			return ret;
> +
> +		*val = fan_boost;
>   		break;
>   	default:
>   		return -EOPNOTSUPP;
> @@ -796,10 +816,27 @@ static int awcc_hwmon_read_string(struct device *dev, enum hwmon_sensor_types ty
>   	return 0;
>   }
>
> +
> +static int awcc_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
> +			    u32 attr, int channel, long val)
> +{
> +	struct awcc_priv *priv = dev_get_drvdata(dev);
> +	u8 fan_id = priv->fan_data[channel].id;
> +
> +	switch (type) {
> +	case hwmon_pwm:
> +		return awcc_thermal_control(priv->wdev, AWCC_OP_SET_FAN_BOOST,
> +					    fan_id, (u8)clamp_val(val, 0, 255));
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
>   static const struct hwmon_ops awcc_hwmon_ops = {
>   	.is_visible = awcc_hwmon_is_visible,
>   	.read = awcc_hwmon_read,
>   	.read_string = awcc_hwmon_read_string,
> +	.write = awcc_hwmon_write,
>   };
>
>   static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
> @@ -814,6 +851,12 @@ static const struct hwmon_channel_info * const awcc_hwmon_info[] = {
>   			   HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX,
>   			   HWMON_F_LABEL | HWMON_F_INPUT | HWMON_F_MIN | HWMON_F_MAX
>   			   ),
> +	HWMON_CHANNEL_INFO(pwm,
> +			   HWMON_PWM_INPUT,
> +			   HWMON_PWM_INPUT,
> +			   HWMON_PWM_INPUT,
> +			   HWMON_PWM_INPUT
> +			   ),
>   	NULL
>   };
>
> @@ -954,8 +997,8 @@ static int awcc_platform_profile_set(struct device *dev,
>   		}
>   	}
>
> -	return awcc_thermal_control(priv->wdev,
> -				    priv->supported_profiles[profile]);
> +	return awcc_thermal_control(priv->wdev, AWCC_OP_ACTIVATE_PROFILE,
> +				    priv->supported_profiles[profile], 0);
>   }
>
>   static int awcc_platform_profile_probe(void *drvdata, unsigned long *choices)