[PATCH v2] platform/x86: hp-wmi: fix platform profile values for Omen 16-wf1xxx

[PATCH v2] platform/x86: hp-wmi: fix platform profile values for Omen 16-wf1xxx

[Krishna Chomal]

HP Omen 16-wf1xxx (board ID 8C78) currently sends the incorrect
Victus-specific thermal profile values via WMI, leading to a logical
inconsistency when switching between platform profiles.

The driver currently uses Victus S values:
0x00 => Balanced / Low-Power
0x01 => Performance

However, Omen Gaming Hub logs / EC register inspection on Windows shows
that this board is intended to use:
0x30 => Balanced / Low-Power
0x31 => Performance

This patch corrects the thermal profile command values to match the
values observed from Omen Gaming Hub logs. The performance benchmarks
and peak power draw (from both CPU and GPU) show no observable change
with this correction (suggesting that the firmware is currently tolerant
of the incorrect values). However sending the correct values prevents
potential regressions after future firmware updates.

Refactor the `victus_s_thermal_profile_boards` array from a simple list
of strings to a `dmi_system_id` table. This allows mapping specific
boards to their corresponding thermal parameter structures via
`driver_data`. This makes addition of future boards cleaner and more
explicit.

Tested on: HP Omen 16-wf1xxx (board 8C78)
Result: Confirmed WMI codes 0x30/0x31 are now sent, resolving the
logical inconsistency and ensuring the value visible in EC registers
match the Windows state for this profile.

Fixes: fb146a38cb11 ("platform/x86: hp-wmi: Add Omen 16-wf1xxx fan support")
Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
---
Changes in v2:
- Refactored `victus_s_thermal_profile_boards` to use `struct dmi_system_id`
- Implemented `driver_data` to handle thermal profile parameters,
  replacing the conditional checks in `platform_profile_victus_s_set_ec`
- Moved enum definitions for thermal profile values earlier in the file
  to support the new `struct thermal_profile_params`.
---
 drivers/platform/x86/hp/hp-wmi.c | 148 ++++++++++++++++++++-----------
 1 file changed, 96 insertions(+), 52 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index f4ea1ea05997..0bcd564ad688 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -53,6 +53,60 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
 
 #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
 
+enum hp_thermal_profile_omen_v0 {
+	HP_OMEN_V0_THERMAL_PROFILE_DEFAULT		= 0x00,
+	HP_OMEN_V0_THERMAL_PROFILE_PERFORMANCE		= 0x01,
+	HP_OMEN_V0_THERMAL_PROFILE_COOL			= 0x02,
+};
+
+enum hp_thermal_profile_omen_v1 {
+	HP_OMEN_V1_THERMAL_PROFILE_DEFAULT		= 0x30,
+	HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE		= 0x31,
+	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,
+	HP_VICTUS_THERMAL_PROFILE_QUIET			= 0x03,
+};
+
+enum hp_thermal_profile_victus_s {
+	HP_VICTUS_S_THERMAL_PROFILE_DEFAULT		= 0x00,
+	HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE		= 0x01,
+};
+
+enum hp_thermal_profile {
+	HP_THERMAL_PROFILE_PERFORMANCE			= 0x00,
+	HP_THERMAL_PROFILE_DEFAULT			= 0x01,
+	HP_THERMAL_PROFILE_COOL				= 0x02,
+	HP_THERMAL_PROFILE_QUIET			= 0x03,
+};
+
+struct thermal_profile_params {
+	u8 performance;
+	u8 balanced;
+	u8 eco;
+};
+
+static const struct thermal_profile_params victus_s_thermal_params = {
+	.performance	= HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE,
+	.balanced	= HP_VICTUS_S_THERMAL_PROFILE_DEFAULT,
+	.eco		= HP_VICTUS_S_THERMAL_PROFILE_DEFAULT
+};
+
+static const struct thermal_profile_params omen_v1_thermal_params = {
+	.performance	= HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE,
+	.balanced	= HP_OMEN_V1_THERMAL_PROFILE_DEFAULT,
+	.eco		= HP_OMEN_V1_THERMAL_PROFILE_DEFAULT
+};
+
 /* DMI board names of devices that should use the omen specific path for
  * thermal profiles.
  * This was obtained by taking a look in the windows omen command center
@@ -99,10 +153,36 @@ static const char * const victus_thermal_profile_boards[] = {
 };
 
 /* DMI Board names of Victus 16-r and Victus 16-s laptops */
-static const char * const victus_s_thermal_profile_boards[] = {
-	"8BBE", "8BD4", "8BD5",
-	"8C78", "8C99", "8C9C",
-	"8D41",
+static const struct dmi_system_id victus_s_thermal_profile_boards[] = {
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BBE") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD4") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD5") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C78") },
+		.driver_data = (void *)&omen_v1_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C99") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C9C") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D41") },
+		.driver_data = (void *)&victus_s_thermal_params,
+	},
+	{},
 };
 
 enum hp_wmi_radio {
@@ -225,42 +305,6 @@ enum hp_wireless2_bits {
 	HPWMI_POWER_FW_OR_HW	= HPWMI_POWER_BIOS | HPWMI_POWER_HARD,
 };
 
-enum hp_thermal_profile_omen_v0 {
-	HP_OMEN_V0_THERMAL_PROFILE_DEFAULT     = 0x00,
-	HP_OMEN_V0_THERMAL_PROFILE_PERFORMANCE = 0x01,
-	HP_OMEN_V0_THERMAL_PROFILE_COOL        = 0x02,
-};
-
-enum hp_thermal_profile_omen_v1 {
-	HP_OMEN_V1_THERMAL_PROFILE_DEFAULT	= 0x30,
-	HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE	= 0x31,
-	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,
-	HP_VICTUS_THERMAL_PROFILE_QUIET			= 0x03,
-};
-
-enum hp_thermal_profile_victus_s {
-	HP_VICTUS_S_THERMAL_PROFILE_DEFAULT		= 0x00,
-	HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE		= 0x01,
-};
-
-enum hp_thermal_profile {
-	HP_THERMAL_PROFILE_PERFORMANCE	= 0x00,
-	HP_THERMAL_PROFILE_DEFAULT		= 0x01,
-	HP_THERMAL_PROFILE_COOL			= 0x02,
-	HP_THERMAL_PROFILE_QUIET		= 0x03,
-};
-
 #define IS_HWBLOCKED(x) ((x & HPWMI_POWER_FW_OR_HW) != HPWMI_POWER_FW_OR_HW)
 #define IS_SWBLOCKED(x) !(x & HPWMI_POWER_SOFT)
 
@@ -1581,15 +1625,7 @@ static int platform_profile_victus_set_ec(enum platform_profile_option profile)
 
 static bool is_victus_s_thermal_profile(void)
 {
-	const char *board_name;
-
-	board_name = dmi_get_system_info(DMI_BOARD_NAME);
-	if (!board_name)
-		return false;
-
-	return match_string(victus_s_thermal_profile_boards,
-			    ARRAY_SIZE(victus_s_thermal_profile_boards),
-			    board_name) >= 0;
+	return dmi_first_match(victus_s_thermal_profile_boards) != NULL;
 }
 
 static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
@@ -1672,25 +1708,33 @@ static int victus_s_set_cpu_pl1_pl2(u8 pl1, u8 pl2)
 
 static int platform_profile_victus_s_set_ec(enum platform_profile_option profile)
 {
+	const struct dmi_system_id *id;
+	const struct thermal_profile_params *params;
 	bool gpu_ctgp_enable, gpu_ppab_enable;
 	u8 gpu_dstate; /* Test shows 1 = 100%, 2 = 50%, 3 = 25%, 4 = 12.5% */
 	int err, tp;
 
+	id = dmi_first_match(victus_s_thermal_profile_boards);
+	if (!id)
+		return -ENODEV;
+
+	params = id->driver_data;
+
 	switch (profile) {
 	case PLATFORM_PROFILE_PERFORMANCE:
-		tp = HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE;
+		tp = params->performance;
 		gpu_ctgp_enable = true;
 		gpu_ppab_enable = true;
 		gpu_dstate = 1;
 		break;
 	case PLATFORM_PROFILE_BALANCED:
-		tp = HP_VICTUS_S_THERMAL_PROFILE_DEFAULT;
+		tp = params->balanced;
 		gpu_ctgp_enable = false;
 		gpu_ppab_enable = true;
 		gpu_dstate = 1;
 		break;
 	case PLATFORM_PROFILE_LOW_POWER:
-		tp = HP_VICTUS_S_THERMAL_PROFILE_DEFAULT;
+		tp = params->eco;
 		gpu_ctgp_enable = false;
 		gpu_ppab_enable = false;
 		gpu_dstate = 1;
-- 
2.52.0

Re: [PATCH v2] platform/x86: hp-wmi: fix platform profile values for Omen 16-wf1xxx

[Ilpo Järvinen]

On Tue, 16 Dec 2025, Krishna Chomal wrote:

> HP Omen 16-wf1xxx (board ID 8C78) currently sends the incorrect
> Victus-specific thermal profile values via WMI, leading to a logical
> inconsistency when switching between platform profiles.
>
> The driver currently uses Victus S values:
> 0x00 => Balanced / Low-Power
> 0x01 => Performance
> 
> However, Omen Gaming Hub logs / EC register inspection on Windows shows
> that this board is intended to use:
> 0x30 => Balanced / Low-Power
> 0x31 => Performance
> 
> This patch corrects the thermal profile command values to match the
> values observed from Omen Gaming Hub logs. The performance benchmarks
> and peak power draw (from both CPU and GPU) show no observable change
> with this correction (suggesting that the firmware is currently tolerant
> of the incorrect values). However sending the correct values prevents
> potential regressions after future firmware updates.
> 
> Refactor the `victus_s_thermal_profile_boards` array from a simple list
> of strings to a `dmi_system_id` table. This allows mapping specific
> boards to their corresponding thermal parameter structures via
> `driver_data`. This makes addition of future boards cleaner and more
> explicit.
> 
> Tested on: HP Omen 16-wf1xxx (board 8C78)

Remove : and add .

> Result: Confirmed WMI codes 0x30/0x31 are now sent, resolving the

Remove "Result: Confirmed" and just continue like a normal paragraph.

> logical inconsistency and ensuring the value visible in EC registers
> match the Windows state for this profile.
> 
> Fixes: fb146a38cb11 ("platform/x86: hp-wmi: Add Omen 16-wf1xxx fan support")
> Signed-off-by: Krishna Chomal <krishna.chomal108@gmail.com>
> ---
> Changes in v2:
> - Refactored `victus_s_thermal_profile_boards` to use `struct dmi_system_id`
> - Implemented `driver_data` to handle thermal profile parameters,
>   replacing the conditional checks in `platform_profile_victus_s_set_ec`
> - Moved enum definitions for thermal profile values earlier in the file
>   to support the new `struct thermal_profile_params`.
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 148 ++++++++++++++++++++-----------
>  1 file changed, 96 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index f4ea1ea05997..0bcd564ad688 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -53,6 +53,60 @@ MODULE_ALIAS("wmi:5FB7F034-2C63-45E9-BE91-3D44E2C707E4");
>  
>  #define zero_if_sup(tmp) (zero_insize_support?0:sizeof(tmp)) // use when zero insize is required
>  
> +enum hp_thermal_profile_omen_v0 {
> +	HP_OMEN_V0_THERMAL_PROFILE_DEFAULT		= 0x00,
> +	HP_OMEN_V0_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> +	HP_OMEN_V0_THERMAL_PROFILE_COOL			= 0x02,
> +};
> +
> +enum hp_thermal_profile_omen_v1 {
> +	HP_OMEN_V1_THERMAL_PROFILE_DEFAULT		= 0x30,
> +	HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE		= 0x31,
> +	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,
> +	HP_VICTUS_THERMAL_PROFILE_QUIET			= 0x03,
> +};
> +
> +enum hp_thermal_profile_victus_s {
> +	HP_VICTUS_S_THERMAL_PROFILE_DEFAULT		= 0x00,
> +	HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> +};
> +
> +enum hp_thermal_profile {
> +	HP_THERMAL_PROFILE_PERFORMANCE			= 0x00,
> +	HP_THERMAL_PROFILE_DEFAULT			= 0x01,
> +	HP_THERMAL_PROFILE_COOL				= 0x02,
> +	HP_THERMAL_PROFILE_QUIET			= 0x03,
> +};
> +
> +struct thermal_profile_params {
> +	u8 performance;
> +	u8 balanced;
> +	u8 eco;
> +};

How do you envision this being used on the get side? You choose not to 
change the get side, so you didn't run to that challenge. Currently 
at least platform_profile_omen_get_ec() is sort of cheating as it maps 
multiple variants without properly differentiation them.

Is "eco" hp specific terminology? (platform_profile_option doesn't use 
that terminology at all.)

Wouldn't we want to include struct platform_profile_ops * somewhere as 
well so this framework can extend beyond just victus_s? (I'm not sure if 
it should be in struct thermal_profile_params or if should be separate 
from that, this whole platform profile code is so hard to read it's hard 
to infer what are all the variations.)

> +static const struct thermal_profile_params victus_s_thermal_params = {
> +	.performance	= HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE,
> +	.balanced	= HP_VICTUS_S_THERMAL_PROFILE_DEFAULT,
> +	.eco		= HP_VICTUS_S_THERMAL_PROFILE_DEFAULT

Please always add trailing comma to any non-terminating entry.

> +};
> +
> +static const struct thermal_profile_params omen_v1_thermal_params = {
> +	.performance	= HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE,
> +	.balanced	= HP_OMEN_V1_THERMAL_PROFILE_DEFAULT,
> +	.eco		= HP_OMEN_V1_THERMAL_PROFILE_DEFAULT
> +};
> +
>  /* DMI board names of devices that should use the omen specific path for
>   * thermal profiles.
>   * This was obtained by taking a look in the windows omen command center
> @@ -99,10 +153,36 @@ static const char * const victus_thermal_profile_boards[] = {
>  };
>  
>  /* DMI Board names of Victus 16-r and Victus 16-s laptops */
> -static const char * const victus_s_thermal_profile_boards[] = {
> -	"8BBE", "8BD4", "8BD5",
> -	"8C78", "8C99", "8C9C",
> -	"8D41",
> +static const struct dmi_system_id victus_s_thermal_profile_boards[] = {
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BBE") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD4") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD5") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C78") },
> +		.driver_data = (void *)&omen_v1_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C99") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C9C") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{
> +		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D41") },
> +		.driver_data = (void *)&victus_s_thermal_params,
> +	},
> +	{},
>  };
>  
>  enum hp_wmi_radio {
> @@ -225,42 +305,6 @@ enum hp_wireless2_bits {
>  	HPWMI_POWER_FW_OR_HW	= HPWMI_POWER_BIOS | HPWMI_POWER_HARD,
>  };
>  
> -enum hp_thermal_profile_omen_v0 {
> -	HP_OMEN_V0_THERMAL_PROFILE_DEFAULT     = 0x00,
> -	HP_OMEN_V0_THERMAL_PROFILE_PERFORMANCE = 0x01,
> -	HP_OMEN_V0_THERMAL_PROFILE_COOL        = 0x02,
> -};
> -
> -enum hp_thermal_profile_omen_v1 {
> -	HP_OMEN_V1_THERMAL_PROFILE_DEFAULT	= 0x30,
> -	HP_OMEN_V1_THERMAL_PROFILE_PERFORMANCE	= 0x31,
> -	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,
> -	HP_VICTUS_THERMAL_PROFILE_QUIET			= 0x03,
> -};
> -
> -enum hp_thermal_profile_victus_s {
> -	HP_VICTUS_S_THERMAL_PROFILE_DEFAULT		= 0x00,
> -	HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE		= 0x01,
> -};
> -
> -enum hp_thermal_profile {
> -	HP_THERMAL_PROFILE_PERFORMANCE	= 0x00,
> -	HP_THERMAL_PROFILE_DEFAULT		= 0x01,
> -	HP_THERMAL_PROFILE_COOL			= 0x02,
> -	HP_THERMAL_PROFILE_QUIET		= 0x03,
> -};
> -
>  #define IS_HWBLOCKED(x) ((x & HPWMI_POWER_FW_OR_HW) != HPWMI_POWER_FW_OR_HW)
>  #define IS_SWBLOCKED(x) !(x & HPWMI_POWER_SOFT)
>  
> @@ -1581,15 +1625,7 @@ static int platform_profile_victus_set_ec(enum platform_profile_option profile)
>  
>  static bool is_victus_s_thermal_profile(void)
>  {
> -	const char *board_name;
> -
> -	board_name = dmi_get_system_info(DMI_BOARD_NAME);
> -	if (!board_name)
> -		return false;
> -
> -	return match_string(victus_s_thermal_profile_boards,
> -			    ARRAY_SIZE(victus_s_thermal_profile_boards),
> -			    board_name) >= 0;
> +	return dmi_first_match(victus_s_thermal_profile_boards) != NULL;
>  }
>  
>  static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
> @@ -1672,25 +1708,33 @@ static int victus_s_set_cpu_pl1_pl2(u8 pl1, u8 pl2)
>  
>  static int platform_profile_victus_s_set_ec(enum platform_profile_option profile)
>  {
> +	const struct dmi_system_id *id;
> +	const struct thermal_profile_params *params;
>  	bool gpu_ctgp_enable, gpu_ppab_enable;
>  	u8 gpu_dstate; /* Test shows 1 = 100%, 2 = 50%, 3 = 25%, 4 = 12.5% */
>  	int err, tp;
>  
> +	id = dmi_first_match(victus_s_thermal_profile_boards);
> +	if (!id)
> +		return -ENODEV;
> +
> +	params = id->driver_data;

We should do this once at init time. Then you can mark the dmi table(s) 
with __init.

Normally the pointer would be stored into a priv struct (which this driver 
doesn't have yet but gazillions of static vars).

>  	switch (profile) {
>  	case PLATFORM_PROFILE_PERFORMANCE:
> -		tp = HP_VICTUS_S_THERMAL_PROFILE_PERFORMANCE;
> +		tp = params->performance;
>  		gpu_ctgp_enable = true;
>  		gpu_ppab_enable = true;
>  		gpu_dstate = 1;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED:
> -		tp = HP_VICTUS_S_THERMAL_PROFILE_DEFAULT;
> +		tp = params->balanced;
>  		gpu_ctgp_enable = false;
>  		gpu_ppab_enable = true;
>  		gpu_dstate = 1;
>  		break;
>  	case PLATFORM_PROFILE_LOW_POWER:
> -		tp = HP_VICTUS_S_THERMAL_PROFILE_DEFAULT;
> +		tp = params->eco;
>  		gpu_ctgp_enable = false;
>  		gpu_ppab_enable = false;
>  		gpu_dstate = 1;

-- 
 i.

Re: [PATCH v2] platform/x86: hp-wmi: fix platform profile values for Omen 16-wf1xxx

[Krishna Chomal]

On Tue, Dec 16, 2025 at 03:07:37PM +0200, Ilpo Järvinen wrote:
>> +enum hp_thermal_profile {
>> +	HP_THERMAL_PROFILE_PERFORMANCE			= 0x00,
>> +	HP_THERMAL_PROFILE_DEFAULT			= 0x01,
>> +	HP_THERMAL_PROFILE_COOL				= 0x02,
>> +	HP_THERMAL_PROFILE_QUIET			= 0x03,
>> +};
>> +
>> +struct thermal_profile_params {
>> +	u8 performance;
>> +	u8 balanced;
>> +	u8 eco;
>> +};
>
>How do you envision this being used on the get side? You choose not to
>change the get side, so you didn't run to that challenge. Currently
>at least platform_profile_omen_get_ec() is sort of cheating as it maps
>multiple variants without properly differentiation them.

The get side for victus_s devices as defined in platform_profile_victus_s_ops
is platform_profile_omen_get(). That function simply returns the value
stored in the global static var: active_platform_profile. As far as
platform_profile_omen_get_ec() is concerned, it being called:
1. In omen_powersource_event() and
2. In thermal_profile_setup()

However both of these are irrelevant for victus_s devices, as it is
guarded by a if(is_omen_thermal_profile()) block. So basically victus_s
devices never actually reach that function. I believe that is because it
internally calls omen_thermal_profile_get(), which itself uses ec_read
to read the current thermal profile. This functionality was added in
commit 4c51ba9af42df, back in the year 2021. Since then the EC layout,
and some WMI calls have been changed in HP laptops. In fact that is the
reason victus_s list exists in the first place.

After some basic EC-dumping from Windows, I know that the EC offset for
these newer boards should be 0x59. If you want, I can add this new EC
mapping too.

So basically for victus_s boards, at least as of now, there is no
get side to read the actual platform profile, hence no challenges faced
yet.

>Is "eco" hp specific terminology? (platform_profile_option doesn't use
>that terminology at all.)

Yes "eco" is hp-specific terminology, but you are right, I will change
it to "low_power" in V3 for consistency.

>Wouldn't we want to include struct platform_profile_ops * somewhere as
>well so this framework can extend beyond just victus_s? (I'm not sure if
>it should be in struct thermal_profile_params or if should be separate
>from that, this whole platform profile code is so hard to read it's hard
>to infer what are all the variations.)

That seems like a valid direction but given the current state of the
driver (with its many static vars), I would prefer to keep this patch
focused on fixing victus_s thermal profile values. I can look into a
more broader refactor later.

>>
>>  static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
>> @@ -1672,25 +1708,33 @@ static int victus_s_set_cpu_pl1_pl2(u8 pl1, u8 pl2)
>>
>>  static int platform_profile_victus_s_set_ec(enum platform_profile_option profile)
>>  {
>> +	const struct dmi_system_id *id;
>> +	const struct thermal_profile_params *params;
>>  	bool gpu_ctgp_enable, gpu_ppab_enable;
>>  	u8 gpu_dstate; /* Test shows 1 = 100%, 2 = 50%, 3 = 25%, 4 = 12.5% */
>>  	int err, tp;
>>
>> +	id = dmi_first_match(victus_s_thermal_profile_boards);
>> +	if (!id)
>> +		return -ENODEV;
>> +
>> +	params = id->driver_data;
>
>We should do this once at init time. Then you can mark the dmi table(s)
>with __init.
>
>Normally the pointer would be stored into a priv struct (which this driver
>doesn't have yet but gazillions of static vars).

Yes I agree querying the dmi table for every profile change is very
suboptimal. I will add that in V3 with yet another static var :)

Re: [PATCH v2] platform/x86: hp-wmi: fix platform profile values for Omen 16-wf1xxx

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4606 bytes --]

On Wed, 17 Dec 2025, Krishna Chomal wrote:

> On Tue, Dec 16, 2025 at 03:07:37PM +0200, Ilpo Järvinen wrote:
> > > +enum hp_thermal_profile {
> > > +	HP_THERMAL_PROFILE_PERFORMANCE			= 0x00,
> > > +	HP_THERMAL_PROFILE_DEFAULT			= 0x01,
> > > +	HP_THERMAL_PROFILE_COOL				= 0x02,
> > > +	HP_THERMAL_PROFILE_QUIET			= 0x03,
> > > +};
> > > +
> > > +struct thermal_profile_params {
> > > +	u8 performance;
> > > +	u8 balanced;
> > > +	u8 eco;
> > > +};
> > 
> > How do you envision this being used on the get side? You choose not to
> > change the get side, so you didn't run to that challenge. Currently
> > at least platform_profile_omen_get_ec() is sort of cheating as it maps
> > multiple variants without properly differentiation them.
> 
> The get side for victus_s devices as defined in platform_profile_victus_s_ops
> is platform_profile_omen_get(). That function simply returns the value
> stored in the global static var: active_platform_profile. As far as
> platform_profile_omen_get_ec() is concerned, it being called:
> 1. In omen_powersource_event() and
> 2. In thermal_profile_setup()
> 
> However both of these are irrelevant for victus_s devices, as it is
> guarded by a if(is_omen_thermal_profile()) block.

Thanks, I assumed all _ec variants are called from the similarly named 
getter/setter but apparently there are exceptions I didn't notice until 
now.

> So basically victus_s
> devices never actually reach that function. I believe that is because it
> internally calls omen_thermal_profile_get(), which itself uses ec_read
> to read the current thermal profile. This functionality was added in
> commit 4c51ba9af42df, back in the year 2021. Since then the EC layout,
> and some WMI calls have been changed in HP laptops. In fact that is the
> reason victus_s list exists in the first place.
> 
> After some basic EC-dumping from Windows, I know that the EC offset for
> these newer boards should be 0x59. If you want, I can add this new EC
> mapping too.

No objection, just put the new stuff into own patch.

> So basically for victus_s boards, at least as of now, there is no
> get side to read the actual platform profile, hence no challenges faced
> yet.

The get side should still be thought at this point as it affects what the 
struct layuout should be. The only way to do the reverse mapping will the 
current struct layout is using one if () per a struct member (e.g. loop is 
going to be impractical). Perhaps some union+array trickery could allow 
looping through them while retaining the current way to name the fields.

> > Is "eco" hp specific terminology? (platform_profile_option doesn't use
> > that terminology at all.)
> 
> Yes "eco" is hp-specific terminology, but you are right, I will change
> it to "low_power" in V3 for consistency.
> 
> > Wouldn't we want to include struct platform_profile_ops * somewhere as
> > well so this framework can extend beyond just victus_s? (I'm not sure if
> > it should be in struct thermal_profile_params or if should be separate
> > from that, this whole platform profile code is so hard to read it's hard
> > to infer what are all the variations.)
> 
> That seems like a valid direction but given the current state of the
> driver (with its many static vars), I would prefer to keep this patch
> focused on fixing victus_s thermal profile values. I can look into a
> more broader refactor later.

Fine. Doing it in smaller steps is easier to review anyway.

> > >  static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
> > > @@ -1672,25 +1708,33 @@ static int victus_s_set_cpu_pl1_pl2(u8 pl1, u8
> > > pl2)
> > > 
> > >  static int platform_profile_victus_s_set_ec(enum platform_profile_option
> > > profile)
> > >  {
> > > +	const struct dmi_system_id *id;
> > > +	const struct thermal_profile_params *params;
> > >  	bool gpu_ctgp_enable, gpu_ppab_enable;
> > >  	u8 gpu_dstate; /* Test shows 1 = 100%, 2 = 50%, 3 = 25%, 4 = 12.5% */
> > >  	int err, tp;
> > > 
> > > +	id = dmi_first_match(victus_s_thermal_profile_boards);
> > > +	if (!id)
> > > +		return -ENODEV;
> > > +
> > > +	params = id->driver_data;
> > 
> > We should do this once at init time. Then you can mark the dmi table(s)
> > with __init.
> > 
> > Normally the pointer would be stored into a priv struct (which this driver
> > doesn't have yet but gazillions of static vars).
> 
> Yes I agree querying the dmi table for every profile change is very
> suboptimal. I will add that in V3 with yet another static var :)

Thanks.

-- 
 i.