platform/x86: hp-wmi: Add thermal profile for Victus 16-d1xxx

Message ID 20230529053959.4876-1-onenowy@gmail.com
State New
Headers
Series platform/x86: hp-wmi: Add thermal profile for Victus 16-d1xxx |

Commit Message

SungHwan Jung May 29, 2023, 5:39 a.m. UTC
  This patch includes Platform Profile support (performance, balanced, quiet)
for Victus 16-d1xxx (8A25).

Signed-off-by: SungHwan Jung <onenowy@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 104 +++++++++++++++++++++++++++++--
 1 file changed, 99 insertions(+), 5 deletions(-)
  

Comments

Ilpo Järvinen May 30, 2023, 8:11 a.m. UTC | #1
On Mon, 29 May 2023, SungHwan Jung wrote:

> This patch includes Platform Profile support (performance, balanced, quiet)
> for Victus 16-d1xxx (8A25).
> 
> Signed-off-by: SungHwan Jung <onenowy@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 104 +++++++++++++++++++++++++++++--
>  1 file changed, 99 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 6364ae262705..6259b907ce63 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -66,6 +66,11 @@ static const char *const omen_thermal_profile_force_v0_boards[] = {
>  	"8607", "8746", "8747", "8749", "874A", "8748"
>  };
>  
> +/* DMI Board names of Victus laptops */
> +static const char * const victus_thermal_profile_boards[] = {
> +	"8A25"
> +};
> +
>  enum hp_wmi_radio {
>  	HPWMI_WIFI	= 0x0,
>  	HPWMI_BLUETOOTH	= 0x1,
> @@ -176,6 +181,12 @@ enum hp_thermal_profile_omen_v1 {
>  	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
>  };
>  
> +enum hp_thermal_profile_victus {
> +	HP_VICTUS_THERMAL_PROFILE_DEFAULT		= 0x00,
> +	HP_VICTUS_THERMAL_PROFILE_PERFORMANCE	= 0x01,
> +	HP_VICTUS_THERMAL_PROFILE_QUIET			= 0x03,

These should be aligned.

> +};
> +
>  enum hp_thermal_profile {
>  	HP_THERMAL_PROFILE_PERFORMANCE	= 0x00,
>  	HP_THERMAL_PROFILE_DEFAULT		= 0x01,
> @@ -1246,6 +1257,70 @@ static int hp_wmi_platform_profile_set(struct platform_profile_handler *pprof,
>  	return 0;
>  }
>  
> +static bool is_victus_thermal_profile(void)
> +{
> +	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
> +
> +	if (!board_name)
> +		return false;
> +
> +	return match_string(victus_thermal_profile_boards,
> +			    ARRAY_SIZE(victus_thermal_profile_boards),
> +			    board_name) >= 0;
> +}
> +
> +static int platform_profile_victus_get(struct platform_profile_handler *pprof,
> +				     enum platform_profile_option *profile)
> +{
> +	int tp;
> +
> +	tp = omen_thermal_profile_get();
> +	if (tp < 0)
> +		return tp;
> +
> +	switch (tp) {
> +	case HP_VICTUS_THERMAL_PROFILE_PERFORMANCE:
> +		*profile =  PLATFORM_PROFILE_PERFORMANCE;
> +		break;
> +	case HP_VICTUS_THERMAL_PROFILE_DEFAULT:
> +		*profile =  PLATFORM_PROFILE_BALANCED;
> +		break;
> +	case HP_VICTUS_THERMAL_PROFILE_QUIET:
> +		*profile =  PLATFORM_PROFILE_QUIET;

Remove the extra space in all three assingments here.

> +		break;
> +	default:
> +		return -EINVAL;

It seems wrong to return -EINVAL when there's nothing wrong done by the 
caller (arguments were not invalid). Maybe use -ENOENT or -ENXIO instead.

> +	}
> +
> +	return 0;
> +}
> +
> +static int platform_profile_victus_set(struct platform_profile_handler *pprof,
> +				     enum platform_profile_option profile)
> +{
> +	int err, tp;
> +
> +	switch (profile) {
> +	case PLATFORM_PROFILE_PERFORMANCE:
> +		tp =  HP_VICTUS_THERMAL_PROFILE_PERFORMANCE;
> +		break;
> +	case PLATFORM_PROFILE_BALANCED:
> +		tp =  HP_VICTUS_THERMAL_PROFILE_DEFAULT;
> +		break;
> +	case PLATFORM_PROFILE_QUIET:
> +		tp =  HP_VICTUS_THERMAL_PROFILE_QUIET;

Again, remove extra spaces.

> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	err = omen_thermal_profile_set(tp);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
>  static int thermal_profile_setup(void)
>  {
>  	int err, tp;
> @@ -1266,6 +1341,25 @@ static int thermal_profile_setup(void)
>  
>  		platform_profile_handler.profile_get = platform_profile_omen_get;
>  		platform_profile_handler.profile_set = platform_profile_omen_set;
> +
> +		set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
> +	} else if (is_victus_thermal_profile()) {
> +		tp = omen_thermal_profile_get();
> +		if (tp < 0)
> +			return tp;
> +
> +		/*
> +		 * call thermal profile write command to ensure that the
> +		 * firmware correctly sets the OEM variables
> +		 */
> +		err = omen_thermal_profile_set(tp);
> +		if (err < 0)
> +			return err;
> +
> +		platform_profile_handler.profile_get = platform_profile_victus_get;
> +		platform_profile_handler.profile_set = platform_profile_victus_set;
> +
> +		set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
>  	} else {
>  		tp = thermal_profile_get();
>  
> @@ -1273,20 +1367,20 @@ static int thermal_profile_setup(void)
>  			return tp;
>  
>  		/*
> -		 * call thermal profile write command to ensure that the
> -		 * firmware correctly sets the OEM variables for the DPTF
> -		 */
> +		 * call thermal profile write command to ensure that the
> +		 * firmware correctly sets the OEM variables for the DPTF
> +		 */

A stray change?

>  		err = thermal_profile_set(tp);
>  		if (err)
>  			return err;
>  
>  		platform_profile_handler.profile_get = hp_wmi_platform_profile_get;
>  		platform_profile_handler.profile_set = hp_wmi_platform_profile_set;
> -
> +

A stray change?

>  		set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
> +		set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
>  	}
>  
> -	set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
>  	set_bit(PLATFORM_PROFILE_BALANCED, platform_profile_handler.choices);
>  	set_bit(PLATFORM_PROFILE_PERFORMANCE, platform_profile_handler.choices);

Besides the minor things noted above, the change looks good.
  
SungHwan Jung May 30, 2023, 2:50 p.m. UTC | #2
Thank you for your review. I have cleaned the patch, but found that it needs to set
the performance profile several times to get maximum power of dedicated gpu.
So I'll look into this. Thanks.
  

Patch

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 6364ae262705..6259b907ce63 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -66,6 +66,11 @@  static const char *const omen_thermal_profile_force_v0_boards[] = {
 	"8607", "8746", "8747", "8749", "874A", "8748"
 };
 
+/* DMI Board names of Victus laptops */
+static const char * const victus_thermal_profile_boards[] = {
+	"8A25"
+};
+
 enum hp_wmi_radio {
 	HPWMI_WIFI	= 0x0,
 	HPWMI_BLUETOOTH	= 0x1,
@@ -176,6 +181,12 @@  enum hp_thermal_profile_omen_v1 {
 	HP_OMEN_V1_THERMAL_PROFILE_COOL		= 0x50,
 };
 
+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 {
 	HP_THERMAL_PROFILE_PERFORMANCE	= 0x00,
 	HP_THERMAL_PROFILE_DEFAULT		= 0x01,
@@ -1246,6 +1257,70 @@  static int hp_wmi_platform_profile_set(struct platform_profile_handler *pprof,
 	return 0;
 }
 
+static bool is_victus_thermal_profile(void)
+{
+	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+
+	if (!board_name)
+		return false;
+
+	return match_string(victus_thermal_profile_boards,
+			    ARRAY_SIZE(victus_thermal_profile_boards),
+			    board_name) >= 0;
+}
+
+static int platform_profile_victus_get(struct platform_profile_handler *pprof,
+				     enum platform_profile_option *profile)
+{
+	int tp;
+
+	tp = omen_thermal_profile_get();
+	if (tp < 0)
+		return tp;
+
+	switch (tp) {
+	case HP_VICTUS_THERMAL_PROFILE_PERFORMANCE:
+		*profile =  PLATFORM_PROFILE_PERFORMANCE;
+		break;
+	case HP_VICTUS_THERMAL_PROFILE_DEFAULT:
+		*profile =  PLATFORM_PROFILE_BALANCED;
+		break;
+	case HP_VICTUS_THERMAL_PROFILE_QUIET:
+		*profile =  PLATFORM_PROFILE_QUIET;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int platform_profile_victus_set(struct platform_profile_handler *pprof,
+				     enum platform_profile_option profile)
+{
+	int err, tp;
+
+	switch (profile) {
+	case PLATFORM_PROFILE_PERFORMANCE:
+		tp =  HP_VICTUS_THERMAL_PROFILE_PERFORMANCE;
+		break;
+	case PLATFORM_PROFILE_BALANCED:
+		tp =  HP_VICTUS_THERMAL_PROFILE_DEFAULT;
+		break;
+	case PLATFORM_PROFILE_QUIET:
+		tp =  HP_VICTUS_THERMAL_PROFILE_QUIET;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	err = omen_thermal_profile_set(tp);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static int thermal_profile_setup(void)
 {
 	int err, tp;
@@ -1266,6 +1341,25 @@  static int thermal_profile_setup(void)
 
 		platform_profile_handler.profile_get = platform_profile_omen_get;
 		platform_profile_handler.profile_set = platform_profile_omen_set;
+
+		set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
+	} else if (is_victus_thermal_profile()) {
+		tp = omen_thermal_profile_get();
+		if (tp < 0)
+			return tp;
+
+		/*
+		 * call thermal profile write command to ensure that the
+		 * firmware correctly sets the OEM variables
+		 */
+		err = omen_thermal_profile_set(tp);
+		if (err < 0)
+			return err;
+
+		platform_profile_handler.profile_get = platform_profile_victus_get;
+		platform_profile_handler.profile_set = platform_profile_victus_set;
+
+		set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
 	} else {
 		tp = thermal_profile_get();
 
@@ -1273,20 +1367,20 @@  static int thermal_profile_setup(void)
 			return tp;
 
 		/*
-		 * call thermal profile write command to ensure that the
-		 * firmware correctly sets the OEM variables for the DPTF
-		 */
+		 * call thermal profile write command to ensure that the
+		 * firmware correctly sets the OEM variables for the DPTF
+		 */
 		err = thermal_profile_set(tp);
 		if (err)
 			return err;
 
 		platform_profile_handler.profile_get = hp_wmi_platform_profile_get;
 		platform_profile_handler.profile_set = hp_wmi_platform_profile_set;
-
+
 		set_bit(PLATFORM_PROFILE_QUIET, platform_profile_handler.choices);
+		set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
 	}
 
-	set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
 	set_bit(PLATFORM_PROFILE_BALANCED, platform_profile_handler.choices);
 	set_bit(PLATFORM_PROFILE_PERFORMANCE, platform_profile_handler.choices);