| Message ID | 20241218190030.1228868-1-naresh.solanki@9elements.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Mario Limonciello |
| Headers | show |
| Series | cpufreq/amd-pstate: Refactor max frequency calculation | expand |
On 12/18/2024 13:00, Naresh Solanki wrote: > Refactor to calculate max-freq more accurately. Can you add some more detail about what you're finding? What was it before, what is it now, why is it more accurate? > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > --- > drivers/cpufreq/amd-pstate.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > index d7630bab2516..78a2cbd14952 100644 > --- a/drivers/cpufreq/amd-pstate.c > +++ b/drivers/cpufreq/amd-pstate.c > @@ -892,7 +892,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > u64 numerator; > u32 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); > @@ -914,8 +914,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator); > if (ret) > return ret; > - boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf); > - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000; > + max_freq = div_u64(numerator * nominal_freq * 1000, nominal_perf); This doesn't apply currently, because of some changes in the superm1.git/linux-next branch; specifically: https://git.kernel.org/superm1/c/68cb0e77b6439 I haven't sent this out to linux-pm yet so it could be in linux-next, but will be doing that soon. So can you please rebase on that branch if this change still makes sense? > > lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf); > lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT,
Hi , On Thu, 19 Dec 2024 at 00:50, Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 12/18/2024 13:00, Naresh Solanki wrote: > > Refactor to calculate max-freq more accurately. > > Can you add some more detail about what you're finding? > What was it before, what is it now, why is it more accurate? Sure. The previous approach had some roundoff error due to division compute when calculating boost ratio. Which intern affected max-freq calculation resulting in reporting lower value In my Glinda SoC board, See below calculations with below values: max_perf = 208 nominal_perf = 100 normal_freq = 2600 How linux kernel calculates: freq = ( (max_perf * 1024 / nominal_perf) * normal_freq) / 1024 freq = 5405 // Integer arithmatic. With the changes freq = (max_perf * normal_freq) / nominal_perf freq = 5408 > > > > > Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > > --- > > drivers/cpufreq/amd-pstate.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > > index d7630bab2516..78a2cbd14952 100644 > > --- a/drivers/cpufreq/amd-pstate.c > > +++ b/drivers/cpufreq/amd-pstate.c > > @@ -892,7 +892,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > > u64 numerator; > > u32 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); > > @@ -914,8 +914,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > > ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator); > > if (ret) > > return ret; > > - boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf); > > - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000; > > + max_freq = div_u64(numerator * nominal_freq * 1000, nominal_perf); > > This doesn't apply currently, because of some changes in the > superm1.git/linux-next branch; specifically: > > https://git.kernel.org/superm1/c/68cb0e77b6439 > > I haven't sent this out to linux-pm yet so it could be in linux-next, > but will be doing that soon. So can you please rebase on that branch if > this change still makes sense? Sure. Will do Regards, Naresh Solanki > > > > > lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf); > > lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT, >
On 12/19/2024 03:21, Naresh Solanki wrote: > Hi , > > On Thu, 19 Dec 2024 at 00:50, Mario Limonciello > <mario.limonciello@amd.com> wrote: >> >> On 12/18/2024 13:00, Naresh Solanki wrote: >>> Refactor to calculate max-freq more accurately. >> >> Can you add some more detail about what you're finding? >> What was it before, what is it now, why is it more accurate? > Sure. > The previous approach had some roundoff error due to division compute > when calculating boost ratio. > Which intern affected max-freq calculation resulting in reporting lower value > In my Glinda SoC board, > See below calculations with below values: > max_perf = 208 > nominal_perf = 100 > normal_freq = 2600 > > How linux kernel calculates: > freq = ( (max_perf * 1024 / nominal_perf) * normal_freq) / 1024 > freq = 5405 // Integer arithmatic. > > With the changes > freq = (max_perf * normal_freq) / nominal_perf > freq = 5408 > > Ah, thanks! Please include some of this info in the commit message for the next version. > >> >>> >>> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> >>> --- >>> drivers/cpufreq/amd-pstate.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c >>> index d7630bab2516..78a2cbd14952 100644 >>> --- a/drivers/cpufreq/amd-pstate.c >>> +++ b/drivers/cpufreq/amd-pstate.c >>> @@ -892,7 +892,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) >>> u64 numerator; >>> u32 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); >>> @@ -914,8 +914,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) >>> ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator); >>> if (ret) >>> return ret; >>> - boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf); >>> - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000; >>> + max_freq = div_u64(numerator * nominal_freq * 1000, nominal_perf); >> >> This doesn't apply currently, because of some changes in the >> superm1.git/linux-next branch; specifically: >> >> https://git.kernel.org/superm1/c/68cb0e77b6439 >> >> I haven't sent this out to linux-pm yet so it could be in linux-next, >> but will be doing that soon. So can you please rebase on that branch if >> this change still makes sense? > Sure. Will do > Thanks! > Regards, > Naresh Solanki >> >>> >>> lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf); >>> lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT, >>
Hi Mario On Thu, 19 Dec 2024 at 20:31, Mario Limonciello <mario.limonciello@amd.com> wrote: > > On 12/19/2024 03:21, Naresh Solanki wrote: > > Hi , > > > > On Thu, 19 Dec 2024 at 00:50, Mario Limonciello > > <mario.limonciello@amd.com> wrote: > >> > >> On 12/18/2024 13:00, Naresh Solanki wrote: > >>> Refactor to calculate max-freq more accurately. > >> > >> Can you add some more detail about what you're finding? > >> What was it before, what is it now, why is it more accurate? > > Sure. > > The previous approach had some roundoff error due to division compute > > when calculating boost ratio. > > Which intern affected max-freq calculation resulting in reporting lower value > > In my Glinda SoC board, > > See below calculations with below values: > > max_perf = 208 > > nominal_perf = 100 > > normal_freq = 2600 > > > > How linux kernel calculates: > > freq = ( (max_perf * 1024 / nominal_perf) * normal_freq) / 1024 > > freq = 5405 // Integer arithmatic. > > > > With the changes > > freq = (max_perf * normal_freq) / nominal_perf > > freq = 5408 > > > > > > Ah, thanks! Please include some of this info in the commit message for > the next version. Sure > > > > >> > >>> > >>> Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> > >>> --- > >>> drivers/cpufreq/amd-pstate.c | 5 ++--- > >>> 1 file changed, 2 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c > >>> index d7630bab2516..78a2cbd14952 100644 > >>> --- a/drivers/cpufreq/amd-pstate.c > >>> +++ b/drivers/cpufreq/amd-pstate.c > >>> @@ -892,7 +892,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > >>> u64 numerator; > >>> u32 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); > >>> @@ -914,8 +914,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) > >>> ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator); > >>> if (ret) > >>> return ret; > >>> - boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf); > >>> - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000; > >>> + max_freq = div_u64(numerator * nominal_freq * 1000, nominal_perf); > >> > >> This doesn't apply currently, because of some changes in the > >> superm1.git/linux-next branch; specifically: > >> > >> https://git.kernel.org/superm1/c/68cb0e77b6439 > >> > >> I haven't sent this out to linux-pm yet so it could be in linux-next, > >> but will be doing that soon. So can you please rebase on that branch if > >> this change still makes sense? > > Sure. Will do > > > > Thanks! Regards, Naresh > > > Regards, > > Naresh Solanki > >> > >>> > >>> lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf); > >>> lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT, > >> >
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c index d7630bab2516..78a2cbd14952 100644 --- a/drivers/cpufreq/amd-pstate.c +++ b/drivers/cpufreq/amd-pstate.c @@ -892,7 +892,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) u64 numerator; u32 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); @@ -914,8 +914,7 @@ static int amd_pstate_init_freq(struct amd_cpudata *cpudata) ret = amd_get_boost_ratio_numerator(cpudata->cpu, &numerator); if (ret) return ret; - boost_ratio = div_u64(numerator << SCHED_CAPACITY_SHIFT, nominal_perf); - max_freq = (nominal_freq * boost_ratio >> SCHED_CAPACITY_SHIFT) * 1000; + max_freq = div_u64(numerator * nominal_freq * 1000, nominal_perf); lowest_nonlinear_perf = READ_ONCE(cpudata->lowest_nonlinear_perf); lowest_nonlinear_ratio = div_u64(lowest_nonlinear_perf << SCHED_CAPACITY_SHIFT,
Refactor to calculate max-freq more accurately. Signed-off-by: Naresh Solanki <naresh.solanki@9elements.com> --- drivers/cpufreq/amd-pstate.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)