[PATCH] platform/x86: thinkpad_acpi: Explicitly set to balanced mode on startup

[PATCH] platform/x86: thinkpad_acpi: Explicitly set to balanced mode on startup

[Mario Limonciello]

It was observed that on a Thinkpad T14 Gen1 (AMD) that the platform
profile is starting up in 'low-power' mode after refreshing what the
firmware had.  This is most likely a firmware bug, but as a harmless
workaround set the default profile to 'balanced' at thinkpad_acpi startup.

Reported-and-tested-by: madcatx@atlas.cz
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216347
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/thinkpad_acpi.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 22d4e8633e30..e7e86c0b9ad7 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -10590,12 +10590,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
 		return -ENODEV;
 
 	/* Ensure initial values are correct */
-	dytc_profile_refresh();
-
-	/* Set AMT correctly now we know current profile */
-	if ((dytc_capabilities & BIT(DYTC_FC_PSC)) &&
-	    (dytc_capabilities & BIT(DYTC_FC_AMT)))
-	    dytc_control_amt(dytc_current_profile == PLATFORM_PROFILE_BALANCED);
+	dytc_profile_set(NULL, PLATFORM_PROFILE_BALANCED);
 
 	return 0;
 }
-- 
2.34.1

Re: [External] [PATCH] platform/x86: thinkpad_acpi: Explicitly set to balanced mode on startup

[Mark Pearson]

On 2022-08-19 10:01, Mario Limonciello wrote:
> It was observed that on a Thinkpad T14 Gen1 (AMD) that the platform
> profile is starting up in 'low-power' mode after refreshing what the
> firmware had.  This is most likely a firmware bug, but as a harmless
> workaround set the default profile to 'balanced' at thinkpad_acpi startup.
> 
> Reported-and-tested-by: madcatx@atlas.cz
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216347>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 22d4e8633e30..e7e86c0b9ad7 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -10590,12 +10590,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>  		return -ENODEV;
>  
>  	/* Ensure initial values are correct */
> -	dytc_profile_refresh();
> -
> -	/* Set AMT correctly now we know current profile */
> -	if ((dytc_capabilities & BIT(DYTC_FC_PSC)) &&
> -	    (dytc_capabilities & BIT(DYTC_FC_AMT)))
> -	    dytc_control_amt(dytc_current_profile == PLATFORM_PROFILE_BALANCED);
> +	dytc_profile_set(NULL, PLATFORM_PROFILE_BALANCED);
>  
>  	return 0;
>  }
I'm hesitant on this and would like some time to dig into it first.

I worry that this would be overriding the setting in the BIOS. On the
Intel platforms (at least on the mobile workstations) we can set the
default power setting in the BIOS. I don't see this on the T14 AMD G1 -
and haven't had a chance to check other platforms so its less of a
concern there.

As a compromise I'd want to force the profile to balanced on the PSC
modes only.

Ideally, if this is a FW bug we should get it fixed in FW. I know our FW
team can be a bit slow, but I'd rather hold off a few more days until I
have a better idea where the issue is. I don't really understand why the
person with the original issue is seeing the behaviour that they are.

Mark

Re: [External] [PATCH] platform/x86: thinkpad_acpi: Explicitly set to balanced mode on startup

[Limonciello, Mario]

On 8/19/2022 11:57, Mark Pearson wrote:
> 
> 
> On 2022-08-19 10:01, Mario Limonciello wrote:
>> It was observed that on a Thinkpad T14 Gen1 (AMD) that the platform
>> profile is starting up in 'low-power' mode after refreshing what the
>> firmware had.  This is most likely a firmware bug, but as a harmless
>> workaround set the default profile to 'balanced' at thinkpad_acpi startup.
>>
>> Reported-and-tested-by: madcatx@atlas.cz
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D216347&amp;data=05%7C01%7Cmario.limonciello%40amd.com%7C7f33822a8c2c428fdb0b08da8203f783%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637965250825035615%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=ua0qRTvx6MmngRCYI8TxgaTjLrd2fnRNtUhA0%2BRS6S8%3D&amp;reserved=0>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/platform/x86/thinkpad_acpi.c | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
>> index 22d4e8633e30..e7e86c0b9ad7 100644
>> --- a/drivers/platform/x86/thinkpad_acpi.c
>> +++ b/drivers/platform/x86/thinkpad_acpi.c
>> @@ -10590,12 +10590,7 @@ static int tpacpi_dytc_profile_init(struct ibm_init_struct *iibm)
>>   		return -ENODEV;
>>   
>>   	/* Ensure initial values are correct */
>> -	dytc_profile_refresh();
>> -
>> -	/* Set AMT correctly now we know current profile */
>> -	if ((dytc_capabilities & BIT(DYTC_FC_PSC)) &&
>> -	    (dytc_capabilities & BIT(DYTC_FC_AMT)))
>> -	    dytc_control_amt(dytc_current_profile == PLATFORM_PROFILE_BALANCED);
>> +	dytc_profile_set(NULL, PLATFORM_PROFILE_BALANCED);
>>   
>>   	return 0;
>>   }
> I'm hesitant on this and would like some time to dig into it first.
> 
> I worry that this would be overriding the setting in the BIOS. On the
> Intel platforms (at least on the mobile workstations) we can set the
> default power setting in the BIOS. I don't see this on the T14 AMD G1 -
> and haven't had a chance to check other platforms so its less of a
> concern there.

Ah wasn't aware you stored it for some systems.

> 
> As a compromise I'd want to force the profile to balanced on the PSC
> modes only.
> 
> Ideally, if this is a FW bug we should get it fixed in FW. I know our FW
> team can be a bit slow, but I'd rather hold off a few more days until I
> have a better idea where the issue is. I don't really understand why the
> person with the original issue is seeing the behaviour that they are.
> 
> Mark

Sure; I'll send out a v2 to that effect.