diff mbox series

[2/2] platform/x86: hp-wmi: Add thermal profile support for 8BAD boards

Message ID ZZFGgfsfrU2vuQoI@alexis-pc (mailing list archive)
State Changes Requested, archived
Headers show
Series [1/2] platform/x86: hp-wmi: Tidy up module source code | expand

Commit Message

Alexis Belmonte Dec. 31, 2023, 10:46 a.m. UTC
Add 8BAD to the list of boards which have thermal profile selection
available. This allows the CPU to draw more power than the default TDP
barrier defined by the 'balanced' thermal profile (around 50W), hence
allowing it to perform better without being throttled by the embedded
controller (around 130W).

We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
This prevents the timer countdown from reaching zero, making the embedded
controller "force-switch" the system's thermal profile back to 'balanced'
automatically.

We also need to put a number of specific flags in
HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
thermal profile:

   - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
     HP_OMEN_EC_FLAGS_NOTIMER;

   - for 'balanced' and 'powersave', we clear out the register to notify
     the system that we want to lower the TDP barrier as soon as possible.

The third flag defined in the hp_thermal_profile_omen_flags enum,
HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.

To prevent potential behaviour breakage with other Omen models, a
separate omen_timed_thermal_profile_boards array has been added to list
which boards expose this behaviour.

Performance benchmarking was done with the help of silver.urih.com and
Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
thermal profile set:

|                  | Performance |     Stress |   TDP |
|------------------|-------------|------------|-------|
|    with my patch |      P84549 |    S0.1891 |  131W | 
| without my patch |      P44084 |    S0.1359 |   47W |

The TDP measurements were done with the help of the s-tui utility,
during the load.

There is still work to be done:

   - tune the CPU and GPU fans to better cool down and enhance
     performance at the right time; right now, it seems that the fans are
     not properly reacting to thermal/performance events, which in turn
     either causes thermal throttling OR makes the fans spin way too long,
     even though the temperatures have lowered down

   - expose the CPU and GPU fan curves to user-land so that they can be
     controlled just like what the Omen Gaming Hub utility proposes to
     its users;

Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 63 +++++++++++++++++++++++++++++++-
 1 file changed, 61 insertions(+), 2 deletions(-)

Comments

Prajna Sariputra Jan. 3, 2024, 12:21 a.m. UTC | #1
On Sunday, 31 December 2023 9:46:25 PM AEDT Alexis Belmonte wrote:
> Add 8BAD to the list of boards which have thermal profile selection
> available. This allows the CPU to draw more power than the default TDP
> barrier defined by the 'balanced' thermal profile (around 50W), hence
> allowing it to perform better without being throttled by the embedded
> controller (around 130W).
> 
> We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
> This prevents the timer countdown from reaching zero, making the embedded
> controller "force-switch" the system's thermal profile back to 'balanced'
> automatically.
> 
> We also need to put a number of specific flags in
> HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
> thermal profile:
> 
>    - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
>      HP_OMEN_EC_FLAGS_NOTIMER;
> 
>    - for 'balanced' and 'powersave', we clear out the register to notify
>      the system that we want to lower the TDP barrier as soon as possible.

Do you know if there's a way to check that a given model has this specific timer,
other than just testing the patch?

I have an Omen 16-n0000 (8A42), which has a Ryzen 7 6800H and a Radeon
RX 6650M, and I've been patching the kernel to add it to the omen_thermal_profile_boards
array for a while now. Just doing that prevents the worst of the throttling from
happening (GPU dropping from 105W to 35W and the CPU being stuck at like 2GHz
or less), but currently the GPU still drops to 75W eventually. Switching to
performance does make it go back to 105W (and even 120W for a bit) before it
goes back down to 75W, so it makes me wonder if there is actually a timer on my
model that's doing it rather than just thermal throttling.

> 
> The third flag defined in the hp_thermal_profile_omen_flags enum,
> HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.
> 
> To prevent potential behaviour breakage with other Omen models, a
> separate omen_timed_thermal_profile_boards array has been added to list
> which boards expose this behaviour.
> 
> Performance benchmarking was done with the help of silver.urih.com and
> Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
> thermal profile set:
> 
> |                  | Performance |     Stress |   TDP |
> |------------------|-------------|------------|-------|
> |    with my patch |      P84549 |    S0.1891 |  131W | 
> | without my patch |      P44084 |    S0.1359 |   47W |
> 
> The TDP measurements were done with the help of the s-tui utility,
> during the load.
> 
> There is still work to be done:
> 
>    - tune the CPU and GPU fans to better cool down and enhance
>      performance at the right time; right now, it seems that the fans are
>      not properly reacting to thermal/performance events, which in turn
>      either causes thermal throttling OR makes the fans spin way too long,
>      even though the temperatures have lowered down

Yeah, that's also a problem with my model, where with a heavy CPU only workload
the CPU would boost high and almost immediately run into thermal throttling and
stays throttled for like a few minutes before the fans ramp up even a little,
which is why I'm not sure that adding my model to the list upstream would be a
good idea. My CPU doesn't seem to boost all that high though, I don't remember
the performance mode making much of a difference the last time I tested it.

Also, for what it's worth I had a conversation with one of the folks who wrote
the platform profile code (Enver Balalic) a few months ago, and they said the
profiles are just fan speed control modes on their Omen model.

> 
>    - expose the CPU and GPU fan curves to user-land so that they can be
>      controlled just like what the Omen Gaming Hub utility proposes to
>      its users;
> 
> Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 63 +++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 95282c3a02ed..79caf5d79e05 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -38,6 +38,8 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
>  #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
>  #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
>  
> +#define HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET 0x62
> +#define HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET 0x63
>  #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
>  
>  #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
> @@ -57,7 +59,7 @@ static const char * const omen_thermal_profile_boards[] = {
>  	"874A", "8603", "8604", "8748", "886B", "886C", "878A", "878B", "878C",
>  	"88C8", "88CB", "8786", "8787", "8788", "88D1", "88D2", "88F4", "88FD",
>  	"88F5", "88F6", "88F7", "88FE", "88FF", "8900", "8901", "8902", "8912",
> -	"8917", "8918", "8949", "894A", "89EB"
> +	"8917", "8918", "8949", "894A", "89EB", "8BAD"
>  };
>  
>  /* DMI Board names of Omen laptops that are specifically set to be thermal
> @@ -68,6 +71,14 @@ static const char * const omen_thermal_profile_force_v0_boards[] = {
>  	"8607", "8746", "8747", "8749", "874A", "8748"
>  };
>  
> +/* DMI board names of Omen laptops that have a thermal profile timer which will
> + * cause the embedded controller to set the thermal profile back to
> + * "balanced" when reaching zero.
> + */
> +static const char * const omen_timed_thermal_profile_boards[] = {
> +	"8BAD"
> +};
> +
>  /* DMI Board names of Victus laptops */
>  static const char * const victus_thermal_profile_boards[] = {
>  	"8A25"
> @@ -184,6 +194,12 @@ enum hp_thermal_profile_omen_v1 {
>  	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
>  };
>  
> +enum hp_thermal_profile_omen_flags {
> +	HP_OMEN_EC_FLAGS_TURBO		= 0x04,
> +	HP_OMEN_EC_FLAGS_NOTIMER	= 0x02,
> +	HP_OMEN_EC_FLAGS_JUSTSET	= 0x01,
> +};
> +
>  enum hp_thermal_profile_victus {
>  	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
>  	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> @@ -451,7 +467,11 @@ static int hp_wmi_get_tablet_mode(void)
>  
>  static int omen_thermal_profile_set(int mode)
>  {
> -	char buffer[2] = {0, mode};
> +	/* The Omen Control Center actively sets the first byte of the buffer to
> +	 * 255, so let's mimic this behaviour to be as close as possible to
> +	 * the original software.
> +	 */
> +	char buffer[2] = {-1, mode};
>  	int ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
> @@ -1203,6 +1221,28 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>  	return 0;
>  }
>  
> +static bool has_omen_thermal_profile_ec_timer(void)
> +{
> +	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> +	if (!board_name)
> +		return false;
> +
> +	return match_string(omen_timed_thermal_profile_boards,
> +			    ARRAY_SIZE(omen_timed_thermal_profile_boards),
> +			    board_name) >= 0;
> +}
> +
> +inline int omen_thermal_profile_ec_flags_set(enum hp_thermal_profile_omen_flags flags)
> +{
> +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET, flags);
> +}
> +
> +inline int omen_thermal_profile_ec_timer_set(char value)
> +{
> +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET, value);
> +}
> +
>  static int platform_profile_omen_set(struct platform_profile_handler *pprof,
>  				     enum platform_profile_option profile)
>  {
> @@ -1240,6 +1280,24 @@ static int platform_profile_omen_set(struct platform_profile_handler *pprof,
>  	if (err < 0)
>  		return err;
>  
> +	if (has_omen_thermal_profile_ec_timer()) {
> +		err = omen_thermal_profile_ec_timer_set(0);
> +		if (err < 0)
> +			return err;
> +
> +		enum hp_thermal_profile_omen_flags flags;
> +
> +		if (profile == PLATFORM_PROFILE_PERFORMANCE)
> +			flags = HP_OMEN_EC_FLAGS_NOTIMER |
> +				HP_OMEN_EC_FLAGS_TURBO;
> +		else
> +			flags = 0;
> +
> +		err = omen_thermal_profile_ec_flags_set(flags);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	return 0;
>  }
>  
>
Ilpo Järvinen Jan. 4, 2024, 6 p.m. UTC | #2
On Sun, 31 Dec 2023, Alexis Belmonte wrote:

> Add 8BAD to the list of boards which have thermal profile selection
> available. This allows the CPU to draw more power than the default TDP
> barrier defined by the 'balanced' thermal profile (around 50W), hence
> allowing it to perform better without being throttled by the embedded
> controller (around 130W).
> 
> We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
> This prevents the timer countdown from reaching zero, making the embedded
> controller "force-switch" the system's thermal profile back to 'balanced'
> automatically.
> 
> We also need to put a number of specific flags in
> HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
> thermal profile:
> 
>    - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
>      HP_OMEN_EC_FLAGS_NOTIMER;
> 
>    - for 'balanced' and 'powersave', we clear out the register to notify
>      the system that we want to lower the TDP barrier as soon as possible.
> 
> The third flag defined in the hp_thermal_profile_omen_flags enum,
> HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.

I don't understand this. Based on the name of the flag, I'd have expected 
it has to be set (but its purpose is not known), but the code below does 
not do that?

> To prevent potential behaviour breakage with other Omen models, a
> separate omen_timed_thermal_profile_boards array has been added to list
> which boards expose this behaviour.
> 
> Performance benchmarking was done with the help of silver.urih.com and
> Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
> thermal profile set:
> 
> |                  | Performance |     Stress |   TDP |
> |------------------|-------------|------------|-------|
> |    with my patch |      P84549 |    S0.1891 |  131W | 
> | without my patch |      P44084 |    S0.1359 |   47W |
> 
> The TDP measurements were done with the help of the s-tui utility,
> during the load.
> 
> There is still work to be done:
> 
>    - tune the CPU and GPU fans to better cool down and enhance
>      performance at the right time; right now, it seems that the fans are
>      not properly reacting to thermal/performance events, which in turn
>      either causes thermal throttling OR makes the fans spin way too long,
>      even though the temperatures have lowered down
> 
>    - expose the CPU and GPU fan curves to user-land so that they can be
>      controlled just like what the Omen Gaming Hub utility proposes to
>      its users;
> 
> Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 63 +++++++++++++++++++++++++++++++-
>  1 file changed, 61 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 95282c3a02ed..79caf5d79e05 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -38,6 +38,8 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
>  #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
>  #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
>  
> +#define HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET 0x62
> +#define HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET 0x63
>  #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
>  
>  #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
> @@ -57,7 +59,7 @@ static const char * const omen_thermal_profile_boards[] = {
>  	"874A", "8603", "8604", "8748", "886B", "886C", "878A", "878B", "878C",
>  	"88C8", "88CB", "8786", "8787", "8788", "88D1", "88D2", "88F4", "88FD",
>  	"88F5", "88F6", "88F7", "88FE", "88FF", "8900", "8901", "8902", "8912",
> -	"8917", "8918", "8949", "894A", "89EB"
> +	"8917", "8918", "8949", "894A", "89EB", "8BAD"
>  };
>  
>  /* DMI Board names of Omen laptops that are specifically set to be thermal
> @@ -68,6 +71,14 @@ static const char * const omen_thermal_profile_force_v0_boards[] = {
>  	"8607", "8746", "8747", "8749", "874A", "8748"
>  };
>  
> +/* DMI board names of Omen laptops that have a thermal profile timer which will
> + * cause the embedded controller to set the thermal profile back to
> + * "balanced" when reaching zero.
> + */
> +static const char * const omen_timed_thermal_profile_boards[] = {
> +	"8BAD"
> +};
> +
>  /* DMI Board names of Victus laptops */
>  static const char * const victus_thermal_profile_boards[] = {
>  	"8A25"
> @@ -184,6 +194,12 @@ enum hp_thermal_profile_omen_v1 {
>  	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
>  };
>  
> +enum hp_thermal_profile_omen_flags {
> +	HP_OMEN_EC_FLAGS_TURBO		= 0x04,
> +	HP_OMEN_EC_FLAGS_NOTIMER	= 0x02,
> +	HP_OMEN_EC_FLAGS_JUSTSET	= 0x01,
> +};
> +
>  enum hp_thermal_profile_victus {
>  	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
>  	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> @@ -451,7 +467,11 @@ static int hp_wmi_get_tablet_mode(void)
>  
>  static int omen_thermal_profile_set(int mode)
>  {
> -	char buffer[2] = {0, mode};
> +	/* The Omen Control Center actively sets the first byte of the buffer to
> +	 * 255, so let's mimic this behaviour to be as close as possible to
> +	 * the original software.
> +	 */
> +	char buffer[2] = {-1, mode};
>  	int ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
> @@ -1203,6 +1221,28 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>  	return 0;
>  }
>  
> +static bool has_omen_thermal_profile_ec_timer(void)
> +{
> +	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> +	if (!board_name)
> +		return false;
> +
> +	return match_string(omen_timed_thermal_profile_boards,
> +			    ARRAY_SIZE(omen_timed_thermal_profile_boards),
> +			    board_name) >= 0;
> +}
> +
> +inline int omen_thermal_profile_ec_flags_set(enum hp_thermal_profile_omen_flags flags)
> +{
> +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET, flags);
> +}
> +
> +inline int omen_thermal_profile_ec_timer_set(char value)

u8 ?

I know this driver uses char in many place but if this is binary data, u8 
is the correct type (and the other char instances should be converted 
to u8 too eventually).

> +{
> +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET, value);
> +}
> +
>  static int platform_profile_omen_set(struct platform_profile_handler *pprof,
>  				     enum platform_profile_option profile)
>  {
> @@ -1240,6 +1280,24 @@ static int platform_profile_omen_set(struct platform_profile_handler *pprof,
>  	if (err < 0)
>  		return err;
>  
> +	if (has_omen_thermal_profile_ec_timer()) {
> +		err = omen_thermal_profile_ec_timer_set(0);
> +		if (err < 0)
> +			return err;
> +
> +		enum hp_thermal_profile_omen_flags flags;

Please put this to the usual place (beginning of the block or function).
I know it's now possible to place them anywhere because of what cleanup.h 
requires but lets not start to spread them all around just because we can,
it makes finding the declaration harder, you could also initialize to 0 
so you don't need the else branch below.

> +
> +		if (profile == PLATFORM_PROFILE_PERFORMANCE)
> +			flags = HP_OMEN_EC_FLAGS_NOTIMER |
> +				HP_OMEN_EC_FLAGS_TURBO;
> +		else
> +			flags = 0;
> +
> +		err = omen_thermal_profile_ec_flags_set(flags);
> +		if (err < 0)
> +			return err;
> +	}
> +
>  	return 0;
>  }
Prajna Sariputra Jan. 4, 2024, 6:28 p.m. UTC | #3
On Wednesday, 3 January 2024 11:21:24 AM AEDT Prajna Sariputra wrote:
> On Sunday, 31 December 2023 9:46:25 PM AEDT Alexis Belmonte wrote:
> > Add 8BAD to the list of boards which have thermal profile selection
> > available. This allows the CPU to draw more power than the default TDP
> > barrier defined by the 'balanced' thermal profile (around 50W), hence
> > allowing it to perform better without being throttled by the embedded
> > controller (around 130W).
> > 
> > We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
> > This prevents the timer countdown from reaching zero, making the embedded
> > controller "force-switch" the system's thermal profile back to 'balanced'
> > automatically.
> > 
> > We also need to put a number of specific flags in
> > HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
> > thermal profile:
> > 
> >    - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
> >      HP_OMEN_EC_FLAGS_NOTIMER;
> > 
> >    - for 'balanced' and 'powersave', we clear out the register to notify
> >      the system that we want to lower the TDP barrier as soon as possible.
> 
> Do you know if there's a way to check that a given model has this specific timer,
> other than just testing the patch?
> 
> I have an Omen 16-n0000 (8A42), which has a Ryzen 7 6800H and a Radeon
> RX 6650M, and I've been patching the kernel to add it to the omen_thermal_profile_boards
> array for a while now. Just doing that prevents the worst of the throttling from
> happening (GPU dropping from 105W to 35W and the CPU being stuck at like 2GHz
> or less), but currently the GPU still drops to 75W eventually. Switching to
> performance does make it go back to 105W (and even 120W for a bit) before it
> goes back down to 75W, so it makes me wonder if there is actually a timer on my
> model that's doing it rather than just thermal throttling.

I ended up just testing the patch for myself (after adding my model number to
the arrays), and it does improve the GPU performance further for me, instead
of the GPU dropping to 75W after 2-4 minutes it is now able to maintain at least
100W even after 10 minutes (tested with Quake 2 RTX). So, it seems like the timer
thing also applies to my Omen 16-n0000 (8A42). That performance also roughly
matches up with how notebookcheck.net says my Omen performs in their review
(103W performance, 72W balanced), so it'd be great if you can also include my
model in your patch.
> 
> > 
> > The third flag defined in the hp_thermal_profile_omen_flags enum,
> > HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.
> > 
> > To prevent potential behaviour breakage with other Omen models, a
> > separate omen_timed_thermal_profile_boards array has been added to list
> > which boards expose this behaviour.
> > 
> > Performance benchmarking was done with the help of silver.urih.com and
> > Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
> > thermal profile set:
> > 
> > |                  | Performance |     Stress |   TDP |
> > |------------------|-------------|------------|-------|
> > |    with my patch |      P84549 |    S0.1891 |  131W | 
> > | without my patch |      P44084 |    S0.1359 |   47W |
> > 
> > The TDP measurements were done with the help of the s-tui utility,
> > during the load.
> > 
> > There is still work to be done:
> > 
> >    - tune the CPU and GPU fans to better cool down and enhance
> >      performance at the right time; right now, it seems that the fans are
> >      not properly reacting to thermal/performance events, which in turn
> >      either causes thermal throttling OR makes the fans spin way too long,
> >      even though the temperatures have lowered down
> 
> Yeah, that's also a problem with my model, where with a heavy CPU only workload
> the CPU would boost high and almost immediately run into thermal throttling and
> stays throttled for like a few minutes before the fans ramp up even a little,
> which is why I'm not sure that adding my model to the list upstream would be a
> good idea. My CPU doesn't seem to boost all that high though, I don't remember
> the performance mode making much of a difference the last time I tested it.

I just ran more tests with the CPU performance, and it seems that I might have
misremembered how bad the fan curve was, since I have been limiting the CPU
frequency to 4GHz instead of letting the CPU do its thing by itself (max boost is
4.7GHz), if I go back to the latter then with a heavily multithreaded workload
(like compiling the kernel) the fans ramp up high within a few seconds of the
CPU reaching 100C on its hottest core, and the CPU seems to maintain that
temperature (or the sensors just don't report values above 100C, not sure). That
seems worrying given that the supposed max operating temperature for the CPU
(Ryzen 7 6800H) is 95C, but then again that probably won't be the case when gaming,
which is the main use case for these laptops anyway.
> 
> Also, for what it's worth I had a conversation with one of the folks who wrote
> the platform profile code (Enver Balalic) a few months ago, and they said the
> profiles are just fan speed control modes on their Omen model.
> 
> > 
> >    - expose the CPU and GPU fan curves to user-land so that they can be
> >      controlled just like what the Omen Gaming Hub utility proposes to
> >      its users;
> > 
> > Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
> > ---
> >  drivers/platform/x86/hp/hp-wmi.c | 63 +++++++++++++++++++++++++++++++-
> >  1 file changed, 61 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> > index 95282c3a02ed..79caf5d79e05 100644
> > --- a/drivers/platform/x86/hp/hp-wmi.c
> > +++ b/drivers/platform/x86/hp/hp-wmi.c
> > @@ -38,6 +38,8 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
> >  #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
> >  #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
> >  
> > +#define HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET 0x62
> > +#define HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET 0x63
> >  #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
> >  
> >  #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
> > @@ -57,7 +59,7 @@ static const char * const omen_thermal_profile_boards[] = {
> >  	"874A", "8603", "8604", "8748", "886B", "886C", "878A", "878B", "878C",
> >  	"88C8", "88CB", "8786", "8787", "8788", "88D1", "88D2", "88F4", "88FD",
> >  	"88F5", "88F6", "88F7", "88FE", "88FF", "8900", "8901", "8902", "8912",
> > -	"8917", "8918", "8949", "894A", "89EB"
> > +	"8917", "8918", "8949", "894A", "89EB", "8BAD"
> >  };
> >  
> >  /* DMI Board names of Omen laptops that are specifically set to be thermal
> > @@ -68,6 +71,14 @@ static const char * const omen_thermal_profile_force_v0_boards[] = {
> >  	"8607", "8746", "8747", "8749", "874A", "8748"
> >  };
> >  
> > +/* DMI board names of Omen laptops that have a thermal profile timer which will
> > + * cause the embedded controller to set the thermal profile back to
> > + * "balanced" when reaching zero.
> > + */
> > +static const char * const omen_timed_thermal_profile_boards[] = {
> > +	"8BAD"
> > +};
> > +
> >  /* DMI Board names of Victus laptops */
> >  static const char * const victus_thermal_profile_boards[] = {
> >  	"8A25"
> > @@ -184,6 +194,12 @@ enum hp_thermal_profile_omen_v1 {
> >  	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
> >  };
> >  
> > +enum hp_thermal_profile_omen_flags {
> > +	HP_OMEN_EC_FLAGS_TURBO		= 0x04,
> > +	HP_OMEN_EC_FLAGS_NOTIMER	= 0x02,
> > +	HP_OMEN_EC_FLAGS_JUSTSET	= 0x01,
> > +};
> > +
> >  enum hp_thermal_profile_victus {
> >  	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
> >  	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> > @@ -451,7 +467,11 @@ static int hp_wmi_get_tablet_mode(void)
> >  
> >  static int omen_thermal_profile_set(int mode)
> >  {
> > -	char buffer[2] = {0, mode};
> > +	/* The Omen Control Center actively sets the first byte of the buffer to
> > +	 * 255, so let's mimic this behaviour to be as close as possible to
> > +	 * the original software.
> > +	 */
> > +	char buffer[2] = {-1, mode};
> >  	int ret;
> >  
> >  	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
> > @@ -1203,6 +1221,28 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
> >  	return 0;
> >  }
> >  
> > +static bool has_omen_thermal_profile_ec_timer(void)
> > +{
> > +	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> > +
> > +	if (!board_name)
> > +		return false;
> > +
> > +	return match_string(omen_timed_thermal_profile_boards,
> > +			    ARRAY_SIZE(omen_timed_thermal_profile_boards),
> > +			    board_name) >= 0;
> > +}
> > +
> > +inline int omen_thermal_profile_ec_flags_set(enum hp_thermal_profile_omen_flags flags)
> > +{
> > +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET, flags);
> > +}
> > +
> > +inline int omen_thermal_profile_ec_timer_set(char value)
> > +{
> > +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET, value);
> > +}
> > +
> >  static int platform_profile_omen_set(struct platform_profile_handler *pprof,
> >  				     enum platform_profile_option profile)
> >  {
> > @@ -1240,6 +1280,24 @@ static int platform_profile_omen_set(struct platform_profile_handler *pprof,
> >  	if (err < 0)
> >  		return err;
> >  
> > +	if (has_omen_thermal_profile_ec_timer()) {
> > +		err = omen_thermal_profile_ec_timer_set(0);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		enum hp_thermal_profile_omen_flags flags;
> > +
> > +		if (profile == PLATFORM_PROFILE_PERFORMANCE)
> > +			flags = HP_OMEN_EC_FLAGS_NOTIMER |
> > +				HP_OMEN_EC_FLAGS_TURBO;
> > +		else
> > +			flags = 0;
> > +
> > +		err = omen_thermal_profile_ec_flags_set(flags);
> > +		if (err < 0)
> > +			return err;
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > 
> 
> 
> 
> 
> 
>
Alexis Belmonte Jan. 4, 2024, 9:20 p.m. UTC | #4
On Thu, Jan 04, 2024 at 08:00:15PM +0200, Ilpo Järvinen wrote:
> On Sun, 31 Dec 2023, Alexis Belmonte wrote:
> 
> > Add 8BAD to the list of boards which have thermal profile selection
> > available. This allows the CPU to draw more power than the default TDP
> > barrier defined by the 'balanced' thermal profile (around 50W), hence
> > allowing it to perform better without being throttled by the embedded
> > controller (around 130W).
> > 
> > We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
> > This prevents the timer countdown from reaching zero, making the embedded
> > controller "force-switch" the system's thermal profile back to 'balanced'
> > automatically.
> > 
> > We also need to put a number of specific flags in
> > HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
> > thermal profile:
> > 
> >    - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
> >      HP_OMEN_EC_FLAGS_NOTIMER;
> > 
> >    - for 'balanced' and 'powersave', we clear out the register to notify
> >      the system that we want to lower the TDP barrier as soon as possible.
> > 
> > The third flag defined in the hp_thermal_profile_omen_flags enum,
> > HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.
> 
> I don't understand this. Based on the name of the flag, I'd have expected 
> it has to be set (but its purpose is not known), but the code below does 
> not do that?

It does not indeed, that's what I meant by 'completeness'. I defined it as a way
to show that I acknowledged the existence of that bit in the register, but I do
not use it.

I can comment it (or even remove it) if necessary.

On Thu, Jan 04, 2024 at 08:00:15PM +0200, Ilpo Järvinen wrote:
> > To prevent potential behaviour breakage with other Omen models, a
> > separate omen_timed_thermal_profile_boards array has been added to list
> > which boards expose this behaviour.
> > 
> > Performance benchmarking was done with the help of silver.urih.com and
> > Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
> > thermal profile set:
> > 
> > |                  | Performance |     Stress |   TDP |
> > |------------------|-------------|------------|-------|
> > |    with my patch |      P84549 |    S0.1891 |  131W | 
> > | without my patch |      P44084 |    S0.1359 |   47W |
> > 
> > The TDP measurements were done with the help of the s-tui utility,
> > during the load.
> > 
> > There is still work to be done:
> > 
> >    - tune the CPU and GPU fans to better cool down and enhance
> >      performance at the right time; right now, it seems that the fans are
> >      not properly reacting to thermal/performance events, which in turn
> >      either causes thermal throttling OR makes the fans spin way too long,
> >      even though the temperatures have lowered down
> > 
> >    - expose the CPU and GPU fan curves to user-land so that they can be
> >      controlled just like what the Omen Gaming Hub utility proposes to
> >      its users;
> > 
> > Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
> > ---
> >  drivers/platform/x86/hp/hp-wmi.c | 63 +++++++++++++++++++++++++++++++-
> >  1 file changed, 61 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> > index 95282c3a02ed..79caf5d79e05 100644
> > --- a/drivers/platform/x86/hp/hp-wmi.c
> > +++ b/drivers/platform/x86/hp/hp-wmi.c
> > @@ -38,6 +38,8 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
> >  #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
> >  #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
> >  
> > +#define HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET 0x62
> > +#define HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET 0x63
> >  #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
> >  
> >  #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
> > @@ -57,7 +59,7 @@ static const char * const omen_thermal_profile_boards[] = {
> >  	"874A", "8603", "8604", "8748", "886B", "886C", "878A", "878B", "878C",
> >  	"88C8", "88CB", "8786", "8787", "8788", "88D1", "88D2", "88F4", "88FD",
> >  	"88F5", "88F6", "88F7", "88FE", "88FF", "8900", "8901", "8902", "8912",
> > -	"8917", "8918", "8949", "894A", "89EB"
> > +	"8917", "8918", "8949", "894A", "89EB", "8BAD"
> >  };
> >  
> >  /* DMI Board names of Omen laptops that are specifically set to be thermal
> > @@ -68,6 +71,14 @@ static const char * const omen_thermal_profile_force_v0_boards[] = {
> >  	"8607", "8746", "8747", "8749", "874A", "8748"
> >  };
> >  
> > +/* DMI board names of Omen laptops that have a thermal profile timer which will
> > + * cause the embedded controller to set the thermal profile back to
> > + * "balanced" when reaching zero.
> > + */
> > +static const char * const omen_timed_thermal_profile_boards[] = {
> > +	"8BAD"
> > +};
> > +
> >  /* DMI Board names of Victus laptops */
> >  static const char * const victus_thermal_profile_boards[] = {
> >  	"8A25"
> > @@ -184,6 +194,12 @@ enum hp_thermal_profile_omen_v1 {
> >  	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
> >  };
> >  
> > +enum hp_thermal_profile_omen_flags {
> > +	HP_OMEN_EC_FLAGS_TURBO		= 0x04,
> > +	HP_OMEN_EC_FLAGS_NOTIMER	= 0x02,
> > +	HP_OMEN_EC_FLAGS_JUSTSET	= 0x01,
> > +};
> > +
> >  enum hp_thermal_profile_victus {
> >  	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
> >  	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> > @@ -451,7 +467,11 @@ static int hp_wmi_get_tablet_mode(void)
> >  
> >  static int omen_thermal_profile_set(int mode)
> >  {
> > -	char buffer[2] = {0, mode};
> > +	/* The Omen Control Center actively sets the first byte of the buffer to
> > +	 * 255, so let's mimic this behaviour to be as close as possible to
> > +	 * the original software.
> > +	 */
> > +	char buffer[2] = {-1, mode};
> >  	int ret;
> >  
> >  	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
> > @@ -1203,6 +1221,28 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
> >  	return 0;
> >  }
> >  
> > +static bool has_omen_thermal_profile_ec_timer(void)
> > +{
> > +	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> > +
> > +	if (!board_name)
> > +		return false;
> > +
> > +	return match_string(omen_timed_thermal_profile_boards,
> > +			    ARRAY_SIZE(omen_timed_thermal_profile_boards),
> > +			    board_name) >= 0;
> > +}
> > +
> > +inline int omen_thermal_profile_ec_flags_set(enum hp_thermal_profile_omen_flags flags)
> > +{
> > +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET, flags);
> > +}
> > +
> > +inline int omen_thermal_profile_ec_timer_set(char value)
> 
> u8 ?
> 
> I know this driver uses char in many place but if this is binary data, u8 
> is the correct type (and the other char instances should be converted 
> to u8 too eventually).
>

Alright, I did not know about this. I am a first timer to this driver
and the whole Linux kernel source code, so I'm happy to learn things
from this experience! :]

> > +{
> > +	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET, value);
> > +}
> > +
> >  static int platform_profile_omen_set(struct platform_profile_handler *pprof,
> >  				     enum platform_profile_option profile)
> >  {
> > @@ -1240,6 +1280,24 @@ static int platform_profile_omen_set(struct platform_profile_handler *pprof,
> >  	if (err < 0)
> >  		return err;
> >  
> > +	if (has_omen_thermal_profile_ec_timer()) {
> > +		err = omen_thermal_profile_ec_timer_set(0);
> > +		if (err < 0)
> > +			return err;
> > +
> > +		enum hp_thermal_profile_omen_flags flags;
> 
> Please put this to the usual place (beginning of the block or function).
> I know it's now possible to place them anywhere because of what cleanup.h 
> requires but lets not start to spread them all around just because we can,
> it makes finding the declaration harder, you could also initialize to 0 
> so you don't need the else branch below.
> 
> > +
> > +		if (profile == PLATFORM_PROFILE_PERFORMANCE)
> > +			flags = HP_OMEN_EC_FLAGS_NOTIMER |
> > +				HP_OMEN_EC_FLAGS_TURBO;
> > +		else
> > +			flags = 0;
> > +
> > +		err = omen_thermal_profile_ec_flags_set(flags);
> > +		if (err < 0)
> > +			return err;
> > +	}
> > +
> >  	return 0;
> >  }
> 
OK, this should be fixed now.
> 
> -- 
>  i.
> 

Thank you for reviewing my 2nd patch -- I've also noticed that I've missed
another detail in my first patch, so I'll send both of them right after this
e-mail so as to separate all of this from my answers to your questionings and
remarks.

I might be slightly less reactive for one or two weeks, as I've started my 2nd
year at my IT school; but I won't go away, as the laptop that I'm implementing
support for is also the same laptop that I daily drive, so I'm pretty much
obligated to finish this, soon or later! :]

Alexis
Alexis Belmonte Jan. 4, 2024, 9:56 p.m. UTC | #5
On Wed, Jan 03, 2024 at 11:21:24AM +1100, Prajna Sariputra wrote:
> On Sunday, 31 December 2023 9:46:25 PM AEDT Alexis Belmonte wrote:
> > Add 8BAD to the list of boards which have thermal profile selection
> > available. This allows the CPU to draw more power than the default TDP
> > barrier defined by the 'balanced' thermal profile (around 50W), hence
> > allowing it to perform better without being throttled by the embedded
> > controller (around 130W).
> > 
> > We first need to set the HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET to zero.
> > This prevents the timer countdown from reaching zero, making the embedded
> > controller "force-switch" the system's thermal profile back to 'balanced'
> > automatically.
> > 
> > We also need to put a number of specific flags in
> > HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET when we're switching to another
> > thermal profile:
> > 
> >    - for 'performance', we need to set both HP_OMEN_EC_FLAGS_TURBO and
> >      HP_OMEN_EC_FLAGS_NOTIMER;
> > 
> >    - for 'balanced' and 'powersave', we clear out the register to notify
> >      the system that we want to lower the TDP barrier as soon as possible.
> 
> Do you know if there's a way to check that a given model has this specific timer,
> other than just testing the patch?

I haven't been able to figure out so yet -- there's a 'device_list.json'
file (IIRC) defined somewhere in the Omen Control Center app which I came
across, but no simple way of universally checking if this behavior is
active :[

I think I remember that I've seen another model ID near mine being defined,
so I think I *could* add it directly to both lists though, so that's
that.

> I have an Omen 16-n0000 (8A42), which has a Ryzen 7 6800H and a Radeon
> RX 6650M, and I've been patching the kernel to add it to the omen_thermal_profile_boards
> array for a while now. Just doing that prevents the worst of the throttling from
> happening (GPU dropping from 105W to 35W and the CPU being stuck at like 2GHz
> or less), but currently the GPU still drops to 75W eventually. Switching to
> performance does make it go back to 105W (and even 120W for a bit) before it
> goes back down to 75W, so it makes me wonder if there is actually a timer on my
> model that's doing it rather than just thermal throttling.

I think you've answered it yourself with your other mail ;]

> 
> > 
> > The third flag defined in the hp_thermal_profile_omen_flags enum,
> > HP_OMEN_EC_FLAGS_JUSTSET, is present for completeness.
> > 
> > To prevent potential behaviour breakage with other Omen models, a
> > separate omen_timed_thermal_profile_boards array has been added to list
> > which boards expose this behaviour.
> > 
> > Performance benchmarking was done with the help of silver.urih.com and
> > Google Chrome 120.0.6099.129, on Gnome 45.2, with the 'performance'
> > thermal profile set:
> > 
> > |                  | Performance |     Stress |   TDP |
> > |------------------|-------------|------------|-------|
> > |    with my patch |      P84549 |    S0.1891 |  131W | 
> > | without my patch |      P44084 |    S0.1359 |   47W |
> > 
> > The TDP measurements were done with the help of the s-tui utility,
> > during the load.
> > 
> > There is still work to be done:
> > 
> >    - tune the CPU and GPU fans to better cool down and enhance
> >      performance at the right time; right now, it seems that the fans are
> >      not properly reacting to thermal/performance events, which in turn
> >      either causes thermal throttling OR makes the fans spin way too long,
> >      even though the temperatures have lowered down
> 
> Yeah, that's also a problem with my model, where with a heavy CPU only workload
> the CPU would boost high and almost immediately run into thermal throttling and
> stays throttled for like a few minutes before the fans ramp up even a little,
> which is why I'm not sure that adding my model to the list upstream would be a
> good idea. My CPU doesn't seem to boost all that high though, I don't remember
> the performance mode making much of a difference the last time I tested it.

I totally agree with you -- I just wanted to make sure that my patch was
conform enough with the rest of the codebae before making further progress :]

> Also, for what it's worth I had a conversation with one of the folks who wrote
> the platform profile code (Enver Balalic) a few months ago, and they said the
> profiles are just fan speed control modes on their Omen model.

I've made some reverse engineering on the Omen Control Center app through a
Windows VM, and I came across a few WMI calls in a class (IIRC `HpaClient`, or
something similar to that name) that do reads to this fan curve.

I haven't yet found the parts that do writes unfortunately, that also
needs to be browsed through. :[

The ACPI table that handles those WMI "methods" is the `SSDT` one --
I've disassembled it with `iasl`, which really helped figuring out the
expected data structures.

There's also a post on Reddit which talks about this feature; since this
was posted 2 years ago, I'd say that at least *some* models support
this -- but maybe I'm just misinterpreting it?

https://www.reddit.com/r/HPOmen/comments/poxe2i/new_hp_omen_update_adds_option_for_manual_fan/

> I ended up just testing the patch for myself (after adding my model number to
> the arrays), and it does improve the GPU performance further for me, instead
> of the GPU dropping to 75W after 2-4 minutes it is now able to maintain at least
> 100W even after 10 minutes (tested with Quake 2 RTX). So, it seems like the timer
> thing also applies to my Omen 16-n0000 (8A42). That performance also roughly
> matches up with how notebookcheck.net says my Omen performs in their review
> (103W performance, 72W balanced), so it'd be great if you can also include my
> model in your patch.

Glad to hear that it helps! Your model will be part of both lists next time I
send my updated patches for a "refreshed" review :]

> I just ran more tests with the CPU performance, and it seems that I might have
> misremembered how bad the fan curve was, since I have been limiting the CPU
> frequency to 4GHz instead of letting the CPU do its thing by itself (max boost is
> 4.7GHz), if I go back to the latter then with a heavily multithreaded workload
> (like compiling the kernel) the fans ramp up high within a few seconds of the
> CPU reaching 100C on its hottest core, and the CPU seems to maintain that
> temperature (or the sensors just don't report values above 100C, not sure). That
> seems worrying given that the supposed max operating temperature for the CPU
> (Ryzen 7 6800H) is 95C, but then again that probably won't be the case when gaming,
> which is the main use case for these laptops anyway.

This is definitely problematic yet also what I kind of experience, even
though we both have a completely different combination of CPUs and GPUs
-- at least we can say that the "draft" patches work regardless of the
backed hardware, which is good to hear :]

Thanks for testing out my patch -- as I've said to Ilpo, I won't be able to do
much progress for a few weeks BUT I'm still on it as soon as I'm available
again!

Alexis
Thorsten Leemhuis Aug. 6, 2024, 4:43 p.m. UTC | #6
On 31.12.23 11:46, Alexis Belmonte wrote:
> Add 8BAD to the list of boards which have thermal profile selection
> available. This allows the CPU to draw more power than the default TDP
> barrier defined by the 'balanced' thermal profile (around 50W), hence
> allowing it to perform better without being throttled by the embedded
> controller (around 130W).
>

TWIMC, someone is asking to add the 8C77 to the list, too:
https://bugzilla.kernel.org/show_bug.cgi?id=219125

This is not a regression, but I saw the report and thought I share my
find with people working in this area.

Ciao, Thorsten
Alexis Belmonte Aug. 6, 2024, 5:55 p.m. UTC | #7
Hi Thorsten!

Thanks for the heads up! I've taken a look at the report and proposed a
patch to them that I've not submitted yet to the mailing list.

I'll wait on their feedback and see if it works for them.

Thanks again and have a great day,

Alexis
diff mbox series

Patch

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 95282c3a02ed..79caf5d79e05 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -38,6 +38,8 @@  MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
 #define HPWMI_EVENT_GUID "95F24279-4D7B-4334-9387-ACCDC67EF61C"
 #define HPWMI_BIOS_GUID "5FB7F034-2C63-45e9-BE91-3D44E2C707E4"
 
+#define HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET 0x62
+#define HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET 0x63
 #define HP_OMEN_EC_THERMAL_PROFILE_OFFSET 0x95
 
 #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
@@ -57,7 +59,7 @@  static const char * const omen_thermal_profile_boards[] = {
 	"874A", "8603", "8604", "8748", "886B", "886C", "878A", "878B", "878C",
 	"88C8", "88CB", "8786", "8787", "8788", "88D1", "88D2", "88F4", "88FD",
 	"88F5", "88F6", "88F7", "88FE", "88FF", "8900", "8901", "8902", "8912",
-	"8917", "8918", "8949", "894A", "89EB"
+	"8917", "8918", "8949", "894A", "89EB", "8BAD"
 };
 
 /* DMI Board names of Omen laptops that are specifically set to be thermal
@@ -68,6 +71,14 @@  static const char * const omen_thermal_profile_force_v0_boards[] = {
 	"8607", "8746", "8747", "8749", "874A", "8748"
 };
 
+/* DMI board names of Omen laptops that have a thermal profile timer which will
+ * cause the embedded controller to set the thermal profile back to
+ * "balanced" when reaching zero.
+ */
+static const char * const omen_timed_thermal_profile_boards[] = {
+	"8BAD"
+};
+
 /* DMI Board names of Victus laptops */
 static const char * const victus_thermal_profile_boards[] = {
 	"8A25"
@@ -184,6 +194,12 @@  enum hp_thermal_profile_omen_v1 {
 	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
 };
 
+enum hp_thermal_profile_omen_flags {
+	HP_OMEN_EC_FLAGS_TURBO		= 0x04,
+	HP_OMEN_EC_FLAGS_NOTIMER	= 0x02,
+	HP_OMEN_EC_FLAGS_JUSTSET	= 0x01,
+};
+
 enum hp_thermal_profile_victus {
 	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
 	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE		= 0x01,
@@ -451,7 +467,11 @@  static int hp_wmi_get_tablet_mode(void)
 
 static int omen_thermal_profile_set(int mode)
 {
-	char buffer[2] = {0, mode};
+	/* The Omen Control Center actively sets the first byte of the buffer to
+	 * 255, so let's mimic this behaviour to be as close as possible to
+	 * the original software.
+	 */
+	char buffer[2] = {-1, mode};
 	int ret;
 
 	ret = hp_wmi_perform_query(HPWMI_SET_PERFORMANCE_MODE, HPWMI_GM,
@@ -1203,6 +1221,28 @@  static int platform_profile_omen_get(struct platform_profile_handler *pprof,
 	return 0;
 }
 
+static bool has_omen_thermal_profile_ec_timer(void)
+{
+	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+	if (!board_name)
+		return false;
+
+	return match_string(omen_timed_thermal_profile_boards,
+			    ARRAY_SIZE(omen_timed_thermal_profile_boards),
+			    board_name) >= 0;
+}
+
+inline int omen_thermal_profile_ec_flags_set(enum hp_thermal_profile_omen_flags flags)
+{
+	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_FLAGS_OFFSET, flags);
+}
+
+inline int omen_thermal_profile_ec_timer_set(char value)
+{
+	return ec_write(HP_OMEN_EC_THERMAL_PROFILE_TIMER_OFFSET, value);
+}
+
 static int platform_profile_omen_set(struct platform_profile_handler *pprof,
 				     enum platform_profile_option profile)
 {
@@ -1240,6 +1280,24 @@  static int platform_profile_omen_set(struct platform_profile_handler *pprof,
 	if (err < 0)
 		return err;
 
+	if (has_omen_thermal_profile_ec_timer()) {
+		err = omen_thermal_profile_ec_timer_set(0);
+		if (err < 0)
+			return err;
+
+		enum hp_thermal_profile_omen_flags flags;
+
+		if (profile == PLATFORM_PROFILE_PERFORMANCE)
+			flags = HP_OMEN_EC_FLAGS_NOTIMER |
+				HP_OMEN_EC_FLAGS_TURBO;
+		else
+			flags = 0;
+
+		err = omen_thermal_profile_ec_flags_set(flags);
+		if (err < 0)
+			return err;
+	}
+
 	return 0;
 }