[PATCH v6 0/4] platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control

[PATCH v6 0/4] platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control

[Radhey Kalra]

Hi,

This is a split version of the Victus 15-fb0xxx fan-control change.
Patches 1 and 2 are no-functional-changes-intended refactors which move
the existing thermal-profile and fan-control decisions into
board-specific .driver_data. Patch 3 fixes the fan-table GPU delta
parsing issue found while testing this board. Patch 4 adds fan-control
support for Victus 15-fb0xxx board 8A3D.

v6 splits the gpu_delta fan-table parsing fix back into its own patch.
The previous 2-patch v5 folded that behavioral parser fix into the board
data preparation patch, which was a mistake.

8A3D is kept fan-control-only because the verified behavior is the
Victus fan-control path. The existing generic platform-profile sysfs path
on 8A3D accepts writes/readbacks without hp_wmi errors, but CPU/GPU
power-policy behavior was not meaningfully validated.

Adding 8A3D to the Victus S thermal-profile table would move it to a
different, Victus S-specific platform-profile path that has not been
verified on this board.

Based on platform-drivers-x86/for-next commit:
165e81354eefd5551358112773f24027aac59d5a

Tested on a Victus by HP Gaming Laptop 15-fb0xxx, board 8A3D, BIOS
F.22. With the series applied, pwm1 and pwm1_enable are exposed and
the EC-layout warning is not emitted for 8A3D.

Radhey Kalra (4):
  platform/x86: hp-wmi: Introduce board-specific feature data
  platform/x86: hp-wmi: Drive fan control from board data
  platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta
  platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control

 drivers/platform/x86/hp/hp-wmi.c | 212 ++++++++++++++++++++++---------
 1 file changed, 153 insertions(+), 59 deletions(-)

---
Changes in v6:
- Split the gpu_delta fan-table parser fix back out into its own patch.
- Keep 8A3D fan-control-only and document why the Victus S
  platform-profile path is not enabled for this board.

Changes in v5:
- Drop the active_thermal_profile_params global and dereference the
  thermal-profile data through active_board_params.
- Move the generic fan-only board handling into patch 1, keeping the
  final 8A3D patch focused on adding the board data and DMI match.
- Wrap the setup comment added in patch 1 to stay under 80 columns.

Changes in v4:
- Split v3 into two no-functional-changes-intended refactors, one
  fan-table parsing fix, and one 8A3D support patch.
- Rebase on platform-drivers-x86/for-next commit 165e81354eefd555.
- Move revision notes to the cover letter.

Changes in v3:
- Rebase on platform-drivers-x86/for-next commit 165e81354eefd555.
- Keep the v2 board-data approach and 8A3D fan-control-only handling.

Changes in v2:
- Use real name in Signed-off-by.
- Replace the ad-hoc fan-control DMI table with board data in
  .driver_data.
- Keep 8A3D fan-control-only instead of enabling Victus S thermal
  profiles.
- Use the fixed fan-table parser path and derive gpu_delta from the
  first non-zero GPU row.

-- 
2.54.0

[PATCH v6 1/4] platform/x86: hp-wmi: Introduce board-specific feature data

[Radhey Kalra]

The victus_s_thermal_profile_boards DMI table is about to carry more
than thermal-profile data. Replace the direct thermal_profile_params
.driver_data pointers with hp_wmi_board_params and rename the
table/setup helper accordingly.

No functional changes intended.

Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 117 ++++++++++++++++++++-----------
 1 file changed, 75 insertions(+), 42 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index d1cc6e7..280861b 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -133,11 +133,35 @@ static const struct thermal_profile_params omen_v1_no_ec_thermal_params = {
 	.ec_tp_offset	= HP_NO_THERMAL_PROFILE_OFFSET,
 };
 
-/*
- * A generic pointer for the currently-active board's thermal profile
- * parameters.
- */
-static struct thermal_profile_params *active_thermal_profile_params;
+struct hp_wmi_board_params {
+	const struct thermal_profile_params *thermal_profile;
+};
+
+static const struct hp_wmi_board_params victus_s_board_params = {
+	.thermal_profile	= &victus_s_thermal_params,
+};
+
+static const struct hp_wmi_board_params omen_v1_board_params = {
+	.thermal_profile	= &omen_v1_thermal_params,
+};
+
+static const struct hp_wmi_board_params omen_v1_legacy_board_params = {
+	.thermal_profile	= &omen_v1_legacy_thermal_params,
+};
+
+static const struct hp_wmi_board_params omen_v1_no_ec_board_params = {
+	.thermal_profile	= &omen_v1_no_ec_thermal_params,
+};
+
+static const struct hp_wmi_board_params *active_board_params;
+
+static const struct thermal_profile_params *hp_wmi_thermal_profile(void)
+{
+	if (!active_board_params)
+		return NULL;
+
+	return active_board_params->thermal_profile;
+}
 
 /* DMI board names of devices that should use the omen specific path for
  * thermal profiles.
@@ -187,67 +211,67 @@ static const char * const victus_thermal_profile_boards[] = {
 	"8A25",
 };
 
-/* DMI Board names of Victus 16-r and Victus 16-s laptops */
-static const struct dmi_system_id victus_s_thermal_profile_boards[] __initconst = {
+/* DMI board-specific feature data for Omen and Victus laptops. */
+static const struct dmi_system_id hp_wmi_feature_boards[] __initconst = {
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A44") },
-		.driver_data = (void *)&omen_v1_legacy_thermal_params,
+		.driver_data = (void *)&omen_v1_legacy_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A4D") },
-		.driver_data = (void *)&omen_v1_legacy_thermal_params,
+		.driver_data = (void *)&omen_v1_legacy_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BAB") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BBE") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BCA") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BCD") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD4") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD5") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C76") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C77") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C78") },
-		.driver_data = (void *)&omen_v1_thermal_params,
+		.driver_data = (void *)&omen_v1_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C99") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C9C") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D41") },
-		.driver_data = (void *)&victus_s_thermal_params,
+		.driver_data = (void *)&victus_s_board_params,
 	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D87") },
-		.driver_data = (void *)&omen_v1_no_ec_thermal_params,
+		.driver_data = (void *)&omen_v1_no_ec_board_params,
 	},
 	{},
 };
@@ -1854,7 +1878,10 @@ static int platform_profile_victus_s_get_ec(enum platform_profile_option *profil
 	u8 current_dstate, current_gpu_slowdown_temp, tp;
 	const struct thermal_profile_params *params;
 
-	params = active_thermal_profile_params;
+	params = hp_wmi_thermal_profile();
+	if (!params)
+		return -ENODEV;
+
 	if (params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN ||
 	    params->ec_tp_offset == HP_NO_THERMAL_PROFILE_OFFSET) {
 		*profile = active_platform_profile;
@@ -1866,10 +1893,10 @@ static int platform_profile_victus_s_get_ec(enum platform_profile_option *profil
 		return ret;
 
 	/*
-	 * We cannot use active_thermal_profile_params here, because boards
-	 * like 8C78 have tp == 0x0 || tp == 0x1 after cold boot, but logically
-	 * it should have tp == 0x30 || tp == 0x31, as corrected by the Omen
-	 * Gaming Hub on windows. Hence accept both of these values.
+	 * Boards like 8C78 have tp == 0x0 || tp == 0x1 after cold boot,
+	 * but logically it should have tp == 0x30 || tp == 0x31, as
+	 * corrected by the Omen Gaming Hub on windows. Hence accept both
+	 * of these values.
 	 */
 	if (tp == victus_s_thermal_params.performance ||
 	    tp == omen_v1_thermal_params.performance) {
@@ -1904,12 +1931,12 @@ static int platform_profile_victus_s_get_ec(enum platform_profile_option *profil
 
 static int platform_profile_victus_s_set_ec(enum platform_profile_option profile)
 {
-	struct thermal_profile_params *params;
+	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;
 
-	params = active_thermal_profile_params;
+	params = hp_wmi_thermal_profile();
 	if (!params)
 		return -ENODEV;
 
@@ -2175,6 +2202,7 @@ static const struct platform_profile_ops hp_wmi_platform_profile_ops = {
 static int thermal_profile_setup(struct platform_device *device)
 {
 	const struct platform_profile_ops *ops;
+	const struct thermal_profile_params *params;
 	int err, tp;
 
 	if (is_omen_thermal_profile()) {
@@ -2206,13 +2234,17 @@ static int thermal_profile_setup(struct platform_device *device)
 
 		ops = &platform_profile_victus_ops;
 	} else if (is_victus_s_thermal_profile()) {
+		params = hp_wmi_thermal_profile();
+		if (!params)
+			return -ENODEV;
+
 		/*
 		 * For an unknown EC layout board, platform_profile_victus_s_get_ec(),
 		 * behaves like a wrapper around active_platform_profile, to avoid using
 		 * uninitialized data, we default to PLATFORM_PROFILE_BALANCED.
 		 */
-		if (active_thermal_profile_params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN ||
-		    active_thermal_profile_params->ec_tp_offset == HP_NO_THERMAL_PROFILE_OFFSET) {
+		if (params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN ||
+		    params->ec_tp_offset == HP_NO_THERMAL_PROFILE_OFFSET) {
 			active_platform_profile = PLATFORM_PROFILE_BALANCED;
 		} else {
 			err = platform_profile_victus_s_get_ec(&active_platform_profile);
@@ -2673,24 +2705,25 @@ static int hp_wmi_hwmon_init(void)
 	return 0;
 }
 
-static void __init setup_active_thermal_profile_params(void)
+static void __init setup_active_board_params(void)
 {
 	const struct dmi_system_id *id;
+	const struct thermal_profile_params *params;
 
-	/*
-	 * Currently only victus_s devices use the
-	 * active_thermal_profile_params
-	 */
-	id = dmi_first_match(victus_s_thermal_profile_boards);
+	id = dmi_first_match(hp_wmi_feature_boards);
 	if (id) {
+		active_board_params = id->driver_data;
+		params = hp_wmi_thermal_profile();
+		if (!params)
+			return;
+
 		/*
 		 * Marking this boolean is required to ensure that
 		 * is_victus_s_thermal_profile() behaves like a valid
 		 * wrapper.
 		 */
 		is_victus_s_board = true;
-		active_thermal_profile_params = id->driver_data;
-		if (active_thermal_profile_params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN) {
+		if (params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN) {
 			pr_warn("Unknown EC layout for board %s. Thermal profile readback will be disabled. Please report this to platform-driver-x86@vger.kernel.org\n",
 				dmi_get_system_info(DMI_BOARD_NAME));
 		}
@@ -2725,10 +2758,10 @@ static int __init hp_wmi_init(void)
 		}
 
 		/*
-		 * Setup active board's thermal profile parameters before
-		 * starting platform driver probe.
+		 * Setup active board feature data before starting platform
+		 * driver probe.
 		 */
-		setup_active_thermal_profile_params();
+		setup_active_board_params();
 		err = platform_driver_probe(&hp_wmi_driver, hp_wmi_bios_setup);
 		if (err)
 			goto err_unregister_device;
-- 
2.54.0

[PATCH v6 2/4] platform/x86: hp-wmi: Drive fan control from board data

[Radhey Kalra]

Use the board-specific .driver_data to describe fan-control support and
fan-speed read callbacks. Existing boards keep the same Victus
fan-control path, but the hwmon code no longer hardcodes that decision
through is_victus_s_thermal_profile().

No functional changes intended.

Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 79 ++++++++++++++++++++++++++------
 1 file changed, 64 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 280861b..ae2abb9 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -133,24 +133,41 @@ static const struct thermal_profile_params omen_v1_no_ec_thermal_params = {
 	.ec_tp_offset	= HP_NO_THERMAL_PROFILE_OFFSET,
 };
 
+struct hp_wmi_fan_profile_params {
+	int (*get_fan_speed)(int fan);
+	bool fan_table;
+};
+
 struct hp_wmi_board_params {
 	const struct thermal_profile_params *thermal_profile;
+	const struct hp_wmi_fan_profile_params *fan_profile;
+};
+
+static int hp_wmi_get_fan_speed_victus_s(int fan);
+
+static const struct hp_wmi_fan_profile_params victus_s_fan_profile_params = {
+	.get_fan_speed	= hp_wmi_get_fan_speed_victus_s,
+	.fan_table	= true,
 };
 
 static const struct hp_wmi_board_params victus_s_board_params = {
 	.thermal_profile	= &victus_s_thermal_params,
+	.fan_profile		= &victus_s_fan_profile_params,
 };
 
 static const struct hp_wmi_board_params omen_v1_board_params = {
 	.thermal_profile	= &omen_v1_thermal_params,
+	.fan_profile		= &victus_s_fan_profile_params,
 };
 
 static const struct hp_wmi_board_params omen_v1_legacy_board_params = {
 	.thermal_profile	= &omen_v1_legacy_thermal_params,
+	.fan_profile		= &victus_s_fan_profile_params,
 };
 
 static const struct hp_wmi_board_params omen_v1_no_ec_board_params = {
 	.thermal_profile	= &omen_v1_no_ec_thermal_params,
+	.fan_profile		= &victus_s_fan_profile_params,
 };
 
 static const struct hp_wmi_board_params *active_board_params;
@@ -1793,6 +1810,38 @@ static bool is_victus_s_thermal_profile(void)
 	return is_victus_s_board;
 }
 
+static const struct hp_wmi_fan_profile_params *hp_wmi_fan_profile(void)
+{
+	if (!active_board_params)
+		return NULL;
+
+	return active_board_params->fan_profile;
+}
+
+static bool hp_wmi_fan_control_supported(void)
+{
+	const struct hp_wmi_fan_profile_params *params = hp_wmi_fan_profile();
+
+	return params && params->get_fan_speed;
+}
+
+static bool hp_wmi_fan_table_supported(void)
+{
+	const struct hp_wmi_fan_profile_params *params = hp_wmi_fan_profile();
+
+	return params && params->fan_table;
+}
+
+static int hp_wmi_get_active_fan_speed(int fan)
+{
+	const struct hp_wmi_fan_profile_params *params = hp_wmi_fan_profile();
+
+	if (!params || !params->get_fan_speed)
+		return -EOPNOTSUPP;
+
+	return params->get_fan_speed(fan);
+}
+
 static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
 					    bool *ppab_enable,
 					    u8 *dstate,
@@ -2412,7 +2461,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 
 	switch (priv->mode) {
 	case PWM_MODE_MAX:
-		if (is_victus_s_thermal_profile()) {
+		if (hp_wmi_fan_control_supported()) {
 			ret = hp_wmi_get_fan_count_userdefine_trigger();
 			if (ret < 0)
 				return ret;
@@ -2424,7 +2473,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
 		return 0;
 	case PWM_MODE_MANUAL:
-		if (!is_victus_s_thermal_profile())
+		if (!hp_wmi_fan_control_supported())
 			return -EOPNOTSUPP;
 		ret = hp_wmi_fan_speed_set(priv, pwm_to_rpm(priv->pwm, priv));
 		if (ret < 0)
@@ -2433,7 +2482,7 @@ static int hp_wmi_apply_fan_settings(struct hp_wmi_hwmon_priv *priv)
 				 secs_to_jiffies(KEEP_ALIVE_DELAY_SECS));
 		return 0;
 	case PWM_MODE_AUTO:
-		if (is_victus_s_thermal_profile()) {
+		if (hp_wmi_fan_control_supported()) {
 			ret = hp_wmi_get_fan_count_userdefine_trigger();
 			if (ret < 0)
 				return ret;
@@ -2457,12 +2506,12 @@ static umode_t hp_wmi_hwmon_is_visible(const void *data,
 {
 	switch (type) {
 	case hwmon_pwm:
-		if (attr == hwmon_pwm_input && !is_victus_s_thermal_profile())
+		if (attr == hwmon_pwm_input && !hp_wmi_fan_control_supported())
 			return 0;
 		return 0644;
 	case hwmon_fan:
-		if (is_victus_s_thermal_profile()) {
-			if (hp_wmi_get_fan_speed_victus_s(channel) >= 0)
+		if (hp_wmi_fan_control_supported()) {
+			if (hp_wmi_get_active_fan_speed(channel) >= 0)
 				return 0444;
 		} else {
 			if (hp_wmi_get_fan_speed(channel) >= 0)
@@ -2486,8 +2535,8 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 	priv = dev_get_drvdata(dev);
 	switch (type) {
 	case hwmon_fan:
-		if (is_victus_s_thermal_profile())
-			ret = hp_wmi_get_fan_speed_victus_s(channel);
+		if (hp_wmi_fan_control_supported())
+			ret = hp_wmi_get_active_fan_speed(channel);
 		else
 			ret = hp_wmi_get_fan_speed(channel);
 		if (ret < 0)
@@ -2496,10 +2545,10 @@ static int hp_wmi_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 		return 0;
 	case hwmon_pwm:
 		if (attr == hwmon_pwm_input) {
-			if (!is_victus_s_thermal_profile())
+			if (!hp_wmi_fan_control_supported())
 				return -EOPNOTSUPP;
 
-			rpm = hp_wmi_get_fan_speed_victus_s(channel);
+			rpm = hp_wmi_get_active_fan_speed(channel);
 			if (rpm < 0)
 				return rpm;
 			*val = rpm_to_pwm(rpm / 100, priv);
@@ -2533,7 +2582,7 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 	switch (type) {
 	case hwmon_pwm:
 		if (attr == hwmon_pwm_input) {
-			if (!is_victus_s_thermal_profile())
+			if (!hp_wmi_fan_control_supported())
 				return -EOPNOTSUPP;
 			/* PWM input is invalid when not in manual mode */
 			if (priv->mode != PWM_MODE_MANUAL)
@@ -2550,13 +2599,13 @@ static int hp_wmi_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
 			priv->mode = PWM_MODE_MAX;
 			return hp_wmi_apply_fan_settings(priv);
 		case PWM_MODE_MANUAL:
-			if (!is_victus_s_thermal_profile())
+			if (!hp_wmi_fan_control_supported())
 				return -EOPNOTSUPP;
 			/*
 			 * When switching to manual mode, set fan speed to
 			 * current RPM values to ensure a smooth transition.
 			 */
-			rpm = hp_wmi_get_fan_speed_victus_s(channel);
+			rpm = hp_wmi_get_active_fan_speed(channel);
 			if (rpm < 0)
 				return rpm;
 			priv->pwm = rpm_to_pwm(rpm / 100, priv);
@@ -2622,8 +2671,8 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 	/* Default behaviour on hwmon init is automatic mode */
 	priv->mode = PWM_MODE_AUTO;
 
-	/* Bypass all non-Victus S devices */
-	if (!is_victus_s_thermal_profile())
+	/* Bypass devices without fan control support. */
+	if (!hp_wmi_fan_table_supported())
 		return 0;
 
 	ret = hp_wmi_perform_query(HPWMI_VICTUS_S_GET_FAN_TABLE_QUERY,
-- 
2.54.0

[PATCH v6 3/4] platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta

[Radhey Kalra]

Some Victus fan tables start with a CPU RPM value while the GPU RPM
field is zero. On board 8A3D the first row is cpu=23, gpu=0, noise=25,
followed by cpu=24, gpu=22, noise=26.

Derive gpu_delta from the first row that contains a non-zero GPU RPM
instead of unconditionally using row 0.

Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index ae2abb9..dcfffe6 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -2666,6 +2666,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 	u8 min_rpm, max_rpm;
 	u8 cpu_rpm, gpu_rpm, noise_db;
 	int gpu_delta, i, num_entries, ret;
+	bool gpu_delta_found = false;
 	size_t header_size, entry_size;
 
 	/* Default behaviour on hwmon init is automatic mode */
@@ -2706,12 +2707,15 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
 			min_rpm = cpu_rpm;
 		if (cpu_rpm > max_rpm)
 			max_rpm = cpu_rpm;
+		if (!gpu_delta_found && gpu_rpm > 0) {
+			gpu_delta = gpu_rpm - cpu_rpm;
+			gpu_delta_found = true;
+		}
 	}
 
-	if (min_rpm == U8_MAX || max_rpm == 0)
+	if (min_rpm == U8_MAX || max_rpm == 0 || !gpu_delta_found)
 		return -EINVAL;
 
-	gpu_delta = fan_table->entries[0].gpu_rpm - fan_table->entries[0].cpu_rpm;
 	priv->min_rpm = min_rpm;
 	priv->max_rpm = max_rpm;
 	priv->gpu_delta = gpu_delta;
-- 
2.54.0

Re: [PATCH v6 3/4] platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta

[Krishna Chomal]

On Thu, May 21, 2026 at 07:03:50PM +0530, Radhey Kalra wrote:
>Some Victus fan tables start with a CPU RPM value while the GPU RPM
>field is zero. On board 8A3D the first row is cpu=23, gpu=0, noise=25,
>followed by cpu=24, gpu=22, noise=26.
>
>Derive gpu_delta from the first row that contains a non-zero GPU RPM
>instead of unconditionally using row 0.

I agree, this is essential for fan-control to work sensibly. But I just
want to give Ilpo Jarvinen a quick heads-up because this would likely
cause conflict with foobisdweik's patch [1], as they would (probably)
remove gpu_delta in v2 [2]

[1]: https://lore.kernel.org/platform-driver-x86/20260513193916.84673-3-dweikmferris@gmail.com
[2]: https://lore.kernel.org/platform-driver-x86/ag2DCOGyCt7MBUL6@archlinux

>Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
>---
> drivers/platform/x86/hp/hp-wmi.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
>index ae2abb9..dcfffe6 100644
>--- a/drivers/platform/x86/hp/hp-wmi.c
>+++ b/drivers/platform/x86/hp/hp-wmi.c
>@@ -2666,6 +2666,7 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
> 	u8 min_rpm, max_rpm;
> 	u8 cpu_rpm, gpu_rpm, noise_db;
> 	int gpu_delta, i, num_entries, ret;
>+	bool gpu_delta_found = false;
> 	size_t header_size, entry_size;
>
> 	/* Default behaviour on hwmon init is automatic mode */
>@@ -2706,12 +2707,15 @@ static int hp_wmi_setup_fan_settings(struct hp_wmi_hwmon_priv *priv)
> 			min_rpm = cpu_rpm;
> 		if (cpu_rpm > max_rpm)
> 			max_rpm = cpu_rpm;
>+		if (!gpu_delta_found && gpu_rpm > 0) {
>+			gpu_delta = gpu_rpm - cpu_rpm;
>+			gpu_delta_found = true;
>+		}
> 	}
>
>-	if (min_rpm == U8_MAX || max_rpm == 0)
>+	if (min_rpm == U8_MAX || max_rpm == 0 || !gpu_delta_found)
> 		return -EINVAL;
>
>-	gpu_delta = fan_table->entries[0].gpu_rpm - fan_table->entries[0].cpu_rpm;
> 	priv->min_rpm = min_rpm;
> 	priv->max_rpm = max_rpm;
> 	priv->gpu_delta = gpu_delta;
>-- 
>2.54.0
>

Re: [PATCH v6 3/4] platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta

[Ilpo Järvinen]

On Thu, 21 May 2026, Krishna Chomal wrote:

> On Thu, May 21, 2026 at 07:03:50PM +0530, Radhey Kalra wrote:
> > Some Victus fan tables start with a CPU RPM value while the GPU RPM
> > field is zero. On board 8A3D the first row is cpu=23, gpu=0, noise=25,
> > followed by cpu=24, gpu=22, noise=26.
> > 
> > Derive gpu_delta from the first row that contains a non-zero GPU RPM
> > instead of unconditionally using row 0.
> 
> I agree, this is essential for fan-control to work sensibly. But I just
> want to give Ilpo Jarvinen a quick heads-up because this would likely
> cause conflict with foobisdweik's patch [1], as they would (probably)
> remove gpu_delta in v2 [2]
> 
> [1]:
> https://lore.kernel.org/platform-driver-x86/20260513193916.84673-3-dweikmferris@gmail.com
> [2]: https://lore.kernel.org/platform-driver-x86/ag2DCOGyCt7MBUL6@archlinux

Okay, so we can work on the other parts and postpone the gpu_delta part to 
that other series. It would be nice if Radhey could test with that GPU fan 
patch once the next version of it is posted.


FYI, sashiko seems to find a few potential corner case issues unrelated to 
this series:

https://sashiko.dev/#/patchset/20260521133351.1512718-1-radheykalra901%40gmail.com

(Mainly to make Krishna aware of them but I certainly wouldn't mind if
somebody else has time to tackle them). But please don't take every 
sashiko's (AI) claim as the perfect truth, it just isn't always right.


-- 
 i.

Re: [PATCH v6 3/4] platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta

[Krishna Chomal]

On Fri, May 22, 2026 at 04:56:02PM +0300, Ilpo Järvinen wrote:
>On Thu, 21 May 2026, Krishna Chomal wrote:
>
>> On Thu, May 21, 2026 at 07:03:50PM +0530, Radhey Kalra wrote:
>> > Some Victus fan tables start with a CPU RPM value while the GPU RPM
>> > field is zero. On board 8A3D the first row is cpu=23, gpu=0, noise=25,
>> > followed by cpu=24, gpu=22, noise=26.
>> >
>> > Derive gpu_delta from the first row that contains a non-zero GPU RPM
>> > instead of unconditionally using row 0.
>>
>> I agree, this is essential for fan-control to work sensibly. But I just
>> want to give Ilpo Jarvinen a quick heads-up because this would likely
>> cause conflict with foobisdweik's patch [1], as they would (probably)
>> remove gpu_delta in v2 [2]
>>
>> [1]:
>> https://lore.kernel.org/platform-driver-x86/20260513193916.84673-3-dweikmferris@gmail.com
>> [2]: https://lore.kernel.org/platform-driver-x86/ag2DCOGyCt7MBUL6@archlinux
>
>Okay, so we can work on the other parts and postpone the gpu_delta part to
>that other series. It would be nice if Radhey could test with that GPU fan
>patch once the next version of it is posted.
>
>
>FYI, sashiko seems to find a few potential corner case issues unrelated to
>this series:
>
>https://sashiko.dev/#/patchset/20260521133351.1512718-1-radheykalra901%40gmail.com
>
>(Mainly to make Krishna aware of them but I certainly wouldn't mind if
>somebody else has time to tackle them). But please don't take every
>sashiko's (AI) claim as the perfect truth, it just isn't always right.

Thanks! I wasn't aware of sashiko, but I'll definitely have a deeper
look at it when I have some time.

>
>
>-- 
> i.
>

[PATCH v6 4/4] platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control

[Radhey Kalra]

HP Victus 15-fb0xxx board 8A3D exposes the Victus fan table and accepts
the existing Victus fan-speed WMI control path. Add it as a
fan-control-only board so the hwmon PWM interface is enabled without
forcing Victus S thermal-profile handling on this model.

Tested on a Victus by HP Gaming Laptop 15-fb0xxx, board 8A3D, BIOS
F.22. The fan table query succeeded and returned rows including
cpu=23/gpu=0 followed by cpu=24/gpu=22. With 8A3D enabled in the
fan-control path, pwm1 and pwm1_enable were exposed through hwmon.

Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index dcfffe6..f463823 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -170,6 +170,10 @@ static const struct hp_wmi_board_params omen_v1_no_ec_board_params = {
 	.fan_profile		= &victus_s_fan_profile_params,
 };
 
+static const struct hp_wmi_board_params victus_8a3d_board_params = {
+	.fan_profile		= &victus_s_fan_profile_params,
+};
+
 static const struct hp_wmi_board_params *active_board_params;
 
 static const struct thermal_profile_params *hp_wmi_thermal_profile(void)
@@ -230,6 +234,10 @@ static const char * const victus_thermal_profile_boards[] = {
 
 /* DMI board-specific feature data for Omen and Victus laptops. */
 static const struct dmi_system_id hp_wmi_feature_boards[] __initconst = {
+	{
+		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A3D") },
+		.driver_data = (void *)&victus_8a3d_board_params,
+	},
 	{
 		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A44") },
 		.driver_data = (void *)&omen_v1_legacy_board_params,
-- 
2.54.0

Re: [PATCH v6 0/4] platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control

[Krishna Chomal]

Hi all,

Apologies, I accidentally replied to the v5 cover letter right as v6
was published. Re-posting my feedback here for the correct thread:

On Thu, May 21, 2026 at 07:03:47PM +0530, Radhey Kalra wrote:
>Hi,
>
>This is a split version of the Victus 15-fb0xxx fan-control change.
>Patches 1 and 2 are no-functional-changes-intended refactors which move
>the existing thermal-profile and fan-control decisions into
>board-specific .driver_data. Patch 3 fixes the fan-table GPU delta
>parsing issue found while testing this board. Patch 4 adds fan-control
>support for Victus 15-fb0xxx board 8A3D.
>
>v6 splits the gpu_delta fan-table parsing fix back into its own patch.
>The previous 2-patch v5 folded that behavioral parser fix into the board
>data preparation patch, which was a mistake.
>
>8A3D is kept fan-control-only because the verified behavior is the
>Victus fan-control path. The existing generic platform-profile sysfs path
>on 8A3D accepts writes/readbacks without hp_wmi errors, but CPU/GPU
>power-policy behavior was not meaningfully validated.

You are right. Because your board is not registered in any list, so it
goes to a fallback route and registers quiet/balanced/performance/cool.
Changes to platform profile are handled through this chain:

hp_wmi_platform_profile_set
	-> thermal_profile_set
		-> hp_wmi_perform_query(HPWMI_THERMAL_PROFILE_QUERY)

For my device, when I selected victus_8a3d_board_params quirk, the driver
tried to use this query HPWMI_THERMAL_PROFILE_QUERY which failed and
caused the driver to not load. In you case, it looks like the query does
not return an error. However, not returning an error does not necessarily
imply that it is working correctly, which can be further confirmed when
you mention that changing power modes has no physical effect on the system.

>Adding 8A3D to the Victus S thermal-profile table would move it to a
>different, Victus S-specific platform-profile path that has not been
>verified on this board.

I apologise if this comes off as pestering, but I do feel that Victus-S
path should be tested at least once. I may be wrong, but at least we
would come to know that it is not the right approach.

>Based on platform-drivers-x86/for-next commit:
>165e81354eefd5551358112773f24027aac59d5a
>
>Tested on a Victus by HP Gaming Laptop 15-fb0xxx, board 8A3D, BIOS
>F.22. With the series applied, pwm1 and pwm1_enable are exposed and
>the EC-layout warning is not emitted for 8A3D.
>
>Radhey Kalra (4):
>  platform/x86: hp-wmi: Introduce board-specific feature data
>  platform/x86: hp-wmi: Drive fan control from board data
>  platform/x86: hp-wmi: Skip zero GPU RPM rows for fan speed delta
>  platform/x86: hp-wmi: Add Victus 15-fb0xxx fan control
>
> drivers/platform/x86/hp/hp-wmi.c | 212 ++++++++++++++++++++++---------
> 1 file changed, 153 insertions(+), 59 deletions(-)
>
>---
>Changes in v6:
>- Split the gpu_delta fan-table parser fix back out into its own patch.
>- Keep 8A3D fan-control-only and document why the Victus S
>  platform-profile path is not enabled for this board.
>
>Changes in v5:
>- Drop the active_thermal_profile_params global and dereference the
>  thermal-profile data through active_board_params.
>- Move the generic fan-only board handling into patch 1, keeping the
>  final 8A3D patch focused on adding the board data and DMI match.
>- Wrap the setup comment added in patch 1 to stay under 80 columns.
>
>Changes in v4:
>- Split v3 into two no-functional-changes-intended refactors, one
>  fan-table parsing fix, and one 8A3D support patch.
>- Rebase on platform-drivers-x86/for-next commit 165e81354eefd555.
>- Move revision notes to the cover letter.
>
>Changes in v3:
>- Rebase on platform-drivers-x86/for-next commit 165e81354eefd555.
>- Keep the v2 board-data approach and 8A3D fan-control-only handling.
>
>Changes in v2:
>- Use real name in Signed-off-by.
>- Replace the ad-hoc fan-control DMI table with board data in
>  .driver_data.
>- Keep 8A3D fan-control-only instead of enabling Victus S thermal
>  profiles.
>- Use the fixed fan-table parser path and derive gpu_delta from the
>  first non-zero GPU row.
>
>-- 
>2.54.0