Re: [PATCH 4/5] platform/x86: hp-wmi: normalize GPU thermal mode errors

[Ilpo Järvinen]

On Thu, 16 Jul 2026, Emre Cecanpunar wrote:

> The GPU thermal mode helpers return positive firmware error codes, but
> their callers only treat negative values as failures. If the mode query
> is rejected, its output parameters remain uninitialized and are then
> used to select a profile or sent back to firmware as the GPU slowdown
> temperature. A rejected mode update is likewise reported as successful.

Does this happen with some HW? Please add the info.

> Zero the query buffer and convert positive firmware status codes to
> -EINVAL in both helpers so callers never consume missing output or ignore
> a failed update.
> 
> Fixes: 6e4ab59b8391 ("platform/x86: hp-wmi: Add fan and thermal profile support for Victus 16-s1000")
> Signed-off-by: Emre Cecanpunar <emreleno@xxxxxxxxx>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 29881749aae6..65c3bac17ad6 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -1885,20 +1885,21 @@ static int victus_s_gpu_thermal_profile_get(bool *ctgp_enable,
>  					    u8 *dstate,
>  					    u8 *gpu_slowdown_temp)
>  {
> -	struct victus_gpu_power_modes gpu_power_modes;
> +	struct victus_gpu_power_modes gpu_power_modes = {};

Why is this necessary if you return error after the if change?

>  	int ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_GET_GPU_THERMAL_MODES_QUERY, HPWMI_GM,
>  				   &gpu_power_modes, sizeof(gpu_power_modes),
>  				   sizeof(gpu_power_modes));
> -	if (ret == 0) {
> -		*ctgp_enable = gpu_power_modes.ctgp_enable ? true : false;
> -		*ppab_enable = gpu_power_modes.ppab_enable ? true : false;
> -		*dstate = gpu_power_modes.dstate;
> -		*gpu_slowdown_temp = gpu_power_modes.gpu_slowdown_temp;
> -	}
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;

Use two separate ifs instead.

> -	return ret;
> +	*ctgp_enable = gpu_power_modes.ctgp_enable ? true : false;
> +	*ppab_enable = gpu_power_modes.ppab_enable ? true : false;
> +	*dstate = gpu_power_modes.dstate;
> +	*gpu_slowdown_temp = gpu_power_modes.gpu_slowdown_temp;
> +
> +	return 0;
>  }
>  
>  static int victus_s_gpu_thermal_profile_set(bool ctgp_enable,
> @@ -1929,8 +1930,10 @@ static int victus_s_gpu_thermal_profile_set(bool ctgp_enable,
>  
>  	ret = hp_wmi_perform_query(HPWMI_SET_GPU_THERMAL_MODES_QUERY, HPWMI_GM,
>  				   &gpu_power_modes, sizeof(gpu_power_modes), 0);
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;
>  
> -	return ret;
> +	return 0;

Now this could be:

	if (ret > 0)
		return -EINVAL;

	return ret;

-- 
 i.