Received-SPF: pass (google.com: domain of linux-kernel+bounces-210468-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <37e3477c-db6c-4fa2-9799-f23a0f37d20c@amd.com>
Date: Tue, 11 Jun 2024 13:50:15 -0500
User-Agent: Mozilla Thunderbird
From: Mario Limonciello <mario.limonciello@amd.com>
Subject: Re: [PATCH v3 01/10] cpufreq: amd-pstate: optimize the initial
 frequency values verification
To: Perry Yuan <perry.yuan@amd.com>, gautham.shenoy@amd.com
Cc: rafael.j.wysocki@intel.com, viresh.kumar@linaro.org, Ray.Huang@amd.com,
 Borislav.Petkov@amd.com, 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.1718095377.git.perry.yuan@amd.com>
 <c3e867be691b6fdb3d34e378199b297256aeebff.1718095377.git.perry.yuan@amd.com>
Content-Language: en-US
In-Reply-To: <c3e867be691b6fdb3d34e378199b297256aeebff.1718095377.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?VXoxSVpnbTNHcFJDV2pVL24wRnU4eHZHYnNPT29pS2lBcHFpSU5tK0dZUktu?=
 =?utf-8?B?citxSlhidVJnMmdwSm9UQWVlYy9JamU5N0NYY29CSG9wN1BrcVo0ZUJCSXVq?=
 =?utf-8?B?MXNXb3BaSWd3bEd3S0JuakpUUTB0Y05rODRMZXVpZHVGdmlHeWF1ZDV2YXgw?=
 =?utf-8?B?OWkvYjRzdlUwYzdGN2tOa3RIc3hGRzNJOU5NNlFwdks3TFBUdTRFV29OcE82?=
 =?utf-8?B?dklDVWE3YWJidG4yTGt3UjBwT0h3eHpOWC9BTlhBM0ZLSW9rdDJBVVBESVBP?=
 =?utf-8?B?UDFXaERTS1p6NXh2SkRRUU56L0l2ZGYvQzZERUVXWmsvWUZSYklITXpJLytR?=
 =?utf-8?B?NG1NYUFMZ1p3ZmZFNHdJYys4YVplY0hoRFlRTTJEREx0N2czamFRSjE0UnFp?=
 =?utf-8?B?U2FoeFhhbWRSMXlDdEZnbkdaR0FqR1V3d21GSDFSclpxWTBrZXZjK1dWdG5M?=
 =?utf-8?B?WU5UZkpEaU9WQVN6TDhuUnFPcWRGNm9weTZXN0RSbGFJcVdkQjNaMVFWVTNy?=
 =?utf-8?B?eUIwbTNLenBVdzhtY2gwd002VjdyK2Vob2VmMTdYOHdkOU1ZcXZKWWJTaXU4?=
 =?utf-8?B?dEorcTZFbDkzWFlqZGpmUGdUY3V5Z2g2TmhJS1JNeXJzZ2ZVU2tKbFFOTG9u?=
 =?utf-8?B?UzZmZnJYYWFBNTVKZ0RPY25wQlBtZ0JsNmM2dGlscDIxNHFXL0M5Q2xYR0VK?=
 =?utf-8?B?NjNNa1c5Q0M4azhiZ0pDbHdWNkRGamdDMlNHdUZheTdxN2ZhcHVVUzI0ekRL?=
 =?utf-8?B?MXBNQlEzVGxoZkhKZGR1M01qT2dXOHduakw3bVVHVFM4RjVHZkNScDdsOVpl?=
 =?utf-8?B?ekROby9Gd3pVSFJPamdyeDJLVXUwQTVvTkt4UUdqbEtiRzRCNW1oYWNUczJz?=
 =?utf-8?B?dnBqMm1JWVpackhOQ3B2NUVvdDRibEZ2UG5wTHNIOHcvcUhhMEpLZlFCVkhU?=
 =?utf-8?B?R3JVMTF4U1IwRWp6d2tDUFQycmlNWlBVTitoS2tOMGxkWjRIaUN0NURZb2hh?=
 =?utf-8?B?WVcyQnBUQXhsSVFMdlpVZXR6TTZUQ0RhOGVHVlJ1RG9oZzhQMzEzU0VSNGdM?=
 =?utf-8?B?RkxkY3pRWW1yZzBUSFFIUWRKMmQvWm4wcVZiQmhlYjRyY0ptb0RkT1N0Uzhv?=
 =?utf-8?B?SGphWUxjZDl4YU53bVNsTFRCR0hzczZNZDJLKzlrNUFyZFNLUE1pd3JZYTN5?=
 =?utf-8?B?UEVoQUo2YnBFVTA1alRBam9kSDBhZFpEVm1iMTlzaldMeGNkV2ZoS2crV2tJ?=
 =?utf-8?B?bWt4QXdvMi9BYnhDQ0xzUGhEL0FyZy95bW5SeVZRSlo4Z05zZnkxRzFKTlJz?=
 =?utf-8?B?em15Q3VRTEd5YjhEQVQxK1pLVTN1cFg2RWR6REcxcU10S1NuSmExcVNBVnJC?=
 =?utf-8?B?UjVTTVdweGxhbm43V3ZBUzVnOXdwOXJvRjZCTll2aDRFaC9ERFdmczZSNDY2?=
 =?utf-8?B?QjFNam1Cck5FVVVjZWdaYWZiUHhiZU9zeCsyOFVqcHl6cFJWRGZTSzI2dEdj?=
 =?utf-8?B?RHo1VHdKWWJJcjRQdTZVNk1MS2wrM1k5WHphUlNoZVp4QXVINnNNY25tVXZF?=
 =?utf-8?B?ck9sdXNkcVZHTWpZYm5kMzhQaml4cURMenM5cWJtaXRkVHd3RmRia1ZZSzdl?=
 =?utf-8?B?QmFVQ1JpMGpaRzl0aGRGTDh2b0xad0xJclI4Nmw0RThGOVdqb0JJMHBNbnBQ?=
 =?utf-8?B?ZlJpOTBqVVR3ZmgxNTZpb1dqcnY5bk0rbEJaNldYTUVhRzZ6amFuVkMyY29W?=
 =?utf-8?B?UW9HOThUaElrVFBZK21Zd3phL0FKaUZ3dGtGRTdVMnBwVjlTMFhTWDZuTXdE?=
 =?utf-8?B?WGhRZVpKcmpkU25yTmVhb21OeVJIa0M3ei83SS93dEdpall4cTA5c09NOGIy?=
 =?utf-8?B?SU5HZDdmd3NTYVExVDVSMzV0aG92R3NPVW1LMFJMRG5FSTJnS29IeHdERmZh?=
 =?utf-8?B?cTRna0VPTjBHSU1NTXJReFBORXBuNUJBaXE0NmJWUUpZZkVWVkpGOGJyS1Vp?=
 =?utf-8?B?S3gwRHE2cEdBN0gwWnNvT2FYT0wwN05LeEoxL3loVkhSTWUyOENQekY4c3Jw?=
 =?utf-8?B?Qi9pZXR1ZUNLOXU5UzNNNDB3NlFGKzh5M2pGRFl3VlVBcmZpRDFuSGxrMkFT?=
 =?utf-8?Q?3579Zvq14lQNQdwHlwpH2v1ec?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: fae03404-f81a-4178-3aa3-08dc8a47567b
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Jun 2024 18:50:17.9632
 (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: DaEBA7KCqX97Ebmj8MmpuR6krvA0YpubwPpMHe09L/h4GjhTFHcvQXCb77ZqINcULxP1DuYhIj232S4fR+YIpg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR12MB6551

On 6/11/2024 03:52, Perry Yuan wrote:
> To enhance the debugging capability of the driver loading failure for
> broken CPPC ACPI tables, it can optimize the expression by moving the
> 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>
> Acked-by: Gautham R. Shenoy <gautham.shenoy@amd.com>

Acked-by: Mario Limonciello <mario.limonciello@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 9ad62dbe8bfb..37fce0569d06 100644
> --- a/drivers/cpufreq/amd-pstate.c
> +++ b/drivers/cpufreq/amd-pstate.c
> @@ -921,6 +921,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 fail to load
> +	 * max_freq is calculated according to (nominal_freq * highest_perf)/nominal_perf
> +	 * lowest_nonlinear_freq is a value between [min_freq, nominal_freq]
> +	 * Check _CPC in ACPI table objects if any values are incorrect
> +	 */
> +	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 * 1000);
> +		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;
>   }
>   
> @@ -959,15 +977,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);
>   
> @@ -1420,14 +1429,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;