| Message ID | 20240625134127.4464-2-Dhananjay.Ugwekar@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | AMD Pstate driver fixes | expand |
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. 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@amd.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 {
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@amd.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") Reviewed-by: Mario Limonciello <mario.limonciello@amd.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 {
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@amd.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 {
[AMD Official Use Only - AMD Internal Distribution Only] Hi Dhananjay: Sorry for the delay. I think the correction to this issue should be in function amd_pstae_init_freq() of file amd-pstate.c. The value of norminal_freq should be consistent with other values(max_freq,min_freq etc.). > -----Original Message----- > From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com> > Sent: Tuesday, June 25, 2024 9:52 PM > To: rafael@kernel.org; viresh.kumar@linaro.org; Shenoy, Gautham Ranjal > <gautham.shenoy@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>; > skhan@linuxfoundation.org; Meng, Li (Jassmine) <Li.Meng@amd.com>; > Huang, Ray <Ray.Huang@amd.com> > Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency > > 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. > > 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@amd.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 {
"Meng, Li (Jassmine)" <Li.Meng@amd.com> writes: Hello Jassmine, > > Hi Dhananjay: > Sorry for the delay. > > I think the correction to this issue should be in function amd_pstae_init_freq() of file amd-pstate.c. > The value of norminal_freq should be consistent with other > values(max_freq,min_freq etc.). Perry wanted nominal_freq to be in MHz since it is not exposed to the user via any of the cpufreq sysfs interfaces. IMO, it woyuld be good to to rename the variables to have their units for better readability along with a bunch of other cleanups/code deduplication. But can it be done separately ? > >> -----Original Message----- >> From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com> >> Sent: Tuesday, June 25, 2024 9:52 PM >> To: rafael@kernel.org; viresh.kumar@linaro.org; Shenoy, Gautham Ranjal >> <gautham.shenoy@amd.com>; Limonciello, Mario >> <Mario.Limonciello@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>; >> skhan@linuxfoundation.org; Meng, Li (Jassmine) <Li.Meng@amd.com>; >> Huang, Ray <Ray.Huang@amd.com> >> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency >> >> Minor modification, the commit subject is supposed to be >> "cpufreq/amd-pstate-ut: Handle the inconsistency between nominal_freq >> and other *_freq units" How about: "cpufreq/amd-pstate-ut: Convert nominal_freq to khz during comparisons" Otherwise the patch looks good to me. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham.
Hello Gautham, On 6/26/2024 10:52 AM, Gautham R.Shenoy wrote: > "Meng, Li (Jassmine)" <Li.Meng@amd.com> writes: > > Hello Jassmine, > >> >> Hi Dhananjay: >> Sorry for the delay. >> >> I think the correction to this issue should be in function amd_pstae_init_freq() of file amd-pstate.c. >> The value of norminal_freq should be consistent with other >> values(max_freq,min_freq etc.). > > Perry wanted nominal_freq to be in MHz since it is not exposed to the > user via any of the cpufreq sysfs interfaces. > > IMO, it woyuld be good to to rename the variables to have their units > for better readability along with a bunch of other > cleanups/code deduplication. But can it be done separately ? Yes that could be part of a separate cleanup patchset. > > >> >>> -----Original Message----- >>> From: Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com> >>> Sent: Tuesday, June 25, 2024 9:52 PM >>> To: rafael@kernel.org; viresh.kumar@linaro.org; Shenoy, Gautham Ranjal >>> <gautham.shenoy@amd.com>; Limonciello, Mario >>> <Mario.Limonciello@amd.com>; Yuan, Perry <Perry.Yuan@amd.com>; >>> skhan@linuxfoundation.org; Meng, Li (Jassmine) <Li.Meng@amd.com>; >>> Huang, Ray <Ray.Huang@amd.com> >>> Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org >>> Subject: Re: [PATCH 1/2] cpufreq/amd-pstate-ut: Handle the inconsistency >>> >>> Minor modification, the commit subject is supposed to be >>> "cpufreq/amd-pstate-ut: Handle the inconsistency between nominal_freq >>> and other *_freq units" > > How about: > > "cpufreq/amd-pstate-ut: Convert nominal_freq to khz during comparisons" Sure, this seems easier to understand and more concise. Will update in v2. > > Otherwise the patch looks good to me. > > Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> Thanks for reviewing! Regards, Dhananjay > > -- > Thanks and Regards > gautham. >
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@amd.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 { >
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@amd.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@amd.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 { >
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 {
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@amd.com> --- drivers/cpufreq/amd-pstate-ut.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)