| Message ID | 20241012174519.897-1-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | [1/4] cpufreq/amd-pstate: Use nominal perf for limits when boost is disabled | expand |
Hello Mario, On Sat, Oct 12, 2024 at 12:45:16PM -0500, Mario Limonciello wrote: > When boost has been disabled the limit for perf should be nominal perf not > the highest perf. Using the latter to do calculations will lead to > incorrect values that are still above nominal. > > Fixes: ad4caad58d91 ("cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()") > Reported-by: Peter Jung <ptr1337@cachyos.org> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219348 > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> The patch looks good to me. Reviewed-by: Gautham R. Shenoy <gautham.shenoy@amd.com> -- Thanks and Regards gautham. > --- > drivers/cpufreq/amd-pstate.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index 30415c30d8b4..dfa9a146769b 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -536,11 +536,16 @@ static int amd_pstate_verify(struct cpufreq_policy_data *policy) > > static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy) > { > - u32 max_limit_perf, min_limit_perf, lowest_perf; > + u32 max_limit_perf, min_limit_perf, lowest_perf, max_perf; > struct amd_cpudata *cpudata = policy->driver_data; > > - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq); > - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq); > + if (cpudata->boost_supported && !policy->boost_enabled) > + max_perf = READ_ONCE(cpudata->nominal_perf); > + else > + max_perf = READ_ONCE(cpudata->highest_perf); > + > + max_limit_perf = div_u64(policy->max * max_perf, policy->cpuinfo.max_freq); > + min_limit_perf = div_u64(policy->min * max_perf, policy->cpuinfo.max_freq); > > lowest_perf = READ_ONCE(cpudata->lowest_perf); > if (min_limit_perf < lowest_perf) > @@ -1506,10 +1511,13 @@ static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy) > u64 value; > s16 epp; > > - max_perf = READ_ONCE(cpudata->highest_perf); > + if (cpudata->boost_supported && !policy->boost_enabled) > + max_perf = READ_ONCE(cpudata->nominal_perf); > + else > + max_perf = READ_ONCE(cpudata->highest_perf); > min_perf = READ_ONCE(cpudata->lowest_perf); > - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq); > - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq); > + max_limit_perf = div_u64(policy->max * max_perf, policy->cpuinfo.max_freq); > + min_limit_perf = div_u64(policy->min * max_perf, policy->cpuinfo.max_freq); > > if (min_limit_perf < min_perf) > min_limit_perf = min_perf; > -- > 2.43.0 >
[AMD Official Use Only - AMD Internal Distribution Only] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@amd.com> > Sent: Sunday, October 13, 2024 1:45 AM > To: Shenoy, Gautham Ranjal <gautham.shenoy@amd.com> > Cc: Yuan, Perry <Perry.Yuan@amd.com>; linux-kernel@vger.kernel.org; linux- > pm@vger.kernel.org; Ugwekar, Dhananjay <Dhananjay.Ugwekar@amd.com>; > Limonciello, Mario <Mario.Limonciello@amd.com>; Peter Jung > <ptr1337@cachyos.org> > Subject: [PATCH 1/4] cpufreq/amd-pstate: Use nominal perf for limits when boost is > disabled > > When boost has been disabled the limit for perf should be nominal perf not the > highest perf. Using the latter to do calculations will lead to incorrect values that are > still above nominal. > > Fixes: ad4caad58d91 ("cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() > into amd_get_boost_ratio_numerator()") > Reported-by: Peter Jung <ptr1337@cachyos.org> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219348 > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > drivers/cpufreq/amd-pstate.c | 20 ++++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index > 30415c30d8b4..dfa9a146769b 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -536,11 +536,16 @@ static int amd_pstate_verify(struct cpufreq_policy_data > *policy) > > static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy) { > - u32 max_limit_perf, min_limit_perf, lowest_perf; > + u32 max_limit_perf, min_limit_perf, lowest_perf, max_perf; > struct amd_cpudata *cpudata = policy->driver_data; > > - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata- > >max_freq); > - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata- > >max_freq); > + if (cpudata->boost_supported && !policy->boost_enabled) > + max_perf = READ_ONCE(cpudata->nominal_perf); > + else > + max_perf = READ_ONCE(cpudata->highest_perf); > + > + max_limit_perf = div_u64(policy->max * max_perf, policy- > >cpuinfo.max_freq); > + min_limit_perf = div_u64(policy->min * max_perf, > +policy->cpuinfo.max_freq); > > lowest_perf = READ_ONCE(cpudata->lowest_perf); > if (min_limit_perf < lowest_perf) > @@ -1506,10 +1511,13 @@ static int amd_pstate_epp_update_limit(struct > cpufreq_policy *policy) > u64 value; > s16 epp; > > - max_perf = READ_ONCE(cpudata->highest_perf); > + if (cpudata->boost_supported && !policy->boost_enabled) > + max_perf = READ_ONCE(cpudata->nominal_perf); > + else > + max_perf = READ_ONCE(cpudata->highest_perf); > min_perf = READ_ONCE(cpudata->lowest_perf); > - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata- > >max_freq); > - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata- > >max_freq); > + max_limit_perf = div_u64(policy->max * max_perf, policy- > >cpuinfo.max_freq); > + min_limit_perf = div_u64(policy->min * max_perf, > +policy->cpuinfo.max_freq); > > if (min_limit_perf < min_perf) > min_limit_perf = min_perf; > -- > 2.43.0 LGTM, thanks for the fix. Reviewed-by: Perry Yuan <perry.yuan@amd.com>
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index 30415c30d8b4..dfa9a146769b 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -536,11 +536,16 @@ static int amd_pstate_verify(struct cpufreq_policy_data *policy) static int amd_pstate_update_min_max_limit(struct cpufreq_policy *policy) { - u32 max_limit_perf, min_limit_perf, lowest_perf; + u32 max_limit_perf, min_limit_perf, lowest_perf, max_perf; struct amd_cpudata *cpudata = policy->driver_data; - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq); - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq); + if (cpudata->boost_supported && !policy->boost_enabled) + max_perf = READ_ONCE(cpudata->nominal_perf); + else + max_perf = READ_ONCE(cpudata->highest_perf); + + max_limit_perf = div_u64(policy->max * max_perf, policy->cpuinfo.max_freq); + min_limit_perf = div_u64(policy->min * max_perf, policy->cpuinfo.max_freq); lowest_perf = READ_ONCE(cpudata->lowest_perf); if (min_limit_perf < lowest_perf) @@ -1506,10 +1511,13 @@ static int amd_pstate_epp_update_limit(struct cpufreq_policy *policy) u64 value; s16 epp; - max_perf = READ_ONCE(cpudata->highest_perf); + if (cpudata->boost_supported && !policy->boost_enabled) + max_perf = READ_ONCE(cpudata->nominal_perf); + else + max_perf = READ_ONCE(cpudata->highest_perf); min_perf = READ_ONCE(cpudata->lowest_perf); - max_limit_perf = div_u64(policy->max * cpudata->highest_perf, cpudata->max_freq); - min_limit_perf = div_u64(policy->min * cpudata->highest_perf, cpudata->max_freq); + max_limit_perf = div_u64(policy->max * max_perf, policy->cpuinfo.max_freq); + min_limit_perf = div_u64(policy->min * max_perf, policy->cpuinfo.max_freq); if (min_limit_perf < min_perf) min_limit_perf = min_perf;
When boost has been disabled the limit for perf should be nominal perf not the highest perf. Using the latter to do calculations will lead to incorrect values that are still above nominal. Fixes: ad4caad58d91 ("cpufreq: amd-pstate: Merge amd_pstate_highest_perf_set() into amd_get_boost_ratio_numerator()") Reported-by: Peter Jung <ptr1337@cachyos.org> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219348 Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- drivers/cpufreq/amd-pstate.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)