Hi Mario,
On Fri, 20 Dec 2024 at 01:40, Naresh Solanki
<naresh.solanki@xxxxxxxxxxxxx> wrote:
Sorry I was wrong. it's not zero. Its roundoff version.
Hi Mario,
On Fri, 20 Dec 2024 at 01:02, Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:
Sure. Somehow my eyes missed that.
On 12/19/2024 13:21, Naresh Solanki wrote:
The previous approach introduced roundoff errors during division when
calculating the boost ratio. This, in turn, affected the maximum
frequency calculation, often resulting in reporting lower frequency
values.
For example, on the Glinda SoC based board with the following
parameters:
max_perf = 208
nominal_perf = 100
nominal_freq = 2600 MHz
The Linux kernel previously calculated the frequency as:
freq = ((max_perf * 1024 / nominal_perf) * nominal_freq) / 1024
freq = 5405 MHz // Integer arithmetic.
With the updated formula:
freq = (max_perf * nominal_freq) / nominal_perf
freq = 5408 MHz
This change ensures more accurate frequency calculations by eliminating
unnecessary shifts and divisions, thereby improving precision.
Signed-off-by: Naresh Solanki <naresh.solanki@xxxxxxxxxxxxx>
Thanks, this makes sense to me.
But looking at it, we should have the same problem with lowest nonlinear
freq as it goes through the same conversion process. Would you mind
fixing that one too?
Also observed that current calculations yields zero for lowest_nonlinear_freq.
After fixing that, it reported frequency 2002 & 1404 depending on the core.
On a side note, I'm also observing that the highest_perf is set to 196 which
should not have been the case as I do have cores with value 208.
Seems like amd_get_boost_ratio_numerator needs some addressing for that.
Regards,
Naresh
Gautham, Perry,
Is there something non-obvious I'm missing about why it was done this
way? It looks like it's been there since the start.
Changes in V2:
1. Rebase on superm1.git/linux-next branch
---
drivers/cpufreq/amd-pstate.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index d7b1de97727a..02a851f93fd6 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -908,9 +908,9 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
{
int ret;
u32 min_freq, max_freq;
- u32 nominal_perf, nominal_freq;
+ u32 highest_perf, nominal_perf, nominal_freq;
u32 lowest_nonlinear_perf, lowest_nonlinear_freq;
- u32 boost_ratio, lowest_nonlinear_ratio;
+ u32 lowest_nonlinear_ratio;
struct cppc_perf_caps cppc_perf;
ret = cppc_get_perf_caps(cpudata->cpu, &cppc_perf);
@@ -927,10 +927,9 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata)
else
nominal_freq = cppc_perf.nominal_freq;
+ highest_perf = READ_ONCE(cpudata->highest_perf);
nominal_perf = READ_ONCE(cpudata->nominal_perf);
-
- boost_ratio = div_u64(cpudata->highest_perf << SCHED_CAPACITY_SHIFT, nominal_perf);
- max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT);
+ max_freq = div_u64((u64)highest_perf * nominal_freq, nominal_perf);
lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf);
lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT,