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

[Mario Limonciello <mario.limonciello@amd.com>]

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,
>>