All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krishna Chomal <krishna.chomal108@gmail.com>
To: Marco Scardovi <scardracs@disroot.org>
Cc: moravec@ukf.sk, hansg@kernel.org, ilpo.jarvinen@linux.intel.com,
	 eliadevito@gmail.com, emreleno@gmail.com, rafael@kernel.org,
	 linux-kernel@vger.kernel.org,
	platform-driver-x86@vger.kernel.org, regressions@leemhuis.info,
	 regressions@lists.linux.dev
Subject: Re: [PATCH 1/1] platform/x86: hp-wmi: fix platform profile issues on generic HP laptops
Date: Tue, 23 Jun 2026 21:22:49 +0530	[thread overview]
Message-ID: <ajqgkq_elUlMC0Ep@archlinux> (raw)
In-Reply-To: <20260619220911.7542-2-scardracs@disroot.org>

On Sat, Jun 20, 2026 at 12:09:11AM +0200, Marco Scardovi wrote:
>On generic (non-Omen/non-Victus) HP laptops supported by the
>hp-wmi driver, the platform_profile sysfs operations could fail,
>leading to 'platform_profile: Failed to get profile for handler hp-wmi'
>errors and preventing userspace power profile management from working
>correctly.
>
>The driver was blindly registering all 4 profiles (quiet, cool,
>balanced, performance) without checking which ones were actually
>supported by the BIOS. Furthermore, when userspace switched profiles,
>hp_wmi_platform_profile_get() was called, which queried the BIOS
>using thermal_profile_get(). If the BIOS query returned an error or
>returned a value not in the recognized cases, the get call failed,
>throwing an error.
>
>Fix this by:
>1. Dynamically probing which thermal profiles are supported by the BIOS
>   during driver registration by temporarily setting each profile and
>   checking the return code, then restoring the original profile.
>2. Initializing and updating active_platform_profile to cache the
>   last set/boot profile.
>3. Falling back to the cached active_platform_profile in
>   hp_wmi_platform_profile_get() if the BIOS query fails or returns
>   an invalid/unmapped value, rather than returning an error.
>
>Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220008

Is the patch tested by them?

>Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221569
>Closes: https://lore.kernel.org/platform-driver-x86/a3b137df-1b21-4460-b003-58c5ca2d59d4@ukf.sk/
>Fixes: 4296f679ca50 ("platform/x86: hp-wmi: add platform profile support")
>Assisted-by: Antigravity:gemini-3.5-flash
>Signed-off-by: Marco Scardovi <scardracs@disroot.org>
>---
> drivers/platform/x86/hp/hp-wmi.c | 53 +++++++++++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
>index 8ba286ed8721..93df269445eb 100644
>--- a/drivers/platform/x86/hp/hp-wmi.c
>+++ b/drivers/platform/x86/hp/hp-wmi.c
>@@ -1662,8 +1662,11 @@ static int hp_wmi_platform_profile_get(struct device *dev,
> 	int tp;
>
> 	tp = thermal_profile_get();
>-	if (tp < 0)
>-		return tp;
>+	if (tp < 0) {
>+		guard(mutex)(&active_platform_profile_lock);
>+		*profile = active_platform_profile;
>+		return 0;
>+	}

This does silence the error but I don't think that's a wise decision
since it effectively makes userspace blind to any WMI failure. Ideally,
if thermal_profile_get() fails then the board isn't supposed to be using
HPWMI_THERMAL_PROFILE_QUERY.

>
> 	switch (tp) {
> 	case HP_THERMAL_PROFILE_PERFORMANCE:
>@@ -1679,7 +1682,9 @@ static int hp_wmi_platform_profile_get(struct device *dev,
> 		*profile = PLATFORM_PROFILE_QUIET;
> 		break;
> 	default:
>-		return -EINVAL;
>+		guard(mutex)(&active_platform_profile_lock);
>+		*profile = active_platform_profile;
>+		break;
> 	}
>
> 	return 0;
>@@ -1707,10 +1712,14 @@ static int hp_wmi_platform_profile_set(struct device *dev,
> 		return -EOPNOTSUPP;
> 	}
>
>+	guard(mutex)(&active_platform_profile_lock);
>+
> 	err = thermal_profile_set(tp);
> 	if (err)
> 		return err;
>
>+	active_platform_profile = profile;
>+
> 	return 0;
> }
>
>@@ -2017,8 +2026,23 @@ static int hp_wmi_platform_profile_probe(void *drvdata, unsigned long *choices)
> 		/* Adding an equivalent to HP Omen software ECO mode: */
> 		set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
> 	} else {
>-		set_bit(PLATFORM_PROFILE_QUIET, choices);
>-		set_bit(PLATFORM_PROFILE_COOL, choices);
>+		int current_tp = thermal_profile_get();
>+
>+		if (current_tp < 0)
>+			return current_tp;
>+
>+		if (thermal_profile_set(HP_THERMAL_PROFILE_QUIET) == 0)
>+			set_bit(PLATFORM_PROFILE_QUIET, choices);
>+		if (thermal_profile_set(HP_THERMAL_PROFILE_COOL) == 0)
>+			set_bit(PLATFORM_PROFILE_COOL, choices);
>+		if (thermal_profile_set(HP_THERMAL_PROFILE_DEFAULT) == 0)
>+			set_bit(PLATFORM_PROFILE_BALANCED, choices);
>+		if (thermal_profile_set(HP_THERMAL_PROFILE_PERFORMANCE) == 0)
>+			set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);

Probing for HP_THERMAL_PROFILE_DEFAULT and HP_THERMAL_PROFILE_PERFORMANCE
seem redundant since those bits are unconditionally set in this function
after the if-else blocks.

>+
>+		/* Restore the original thermal profile */
>+		thermal_profile_set(current_tp);
>+		return 0;
> 	}
>
> 	set_bit(PLATFORM_PROFILE_BALANCED, choices);
>@@ -2263,6 +2287,25 @@ static int thermal_profile_setup(struct platform_device *device)
> 		if (err)
> 			return err;
>
>+		/* Initialize active_platform_profile */
>+		switch (tp) {
>+		case HP_THERMAL_PROFILE_PERFORMANCE:
>+			active_platform_profile = PLATFORM_PROFILE_PERFORMANCE;
>+			break;
>+		case HP_THERMAL_PROFILE_DEFAULT:
>+			active_platform_profile = PLATFORM_PROFILE_BALANCED;
>+			break;
>+		case HP_THERMAL_PROFILE_COOL:
>+			active_platform_profile = PLATFORM_PROFILE_COOL;
>+			break;
>+		case HP_THERMAL_PROFILE_QUIET:
>+			active_platform_profile = PLATFORM_PROFILE_QUIET;
>+			break;
>+		default:
>+			active_platform_profile = PLATFORM_PROFILE_BALANCED;
>+			break;
>+		}
>+
> 		ops = &hp_wmi_platform_profile_ops;
> 	}
>
>-- 
>2.54.0
>

  parent reply	other threads:[~2026-06-23 15:52 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-19 22:09 [PATCH 0/1] platform/x86: hp-wmi: fix platform profile issues on generic HP laptops Marco Scardovi
2026-06-19 22:09 ` [PATCH 1/1] " Marco Scardovi
2026-06-20  9:36   ` Milan Oravec
2026-06-23 15:52   ` Krishna Chomal [this message]
2026-06-23 16:45     ` [PATCH v2 0/1] " Marco Scardovi
2026-06-23 16:45       ` [PATCH v2 1/1] " Marco Scardovi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ajqgkq_elUlMC0Ep@archlinux \
    --to=krishna.chomal108@gmail.com \
    --cc=eliadevito@gmail.com \
    --cc=emreleno@gmail.com \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=moravec@ukf.sk \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=rafael@kernel.org \
    --cc=regressions@leemhuis.info \
    --cc=regressions@lists.linux.dev \
    --cc=scardracs@disroot.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.