Received-SPF: pass (google.com: domain of linux-kernel+bounces-171603-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <874746cc-5e61-4f3a-b621-3d0a60e50733@amd.com>
Date: Tue, 7 May 2024 09:44:00 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 01/11] cpufreq: amd-pstate: optimiza the initial frequency
 values verification
To: Perry Yuan <perry.yuan@amd.com>, rafael.j.wysocki@intel.com,
 viresh.kumar@linaro.org, Ray.Huang@amd.com, gautham.shenoy@amd.com,
 Borislav.Petkov@amd.com
Cc: Alexander.Deucher@amd.com, Xinmei.Huang@amd.com, Xiaojian.Du@amd.com,
 Li.Meng@amd.com, linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <cover.1715065568.git.perry.yuan@amd.com>
 <0049ad44052b051cf57d1059bf71b7ce227a5f21.1715065568.git.perry.yuan@amd.com>
Content-Language: en-US
From: Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <0049ad44052b051cf57d1059bf71b7ce227a5f21.1715065568.git.perry.yuan@amd.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?ekVPM3pncVdxenhweFgwQ0dER3dLNHBDMkZWV2xvSlN1VGw5aHAxaUkvaHRJ?=
 =?utf-8?B?MWN4NVBtMHVBS0FmS1Zhb29DS0FCekJMNGhjQXdzM0lYWllNcm51TDZpTGJu?=
 =?utf-8?B?MmU4REc2L3BRdkVSTm5GU3NPd205N3BVRC8rUDZ0MFVYRmpCWWZYemRyQWZj?=
 =?utf-8?B?RzdYKzRld3E0UXlIZ1pxOFhrTG9rSTgwMnM2K2Z3L2tmNFE3SmZmTXhkRTZ6?=
 =?utf-8?B?OUFvbVNVaXFZSFowaEprUDJCYS9qZDhNaXUzS2tBQXVRa05xZ1ZoT2hEbU9r?=
 =?utf-8?B?WXlJM3FxVkN2U2grVFBWdUJHRG1YTGhpcXQ2V3FxdVFydG1iTS9wc1hPL2lt?=
 =?utf-8?B?SEZFM253ZUdRei9ORnEzMXNKb0pmeEg0K3hBMUw5cnBOS0RwQ0xvM2FIcnBz?=
 =?utf-8?B?ckNWT3ZpUlJPRHNFbVViWG1HRHNjSFM5M0EvZy9aUDNJU2lYUi82VFJodGpo?=
 =?utf-8?B?UVA0M1NJekhCZkhISEliTnBhV2MranpsVytGMGt0VE9zbEF4aVRhQW9Xcm9p?=
 =?utf-8?B?YlFsNFBCRFZvVTdoT3hEdThLdUxaNFIvTDU4UGdNNzJxK3h3ZWtNckZyWmFn?=
 =?utf-8?B?OWpUSXFZR1psV0N6SFc4dGdzU2pXUkdsRittNGc2MjdRTjRaQm50NHAxTVZu?=
 =?utf-8?B?MWMzY2hMNmYvWitpbzdZQTZzakxLL3NtRk00Y0ZUYmZMc25wSmppZHNYOXdu?=
 =?utf-8?B?eFN5cmY4cWNnNnpVc3k2eTFjQVpQcElzd1N3clJKemhleFpJZXBvbXQ3cXdJ?=
 =?utf-8?B?Y1JPNGJ3bFEwTWkrWSs4OE5mZ1BGQ3lycUZlb2gwWHZ4MXBJT3FDTFZ4cWlP?=
 =?utf-8?B?WHptZ0FqU1VPdzJFVWR6cEkvN1JhRkhYOTY4UWxWSEI3TXNqV09WS3JXMldQ?=
 =?utf-8?B?WmlBbEZrTzNhcExVSHdTYW5yRHdwalNiRlFnVFpSRHYrT0Fwajd2bmNKZVdj?=
 =?utf-8?B?L3NmL2J6S215cnlTOGs2eXJhdmxzZXcxRmEwWXZjQWRJQ1dESWJXOVNPaEhi?=
 =?utf-8?B?dW00bGp4UngrQys0NkY2MVhkYjQrN3I4NURMaTdmZUFyTzNFR3FkZnBEVFRy?=
 =?utf-8?B?TlB2WVNpL0tUMUl3RDQwN1IzS0JCQ3JiT0loaGZ3SjljT2VuNDZqa3NzVEVh?=
 =?utf-8?B?Y1JEWVdKZithaG1mZlEwQkw4UnhuN21RbE1FTmNMcmhSTVFGcFFWYUNQMEtV?=
 =?utf-8?B?aEttRExCTnBCTXhuVUc5U0h1UWhlYUdIa3NCNWRhYlYrOCtCd3RGbnVDRGRO?=
 =?utf-8?B?bDNGaFd0clJFUnB4VERJR0lWL1ZvWWIybDB6MFp2RmFETXlZS2w4Wjc4YW4r?=
 =?utf-8?B?K2tyRVZ4b3h5NndWdkc1d1Q2V0daaFArWmJUajUveWV0K3Ric3VUb1phcUhO?=
 =?utf-8?B?aElmTXhEWmVkd1Q2TFRJaDFCSDJOT2ROMUFSRG5wNkFVSXBQRDVwUDhxSTFk?=
 =?utf-8?B?a3N5RzF6WG5oT3RNRHEwekdMMCtFTHJvckZRbDYzSFhtZ2VXMDlWZUZqTVk3?=
 =?utf-8?B?Mll5WjVvQ3lYMzd0ME1HMUgxaElHZUFoaVV1REZ4QmZNZFBoclRnKzgxdlRI?=
 =?utf-8?B?SnVuRVFVQjF4a1Z2ZW1yNllNai91MTg3K3I1Vlh6U3FpcUpiVFd1YlJMbmlO?=
 =?utf-8?B?c2RWS0xwUDR6cHBYUFNqRlZpQW84bGhtQmh5VlgyM2dWUEt0dUZzV0M0Q3pY?=
 =?utf-8?B?VUhzUHh5cjYvWmtpVWw4eVN5VCtOdTcveHFBYlhYQ3VzaVlVcXh6SEpwZ0xS?=
 =?utf-8?B?M2ZzWFpjdXZEb1JUTVlxRmt0RWtEMHc5UGIwS1JLM3hIdnYwdHQ5NEFsakEx?=
 =?utf-8?B?ZEIrS2hrUzZjRkh5em5tNU9ETFJpaUI3OFBtWDl6S282cEZWbzFZZFlnRU5x?=
 =?utf-8?B?T0Ruc3Fua2I5ZmdTZWFNMmcydWM4U1hCSEdYTEJWRDRnNzE4U2Q1bWl5Sjg5?=
 =?utf-8?B?VzJQZjBHaW1ETy93NVpkcjFoeHVGVnkxeDlobE1WcjI4a0w4L2Jxa0xDSE03?=
 =?utf-8?B?TExGWEcwOXdYbDlZYnppSW8xUHJ1UEdCU1BHRWluQUtoU3NELzI0NDR2aS9J?=
 =?utf-8?B?b2VrR1RQdFlVQjdEMCtUL1krZENDNm5HZVRlUWpiNzdLNVl6NHBiVmJOY2cv?=
 =?utf-8?Q?bEn1+kMGhKzGRT6/v9ulPwxYq?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e546245c-fc36-4983-8398-08dc6ea42365
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 May 2024 14:44:02.8965
 (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: TlEaMzDR0QJfQ8tvivQL9hZXSaDNU6CaTBzIMaSa6BkYL3NycY0hUfwa1+nmkdWNwlfOBviGjdmFWGt044cimQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR12MB6664

The title has a typo s/optimiza/optimize/

On 5/7/2024 02:15, Perry Yuan wrote:
> To enhance the debugging capability of the driver loading failure for
> broken CPPC ACPI tables, we can optimize the expression by moving the

Remove the "we" here.

> verification of `min_freq`, `nominal_freq`, and other dependency values
> to the `amd_pstate_init_freq()` function where they are initialized.
> If any of these values are incorrect, the `amd-pstate` driver will not be registered.
> 
> By ensuring that these values are correct before they are used, it will facilitate
> the debugging process when encountering driver loading failures due to faulty CPPC
> ACPI tables from BIOS
> 
> Signed-off-by: Perry Yuan <perry.yuan@amd.com>
> ---
>   drivers/cpufreq/amd-pstate.c | 35 ++++++++++++++++++-----------------
>   1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
> index 2db095867d03..be7f2d3c86b6 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -873,6 +873,24 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
>   	WRITE_ONCE(cpudata->nominal_freq, nominal_freq);
>   	WRITE_ONCE(cpudata->max_freq, max_freq);
>   
> +	/**
> +	 * Below values need to be initialized correctly, otherwise driver will be failed to load

"will fail to load"

> +	 * max_freq is calculated accoreding to (nominal_freq * highest_perf)/nominal_perf

according to

> +	 * lowest_nonlinear_freq is a value between [min_freq, nominal_freq]
> +	 * Check _CPC in ACPI table ojbects if any values are incorrect

objects

> +	 */
> +	if (min_freq <= 0 || max_freq <= 0 || nominal_freq <= 0 || min_freq > max_freq) {
> +		pr_err("min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect\n",
> +			min_freq, max_freq, nominal_freq);
> +		return -EINVAL;
> +	}
> +
> +	if (lowest_nonlinear_freq <= min_freq || lowest_nonlinear_freq > nominal_freq * 1000) {
> +		pr_err("lowest_nonlinear_freq(%d) value is out of range [min_freq(%d), nominal_freq(%d)]\n",
> +			lowest_nonlinear_freq, min_freq, nominal_freq * 1000);
> +		return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -911,15 +929,6 @@ static int amd_pstate_cpu_init(struct cpufreq_policy *policy)
>   	max_freq = READ_ONCE(cpudata->max_freq);
>   	nominal_freq = READ_ONCE(cpudata->nominal_freq);
>   
> -	if (min_freq <= 0 || max_freq <= 0 ||
> -	    nominal_freq <= 0 || min_freq > max_freq) {
> -		dev_err(dev,
> -			"min_freq(%d) or max_freq(%d) or nominal_freq (%d) value is incorrect, check _CPC in ACPI tables\n",
> -			min_freq, max_freq, nominal_freq);
> -		ret = -EINVAL;
> -		goto free_cpudata1;
> -	}
> -
>   	policy->cpuinfo.transition_latency = amd_pstate_get_transition_latency(policy->cpu);
>   	policy->transition_delay_us = amd_pstate_get_transition_delay_us(policy->cpu);
>   
> @@ -1372,14 +1381,6 @@ static int amd_pstate_epp_cpu_init(struct cpufreq_policy *policy)
>   	min_freq = READ_ONCE(cpudata->min_freq);
>   	max_freq = READ_ONCE(cpudata->max_freq);
>   	nominal_freq = READ_ONCE(cpudata->nominal_freq);
> -	if (min_freq <= 0 || max_freq <= 0 ||
> -	    nominal_freq <= 0 || min_freq > max_freq) {
> -		dev_err(dev,
> -			"min_freq(%d) or max_freq(%d) or nominal_freq(%d) value is incorrect, check _CPC in ACPI tables\n",
> -			min_freq, max_freq, nominal_freq);
> -		ret = -EINVAL;
> -		goto free_cpudata1;
> -	}
>   
>   	policy->cpuinfo.min_freq = min_freq;
>   	policy->cpuinfo.max_freq = max_freq;