linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ionela Voinescu <ionela.voinescu@arm.com>
To: rjw@rjwysocki.net, viresh.kumar@linaro.org,
	catalin.marinas@arm.com, sudeep.holla@arm.com, will@kernel.org,
	linux@armlinux.org.uk, valentin.schneider@arm.com
Cc: mingo@redhat.com, peterz@infradead.org, dietmar.eggemann@arm.com,
	ionela.voinescu@arm.com, linux-pm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Liviu Dudau <liviu.dudau@arm.com>
Subject: [PATCH 4/8] cpufreq,vexpress-spc: fix Frequency Invariance (FI) for bL switching
Date: Wed,  1 Jul 2020 10:07:47 +0100	[thread overview]
Message-ID: <20200701090751.7543-5-ionela.voinescu@arm.com> (raw)
In-Reply-To: <20200701090751.7543-1-ionela.voinescu@arm.com>

In the majority of cases, the index argument to cpufreq's target_index()
is meant to identify the frequency that is requested from the hardware,
according to the frequency table: policy->freq_table[index].frequency.

After successfully requesting it from the hardware, this value, together
with the maximum hardware frequency (policy->cpuinfo.max_freq) are used
as arguments to arch_set_freq_scale(), in order to set the task scheduler
frequency scale factor. This is a normalized indication of a CPU's
current performance.

But for the vexpress-spc-cpufreq driver, when big.LITTLE switching [1]
is enabled, there are three issues with using the above information for
setting the FI scale factor:

 - cur_freq: policy->freq_table[index].frequency is not the frequency
   requested from the hardware. ve_spc_cpufreq_set_rate() will convert
   from this virtual frequency to an actual frequency, which is then
   requested from the hardware. For the A7 cluster, the virtual frequency
   is half the actual frequency. The use of the virtual policy->freq_table
   frequency results in an incorrect FI scale factor.

 - max_freq: policy->cpuinfo.max_freq does not correctly identify the
   maximum frequency of the physical cluster. This value identifies the
   maximum frequency achievable by the big-LITTLE pair, that is the
   maximum frequency of the big CPU. But when the LITTLE CPU in the group
   is used, the hardware maximum frquency passed to arch_set_freq_scale()
   is incorrect.

 - missing a scale factor update: when switching clusters, the driver
   recalculates the frequency of the old clock domain based on the
   requests of the remaining CPUs in the domain and asks for a clock
   change. But this does not result in an update in the scale factor.

Therefore, introduce a local function bLs_set_sched_freq_scale() that
helps call arch_set_freq_scale() with correct information for the
is_bL_switching_enabled() case, while maintaining the old, more
efficient, call site of arch_set_freq_scale() for when cluster
switching is disabled.

Also, because of these requirements in computing the scale factor, this
driver is the only one that maintains custom support for FI, which is
marked by the presence of the CPUFREQ_CUSTOM_SET_FREQ_SCALE flag.

[1] https://lwn.net/Articles/481055/

Signed-off-by: Ionela Voinescu <ionela.voinescu@arm.com>
Cc: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Rafael J. Wysocki <rjw@rjwysocki.net>
Cc: Liviu Dudau <liviu.dudau@arm.com>
---
 drivers/cpufreq/vexpress-spc-cpufreq.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/vexpress-spc-cpufreq.c b/drivers/cpufreq/vexpress-spc-cpufreq.c
index e0a1a3367ec5..f2caf67d4050 100644
--- a/drivers/cpufreq/vexpress-spc-cpufreq.c
+++ b/drivers/cpufreq/vexpress-spc-cpufreq.c
@@ -55,6 +55,8 @@ static atomic_t cluster_usage[MAX_CLUSTERS + 1];
 static unsigned int clk_big_min;	/* (Big) clock frequencies */
 static unsigned int clk_little_max;	/* Maximum clock frequency (Little) */
 
+static inline u32 get_table_max(struct cpufreq_frequency_table *table);
+
 static DEFINE_PER_CPU(unsigned int, physical_cluster);
 static DEFINE_PER_CPU(unsigned int, cpu_last_req_freq);
 
@@ -87,6 +89,18 @@ static unsigned int find_cluster_maxfreq(int cluster)
 	return max_freq;
 }
 
+static void bLs_set_sched_freq_scale(int cluster, unsigned long cur_freq)
+{
+	unsigned long max_freq = get_table_max(freq_table[cluster]);
+	int j;
+
+	for_each_online_cpu(j) {
+		if (cluster == per_cpu(physical_cluster, j))
+			arch_set_freq_scale(get_cpu_mask(j), cur_freq,
+					    max_freq);
+	}
+}
+
 static unsigned int clk_get_cpu_rate(unsigned int cpu)
 {
 	u32 cur_cluster = per_cpu(physical_cluster, cpu);
@@ -154,6 +168,9 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 
 	mutex_unlock(&cluster_lock[new_cluster]);
 
+	if (bLs)
+		bLs_set_sched_freq_scale(new_cluster, new_rate);
+
 	/* Recalc freq for old cluster when switching clusters */
 	if (old_cluster != new_cluster) {
 		/* Switch cluster */
@@ -170,7 +187,11 @@ ve_spc_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate)
 			pr_err("%s: clk_set_rate failed: %d, old cluster: %d\n",
 			       __func__, ret, old_cluster);
 		}
+
 		mutex_unlock(&cluster_lock[old_cluster]);
+
+		if (new_rate)
+			bLs_set_sched_freq_scale(old_cluster, new_rate);
 	}
 
 	return 0;
@@ -200,7 +221,7 @@ static int ve_spc_cpufreq_set_target(struct cpufreq_policy *policy,
 	ret = ve_spc_cpufreq_set_rate(cpu, actual_cluster, new_cluster,
 				      freqs_new);
 
-	if (!ret) {
+	if (!is_bL_switching_enabled() && !ret) {
 		arch_set_freq_scale(policy->related_cpus, freqs_new,
 				    policy->cpuinfo.max_freq);
 	}
-- 
2.17.1


  parent reply	other threads:[~2020-07-01  9:09 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-01  9:07 [PATCH 0/8] cpufreq: improve frequency invariance support Ionela Voinescu
2020-07-01  9:07 ` [PATCH 1/8] cpufreq: allow drivers to flag custom support for freq invariance Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01 13:33     ` Ionela Voinescu
2020-07-01 16:05       ` Rafael J. Wysocki
2020-07-01 18:06         ` Ionela Voinescu
2020-07-02  2:58         ` Viresh Kumar
2020-07-02 11:44           ` Ionela Voinescu
2020-07-06 12:14             ` Dietmar Eggemann
2020-07-09  8:53               ` Ionela Voinescu
2020-07-09  9:09                 ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 2/8] cpufreq: move invariance setter calls in cpufreq core Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01 15:27     ` Ionela Voinescu
2020-07-01 15:51       ` Rafael J. Wysocki
2020-07-02  3:01         ` Viresh Kumar
2020-07-02 11:45         ` Ionela Voinescu
2020-07-01  9:07 ` [PATCH 3/8] cpufreq,drivers: remove setting of frequency scale factor Ionela Voinescu
2020-07-01  9:07 ` Ionela Voinescu [this message]
2020-07-01 10:46   ` [PATCH 4/8] cpufreq,vexpress-spc: fix Frequency Invariance (FI) for bL switching Viresh Kumar
2020-07-01 14:07     ` Ionela Voinescu
2020-07-02  3:05       ` Viresh Kumar
2020-07-02 11:41         ` Ionela Voinescu
2020-07-02 11:46           ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 5/8] cpufreq: report whether cpufreq supports Frequency Invariance (FI) Ionela Voinescu
2020-07-01 10:46   ` Viresh Kumar
2020-07-01  9:07 ` [PATCH 6/8] arch_topology,cpufreq,sched/core: constify arch_* cpumasks Ionela Voinescu
2020-07-01  9:07 ` [PATCH 7/8] arch_topology,arm64: define arch_scale_freq_invariant() Ionela Voinescu
2020-07-01  9:07 ` [PATCH 8/8] cpufreq: make schedutil the default for arm and arm64 Ionela Voinescu

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=20200701090751.7543-5-ionela.voinescu@arm.com \
    --to=ionela.voinescu@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=liviu.dudau@arm.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rjw@rjwysocki.net \
    --cc=sudeep.holla@arm.com \
    --cc=valentin.schneider@arm.com \
    --cc=viresh.kumar@linaro.org \
    --cc=will@kernel.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).