[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7e1c6afa-3d9e-48a7-926b-39c368f1ef37@amd.com>
Date: Wed, 26 Jun 2024 13:32:34 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: Mario Limonciello <mario.limonciello@....com>, rafael@...nel.org,
viresh.kumar@...aro.org, gautham.shenoy@....com, perry.yuan@....com,
skhan@...uxfoundation.org, li.meng@....com, ray.huang@....com
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency
Hello Mario,
On 6/25/2024 8:35 PM, Mario Limonciello wrote:
> On 6/25/2024 08:41, 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>
>
> The code change below looks fine to me, but I think the tag is wrong. It should go with the "fix" that caused the inconsistency. Here is what
> I think the correct tag should be:
>
> Fixes: e4731baaf294 ("cpufreq: amd-pstate: Fix the inconsistency in max frequency units")
Makes sense, will update the tag.
Thanks for reviewing!
Regards,
Dhananjay
>
> Reviewed-by: Mario Limonciello <mario.limonciello@....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