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

["Kurt Borja" <kuurtb@gmail.com>]

On Sun Feb 16, 2025 at 1:12 AM -05, Armin Wolf wrote:
> 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

Actually, in the first draft of this patch I actually did this. I
exposed it as pwmX_boost because it behaved like a "percentage" kind of
thing. I'm ok with fanX_boost tho, it's more explicit in it's intention.

> documenting this special behavior would be nice for future users, maybe you can write
> this down under Documentation/admin-guide/laptops?

Sure! I will. I also want to document the legacy driver features, just
like an historical archive, although some people still use these
features.

-- 
 ~ Kurt

>
> Thanks,
> Armin Wolf
>
>> Cc: Guenter Roeck <linux@roeck-us.net>
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>> ---
>>   .../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)