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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
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;
