Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <1cb582a3-c372-5f6b-b228-523ad45ed8e9@redhat.com>
Date:   Tue, 27 Sep 2022 12:24:10 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH] platform/x86: hp-wmi: add support for quiet thermal
 profile
Content-Language: en-US
To:     Elia Devito <eliadevito@gmail.com>,
        Jorge Lopez <jorgealtxwork@gmail.com>
Cc:     Philippe Rouquier <bonfire-app@wanadoo.fr>,
        Mark Gross <markgross@kernel.org>,
        platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220925204244.53506-1-eliadevito@gmail.com>
From:   Hans de Goede <hdegoede@redhat.com>
In-Reply-To: <20220925204244.53506-1-eliadevito@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

Hi Elia,

On 9/25/22 22:42, Elia Devito wrote:
> The quiet profile is available only on models without dGPU,
> so we enable it when the device has only one GPU onboard.
> 
> Signed-off-by: Elia Devito <eliadevito@gmail.com>
> Tested-by: Philippe Rouquier <bonfire-app@wanadoo.fr>

Thank you for your patch.

I have just landed a very similar patch form Jorge from HP:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/commit/?h=for-next&id=00b1829294b7c88ecba92c661fbe6fe347b364d2

The first 3 hunks of the patch are the same, only the conditions
under which the:

	set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);

call is made are different, Jorge's patch does this essentially like this:
	
	if (!is_omen_thermal_profile())
		set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);

which seems a bit cleaner to me and hopefully works just as well
or maybe even better.

Regards,

Hans


> ---
>  drivers/platform/x86/hp-wmi.c | 21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index bc7020e9df9e..3e0e67be8001 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -177,7 +177,8 @@ enum hp_thermal_profile_omen_v1 {
>  enum hp_thermal_profile {
>  	HP_THERMAL_PROFILE_PERFORMANCE	= 0x00,
>  	HP_THERMAL_PROFILE_DEFAULT		= 0x01,
> -	HP_THERMAL_PROFILE_COOL			= 0x02
> +	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)
> @@ -1194,6 +1195,9 @@ static int hp_wmi_platform_profile_get(struct platform_profile_handler *pprof,
>  	case HP_THERMAL_PROFILE_COOL:
>  		*profile =  PLATFORM_PROFILE_COOL;
>  		break;
> +	case HP_THERMAL_PROFILE_QUIET:
> +		*profile =  PLATFORM_PROFILE_QUIET;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> @@ -1216,6 +1220,9 @@ static int hp_wmi_platform_profile_set(struct platform_profile_handler *pprof,
>  	case PLATFORM_PROFILE_COOL:
>  		tp =  HP_THERMAL_PROFILE_COOL;
>  		break;
> +	case PLATFORM_PROFILE_QUIET:
> +		tp =  HP_THERMAL_PROFILE_QUIET;
> +		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -1230,6 +1237,8 @@ static int hp_wmi_platform_profile_set(struct platform_profile_handler *pprof,
>  static int thermal_profile_setup(void)
>  {
>  	int err, tp;
> +	unsigned int n_gpu = 0;
> +	const struct dmi_device *dev = NULL;
>  
>  	if (is_omen_thermal_profile()) {
>  		tp = omen_thermal_profile_get();
> @@ -1263,6 +1272,16 @@ static int thermal_profile_setup(void)
>  
>  		platform_profile_handler.profile_get = hp_wmi_platform_profile_get;
>  		platform_profile_handler.profile_set = hp_wmi_platform_profile_set;
> +
> +		/*
> +		 * The quiet profile is available only on models without dGPU,
> +		 * so we enable it when the device has only one GPU onboard.
> +		 */
> +		while ((dev = dmi_find_device(DMI_DEV_TYPE_VIDEO, NULL, dev)))
> +			n_gpu++;
> +
> +		if (n_gpu == 1)
> +			set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
>  	}
>  
>  	set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);