* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
From: Rafael J. Wysocki @ 2019-06-27 17:49 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch, rajatja
In-Reply-To: <20190619175142.237794-1-ravisadineni@chromium.org>
On Wednesday, June 19, 2019 7:51:42 PM CEST Ravi Chandra Sadineni wrote:
> events_check_enabled bool is set when wakeup_count sysfs attribute
> is written. User level daemon is expected to write this attribute
> just before suspend.
>
> When this boolean is set, calls to pm_wakeup_event() will result in
> increment of per device and global wakeup count that helps in
> identifying the wake source. global wakeup count is also used by
> pm_wakeup_pending() to identify if there are any pending events that
> should result in an suspend abort.
>
> Currently calls to pm_wakeup_pending() also clears events_check_enabled.
> This can be a problem when there are multiple wake events or when the
> suspend is aborted due to an interrupt on a shared interrupt line.
> For example an Mfd device can create several platform devices which
> might fetch the state on resume in the driver resume method and increment
> the wakeup count if needed. But if events_check_enabled is cleared before
> resume methods get to execute, wakeup count will not be incremented. Thus
> let us not reset the bool here.
>
> Note that events_check_enabled is also cleared in suspend.c/enter_state()
> on every resume at the end.
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> drivers/base/power/wakeup.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..88aade871589 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
>
> split_counters(&cnt, &inpr);
> ret = (cnt != saved_count || inpr > 0);
> - events_check_enabled = !ret;
This effectively changes the meaning of the wakeup_count metric. so it cannot be applied.
> }
> raw_spin_unlock_irqrestore(&events_lock, flags);
>
>
^ permalink raw reply
* [PATCH 43/87] kernel: power: replace kmalloc and memset with kzalloc
From: Fuqian Huang @ 2019-06-27 17:40 UTC (permalink / raw)
Cc: Fuqian Huang, Rafael J. Wysocki, Len Brown, Pavel Machek,
linux-pm, linux-kernel
kmalloc + memset(0) -> kzalloc
Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
---
kernel/power/swap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index e1912ad13bdc..ca0fcb5ced71 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -974,12 +974,11 @@ static int get_swap_reader(struct swap_map_handle *handle,
last = handle->maps = NULL;
offset = swsusp_header->image;
while (offset) {
- tmp = kmalloc(sizeof(*handle->maps), GFP_KERNEL);
+ tmp = kzalloc(sizeof(*handle->maps), GFP_KERNEL);
if (!tmp) {
release_swap_reader(handle);
return -ENOMEM;
}
- memset(tmp, 0, sizeof(*tmp));
if (!handle->maps)
handle->maps = tmp;
if (last)
--
2.11.0
^ permalink raw reply related
* [RFC PATCH v2 2/5] sched/cpufreq: Attach perf domain to sugov policy
From: Douglas RAILLARD @ 2019-06-27 17:16 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
In-Reply-To: <20190627171603.14767-1-douglas.raillard@arm.com>
Attach an Energy Model perf_domain to each sugov_policy to prepare the
ground for energy-aware schedutil.
Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
---
kernel/sched/cpufreq_schedutil.c | 39 ++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 9c0419087260..0a3ccc20adeb 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -41,6 +41,10 @@ struct sugov_policy {
bool work_in_progress;
bool need_freq_update;
+
+#ifdef CONFIG_ENERGY_MODEL
+ struct em_perf_domain *pd;
+#endif
};
struct sugov_cpu {
@@ -65,6 +69,38 @@ static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu);
/************************ Governor internals ***********************/
+#ifdef CONFIG_ENERGY_MODEL
+static void sugov_policy_attach_pd(struct sugov_policy *sg_policy)
+{
+ struct em_perf_domain *pd;
+ struct cpufreq_policy *policy = sg_policy->policy;
+
+ sg_policy->pd = NULL;
+ pd = em_cpu_get(policy->cpu);
+ if (!pd)
+ return;
+
+ if (cpumask_equal(policy->related_cpus, to_cpumask(pd->cpus)))
+ sg_policy->pd = pd;
+ else
+ pr_warn("%s: Not all CPUs in schedutil policy %u share the same perf domain, no perf domain for that policy will be registered\n",
+ __func__, policy->cpu);
+}
+
+static struct em_perf_domain *sugov_policy_get_pd(
+ struct sugov_policy *sg_policy)
+{
+ return sg_policy->pd;
+}
+#else /* CONFIG_ENERGY_MODEL */
+static void sugov_policy_attach_pd(struct sugov_policy *sg_policy) {}
+static struct em_perf_domain *sugov_policy_get_pd(
+ struct sugov_policy *sg_policy)
+{
+ return NULL;
+}
+#endif /* CONFIG_ENERGY_MODEL */
+
static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time)
{
s64 delta_ns;
@@ -850,6 +886,9 @@ static int sugov_start(struct cpufreq_policy *policy)
sugov_update_shared :
sugov_update_single);
}
+
+ sugov_policy_attach_pd(sg_policy);
+
return 0;
}
--
2.22.0
^ permalink raw reply related
* [RFC PATCH v2 3/5] sched/cpufreq: Hook em_pd_get_higher_power() into get_next_freq()
From: Douglas RAILLARD @ 2019-06-27 17:16 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
In-Reply-To: <20190627171603.14767-1-douglas.raillard@arm.com>
Choose the highest OPP for a given energy cost, allowing to skip lower
frequencies that would not be cheaper in terms of consumed power. These
frequencies can still be interesting to keep in the energy model to give
more freedom to thermal throttling, but should not be selected under
normal circumstances.
This also prepares the ground for energy-aware frequency boosting.
Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
---
kernel/sched/cpufreq_schedutil.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 0a3ccc20adeb..7ffc6fe3b670 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -10,6 +10,7 @@
#include "sched.h"
+#include <linux/energy_model.h>
#include <linux/sched/cpufreq.h>
#include <trace/events/power.h>
@@ -201,9 +202,16 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
struct cpufreq_policy *policy = sg_policy->policy;
unsigned int freq = arch_scale_freq_invariant() ?
policy->cpuinfo.max_freq : policy->cur;
+ struct em_perf_domain *pd = sugov_policy_get_pd(sg_policy);
freq = map_util_freq(util, freq, max);
+ /*
+ * Try to get a higher frequency if one is available, given the extra
+ * power we are ready to spend.
+ */
+ freq = em_pd_get_higher_freq(pd, freq, 0);
+
if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
return sg_policy->next_freq;
--
2.22.0
^ permalink raw reply related
* [RFC PATCH v2 4/5] sched/cpufreq: Introduce sugov_cpu_ramp_boost
From: Douglas RAILLARD @ 2019-06-27 17:16 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
In-Reply-To: <20190627171603.14767-1-douglas.raillard@arm.com>
Use the utilization signals dynamic to detect when the utilization of a
set of tasks starts increasing because of a change in tasks' behavior.
This allows detecting when spending extra power for faster frequency
ramp up response would be beneficial to the reactivity of the system.
This ramp boost is computed as the difference
util_avg-util_est_enqueued. This number somehow represents a lower bound
of how much extra utilization this tasks is actually using, compared to
our best current stable knowledge of it (which is util_est_enqueued).
When the set of runnable tasks changes, the boost is disabled as the
impact of blocked utilization on util_avg will make the delta with
util_est_enqueued not very informative.
Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
---
kernel/sched/cpufreq_schedutil.c | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 7ffc6fe3b670..3eabfd815195 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -60,6 +60,9 @@ struct sugov_cpu {
unsigned long bw_dl;
unsigned long max;
+ unsigned long ramp_boost;
+ unsigned long util_est_enqueued;
+
/* The field below is for single-CPU policies only: */
#ifdef CONFIG_NO_HZ_COMMON
unsigned long saved_idle_calls;
@@ -174,6 +177,41 @@ static void sugov_deferred_update(struct sugov_policy *sg_policy, u64 time,
}
}
+static unsigned long sugov_cpu_ramp_boost(struct sugov_cpu *sg_cpu)
+{
+ return READ_ONCE(sg_cpu->ramp_boost);
+}
+
+static unsigned long sugov_cpu_ramp_boost_update(struct sugov_cpu *sg_cpu,
+ unsigned long util)
+{
+ struct rq *rq = cpu_rq(sg_cpu->cpu);
+ unsigned long util_est_enqueued;
+ unsigned long util_avg;
+ unsigned long boost = 0;
+
+ util_est_enqueued = READ_ONCE(rq->cfs.avg.util_est.enqueued);
+ util_avg = READ_ONCE(rq->cfs.avg.util_avg);
+
+ /*
+ * Boost when util_avg becomes higher than the previous stable
+ * knowledge of the enqueued tasks' set util, which is CPU's
+ * util_est_enqueued.
+ *
+ * We try to spot changes in the workload itself, so we want to
+ * avoid the noise of tasks being enqueued/dequeued. To do that,
+ * we only trigger boosting when the "amount of work' enqueued
+ * is stable.
+ */
+ if (util_est_enqueued == sg_cpu->util_est_enqueued
+ && util_avg > util_est_enqueued)
+ boost = util_avg - util_est_enqueued;
+
+ sg_cpu->util_est_enqueued = util_est_enqueued;
+ WRITE_ONCE(sg_cpu->ramp_boost, boost);
+ return boost;
+}
+
/**
* get_next_freq - Compute a new frequency for a given cpufreq policy.
* @sg_policy: schedutil policy object to compute the new frequency for.
@@ -504,6 +542,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
busy = sugov_cpu_is_busy(sg_cpu);
util = sugov_get_util(sg_cpu);
+ sugov_cpu_ramp_boost_update(sg_cpu, util);
max = sg_cpu->max;
util = sugov_iowait_apply(sg_cpu, time, util, max);
next_f = get_next_freq(sg_policy, util, max);
@@ -544,6 +583,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
unsigned long j_util, j_max;
j_util = sugov_get_util(j_sg_cpu);
+ if (j_sg_cpu == sg_cpu)
+ sugov_cpu_ramp_boost_update(sg_cpu, j_util);
j_max = j_sg_cpu->max;
j_util = sugov_iowait_apply(j_sg_cpu, time, j_util, j_max);
@@ -553,6 +594,7 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
}
}
+
return get_next_freq(sg_policy, util, max);
}
--
2.22.0
^ permalink raw reply related
* [RFC PATCH v2 5/5] sched/cpufreq: Boost schedutil frequency ramp up
From: Douglas RAILLARD @ 2019-06-27 17:16 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
In-Reply-To: <20190627171603.14767-1-douglas.raillard@arm.com>
In some situations, it can be interesting to spend temporarily more
power if that can give a useful frequency boost.
Use the new sugov_cpu_ramp_boost() function to drive an energy-aware
boost, on top of the minimal required frequency.
As that boost number is not accurate (and cannot be without a crystal
ball), we only use it in a way that allows direct control over the power
it is going to cost. This allows keeping a platform-independant level of
control over the average power, while allowing for frequency bursts when
we know a (set of) tasks can make use of it.
In shared policies, the maximum of all CPU's boost is used. Since the
extra power expenditure is bounded, it cannot skyrocket even on
platforms with a large number of cores in the same frequency domain
and/or very high ratio between lowest and highest OPP cost.
Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
---
kernel/sched/cpufreq_schedutil.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c
index 3eabfd815195..d70bbbeaa5cf 100644
--- a/kernel/sched/cpufreq_schedutil.c
+++ b/kernel/sched/cpufreq_schedutil.c
@@ -217,6 +217,9 @@ static unsigned long sugov_cpu_ramp_boost_update(struct sugov_cpu *sg_cpu,
* @sg_policy: schedutil policy object to compute the new frequency for.
* @util: Current CPU utilization.
* @max: CPU capacity.
+ * @boost: Extra power that can be spent on top of the minimum amount of power
+ * required to meet capacity requirements, as a percentage between 0 and
+ * EM_COST_MARGIN_SCALE.
*
* If the utilization is frequency-invariant, choose the new frequency to be
* proportional to it, that is
@@ -235,7 +238,8 @@ static unsigned long sugov_cpu_ramp_boost_update(struct sugov_cpu *sg_cpu,
* cpufreq driver limitations.
*/
static unsigned int get_next_freq(struct sugov_policy *sg_policy,
- unsigned long util, unsigned long max)
+ unsigned long util, unsigned long max,
+ unsigned long boost)
{
struct cpufreq_policy *policy = sg_policy->policy;
unsigned int freq = arch_scale_freq_invariant() ?
@@ -248,7 +252,7 @@ static unsigned int get_next_freq(struct sugov_policy *sg_policy,
* Try to get a higher frequency if one is available, given the extra
* power we are ready to spend.
*/
- freq = em_pd_get_higher_freq(pd, freq, 0);
+ freq = em_pd_get_higher_freq(pd, freq, boost);
if (freq == sg_policy->cached_raw_freq && !sg_policy->need_freq_update)
return sg_policy->next_freq;
@@ -530,6 +534,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
unsigned long util, max;
unsigned int next_f;
bool busy;
+ unsigned long ramp_boost = 0;
sugov_iowait_boost(sg_cpu, time, flags);
sg_cpu->last_update = time;
@@ -542,10 +547,10 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
busy = sugov_cpu_is_busy(sg_cpu);
util = sugov_get_util(sg_cpu);
- sugov_cpu_ramp_boost_update(sg_cpu, util);
+ ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu, util);
max = sg_cpu->max;
util = sugov_iowait_apply(sg_cpu, time, util, max);
- next_f = get_next_freq(sg_policy, util, max);
+ next_f = get_next_freq(sg_policy, util, max, ramp_boost);
/*
* Do not reduce the frequency if the CPU has not been idle
* recently, as the reduction is likely to be premature then.
@@ -577,6 +582,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
struct cpufreq_policy *policy = sg_policy->policy;
unsigned long util = 0, max = 1;
unsigned int j;
+ unsigned long ramp_boost = 0;
+ unsigned long j_ramp_boost = 0;
for_each_cpu(j, policy->cpus) {
struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j);
@@ -584,7 +591,11 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
j_util = sugov_get_util(j_sg_cpu);
if (j_sg_cpu == sg_cpu)
- sugov_cpu_ramp_boost_update(sg_cpu, j_util);
+ j_ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu, j_util);
+ else
+ j_ramp_boost = sugov_cpu_ramp_boost(j_sg_cpu);
+ ramp_boost = max(ramp_boost, j_ramp_boost);
+
j_max = j_sg_cpu->max;
j_util = sugov_iowait_apply(j_sg_cpu, time, j_util, j_max);
@@ -595,7 +606,7 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
}
- return get_next_freq(sg_policy, util, max);
+ return get_next_freq(sg_policy, util, max, ramp_boost);
}
static void
--
2.22.0
^ permalink raw reply related
* [RFC PATCH v2 1/5] PM: Introduce em_pd_get_higher_freq()
From: Douglas RAILLARD @ 2019-06-27 17:15 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
In-Reply-To: <20190627171603.14767-1-douglas.raillard@arm.com>
em_pd_get_higher_freq() returns a frequency greater or equal to the
provided one while taking into account a given cost margin. It also
skips inefficient OPPs that have a higher cost than another one with a
higher frequency.
The efficiency of an OPP is measured as efficiency=capacity/power.
OPPs with the same efficiency are assumed to be equivalent, since they
will consume as much energy for a given amount of work to do. That may
take more or less time depending on the frequency, but will consume the
same energy.
Signed-off-by: Douglas RAILLARD <douglas.raillard@arm.com>
---
include/linux/energy_model.h | 53 ++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/include/linux/energy_model.h b/include/linux/energy_model.h
index aa027f7bcb3e..cc9819967f8d 100644
--- a/include/linux/energy_model.h
+++ b/include/linux/energy_model.h
@@ -159,6 +159,53 @@ static inline int em_pd_nr_cap_states(struct em_perf_domain *pd)
return pd->nr_cap_states;
}
+#define EM_COST_MARGIN_SCALE 1024U
+
+/**
+ * em_pd_get_higher_freq() - Get the highest frequency that does not exceed the
+ * given cost margin compared to min_freq
+ * @pd : performance domain for which this must be done
+ * @min_freq : minimum frequency to return
+ * @cost_margin : allowed margin compared to min_freq, on the
+ * EM_COST_MARGIN_SCALE scale.
+ *
+ * Return: the chosen frequency, guaranteed to be at least as high as min_freq.
+ */
+static inline unsigned long em_pd_get_higher_freq(struct em_perf_domain *pd,
+ unsigned long min_freq, unsigned long cost_margin)
+{
+ unsigned long max_cost = 0;
+ struct em_cap_state *cs;
+ int i;
+
+ if (!pd)
+ return min_freq;
+
+ /* Compute the maximum allowed cost */
+ for (i = 0; i < pd->nr_cap_states; i++) {
+ cs = &pd->table[i];
+ if (cs->frequency >= min_freq) {
+ max_cost = cs->cost +
+ (cs->cost * cost_margin) / EM_COST_MARGIN_SCALE;
+ break;
+ }
+ }
+
+ /* Find the highest frequency that will not exceed the cost margin */
+ for (i = pd->nr_cap_states-1; i >= 0; i--) {
+ cs = &pd->table[i];
+ if (cs->cost <= max_cost)
+ return cs->frequency;
+ }
+
+ /*
+ * We should normally never reach here, unless min_freq was higher than
+ * the highest available frequency, which is not expected to happen.
+ */
+ return min_freq;
+}
+
+
#else
struct em_perf_domain {};
struct em_data_callback {};
@@ -182,6 +229,12 @@ static inline int em_pd_nr_cap_states(struct em_perf_domain *pd)
{
return 0;
}
+
+static inline unsigned long em_pd_get_higher_freq(struct em_perf_domain *pd,
+ unsigned long min_freq, unsigned long cost_margin)
+{
+ return min_freq;
+}
#endif
#endif
--
2.22.0
^ permalink raw reply related
* [RFC PATCH v2 0/5] sched/cpufreq: Make schedutil energy aware
From: Douglas RAILLARD @ 2019-06-27 17:15 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pm, mingo, peterz, rjw, viresh.kumar, quentin.perret,
douglas.raillard, patrick.bellasi, dietmar.eggemann
Make schedutil cpufreq governor energy-aware.
- patch 1 introduces a function to retrieve a frequency given a base
frequency and an energy cost margin.
- patch 2 links Energy Model perf_domain to sugov_policy.
- patch 3 updates get_next_freq() to make use of the Energy Model.
- patch 4 adds sugov_cpu_ramp_boost() function.
- patch 5 updates sugov_update_(single|shared)() to make use of
sugov_cpu_ramp_boost().
The benefits of using the EM in schedutil are twofold:
1) Selecting the highest possible frequency for a given cost. Some
platforms can have lower frequencies that are less efficient than
higher ones, in which case they should be skipped for most purposes.
They can still be useful to give more freedom to thermal throttling
mechanisms, but not under normal circumstances.
note: the EM framework will warn about such OPPs "hertz/watts ratio
non-monotonically decreasing"
2) Driving the frequency selection with power in mind, in addition to
maximizing the utilization of the non-idle CPUs in the system.
Point 1) is implemented in "PM: Introduce em_pd_get_higher_freq()" and
enabled in schedutil by
"sched/cpufreq: Hook em_pd_get_higher_power() into get_next_freq()".
Point 2) is enabled in
"sched/cpufreq: Boost schedutil frequency ramp up". It allows using
higher frequencies when it is known that the true utilization of
currently running tasks is exceeding their previous stable point.
The benefits are:
* Boosting the frequency when the behavior of a runnable task changes,
leading to an increase in utilization. That shortens the frequency
ramp up duration, which in turns allows the utilization signal to
reach stable values quicker. Since the allowed frequency boost is
bounded in energy, it will behave consistently across platforms,
regardless of the OPP cost range.
* The boost is only transient, and should not impact a lot the energy
consumed of workloads with very stable utilization signals.
This has been ligthly tested with a rtapp task ramping from 10% to 75%
utilisation on a big core. Results are improved by fast ramp-up
EWMA [1], since it greatly reduces the oscillation in frequency at first
idle when ramping up.
[1] [PATCH] sched/fair: util_est: fast ramp-up EWMA on utilization increases
Message-ID: <20190620150555.15717-1-patrick.bellasi@arm.com>
https://lore.kernel.org/lkml/20190620150555.15717-1-patrick.bellasi@arm.com/
v1 -> v2:
* Split the new sugov_cpu_ramp_boost() from the existing
sugov_cpu_is_busy() as they seem to seek a different goal.
* Implement sugov_cpu_ramp_boost() based on CFS util_avg and
util_est_enqueued signals, rather than using idle calls count.
This makes the ramp boost much more accurate in finding boost
opportunities, and give a "continuous" output rather than a boolean.
* Add EM_COST_MARGIN_SCALE=1024 to represent the
margin values of em_pd_get_higher_freq().
Douglas RAILLARD (5):
PM: Introduce em_pd_get_higher_freq()
sched/cpufreq: Attach perf domain to sugov policy
sched/cpufreq: Hook em_pd_get_higher_power() into get_next_freq()
sched/cpufreq: Introduce sugov_cpu_ramp_boost
sched/cpufreq: Boost schedutil frequency ramp up
include/linux/energy_model.h | 53 ++++++++++++++++
kernel/sched/cpufreq_schedutil.c | 106 ++++++++++++++++++++++++++++++-
2 files changed, 156 insertions(+), 3 deletions(-)
--
2.22.0
^ permalink raw reply
* [PATCH RFC 1/4] OPP: Add and export helper to update voltage
From: Sibi Sankar @ 2019-06-27 13:34 UTC (permalink / raw)
To: viresh.kumar, nm, sboyd, georgi.djakov
Cc: agross, david.brown, robh+dt, mark.rutland, rjw, linux-arm-msm,
devicetree, linux-kernel, linux-pm, saravanak, Sibi Sankar
In-Reply-To: <20190627133424.4980-1-sibis@codeaurora.org>
Add and export 'dev_pm_opp_update_voltage' to find and update voltage
of an opp for a given frequency. This will be useful to update the opps
with voltages read back from firmware.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/opp/core.c | 52 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 10 ++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 68551d6366e6b..c85c04dc2c7de 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2197,6 +2197,58 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq)
}
EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
+/**
+ * dev_pm_opp_update_voltage() - Find and update voltage
+ * @dev: device for which we do this operation
+ * @freq: OPP frequency to update voltage
+ * @u_volt: voltage requested for this opp
+ *
+ * Find and update voltage of a disabled opp corresponding to the given
+ * frequency. This is useful only for devices with single power supply.
+ *
+ * Return: 0 if no modification was done OR modification was
+ * successful or a negative error value.
+ */
+int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
+ unsigned long u_volt)
+{
+ struct dev_pm_opp *opp = ERR_PTR(-ENODEV);
+ struct opp_table *opp_table;
+ unsigned long tol;
+ int ret = 0;
+
+ opp = dev_pm_opp_find_freq_exact(dev, freq, false);
+ if (IS_ERR(opp))
+ return PTR_ERR(opp);
+
+ /* Find the opp_table */
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret);
+ goto put_opp;
+ }
+
+ mutex_lock(&opp_table->lock);
+
+ /* update only if the opp is disabled */
+ if (opp->available)
+ goto unlock;
+
+ tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
+ opp->supplies[0].u_volt_min = u_volt - tol;
+ opp->supplies[0].u_volt = u_volt;
+ opp->supplies[0].u_volt_min = u_volt + tol;
+
+unlock:
+ mutex_unlock(&opp_table->lock);
+ dev_pm_opp_put_opp_table(opp_table);
+put_opp:
+ dev_pm_opp_put(opp);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_update_voltage);
+
/**
* dev_pm_opp_register_notifier() - Register OPP notifier for the device
* @dev: Device for which notifier needs to be registered
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 87fa09d93d8c2..a17c462974851 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -130,6 +130,9 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq);
int dev_pm_opp_disable(struct device *dev, unsigned long freq);
+int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
+ unsigned long u_volt);
+
int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb);
int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb);
@@ -261,6 +264,13 @@ static inline int dev_pm_opp_disable(struct device *dev, unsigned long freq)
return 0;
}
+static inline int dev_pm_opp_update_voltage(struct device *dev,
+ unsigned long freq,
+ unsigned long u_volt)
+{
+ return 0;
+}
+
static inline int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb)
{
return -ENOTSUPP;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 2/4] OPP: Add and export helper to set bandwidth
From: Sibi Sankar @ 2019-06-27 13:34 UTC (permalink / raw)
To: viresh.kumar, nm, sboyd, georgi.djakov
Cc: agross, david.brown, robh+dt, mark.rutland, rjw, linux-arm-msm,
devicetree, linux-kernel, linux-pm, saravanak, Sibi Sankar
In-Reply-To: <20190627133424.4980-1-sibis@codeaurora.org>
Add and export 'dev_pm_opp_set_bw' to set the bandwidth
levels associated with an OPP for a given frequency.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/opp/core.c | 46 ++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_opp.h | 6 ++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index c85c04dc2c7de..78f42960860d1 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -746,6 +746,52 @@ static int _set_required_opps(struct device *dev,
return ret;
}
+/**
+ * dev_pm_opp_set_bw() - Configures OPP bandwidth levels
+ * @dev: device for which we do this operation
+ * @freq: bandwidth values to set with matching 'freq'
+ *
+ * This configures the bandwidth to the levels specified
+ * by the OPP corresponding to the given frequency.
+ *
+ * Return: 0 on success or a negative error value.
+ */
+int dev_pm_opp_set_bw(struct device *dev, unsigned long freq)
+{
+ struct opp_table *opp_table;
+ struct dev_pm_opp *opp;
+ int ret = 0;
+ int i;
+
+ opp = dev_pm_opp_find_freq_exact(dev, freq, true);
+ if (IS_ERR(opp))
+ return PTR_ERR(opp);
+
+ opp_table = _find_opp_table(dev);
+ if (IS_ERR(opp_table)) {
+ dev_err(dev, "%s: device opp table doesn't exist\n", __func__);
+ ret = PTR_ERR(opp_table);
+ goto put_opp;
+ }
+
+ if (IS_ERR_OR_NULL(opp_table->paths)) {
+ ret = -ENODEV;
+ goto put_opp_table;
+ }
+
+ for (i = 0; i < opp_table->path_count; i++) {
+ ret = icc_set_bw(opp_table->paths[i], opp->bandwidth[i].avg,
+ opp->bandwidth[i].peak);
+ }
+
+put_opp_table:
+ dev_pm_opp_put_opp_table(opp_table);
+put_opp:
+ dev_pm_opp_put(opp);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_set_bw);
+
/**
* dev_pm_opp_set_rate() - Configure new OPP based on frequency
* @dev: device for which we do this operation
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index a17c462974851..1cdc2d0a2b20e 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -152,6 +152,7 @@ struct opp_table *dev_pm_opp_attach_genpd(struct device *dev, const char **names
void dev_pm_opp_detach_genpd(struct opp_table *opp_table);
int dev_pm_opp_xlate_performance_state(struct opp_table *src_table, struct opp_table *dst_table, unsigned int pstate);
int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq);
+int dev_pm_opp_set_bw(struct device *dev, unsigned long freq);
int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask);
int dev_pm_opp_get_sharing_cpus(struct device *cpu_dev, struct cpumask *cpumask);
void dev_pm_opp_remove_table(struct device *dev);
@@ -336,6 +337,11 @@ static inline int dev_pm_opp_set_rate(struct device *dev, unsigned long target_f
return -ENOTSUPP;
}
+static inline int dev_pm_opp_set_bw(struct device *dev, unsigned long freq)
+{
+ return -ENOTSUPP;
+}
+
static inline int dev_pm_opp_set_sharing_cpus(struct device *cpu_dev, const struct cpumask *cpumask)
{
return -ENOTSUPP;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 3/4] cpufreq: qcom: Update the bandwidth levels on frequency change
From: Sibi Sankar @ 2019-06-27 13:34 UTC (permalink / raw)
To: viresh.kumar, nm, sboyd, georgi.djakov
Cc: agross, david.brown, robh+dt, mark.rutland, rjw, linux-arm-msm,
devicetree, linux-kernel, linux-pm, saravanak, Sibi Sankar
In-Reply-To: <20190627133424.4980-1-sibis@codeaurora.org>
Add support to parse and update optional OPP tables attached to the
cpu nodes when the OPP bandwidth values are populated to enable
scaling of DDR/L3 bandwidth levels with frequency change.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
drivers/cpufreq/qcom-cpufreq-hw.c | 77 ++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+), 2 deletions(-)
diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index 4b0b50403901b..eacc75fac9b00 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -6,6 +6,7 @@
#include <linux/bitfield.h>
#include <linux/cpufreq.h>
#include <linux/init.h>
+#include <linux/interconnect.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/of_address.h>
@@ -30,13 +31,41 @@
static unsigned long cpu_hw_rate, xo_rate;
static struct platform_device *global_pdev;
+static int qcom_cpufreq_set_bw(struct cpufreq_policy *policy,
+ unsigned long freq_khz)
+{
+ struct device *dev;
+
+ dev = get_cpu_device(policy->cpu);
+ if (!dev)
+ return -ENODEV;
+
+ return dev_pm_opp_set_bw(dev, freq_khz * 1000);
+}
+
+static int qcom_cpufreq_update_opp(struct device *cpu_dev,
+ unsigned long freq_khz,
+ unsigned long volt)
+{
+ unsigned long freq_hz = freq_khz * 1000;
+
+ if (dev_pm_opp_update_voltage(cpu_dev, freq_hz, volt))
+ return dev_pm_opp_add(cpu_dev, freq_hz, volt);
+
+ /* Enable the opp after voltage update*/
+ return dev_pm_opp_enable(cpu_dev, freq_hz);
+}
+
static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
unsigned int index)
{
void __iomem *perf_state_reg = policy->driver_data;
+ u32 freq = policy->freq_table[index].frequency;
writel_relaxed(index, perf_state_reg);
+ qcom_cpufreq_set_bw(policy, freq);
+
return 0;
}
@@ -79,13 +108,29 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
{
u32 data, src, lval, i, core_count, prev_cc = 0, prev_freq = 0, freq;
u32 volt;
+ u64 rate;
unsigned int max_cores = cpumask_weight(policy->cpus);
struct cpufreq_frequency_table *table;
+ struct device_node *opp_table_np, *np;
+ int ret;
table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
if (!table)
return -ENOMEM;
+ ret = dev_pm_opp_of_add_table(cpu_dev);
+ if (!ret) {
+ /* Disable all opps and cross-validate against LUT */
+ opp_table_np = dev_pm_opp_of_get_opp_desc_node(cpu_dev);
+ for_each_available_child_of_node(opp_table_np, np) {
+ ret = of_property_read_u64(np, "opp-hz", &rate);
+ dev_pm_opp_disable(cpu_dev, rate);
+ }
+ of_node_put(opp_table_np);
+ } else {
+ dev_err(cpu_dev, "Couldn't add OPP table from dt\n");
+ }
+
for (i = 0; i < LUT_MAX_ENTRIES; i++) {
data = readl_relaxed(base + REG_FREQ_LUT +
i * LUT_ROW_SIZE);
@@ -104,7 +149,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
if (freq != prev_freq && core_count == max_cores) {
table[i].frequency = freq;
- dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+ qcom_cpufreq_update_opp(cpu_dev, freq, volt);
dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
freq, core_count);
} else {
@@ -125,7 +170,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
if (prev_cc != max_cores) {
prev->frequency = prev_freq;
prev->flags = CPUFREQ_BOOST_FREQ;
- dev_pm_opp_add(cpu_dev, prev_freq * 1000, volt);
+ qcom_cpufreq_update_opp(cpu_dev, prev_freq,
+ volt);
}
break;
@@ -168,6 +214,7 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
{
struct device *dev = &global_pdev->dev;
+ struct opp_table *opp_table = NULL;
struct of_phandle_args args;
struct device_node *cpu_np;
struct device *cpu_dev;
@@ -202,6 +249,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
if (!base)
return -ENOMEM;
+ opp_table = dev_pm_opp_set_paths(cpu_dev);
+
/* HW should be in enabled state to proceed */
if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
@@ -237,6 +286,8 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
return 0;
error:
+ if (opp_table)
+ dev_pm_opp_put_paths(opp_table);
devm_iounmap(dev, base);
return ret;
}
@@ -275,6 +326,8 @@ static struct cpufreq_driver cpufreq_qcom_hw_driver = {
static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
{
+ struct opp_table *opp_table = NULL;
+ struct device *cpu_dev;
struct clk *clk;
int ret;
@@ -294,6 +347,26 @@ static int qcom_cpufreq_hw_driver_probe(struct platform_device *pdev)
global_pdev = pdev;
+ /* Check for optional interconnect paths on CPU0 */
+ cpu_dev = get_cpu_device(0);
+ if (!cpu_dev) {
+ dev_err(&pdev->dev, "failed to get cpu0 device\n");
+ return -ENODEV;
+ }
+
+ opp_table = dev_pm_opp_set_paths(cpu_dev);
+ if (IS_ERR(opp_table)) {
+ ret = PTR_ERR(opp_table);
+ if (ret == -EPROBE_DEFER) {
+ dev_dbg(&pdev->dev, "defer icc set paths: %d\n", ret);
+ return ret;
+ }
+ dev_err(&pdev->dev, "set paths failed ddr/l3 scaling off: %d\n",
+ ret);
+ } else {
+ dev_pm_opp_put_paths(opp_table);
+ }
+
ret = cpufreq_register_driver(&cpufreq_qcom_hw_driver);
if (ret)
dev_err(&pdev->dev, "CPUFreq HW driver failed to register\n");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 4/4] arm64: dts: qcom: sdm845: Add cpu OPP tables
From: Sibi Sankar @ 2019-06-27 13:34 UTC (permalink / raw)
To: viresh.kumar, nm, sboyd, georgi.djakov
Cc: agross, david.brown, robh+dt, mark.rutland, rjw, linux-arm-msm,
devicetree, linux-kernel, linux-pm, saravanak, Sibi Sankar
In-Reply-To: <20190627133424.4980-1-sibis@codeaurora.org>
Add OPP tables for the cpu nodes.
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 343 +++++++++++++++++++++++++++
1 file changed, 343 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 471cbb7d9bc39..8cabbb274d3e7 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -197,6 +197,10 @@
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_0>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_0: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -218,6 +222,10 @@
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_100>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_100: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -236,6 +244,10 @@
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_200>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_200: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -254,6 +266,10 @@
qcom,freq-domain = <&cpufreq_hw 0>;
#cooling-cells = <2>;
next-level-cache = <&L2_300>;
+ operating-points-v2 = <&cpu0_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_300: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -272,6 +288,10 @@
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_400>;
+ operating-points-v2 = <&cpu4_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_400: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -290,6 +310,10 @@
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_500>;
+ operating-points-v2 = <&cpu4_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_500: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -308,6 +332,10 @@
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_600>;
+ operating-points-v2 = <&cpu4_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_600: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -326,6 +354,10 @@
qcom,freq-domain = <&cpufreq_hw 1>;
#cooling-cells = <2>;
next-level-cache = <&L2_700>;
+ operating-points-v2 = <&cpu4_opp_table>;
+ /* path between CPU and DDR memory and CPU and L3 */
+ interconnects = <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_APPSS_PROC &osm_l3 SLAVE_OSM_L3>;
L2_700: l2-cache {
compatible = "cache";
next-level-cache = <&L3_0>;
@@ -423,6 +455,317 @@
};
};
+ cpu0_opp_table: cpu0_opp_table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ cpu0_opp1: opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 762 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 4577 MB/s peak */
+ bandwidth-MBps = <0 762>, <0 4577>;
+ };
+
+ cpu0_opp2: opp-403200000 {
+ opp-hz = /bits/ 64 <403200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 762 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 4577 MB/s peak */
+ bandwidth-MBps = <0 762>, <0 4577>;
+ };
+
+ cpu0_opp3: opp-480000000 {
+ opp-hz = /bits/ 64 <480000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 762 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 6152 MB/s peak */
+ bandwidth-MBps = <0 762>, <0 6152>;
+ };
+
+ cpu0_opp4: opp-576000000 {
+ opp-hz = /bits/ 64 <576000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 762 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 6152 MB/s peak */
+ bandwidth-MBps = <0 762>, <0 6152>;
+ };
+
+ cpu0_opp5: opp-652800000 {
+ opp-hz = /bits/ 64 <652800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 762 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 7324 MB/s peak */
+ bandwidth-MBps = <0 762>, <0 7324>;
+ };
+
+ cpu0_opp6: opp-748800000 {
+ opp-hz = /bits/ 64 <748800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1720 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 8789 MB/s peak */
+ bandwidth-MBps = <0 1720>, <0 8789>;
+ };
+
+ cpu0_opp7: opp-825600000 {
+ opp-hz = /bits/ 64 <825600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1720 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 8789 MB/s peak */
+ bandwidth-MBps = <0 1720>, <0 8789>;
+ };
+
+ cpu0_opp8: opp-902400000 {
+ opp-hz = /bits/ 64 <902400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1720 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 9960 MB/s peak */
+ bandwidth-MBps = <0 1720>, <0 9960>;
+ };
+
+ cpu0_opp9: opp-979200000 {
+ opp-hz = /bits/ 64 <979200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1720 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 11425 MB/s peak */
+ bandwidth-MBps = <0 1720>, <0 11425>;
+ };
+
+ cpu0_opp10: opp-1056000000 {
+ opp-hz = /bits/ 64 <1056000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1720 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 11425 MB/s peak */
+ bandwidth-MBps = <0 1720>, <0 11425>;
+ };
+
+ cpu0_opp11: opp-1132800000 {
+ opp-hz = /bits/ 64 <1132800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2086 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 12890 MB/s peak */
+ bandwidth-MBps = <0 2086>, <0 12890>;
+ };
+
+ cpu0_opp12: opp-1228800000 {
+ opp-hz = /bits/ 64 <1228800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2086 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 14355 MB/s peak */
+ bandwidth-MBps = <0 2086>, <0 14355>;
+ };
+
+ cpu0_opp13: opp-1324800000 {
+ opp-hz = /bits/ 64 <1324800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2086 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 15820 MB/s peak */
+ bandwidth-MBps = <0 2086>, <0 15820>;
+ };
+
+ cpu0_opp14: opp-1420800000 {
+ opp-hz = /bits/ 64 <1420800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2086 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 17285 MB/s peak */
+ bandwidth-MBps = <0 2086>, <0 17285>;
+ };
+
+ cpu0_opp15: opp-1516800000 {
+ opp-hz = /bits/ 64 <1516800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2597 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 2597>, <0 18457>;
+ };
+
+ cpu0_opp16: opp-1612800000 {
+ opp-hz = /bits/ 64 <1612800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 18457>;
+ };
+
+ cpu0_opp17: opp-1689600000 {
+ opp-hz = /bits/ 64 <1689600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 19921>;
+ };
+
+ cpu0_opp18: opp-1766400000 {
+ opp-hz = /bits/ 64 <1766400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 21386 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 21386>;
+ };
+ };
+
+ cpu4_opp_table: cpu4_opp_table {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ cpu4_opp1: opp-825600000 {
+ opp-hz = /bits/ 64 <825600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1144 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 8789 MB/s peak */
+ bandwidth-MBps = <0 1144>, <0 8789>;
+ };
+
+ cpu4_opp2: opp-902400000 {
+ opp-hz = /bits/ 64 <902400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1144 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 8789 MB/s peak */
+ bandwidth-MBps = <0 1144>, <0 8789>;
+ };
+
+ cpu4_opp3: opp-979200000 {
+ opp-hz = /bits/ 64 <979200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 1144 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 8789 MB/s peak */
+ bandwidth-MBps = <0 1144>, <0 8789>;
+ };
+
+ cpu4_opp4: opp-1056000000 {
+ opp-hz = /bits/ 64 <1056000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 2929 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 11425 MB/s peak */
+ bandwidth-MBps = <0 2929>, <0 11425>;
+ };
+
+ cpu4_opp5: opp-1209600000 {
+ opp-hz = /bits/ 64 <1209600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 11425 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 11425>;
+ };
+
+ cpu4_opp6: opp-1286400000 {
+ opp-hz = /bits/ 64 <1286400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 11425 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 11425>;
+ };
+
+ cpu4_opp7: opp-1363200000 {
+ opp-hz = /bits/ 64 <1363200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 14355 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 14355>;
+ };
+
+ cpu4_opp8: opp-1459200000 {
+ opp-hz = /bits/ 64 <1459200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 14355 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 14355>;
+ };
+
+ cpu4_opp9: opp-1536000000 {
+ opp-hz = /bits/ 64 <1536000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 3879 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 14355 MB/s peak */
+ bandwidth-MBps = <0 3879>, <0 14355>;
+ };
+
+ cpu4_opp10: opp-1612800000 {
+ opp-hz = /bits/ 64 <1612800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 4943 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 14355 MB/s peak */
+ bandwidth-MBps = <0 4943>, <0 14355>;
+ };
+
+ cpu4_opp11: opp-1689600000 {
+ opp-hz = /bits/ 64 <1689600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 4943 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 4943>, <0 18457>;
+ };
+
+ cpu4_opp12: opp-1766400000 {
+ opp-hz = /bits/ 64 <1766400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 5931 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 5931>, <0 18457>;
+ };
+
+ cpu4_opp13: opp-1843200000 {
+ opp-hz = /bits/ 64 <1843200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 5931 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 5931>, <0 18457>;
+ };
+
+ cpu4_opp14: opp-1920000000 {
+ opp-hz = /bits/ 64 <1920000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 5931 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 18457 MB/s peak */
+ bandwidth-MBps = <0 5931>, <0 18457>;
+ };
+
+ cpu4_opp15: opp-1996800000 {
+ opp-hz = /bits/ 64 <1996800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 19921>;
+ };
+
+ cpu4_opp16: opp-2092800000 {
+ opp-hz = /bits/ 64 <2092800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 19921>;
+ };
+
+ cpu4_opp17: opp-2169600000 {
+ opp-hz = /bits/ 64 <2169600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 19921>;
+ };
+
+ cpu4_opp18: opp-2246400000 {
+ opp-hz = /bits/ 64 <2246400000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 19921>;
+ };
+
+ cpu4_opp19: opp-2323200000 {
+ opp-hz = /bits/ 64 <2323200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 19921 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 19921>;
+ };
+
+ cpu4_opp20: opp-2400000000 {
+ opp-hz = /bits/ 64 <2400000000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 21386 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 21386>;
+ };
+
+ cpu4_opp21: opp-2476800000 {
+ opp-hz = /bits/ 64 <2476800000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 21386 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 21386>;
+ };
+
+ cpu4_opp22: opp-2553600000 {
+ opp-hz = /bits/ 64 <2553600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 21386 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 21386>;
+ };
+
+ cpu4_opp23: opp-2649600000 {
+ opp-hz = /bits/ 64 <2649600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 21386 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 21386>;
+ };
+
+ cpu4_opp24: opp-2745600000 {
+ opp-hz = /bits/ 64 <2745600000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 22558 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 22558>;
+ };
+
+ cpu4_opp25: opp-2803200000 {
+ opp-hz = /bits/ 64 <2803200000>;
+ /* CPU<->DDR bandwidth: 0 MB/s average, 6881 MB/s peak */
+ /* CPU<->L3 bandwidth: 0 MB/s average, 22558 MB/s peak */
+ bandwidth-MBps = <0 6881>, <0 22558>;
+ };
+ };
+
pmu {
compatible = "arm,armv8-pmuv3";
interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related
* [PATCH RFC 0/4] DDR/L3 Scaling support on SDM845 SoCs
From: Sibi Sankar @ 2019-06-27 13:34 UTC (permalink / raw)
To: viresh.kumar, nm, sboyd, georgi.djakov
Cc: agross, david.brown, robh+dt, mark.rutland, rjw, linux-arm-msm,
devicetree, linux-kernel, linux-pm, saravanak, Sibi Sankar
This RFC series aims to extend cpu based scaling support to L3/DDR on
SDM845 SoCs. The patch series depends on "Introduce OPP bandwidth bindings"
series (https://patchwork.kernel.org/cover/10912993/). A part of the
series will still be applicable if we decide to go ahead with the proposal
from Saravana as well so I decided to post this out.
v2:
* Incorporated Viresh's comments from:
[1]https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
[2]https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
Sibi Sankar (4):
OPP: Add and export helper to update voltage
OPP: Add and export helper to set bandwidth
cpufreq: qcom: Update the bandwidth levels on frequency change
arm64: dts: qcom: sdm845: Add cpu OPP tables
arch/arm64/boot/dts/qcom/sdm845.dtsi | 343 +++++++++++++++++++++++++++
drivers/cpufreq/qcom-cpufreq-hw.c | 77 +++++-
drivers/opp/core.c | 98 ++++++++
include/linux/pm_opp.h | 16 ++
4 files changed, 532 insertions(+), 2 deletions(-)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH 1/2] thermal/drivers/core: Add init section table for self-encapsulation
From: Daniel Lezcano @ 2019-06-27 13:31 UTC (permalink / raw)
To: Zhang Rui; +Cc: linux-kernel, linux-pm, amit.kucheria
In-Reply-To: <1561641692.2096.3.camel@intel.com>
On 27/06/2019 15:21, Zhang Rui wrote:
> On 一, 2019-06-24 at 09:32 +0200, Daniel Lezcano wrote:
>> Any chance this patch gets merged for v5.4?
>>
>> Thanks
>> -- Daniel
>>
>
> have you run compile test for the patch?
> I got the following errors when compiling.
Yes I did and also booted, changed the governor at runtime, etc ... I
already got this error and fixed it. I probably forgot to fold or commit
the fix ... :/
> In file included from drivers/thermal/fair_share.c:16:0:
> drivers/thermal/thermal_core.h:23:3: error: expected identifier or ‘(’
> before ‘static’
> (static typeof(name) *__thermal_table_entry_##name \
> ^
> drivers/thermal/thermal_core.h:26:40: note: in expansion of macro
> ‘THERMAL_TABLE_ENTRY’
> #define THERMAL_GOVERNOR_DECLARE(name) THERMAL_TABLE_ENTRY(governor,
> name)
> ^
[ ... ]
> ^
> make[2]: *** [drivers/thermal/gov_bang_bang.o] Error 1
> make[1]: *** [drivers/thermal] Error 2
> make[1]: *** Waiting for unfinished jobs....
> make: *** [drivers] Error 2
>
> Fix the problem by removing the round brackets
> of THERMAL_TABLE_ENTRY(), and applied.
Ok, thanks for fixing it!
-- Daniel
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply
* [GIT PULL] Immutable branch between MFD, Clk, GPIO, Power, Regulator and RTC due for the v5.3 merge window
From: Lee Jones @ 2019-06-27 13:30 UTC (permalink / raw)
To: Matti Vaittinen
Cc: mazziesaccount, Rob Herring, Mark Rutland, Michael Turquette,
Stephen Boyd, Linus Walleij, Bartosz Golaszewski,
Sebastian Reichel, Liam Girdwood, Mark Brown, Alessandro Zummo,
Alexandre Belloni, devicetree, linux-kernel, linux-clk,
linux-gpio, linux-pm, linux-rtc
In-Reply-To: <cover.1559546139.git.matti.vaittinen@fi.rohmeurope.com>
Enjoy!
The following changes since commit a188339ca5a396acc588e5851ed7e19f66b0ebd9:
Linux 5.2-rc1 (2019-05-19 15:47:09 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git ib-mfd-clk-gpio-power-regulator-rtc-v5.3
for you to fetch changes up to f8c7f7ddd8ef0855d06cff5d1cc7713b556006a7:
power: supply: Initial support for ROHM BD70528 PMIC charger block (2019-06-27 10:57:24 +0100)
----------------------------------------------------------------
Immutable branch between MFD, Clk, GPIO, Power, Regulator and RTC due for the v5.3 merge window
----------------------------------------------------------------
Matti Vaittinen (7):
mfd: regulator: clk: Split rohm-bd718x7.h
mfd: bd70528: Support ROHM bd70528 PMIC core
clk: bd718x7: Support ROHM BD70528 clk block
dt-bindings: mfd: Document first ROHM BD70528 bindings
gpio: Initial support for ROHM bd70528 GPIO block
rtc: bd70528: Initial support for ROHM bd70528 RTC
power: supply: Initial support for ROHM BD70528 PMIC charger block
.../devicetree/bindings/mfd/rohm,bd70528-pmic.txt | 102 +++
drivers/clk/Kconfig | 6 +-
drivers/clk/clk-bd718x7.c | 24 +-
drivers/gpio/Kconfig | 11 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-bd70528.c | 232 +++++++
drivers/mfd/Kconfig | 17 +
drivers/mfd/Makefile | 2 +
drivers/mfd/rohm-bd70528.c | 316 +++++++++
drivers/mfd/rohm-bd718x7.c | 23 +-
drivers/power/supply/Kconfig | 9 +
drivers/power/supply/Makefile | 1 +
drivers/power/supply/bd70528-charger.c | 743 +++++++++++++++++++++
drivers/regulator/bd718x7-regulator.c | 25 +-
drivers/rtc/Kconfig | 8 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-bd70528.c | 500 ++++++++++++++
include/linux/mfd/rohm-bd70528.h | 408 +++++++++++
include/linux/mfd/rohm-bd718x7.h | 22 +-
include/linux/mfd/rohm-generic.h | 20 +
20 files changed, 2425 insertions(+), 46 deletions(-)
create mode 100644 Documentation/devicetree/bindings/mfd/rohm,bd70528-pmic.txt
create mode 100644 drivers/gpio/gpio-bd70528.c
create mode 100644 drivers/mfd/rohm-bd70528.c
create mode 100644 drivers/power/supply/bd70528-charger.c
create mode 100644 drivers/rtc/rtc-bd70528.c
create mode 100644 include/linux/mfd/rohm-bd70528.h
create mode 100644 include/linux/mfd/rohm-generic.h
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH 1/2] thermal/drivers/core: Add init section table for self-encapsulation
From: Zhang Rui @ 2019-06-27 13:21 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-kernel, linux-pm, amit.kucheria
In-Reply-To: <b6bbd5f4-a86a-d649-e3c0-2156f7975291@linaro.org>
On 一, 2019-06-24 at 09:32 +0200, Daniel Lezcano wrote:
> Any chance this patch gets merged for v5.4?
>
> Thanks
> -- Daniel
>
have you run compile test for the patch?
I got the following errors when compiling.
In file included from drivers/thermal/fair_share.c:16:0:
drivers/thermal/thermal_core.h:23:3: error: expected identifier or ‘(’
before ‘static’
(static typeof(name) *__thermal_table_entry_##name \
^
drivers/thermal/thermal_core.h:26:40: note: in expansion of macro
‘THERMAL_TABLE_ENTRY’
#define THERMAL_GOVERNOR_DECLARE(name) THERMAL_TABLE_ENTRY(governor,
name)
^
drivers/thermal/fair_share.c:120:1: note: in expansion of macro
‘THERMAL_GOVERNOR_DECLARE’
THERMAL_GOVERNOR_DECLARE(thermal_gov_fair_share);
^
drivers/thermal/fair_share.c:116:32: warning: ‘thermal_gov_fair_share’
defined but not used [-Wunused-variable]
static struct thermal_governor thermal_gov_fair_share = {
^
make[2]: *** [drivers/thermal/fair_share.o] Error 1
make[2]: *** Waiting for unfinished jobs....
In file included from drivers/thermal/gov_bang_bang.c:14:0:
drivers/thermal/thermal_core.h:23:3: error: expected identifier or ‘(’
before ‘static’
(static typeof(name) *__thermal_table_entry_##name \
^
drivers/thermal/thermal_core.h:26:40: note: in expansion of macro
‘THERMAL_TABLE_ENTRY’
#define THERMAL_GOVERNOR_DECLARE(name) THERMAL_TABLE_ENTRY(governor,
name)
^
drivers/thermal/gov_bang_bang.c:119:1: note: in expansion of macro
‘THERMAL_GOVERNOR_DECLARE’
THERMAL_GOVERNOR_DECLARE(thermal_gov_bang_bang);
^
drivers/thermal/gov_bang_bang.c:115:32: warning:
‘thermal_gov_bang_bang’ defined but not used [-Wunused-variable]
static struct thermal_governor thermal_gov_bang_bang = {
^
make[2]: *** [drivers/thermal/gov_bang_bang.o] Error 1
make[1]: *** [drivers/thermal] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [drivers] Error 2
Fix the problem by removing the round brackets
of THERMAL_TABLE_ENTRY(), and applied.
thanks,
rui
> On 12/06/2019 22:13, Daniel Lezcano wrote:
> >
> > Currently the governors are declared in their respective files but
> > they
> > export their [un]register functions which in turn call the
> > [un]register
> > governors core's functions. That implies a cyclic dependency which
> > is
> > not desirable. There is a way to self-encapsulate the governors by
> > letting
> > them to declare themselves in a __init section table.
> >
> > Define the table in the asm generic linker description like the
> > other
> > tables and provide the specific macros to deal with.
> >
> > Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> > drivers/thermal/thermal_core.h | 15 +++++++++++++++
> > include/asm-generic/vmlinux.lds.h | 11 +++++++++++
> > 2 files changed, 26 insertions(+)
> >
> > diff --git a/drivers/thermal/thermal_core.h
> > b/drivers/thermal/thermal_core.h
> > index 0df190ed82a7..be901e84aa65 100644
> > --- a/drivers/thermal/thermal_core.h
> > +++ b/drivers/thermal/thermal_core.h
> > @@ -15,6 +15,21 @@
> > /* Initial state of a cooling device during binding */
> > #define THERMAL_NO_TARGET -1UL
> >
> > +/* Init section thermal table */
> > +extern struct thermal_governor *__governor_thermal_table[];
> > +extern struct thermal_governor *__governor_thermal_table_end[];
> > +
> > +#define THERMAL_TABLE_ENTRY(table, name) \
> > + (static typeof(name) *__thermal_table_entry_##name
> > \
> > + __used __section(__##table##_thermal_table) = &name)
> > +
> > +#define THERMAL_GOVERNOR_DECLARE(name) THERMAL_TABLE_ENTRY(
> > governor, name)
> > +
> > +#define for_each_governor_table(__governor) \
> > + for (__governor = __governor_thermal_table; \
> > + __governor < __governor_thermal_table_end; \
> > + __governor++)
> > +
> > /*
> > * This structure is used to describe the behavior of
> > * a certain cooling device on a certain trip point
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-
> > generic/vmlinux.lds.h
> > index f8f6f04c4453..8312fdc2b2fa 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -239,6 +239,16 @@
> > #define ACPI_PROBE_TABLE(name)
> > #endif
> >
> > +#ifdef CONFIG_THERMAL
> > +#define THERMAL_TABLE(name)
> > \
> > + . = ALIGN(8);
> > \
> > + __##name##_thermal_table = .;
> > \
> > + KEEP(*(__##name##_thermal_table))
> > \
> > + __##name##_thermal_table_end = .;
> > +#else
> > +#define THERMAL_TABLE(name)
> > +#endif
> > +
> > #define KERNEL_DTB()
> > \
> > STRUCT_ALIGN();
> > \
> > __dtb_start = .;
> > \
> > @@ -609,6 +619,7 @@
> > IRQCHIP_OF_MATCH_TABLE()
> > \
> > ACPI_PROBE_TABLE(irqchip)
> > \
> > ACPI_PROBE_TABLE(timer)
> > \
> > + THERMAL_TABLE(governor)
> > \
> > EARLYCON_TABLE()
> > \
> > LSM_TABLE()
> >
> >
>
^ permalink raw reply
* [PATCH v3 4/4] dt-bindings: clk: armada3700: document the PCIe clock
From: Miquel Raynal @ 2019-06-27 12:52 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, devicetree, Thomas Petazzoni, Antoine Tenart,
Gregory Clement, Maxime Chevallier, Nadav Haklai, Bjorn Helgaas,
Rafael J . Wysocki, linux-pm, Marek Behún, Miquel Raynal
In-Reply-To: <20190627125245.26788-1-miquel.raynal@bootlin.com>
Add a reference to the missing PCIe clock managed by this IP. The
clock resides in the south bridge.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/clock/armada3700-periph-clock.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
index 85972715e593..fbf58c443c04 100644
--- a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
+++ b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
@@ -46,6 +46,7 @@ ID Clock name Description
10 sdio SDIO
11 usb32-sub2-sys USB 2 clock
12 usb32-ss-sys USB 3 clock
+13 pcie PCIe controller
Required properties:
--
2.19.1
^ permalink raw reply related
* [PATCH v3 3/4] dt-bindings: clk: armada3700: fix typo in SoC name
From: Miquel Raynal @ 2019-06-27 12:52 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, devicetree, Thomas Petazzoni, Antoine Tenart,
Gregory Clement, Maxime Chevallier, Nadav Haklai, Bjorn Helgaas,
Rafael J . Wysocki, linux-pm, Marek Behún, Miquel Raynal
In-Reply-To: <20190627125245.26788-1-miquel.raynal@bootlin.com>
This documentation is about Armada 3700 SoCs.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/clock/armada3700-periph-clock.txt | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
index 1e3370ba189f..85972715e593 100644
--- a/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
+++ b/Documentation/devicetree/bindings/clock/armada3700-periph-clock.txt
@@ -9,7 +9,7 @@ bridge.
The peripheral clock consumer should specify the desired clock by
having the clock ID in its "clocks" phandle cell.
-The following is a list of provided IDs for Armada 370 North bridge clocks:
+The following is a list of provided IDs for Armada 3700 North bridge clocks:
ID Clock name Description
-----------------------------------
0 mmc MMC controller
@@ -30,7 +30,7 @@ ID Clock name Description
15 eip97 EIP 97
16 cpu CPU
-The following is a list of provided IDs for Armada 370 South bridge clocks:
+The following is a list of provided IDs for Armada 3700 South bridge clocks:
ID Clock name Description
-----------------------------------
0 gbe-50 50 MHz parent clock for Gigabit Ethernet
--
2.19.1
^ permalink raw reply related
* [PATCH v3 2/4] clk: mvebu: armada-37xx-periph: change suspend/resume time
From: Miquel Raynal @ 2019-06-27 12:52 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, devicetree, Thomas Petazzoni, Antoine Tenart,
Gregory Clement, Maxime Chevallier, Nadav Haklai, Bjorn Helgaas,
Rafael J . Wysocki, linux-pm, Marek Behún, Miquel Raynal
In-Reply-To: <20190627125245.26788-1-miquel.raynal@bootlin.com>
Armada 3700 PCIe IP relies on the PCIe clock managed by this
driver. For reasons related to the PCI core's organization when
suspending/resuming, PCI host controller drivers must reconfigure
their registers at suspend_noirq()/resume_noirq() which happens after
suspend()/suspend_late() and before resume_early()/resume().
Device link support in the clock framework enforce that the clock
driver's resume() callback will be called before the PCIe
driver's. But, any resume_noirq() callback will be called before all
the registered resume() callbacks.
The solution to support PCIe resume operation is to change the
"priority" of this clock driver PM callbacks to "_noirq()".
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/mvebu/armada-37xx-periph.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index 1e18c5a875bd..bee45e43a85f 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -715,8 +715,8 @@ static int __maybe_unused armada_3700_periph_clock_resume(struct device *dev)
}
static const struct dev_pm_ops armada_3700_periph_clock_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(armada_3700_periph_clock_suspend,
- armada_3700_periph_clock_resume)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(armada_3700_periph_clock_suspend,
+ armada_3700_periph_clock_resume)
};
static int armada_3700_periph_clock_probe(struct platform_device *pdev)
--
2.19.1
^ permalink raw reply related
* [PATCH v3 1/4] clk: mvebu: armada-37xx-periph: add PCIe gated clock
From: Miquel Raynal @ 2019-06-27 12:52 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, devicetree, Thomas Petazzoni, Antoine Tenart,
Gregory Clement, Maxime Chevallier, Nadav Haklai, Bjorn Helgaas,
Rafael J . Wysocki, linux-pm, Marek Behún, Miquel Raynal
In-Reply-To: <20190627125245.26788-1-miquel.raynal@bootlin.com>
The PCIe clock is a gated clock which has the same source as GbE0
(both IPs share a set of registers). This source clock is called
'gbe_core' in the driver.
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
drivers/clk/mvebu/armada-37xx-periph.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/clk/mvebu/armada-37xx-periph.c b/drivers/clk/mvebu/armada-37xx-periph.c
index 4c20093a42fb..1e18c5a875bd 100644
--- a/drivers/clk/mvebu/armada-37xx-periph.c
+++ b/drivers/clk/mvebu/armada-37xx-periph.c
@@ -304,6 +304,7 @@ PERIPH_CLK_GATE_DIV(gbe_bm, 12, DIV_SEL1, 0, clk_table1);
PERIPH_CLK_FULL_DD(sdio, 11, 14, DIV_SEL0, DIV_SEL0, 3, 6);
PERIPH_CLK_FULL_DD(usb32_usb2_sys, 16, 16, DIV_SEL0, DIV_SEL0, 9, 12);
PERIPH_CLK_FULL_DD(usb32_ss_sys, 17, 18, DIV_SEL0, DIV_SEL0, 15, 18);
+static PERIPH_GATE(pcie, 14);
static struct clk_periph_data data_sb[] = {
REF_CLK_MUX_DD(gbe_50),
@@ -319,6 +320,7 @@ static struct clk_periph_data data_sb[] = {
REF_CLK_FULL_DD(sdio),
REF_CLK_FULL_DD(usb32_usb2_sys),
REF_CLK_FULL_DD(usb32_ss_sys),
+ REF_CLK_GATE(pcie, "gbe_core"),
{ },
};
--
2.19.1
^ permalink raw reply related
* [PATCH v3 0/4] Prepare Armada 3700 PCIe suspend to RAM support
From: Miquel Raynal @ 2019-06-27 12:52 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland
Cc: linux-clk, devicetree, Thomas Petazzoni, Antoine Tenart,
Gregory Clement, Maxime Chevallier, Nadav Haklai, Bjorn Helgaas,
Rafael J . Wysocki, linux-pm, Marek Behún, Miquel Raynal
Hello,
As part of an effort to bring suspend to RAM support to the Armada
3700 SoC (main target: ESPRESSObin board), there are small things to
do in the Armada 3700 peripherals clock driver:
* On this SoC, the PCIe controller gets fed by a gated clock in the
south bridge. This clock is missing in the current driver, patch 1
adds it.
* Because of a constraint in the PCI core, the resume function of a
PCIe controller driver must be run at an early stage
(->suspend/resume_noirq()), before the core tries to ->read/write()
in the PCIe registers to do more configuration. Hence, the PCIe
clock must be resumed before. This is enforced thanks to two
changes:
1/ Add device links to the clock framework. This enforce order in
the PM core: the clocks are resumed before the consumers. Series
has been posted, see [1].
2/ Even with the above feature, the clock's resume() callback is
called after the PCI controller's resume_noirq() callback. The
only way to fix this is to change the "priority" of the clock
suspend/resume callbacks. This is done in patch 2.
* The bindings are updated with the PCI clock in patch 4 while patch 3
is just a typo correction in the same file.
If there is anything unclear please feel free to ask.
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2018-November/614527.html
Thanks,
Miquèl
Changes in v3:
==============
* Fixed one typo: s/their register/their registers/.
Changes in v2:
==============
* Rebased on top of v5.2-rc1.
* Added Rob's R-by tags.
* No change on the "change suspend/resume time" patch as, despite my
pings, I got no answer and IMHO the proposed approach is entirely
valid.
Miquel Raynal (4):
clk: mvebu: armada-37xx-periph: add PCIe gated clock
clk: mvebu: armada-37xx-periph: change suspend/resume time
dt-bindings: clk: armada3700: fix typo in SoC name
dt-bindings: clk: armada3700: document the PCIe clock
.../devicetree/bindings/clock/armada3700-periph-clock.txt | 5 +++--
drivers/clk/mvebu/armada-37xx-periph.c | 6 ++++--
2 files changed, 7 insertions(+), 4 deletions(-)
--
2.19.1
^ permalink raw reply
* Re: [PATCH RESEND] PCI: disable runtime PM for PLX switches
From: Alexander Fomichev @ 2019-06-27 11:06 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Bjorn Helgaas, Mika Westerberg, Linux PCI, linux,
Rafael J. Wysocki, Linux PM, Logan Gunthorpe
In-Reply-To: <CAJZ5v0gZhgMy+oQMpEGM8bsU_57w7i3iCP1nb9PcOHxENfnwAw@mail.gmail.com>
Hi.
On Wed, Apr 24, 2019 at 11:09:52PM +0200, Rafael J. Wysocki wrote:
> On Wed, Apr 24, 2019 at 8:01 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Wed, Apr 24, 2019 at 05:58:19PM +0300, Mika Westerberg wrote:
> > > On Wed, Apr 24, 2019 at 09:11:48AM -0500, Bjorn Helgaas wrote:
> > > > - Maybe the PCI sysfs accessors (pci_mmap_resource(), etc) should
> > > > turn off runtime PM? If we allow mmap of a BAR and then put the
> > > > device in D3hot, that seems like a bug that could affect lots of
> > > > things. But maybe that's already done magically elsewhere?
> > >
> > > IIRC there is no PM magic happening for MMIO userspace accesses.
> > >
> > > What you suggest above sounds like a good way to fix it. We already do
> > > similar for config space access from userspace (if the device is in
> > > D3cold) so definitely makes sense to do the same for MMIO. However, I
> > > don't think we need to disable runtime PM - it should be enough to
> > > increase the reference count (pm_runtime_get_sync() and friends) during
> > > the time the MMIO resource is mmapped.
> >
> > OK, so if I understand correctly this would be basically adding
> > pm_runtime_get_sync(dev) in pci_mmap_resource(). I don't know what
> > the unmap path is, but there would have to be a matching
> > pm_runtime_put() somewhere.
>
> Right.
>
> > And a similar change in the read/write path for /sys/.../resource<N>;
> > I think this must be related to the sysfs_create_bin_file() call in
> > pci_create_attr(), but I don't see the path where the actual
> > read/write to the device is done.
> >
> > And probably something similar should be done in pci_resource_io(),
> > pci_map_rom(), and pci_unmap_rom().
>
> In general, every path in which there is a memory or IO address space
> access requires pm_runtime_get_sync()/pm_runtime_put() around it as
> these accesses are only guaranteed to work in D0.
>
Tested a solution based on proposals by Logan, Bjorn, Mika, Rafael (thanks all
of you, guys), I managed to fix the problem inside the PLX driver code. So no
additional quirks or other modifications in Linux kernel needed. I think
my patch can be easily rejected.
Thanks for help.
--
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH v4 4/5] Documentation: devicetree: add PPMU events description
From: Lukasz Luba @ 2019-06-27 10:31 UTC (permalink / raw)
To: Chanwoo Choi, Krzysztof Kozlowski, cwchoi00
Cc: devicetree, linux-kernel, linux-pm,
linux-samsung-soc@vger.kernel.org, linux-arm-kernel,
Bartłomiej Żołnierkiewicz, robh+dt, mark.rutland,
kyungmin.park, Marek Szyprowski, s.nawrocki, myungjoo.ham, kgene,
willy.mh.wolff.ml
In-Reply-To: <99a47066-3713-77fa-4afb-6f2f17a2721a@samsung.com>
Hi Chanwoo,
On 6/27/19 3:11 AM, Chanwoo Choi wrote:
> Hi Lukasz,
>
> On 19. 6. 26. 오후 11:17, Lukasz Luba wrote:
>> Hi Krzysztof,
>>
>> On 6/26/19 4:03 PM, Krzysztof Kozlowski wrote:
>>> On Wed, 26 Jun 2019 at 15:58, Lukasz Luba <l.luba@partner.samsung.com> wrote:
>>>>
>>>> Hi Chanwoo,
>>>>
>>>> On 6/26/19 10:23 AM, Chanwoo Choi wrote:
>>>>> Hi Lukasz,
>>>>>
>>>>> 2019년 6월 5일 (수) 18:14, Lukasz Luba <l.luba@partner.samsung.com
>>>>> <mailto:l.luba@partner.samsung.com>>님이 작성:
>>>>>
>>>>> Extend the documenation by events description with new 'event-data-type'
>>>>> field. Add example how the event might be defined in DT.
>>>>>
>>>>> Signed-off-by: Lukasz Luba <l.luba@partner.samsung.com
>>>>> <mailto:l.luba@partner.samsung.com>>
>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com
>>>>> <mailto:cw00.choi@samsung.com>>
>>>>> ---
>>>>> .../bindings/devfreq/event/exynos-ppmu.txt | 26 +++++++++++++++++--
>>>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>>>
>>>>>
>>>>>
>>>>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com
>>>>
>>>> Thank you for the ACKs for this a 2/5 patch.
>>>> Do you think the v4 could be merged now?
>>>
>>> I think you have all necessary acks. I can take the DTS patch (5/5)
>>> although probably for next merge window as I just sent one.
>> There was one patch 3/5
>> https://protect2.fireeye.com/url?k=82dd0d0cbe2abd04.82dc8643-d13ecd7e5f989b8d&u=https://lkml.org/lkml/2019/6/5/215
>> which was waiting ACK or I missed the email somehow.
>
> When I was in vacation, your patches are removed on my email account
> because of the email expiration. So, I replied with my Ack through
> gmail account on mobile phone. But, there are some problem. My reply
> didn't arrive the mailing list.
>
> I have no any way to reply about this at company. After leaving one's
> office, I'll reply with Ack again at home.
>
OK, no worries, it is not an emergency issue.
Regards,
Lukasz
^ permalink raw reply
* Re: [PATCH V2 2/5] cpufreq: Replace few CPUFREQ_CONST_LOOPS checks with has_target()
From: Rafael J. Wysocki @ 2019-06-27 9:52 UTC (permalink / raw)
To: Viresh Kumar
Cc: Rafael Wysocki, Linux PM, Vincent Guittot,
Linux Kernel Mailing List
In-Reply-To: <20190627050048.b44kitdfuenxnzfi@vireshk-i7>
On Thu, Jun 27, 2019 at 7:00 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 20-06-19, 08:35, Viresh Kumar wrote:
> > > CPUFREQ_CONST_LOOPS was introduced in a very old commit from pre-2.6
> > > kernel release commit 6a4a93f9c0d5 ("[CPUFREQ] Fix 'out of sync'
> > > issue").
> > >
> > > Probably the initial idea was to just avoid these checks for set_policy
> > > type drivers and then things got changed over the years. And it is very
> > > unclear why these checks are there at all.
> > >
> > > Replace the CPUFREQ_CONST_LOOPS check with has_target(), which makes
> > > more sense now.
> > >
> > > cpufreq_notify_transition() is only called for has_target() type driver
> > > and not for set_policy type, and the check is simply redundant. Remove
> > > it as well.
> > >
> > > Also remove () around freq comparison statement as they aren't required
> > > and checkpatch also warns for them.
> > >
> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> > > ---
> > > drivers/cpufreq/cpufreq.c | 13 +++++--------
> > > 1 file changed, 5 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> > > index 54befd775bd6..41ac701e324f 100644
> > > --- a/drivers/cpufreq/cpufreq.c
> > > +++ b/drivers/cpufreq/cpufreq.c
> > > @@ -359,12 +359,10 @@ static void cpufreq_notify_transition(struct cpufreq_policy *policy,
> > > * which is not equal to what the cpufreq core thinks is
> > > * "old frequency".
> > > */
> > > - if (!(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> > > - if (policy->cur && (policy->cur != freqs->old)) {
> > > - pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > > - freqs->old, policy->cur);
> > > - freqs->old = policy->cur;
> > > - }
> > > + if (policy->cur && policy->cur != freqs->old) {
> > > + pr_debug("Warning: CPU frequency is %u, cpufreq assumed %u kHz\n",
> > > + freqs->old, policy->cur);
> > > + freqs->old = policy->cur;
> > > }
> > >
> > > srcu_notifier_call_chain(&cpufreq_transition_notifier_list,
> > > @@ -1618,8 +1616,7 @@ static unsigned int __cpufreq_get(struct cpufreq_policy *policy)
> > > if (policy->fast_switch_enabled)
> > > return ret_freq;
> > >
> > > - if (ret_freq && policy->cur &&
> > > - !(cpufreq_driver->flags & CPUFREQ_CONST_LOOPS)) {
> > > + if (has_target() && ret_freq && policy->cur) {
> > > /* verify no discrepancy between actual and
> > > saved value exists */
> > > if (unlikely(ret_freq != policy->cur)) {
>
> @Rafael: Here are your comments from the IRC exchange we had
> yesterday:
>
> > <rafael>:
> >
> > so the problem is that, because of the CPUFREQ_CONST_LOOPS check in
> > __cpufreq_get(), it almost never does the cpufreq_out_of_sync() thing
> > now. Because many drivers set CPUFREQ_CONST_LOOPS most of the time,
> > some of them even unconditionally. This patch changes the code that
> > runs very rarely into code that runs relatively often.
>
> Right, we will do the frequency verification on has_target() platforms
> with CPUFREQ_CONST_LOOPS set after this patch. But why is it the wrong
> thing to do ?
Well, my point was exactly what I said.
The patch pretended to be a cleanup and changed the code in a
meaningful way (at least for some drivers).
> What we do here is that we verify that the cached value of current
> frequency is same as the real frequency the hardware is running at. It
> makes sense to not do this check for setpolicy type drivers as the
> cpufreq core isn't always aware of what the driver will end up doing
> with the frequency and so no verification.
>
> But for has_target() type drivers, cpufreq core caches the value with
> it and it should check it to make sure everything is fine. I don't see
> a correlation with CPUFREQ_CONST_LOOPS flag here, that's it. Either we
> do this verification or we don't, but there is no reason (as per my
> understanding) of skipping it using this flag.
>
> So if you look at the commit I pointed in the history git [1], it does
> two things:
> - It adds the verification code (which is quite similar today as
> well).
> - And it sets the CPUFREQ_CONST_LOOPS flag only for setpolicy drivers,
> rightly so.
>
> The problem happened when we started to use CPUFREQ_CONST_LOOPS for
> constant loops-per-jiffy thing as well and many has_target() drivers
> started using the same flag and unknowingly skipped the verification
> of frequency.
>
> So, I think the current code is doing the wrong thing by skipping the
> verification using CPUFREQ_CONST_LOOPS flag.
All right then, thanks for explaining it here.
The patch is a bug fix, not a cleanup, and it fixes the changes that
caused CPUFREQ_CONST_LOOPS to be used for a different purpose without
adjusting the original code accordingly.
I can agree with this rationale, but please fix the changelog.
^ permalink raw reply
* Re: [PATCH v2 3/5] OPP: Add support for parsing the interconnect bandwidth
From: Sibi Sankar @ 2019-06-27 6:27 UTC (permalink / raw)
To: Viresh Kumar, Georgi Djakov
Cc: vireshk, sboyd, nm, robh+dt, mark.rutland, rjw, jcrouse,
vincent.guittot, bjorn.andersson, amit.kucheria, seansw,
daidavid1, evgreen, linux-pm, devicetree, linux-kernel,
linux-arm-msm
In-Reply-To: <20190424055208.kermzymve2foguhl@vireshk-i7>
Hey Georgi,
In addition to Viresh's comments I found a few more while testing the
series on SDM845.
On 4/24/19 11:22 AM, Viresh Kumar wrote:
> On 23-04-19, 16:28, Georgi Djakov wrote:
>> The OPP bindings now support bandwidth values, so add support to parse it
>> from device tree and store it into the new dev_pm_opp_icc_bw struct, which
>> is part of the dev_pm_opp.
>>
>> Also add and export the dev_pm_opp_set_paths() and dev_pm_opp_put_paths()
>> helpers, to set (and release) an interconnect paths to a device. The
>> bandwidth of these paths will be updated when the OPPs are switched.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> drivers/opp/core.c | 87 ++++++++++++++++++++++++++++++++++-
>> drivers/opp/of.c | 102 +++++++++++++++++++++++++++++++++++++++++
>> drivers/opp/opp.h | 9 ++++
>> include/linux/pm_opp.h | 14 ++++++
>> 4 files changed, 210 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 0420f7e8ad5b..97ee39ecdebd 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -19,6 +19,7 @@
>> #include <linux/slab.h>
>> #include <linux/device.h>
>> #include <linux/export.h>
>> +#include <linux/interconnect.h>
>
> Just include this once in opp.h and the other .c files won't need it.
>
>> #include <linux/pm_domain.h>
>> #include <linux/regulator/consumer.h>
>>
>> @@ -876,6 +877,8 @@ static struct opp_table *_allocate_opp_table(struct device *dev, int index)
>> ret);
>> }
>>
>> + _of_find_paths(opp_table, dev);
>> +
>> BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
>> INIT_LIST_HEAD(&opp_table->opp_list);
>> kref_init(&opp_table->kref);
>> @@ -1129,11 +1132,12 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
>> struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>> {
>> struct dev_pm_opp *opp;
>> - int count, supply_size;
>> + int count, supply_size, icc_size;
>>
>> /* Allocate space for at least one supply */
>> count = table->regulator_count > 0 ? table->regulator_count : 1;
>> supply_size = sizeof(*opp->supplies) * count;
>> + icc_size = sizeof(*opp->bandwidth) * table->path_count;
>>
>> /* allocate new OPP node and supplies structures */
>> opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
>
> You never updated this to include icc_size :(
>
>> @@ -1141,7 +1145,8 @@ struct dev_pm_opp *_opp_allocate(struct opp_table *table)
>> return NULL;
>>
>> /* Put the supplies at the end of the OPP structure as an empty array */
>> - opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
>> + opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp + 1);
>
> Keep the order as supplies and then bandwidth.
>
>> + opp->supplies = (struct dev_pm_opp_supply *)(opp + icc_size + 1);
opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + 1);
>
> Did you check what address gets assigned here ? I think the pointer
> addition will screw things up for you.
>
>> INIT_LIST_HEAD(&opp->node);
>>
>> return opp;
>> @@ -1637,6 +1642,84 @@ void dev_pm_opp_put_clkname(struct opp_table *opp_table)
>> }
>> EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname);
>>
>> +/**
>> + * dev_pm_opp_set_paths() - Set interconnect path for a device
>> + * @dev: Device for which interconnect path is being set.
>> + *
>> + * This must be called before any OPPs are initialized for the device.
>> + */
>> +struct opp_table *dev_pm_opp_set_paths(struct device *dev)
>
> I got a bit confused. Why is this routine required exactly as
> _of_find_paths() would have already done something similar ?
>
>> +{
>> + struct opp_table *opp_table;
>> + int ret, i;
>> +
>> + opp_table = dev_pm_opp_get_opp_table(dev);
>> + if (!opp_table)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + /* This should be called before OPPs are initialized */
>> + if (WARN_ON(!list_empty(&opp_table->opp_list))) {
>> + ret = -EBUSY;
>> + goto err;
>> + }
>> +
>> + /* Another CPU that shares the OPP table has set the path */
>> + if (opp_table->paths)
>> + return opp_table;
>> + >> + opp_table->paths = kmalloc_array(opp_table->path_count,
of_find_paths might have failed so you would want to
re-calculate opp_table->path_count.
>> + sizeof(*opp_table->paths), GFP_KERNEL);
>> +
>> + /* Find interconnect path(s) for the device */
>> + for (i = 0; i < opp_table->path_count; i++) {
>> + opp_table->paths[i] = of_icc_get_by_index(dev, i);
>> + if (IS_ERR(opp_table->paths[i])) {
>> + ret = PTR_ERR(opp_table->paths[i]);
>> + if (ret != -EPROBE_DEFER)
>> + dev_err(dev, "%s: Couldn't find path%d: %d\n",
>> + __func__, i, ret);
we should clean up by call icc_put on the paths that succeeded
and free/set the opp_table->paths to NULL.
>> + goto err;
>> + }
>> + }
>> +
>> + return opp_table;
>> +
>> +err:
>> + dev_pm_opp_put_opp_table(opp_table);
>> +
>> + return ERR_PTR(ret);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_set_paths);
>> +
>> +/**
>> + * dev_pm_opp_put_paths() - Release interconnect path resources
>> + * @opp_table: OPP table returned from dev_pm_opp_set_paths().
>> + */
>> +void dev_pm_opp_put_paths(struct opp_table *opp_table)
>> +{
>> + int i;
>> +
>> + if (!opp_table->paths) {
>> + pr_err("%s: Doesn't have paths set\n", __func__);
>> + return;
>> + }
>> +
>> + /* Make sure there are no concurrent readers while updating opp_table */
>> + WARN_ON(!list_empty(&opp_table->opp_list));
>> +
>> + for (i = opp_table->path_count - 1; i >= 0; i--)
>> + icc_put(opp_table->paths[i]);
>> +
>> + _free_set_opp_data(opp_table);
>> +
>> + kfree(opp_table->paths);
>> + opp_table->paths = NULL;
>> + opp_table->path_count = 0;
>> +
>> + dev_pm_opp_put_opp_table(opp_table);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_opp_put_paths);
>> +
>> /**
>> * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper
>> * @dev: Device for which the helper is getting registered.
>> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
>> index c10c782d15aa..00af23280bc6 100644
>> --- a/drivers/opp/of.c
>> +++ b/drivers/opp/of.c
>> @@ -16,6 +16,7 @@
>> #include <linux/cpu.h>
>> #include <linux/errno.h>
>> #include <linux/device.h>
>> +#include <linux/interconnect.h>
>> #include <linux/of_device.h>
>> #include <linux/pm_domain.h>
>> #include <linux/slab.h>
>> @@ -363,6 +364,45 @@ static int _of_opp_alloc_required_opps(struct opp_table *opp_table,
>> return ret;
>> }
>>
>> +int _of_find_paths(struct opp_table *opp_table, struct device *dev)
>> +{
>> + struct device_node *np;
>> + int ret, i, count, num_paths;
>> +
>> + np = of_node_get(dev->of_node);
>> + if (np) {
>
> I would rather do:
>
> if (!np)
> return 0;
>
> That will kill unnecessary line breaks and indentation.
>
>> + count = of_count_phandle_with_args(np, "interconnects",
>> + "#interconnect-cells");
count will be an error code if interconnect property is missing
so we need to account for that as well.
>
> You can do of_node_put() right here as it isn't used afterwards.
>
>> + if (count % 2) {
>
> Shouldn't this be 4 instead of 2 ? Each path is represented as:
>
> <&noc MASTER_CPU &noc SLAVE_DDR>
>
> which has 4 fields.
>
>> + dev_err(dev, "%s: Invalid interconnects values\n",
>> + __func__);
>> + ret = -EINVAL;
>> + goto put_of_node;
>> + }
>> +
>> + num_paths = count / 2;
>> + opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
>> + GFP_KERNEL);
>> + if (!opp_table->paths) {
>> + ret = -ENOMEM;
>> + goto put_of_node;
>> + }
>> +
>> + for (i = 0; i < num_paths; i++)
>> + opp_table->paths[i] = of_icc_get_by_index(dev, i);
we should clean up by call icc_put on the paths that succeeded
and free/set the opp_table->paths to NULL.
>> +
>> + opp_table->path_count = num_paths;
>> + of_node_put(np);
>> + }
>> +
>> + return 0;
>> +
>> +put_of_node:
>> + of_node_put(np);
>> +
>> + return ret;
>> +}
>> +
>> static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
>> struct device_node *np)
>> {
>> @@ -539,6 +579,64 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev,
>> return ret;
>> }
>>
>> +static int opp_parse_icc_bw(struct dev_pm_opp *opp, struct device *dev,
>> + struct opp_table *opp_table)
>> +{
>> + struct property *prop = NULL;
>> + u32 *bandwidth;
>> + char name[] = "bandwidth-MBps";
>> + int count, i, j, ret;
>> +
>> + /* Search for "bandwidth-MBps" */
>> + prop = of_find_property(opp->np, name, NULL);
>> +
>> + /* Missing property is not a problem */
>> + if (!prop) {
>> + dev_dbg(dev, "%s: Missing %s property\n", __func__, name);
>> + return 0;
>> + }
>> +
>> + if (!prop->value) {
>> + dev_dbg(dev, "%s: Missing %s value\n", __func__, name);
>> + return -ENODATA;
>> + }
>> +
>> + /*
>> + * Bandwidth consists of average and peak values like:
>> + * bandwidth-MBps = <avg-MBps peak-MBps>
>> + */
>> + count = prop->length / sizeof(u32);
>> + if (count % 2) {
>> + dev_err(dev, "%s: Invalid %s values\n", __func__, name);
>> + return -EINVAL;
>> + }
>> +
>> + if (opp_table->path_count != count / 2) {
>> + dev_err(dev, "%s Mismatch between values and paths (%d %d)\n",
>> + __func__, opp_table->path_count, count / 2);
>> + return -EINVAL;
>> + }
>> +
>> + bandwidth = kmalloc_array(count, sizeof(*bandwidth), GFP_KERNEL);
>> + if (!bandwidth)
>> + return -ENOMEM;
>> +
>> + ret = of_property_read_u32_array(opp->np, name, bandwidth, count);
>> + if (ret) {
>> + dev_err(dev, "%s: Error parsing %s: %d\n", __func__, name, ret);
>> + goto free_bandwidth;
>> + }
>> + for (i = 0, j = 0; i < count; i++) {
>> + opp->bandwidth[i].avg = MBps_to_icc(bandwidth[j++]);
>> + opp->bandwidth[i].peak = MBps_to_icc(bandwidth[j++]);
>> + }
>> +
>> +free_bandwidth:
>> + kfree(bandwidth);
>> +
>> + return ret;
>> +}
>> +
>> /**
>> * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT
>> * entries
>> @@ -635,6 +733,10 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
>> if (opp_table->is_genpd)
>> new_opp->pstate = pm_genpd_opp_to_performance_state(dev, new_opp);
>>
>> + ret = opp_parse_icc_bw(new_opp, dev, opp_table);
>> + if (ret)
>> + goto free_opp;
>> +
>> ret = _opp_add(dev, new_opp, opp_table, rate_not_available);
>> if (ret) {
>> /* Don't return error for duplicate OPPs */
>> diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
>> index 569b3525aa67..70a537f2dbd3 100644
>> --- a/drivers/opp/opp.h
>> +++ b/drivers/opp/opp.h
>> @@ -24,6 +24,7 @@
>>
>> struct clk;
>> struct regulator;
>> +struct icc_path;
>>
>> /* Lock to allow exclusive modification to the device and opp lists */
>> extern struct mutex opp_table_lock;
>> @@ -62,6 +63,7 @@ extern struct list_head opp_tables;
>> * @rate: Frequency in hertz
>> * @level: Performance level
>> * @supplies: Power supplies voltage/current values
>> + * @bandwidth: Interconnect bandwidth values
>> * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
>> * frequency from any other OPP's frequency.
>> * @required_opps: List of OPPs that are required by this OPP.
>> @@ -84,6 +86,7 @@ struct dev_pm_opp {
>> unsigned int level;
>>
>> struct dev_pm_opp_supply *supplies;
>> + struct dev_pm_opp_icc_bw *bandwidth;
>>
>> unsigned long clock_latency_ns;
>>
>> @@ -150,6 +153,8 @@ enum opp_table_access {
>> * @regulator_count: Number of power supply regulators. Its value can be -1
>> * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
>> * property).
>> + * @paths: Interconnect path handles
>> + * @path_count: Number of interconnect paths
>> * @genpd_performance_state: Device's power domain support performance state.
>> * @is_genpd: Marks if the OPP table belongs to a genpd.
>> * @set_opp: Platform specific set_opp callback
>> @@ -194,6 +199,8 @@ struct opp_table {
>> struct clk *clk;
>> struct regulator **regulators;
>> int regulator_count;
>> + struct icc_path **paths;
>> + unsigned int path_count;
>> bool genpd_performance_state;
>> bool is_genpd;
>>
>> @@ -228,12 +235,14 @@ void _of_clear_opp_table(struct opp_table *opp_table);
>> struct opp_table *_managed_opp(struct device *dev, int index);
>> void _of_opp_free_required_opps(struct opp_table *opp_table,
>> struct dev_pm_opp *opp);
>> +int _of_find_paths(struct opp_table *opp_table, struct device *dev);
>> #else
>> static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
>> static inline void _of_clear_opp_table(struct opp_table *opp_table) {}
>> static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
>> static inline void _of_opp_free_required_opps(struct opp_table *opp_table,
>> struct dev_pm_opp *opp) {}
>> +static inline int _of_find_paths(struct opp_table *opp_table, struct device *dev) { return 0; }
>> #endif
>>
>> #ifdef CONFIG_DEBUG_FS
>> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
>> index 24c757a32a7b..dabee09a92b8 100644
>> --- a/include/linux/pm_opp.h
>> +++ b/include/linux/pm_opp.h
>> @@ -43,6 +43,18 @@ struct dev_pm_opp_supply {
>> unsigned long u_amp;
>> };
>>
>> +/**
>> + * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
>> + * @avg: Average bandwidth corresponding to this OPP (in icc units)
>> + * @peak: Peak bandwidth corresponding to this OPP (in icc units)
>> + *
>> + * This structure stores the bandwidth values for a single interconnect path.
>> + */
>> +struct dev_pm_opp_icc_bw {
>> + u32 avg;
>> + u32 peak;
>> +};
>> +
>> /**
>> * struct dev_pm_opp_info - OPP freq/voltage/current values
>> * @rate: Target clk rate in hz
>> @@ -127,6 +139,8 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * con
>> void dev_pm_opp_put_regulators(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char * name);
>> void dev_pm_opp_put_clkname(struct opp_table *opp_table);
>> +struct opp_table *dev_pm_opp_set_paths(struct device *dev);
>> +void dev_pm_opp_put_paths(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data));
>> void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table);
>> struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index);
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox