1. Patchew
  2. linux
  3. View series

[PATCH] platform/x86: thinkpad_acpi: Support for V9 DYTC platform profiles

Mark Pearson posted 1 patch 1 year ago
There is a newer version of this series
drivers/platform/x86/thinkpad_acpi.c | 33 ++++++++++++++++++----------
1 file changed, 21 insertions(+), 12 deletions(-)
[PATCH] platform/x86: thinkpad_acpi: Support for V9 DYTC platform profiles
Posted by Mark Pearson 1 year ago 
Newer Thinkpad AMD platforms are using V9 DYTC and this changes the
profiles used for PSC mode. Add support for this update.
Tested on P14s G5 AMD

Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
 drivers/platform/x86/thinkpad_acpi.c | 33 ++++++++++++++++++----------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 1fcb0f99695a..cae457bc0b07 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10319,6 +10319,10 @@ static struct ibm_struct proxsensor_driver_data = {
 #define DYTC_MODE_PSC_BALANCE  5  /* Default mode aka balanced */
 #define DYTC_MODE_PSC_PERFORM  7  /* High power mode aka performance */
 
+#define DYTC_MODE_PSCV9_LOWPOWER 1  /* Low power mode */
+#define DYTC_MODE_PSCV9_BALANCE  3  /* Default mode aka balanced */
+#define DYTC_MODE_PSCV9_PERFORM  4  /* High power mode aka performance */
+
 #define DYTC_ERR_MASK       0xF  /* Bits 0-3 in cmd result are the error result */
 #define DYTC_ERR_SUCCESS      1  /* CMD completed successful */
 
@@ -10339,6 +10343,10 @@ static int dytc_capabilities;
 static bool dytc_mmc_get_available;
 static int profile_force;
 
+static int platform_psc_profile_lowpower = DYTC_MODE_PSC_LOWPOWER;
+static int platform_psc_profile_balanced = DYTC_MODE_PSC_BALANCE;
+static int platform_psc_profile_performance = DYTC_MODE_PSC_PERFORM;
+
 static int convert_dytc_to_profile(int funcmode, int dytcmode,
 		enum platform_profile_option *profile)
 {
@@ -10360,19 +10368,14 @@ static int convert_dytc_to_profile(int funcmode, int dytcmode,
 		}
 		return 0;
 	case DYTC_FUNCTION_PSC:
-		switch (dytcmode) {
-		case DYTC_MODE_PSC_LOWPOWER:
+		if (dytcmode == platform_psc_profile_lowpower)
 			*profile = PLATFORM_PROFILE_LOW_POWER;
-			break;
-		case DYTC_MODE_PSC_BALANCE:
+		else if (dytcmode == platform_psc_profile_balanced)
 			*profile =  PLATFORM_PROFILE_BALANCED;
-			break;
-		case DYTC_MODE_PSC_PERFORM:
+		else if (dytcmode == platform_psc_profile_performance)
 			*profile =  PLATFORM_PROFILE_PERFORMANCE;
-			break;
-		default: /* Unknown mode */
+		else
 			return -EINVAL;
-		}
 		return 0;
 	case DYTC_FUNCTION_AMT:
 		/* For now return balanced. It's the closest we have to 'auto' */
@@ -10393,19 +10396,19 @@ static int convert_profile_to_dytc(enum platform_profile_option profile, int *pe
 		if (dytc_capabilities & BIT(DYTC_FC_MMC))
 			*perfmode = DYTC_MODE_MMC_LOWPOWER;
 		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
-			*perfmode = DYTC_MODE_PSC_LOWPOWER;
+			*perfmode = platform_psc_profile_lowpower;
 		break;
 	case PLATFORM_PROFILE_BALANCED:
 		if (dytc_capabilities & BIT(DYTC_FC_MMC))
 			*perfmode = DYTC_MODE_MMC_BALANCE;
 		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
-			*perfmode = DYTC_MODE_PSC_BALANCE;
+			*perfmode = platform_psc_profile_balanced;
 		break;
 	case PLATFORM_PROFILE_PERFORMANCE:
 		if (dytc_capabilities & BIT(DYTC_FC_MMC))
 			*perfmode = DYTC_MODE_MMC_PERFORM;
 		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
-			*perfmode = DYTC_MODE_PSC_PERFORM;
+			*perfmode = platform_psc_profile_performance;
 		break;
 	default: /* Unknown profile */
 		return -EOPNOTSUPP;
@@ -10599,6 +10602,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 	if (output & BIT(DYTC_QUERY_ENABLE_BIT))
 		dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
 
+	dbg_printk(TPACPI_DBG_INIT, "DYTC version %d\n", dytc_version);
 	/* Check DYTC is enabled and supports mode setting */
 	if (dytc_version < 5)
 		return -ENODEV;
@@ -10637,6 +10641,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 		}
 	} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) { /* PSC MODE */
 		pr_debug("PSC is supported\n");
+		if (dytc_version >= 9) { /* update profiles for DYTC 9 and up */
+			platform_psc_profile_lowpower = DYTC_MODE_PSCV9_LOWPOWER;
+			platform_psc_profile_balanced = DYTC_MODE_PSCV9_BALANCE;
+			platform_psc_profile_performance = DYTC_MODE_PSCV9_PERFORM;
+		}
 	} else {
 		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");
 		return -ENODEV;
-- 
2.47.1
Re: [PATCH] platform/x86: thinkpad_acpi: Support for V9 DYTC platform profiles
Posted by Hans de Goede 1 year ago 
Hi Mark,

On 30-Jan-25 4:45 PM, Mark Pearson wrote:
> Newer Thinkpad AMD platforms are using V9 DYTC and this changes the
> profiles used for PSC mode. Add support for this update.
> Tested on P14s G5 AMD
> 
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 33 ++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 1fcb0f99695a..cae457bc0b07 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10319,6 +10319,10 @@ static struct ibm_struct proxsensor_driver_data = {
>  #define DYTC_MODE_PSC_BALANCE  5  /* Default mode aka balanced */
>  #define DYTC_MODE_PSC_PERFORM  7  /* High power mode aka performance */
>  
> +#define DYTC_MODE_PSCV9_LOWPOWER 1  /* Low power mode */
> +#define DYTC_MODE_PSCV9_BALANCE  3  /* Default mode aka balanced */
> +#define DYTC_MODE_PSCV9_PERFORM  4  /* High power mode aka performance */
> +
>  #define DYTC_ERR_MASK       0xF  /* Bits 0-3 in cmd result are the error result */
>  #define DYTC_ERR_SUCCESS      1  /* CMD completed successful */
>  
> @@ -10339,6 +10343,10 @@ static int dytc_capabilities;
>  static bool dytc_mmc_get_available;
>  static int profile_force;
>  
> +static int platform_psc_profile_lowpower = DYTC_MODE_PSC_LOWPOWER;
> +static int platform_psc_profile_balanced = DYTC_MODE_PSC_BALANCE;
> +static int platform_psc_profile_performance = DYTC_MODE_PSC_PERFORM;
> +
>  static int convert_dytc_to_profile(int funcmode, int dytcmode,
>  		enum platform_profile_option *profile)
>  {
> @@ -10360,19 +10368,14 @@ static int convert_dytc_to_profile(int funcmode, int dytcmode,
>  		}
>  		return 0;
>  	case DYTC_FUNCTION_PSC:
> -		switch (dytcmode) {
> -		case DYTC_MODE_PSC_LOWPOWER:
> +		if (dytcmode == platform_psc_profile_lowpower)
>  			*profile = PLATFORM_PROFILE_LOW_POWER;
> -			break;
> -		case DYTC_MODE_PSC_BALANCE:
> +		else if (dytcmode == platform_psc_profile_balanced)
>  			*profile =  PLATFORM_PROFILE_BALANCED;
> -			break;
> -		case DYTC_MODE_PSC_PERFORM:
> +		else if (dytcmode == platform_psc_profile_performance)
>  			*profile =  PLATFORM_PROFILE_PERFORMANCE;
> -			break;
> -		default: /* Unknown mode */
> +		else
>  			return -EINVAL;
> -		}
>  		return 0;

Maybe replace the removed '}' with an empty line instead of
removing the entire line?

Currently after your patch the new code looks like this:

			...
		else
			return -EINVAL;
		return 0;

which look a bit weird, personally I would prefer:

			...
		else
			return -EINVAL;

		return 0;

Otherwise this looks good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans





>  	case DYTC_FUNCTION_AMT:
>  		/* For now return balanced. It's the closest we have to 'auto' */
> @@ -10393,19 +10396,19 @@ static int convert_profile_to_dytc(enum platform_profile_option profile, int *pe
>  		if (dytc_capabilities & BIT(DYTC_FC_MMC))
>  			*perfmode = DYTC_MODE_MMC_LOWPOWER;
>  		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
> -			*perfmode = DYTC_MODE_PSC_LOWPOWER;
> +			*perfmode = platform_psc_profile_lowpower;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED:
>  		if (dytc_capabilities & BIT(DYTC_FC_MMC))
>  			*perfmode = DYTC_MODE_MMC_BALANCE;
>  		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
> -			*perfmode = DYTC_MODE_PSC_BALANCE;
> +			*perfmode = platform_psc_profile_balanced;
>  		break;
>  	case PLATFORM_PROFILE_PERFORMANCE:
>  		if (dytc_capabilities & BIT(DYTC_FC_MMC))
>  			*perfmode = DYTC_MODE_MMC_PERFORM;
>  		else if (dytc_capabilities & BIT(DYTC_FC_PSC))
> -			*perfmode = DYTC_MODE_PSC_PERFORM;
> +			*perfmode = platform_psc_profile_performance;
>  		break;
>  	default: /* Unknown profile */
>  		return -EOPNOTSUPP;
> @@ -10599,6 +10602,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  	if (output & BIT(DYTC_QUERY_ENABLE_BIT))
>  		dytc_version = (output >> DYTC_QUERY_REV_BIT) & 0xF;
>  
> +	dbg_printk(TPACPI_DBG_INIT, "DYTC version %d\n", dytc_version);
>  	/* Check DYTC is enabled and supports mode setting */
>  	if (dytc_version < 5)
>  		return -ENODEV;
> @@ -10637,6 +10641,11 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  		}
>  	} else if (dytc_capabilities & BIT(DYTC_FC_PSC)) { /* PSC MODE */
>  		pr_debug("PSC is supported\n");
> +		if (dytc_version >= 9) { /* update profiles for DYTC 9 and up */
> +			platform_psc_profile_lowpower = DYTC_MODE_PSCV9_LOWPOWER;
> +			platform_psc_profile_balanced = DYTC_MODE_PSCV9_BALANCE;
> +			platform_psc_profile_performance = DYTC_MODE_PSCV9_PERFORM;
> +		}
>  	} else {
>  		dbg_printk(TPACPI_DBG_INIT, "No DYTC support available\n");
>  		return -ENODEV;
Re: [PATCH] platform/x86: thinkpad_acpi: Support for V9 DYTC platform profiles
Posted by Mark Pearson 1 year ago 
Hi Hans,

Thanks for the review.

On Thu, Jan 30, 2025, at 11:25 AM, Hans de Goede wrote:
> Hi Mark,
>
> On 30-Jan-25 4:45 PM, Mark Pearson wrote:
>> Newer Thinkpad AMD platforms are using V9 DYTC and this changes the
>> profiles used for PSC mode. Add support for this update.
>> Tested on P14s G5 AMD
>> 
>> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>> ---
>>  drivers/platform/x86/thinkpad_acpi.c | 33 ++++++++++++++++++----------
>>  1 file changed, 21 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 1fcb0f99695a..cae457bc0b07 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -10319,6 +10319,10 @@ static struct ibm_struct proxsensor_driver_data = {
>>  #define DYTC_MODE_PSC_BALANCE  5  /* Default mode aka balanced */
>>  #define DYTC_MODE_PSC_PERFORM  7  /* High power mode aka performance */
>>  
>> +#define DYTC_MODE_PSCV9_LOWPOWER 1  /* Low power mode */
>> +#define DYTC_MODE_PSCV9_BALANCE  3  /* Default mode aka balanced */
>> +#define DYTC_MODE_PSCV9_PERFORM  4  /* High power mode aka performance */
>> +
>>  #define DYTC_ERR_MASK       0xF  /* Bits 0-3 in cmd result are the error result */
>>  #define DYTC_ERR_SUCCESS      1  /* CMD completed successful */
>>  
>> @@ -10339,6 +10343,10 @@ static int dytc_capabilities;
>>  static bool dytc_mmc_get_available;
>>  static int profile_force;
>>  
>> +static int platform_psc_profile_lowpower = DYTC_MODE_PSC_LOWPOWER;
>> +static int platform_psc_profile_balanced = DYTC_MODE_PSC_BALANCE;
>> +static int platform_psc_profile_performance = DYTC_MODE_PSC_PERFORM;
>> +
>>  static int convert_dytc_to_profile(int funcmode, int dytcmode,
>>  		enum platform_profile_option *profile)
>>  {
>> @@ -10360,19 +10368,14 @@ static int convert_dytc_to_profile(int funcmode, int dytcmode,
>>  		}
>>  		return 0;
>>  	case DYTC_FUNCTION_PSC:
>> -		switch (dytcmode) {
>> -		case DYTC_MODE_PSC_LOWPOWER:
>> +		if (dytcmode == platform_psc_profile_lowpower)
>>  			*profile = PLATFORM_PROFILE_LOW_POWER;
>> -			break;
>> -		case DYTC_MODE_PSC_BALANCE:
>> +		else if (dytcmode == platform_psc_profile_balanced)
>>  			*profile =  PLATFORM_PROFILE_BALANCED;
>> -			break;
>> -		case DYTC_MODE_PSC_PERFORM:
>> +		else if (dytcmode == platform_psc_profile_performance)
>>  			*profile =  PLATFORM_PROFILE_PERFORMANCE;
>> -			break;
>> -		default: /* Unknown mode */
>> +		else
>>  			return -EINVAL;
>> -		}
>>  		return 0;
>
> Maybe replace the removed '}' with an empty line instead of
> removing the entire line?
>
> Currently after your patch the new code looks like this:
>
> 			...
> 		else
> 			return -EINVAL;
> 		return 0;
>
> which look a bit weird, personally I would prefer:
>
> 			...
> 		else
> 			return -EINVAL;
>
> 		return 0;

Agreed - it does look oddly ugly doesn't it.

I'll wait and see if there is any other feedback, and then make that change for v2

>
> Otherwise this looks good to me:
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
Thanks
Mark

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.