[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e9811d4e-60bb-49ea-80da-5d4597ef8705@amd.com>
Date: Tue, 25 Jun 2024 10:11:29 -0500
From: Mario Limonciello <mario.limonciello@....com>
To: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
rafael@...nel.org, viresh.kumar@...aro.org, gautham.shenoy@....com,
perry.yuan@....com, skhan@...uxfoundation.org, li.meng@....com,
ray.huang@....com
Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency
On 6/25/2024 08:51, Dhananjay Ugwekar wrote:
> Minor modification, the commit subject is supposed to be
> "cpufreq/amd-pstate-ut: Handle the inconsistency between nominal_freq and other *_freq units"
>
> The second half disappeared due to the word wrapping I guess.
I had some other feedback on the series, so when you submit a v2 can you
try to fix the title on the first patch?
>
> Regards,
> Dhananjay
>
> On 6/25/2024 7:11 PM, Dhananjay Ugwekar wrote:
>> cpudata->nominal_freq being in MHz whereas other frequencies being in
>> KHz breaks the amd-pstate-ut frequency sanity check. This fixes it.
>>
>> Fixes: 14eb1c96e3a3 ("cpufreq: amd-pstate: Add test module for amd-pstate driver")
>> Signed-off-by: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
>> ---
>> drivers/cpufreq/amd-pstate-ut.c | 12 +++++++-----
>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/cpufreq/amd-pstate-ut.c b/drivers/cpufreq/amd-pstate-ut.c
>> index fc275d41d51e..66b73c308ce6 100644
>> --- a/drivers/cpufreq/amd-pstate-ut.c
>> +++ b/drivers/cpufreq/amd-pstate-ut.c
>> @@ -202,6 +202,7 @@ static void amd_pstate_ut_check_freq(u32 index)
>> int cpu = 0;
>> struct cpufreq_policy *policy = NULL;
>> struct amd_cpudata *cpudata = NULL;
>> + u32 nominal_freq_khz;
>>
>> for_each_possible_cpu(cpu) {
>> policy = cpufreq_cpu_get(cpu);
>> @@ -209,13 +210,14 @@ static void amd_pstate_ut_check_freq(u32 index)
>> break;
>> cpudata = policy->driver_data;
>>
>> - if (!((cpudata->max_freq >= cpudata->nominal_freq) &&
>> - (cpudata->nominal_freq > cpudata->lowest_nonlinear_freq) &&
>> + nominal_freq_khz = cpudata->nominal_freq*1000;
>> + if (!((cpudata->max_freq >= nominal_freq_khz) &&
>> + (nominal_freq_khz > cpudata->lowest_nonlinear_freq) &&
>> (cpudata->lowest_nonlinear_freq > cpudata->min_freq) &&
>> (cpudata->min_freq > 0))) {
>> amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>> pr_err("%s cpu%d max=%d >= nominal=%d > lowest_nonlinear=%d > min=%d > 0, the formula is incorrect!\n",
>> - __func__, cpu, cpudata->max_freq, cpudata->nominal_freq,
>> + __func__, cpu, cpudata->max_freq, nominal_freq_khz,
>> cpudata->lowest_nonlinear_freq, cpudata->min_freq);
>> goto skip_test;
>> }
>> @@ -229,13 +231,13 @@ static void amd_pstate_ut_check_freq(u32 index)
>>
>> if (cpudata->boost_supported) {
>> if ((policy->max == cpudata->max_freq) ||
>> - (policy->max == cpudata->nominal_freq))
>> + (policy->max == nominal_freq_khz))
>> amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_PASS;
>> else {
>> amd_pstate_ut_cases[index].result = AMD_PSTATE_UT_RESULT_FAIL;
>> pr_err("%s cpu%d policy_max=%d should be equal cpu_max=%d or cpu_nominal=%d !\n",
>> __func__, cpu, policy->max, cpudata->max_freq,
>> - cpudata->nominal_freq);
>> + nominal_freq_khz);
>> goto skip_test;
>> }
>> } else {
Powered by blists - more mailing lists