From: "Doug Smythies" <dsmythies@telus.net>
To: 'Dirk Brandewie' <dirk.brandewie@gmail.com>,
"'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
'Stratos Karafotis' <stratosk@semaphore.gr>
Cc: viresh.kumar@linaro.org, dirk.j.brandewie@intel.com,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: RE: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct
Date: Fri, 13 Jun 2014 07:36:21 -0700 [thread overview]
Message-ID: <002201cf8714$d933cca0$8b9b65e0$@net> (raw)
In-Reply-To: <539B0147.8020407@gmail.com>
On 2014.06.12 06:49 Dirk Brandewie wrote:
> On 06/12/2014 01:03 PM, Rafael J. Wysocki wrote:
>> On Thursday, June 12, 2014 05:35:59 PM Stratos Karafotis wrote:
>>> On 12/06/2014 12:15 πμ, Doug Smythies wrote:
>>>>> Could you please elaborate a little bit more what we need these 2 lines below?
>>>>>
> Sorry for being MIA on this thread I have been up to my eyeballs.
>>>> if ((rem << 1) >= int_tofp(sample->mperf))
>>>> core_pct += 1;
> The rounding should have been
> core_pct += (1 << (FRAC_BITS-1));
> Since core_pct is is in fixeded point notation at this point. Adding .5 to
> core_pct to round up.
> As Stratos pointed out the the current code only adds 1/256 to core_pct
> Since core_pct_busy stays in fixed point through out the rest of the
> calculations ans we only do the rounding when the PID is returning an
> int I think we can safely remove these two lines.
Absolutely, no.
That code was doing exactly what I wanted it to do.
But, as and I have already admitted, it was overkill, and yes the entire thing
can be changed to use div_64 instead.
We do not want to add 1/2 to core percent here at this spot.
You would just be bringing back the arbitrary and incorrect biasing
of core_pct upwards that used to be there in two spots before.
You would add 1/2 when you want to convert to an integer, not before (and we don't right now, for the call to trace_pstate_sample).
... Doug
WARNING: multiple messages have this Message-ID (diff)
From: "Doug Smythies" <dsmythies@telus.net>
To: "'Dirk Brandewie'" <dirk.brandewie@gmail.com>,
"'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
"'Stratos Karafotis'" <stratosk@semaphore.gr>
Cc: <viresh.kumar@linaro.org>, <dirk.j.brandewie@intel.com>,
<linux-pm@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct
Date: Fri, 13 Jun 2014 07:36:21 -0700 [thread overview]
Message-ID: <002201cf8714$d933cca0$8b9b65e0$@net> (raw)
In-Reply-To: <539B0147.8020407@gmail.com>
On 2014.06.12 06:49 Dirk Brandewie wrote:
> On 06/12/2014 01:03 PM, Rafael J. Wysocki wrote:
>> On Thursday, June 12, 2014 05:35:59 PM Stratos Karafotis wrote:
>>> On 12/06/2014 12:15 πμ, Doug Smythies wrote:
>>>>> Could you please elaborate a little bit more what we need these 2 lines below?
>>>>>
> Sorry for being MIA on this thread I have been up to my eyeballs.
>>>> if ((rem << 1) >= int_tofp(sample->mperf))
>>>> core_pct += 1;
> The rounding should have been
> core_pct += (1 << (FRAC_BITS-1));
> Since core_pct is is in fixeded point notation at this point. Adding .5 to
> core_pct to round up.
> As Stratos pointed out the the current code only adds 1/256 to core_pct
> Since core_pct_busy stays in fixed point through out the rest of the
> calculations ans we only do the rounding when the PID is returning an
> int I think we can safely remove these two lines.
Absolutely, no.
That code was doing exactly what I wanted it to do.
But, as and I have already admitted, it was overkill, and yes the entire thing
can be changed to use div_64 instead.
We do not want to add 1/2 to core percent here at this spot.
You would just be bringing back the arbitrary and incorrect biasing
of core_pct upwards that used to be there in two spots before.
You would add 1/2 when you want to convert to an integer, not before (and we don't right now, for the call to trace_pstate_sample).
... Doug
next prev parent reply other threads:[~2014-06-13 14:36 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-11 12:33 [PATCH] cpufreq: intel_pstate: Fix rounding of core_pct Stratos Karafotis
2014-06-11 13:41 ` Doug Smythies
2014-06-11 13:41 ` Doug Smythies
2014-06-11 14:08 ` Stratos Karafotis
2014-06-11 15:02 ` Doug Smythies
2014-06-11 15:02 ` Doug Smythies
2014-06-11 18:28 ` Rafael J. Wysocki
2014-06-11 21:40 ` Doug Smythies
2014-06-11 21:40 ` Doug Smythies
2014-06-11 21:45 ` Rafael J. Wysocki
2014-06-12 6:56 ` Doug Smythies
2014-06-12 6:56 ` Doug Smythies
2014-06-11 20:20 ` Stratos Karafotis
2014-06-11 21:15 ` Doug Smythies
2014-06-11 21:15 ` Doug Smythies
2014-06-12 14:35 ` Stratos Karafotis
2014-06-12 20:03 ` Rafael J. Wysocki
2014-06-13 6:49 ` Doug Smythies
2014-06-13 6:49 ` Doug Smythies
2014-06-13 17:39 ` Stratos Karafotis
2014-06-13 13:48 ` Dirk Brandewie
2014-06-13 14:36 ` Doug Smythies [this message]
2014-06-13 14:36 ` Doug Smythies
2014-06-13 16:56 ` Stratos Karafotis
2014-06-11 14:27 ` Doug Smythies
2014-06-11 14:27 ` Doug Smythies
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='002201cf8714$d933cca0$8b9b65e0$@net' \
--to=dsmythies@telus.net \
--cc=dirk.brandewie@gmail.com \
--cc=dirk.j.brandewie@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=stratosk@semaphore.gr \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.