linux-kernel - Re: [PATCH] cpufreq/amd-pstate: Refactor max frequency calculation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABqG17g1u0ibMZz_4E0mE0a36g4_Tk8D2gL722GFNWBxQYrgiQ@mail.gmail.com>
Date: Thu, 19 Dec 2024 22:03:48 +0530
From: Naresh Solanki <naresh.solanki@...ements.com>
To: Mario Limonciello <mario.limonciello@....com>
Cc: Huang Rui <ray.huang@....com>, "Gautham R. Shenoy" <gautham.shenoy@....com>, 
	Perry Yuan <perry.yuan@....com>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Viresh Kumar <viresh.kumar@...aro.org>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cpufreq/amd-pstate: Refactor max frequency calculation

Hi Mario

On Thu, 19 Dec 2024 at 20:31, Mario Limonciello
<mario.limonciello@....com> wrote:
>
> On 12/19/2024 03:21, Naresh Solanki wrote:
> > Hi ,
> >
> > On Thu, 19 Dec 2024 at 00:50, Mario Limonciello
> > <mario.limonciello@....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@...ements.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,
> >>
>