diff mbox series

[v2] platform/x86: thinkpad_acpi: Fix profile mode display in AMT mode

Message ID 20230112221228.490946-1-mpearson-lenovo@squebb.ca (mailing list archive)
State Accepted, archived
Headers show
Series [v2] platform/x86: thinkpad_acpi: Fix profile mode display in AMT mode | expand

Commit Message

Mark Pearson Jan. 12, 2023, 10:12 p.m. UTC 
Recently AMT mode was enabled (somewhat unexpectedly) on the Lenovo
Z13 platform. The FW is advertising it is available and the driver tries
to use it - unfortunately it reports the profile mode incorrectly.

Note, there is also some extra work needed to enable the dynamic aspect
of AMT support that I will be following up with; but more testing is
needed first. This patch just fixes things so the profiles are reported
correctly.

Link: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/115
Fixes: 46dcbc61b739 ("platform/x86: thinkpad-acpi: Add support for automatic mode transitions")

Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>
---
v2: Corrected title and added link and fixes details

 drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Hans de Goede Jan. 13, 2023, 10:43 a.m. UTC | #1 
Hi,

On 1/12/23 23:12, Mark Pearson wrote:
> Recently AMT mode was enabled (somewhat unexpectedly) on the Lenovo
> Z13 platform. The FW is advertising it is available and the driver tries
> to use it - unfortunately it reports the profile mode incorrectly.
> 
> Note, there is also some extra work needed to enable the dynamic aspect
> of AMT support that I will be following up with; but more testing is
> needed first. This patch just fixes things so the profiles are reported
> correctly.
> 
> Link: https://gitlab.freedesktop.org/hadess/power-profiles-daemon/-/issues/115
> Fixes: 46dcbc61b739 ("platform/x86: thinkpad-acpi: Add support for automatic mode transitions")
> 
> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Mark Pearson <mpearson-lenovo@squebb.ca>

Thank you for your patch, I've applied this patch to my fixes
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes

Note it will show up in my fixes branch once I've pushed my
local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans



> ---
> v2: Corrected title and added link and fixes details
> 
>  drivers/platform/x86/thinkpad_acpi.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 1195293b22fd..a95946800ae9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10311,9 +10311,11 @@ static DEFINE_MUTEX(dytc_mutex);
>  static int dytc_capabilities;
>  static bool dytc_mmc_get_available;
>  
> -static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
> +static int convert_dytc_to_profile(int funcmode, int dytcmode,
> +		enum platform_profile_option *profile)
>  {
> -	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
> +	switch (funcmode) {
> +	case DYTC_FUNCTION_MMC:
>  		switch (dytcmode) {
>  		case DYTC_MODE_MMC_LOWPOWER:
>  			*profile = PLATFORM_PROFILE_LOW_POWER;
> @@ -10329,8 +10331,7 @@ static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *p
>  			return -EINVAL;
>  		}
>  		return 0;
> -	}
> -	if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
> +	case DYTC_FUNCTION_PSC:
>  		switch (dytcmode) {
>  		case DYTC_MODE_PSC_LOWPOWER:
>  			*profile = PLATFORM_PROFILE_LOW_POWER;
> @@ -10344,6 +10345,14 @@ static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *p
>  		default: /* Unknown mode */
>  			return -EINVAL;
>  		}
> +		return 0;
> +	case DYTC_FUNCTION_AMT:
> +		/* For now return balanced. It's the closest we have to 'auto' */
> +		*profile =  PLATFORM_PROFILE_BALANCED;
> +		return 0;
> +	default:
> +		/* Unknown function */
> +		return -EOPNOTSUPP;
>  	}
>  	return 0;
>  }
> @@ -10492,6 +10501,7 @@ static int dytc_profile_set(struct platform_profile_handler *pprof,
>  		err = dytc_command(DYTC_SET_COMMAND(DYTC_FUNCTION_PSC, perfmode, 1), &output);
>  		if (err)
>  			goto unlock;
> +
>  		/* system supports AMT, activate it when on balanced */
>  		if (dytc_capabilities & BIT(DYTC_FC_AMT))
>  			dytc_control_amt(profile == PLATFORM_PROFILE_BALANCED);
> @@ -10507,7 +10517,7 @@ static void dytc_profile_refresh(void)
>  {
>  	enum platform_profile_option profile;
>  	int output, err = 0;
> -	int perfmode;
> +	int perfmode, funcmode;
>  
>  	mutex_lock(&dytc_mutex);
>  	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
> @@ -10522,8 +10532,9 @@ static void dytc_profile_refresh(void)
>  	if (err)
>  		return;
>  
> +	funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
>  	perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
> -	convert_dytc_to_profile(perfmode, &profile);
> +	convert_dytc_to_profile(funcmode, perfmode, &profile);
>  	if (profile != dytc_current_profile) {
>  		dytc_current_profile = profile;
>  		platform_profile_notify();
diff mbox series

Patch

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 1195293b22fd..a95946800ae9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10311,9 +10311,11 @@  static DEFINE_MUTEX(dytc_mutex);
 static int dytc_capabilities;
 static bool dytc_mmc_get_available;
 
-static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *profile)
+static int convert_dytc_to_profile(int funcmode, int dytcmode,
+		enum platform_profile_option *profile)
 {
-	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
+	switch (funcmode) {
+	case DYTC_FUNCTION_MMC:
 		switch (dytcmode) {
 		case DYTC_MODE_MMC_LOWPOWER:
 			*profile = PLATFORM_PROFILE_LOW_POWER;
@@ -10329,8 +10331,7 @@  static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *p
 			return -EINVAL;
 		}
 		return 0;
-	}
-	if (dytc_capabilities & BIT(DYTC_FC_PSC)) {
+	case DYTC_FUNCTION_PSC:
 		switch (dytcmode) {
 		case DYTC_MODE_PSC_LOWPOWER:
 			*profile = PLATFORM_PROFILE_LOW_POWER;
@@ -10344,6 +10345,14 @@  static int convert_dytc_to_profile(int dytcmode, enum platform_profile_option *p
 		default: /* Unknown mode */
 			return -EINVAL;
 		}
+		return 0;
+	case DYTC_FUNCTION_AMT:
+		/* For now return balanced. It's the closest we have to 'auto' */
+		*profile =  PLATFORM_PROFILE_BALANCED;
+		return 0;
+	default:
+		/* Unknown function */
+		return -EOPNOTSUPP;
 	}
 	return 0;
 }
@@ -10492,6 +10501,7 @@  static int dytc_profile_set(struct platform_profile_handler *pprof,
 		err = dytc_command(DYTC_SET_COMMAND(DYTC_FUNCTION_PSC, perfmode, 1), &output);
 		if (err)
 			goto unlock;
+
 		/* system supports AMT, activate it when on balanced */
 		if (dytc_capabilities & BIT(DYTC_FC_AMT))
 			dytc_control_amt(profile == PLATFORM_PROFILE_BALANCED);
@@ -10507,7 +10517,7 @@  static void dytc_profile_refresh(void)
 {
 	enum platform_profile_option profile;
 	int output, err = 0;
-	int perfmode;
+	int perfmode, funcmode;
 
 	mutex_lock(&dytc_mutex);
 	if (dytc_capabilities & BIT(DYTC_FC_MMC)) {
@@ -10522,8 +10532,9 @@  static void dytc_profile_refresh(void)
 	if (err)
 		return;
 
+	funcmode = (output >> DYTC_GET_FUNCTION_BIT) & 0xF;
 	perfmode = (output >> DYTC_GET_MODE_BIT) & 0xF;
-	convert_dytc_to_profile(perfmode, &profile);
+	convert_dytc_to_profile(funcmode, perfmode, &profile);
 	if (profile != dytc_current_profile) {
 		dytc_current_profile = profile;
 		platform_profile_notify();