| Message ID | 20241025191514.15032-3-W_Armin@gmx.de (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86: asus-wmi: Fix thermal profile handling | expand |
Hi Armin, On 25-Oct-24 9:15 PM, Armin Wolf wrote: > When changing the thermal policy using the platform profile API, > a Vivobook thermal policy is stored in throttle_thermal_policy_mode. > > However everywhere else a normal thermal policy is stored inside this > variable, potentially confusing the platform profile. You say "potentially confusing the platform profile", but did you spot any actual issues when reviewing the code ? > Fix this by always storing normal thermal policy values inside > throttle_thermal_policy_mode and only do the conversion when writing > the thermal policy to hardware. > > Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") > Signed-off-by: Armin Wolf <W_Armin@gmx.de> The problem with this approach is that it changes the order in which we step through the modes in throttle_thermal_policy_switch_next() after this change we have: Normal Asus: balanced -> performance -> silent -> balanced -> etc. Vivobook: balanced -> silent -> performance -> balanced -> etc. where if we see "silent" as lower performance then the other 2, the vivobook order is a bit weird. I wonder if this is a big enough issue to really worry about it though; and I do like the cleanup / simpler code. Note that this also causes a behavior change in fan_curve_get_factory_default() as I mentioned in my cover-letter. I think that that behavior change might be a good thing to do actually, but at a minimum it needs to be documented in the commit msg. Regards, Hans > --- > drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- > 1 file changed, 21 insertions(+), 43 deletions(-) > > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c > index ab9342a01a48..ce60835d0303 100644 > --- a/drivers/platform/x86/asus-wmi.c > +++ b/drivers/platform/x86/asus-wmi.c > @@ -3696,10 +3696,28 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus) > /* Throttle thermal policy ****************************************************/ > static int throttle_thermal_policy_write(struct asus_wmi *asus) > { > - u8 value = asus->throttle_thermal_policy_mode; > u32 retval; > + u8 value; > int err; > > + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO) { > + switch (asus->throttle_thermal_policy_mode) { > + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: > + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; > + break; > + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: > + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; > + break; > + case ASUS_THROTTLE_THERMAL_POLICY_SILENT: > + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; > + break; > + default: > + return -EINVAL; > + } > + } else { > + value = asus->throttle_thermal_policy_mode; > + } > + > err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev, > value, &retval); > > @@ -3804,46 +3822,6 @@ static ssize_t throttle_thermal_policy_store(struct device *dev, > static DEVICE_ATTR_RW(throttle_thermal_policy); > > /* Platform profile ***********************************************************/ > -static int asus_wmi_platform_profile_to_vivo(struct asus_wmi *asus, int mode) > -{ > - bool vivo; > - > - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; > - > - if (vivo) { > - switch (mode) { > - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: > - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; > - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: > - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; > - case ASUS_THROTTLE_THERMAL_POLICY_SILENT: > - return ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; > - } > - } > - > - return mode; > -} > - > -static int asus_wmi_platform_profile_mode_from_vivo(struct asus_wmi *asus, int mode) > -{ > - bool vivo; > - > - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; > - > - if (vivo) { > - switch (mode) { > - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO: > - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT; > - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO: > - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST; > - case ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO: > - return ASUS_THROTTLE_THERMAL_POLICY_SILENT; > - } > - } > - > - return mode; > -} > - > static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, > enum platform_profile_option *profile) > { > @@ -3853,7 +3831,7 @@ static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, > asus = container_of(pprof, struct asus_wmi, platform_profile_handler); > tp = asus->throttle_thermal_policy_mode; > > - switch (asus_wmi_platform_profile_mode_from_vivo(asus, tp)) { > + switch (tp) { > case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: > *profile = PLATFORM_PROFILE_BALANCED; > break; > @@ -3892,7 +3870,7 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof, > return -EOPNOTSUPP; > } > > - asus->throttle_thermal_policy_mode = asus_wmi_platform_profile_to_vivo(asus, tp); > + asus->throttle_thermal_policy_mode = tp; > return throttle_thermal_policy_write(asus); > } > > -- > 2.39.5 >
On Fri, Oct 25, 2024 at 09:15:14PM +0200, Armin Wolf wrote: > When changing the thermal policy using the platform profile API, > a Vivobook thermal policy is stored in throttle_thermal_policy_mode. > > However everywhere else a normal thermal policy is stored inside this > variable, potentially confusing the platform profile. > > Fix this by always storing normal thermal policy values inside > throttle_thermal_policy_mode and only do the conversion when writing > the thermal policy to hardware. > > Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") > Signed-off-by: Armin Wolf <W_Armin@gmx.de> > --- > drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- > 1 file changed, 21 insertions(+), 43 deletions(-) the original patch that i submitted did actually have the remapping of the different fan profiles in the throttle_thermal_policy_write() methods because it was the cleaner solution [1]. however after having a discussion with luke, he shared that he might be planning to remove the throttle_thermal_policy sysfs interface in favour of platform_profiles [2] because of a refactoring he had been working on. currently to control fan profiles through this driver you could use either /sys/devices/platform/asus-nb-wmi/throttle_thermal_policy (redundant and might get removed in the future) or through platform profiles which is the better way of doing things. for the reasons mentionned above, I decided to keep throttle_therma_policy_write() unchanged and to move the remapping logic to the asus_wmi_platform_profile_set(). this adopts the approach of having a logical mapping stored in asus_wmi struct that has to be converted to a physical mapping whenever needed [3]. so, if luke thinks that this won't cause any merge conflicts with his work [4] then i see no problem with this approach even though it might cause an order change when calling throttle_thermal_policy_switch_next() Best Regards, Mohamed G. Link: https://lore.kernel.org/platform-driver-x86/20240421194320.48258-2-mohamed.ghanmi@supcom.tn/ # [1] Link: https://lore.kernel.org/platform-driver-x86/4de768c5-aae5-4fda-a139-a8b73c8495a1@app.fastmail.com/ # [2] Link: https://lore.kernel.org/platform-driver-x86/ZnlEuiP4Dgqpf51C@laptop/ # [3] Link: https://lore.kernel.org/platform-driver-x86/20240930000046.51388-1-luke@ljones.dev/ # [4]
Hi, On 26-Oct-24 12:45 PM, Mohamed Ghanmi wrote: > On Fri, Oct 25, 2024 at 09:15:14PM +0200, Armin Wolf wrote: >> When changing the thermal policy using the platform profile API, >> a Vivobook thermal policy is stored in throttle_thermal_policy_mode. >> >> However everywhere else a normal thermal policy is stored inside this >> variable, potentially confusing the platform profile. >> >> Fix this by always storing normal thermal policy values inside >> throttle_thermal_policy_mode and only do the conversion when writing >> the thermal policy to hardware. >> >> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- >> 1 file changed, 21 insertions(+), 43 deletions(-) > > the original patch that i submitted did actually have the remapping > of the different fan profiles in the throttle_thermal_policy_write() methods > because it was the cleaner solution [1]. however after having a discussion with luke, > he shared that he might be planning to remove the throttle_thermal_policy sysfs interface > in favour of platform_profiles [2] because of a refactoring he had been working on. > > currently to control fan profiles through this driver you could use > either /sys/devices/platform/asus-nb-wmi/throttle_thermal_policy > (redundant and might get removed in the future) or through platform profiles which is the > better way of doing things. > > for the reasons mentionned above, I decided to keep > throttle_therma_policy_write() unchanged and to move the remapping logic > to the asus_wmi_platform_profile_set(). this adopts the approach of > having a logical mapping stored in asus_wmi struct that has to be > converted to a physical mapping whenever needed [3]. > > so, if luke thinks that this won't cause any merge conflicts with his > work [4] then i see no problem with this approach even though it might cause an > order change when calling throttle_thermal_policy_switch_next() Talking about throttle_thermal_policy_switch_next() we also have platform_profile_cycle() and since asus-wmi supports platform-profiles now I'm wondering if it would not be better to simply completely drop throttle_thermal_policy_switch_next() and call platform_profile_cycle() instead? This will also keep the cycle order the same for "normal" vs vivo even after Armin's patch. Anyways I'll go and apply patch 1/2 to pdx86/fixes since that one is obviously correct and fixes th Lunar Lake performance issues. And we can keep discussing what to do wrt 2/2 and maybe also drop throttle_thermal_policy_switch_next() if favor of platform_profile_cycle(). Regards, Hans > > Best Regards, > Mohamed G. > > Link: https://lore.kernel.org/platform-driver-x86/20240421194320.48258-2-mohamed.ghanmi@supcom.tn/ # [1] > Link: https://lore.kernel.org/platform-driver-x86/4de768c5-aae5-4fda-a139-a8b73c8495a1@app.fastmail.com/ # [2] > Link: https://lore.kernel.org/platform-driver-x86/ZnlEuiP4Dgqpf51C@laptop/ # [3] > Link: https://lore.kernel.org/platform-driver-x86/20240930000046.51388-1-luke@ljones.dev/ # [4] >
Am 26.10.24 um 11:56 schrieb Hans de Goede: > Hi Armin, > > On 25-Oct-24 9:15 PM, Armin Wolf wrote: >> When changing the thermal policy using the platform profile API, >> a Vivobook thermal policy is stored in throttle_thermal_policy_mode. >> >> However everywhere else a normal thermal policy is stored inside this >> variable, potentially confusing the platform profile. > You say "potentially confusing the platform profile", but did you > spot any actual issues when reviewing the code ? > Yes, for example: 1. User sets thermal policy to "1" (overboost) through the throttle_thermal_policy sysfs attr. 2. "1" gets stored inside throttle_thermal_policy_mode. 3. Platform profile will now think that ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO (1) is set. 4. Platform profile reports current mode as PLATFORM_PROFILE_QUIET. => error! >> Fix this by always storing normal thermal policy values inside >> throttle_thermal_policy_mode and only do the conversion when writing >> the thermal policy to hardware. >> >> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> > The problem with this approach is that it changes the order in which > we step through the modes in throttle_thermal_policy_switch_next() > after this change we have: > > Normal Asus: balanced -> performance -> silent -> balanced -> etc. > Vivobook: balanced -> silent -> performance -> balanced -> etc. > > where if we see "silent" as lower performance then the other 2, > the vivobook order is a bit weird. > > I wonder if this is a big enough issue to really worry about it > though; and I do like the cleanup / simpler code. I think users expect the normal Asus switching behavior, but you are right that this change should be explained inside the commit message. > > Note that this also causes a behavior change in > fan_curve_get_factory_default() as I mentioned in my cover-letter. Since fan_curve_get_factory_default() was introduced before commit bcbfcebda2cb, i think this patches actually fixes the behavior of this function. > > I think that that behavior change might be a good thing to do actually, > but at a minimum it needs to be documented in the commit msg. > > Regards, > > Hans > I will wait till the maintainer of asus-wmi can clarify whether or not fan_curve_get_factory_default() expects normal Asus thermal policy values or not. Once we resolved this i will send an updated series. Thanks, Armin Wolf >> --- >> drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- >> 1 file changed, 21 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c >> index ab9342a01a48..ce60835d0303 100644 >> --- a/drivers/platform/x86/asus-wmi.c >> +++ b/drivers/platform/x86/asus-wmi.c >> @@ -3696,10 +3696,28 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus) >> /* Throttle thermal policy ****************************************************/ >> static int throttle_thermal_policy_write(struct asus_wmi *asus) >> { >> - u8 value = asus->throttle_thermal_policy_mode; >> u32 retval; >> + u8 value; >> int err; >> >> + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO) { >> + switch (asus->throttle_thermal_policy_mode) { >> + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: >> + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; >> + break; >> + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: >> + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; >> + break; >> + case ASUS_THROTTLE_THERMAL_POLICY_SILENT: >> + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; >> + break; >> + default: >> + return -EINVAL; >> + } >> + } else { >> + value = asus->throttle_thermal_policy_mode; >> + } >> + >> err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev, >> value, &retval); >> >> @@ -3804,46 +3822,6 @@ static ssize_t throttle_thermal_policy_store(struct device *dev, >> static DEVICE_ATTR_RW(throttle_thermal_policy); >> >> /* Platform profile ***********************************************************/ >> -static int asus_wmi_platform_profile_to_vivo(struct asus_wmi *asus, int mode) >> -{ >> - bool vivo; >> - >> - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; >> - >> - if (vivo) { >> - switch (mode) { >> - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: >> - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; >> - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: >> - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; >> - case ASUS_THROTTLE_THERMAL_POLICY_SILENT: >> - return ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; >> - } >> - } >> - >> - return mode; >> -} >> - >> -static int asus_wmi_platform_profile_mode_from_vivo(struct asus_wmi *asus, int mode) >> -{ >> - bool vivo; >> - >> - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; >> - >> - if (vivo) { >> - switch (mode) { >> - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO: >> - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT; >> - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO: >> - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST; >> - case ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO: >> - return ASUS_THROTTLE_THERMAL_POLICY_SILENT; >> - } >> - } >> - >> - return mode; >> -} >> - >> static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, >> enum platform_profile_option *profile) >> { >> @@ -3853,7 +3831,7 @@ static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, >> asus = container_of(pprof, struct asus_wmi, platform_profile_handler); >> tp = asus->throttle_thermal_policy_mode; >> >> - switch (asus_wmi_platform_profile_mode_from_vivo(asus, tp)) { >> + switch (tp) { >> case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: >> *profile = PLATFORM_PROFILE_BALANCED; >> break; >> @@ -3892,7 +3870,7 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof, >> return -EOPNOTSUPP; >> } >> >> - asus->throttle_thermal_policy_mode = asus_wmi_platform_profile_to_vivo(asus, tp); >> + asus->throttle_thermal_policy_mode = tp; >> return throttle_thermal_policy_write(asus); >> } >> >> -- >> 2.39.5 >> >
Am 26.10.24 um 12:45 schrieb Mohamed Ghanmi: > On Fri, Oct 25, 2024 at 09:15:14PM +0200, Armin Wolf wrote: >> When changing the thermal policy using the platform profile API, >> a Vivobook thermal policy is stored in throttle_thermal_policy_mode. >> >> However everywhere else a normal thermal policy is stored inside this >> variable, potentially confusing the platform profile. >> >> Fix this by always storing normal thermal policy values inside >> throttle_thermal_policy_mode and only do the conversion when writing >> the thermal policy to hardware. >> >> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- >> 1 file changed, 21 insertions(+), 43 deletions(-) > the original patch that i submitted did actually have the remapping > of the different fan profiles in the throttle_thermal_policy_write() methods > because it was the cleaner solution [1]. however after having a discussion with luke, > he shared that he might be planning to remove the throttle_thermal_policy sysfs interface > in favour of platform_profiles [2] because of a refactoring he had been working on. > > currently to control fan profiles through this driver you could use > either /sys/devices/platform/asus-nb-wmi/throttle_thermal_policy > (redundant and might get removed in the future) or through platform profiles which is the > better way of doing things. > > for the reasons mentionned above, I decided to keep > throttle_therma_policy_write() unchanged and to move the remapping logic > to the asus_wmi_platform_profile_set(). this adopts the approach of > having a logical mapping stored in asus_wmi struct that has to be > converted to a physical mapping whenever needed [3]. > > so, if luke thinks that this won't cause any merge conflicts with his > work [4] then i see no problem with this approach even though it might cause an > order change when calling throttle_thermal_policy_switch_next() Ok, i will wait for Luke to give feedback on this series. In my opinion the order change is ok since users likely expect the "old" Asus switching order. > > Best Regards, > Mohamed G. > > Link: https://lore.kernel.org/platform-driver-x86/20240421194320.48258-2-mohamed.ghanmi@supcom.tn/ # [1] > Link: https://lore.kernel.org/platform-driver-x86/4de768c5-aae5-4fda-a139-a8b73c8495a1@app.fastmail.com/ # [2] > Link: https://lore.kernel.org/platform-driver-x86/ZnlEuiP4Dgqpf51C@laptop/ # [3] > Link: https://lore.kernel.org/platform-driver-x86/20240930000046.51388-1-luke@ljones.dev/ # [4] >
Am 26.10.24 um 12:59 schrieb Hans de Goede: > Hi, > > On 26-Oct-24 12:45 PM, Mohamed Ghanmi wrote: >> On Fri, Oct 25, 2024 at 09:15:14PM +0200, Armin Wolf wrote: >>> When changing the thermal policy using the platform profile API, >>> a Vivobook thermal policy is stored in throttle_thermal_policy_mode. >>> >>> However everywhere else a normal thermal policy is stored inside this >>> variable, potentially confusing the platform profile. >>> >>> Fix this by always storing normal thermal policy values inside >>> throttle_thermal_policy_mode and only do the conversion when writing >>> the thermal policy to hardware. >>> >>> Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") >>> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >>> --- >>> drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- >>> 1 file changed, 21 insertions(+), 43 deletions(-) >> the original patch that i submitted did actually have the remapping >> of the different fan profiles in the throttle_thermal_policy_write() methods >> because it was the cleaner solution [1]. however after having a discussion with luke, >> he shared that he might be planning to remove the throttle_thermal_policy sysfs interface >> in favour of platform_profiles [2] because of a refactoring he had been working on. >> >> currently to control fan profiles through this driver you could use >> either /sys/devices/platform/asus-nb-wmi/throttle_thermal_policy >> (redundant and might get removed in the future) or through platform profiles which is the >> better way of doing things. >> >> for the reasons mentionned above, I decided to keep >> throttle_therma_policy_write() unchanged and to move the remapping logic >> to the asus_wmi_platform_profile_set(). this adopts the approach of >> having a logical mapping stored in asus_wmi struct that has to be >> converted to a physical mapping whenever needed [3]. >> >> so, if luke thinks that this won't cause any merge conflicts with his >> work [4] then i see no problem with this approach even though it might cause an >> order change when calling throttle_thermal_policy_switch_next() > Talking about throttle_thermal_policy_switch_next() we also > have platform_profile_cycle() and since asus-wmi supports > platform-profiles now I'm wondering if it would not be better > to simply completely drop throttle_thermal_policy_switch_next() > and call platform_profile_cycle() instead? > > This will also keep the cycle order the same for "normal" vs > vivo even after Armin's patch. > > Anyways I'll go and apply patch 1/2 to pdx86/fixes since that one is > obviously correct and fixes th Lunar Lake performance issues. > > And we can keep discussing what to do wrt 2/2 and maybe also drop > throttle_thermal_policy_switch_next() if favor of > platform_profile_cycle(). > > Regards, > > Hans Good idea, using platform_profile_cycle() would also solve a potential locking issue here. Thanks, Armin Wolf >> Best Regards, >> Mohamed G. >> >> Link: https://lore.kernel.org/platform-driver-x86/20240421194320.48258-2-mohamed.ghanmi@supcom.tn/ # [1] >> Link: https://lore.kernel.org/platform-driver-x86/4de768c5-aae5-4fda-a139-a8b73c8495a1@app.fastmail.com/ # [2] >> Link: https://lore.kernel.org/platform-driver-x86/ZnlEuiP4Dgqpf51C@laptop/ # [3] >> Link: https://lore.kernel.org/platform-driver-x86/20240930000046.51388-1-luke@ljones.dev/ # [4] >> >
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c index ab9342a01a48..ce60835d0303 100644 --- a/drivers/platform/x86/asus-wmi.c +++ b/drivers/platform/x86/asus-wmi.c @@ -3696,10 +3696,28 @@ static int asus_wmi_custom_fan_curve_init(struct asus_wmi *asus) /* Throttle thermal policy ****************************************************/ static int throttle_thermal_policy_write(struct asus_wmi *asus) { - u8 value = asus->throttle_thermal_policy_mode; u32 retval; + u8 value; int err; + if (asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO) { + switch (asus->throttle_thermal_policy_mode) { + case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: + value = ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; + break; + case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: + value = ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; + break; + case ASUS_THROTTLE_THERMAL_POLICY_SILENT: + value = ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; + break; + default: + return -EINVAL; + } + } else { + value = asus->throttle_thermal_policy_mode; + } + err = asus_wmi_set_devstate(asus->throttle_thermal_policy_dev, value, &retval); @@ -3804,46 +3822,6 @@ static ssize_t throttle_thermal_policy_store(struct device *dev, static DEVICE_ATTR_RW(throttle_thermal_policy); /* Platform profile ***********************************************************/ -static int asus_wmi_platform_profile_to_vivo(struct asus_wmi *asus, int mode) -{ - bool vivo; - - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; - - if (vivo) { - switch (mode) { - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO; - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST: - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO; - case ASUS_THROTTLE_THERMAL_POLICY_SILENT: - return ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO; - } - } - - return mode; -} - -static int asus_wmi_platform_profile_mode_from_vivo(struct asus_wmi *asus, int mode) -{ - bool vivo; - - vivo = asus->throttle_thermal_policy_dev == ASUS_WMI_DEVID_THROTTLE_THERMAL_POLICY_VIVO; - - if (vivo) { - switch (mode) { - case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT_VIVO: - return ASUS_THROTTLE_THERMAL_POLICY_DEFAULT; - case ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST_VIVO: - return ASUS_THROTTLE_THERMAL_POLICY_OVERBOOST; - case ASUS_THROTTLE_THERMAL_POLICY_SILENT_VIVO: - return ASUS_THROTTLE_THERMAL_POLICY_SILENT; - } - } - - return mode; -} - static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, enum platform_profile_option *profile) { @@ -3853,7 +3831,7 @@ static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof, asus = container_of(pprof, struct asus_wmi, platform_profile_handler); tp = asus->throttle_thermal_policy_mode; - switch (asus_wmi_platform_profile_mode_from_vivo(asus, tp)) { + switch (tp) { case ASUS_THROTTLE_THERMAL_POLICY_DEFAULT: *profile = PLATFORM_PROFILE_BALANCED; break; @@ -3892,7 +3870,7 @@ static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof, return -EOPNOTSUPP; } - asus->throttle_thermal_policy_mode = asus_wmi_platform_profile_to_vivo(asus, tp); + asus->throttle_thermal_policy_mode = tp; return throttle_thermal_policy_write(asus); }
When changing the thermal policy using the platform profile API, a Vivobook thermal policy is stored in throttle_thermal_policy_mode. However everywhere else a normal thermal policy is stored inside this variable, potentially confusing the platform profile. Fix this by always storing normal thermal policy values inside throttle_thermal_policy_mode and only do the conversion when writing the thermal policy to hardware. Fixes: bcbfcebda2cb ("platform/x86: asus-wmi: add support for vivobook fan profiles") Signed-off-by: Armin Wolf <W_Armin@gmx.de> --- drivers/platform/x86/asus-wmi.c | 64 +++++++++++---------------------- 1 file changed, 21 insertions(+), 43 deletions(-) -- 2.39.5