When support for Vivobook fan profiles was added, two mistakes where
made:

1. throttle_thermal_policy_set_default() was not called anymore during
probe.

2. The new thermal profiles where used inconsistently.

This patch series aims to fix both issues. Compile-tested only.

Armin Wolf (2):
  platform/x86: asus-wmi: Fix thermal profile initialization
  platform/x86: asus-wmi: Fix inconsistent use of thermal policies

 drivers/platform/x86/asus-wmi.c | 74 ++++++++++++++-------------------
 1 file changed, 31 insertions(+), 43 deletions(-)

--
2.39.5

Hi All,

On 25-Oct-24 9:15 PM, Armin Wolf wrote:
> When support for Vivobook fan profiles was added, two mistakes where
> made:
> 
> 1. throttle_thermal_policy_set_default() was not called anymore during
> probe.
> 
> 2. The new thermal profiles where used inconsistently.
> 
> This patch series aims to fix both issues. Compile-tested only.
> 
> Armin Wolf (2):
>   platform/x86: asus-wmi: Fix thermal profile initialization
>   platform/x86: asus-wmi: Fix inconsistent use of thermal policies

Taking another look at the vivobook stuff because of this series this
pre-existing code stands out to me:

static int fan_curve_get_factory_default(struct asus_wmi *asus, u32 fan_dev)
{
        struct fan_curve_data *curves;
        u8 buf[FAN_CURVE_BUF_LEN];
        int err, fan_idx;
        u8 mode = 0;

        if (asus->throttle_thermal_policy_dev)
                mode = asus->throttle_thermal_policy_mode;
        /* DEVID_<C/G>PU_FAN_CURVE is switched for OVERBOOST vs SILENT */
        if (mode == 2)
                mode = 1;
        else if (mode == 1)
                mode = 2;


Since the vivobook has silent and overboost swapped I wonder if we should
do this on vivobook to ?

Also note that patch 2/2 of this series impacts this code too. Until
now we were storing the swapped vivobook values in asus->throttle_thermal_policy_dev
and then here we are swapping them a second time, in essence using unswapped
non vivobook values here due to the double swapping.

Where as after Armin's changes from 2/2 we now store unswapped standard
asus laptop values in asus->throttle_thermal_policy_dev and swap them
here, using the same mode values as with normal asus laptops on vivobooks
now ( mode is swapped from non vivo throttle_thermal_policy_dev values).

Does anyone have any insight what we should do here ?

Regards,

Hans

+Casey

On Fri, 2024-10-25 at 21:15 +0200, Armin Wolf wrote:
> When support for Vivobook fan profiles was added, two mistakes where
> made:
> 
> 1. throttle_thermal_policy_set_default() was not called anymore
> during
> probe.
> 
> 2. The new thermal profiles where used inconsistently.
> 
> This patch series aims to fix both issues. Compile-tested only.
> 
Thanks for these patches. The first one I already tested with the same
change, for the second one added Casey to check if he can give a quick
test for both on the new Asus Lunar Lake laptop.

Thanks,
Srinivas




> Armin Wolf (2):
>   platform/x86: asus-wmi: Fix thermal profile initialization
>   platform/x86: asus-wmi: Fix inconsistent use of thermal policies
> 
>  drivers/platform/x86/asus-wmi.c | 74 ++++++++++++++-----------------
> --
>  1 file changed, 31 insertions(+), 43 deletions(-)
> 
> --
> 2.39.5
> 

Ran a quick test with the two patches on top of drm-tip while monitoring the sensor's temperature as well as thermald logs.

While running "stress -c 8" for around two minutes for each mode, I can see the SEN1 temperature plateau at the expected areas for the various power modes:

- Power Saver
    - Max temp observed: 51 C
    - Trip temperature: 51 C

- Balanced
    - Max temp observed: 56 C
    - Trip temperature: 56 C

- Performance
    - Max temp observed: 61 C
    - Trip temperature: 61 C

Cycling from higher power modes to lower power modes works as expected, with the temperatures falling back to the peak values listed for the given mode.

From my observations, these patches are working as intended.

Regards,
Casey

________________________________________
From: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
Sent: Friday, October 25, 2024 9:09 PM
To: Armin Wolf <W_Armin@gmx.de>; corentin.chary@gmail.com <corentin.chary@gmail.com>; luke@ljones.dev <luke@ljones.dev>; Ghanmi, Mohamed <mohamed.ghanmi@supcom.tn>; Bowman, Casey G <casey.g.bowman@intel.com>
Cc: hdegoede@redhat.com <hdegoede@redhat.com>; ilpo.jarvinen@linux.intel.com <ilpo.jarvinen@linux.intel.com>; Michael@phoronix.com <Michael@phoronix.com>; Bowman, Casey G <casey.g.bowman@intel.com>; platform-driver-x86@vger.kernel.org <platform-driver-x86@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 0/2] platform/x86: asus-wmi: Fix thermal profile handling
 
+Casey

On Fri, 2024-10-25 at 21:15 +0200, Armin Wolf wrote:
> When support for Vivobook fan profiles was added, two mistakes where
> made:
>
> 1. throttle_thermal_policy_set_default() was not called anymore
> during
> probe.
>
> 2. The new thermal profiles where used inconsistently.
>
> This patch series aims to fix both issues. Compile-tested only.
>
Thanks for these patches. The first one I already tested with the same
change, for the second one added Casey to check if he can give a quick
test for both on the new Asus Lunar Lake laptop.

Thanks,
Srinivas




> Armin Wolf (2):
>   platform/x86: asus-wmi: Fix thermal profile initialization
>   platform/x86: asus-wmi: Fix inconsistent use of thermal policies
>
>  drivers/platform/x86/asus-wmi.c | 74 ++++++++++++++-----------------
> --
>  1 file changed, 31 insertions(+), 43 deletions(-)
>
> --
> 2.39.5
>