[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D7U6RB4K0P69.241M4E5RQZCTL@gmail.com>
Date: Sun, 16 Feb 2025 16:25:24 -0500
From: "Kurt Borja" <kuurtb@...il.com>
To: "Armin Wolf" <W_Armin@....de>, Ilpo Järvinen
<ilpo.jarvinen@...ux.intel.com>
Cc: <platform-driver-x86@...r.kernel.org>, "Hans de Goede"
<hdegoede@...hat.com>, <Dell.Client.Kernel@...l.com>,
<linux-kernel@...r.kernel.org>, "Guenter Roeck" <linux@...ck-us.net>
Subject: Re: [PATCH 08/10] platform/x86: alienware-wmi-wmax: Add support for
manual fan control
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@...ck-us.net>
>> Signed-off-by: Kurt Borja <kuurtb@...il.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)
Powered by blists - more mailing lists