From: "Doug Smythies" <dsmythies@telus.net>
To: "'Rafael J. Wysocki'" <rjw@rjwysocki.net>,
'Srinivas Pandruvada' <srinivas.pandruvada@linux.intel.com>
Cc: 'LKML' <linux-kernel@vger.kernel.org>,
'Linux PM' <linux-pm@vger.kernel.org>
Subject: RE: [PATCH 00/14] cpufreq: intel_pstate: Fixes, cleanups and optimizations
Date: Sun, 12 Mar 2017 23:29:24 -0700 [thread overview]
Message-ID: <001301d29bc3$29c351c0$7d49f540$@net> (raw)
In-Reply-To: n7NGc1Po4sVy3n7NIcDiMl
On 2017.03.12 10:12 Rafael J. Wysocki wrote:
> This patch series fixes a couple of bugs in intel_pstate, cleans up the code in
> it somewhat and makes some changes targeted at overhead reductions.
>
If clean up and overhead reductions are being considered, is there any interest
in changing the PID controller to a P controller and eliminating the integral
and derivative code entirely?
Why? The application is not really best suited to a PID
(Proportional Integral Derivative) controller.
Integral terms are normally used to null out accumulated errors. For example
position errors as a function of integrated velocity, where the overall
position is supposed to be time * nominal velocity, but the actual velocity
at any sample point is not perfect.
In signal processing, derivatives are difficult at the best of times, let alone
with the drastic sample time variations (anywhere from 10 milliseconds to 5 seconds)
experienced here. Myself, I can not think of a need for a derivative term anyhow.
Readers might note the old non-zero integral gain for the old methods used
with Atom processors (being eliminated in this patch set, see patch 2 of 14).
However that was due to the low proportional gain used and was needed to get
target pstates to tick up or down as it settled to some steady state value,
as otherwise and with a setpoint of 97 (which is what was being used at the
time), it would not. I'm saying the integral term was being used in way that
was not intended to overcome another issue. In that scenario, and at the very
least, the error term should have been cleared upon integration to the point
where the pstate ticked up or down as a result.
To be clear, I'm not talking about changing the proportional code at all,
but only about eliminating the integral and derivative code that has never
been used.
If there is interest, I'll prepare and submit a patch.
> Patch [1/14] is a regression fix, patch [2/14] can be regarded as a fix too,
>
> Patches [3-9/14] are cleanups mostly getting rid of unnecessary stuff.
>
> Patches [10-11/14] make changes to reduce the overhead of utilization update
> callbacks used in the active mode.
>
> Patches [12-14/14] make more cleanups on top of that.
>
> Refer to the changelogs for details.
next reply other threads:[~2017-03-13 6:29 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-13 6:29 Doug Smythies [this message]
2017-03-13 11:16 ` [PATCH 00/14] cpufreq: intel_pstate: Fixes, cleanups and optimizations Rafael J. Wysocki
2017-03-13 21:48 ` Doug Smythies
-- strict thread matches above, loose matches on Subject: below --
2017-03-12 17:11 Rafael J. Wysocki
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='001301d29bc3$29c351c0$7d49f540$@net' \
--to=dsmythies@telus.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=rjw@rjwysocki.net \
--cc=srinivas.pandruvada@linux.intel.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).