[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e1132f04-9569-4f64-9b6a-96cd810ef5b8@amd.com>
Date: Wed, 26 Jun 2024 13:26:09 +0530
From: Dhananjay Ugwekar <Dhananjay.Ugwekar@....com>
To: Mario Limonciello <mario.limonciello@....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
Hello Mario,
On 6/25/2024 8:41 PM, Mario Limonciello wrote:
> 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?
Yup, will do!
Thanks,
Dhananjay
>
>>
>> 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