* [PATCH v2 16/17] PM / devfreq: tegra30: Use kHz units uniformly in the code
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
Now that all kHz-conversion related bugs are fixed, we can use the kHz
uniformly. This makes code cleaner and avoids integer divisions in the
code, which is useful in a case of Tegra30 that has Cortex A9 CPU that
doesn't support integer division instructions, hence all divisions are
actually made in software mode. Another small benefit from this change
is that now powertop utility correctly displays devfreq's stats, for
some reason it expects them to be in kHz.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 78 +++++++++++++++++--------------
1 file changed, 43 insertions(+), 35 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index c03135000af7..778c5a940256 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -72,6 +72,8 @@
#define KHZ 1000
+#define KHZ_MAX (ULONG_MAX / KHZ)
+
/* Assume that the bus is saturated if the utilization is 25% */
#define BUS_SATURATION_RATIO 25
@@ -124,7 +126,7 @@ static const struct tegra_devfreq_device_config actmon_device_configs[] = {
.boost_down_coeff = 90,
.boost_up_threshold = 27,
.boost_down_threshold = 10,
- .avg_dependency_threshold = 50000,
+ .avg_dependency_threshold = 50000 / ACTMON_SAMPLING_PERIOD,
},
};
@@ -137,8 +139,11 @@ struct tegra_devfreq_device {
const struct tegra_devfreq_device_config *config;
void __iomem *regs;
- /* Average event count sampled in the last interrupt */
- u32 avg_count;
+ /*
+ * Average event count sampled in the last interrupt and converted
+ * to frequency value.
+ */
+ u32 avg_freq;
/*
* Extra frequency to increase the target by due to consecutive
@@ -172,7 +177,7 @@ struct tegra_actmon_emc_ratio {
};
static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
- { 1400000, ULONG_MAX / KHZ },
+ { 1400000, KHZ_MAX },
{ 1200000, 750000 },
{ 1100000, 600000 },
{ 1000000, 500000 },
@@ -234,7 +239,7 @@ tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
unsigned long static_cpu_emc_freq;
if (dev->config->avg_dependency_threshold &&
- dev->config->avg_dependency_threshold < dev->avg_count) {
+ dev->config->avg_dependency_threshold < dev->avg_freq) {
static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra);
target_freq = max(target_freq, static_cpu_emc_freq);
}
@@ -265,7 +270,7 @@ static unsigned long tegra_actmon_upper_freq(struct tegra_devfreq *tegra,
opp = dev_pm_opp_find_freq_ceil(tegra->devfreq->dev.parent, &upper);
if (IS_ERR(opp))
- upper = ULONG_MAX;
+ upper = KHZ_MAX;
else
dev_pm_opp_put(opp);
@@ -287,7 +292,7 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
* range in a case where target_freq falls into a range of
* next_possible_opp_freq - 1MHz.
*/
- target_freq = round_down(target_freq, 1000000);
+ target_freq = round_down(target_freq, 1000);
/* watermarks are set at the borders of the corresponding OPPs */
*lower = tegra_actmon_lower_freq(tegra, target_freq);
@@ -296,9 +301,6 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
offset, target_freq, *lower, *upper);
- *lower /= KHZ;
- *upper /= KHZ;
-
/*
* The upper watermark should take into account CPU's frequency
* because cpu_to_emc_rate() may override the target_freq with
@@ -314,22 +316,23 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- unsigned long lower, upper, freq;
+ unsigned long avg_threshold, lower, upper;
- freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
- tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
+ tegra_actmon_get_lower_upper(tegra, dev, dev->avg_freq, &lower, &upper);
+
+ avg_threshold = dev->config->avg_dependency_threshold;
+ avg_threshold = avg_threshold * ACTMON_SAMPLING_PERIOD;
/*
* We want to get interrupts when MCCPU client crosses the
* dependency threshold in order to take into / out of account
* the CPU's freq.
*/
- if (lower < dev->config->avg_dependency_threshold &&
- upper > dev->config->avg_dependency_threshold) {
- if (dev->avg_count < dev->config->avg_dependency_threshold)
- upper = dev->config->avg_dependency_threshold;
+ if (lower < avg_threshold && upper > avg_threshold) {
+ if (dev->avg_freq < dev->config->avg_dependency_threshold)
+ upper = avg_threshold;
else
- lower = dev->config->avg_dependency_threshold;
+ lower = avg_threshold;
}
device_writel(dev, lower, ACTMON_DEV_AVG_LOWER_WMARK);
@@ -368,8 +371,7 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
* device. Once that mark is hit and boosting is stopped, the
* interrupt is disabled by ISR.
*/
- freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
- tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
+ tegra_actmon_get_lower_upper(tegra, dev, dev->avg_freq, &lower, &upper);
delta = do_percent(upper - lower, dev->config->boost_down_threshold);
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
@@ -396,14 +398,16 @@ static void actmon_device_debug(struct tegra_devfreq *tegra,
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 intr_status, dev_ctrl, avg_intr_mask;
+ u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
actmon_device_debug(tegra, dev, "isr+");
- dev->avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
+ avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
+ dev->avg_freq = avg_count / ACTMON_SAMPLING_PERIOD;
+
avg_intr_mask = ACTMON_DEV_INTR_AVG_BELOW_WMARK |
ACTMON_DEV_INTR_AVG_ABOVE_WMARK;
@@ -457,8 +461,7 @@ static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{ unsigned long target_freq;
- target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
- target_freq = min(target_freq, ULONG_MAX / KHZ);
+ target_freq = min(dev->avg_freq + dev->boost_freq, KHZ_MAX);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
return target_freq;
@@ -495,6 +498,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
struct clk_notifier_data *data = ptr;
struct tegra_devfreq_device *dev;
struct tegra_devfreq *tegra;
+ unsigned long freq;
unsigned int i;
if (action != POST_RATE_CHANGE)
@@ -502,6 +506,8 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
tegra = container_of(nb, struct tegra_devfreq, clk_rate_change_nb);
+ freq = data->new_rate / KHZ;
+
/*
* EMC rate could change due to three reasons:
*
@@ -523,7 +529,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
dev = &tegra->devices[i];
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev, data->new_rate);
+ tegra_devfreq_update_wmark(tegra, dev, freq);
}
return NOTIFY_OK;
@@ -563,17 +569,17 @@ static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 val = 0, target_freq;
+ u32 val = 0;
/* we don't want boosting on restart */
dev->boost_freq = 0;
- target_freq = clk_get_rate(tegra->emc_clock) / KHZ;
- dev->avg_count = target_freq * ACTMON_SAMPLING_PERIOD;
- device_writel(dev, dev->avg_count, ACTMON_DEV_INIT_AVG);
+ dev->avg_freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ device_writel(dev, dev->avg_freq * ACTMON_SAMPLING_PERIOD,
+ ACTMON_DEV_INIT_AVG);
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev, target_freq);
+ tegra_devfreq_update_wmark(tegra, dev, dev->avg_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
@@ -685,7 +691,7 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
rate = dev_pm_opp_get_freq(opp);
dev_pm_opp_put(opp);
- err = clk_set_min_rate(tegra->emc_clock, rate);
+ err = clk_set_min_rate(tegra->emc_clock, rate * KHZ);
if (err)
return err;
@@ -708,7 +714,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
struct tegra_devfreq_device *actmon_dev;
unsigned long cur_freq;
- cur_freq = clk_get_rate(tegra->emc_clock);
+ cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
/* To be used by the tegra governor */
stat->private_data = tegra;
@@ -725,7 +731,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
stat->busy_time *= 100 / BUS_SATURATION_RATIO;
/* Number of cycles in a sampling period */
- stat->total_time = cur_freq / KHZ * ACTMON_SAMPLING_PERIOD;
+ stat->total_time = cur_freq * ACTMON_SAMPLING_PERIOD;
stat->busy_time = min(stat->busy_time, stat->total_time);
@@ -771,7 +777,7 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
actmon_device_debug(tegra, dev, "upd");
}
- *freq = target_freq * KHZ;
+ *freq = target_freq;
return 0;
}
@@ -907,7 +913,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
goto remove_opps;
}
- err = dev_pm_opp_add(&pdev->dev, rate, 0);
+ err = dev_pm_opp_add(&pdev->dev, rate / KHZ, 0);
if (err) {
dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
goto remove_opps;
@@ -927,6 +933,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
}
tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
+ tegra_devfreq_profile.initial_freq /= KHZ;
+
devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
"tegra_actmon", NULL);
if (IS_ERR(tegra->devfreq)) {
--
2.22.0
^ permalink raw reply related
* [PATCH v2 14/17] PM / devfreq: tegra30: Ensure that target freq won't overflow
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
Potentially very high boosting could cause an integer overflow for a
highly clocked memory after conversion to MHz.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index d85de0e3dce0..1dcc2d2fa9e0 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -458,6 +458,7 @@ static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
{ unsigned long target_freq;
target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
+ target_freq = min(target_freq, ULONG_MAX / KHZ);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
return target_freq;
--
2.22.0
^ permalink raw reply related
* [PATCH v2 13/17] PM / devfreq: tegra30: Constify structs
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
Constify unmodifiable structs for consistency.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 8630dca99905..d85de0e3dce0 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -106,7 +106,7 @@ enum tegra_actmon_device {
MCCPU,
};
-static struct tegra_devfreq_device_config actmon_device_configs[] = {
+static const struct tegra_devfreq_device_config actmon_device_configs[] = {
{
/* MCALL: All memory accesses (including from the CPUs) */
.offset = 0x1c0,
@@ -171,7 +171,7 @@ struct tegra_actmon_emc_ratio {
unsigned long emc_freq;
};
-static struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
+static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
{ 1400000, ULONG_MAX },
{ 1200000, 750000 },
{ 1100000, 600000 },
@@ -210,7 +210,7 @@ static inline unsigned long do_percent(unsigned long val, unsigned int pct)
static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra)
{
- struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
+ const struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
unsigned int cpu_freq = cpufreq_get(0);
unsigned int i;
--
2.22.0
^ permalink raw reply related
* [PATCH v2 05/17] PM / devfreq: tegra30: Set up watermarks properly
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
The current implementation is inaccurate and results in very intensive
interrupt activity, which neglects the whole idea of polling offload to
hardware. The reason of the shortcoming is that watermarks are not set
up correctly and this results in ACTMON constantly asking to change freq
and then these requests are ignored. The end result of this patch is that
there are few hundreds of ACTMON's interrupts instead of tens thousands
after few minutes of a working devfreq, meanwhile the transitions activity
stays about the same and governor becomes more reactive.
Since watermarks are set precisely correct now, the boosting logic is
changed a tad to accommodate the change. The "average sustain coefficient"
multiplier is gone now since there is no need to compensate the improper
watermarks and EMC frequency-bump happens once boosting hits the upper
watermark enough times, depending on the per-device boosting threshold.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 295 +++++++++++++++++++++---------
1 file changed, 209 insertions(+), 86 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 4be7858c33bc..30c5b3531d6f 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -47,6 +47,8 @@
#define ACTMON_DEV_INTR_CONSECUTIVE_UPPER BIT(31)
#define ACTMON_DEV_INTR_CONSECUTIVE_LOWER BIT(30)
+#define ACTMON_DEV_INTR_AVG_BELOW_WMARK BIT(25)
+#define ACTMON_DEV_INTR_AVG_ABOVE_WMARK BIT(24)
#define ACTMON_ABOVE_WMARK_WINDOW 1
#define ACTMON_BELOW_WMARK_WINDOW 3
@@ -63,9 +65,8 @@
* ACTMON_AVERAGE_WINDOW_LOG2: default value for @DEV_CTRL_K_VAL, which
* translates to 2 ^ (K_VAL + 1). ex: 2 ^ (6 + 1) = 128
*/
-#define ACTMON_AVERAGE_WINDOW_LOG2 6
-#define ACTMON_SAMPLING_PERIOD 12 /* ms */
-#define ACTMON_DEFAULT_AVG_BAND 6 /* 1/10 of % */
+#define ACTMON_AVERAGE_WINDOW_LOG2 6
+#define ACTMON_SAMPLING_PERIOD 12 /* ms */
#define KHZ 1000
@@ -142,9 +143,6 @@ struct tegra_devfreq_device {
* watermark breaches.
*/
unsigned long boost_freq;
-
- /* Optimal frequency calculated from the stats for this device */
- unsigned long target_freq;
};
struct tegra_devfreq {
@@ -156,7 +154,6 @@ struct tegra_devfreq {
struct clk *emc_clock;
unsigned long max_freq;
- unsigned long cur_freq;
struct notifier_block rate_change_nb;
struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
@@ -205,42 +202,191 @@ static unsigned long do_percent(unsigned long val, unsigned int pct)
return val * pct / 100;
}
+static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra)
+{
+ struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
+ unsigned int cpu_freq = cpufreq_get(0);
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(actmon_emc_ratios); i++, ratio++) {
+ if (cpu_freq >= ratio->cpu_freq) {
+ if (ratio->emc_freq >= tegra->max_freq)
+ return tegra->max_freq;
+ else
+ return ratio->emc_freq;
+ }
+ }
+
+ return 0;
+}
+
+static unsigned long
+tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ unsigned long target_freq)
+{
+ unsigned long static_cpu_emc_freq;
+
+ if (dev->config->avg_dependency_threshold &&
+ dev->config->avg_dependency_threshold < dev->avg_count) {
+ static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra);
+ target_freq = max(target_freq, static_cpu_emc_freq);
+ }
+
+ return target_freq;
+}
+
+static unsigned long tegra_actmon_lower_freq(struct tegra_devfreq *tegra,
+ unsigned long target_freq)
+{
+ unsigned long lower = target_freq;
+ struct dev_pm_opp *opp;
+
+ opp = dev_pm_opp_find_freq_floor(tegra->devfreq->dev.parent, &lower);
+ if (IS_ERR(opp))
+ lower = 0;
+ else
+ dev_pm_opp_put(opp);
+
+ return lower;
+}
+
+static unsigned long tegra_actmon_upper_freq(struct tegra_devfreq *tegra,
+ unsigned long target_freq)
+{
+ unsigned long upper = target_freq + 1;
+ struct dev_pm_opp *opp;
+
+ opp = dev_pm_opp_find_freq_ceil(tegra->devfreq->dev.parent, &upper);
+ if (IS_ERR(opp))
+ upper = ULONG_MAX;
+ else
+ dev_pm_opp_put(opp);
+
+ return upper;
+}
+
+static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ unsigned long target_freq,
+ unsigned long *lower,
+ unsigned long *upper)
+{
+ /*
+ * Memory frequencies are guaranteed to have 1MHz granularity
+ * and thus we need this rounding down to get a proper watermarks
+ * range in a case where target_freq falls into a range of
+ * next_possible_opp_freq - 1MHz.
+ */
+ target_freq = round_down(target_freq, 1000000);
+
+ /* watermarks are set at the borders of the corresponding OPPs */
+ *lower = tegra_actmon_lower_freq(tegra, target_freq);
+ *upper = tegra_actmon_upper_freq(tegra, target_freq);
+
+ *lower /= KHZ;
+ *upper /= KHZ;
+
+ /*
+ * The upper watermark should take into account CPU's frequency
+ * because cpu_to_emc_rate() may override the target_freq with
+ * a higher value and thus upper watermark need to be set up
+ * accordingly to avoid parasitic upper-events.
+ */
+ *upper = tegra_actmon_account_cpu_freq(tegra, dev, *upper);
+
+ *lower *= ACTMON_SAMPLING_PERIOD;
+ *upper *= ACTMON_SAMPLING_PERIOD;
+}
+
static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 avg = dev->avg_count;
- u32 avg_band_freq = tegra->max_freq * ACTMON_DEFAULT_AVG_BAND / KHZ;
- u32 band = avg_band_freq * ACTMON_SAMPLING_PERIOD;
+ unsigned long lower, upper, freq;
+
+ freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
+ tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
- device_writel(dev, avg + band, ACTMON_DEV_AVG_UPPER_WMARK);
+ /*
+ * We want to get interrupts when MCCPU client crosses the
+ * dependency threshold in order to take into / out of account
+ * the CPU's freq.
+ */
+ if (lower < dev->config->avg_dependency_threshold &&
+ upper > dev->config->avg_dependency_threshold) {
+ if (dev->avg_count < dev->config->avg_dependency_threshold)
+ upper = dev->config->avg_dependency_threshold;
+ else
+ lower = dev->config->avg_dependency_threshold;
+ }
- avg = max(dev->avg_count, band);
- device_writel(dev, avg - band, ACTMON_DEV_AVG_LOWER_WMARK);
+ device_writel(dev, lower, ACTMON_DEV_AVG_LOWER_WMARK);
+ device_writel(dev, upper, ACTMON_DEV_AVG_UPPER_WMARK);
}
static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev)
+ struct tegra_devfreq_device *dev,
+ unsigned long freq)
{
- u32 val = tegra->cur_freq * ACTMON_SAMPLING_PERIOD;
+ unsigned long lower, upper, delta;
+
+ /*
+ * Boosting logic kicks-in once lower / upper watermark is hit.
+ * The watermarks are based on the updated EMC rate and the
+ * average activity.
+ *
+ * The higher watermark is set in accordance to the EMC rate
+ * because we want to set it to the highest mark here and EMC rate
+ * represents that mark. The consecutive-upper interrupts are
+ * always enabled and we don't want to receive them if they won't
+ * do anything useful, hence the upper watermark is capped to maximum.
+ * Note that the EMC rate is changed once boosting pushed the rate
+ * too high, in that case boosting-up will be stopped because
+ * upper watermark is much higher now and it is *important* to
+ * stop the unwanted interrupts.
+ */
+ tegra_actmon_get_lower_upper(tegra, dev, freq - 1, &lower, &upper);
- device_writel(dev, do_percent(val, dev->config->boost_up_threshold),
- ACTMON_DEV_UPPER_WMARK);
+ delta = do_percent(upper - lower, dev->config->boost_up_threshold);
+ device_writel(dev, lower + delta, ACTMON_DEV_UPPER_WMARK);
- device_writel(dev, do_percent(val, dev->config->boost_down_threshold),
- ACTMON_DEV_LOWER_WMARK);
+ /*
+ * Meanwhile the lower mark is based on the average value
+ * because it is the lowest possible consecutive-mark for this
+ * device. Once that mark is hit and boosting is stopped, the
+ * interrupt is disabled by ISR.
+ */
+ freq = dev->avg_count / ACTMON_SAMPLING_PERIOD * KHZ;
+ tegra_actmon_get_lower_upper(tegra, dev, freq, &lower, &upper);
+
+ delta = do_percent(upper - lower, dev->config->boost_down_threshold);
+ device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 intr_status, dev_ctrl;
+ u32 intr_status, dev_ctrl, avg_intr_mask;
dev->avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
- tegra_devfreq_update_avg_wmark(tegra, dev);
-
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
+ avg_intr_mask = ACTMON_DEV_INTR_AVG_BELOW_WMARK |
+ ACTMON_DEV_INTR_AVG_ABOVE_WMARK;
+
+ if (intr_status & avg_intr_mask) {
+ tegra_devfreq_update_avg_wmark(tegra, dev);
+
+ /*
+ * Once average watermark is hit, it means that the memory
+ * activity changed significantly and thus boosting-up shall
+ * be reset because EMC clock rate will be changed and
+ * boosting will restart in this case.
+ */
+ dev->boost_freq = 0;
+ }
+
if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_UPPER) {
/*
* new_boost = min(old_boost * up_coef + step, max_freq)
@@ -253,8 +399,6 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
if (dev->boost_freq >= tegra->max_freq)
dev->boost_freq = tegra->max_freq;
- else
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
} else if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_LOWER) {
/*
* new_boost = old_boost * down_coef
@@ -263,63 +407,26 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
dev->boost_freq = do_percent(dev->boost_freq,
dev->config->boost_down_coeff);
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
-
if (dev->boost_freq < (ACTMON_BOOST_FREQ_STEP >> 1))
dev->boost_freq = 0;
- else
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
}
- if (dev->config->avg_dependency_threshold) {
- if (dev->avg_count >= dev->config->avg_dependency_threshold)
- dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
- else if (dev->boost_freq == 0)
- dev_ctrl &= ~ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
- }
+ /* no boosting => no need for consecutive-down interrupt */
+ if (dev->boost_freq == 0)
+ dev_ctrl &= ~ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
-
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
}
-static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
- unsigned long cpu_freq)
-{
- unsigned int i;
- struct tegra_actmon_emc_ratio *ratio = actmon_emc_ratios;
-
- for (i = 0; i < ARRAY_SIZE(actmon_emc_ratios); i++, ratio++) {
- if (cpu_freq >= ratio->cpu_freq) {
- if (ratio->emc_freq >= tegra->max_freq)
- return tegra->max_freq;
- else
- return ratio->emc_freq;
- }
- }
-
- return 0;
-}
-
-static void actmon_update_target(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev)
-{
- unsigned long cpu_freq = 0;
- unsigned long static_cpu_emc_freq = 0;
- unsigned int avg_sustain_coef;
-
- if (dev->config->avg_dependency_threshold) {
- cpu_freq = cpufreq_get(0);
- static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
- }
+static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev)
+{ unsigned long target_freq;
- dev->target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD;
- avg_sustain_coef = 100 * 100 / dev->config->boost_up_threshold;
- dev->target_freq = do_percent(dev->target_freq, avg_sustain_coef);
- dev->target_freq += dev->boost_freq;
+ target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD + dev->boost_freq;
+ target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
- if (dev->avg_count >= dev->config->avg_dependency_threshold)
- dev->target_freq = max(dev->target_freq, static_cpu_emc_freq);
+ return target_freq;
}
static irqreturn_t actmon_thread_isr(int irq, void *data)
@@ -351,8 +458,8 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
unsigned long action, void *ptr)
{
struct clk_notifier_data *data = ptr;
- struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
+ struct tegra_devfreq *tegra;
unsigned int i;
if (action != POST_RATE_CHANGE)
@@ -360,12 +467,28 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
tegra = container_of(nb, struct tegra_devfreq, rate_change_nb);
- tegra->cur_freq = data->new_rate / KHZ;
-
+ /*
+ * EMC rate could change due to three reasons:
+ *
+ * 1. Average watermark hit
+ * 2. Boosting overflow
+ * 3. CPU freq change
+ *
+ * Once rate is changed, the consecutive watermarks need to be
+ * updated in order for boosting to work properly and to avoid
+ * unnecessary interrupts. Note that the consecutive range is set for
+ * all of devices using the same rate, hence if CPU is doing much
+ * less than the other memory clients, then its upper watermark will
+ * be very high in comparison to the actual activity (lower watermark)
+ * and thus unnecessary upper-interrupts will be suppressed.
+ *
+ * The average watermarks also should be updated because of 3.
+ */
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- tegra_devfreq_update_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_wmark(tegra, dev, data->new_rate);
}
return NOTIFY_OK;
@@ -374,15 +497,14 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
- u32 val = 0;
-
- dev->target_freq = tegra->cur_freq;
+ u32 val = 0, target_freq;
- dev->avg_count = tegra->cur_freq * ACTMON_SAMPLING_PERIOD;
+ target_freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ dev->avg_count = target_freq * ACTMON_SAMPLING_PERIOD;
device_writel(dev, dev->avg_count, ACTMON_DEV_INIT_AVG);
tegra_devfreq_update_avg_wmark(tegra, dev);
- tegra_devfreq_update_wmark(tegra, dev);
+ tegra_devfreq_update_wmark(tegra, dev, target_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
@@ -469,13 +591,13 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
struct tegra_devfreq_device *actmon_dev;
unsigned long cur_freq;
- cur_freq = READ_ONCE(tegra->cur_freq);
+ cur_freq = clk_get_rate(tegra->emc_clock);
/* To be used by the tegra governor */
stat->private_data = tegra;
/* The below are to be used by the other governors */
- stat->current_frequency = cur_freq * KHZ;
+ stat->current_frequency = cur_freq;
actmon_dev = &tegra->devices[MCALL];
@@ -486,7 +608,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev,
stat->busy_time *= 100 / BUS_SATURATION_RATIO;
/* Number of cycles in a sampling period */
- stat->total_time = ACTMON_SAMPLING_PERIOD * cur_freq;
+ stat->total_time = cur_freq / KHZ * ACTMON_SAMPLING_PERIOD;
stat->busy_time = min(stat->busy_time, stat->total_time);
@@ -505,6 +627,7 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
+ unsigned long dev_target_freq;
unsigned long target_freq = 0;
unsigned int i;
int err;
@@ -520,9 +643,9 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- actmon_update_target(tegra, dev);
+ dev_target_freq = actmon_update_target(tegra, dev);
- target_freq = max(target_freq, dev->target_freq);
+ target_freq = max(target_freq, dev_target_freq);
}
*freq = target_freq * KHZ;
@@ -642,7 +765,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
return rate;
}
- tegra->cur_freq = clk_get_rate(tegra->emc_clock) / KHZ;
tegra->max_freq = rate / KHZ;
for (i = 0; i < ARRAY_SIZE(actmon_device_configs); i++) {
@@ -671,7 +793,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, tegra);
tegra->rate_change_nb.notifier_call = tegra_actmon_rate_notify_cb;
- err = clk_notifier_register(tegra->emc_clock, &tegra->rate_change_nb);
+ err = clk_notifier_register(tegra->emc_clock,
+ &tegra->rate_change_nb);
if (err) {
dev_err(&pdev->dev,
"Failed to register rate change notifier\n");
--
2.22.0
^ permalink raw reply related
* [PATCH v2 06/17] PM / devfreq: tegra30: Tuneup boosting thresholds
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
Now that average-sustain coefficient / multiplier is gone, it won't hurt
to re-tune the boosting thresholds to get a bit harder boosting for MCALL
clients, resulting in a more reactive governing in a case of multimedia
applications usage like 3d / video.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 30c5b3531d6f..9b172e2acd40 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -111,8 +111,8 @@ static struct tegra_devfreq_device_config actmon_device_configs[] = {
.irq_mask = 1 << 26,
.boost_up_coeff = 200,
.boost_down_coeff = 50,
- .boost_up_threshold = 60,
- .boost_down_threshold = 40,
+ .boost_up_threshold = 50,
+ .boost_down_threshold = 25,
},
{
/* MCCPU: memory accesses from the CPUs */
--
2.22.0
^ permalink raw reply related
* [PATCH v2 04/17] PM / devfreq: tegra30: Drop write-barrier
From: Dmitry Osipenko @ 2019-06-26 1:32 UTC (permalink / raw)
To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Jonathan Hunter, Tomeu Vizoso
Cc: linux-pm, linux-tegra, linux-kernel
In-Reply-To: <20190626013252.30470-1-digetx@gmail.com>
There is no need in a write-barrier now, given that interrupt masking is
handled by CPU's GIC now. Hence we know exactly that interrupt won't fire
after stopping the devfreq's governor. In other cases we don't care about
potential buffering of the writes to hardware and thus there is no need to
stall CPU.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 5e606ae3f238..4be7858c33bc 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -230,12 +230,6 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
ACTMON_DEV_LOWER_WMARK);
}
-static void actmon_write_barrier(struct tegra_devfreq *tegra)
-{
- /* ensure the update has reached the ACTMON */
- readl(tegra->regs + ACTMON_GLB_STATUS);
-}
-
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
@@ -287,8 +281,6 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
-
- actmon_write_barrier(tegra);
}
static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
@@ -376,8 +368,6 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
tegra_devfreq_update_wmark(tegra, dev);
}
- actmon_write_barrier(tegra);
-
return NOTIFY_OK;
}
@@ -423,8 +413,6 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_configure_device(tegra, &tegra->devices[i]);
- actmon_write_barrier(tegra);
-
enable_irq(tegra->irq);
}
@@ -439,8 +427,6 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
device_writel(&tegra->devices[i], ACTMON_INTR_STATUS_CLEAR,
ACTMON_DEV_INTR_STATUS);
}
-
- actmon_write_barrier(tegra);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
--
2.22.0
^ permalink raw reply related
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-26 1:33 UTC (permalink / raw)
To: Greg KH
Cc: Rafael J. Wysocki, Viresh Kumar, Rafael J. Wysocki,
Hridya Valsaraju, Sandeep Patil, LKML, Linux PM,
Cc: Android Kernel
In-Reply-To: <20190626011221.GB22454@kroah.com>
On Tue, Jun 25, 2019 at 6:12 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Jun 25, 2019 at 05:54:49PM -0700, Tri Vo wrote:
> > Userspace can use wakeup_sources debugfs node to plot history of suspend
> > blocking wakeup sources over device's boot cycle. This information can
> > then be used (1) for power-specific bug reporting and (2) towards
> > attributing battery consumption to specific processes over a period of
> > time.
> >
> > However, debugfs doesn't have stable ABI. For this reason, expose wakeup
> > sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
> >
> > Add following attributes to each wakeup source. These attributes match
> > the columns of /sys/kernel/debug/wakeup_sources.
> >
> > active_count
> > event_count
> > wakeup_count
> > expire_count
> > active_time (named "active_since" in debugfs)
> > total_time
> > max_time
> > last_change
> > prevent_suspend_time
>
> Can you also add a Documentation/ABI/ update for your new sysfs files so
> that we can properly review this?
>
> > Embedding a struct kobject into struct wakeup_source changes lifetime
> > requirements on the latter. To that end, change deallocation of struct
> > wakeup_source using kfree to kobject_put().
Will do.
>
> Ick, are you sure you need a new kobject here? Why wouldn't a named
> attribute group work instead? That should keep this patch much smaller
> and simpler.
Yeah, named attribute groups might be a much cleaner way to do this.
Let me investigate.
>
> > +static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
> > + struct wakeup_source_attribute *attr,
> > + char *buf)
> > +{
> > + unsigned long flags;
> > + unsigned long var;
> > +
> > + spin_lock_irqsave(&ws->lock, flags);
> > + if (strcmp(attr->attr.name, "active_count") == 0)
> > + var = ws->active_count;
> > + else if (strcmp(attr->attr.name, "event_count") == 0)
> > + var = ws->event_count;
> > + else if (strcmp(attr->attr.name, "wakeup_count") == 0)
> > + var = ws->wakeup_count;
> > + else
> > + var = ws->expire_count;
> > + spin_unlock_irqrestore(&ws->lock, flags);
> > +
> > + return sprintf(buf, "%lu\n", var);
> > +}
>
> Why is this lock always needed to be grabbed? You are just reading a
> value, who cares if it changes inbetween reading it and returning the
> buffer string as it can change at that point in time anyway?
Right, we don't care if the value changes in between us reading and
printing it. However, IIUC not grabbing this lock results in a data
race, which is undefined behavior.
^ permalink raw reply
* RE: [PATCH v2 4/4] PM / devfreq: Add required OPPs support to passive governor
From: MyungJoo Ham @ 2019-06-26 1:31 UTC (permalink / raw)
To: Kyungmin Park, Chanwoo Choi, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team@android.com,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190625213337.157525-5-saravanak@google.com>
>Look at the required OPPs of the "parent" device to determine the OPP that
>is required from the slave device managed by the passive governor. This
>allows having mappings between a parent device and a slave device even when
>they don't have the same number of OPPs.
>
>Signed-off-by: Saravana Kannan <saravanak@google.com>
>---
> drivers/devfreq/governor_passive.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Cheers,
MyungJoo
^ permalink raw reply
* RE: [PATCH v2 3/4] PM / devfreq: Cache OPP table reference in devfreq
From: MyungJoo Ham @ 2019-06-26 1:30 UTC (permalink / raw)
To: Kyungmin Park, Chanwoo Choi, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki
Cc: Saravana Kannan, kernel-team@android.com,
linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190625213337.157525-4-saravanak@google.com>
>The OPP table can be used often in devfreq. Trying to get it each time can
>be expensive, so cache it in the devfreq struct.
>
>Signed-off-by: Saravana Kannan <saravanak@google.com>
>---
> drivers/devfreq/devfreq.c | 6 ++++++
> include/linux/devfreq.h | 1 +
> 2 files changed, 7 insertions(+)
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Cheers,
MyungJoo
^ permalink raw reply
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Greg KH @ 2019-06-26 1:12 UTC (permalink / raw)
To: Tri Vo
Cc: rjw, viresh.kumar, rafael, hridya, sspatil, linux-kernel,
linux-pm, kernel-team
In-Reply-To: <20190626005449.225796-1-trong@android.com>
On Tue, Jun 25, 2019 at 05:54:49PM -0700, Tri Vo wrote:
> Userspace can use wakeup_sources debugfs node to plot history of suspend
> blocking wakeup sources over device's boot cycle. This information can
> then be used (1) for power-specific bug reporting and (2) towards
> attributing battery consumption to specific processes over a period of
> time.
>
> However, debugfs doesn't have stable ABI. For this reason, expose wakeup
> sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
>
> Add following attributes to each wakeup source. These attributes match
> the columns of /sys/kernel/debug/wakeup_sources.
>
> active_count
> event_count
> wakeup_count
> expire_count
> active_time (named "active_since" in debugfs)
> total_time
> max_time
> last_change
> prevent_suspend_time
Can you also add a Documentation/ABI/ update for your new sysfs files so
that we can properly review this?
> Embedding a struct kobject into struct wakeup_source changes lifetime
> requirements on the latter. To that end, change deallocation of struct
> wakeup_source using kfree to kobject_put().
Ick, are you sure you need a new kobject here? Why wouldn't a named
attribute group work instead? That should keep this patch much smaller
and simpler.
> +static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + unsigned long var;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + if (strcmp(attr->attr.name, "active_count") == 0)
> + var = ws->active_count;
> + else if (strcmp(attr->attr.name, "event_count") == 0)
> + var = ws->event_count;
> + else if (strcmp(attr->attr.name, "wakeup_count") == 0)
> + var = ws->wakeup_count;
> + else
> + var = ws->expire_count;
> + spin_unlock_irqrestore(&ws->lock, flags);
> +
> + return sprintf(buf, "%lu\n", var);
> +}
Why is this lock always needed to be grabbed? You are just reading a
value, who cares if it changes inbetween reading it and returning the
buffer string as it can change at that point in time anyway?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-26 0:59 UTC (permalink / raw)
To: Rafael J. Wysocki, Greg Kroah-Hartman, Viresh Kumar
Cc: Rafael J. Wysocki, Hridya Valsaraju, Sandeep Patil, LKML,
Linux PM, kernel-team
In-Reply-To: <20190626005449.225796-1-trong@android.com>
On Tue, Jun 25, 2019 at 5:55 PM Tri Vo <trong@android.com> wrote:
>
> Userspace can use wakeup_sources debugfs node to plot history of suspend
> blocking wakeup sources over device's boot cycle. This information can
> then be used (1) for power-specific bug reporting and (2) towards
> attributing battery consumption to specific processes over a period of
> time.
>
> However, debugfs doesn't have stable ABI. For this reason, expose wakeup
> sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
>
> Add following attributes to each wakeup source. These attributes match
> the columns of /sys/kernel/debug/wakeup_sources.
>
> active_count
> event_count
> wakeup_count
> expire_count
> active_time (named "active_since" in debugfs)
> total_time
> max_time
> last_change
> prevent_suspend_time
>
> Embedding a struct kobject into struct wakeup_source changes lifetime
> requirements on the latter. To that end, change deallocation of struct
> wakeup_source using kfree to kobject_put().
>
> Signed-off-by: Tri Vo <trong@android.com>
> ---
> drivers/base/power/Makefile | 2 +-
> drivers/base/power/wakeup.c | 13 +-
> drivers/base/power/wakeup_sysfs.c | 204 ++++++++++++++++++++++++++++++
> include/linux/pm_wakeup.h | 9 ++
> kernel/power/wakelock.c | 13 +-
> 5 files changed, 236 insertions(+), 5 deletions(-)
> create mode 100644 drivers/base/power/wakeup_sysfs.c
>
> diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
> index e1bb691cf8f1..e6ea9eb46275 100644
> --- a/drivers/base/power/Makefile
> +++ b/drivers/base/power/Makefile
> @@ -1,6 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_PM) += sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o
> -obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
> +obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o wakeup_sysfs.o
> obj-$(CONFIG_PM_TRACE_RTC) += trace.o
> obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o
> obj-$(CONFIG_HAVE_CLK) += clock_ops.o
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..6fcbb4d2045f 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -96,12 +96,22 @@ EXPORT_SYMBOL_GPL(wakeup_source_prepare);
> struct wakeup_source *wakeup_source_create(const char *name)
> {
> struct wakeup_source *ws;
> + int ret;
>
> ws = kmalloc(sizeof(*ws), GFP_KERNEL);
> if (!ws)
> return NULL;
>
> wakeup_source_prepare(ws, name ? kstrdup_const(name, GFP_KERNEL) : NULL);
> +
> + ws->kobj.kset = wakeup_source_kset;
> + ret = kobject_init_and_add(&ws->kobj, &wakeup_source_ktype, NULL, "%s",
> + ws->name);
> + if (ret) {
> + kobject_put(&ws->kobj);
> + return NULL;
> + }
> +
> return ws;
> }
> EXPORT_SYMBOL_GPL(wakeup_source_create);
> @@ -147,8 +157,7 @@ void wakeup_source_destroy(struct wakeup_source *ws)
>
> __pm_relax(ws);
> wakeup_source_record(ws);
> - kfree_const(ws->name);
> - kfree(ws);
> + kobject_put(&ws->kobj);
> }
> EXPORT_SYMBOL_GPL(wakeup_source_destroy);
>
> diff --git a/drivers/base/power/wakeup_sysfs.c b/drivers/base/power/wakeup_sysfs.c
> new file mode 100644
> index 000000000000..d872afc645d9
> --- /dev/null
> +++ b/drivers/base/power/wakeup_sysfs.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/slab.h>
> +
> +#include "power.h"
> +
> +struct wakeup_source_attribute {
> + struct attribute attr;
> + ssize_t (*show)(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr, char *buf);
> + ssize_t (*store)(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr, const char *buf,
> + size_t count);
> +};
> +
> +#define to_wakeup_source_obj(x) container_of(x, struct wakeup_source, kobj)
> +#define to_wakeup_source_attr(x) \
> + container_of(x, struct wakeup_source_attribute, attr)
> +
> +static ssize_t wakeup_source_attr_show(struct kobject *kobj,
> + struct attribute *attr,
> + char *buf)
> +{
> + struct wakeup_source_attribute *attribute;
> + struct wakeup_source *ws;
> +
> + ws = to_wakeup_source_obj(kobj);
> + attribute = to_wakeup_source_attr(attr);
> +
> + if (!attribute->show)
> + return -EIO;
> +
> + return attribute->show(ws, attribute, buf);
> +}
> +
> +static const struct sysfs_ops wakeup_source_sysfs_ops = {
> + .show = wakeup_source_attr_show,
> +};
> +
> +static void wakeup_source_release(struct kobject *kobj)
> +{
> + struct wakeup_source *ws;
> +
> + ws = to_wakeup_source_obj(kobj);
> + kfree_const(ws->name);
> + kfree(ws);
> +}
> +
> +static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + unsigned long var;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + if (strcmp(attr->attr.name, "active_count") == 0)
> + var = ws->active_count;
> + else if (strcmp(attr->attr.name, "event_count") == 0)
> + var = ws->event_count;
> + else if (strcmp(attr->attr.name, "wakeup_count") == 0)
> + var = ws->wakeup_count;
> + else
> + var = ws->expire_count;
> + spin_unlock_irqrestore(&ws->lock, flags);
> +
> + return sprintf(buf, "%lu\n", var);
> +}
> +
> +#define wakeup_source_count_attr(_name) \
> +static struct wakeup_source_attribute _name##_attr = { \
> + .attr = { \
> + .name = __stringify(_name), \
> + .mode = 0444, \
> + }, \
> + .show = wakeup_source_count_show, \
> +}
> +
> +wakeup_source_count_attr(active_count);
> +wakeup_source_count_attr(event_count);
> +wakeup_source_count_attr(wakeup_count);
> +wakeup_source_count_attr(expire_count);
> +
> +#define wakeup_source_attr(_name) \
> +static struct wakeup_source_attribute _name##_attr = __ATTR_RO(_name)
> +
> +static ssize_t active_time_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + ktime_t active_time;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + active_time = ws->active ? ktime_sub(ktime_get(), ws->last_time) : 0;
> + spin_unlock_irqrestore(&ws->lock, flags);
> + return sprintf(buf, "%lld\n", ktime_to_ms(active_time));
> +}
> +wakeup_source_attr(active_time);
> +
> +static ssize_t total_time_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + ktime_t active_time;
> + ktime_t total_time;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + total_time = ws->total_time;
> + if (ws->active) {
> + active_time = ktime_sub(ktime_get(), ws->last_time);
> + total_time = ktime_add(total_time, active_time);
> + }
> + spin_unlock_irqrestore(&ws->lock, flags);
> + return sprintf(buf, "%lld\n", ktime_to_ms(total_time));
> +}
> +wakeup_source_attr(total_time);
> +
> +static ssize_t max_time_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + ktime_t active_time;
> + ktime_t max_time;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + max_time = ws->max_time;
> + if (ws->active) {
> + active_time = ktime_sub(ktime_get(), ws->last_time);
> + if (active_time > max_time)
> + max_time = active_time;
> + }
> + spin_unlock_irqrestore(&ws->lock, flags);
> + return sprintf(buf, "%lld\n", ktime_to_ms(max_time));
> +}
> +wakeup_source_attr(max_time);
> +
> +static ssize_t last_change_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + ktime_t last_time;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + last_time = ws->last_time;
> + spin_unlock_irqrestore(&ws->lock, flags);
> + return sprintf(buf, "%lld\n", ktime_to_ms(last_time));
> +}
> +wakeup_source_attr(last_change);
> +
> +static ssize_t prevent_suspend_time_show(struct wakeup_source *ws,
> + struct wakeup_source_attribute *attr,
> + char *buf)
> +{
> + unsigned long flags;
> + ktime_t prevent_sleep_time;
> +
> + spin_lock_irqsave(&ws->lock, flags);
> + prevent_sleep_time = ws->prevent_sleep_time;
> + if (ws->active && ws->autosleep_enabled) {
> + prevent_sleep_time = ktime_add(prevent_sleep_time,
> + ktime_sub(ktime_get(), ws->start_prevent_time));
> + }
> + spin_unlock_irqrestore(&ws->lock, flags);
> + return sprintf(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
> +}
> +wakeup_source_attr(prevent_suspend_time);
> +
> +static struct attribute *wakeup_source_default_attrs[] = {
> + &active_count_attr.attr,
> + &event_count_attr.attr,
> + &wakeup_count_attr.attr,
> + &expire_count_attr.attr,
> + &active_time_attr.attr,
> + &total_time_attr.attr,
> + &max_time_attr.attr,
> + &last_change_attr.attr,
> + &prevent_suspend_time_attr.attr,
> + NULL,
> +};
> +ATTRIBUTE_GROUPS(wakeup_source_default);
> +
> +struct kobj_type wakeup_source_ktype = {
> + .sysfs_ops = &wakeup_source_sysfs_ops,
> + .release = wakeup_source_release,
> + .default_groups = wakeup_source_default_groups,
> +};
> +
> +struct kset *wakeup_source_kset;
> +
> +static int __init wakeup_sources_sysfs_init(void)
> +{
> + wakeup_source_kset = kset_create_and_add("wakeup_sources", NULL,
> + power_kobj);
> + if (!wakeup_source_kset)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +postcore_initcall(wakeup_sources_sysfs_init);
> diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
> index ce57771fab9b..9af3bdd50557 100644
> --- a/include/linux/pm_wakeup.h
> +++ b/include/linux/pm_wakeup.h
> @@ -57,6 +57,7 @@ struct wakeup_source {
> unsigned long wakeup_count;
> bool active:1;
> bool autosleep_enabled:1;
> + struct kobject kobj;
> };
>
> #ifdef CONFIG_PM_SLEEP
> @@ -100,6 +101,10 @@ extern void pm_relax(struct device *dev);
> extern void pm_wakeup_ws_event(struct wakeup_source *ws, unsigned int msec, bool hard);
> extern void pm_wakeup_dev_event(struct device *dev, unsigned int msec, bool hard);
>
> +/* drivers/base/power/wakeup_sysfs.c */
> +extern struct kobj_type wakeup_source_ktype;
> +extern struct kset *wakeup_source_kset;
> +
> #else /* !CONFIG_PM_SLEEP */
>
> static inline void device_set_wakeup_capable(struct device *dev, bool capable)
> @@ -179,6 +184,10 @@ static inline void pm_wakeup_ws_event(struct wakeup_source *ws,
> static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
> bool hard) {}
>
> +/* drivers/base/power/wakeup_sysfs.c */
> +static struct kobj_type wakeup_source_ktype;
> +static struct kset *wakeup_source_kset;
> +
> #endif /* !CONFIG_PM_SLEEP */
>
> static inline void wakeup_source_init(struct wakeup_source *ws,
> diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
> index 4210152e56f0..71535eebfbbf 100644
> --- a/kernel/power/wakelock.c
> +++ b/kernel/power/wakelock.c
> @@ -124,8 +124,7 @@ static void __wakelocks_gc(struct work_struct *work)
> wakeup_source_remove(&wl->ws);
> rb_erase(&wl->node, &wakelocks_tree);
> list_del(&wl->lru);
> - kfree(wl->name);
> - kfree(wl);
> + kobject_put(&wl->ws->kobj);
> decrement_wakelocks_number();
> }
> }
> @@ -153,6 +152,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
> struct rb_node **node = &wakelocks_tree.rb_node;
> struct rb_node *parent = *node;
> struct wakelock *wl;
> + int ret;
>
> while (*node) {
> int diff;
> @@ -189,6 +189,15 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
> }
> wl->ws.name = wl->name;
> wl->ws.last_time = ktime_get();
> +
> + wl->ws.kobj.kset = wakeup_source_kset;
> + ret = kobject_init_and_add(&wl->ws.kobj, &wakeup_source_ktype, NULL,
> + "%s", wl->ws.name);
> + if (ret) {
> + kobject_put(&wl->ws.kobj);
> + return ERR_PTR(ret);
> + }
> +
> wakeup_source_add(&wl->ws);
> rb_link_node(&wl->node, parent, node);
> rb_insert_color(&wl->node, &wakelocks_tree);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
^ permalink raw reply
* [PATCH] PM / wakeup: show wakeup sources stats in sysfs
From: Tri Vo @ 2019-06-26 0:54 UTC (permalink / raw)
To: rjw, gregkh, viresh.kumar
Cc: rafael, hridya, sspatil, linux-kernel, linux-pm, kernel-team,
Tri Vo
Userspace can use wakeup_sources debugfs node to plot history of suspend
blocking wakeup sources over device's boot cycle. This information can
then be used (1) for power-specific bug reporting and (2) towards
attributing battery consumption to specific processes over a period of
time.
However, debugfs doesn't have stable ABI. For this reason, expose wakeup
sources statistics in sysfs under /sys/power/wakeup_sources/<name>/
Add following attributes to each wakeup source. These attributes match
the columns of /sys/kernel/debug/wakeup_sources.
active_count
event_count
wakeup_count
expire_count
active_time (named "active_since" in debugfs)
total_time
max_time
last_change
prevent_suspend_time
Embedding a struct kobject into struct wakeup_source changes lifetime
requirements on the latter. To that end, change deallocation of struct
wakeup_source using kfree to kobject_put().
Signed-off-by: Tri Vo <trong@android.com>
---
drivers/base/power/Makefile | 2 +-
drivers/base/power/wakeup.c | 13 +-
drivers/base/power/wakeup_sysfs.c | 204 ++++++++++++++++++++++++++++++
include/linux/pm_wakeup.h | 9 ++
kernel/power/wakelock.c | 13 +-
5 files changed, 236 insertions(+), 5 deletions(-)
create mode 100644 drivers/base/power/wakeup_sysfs.c
diff --git a/drivers/base/power/Makefile b/drivers/base/power/Makefile
index e1bb691cf8f1..e6ea9eb46275 100644
--- a/drivers/base/power/Makefile
+++ b/drivers/base/power/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_PM) += sysfs.o generic_ops.o common.o qos.o runtime.o wakeirq.o
-obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o
+obj-$(CONFIG_PM_SLEEP) += main.o wakeup.o wakeup_sysfs.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o domain_governor.o
obj-$(CONFIG_HAVE_CLK) += clock_ops.o
diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..6fcbb4d2045f 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -96,12 +96,22 @@ EXPORT_SYMBOL_GPL(wakeup_source_prepare);
struct wakeup_source *wakeup_source_create(const char *name)
{
struct wakeup_source *ws;
+ int ret;
ws = kmalloc(sizeof(*ws), GFP_KERNEL);
if (!ws)
return NULL;
wakeup_source_prepare(ws, name ? kstrdup_const(name, GFP_KERNEL) : NULL);
+
+ ws->kobj.kset = wakeup_source_kset;
+ ret = kobject_init_and_add(&ws->kobj, &wakeup_source_ktype, NULL, "%s",
+ ws->name);
+ if (ret) {
+ kobject_put(&ws->kobj);
+ return NULL;
+ }
+
return ws;
}
EXPORT_SYMBOL_GPL(wakeup_source_create);
@@ -147,8 +157,7 @@ void wakeup_source_destroy(struct wakeup_source *ws)
__pm_relax(ws);
wakeup_source_record(ws);
- kfree_const(ws->name);
- kfree(ws);
+ kobject_put(&ws->kobj);
}
EXPORT_SYMBOL_GPL(wakeup_source_destroy);
diff --git a/drivers/base/power/wakeup_sysfs.c b/drivers/base/power/wakeup_sysfs.c
new file mode 100644
index 000000000000..d872afc645d9
--- /dev/null
+++ b/drivers/base/power/wakeup_sysfs.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/slab.h>
+
+#include "power.h"
+
+struct wakeup_source_attribute {
+ struct attribute attr;
+ ssize_t (*show)(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr, char *buf);
+ ssize_t (*store)(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr, const char *buf,
+ size_t count);
+};
+
+#define to_wakeup_source_obj(x) container_of(x, struct wakeup_source, kobj)
+#define to_wakeup_source_attr(x) \
+ container_of(x, struct wakeup_source_attribute, attr)
+
+static ssize_t wakeup_source_attr_show(struct kobject *kobj,
+ struct attribute *attr,
+ char *buf)
+{
+ struct wakeup_source_attribute *attribute;
+ struct wakeup_source *ws;
+
+ ws = to_wakeup_source_obj(kobj);
+ attribute = to_wakeup_source_attr(attr);
+
+ if (!attribute->show)
+ return -EIO;
+
+ return attribute->show(ws, attribute, buf);
+}
+
+static const struct sysfs_ops wakeup_source_sysfs_ops = {
+ .show = wakeup_source_attr_show,
+};
+
+static void wakeup_source_release(struct kobject *kobj)
+{
+ struct wakeup_source *ws;
+
+ ws = to_wakeup_source_obj(kobj);
+ kfree_const(ws->name);
+ kfree(ws);
+}
+
+static ssize_t wakeup_source_count_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ unsigned long var;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ if (strcmp(attr->attr.name, "active_count") == 0)
+ var = ws->active_count;
+ else if (strcmp(attr->attr.name, "event_count") == 0)
+ var = ws->event_count;
+ else if (strcmp(attr->attr.name, "wakeup_count") == 0)
+ var = ws->wakeup_count;
+ else
+ var = ws->expire_count;
+ spin_unlock_irqrestore(&ws->lock, flags);
+
+ return sprintf(buf, "%lu\n", var);
+}
+
+#define wakeup_source_count_attr(_name) \
+static struct wakeup_source_attribute _name##_attr = { \
+ .attr = { \
+ .name = __stringify(_name), \
+ .mode = 0444, \
+ }, \
+ .show = wakeup_source_count_show, \
+}
+
+wakeup_source_count_attr(active_count);
+wakeup_source_count_attr(event_count);
+wakeup_source_count_attr(wakeup_count);
+wakeup_source_count_attr(expire_count);
+
+#define wakeup_source_attr(_name) \
+static struct wakeup_source_attribute _name##_attr = __ATTR_RO(_name)
+
+static ssize_t active_time_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ ktime_t active_time;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ active_time = ws->active ? ktime_sub(ktime_get(), ws->last_time) : 0;
+ spin_unlock_irqrestore(&ws->lock, flags);
+ return sprintf(buf, "%lld\n", ktime_to_ms(active_time));
+}
+wakeup_source_attr(active_time);
+
+static ssize_t total_time_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ ktime_t active_time;
+ ktime_t total_time;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ total_time = ws->total_time;
+ if (ws->active) {
+ active_time = ktime_sub(ktime_get(), ws->last_time);
+ total_time = ktime_add(total_time, active_time);
+ }
+ spin_unlock_irqrestore(&ws->lock, flags);
+ return sprintf(buf, "%lld\n", ktime_to_ms(total_time));
+}
+wakeup_source_attr(total_time);
+
+static ssize_t max_time_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ ktime_t active_time;
+ ktime_t max_time;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ max_time = ws->max_time;
+ if (ws->active) {
+ active_time = ktime_sub(ktime_get(), ws->last_time);
+ if (active_time > max_time)
+ max_time = active_time;
+ }
+ spin_unlock_irqrestore(&ws->lock, flags);
+ return sprintf(buf, "%lld\n", ktime_to_ms(max_time));
+}
+wakeup_source_attr(max_time);
+
+static ssize_t last_change_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ ktime_t last_time;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ last_time = ws->last_time;
+ spin_unlock_irqrestore(&ws->lock, flags);
+ return sprintf(buf, "%lld\n", ktime_to_ms(last_time));
+}
+wakeup_source_attr(last_change);
+
+static ssize_t prevent_suspend_time_show(struct wakeup_source *ws,
+ struct wakeup_source_attribute *attr,
+ char *buf)
+{
+ unsigned long flags;
+ ktime_t prevent_sleep_time;
+
+ spin_lock_irqsave(&ws->lock, flags);
+ prevent_sleep_time = ws->prevent_sleep_time;
+ if (ws->active && ws->autosleep_enabled) {
+ prevent_sleep_time = ktime_add(prevent_sleep_time,
+ ktime_sub(ktime_get(), ws->start_prevent_time));
+ }
+ spin_unlock_irqrestore(&ws->lock, flags);
+ return sprintf(buf, "%lld\n", ktime_to_ms(prevent_sleep_time));
+}
+wakeup_source_attr(prevent_suspend_time);
+
+static struct attribute *wakeup_source_default_attrs[] = {
+ &active_count_attr.attr,
+ &event_count_attr.attr,
+ &wakeup_count_attr.attr,
+ &expire_count_attr.attr,
+ &active_time_attr.attr,
+ &total_time_attr.attr,
+ &max_time_attr.attr,
+ &last_change_attr.attr,
+ &prevent_suspend_time_attr.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(wakeup_source_default);
+
+struct kobj_type wakeup_source_ktype = {
+ .sysfs_ops = &wakeup_source_sysfs_ops,
+ .release = wakeup_source_release,
+ .default_groups = wakeup_source_default_groups,
+};
+
+struct kset *wakeup_source_kset;
+
+static int __init wakeup_sources_sysfs_init(void)
+{
+ wakeup_source_kset = kset_create_and_add("wakeup_sources", NULL,
+ power_kobj);
+ if (!wakeup_source_kset)
+ return -ENOMEM;
+
+ return 0;
+}
+
+postcore_initcall(wakeup_sources_sysfs_init);
diff --git a/include/linux/pm_wakeup.h b/include/linux/pm_wakeup.h
index ce57771fab9b..9af3bdd50557 100644
--- a/include/linux/pm_wakeup.h
+++ b/include/linux/pm_wakeup.h
@@ -57,6 +57,7 @@ struct wakeup_source {
unsigned long wakeup_count;
bool active:1;
bool autosleep_enabled:1;
+ struct kobject kobj;
};
#ifdef CONFIG_PM_SLEEP
@@ -100,6 +101,10 @@ extern void pm_relax(struct device *dev);
extern void pm_wakeup_ws_event(struct wakeup_source *ws, unsigned int msec, bool hard);
extern void pm_wakeup_dev_event(struct device *dev, unsigned int msec, bool hard);
+/* drivers/base/power/wakeup_sysfs.c */
+extern struct kobj_type wakeup_source_ktype;
+extern struct kset *wakeup_source_kset;
+
#else /* !CONFIG_PM_SLEEP */
static inline void device_set_wakeup_capable(struct device *dev, bool capable)
@@ -179,6 +184,10 @@ static inline void pm_wakeup_ws_event(struct wakeup_source *ws,
static inline void pm_wakeup_dev_event(struct device *dev, unsigned int msec,
bool hard) {}
+/* drivers/base/power/wakeup_sysfs.c */
+static struct kobj_type wakeup_source_ktype;
+static struct kset *wakeup_source_kset;
+
#endif /* !CONFIG_PM_SLEEP */
static inline void wakeup_source_init(struct wakeup_source *ws,
diff --git a/kernel/power/wakelock.c b/kernel/power/wakelock.c
index 4210152e56f0..71535eebfbbf 100644
--- a/kernel/power/wakelock.c
+++ b/kernel/power/wakelock.c
@@ -124,8 +124,7 @@ static void __wakelocks_gc(struct work_struct *work)
wakeup_source_remove(&wl->ws);
rb_erase(&wl->node, &wakelocks_tree);
list_del(&wl->lru);
- kfree(wl->name);
- kfree(wl);
+ kobject_put(&wl->ws->kobj);
decrement_wakelocks_number();
}
}
@@ -153,6 +152,7 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
struct rb_node **node = &wakelocks_tree.rb_node;
struct rb_node *parent = *node;
struct wakelock *wl;
+ int ret;
while (*node) {
int diff;
@@ -189,6 +189,15 @@ static struct wakelock *wakelock_lookup_add(const char *name, size_t len,
}
wl->ws.name = wl->name;
wl->ws.last_time = ktime_get();
+
+ wl->ws.kobj.kset = wakeup_source_kset;
+ ret = kobject_init_and_add(&wl->ws.kobj, &wakeup_source_ktype, NULL,
+ "%s", wl->ws.name);
+ if (ret) {
+ kobject_put(&wl->ws.kobj);
+ return ERR_PTR(ret);
+ }
+
wakeup_source_add(&wl->ws);
rb_link_node(&wl->node, parent, node);
rb_insert_color(&wl->node, &wakelocks_tree);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* Re: [PATCH v4 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
From: Stephen Boyd @ 2019-06-25 23:05 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190612182500.4097-6-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-12 11:24:57)
> As 'clk-raspberrypi' depends on RPi's firmware interface, which might be
> configured as a module, the cpu clock might not be available for the
> cpufreq driver during it's init process. So we register the
> 'raspberrypi-cpufreq' platform device after the probe sequence succeeds.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v4 3/7] firmware: raspberrypi: register clk device
From: Stephen Boyd @ 2019-06-25 23:04 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190612182500.4097-4-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-12 11:24:55)
> Since clk-raspberrypi is tied to the VC4 firmware instead of particular
> hardware it's registration should be performed by the firmware driver.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v4 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
From: Stephen Boyd @ 2019-06-25 23:04 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190612182500.4097-3-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-12 11:24:54)
> Raspberry Pi's firmware offers an interface though which update it's
> clock's frequencies. This is specially useful in order to change the CPU
> clock (pllb_arm) which is 'owned' by the firmware and we're unable to
> scale using the register interface provided by clk-bcm2835.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v4 1/7] clk: bcm2835: remove pllb
From: Stephen Boyd @ 2019-06-25 23:04 UTC (permalink / raw)
To: Eric Anholt, Florian Fainelli, Nicolas Saenz Julienne, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne, linux-kernel
In-Reply-To: <20190612182500.4097-2-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-12 11:24:53)
> Raspberry Pi's firmware controls this pll, we should use the firmware
> interface to access it.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v4 0/7] cpufreq support for Raspberry Pi
From: Stephen Boyd @ 2019-06-25 23:02 UTC (permalink / raw)
To: Mike Turquette, Stefan Wahren
Cc: Florian Fainelli, Nicolas Saenz Julienne, linux-kernel, mbrugger,
viresh.kumar, rjw, eric, bcm-kernel-feedback-list, ptesarik,
linux-rpi-kernel, ssuloev, linux-clk, linux-arm-kernel, linux-pm
In-Reply-To: <d8b20179-45ef-479a-47dc-390a4a2dfddf@i2se.com>
Quoting Stefan Wahren (2019-06-22 02:54:47)
> Hi Stephen,
> hi Mike,
>
> Am 13.06.19 um 06:31 schrieb Stefan Wahren:
> > Hi Florian,
> > hi Stephen,
> >
> > Am 13.06.19 um 05:31 schrieb Florian Fainelli:
> >> On 6/12/2019 11:24 AM, Nicolas Saenz Julienne wrote:
> >>> Hi all,
> >>> this aims at adding cpufreq support to the Raspberry Pi family of
> >>> boards.
> >>>
> >>> The series first factors out 'pllb' from clk-bcm2385 and creates a new
> >>> clk driver that operates it over RPi's firmware interface[1]. We are
> >>> forced to do so as the firmware 'owns' the pll and we're not allowed to
> >>> change through the register interface directly as we might race with the
> >>> over-temperature and under-voltage protections provided by the firmware.
> >>>
> >>> Next it creates a minimal cpufreq driver that populates the CPU's opp
> >>> table, and registers cpufreq-dt. Which is needed as the firmware
> >>> controls the max and min frequencies available.
> >>>
> >>> This was tested on a RPi3b+ and RPI2b, both using multi_v7_defconfig and
> >>> arm64's defconfig.
> >> How do we go about merging this? Stefan, will you pick up patch 3, 6 and
> >> 7 and submit them for 5.3/5.4? Viresh has already picked up patch 4.
> > is it possible to let patches 1,2, 3 and 5 go via clk-tree?
> >
> > I would take care of 6 and 7.
> >
> > Stefan
> are you fine with the series, since Viresh already picked up patch 4?
>
> are you okay with my suggestion above?
>
Oh I missed this. I can apply the clk patches most likely assuming it
doesn't break anything bisection-wise.
^ permalink raw reply
* Re: [PATCH v3 5/7] clk: raspberrypi: register platform device for raspberrypi-cpufreq
From: Stephen Boyd @ 2019-06-25 23:01 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190611175839.28351-6-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-11 10:58:42)
> As 'clk-raspberrypi' depends on RPi's firmware interface, which might be
> configured as a module, the cpu clock might not be available for the
> cpufreq driver during it's init process. So we register the
> 'raspberrypi-cpufreq' platform device after the probe sequence succeeds.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 3/7] firmware: raspberrypi: register clk device
From: Stephen Boyd @ 2019-06-25 23:00 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190611175839.28351-4-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-11 10:58:38)
> Since clk-raspberrypi is tied to the VC4 firmware instead of particular
> hardware it's registration should be performed by the firmware driver.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 2/7] clk: bcm283x: add driver interfacing with Raspberry Pi's firmware
From: Stephen Boyd @ 2019-06-25 23:00 UTC (permalink / raw)
To: Nicolas Saenz Julienne, linux-kernel, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, eric, f.fainelli,
bcm-kernel-feedback-list, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne
In-Reply-To: <20190611175839.28351-3-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-11 10:58:36)
> Raspberry Pi's firmware offers an interface though which update it's
> clock's frequencies. This is specially useful in order to change the CPU
> clock (pllb_arm) which is 'owned' by the firmware and we're unable to
> scale using the register interface provided by clk-bcm2835.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 1/7] clk: bcm2835: remove pllb
From: Stephen Boyd @ 2019-06-25 23:00 UTC (permalink / raw)
To: Eric Anholt, Florian Fainelli, Nicolas Saenz Julienne, Ray Jui,
Scott Branden, bcm-kernel-feedback-list, stefan.wahren
Cc: mbrugger, viresh.kumar, rjw, ptesarik, linux-rpi-kernel, ssuloev,
linux-clk, linux-arm-kernel, mturquette, linux-pm,
Nicolas Saenz Julienne, linux-kernel
In-Reply-To: <20190611175839.28351-2-nsaenzjulienne@suse.de>
Quoting Nicolas Saenz Julienne (2019-06-11 10:58:34)
> Raspberry Pi's firmware controls this pll, we should use the firmware
> interface to access it.
>
> Signed-off-by: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
> Acked-by: Eric Anholt <eric@anholt.net>
> ---
Reviewed-by: Stephen Boyd <sboyd@kernel.org>
^ permalink raw reply
* Re: [PATCH v2 3/5] MIPS: jz4740: PM: Let CGU driver suspend clocks and set sleep mode
From: Stephen Boyd @ 2019-06-25 22:48 UTC (permalink / raw)
To: James Hogan, Michael Turquette, Paul Burton, Paul Cercueil,
Ralf Baechle
Cc: od, linux-mips, linux-pm, linux-clk, linux-kernel, Paul Cercueil
In-Reply-To: <20190611180757.32299-3-paul@crapouillou.net>
Quoting Paul Cercueil (2019-06-11 11:07:55)
> Instead of forcing the jz4740 clocks to suspend here, we let the CGU
> driver handle it.
> We also let the CGU driver set the "sleep mode" bit.
>
> This has the added benefit that now it is possible to build a kernel on
> SoCs newer than the JZ4740 with CONFIG_PM.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 4/5] clk: ingenic: Remove unused functions
From: Stephen Boyd @ 2019-06-25 22:48 UTC (permalink / raw)
To: James Hogan, Michael Turquette, Paul Burton, Paul Cercueil,
Ralf Baechle
Cc: od, linux-mips, linux-pm, linux-clk, linux-kernel, Paul Cercueil
In-Reply-To: <20190611180757.32299-4-paul@crapouillou.net>
Quoting Paul Cercueil (2019-06-11 11:07:56)
> These functions are not called anywhere anymore, they can safely be
> removed.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 5/5] MIPS: Remove dead code
From: Stephen Boyd @ 2019-06-25 22:48 UTC (permalink / raw)
To: James Hogan, Michael Turquette, Paul Burton, Paul Cercueil,
Ralf Baechle
Cc: od, linux-mips, linux-pm, linux-clk, linux-kernel, Paul Cercueil
In-Reply-To: <20190611180757.32299-5-paul@crapouillou.net>
Quoting Paul Cercueil (2019-06-11 11:07:57)
> Remove the unused <asm/mach-jz4740/clock.h> include.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
Applied to clk-next
^ permalink raw reply
* Re: [PATCH v2 2/5] clk: ingenic: Handle setting the Low-Power Mode bit
From: Stephen Boyd @ 2019-06-25 22:48 UTC (permalink / raw)
To: James Hogan, Michael Turquette, Paul Burton, Paul Cercueil,
Ralf Baechle
Cc: od, linux-mips, linux-pm, linux-clk, linux-kernel, Paul Cercueil
In-Reply-To: <20190611180757.32299-2-paul@crapouillou.net>
Quoting Paul Cercueil (2019-06-11 11:07:54)
> The Low-Power Mode, when enabled, will make the "wait" MIPS instruction
> suspend the system.
>
> This is not really clock-related, but this bit happens to be in the
> register set of the CGU.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> ---
Applied to clk-next
^ 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