Received-SPF: pass (google.com: domain of linux-kernel+bounces-75327-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <a92eb6e2-1636-4f80-8db9-23bbcf885337@amd.com>
Date: Wed, 21 Feb 2024 12:47:43 -0600
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] [PATCH] amd_pstate: fix erroneous highest_perf value on
 some CPUs
Content-Language: en-US
To: Lucas Lee Jing Yi <lucasleeeeeeeee@gmail.com>, oleksandr@natalenko.name
Cc: Perry.Yuan@amd.com, Xiaojian.Du@amd.com, alexander.deucher@amd.com,
 bp@alien8.de, deepak.sharma@amd.com, li.meng@amd.com,
 linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-kselftest@vger.kernel.org, linux-pm@vger.kernel.org,
 nathan.fontenot@amd.com, rafael.j.wysocki@intel.com, rafael@kernel.org,
 ray.huang@amd.com, shimmer.huang@amd.com, skhan@linuxfoundation.org,
 viresh.kumar@linaro.org, x86@kernel.org
References: <3868832.mvXUDI8C0e@natalenko.name>
 <20240221172404.99765-1-lucasleeeeeeeee@gmail.com>
 <20240221172404.99765-2-lucasleeeeeeeee@gmail.com>
From: Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20240221172404.99765-2-lucasleeeeeeeee@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?ZWdaRGFHdWZyWlZIUWdWUCtHajNVSTdkbFY4S2llRTR0YnZPWHFCR0lLQ2Vt?=
 =?utf-8?B?UXNtdTl0cG5qQlVLdmpkRWp6RUxLcVViazJhWFh4WXF3SDJUaFZZUklOcDJn?=
 =?utf-8?B?OTJ0S3h6TlJJYmFnTHNsazBCTlV2SE1uWVk2TVZ4OGJVQkV2dFVHeTdPY0c5?=
 =?utf-8?B?UkxWc3N4MUl3dlFXTkhsd1FLT1haakpRNXVmRitZOE8xRFQ2czRTVGV3Ymo5?=
 =?utf-8?B?RkhhZzJSb1N3MGlRVU5XZThoVHYyRXJoNWhMcEFmNEN1U2xhTnEvQnI2YlNB?=
 =?utf-8?B?MWczTUkyR0g1Q1gvV01RRi9YUzBvS2FaUjhXZS9vVEV6bDV4dTYyWVBjYW5C?=
 =?utf-8?B?NEpMQm10ak5PWmNpQ3pQOGZHTmw3NjdyZFdsQjRYVnJ6MG9jODlOT1c5dkNX?=
 =?utf-8?B?WEtNeGt6RTVNVGdUL2RzWnVwcEcrNDlqV2llTjRDS0l5MXZLQnU3cTN2aXlq?=
 =?utf-8?B?ZUpqMllTOEN0WXdPM0dIajE1MGZDaFZWOGhKT1F6N0tFQVgzalNjLzRwQUly?=
 =?utf-8?B?aTN6L3haQUZyRmd0M2FyOHJteDBFWjhSZHdhMDBXL2xZeDRkUlZ6TkUzcmF2?=
 =?utf-8?B?c0xTdHViU0hFbkJYaTJZcnV6WDhUUEdSVHFINkZXVjQwMUdMQUhteTNMUnRu?=
 =?utf-8?B?cUxCcWpzMGZibEdBSG13VmtRSlc1dkJ6cnc2QitrcUlld01FK3J4N0EvclNo?=
 =?utf-8?B?eS85WWNVdWJSbXhzZks5STJFdG45RFJZRmRPSDRvVTRnZE5qNVV5eTN1MUVY?=
 =?utf-8?B?RDh5QXZqaWRSNlBhTm1CSmVIczRmRm9FMzJCQTJucW9tdnBxaDViYmxCcVNx?=
 =?utf-8?B?Y2NDeUlYVUEzUWlScGdLZTVCKzZQZ1hWNEtFT1Y3OEZIM0xnYm4xS1ViMVRy?=
 =?utf-8?B?NEV0YzJCWVZOV0RQOUZrVnFjTVhDVFpmZ0hiOGVqbDdRQ25PTzROZ05wYXUy?=
 =?utf-8?B?K0hHMzI0SlVKZUlMbEMvRGh6VnF5SjRDVTY5bllEWmk4QlcwVU41ZFg3NmdZ?=
 =?utf-8?B?clZEWWYrdmlVUG0yOFRuOTJxNldvV2pHd1I4STdzandsMlQzd05sUloxWEVE?=
 =?utf-8?B?VGpNR1RQTGpXZVBNQTEyNllWNHFiNi8rdHJNNmlla1dOZ2NONnUya2s3cGNR?=
 =?utf-8?B?aHc1Q1dVTUtNQ2pUTGx1NDJwUEdUaHdWUjVyeit5QkdPWmRnd01MWXJUWTBO?=
 =?utf-8?B?U1dqN1FINmVjOUdETW5tUWxlN21xRG1QNHJscmowa25GRXFsZFhaTFFlUUsy?=
 =?utf-8?B?dll5SmY4TnovbFhybkZYYkc2MnBCMSswYkNnVXV5cEVUNVl3S0oyQzJCc0xD?=
 =?utf-8?B?MEYrdHlIaXErTGJDMWRWSFZFTnhFaXNzUTVSTCt0ZC9UNFYwOXdWcXhjdVFW?=
 =?utf-8?B?TVNvODZNcklrSTdjWXJycmpVaUh6bmoxSWMzMGN3KzJRYjM0d3pTQlUwN2Zy?=
 =?utf-8?B?dlc4RVhMVmZkNlpabGFVbXF1bnBGd1IxUmR2YkNFOThWV1hxZEtTOWl3cCtu?=
 =?utf-8?B?Ky9lUnJQcDRWZ3Q3VzJPZlh4bnpNYzVCb1JRcUNZZkNYVU1oTGVhQzNyZDJn?=
 =?utf-8?B?dTU1T0VLQ2lkQkJvaENEeDFhd1ZkRFc1MGtibzNYMHNTb096UVNraEwxWG9h?=
 =?utf-8?B?c1ZHanNRRURRVGE4bm0ydEZ3VkZaTlhJWXU2Qkc3S2g5Qi9BQVhBQlB6UDNw?=
 =?utf-8?B?eEpkSGlVVTlMK2JNYS9LL255cFgxUnd1cVZxZWRiamtGd0FLTmdIVS93Wklp?=
 =?utf-8?B?QmhqaW00bFVkS3BCT3RQU0RrNFBpenBkVU1sUFlQUDR1UkYvendPRHg1em9r?=
 =?utf-8?B?Q2lnNVFVaXdXL0ZBdys1MTFvN3V1SDNXUElWdmZHdDFBK2pFRnZVOTFKdUU4?=
 =?utf-8?B?MHJBa1RyNzA0WWxLbWJXbFhqZzBIbkFpMzZmY2dEOXdMcE9pR3BKZVVFa2Zx?=
 =?utf-8?B?OWtiTmZGeDhrQkFRRllvcG1Ua3VYQVNiOFBwbmI4OHFZTjd4Y09CK3VmRzJV?=
 =?utf-8?B?cVV2bUwyb0VXSWQrQ0svN0x5bndCSnhadFgweDZRNjlza1lxRE5EajlOUXda?=
 =?utf-8?B?TFZkRFpTdkxJQ09XcUQvOUZuMXc0K0MrS0IwV295MklPelVlRm5jdHBmZDdh?=
 =?utf-8?Q?glOd7upklYOl5IUQbrHDHcg5h?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0b9554d8-2e79-45ba-b4d3-08dc330d983c
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Feb 2024 18:47:46.3412
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cKnymOQsyxr5lg3AfUbGGYZ7uCSkGQOjPwRZLjO1N+JhihQmM4pfGYYh9+U2D2D+f6pb3qdEngYPbd/A7tO9sw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4413

On 2/21/2024 11:19, Lucas Lee Jing Yi wrote:
> On a Ryzen 7840HS the highest_perf value is 196, not 166 as AMD assumed.
> This leads to the advertised max clock speed to only be 4.35ghz
> instead of 5.14ghz leading to a large degradation in performance.
> 
> Fix the broken assumption and revert back to the old logic for
> getting highest_perf.
> 
> TEST:
> Geekbench 6 Before Patch:
> Single Core:	2325 (-22%)!
> Multi Core:	11335 (-10%)
> 
> Geekbench 6 AFTER Patch:
> Single Core:	2635
> Multi Core:	12487
> 

Yes; the max boost for your system should be 5.1GHz according to the 
specification [1].

Would you please open a kernel Bugzilla and attach an acpidump and dmesg 
for your system?  I believe we need to better understand your system's 
situation before deciding on how to correctly approach it.

[1] https://www.amd.com/en/product/13041

> Signed-off-by: Lucas Lee Jing Yi <lucasleeeeeeeee@gmail.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 22 ++++++++++------------
>   1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 08e112444c27..54df68773620 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -50,7 +50,6 @@
>   
>   #define AMD_PSTATE_TRANSITION_LATENCY	20000
>   #define AMD_PSTATE_TRANSITION_DELAY	1000
> -#define AMD_PSTATE_PREFCORE_THRESHOLD	166
>   
>   /*
>    * TODO: We need more time to fine tune processors with shared memory solution
> @@ -299,15 +298,12 @@ static int pstate_init_perf(struct amd_cpudata *cpudata)
>   				     &cap1);
>   	if (ret)
>   		return ret;
> -
> -	/* For platforms that do not support the preferred core feature, the
> -	 * highest_pef may be configured with 166 or 255, to avoid max frequency
> -	 * calculated wrongly. we take the AMD_CPPC_HIGHEST_PERF(cap1) value as
> -	 * the default max perf.
> +
> +	/* Some CPUs have different highest_perf from others, it is safer
> +	 * to read it than to assume some erroneous value, leading to performance issues.
>   	 */
> -	if (cpudata->hw_prefcore)
> -		highest_perf = AMD_PSTATE_PREFCORE_THRESHOLD;
> -	else
> +	highest_perf = amd_get_highest_perf();
> +	if (highest_perf > AMD_CPPC_HIGHEST_PERF(cap1))
>   		highest_perf = AMD_CPPC_HIGHEST_PERF(cap1);
>   
>   	WRITE_ONCE(cpudata->highest_perf, highest_perf);
> @@ -329,9 +325,11 @@ static int cppc_init_perf(struct amd_cpudata *cpudata)
>   	if (ret)
>   		return ret;
>   
> -	if (cpudata->hw_prefcore)
> -		highest_perf = AMD_PSTATE_PREFCORE_THRESHOLD;
> -	else
> +	/* Some CPUs have different highest_perf from others, it is safer
> +	 * to read it than to assume some erroneous value, leading to performance issues.
> +	 */
> +	highest_perf = amd_get_highest_perf();
> +	if (highest_perf > cppc_perf.highest_perf)
>   		highest_perf = cppc_perf.highest_perf;
>   
>   	WRITE_ONCE(cpudata->highest_perf, highest_perf);