RE: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct

["Doug Smythies" <dsmythies@telus.net>]

-----Original Message-----
From: Stratos Karafotis [mailto:stratosk@semaphore.gr] 
Sent: June-11-2014 13:20
To: Doug Smythies
Cc: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; rjw@rjwysocki.net; viresh.kumar@linaro.org; dirk.j.brandewie@intel.com
Subject: Re: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct

On 2014.06.11 13:20 Stratos Karafotis wrote:
> On 11/06/2014 06:02 μμ, Doug Smythies wrote:
>> 
>> On 2104.06.11 07:08 Stratos Karafotis wrote:
>>> On 11/06/2014 04:41 μμ, Doug Smythies wrote:
>>>
>>> No.
>>
>>> The intent was only ever to round properly the pseudo floating point result of the divide.
>>> It was much more important (ugh, well 4 times more) when FRACBITS was still 6, which also got changed to 8 in a recent patch.
>>>
>> 
>> Are you sure?
>> 
>> Yes.
>> 
>>> This rounding was very recently added.
>>> As far as I can understand, I don't see the meaning of this rounding, as is.
>>> Even if FRAC_BITS was 6, I think it would have almost no improvement in
>>> calculations.
>> 
>> Note: I had not seen this e-mail when I wrote a few minutes ago:
>> 
>> You may be correct.
>> If Dirk agrees, I will re-analyse the entire driver for rounding effects soon.
>> When FRACBITS was 6 there were subtle cases where the driver would get stuck, and not make a final pstate change, with the default PID gains.
>> Other things have changed, and the analysis needs to be re-done.
>> 

> Could you please elaborate a little bit more what we need these 2 lines below?
>
>        if ((rem << 1) >= int_tofp(sample->mperf))
>                core_pct += 1;
>
> Because nothing is mentioned for them in commit's changelog.
> Do we need to round core_pct or not?
> Because if we try to round it, I think this patch should work.

As mentioned originally, they are there just to round the pseudo floating number, not the integer portion only.
Let us bring back the very numbers you originally gave and work through it.

aperf = 5024
mperf = 10619

core_pct = 47.31142292%
or 47 and 79.724267 256ths
or to the closest kept fractional part 47 and 80 256ths
or 12112 as a pseudo float.
The maximum error with this rounding will be 1 part in 512 and symmetric instead of the 1 part in 256 always in one direction without.

Now if FRACBITS was still 6:
core_pct = 47.31142292%
or 47 and 19.931 64ths
or to the closest kept fractional part 47 and 20 64ths
or 3028 as a pseudo float.
The maximum error with this rounding will be 1 part in 128 and symmetric instead of the 1 part in 64 (1.6% !!!) always in one direction without.

Hope this helps.

... Doug