From: Viresh Kumar <viresh.kumar@linaro.org>
To: Rafael Wysocki <rjw@rjwysocki.net>,
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>,
Len Brown <lenb@kernel.org>,
Viresh Kumar <viresh.kumar@linaro.org>
Cc: linaro-kernel@lists.linaro.org, linux-pm@vger.kernel.org,
linux-kernel@vger.kernel.org,
Vincent Guittot <vincent.guittot@linaro.org>,
Juri Lelli <Juri.Lelli@arm.com>, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
patrick.bellasi@arm.com, john.ettedgui@gmail.com,
Joel Fernandes <joelaf@google.com>,
Morten Rasmussen <morten.rasmussen@arm.com>
Subject: [PATCH 3/3] cpufreq: intel_pstate: Provide resolve_freq() to fix regression
Date: Fri, 9 Jun 2017 15:45:56 +0530 [thread overview]
Message-ID: <eb1c4db95f52dbd4014438b901cc21dd693df005.1497002895.git.viresh.kumar@linaro.org> (raw)
In-Reply-To: <cover.1497002895.git.viresh.kumar@linaro.org>
In-Reply-To: <cover.1497002895.git.viresh.kumar@linaro.org>
When the schedutil governor calls cpufreq_driver_resolve_freq() for the
intel_pstate (in passive mode) driver, it simply returns the requested
frequency as there is no ->resolve_freq() callback provided.
The result is that get_next_freq() doesn't get a chance to know the
frequency which will be set eventually and we can hit a potential
regression as explained in the following paragraph.
For example, consider the possible range of frequencies as 900 MHz, 1
GHz, 1.1 GHz, and 1.2 GHz. If the current frequency is 1.1 GHz and the
next frequency (based on current utilization) is 1 GHz, then the
schedutil governor will try to set the average of these as the next
frequency (i.e. 1.05 GHz).
Because we always try to find the lowest frequency greater than equal to
the target frequency, the intel_pstate driver will end up setting the
frequency as 1.1 GHz.
Though the sg_policy->next_freq field gets updated with the average
frequency only. And so we will finally select the min frequency when the
next_freq is 1 more than the min frequency as the average then will be
equal to the min frequency. But that will also take lots of iterations
of the schedutil update callbacks.
Fix that by providing a resolve_freq() callback.
Tested on desktop with Intel Skylake processors.
Fixes: 39b64aa1c007 ("cpufreq: schedutil: Reduce frequencies slower")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/cpufreq/intel_pstate.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/cpufreq/intel_pstate.c b/drivers/cpufreq/intel_pstate.c
index 029a93bfb558..e177352180c3 100644
--- a/drivers/cpufreq/intel_pstate.c
+++ b/drivers/cpufreq/intel_pstate.c
@@ -2213,6 +2213,19 @@ static int intel_cpufreq_target(struct cpufreq_policy *policy,
return 0;
}
+unsigned int intel_cpufreq_resolve_freq(struct cpufreq_policy *policy,
+ unsigned int target_freq)
+{
+ struct cpudata *cpu = all_cpu_data[policy->cpu];
+ int target_pstate;
+
+ update_turbo_state();
+
+ target_pstate = DIV_ROUND_UP(target_freq, cpu->pstate.scaling);
+ target_pstate = intel_pstate_prepare_request(cpu, target_pstate);
+ return target_pstate * cpu->pstate.scaling;
+}
+
static unsigned int intel_cpufreq_fast_switch(struct cpufreq_policy *policy,
unsigned int target_freq)
{
@@ -2246,6 +2259,7 @@ static struct cpufreq_driver intel_cpufreq = {
.flags = CPUFREQ_CONST_LOOPS,
.verify = intel_cpufreq_verify_policy,
.target = intel_cpufreq_target,
+ .resolve_freq = intel_cpufreq_resolve_freq,
.fast_switch = intel_cpufreq_fast_switch,
.init = intel_cpufreq_cpu_init,
.exit = intel_pstate_cpu_exit,
--
2.13.0.70.g6367777092d9
next prev parent reply other threads:[~2017-06-09 10:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-09 10:15 [PATCH 0/3] cpufreq: schedutil: Fix 4.12 regressions Viresh Kumar
2017-06-09 10:15 ` [PATCH 1/3] cpufreq: schedutil: Restore cached_raw_freq behavior Viresh Kumar
2017-06-09 10:15 ` [PATCH 2/3] cpufreq: schedutil: Fix selection algorithm while reducing frequency Viresh Kumar
2017-06-10 9:11 ` Joel Fernandes
2017-06-11 6:21 ` Joel Fernandes
2017-06-12 3:44 ` Viresh Kumar
2017-06-12 12:33 ` Rafael J. Wysocki
2017-06-09 10:15 ` Viresh Kumar [this message]
2017-06-10 9:26 ` [PATCH 3/3] cpufreq: intel_pstate: Provide resolve_freq() to fix regression Joel Fernandes
2017-06-12 3:41 ` Viresh Kumar
2017-06-09 12:24 ` [PATCH 0/3] cpufreq: schedutil: Fix 4.12 regressions Rafael J. Wysocki
2017-06-09 12:32 ` Viresh Kumar
2017-06-09 13:25 ` 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=eb1c4db95f52dbd4014438b901cc21dd693df005.1497002895.git.viresh.kumar@linaro.org \
--to=viresh.kumar@linaro.org \
--cc=Juri.Lelli@arm.com \
--cc=joelaf@google.com \
--cc=john.ettedgui@gmail.com \
--cc=lenb@kernel.org \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=morten.rasmussen@arm.com \
--cc=patrick.bellasi@arm.com \
--cc=peterz@infradead.org \
--cc=rjw@rjwysocki.net \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=vincent.guittot@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 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).