* [PATCH v4 08/24] PM / devfreq: tegra30: Move clk-notifier's registration to governor's start
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
There is no point in receiving of the notifications while governor is
stopped, let's keep them disabled like we do for the CPU freq-change
notifications. This also fixes a potential use-after-free bug if
notification happens after device's removal.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 33 ++++++++++++++++++-------------
1 file changed, 19 insertions(+), 14 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 48a799fa5f63..d5d04c25023b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -576,6 +576,19 @@ static int tegra_actmon_start(struct tegra_devfreq *tegra)
actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
ACTMON_GLB_PERIOD_CTRL);
+ /*
+ * CLK notifications are needed in order to reconfigure the upper
+ * consecutive watermark in accordance to the actual clock rate
+ * to avoid unnecessary upper interrupts.
+ */
+ err = clk_notifier_register(tegra->emc_clock,
+ &tegra->clk_rate_change_nb);
+ if (err) {
+ dev_err(tegra->devfreq->dev.parent,
+ "Failed to register rate change notifier\n");
+ return err;
+ }
+
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_configure_device(tegra, &tegra->devices[i]);
@@ -602,6 +615,8 @@ static int tegra_actmon_start(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_stop_device(&tegra->devices[i]);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
+
return err;
}
@@ -618,6 +633,8 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
tegra_actmon_stop_device(&tegra->devices[i]);
+
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -862,22 +879,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, tegra);
+ tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
tegra->cpu_rate_change_nb.notifier_call = tegra_actmon_cpu_notify_cb;
INIT_WORK(&tegra->update_work, tegra_actmon_delayed_update);
- tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
- err = clk_notifier_register(tegra->emc_clock,
- &tegra->clk_rate_change_nb);
- if (err) {
- dev_err(&pdev->dev,
- "Failed to register rate change notifier\n");
- goto remove_opps;
- }
-
err = devfreq_add_governor(&tegra_devfreq_governor);
if (err) {
dev_err(&pdev->dev, "Failed to add governor: %d\n", err);
- goto unreg_notifier;
+ goto remove_opps;
}
tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
@@ -893,9 +902,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
remove_governor:
devfreq_remove_governor(&tegra_devfreq_governor);
-unreg_notifier:
- clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
-
remove_opps:
dev_pm_opp_remove_all_dynamic(&pdev->dev);
@@ -912,7 +918,6 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
devfreq_remove_device(tegra->devfreq);
devfreq_remove_governor(&tegra_devfreq_governor);
- clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
dev_pm_opp_remove_all_dynamic(&pdev->dev);
reset_control_reset(tegra->reset);
--
2.22.0
^ permalink raw reply related
* [PATCH v4 10/24] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
The consecutive-down event tells that we should perform frequency
de-boosting, but boosting is in a reset state on start and hence the
event won't do anything useful for us and it will be just a dummy
interrupt request.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 32fe95458ee7..878c9396bb8c 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -558,7 +558,6 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
<< ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_NUM_SHIFT;
val |= ACTMON_DEV_CTRL_AVG_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_AVG_BELOW_WMARK_EN;
- val |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
val |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
val |= ACTMON_DEV_CTRL_ENB;
--
2.22.0
^ permalink raw reply related
* [PATCH v4 09/24] PM / devfreq: tegra30: Reset boosting on startup
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
Governor could be stopped while boosting is active. We have assumption
that everything is reset on governor's restart, including the boosting
value, which was missed.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index d5d04c25023b..32fe95458ee7 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -536,6 +536,9 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
{
u32 val = 0, target_freq;
+ /* 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);
--
2.22.0
^ permalink raw reply related
* [PATCH v4 11/24] PM / devfreq: tegra30: Add debug messages
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
Add debug messages to know about what's happening in hardware and how
driver reacts.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 35 +++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 878c9396bb8c..c6c4a07d3e07 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -41,6 +41,7 @@
#define ACTMON_DEV_AVG_UPPER_WMARK 0x10
#define ACTMON_DEV_AVG_LOWER_WMARK 0x14
#define ACTMON_DEV_COUNT_WEIGHT 0x18
+#define ACTMON_DEV_COUNT 0x1c
#define ACTMON_DEV_AVG_COUNT 0x20
#define ACTMON_DEV_INTR_STATUS 0x24
@@ -276,6 +277,9 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
unsigned long *lower,
unsigned long *upper)
{
+ struct device *ddev = tegra->devfreq->dev.parent;
+ u32 offset = dev->config->offset;
+
/*
* Memory frequencies are guaranteed to have 1MHz granularity
* and thus we need this rounding down to get a proper watermarks
@@ -288,6 +292,9 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
*lower = tegra_actmon_lower_freq(tegra, target_freq);
*upper = tegra_actmon_upper_freq(tegra, target_freq);
+ dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
+ offset, target_freq, *lower, *upper);
+
*lower /= KHZ;
*upper /= KHZ;
@@ -367,11 +374,31 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
+static void actmon_device_debug(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev,
+ const char *prefix)
+{
+ dev_dbg(tegra->devfreq->dev.parent,
+ "%03x: %s: 0x%08x 0x%08x a %u %u %u c %u %u %u b %lu cpu %u\n",
+ dev->config->offset, prefix,
+ device_readl(dev, ACTMON_DEV_INTR_STATUS),
+ device_readl(dev, ACTMON_DEV_CTRL),
+ device_readl(dev, ACTMON_DEV_AVG_COUNT),
+ device_readl(dev, ACTMON_DEV_AVG_LOWER_WMARK),
+ device_readl(dev, ACTMON_DEV_AVG_UPPER_WMARK),
+ device_readl(dev, ACTMON_DEV_COUNT),
+ device_readl(dev, ACTMON_DEV_LOWER_WMARK),
+ device_readl(dev, ACTMON_DEV_UPPER_WMARK),
+ dev->boost_freq, cpufreq_get(0));
+}
+
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask;
+ 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);
dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
@@ -422,6 +449,8 @@ 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_device_debug(tegra, dev, "isr-");
}
static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
@@ -712,6 +741,7 @@ static struct devfreq_dev_profile tegra_devfreq_profile = {
static int tegra_governor_get_target(struct devfreq *devfreq,
unsigned long *freq)
{
+ struct device *ddev = devfreq->dev.parent;
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
@@ -734,6 +764,11 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
dev_target_freq = actmon_update_target(tegra, dev);
target_freq = max(target_freq, dev_target_freq);
+
+ dev_dbg(ddev, "%03x: upd: dev_target_freq %lu\n",
+ dev->config->offset, dev_target_freq);
+
+ actmon_device_debug(tegra, dev, "upd");
}
*freq = target_freq * KHZ;
--
2.22.0
^ permalink raw reply related
* [PATCH v4 12/24] PM / devfreq: tegra30: Inline all one-line functions
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
Depending on a kernel's configuration, a single line functions may not be
inlined by compiler (like enabled ftracing for example). Let's inline such
functions explicitly for consistency.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index c6c4a07d3e07..1a10df5dbbed 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -181,28 +181,29 @@ static struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
{ 250000, 100000 },
};
-static u32 actmon_readl(struct tegra_devfreq *tegra, u32 offset)
+static inline u32 actmon_readl(struct tegra_devfreq *tegra, u32 offset)
{
return readl_relaxed(tegra->regs + offset);
}
-static void actmon_writel(struct tegra_devfreq *tegra, u32 val, u32 offset)
+static inline void actmon_writel(struct tegra_devfreq *tegra,
+ u32 val, u32 offset)
{
writel_relaxed(val, tegra->regs + offset);
}
-static u32 device_readl(struct tegra_devfreq_device *dev, u32 offset)
+static inline u32 device_readl(struct tegra_devfreq_device *dev, u32 offset)
{
return readl_relaxed(dev->regs + offset);
}
-static void device_writel(struct tegra_devfreq_device *dev, u32 val,
- u32 offset)
+static inline void device_writel(struct tegra_devfreq_device *dev,
+ u32 val, u32 offset)
{
writel_relaxed(val, dev->regs + offset);
}
-static unsigned long do_percent(unsigned long val, unsigned int pct)
+static inline unsigned long do_percent(unsigned long val, unsigned int pct)
{
return val * pct / 100;
}
--
2.22.0
^ permalink raw reply related
* [PATCH v4 15/24] PM / devfreq: tegra30: Fix integer overflow on CPU's freq max out
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
There is another kHz-conversion bug in the code, resulting in integer
overflow. Although, this time the resulting value is 4294966296 and it's
close to ULONG_MAX, which is okay in this case.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 0de1efdaabf4..60ebf9c9cd86 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -172,7 +172,7 @@ struct tegra_actmon_emc_ratio {
};
static const struct tegra_actmon_emc_ratio actmon_emc_ratios[] = {
- { 1400000, ULONG_MAX },
+ { 1400000, ULONG_MAX / KHZ },
{ 1200000, 750000 },
{ 1100000, 600000 },
{ 1000000, 500000 },
--
2.22.0
^ permalink raw reply related
* [PATCH v4 14/24] PM / devfreq: tegra30: Ensure that target freq won't overflow
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-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 2f59c78930bd..0de1efdaabf4 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -460,6 +460,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 v4 16/24] PM / devfreq: tegra30: Use kHz units uniformly in the code
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-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 60ebf9c9cd86..6ebf0f505767 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;
@@ -459,8 +463,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 = min(dev->avg_freq + dev->boost_freq, KHZ_MAX);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
return target_freq;
@@ -497,6 +500,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)
@@ -504,6 +508,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:
*
@@ -525,7 +531,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;
@@ -565,17 +571,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);
@@ -687,7 +693,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;
@@ -710,7 +716,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;
@@ -727,7 +733,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);
@@ -773,7 +779,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;
}
@@ -909,7 +915,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;
@@ -929,6 +935,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 v4 17/24] PM / devfreq: tegra30: Use tracepoints for debugging
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
Debug messages create too much CPU and memory activity by themselves,
so it's difficult to debug lower rates and catch unwanted interrupts
that happen rarely. Tracepoints are ideal in that regards because they
do not contribute to the sampled date at all. This allowed me to catch
few problems which are fixed by the followup patches, without tracepoints
it would be much harder to do.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 43 +++-------
include/trace/events/tegra30_devfreq.h | 105 +++++++++++++++++++++++++
2 files changed, 117 insertions(+), 31 deletions(-)
create mode 100644 include/trace/events/tegra30_devfreq.h
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 6ebf0f505767..43c9c5fbfe91 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -19,6 +19,9 @@
#include <linux/reset.h>
#include <linux/workqueue.h>
+#define CREATE_TRACE_POINTS
+#include <trace/events/tegra30_devfreq.h>
+
#include "governor.h"
#define ACTMON_GLB_STATUS 0x0
@@ -283,9 +286,6 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
unsigned long *lower,
unsigned long *upper)
{
- struct device *ddev = tegra->devfreq->dev.parent;
- u32 offset = dev->config->offset;
-
/*
* Memory frequencies are guaranteed to have 1MHz granularity
* and thus we need this rounding down to get a proper watermarks
@@ -298,8 +298,8 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
*lower = tegra_actmon_lower_freq(tegra, target_freq);
*upper = tegra_actmon_upper_freq(tegra, target_freq);
- dev_dbg(ddev, "%03x: target_freq %lu lower freq %lu upper freq %lu\n",
- offset, target_freq, *lower, *upper);
+ trace_device_lower_upper(dev->config->offset, target_freq,
+ *lower, *upper);
/*
* The upper watermark should take into account CPU's frequency
@@ -377,30 +377,13 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
device_writel(dev, lower + delta, ACTMON_DEV_LOWER_WMARK);
}
-static void actmon_device_debug(struct tegra_devfreq *tegra,
- struct tegra_devfreq_device *dev,
- const char *prefix)
-{
- dev_dbg(tegra->devfreq->dev.parent,
- "%03x: %s: 0x%08x 0x%08x a %u %u %u c %u %u %u b %lu cpu %u\n",
- dev->config->offset, prefix,
- device_readl(dev, ACTMON_DEV_INTR_STATUS),
- device_readl(dev, ACTMON_DEV_CTRL),
- device_readl(dev, ACTMON_DEV_AVG_COUNT),
- device_readl(dev, ACTMON_DEV_AVG_LOWER_WMARK),
- device_readl(dev, ACTMON_DEV_AVG_UPPER_WMARK),
- device_readl(dev, ACTMON_DEV_COUNT),
- device_readl(dev, ACTMON_DEV_LOWER_WMARK),
- device_readl(dev, ACTMON_DEV_UPPER_WMARK),
- dev->boost_freq, cpufreq_get(0));
-}
-
static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
- actmon_device_debug(tegra, dev, "isr+");
+ trace_device_isr_enter(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
intr_status = device_readl(dev, ACTMON_DEV_INTR_STATUS);
avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
@@ -455,7 +438,8 @@ 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_device_debug(tegra, dev, "isr-");
+ trace_device_isr_exit(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
}
static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
@@ -749,7 +733,6 @@ static struct devfreq_dev_profile tegra_devfreq_profile = {
static int tegra_governor_get_target(struct devfreq *devfreq,
unsigned long *freq)
{
- struct device *ddev = devfreq->dev.parent;
struct devfreq_dev_status *stat;
struct tegra_devfreq *tegra;
struct tegra_devfreq_device *dev;
@@ -770,13 +753,11 @@ static int tegra_governor_get_target(struct devfreq *devfreq,
dev = &tegra->devices[i];
dev_target_freq = actmon_update_target(tegra, dev);
-
target_freq = max(target_freq, dev_target_freq);
- dev_dbg(ddev, "%03x: upd: dev_target_freq %lu\n",
- dev->config->offset, dev_target_freq);
-
- actmon_device_debug(tegra, dev, "upd");
+ trace_device_target_freq(dev->config->offset, dev_target_freq);
+ trace_device_target_update(tegra->regs, dev->config->offset,
+ dev->boost_freq, cpufreq_get(0));
}
*freq = target_freq;
diff --git a/include/trace/events/tegra30_devfreq.h b/include/trace/events/tegra30_devfreq.h
new file mode 100644
index 000000000000..8f264a489daf
--- /dev/null
+++ b/include/trace/events/tegra30_devfreq.h
@@ -0,0 +1,105 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM tegra30_devfreq
+
+#if !defined(_TRACE_TEGRA30_DEVFREQ_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_TEGRA30_DEVFREQ_H
+
+#include <linux/io.h>
+#include <linux/tracepoint.h>
+#include <linux/types.h>
+
+DECLARE_EVENT_CLASS(device_state,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, intr_status)
+ __field(u32, ctrl)
+ __field(u32, avg_count)
+ __field(u32, avg_lower)
+ __field(u32, avg_upper)
+ __field(u32, count)
+ __field(u32, lower)
+ __field(u32, upper)
+ __field(u32, boost_freq)
+ __field(u32, cpu_freq)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->intr_status = readl_relaxed(base + offset + 0x24);
+ __entry->ctrl = readl_relaxed(base + offset + 0x0);
+ __entry->avg_count = readl_relaxed(base + offset + 0x20);
+ __entry->avg_lower = readl_relaxed(base + offset + 0x14);
+ __entry->avg_upper = readl_relaxed(base + offset + 0x10);
+ __entry->count = readl_relaxed(base + offset + 0x1c);
+ __entry->lower = readl_relaxed(base + offset + 0x8);
+ __entry->upper = readl_relaxed(base + offset + 0x4);
+ __entry->boost_freq = boost;
+ __entry->cpu_freq = cpufreq;
+ ),
+ TP_printk("%03x: intr 0x%08x ctrl 0x%08x avg %010u %010u %010u cnt %010u %010u %010u boost %010u cpu %u",
+ __entry->offset,
+ __entry->intr_status,
+ __entry->ctrl,
+ __entry->avg_count,
+ __entry->avg_lower,
+ __entry->avg_upper,
+ __entry->count,
+ __entry->lower,
+ __entry->upper,
+ __entry->boost_freq,
+ __entry->cpu_freq)
+);
+
+DEFINE_EVENT(device_state, device_isr_enter,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+DEFINE_EVENT(device_state, device_isr_exit,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+DEFINE_EVENT(device_state, device_target_update,
+ TP_PROTO(void __iomem *base, u32 offset, u32 boost, u32 cpufreq),
+ TP_ARGS(base, offset, boost, cpufreq));
+
+TRACE_EVENT(device_lower_upper,
+ TP_PROTO(u32 offset, u32 target, u32 lower, u32 upper),
+ TP_ARGS(offset, target, lower, upper),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, target)
+ __field(u32, lower)
+ __field(u32, upper)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->target = target;
+ __entry->lower = lower;
+ __entry->upper = upper;
+ ),
+ TP_printk("%03x: freq %010u lower freq %010u upper freq %010u",
+ __entry->offset,
+ __entry->target,
+ __entry->lower,
+ __entry->upper)
+);
+
+TRACE_EVENT(device_target_freq,
+ TP_PROTO(u32 offset, u32 target),
+ TP_ARGS(offset, target),
+ TP_STRUCT__entry(
+ __field(u32, offset)
+ __field(u32, target)
+ ),
+ TP_fast_assign(
+ __entry->offset = offset;
+ __entry->target = target;
+ ),
+ TP_printk("%03x: freq %010u", __entry->offset, __entry->target)
+);
+#endif /* _TRACE_TEGRA30_DEVFREQ_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
--
2.22.0
^ permalink raw reply related
* [PATCH v4 19/24] PM / devfreq: tegra30: Optimize upper consecutive watermark selection
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
The memory activity counter may get a bit higher than a watermark which
is selected based on OPP that corresponds to a highest EMC rate, in this
case watermark is lower than the actual memory activity is and thus
results in unwanted "upper" interrupts.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 8d6bf6e9f1ae..c3cf87231d25 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -363,7 +363,18 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
tegra_actmon_get_lower_upper(tegra, dev, freq - 1, &lower, &upper);
delta = do_percent(upper - lower, dev->config->boost_up_threshold);
- device_writel(dev, lower + delta, ACTMON_DEV_UPPER_WMARK);
+
+ /*
+ * The memory events count could go a bit higher than the maximum
+ * defined by the OPPs, hence make the upper watermark infinitely
+ * high to avoid unnecessary upper interrupts in that case.
+ */
+ if (freq == tegra->max_freq)
+ upper = ULONG_MAX;
+ else
+ upper = lower + delta;
+
+ device_writel(dev, upper, ACTMON_DEV_UPPER_WMARK);
/*
* Meanwhile the lower mark is based on the average value
--
2.22.0
^ permalink raw reply related
* [PATCH v4 18/24] PM / devfreq: tegra30: Optimize CPUFreq notifier
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
When CPU's memory activity is low or memory activity is high such that
CPU's frequency contribution to the boosting is not taken into account,
then there is no need to schedule devfreq's update. This eliminates
unnecessary CPU activity during of idling caused by the scheduled work.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 73 +++++++++++++++++++++++++++----
1 file changed, 64 insertions(+), 9 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 43c9c5fbfe91..8d6bf6e9f1ae 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -216,10 +216,10 @@ static inline 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)
+static unsigned long actmon_cpu_to_emc_rate(struct tegra_devfreq *tegra,
+ unsigned int cpu_freq)
{
const 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++) {
@@ -239,15 +239,15 @@ tegra_actmon_account_cpu_freq(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev,
unsigned long target_freq)
{
- unsigned long static_cpu_emc_freq;
+ unsigned long cpu_emc_freq = 0;
- if (dev->config->avg_dependency_threshold &&
- 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);
- }
+ if (!dev->config->avg_dependency_threshold)
+ return target_freq;
- return target_freq;
+ if (dev->avg_freq > dev->config->avg_dependency_threshold)
+ cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpufreq_get(0));
+
+ return max(target_freq, cpu_emc_freq);
}
static unsigned long tegra_actmon_lower_freq(struct tegra_devfreq *tegra,
@@ -531,16 +531,71 @@ static void tegra_actmon_delayed_update(struct work_struct *work)
mutex_unlock(&tegra->devfreq->lock);
}
+static unsigned long
+tegra_actmon_cpufreq_contribution(struct tegra_devfreq *tegra,
+ unsigned int cpu_freq)
+{
+ unsigned long freq, static_cpu_emc_freq;
+
+ /* check whether CPU's freq is taken into account at all */
+ if (tegra->devices[MCCPU].avg_freq <=
+ tegra->devices[MCCPU].config->avg_dependency_threshold)
+ return 0;
+
+ static_cpu_emc_freq = actmon_cpu_to_emc_rate(tegra, cpu_freq);
+
+ /* compare static CPU-EMC freq with MCALL */
+ freq = tegra->devices[MCALL].avg_freq +
+ tegra->devices[MCALL].boost_freq;
+
+ freq = tegra_actmon_upper_freq(tegra, freq);
+
+ if (freq == tegra->max_freq || freq >= static_cpu_emc_freq)
+ return 0;
+
+ /* compare static CPU-EMC freq with MCCPU */
+ freq = tegra->devices[MCCPU].avg_freq +
+ tegra->devices[MCCPU].boost_freq;
+
+ freq = tegra_actmon_upper_freq(tegra, freq);
+
+ if (freq == tegra->max_freq || freq >= static_cpu_emc_freq)
+ return 0;
+
+ return static_cpu_emc_freq;
+}
+
static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
unsigned long action, void *ptr)
{
+ struct cpufreq_freqs *freqs = ptr;
struct tegra_devfreq *tegra;
+ unsigned long old, new;
if (action != CPUFREQ_POSTCHANGE)
return NOTIFY_OK;
tegra = container_of(nb, struct tegra_devfreq, cpu_rate_change_nb);
+ /*
+ * Quickly check whether CPU frequency should be taken into account
+ * at all, without blocking CPUFreq's core.
+ */
+ if (mutex_trylock(&tegra->devfreq->lock)) {
+ old = tegra_actmon_cpufreq_contribution(tegra, freqs->old);
+ new = tegra_actmon_cpufreq_contribution(tegra, freqs->new);
+ mutex_unlock(&tegra->devfreq->lock);
+
+ /*
+ * If CPU's frequency shouldn't be taken into account at
+ * the moment, then there is no need to update the devfreq's
+ * state because ISR will re-check CPU's frequency on the
+ * next interrupt.
+ */
+ if (old == new)
+ return NOTIFY_OK;
+ }
+
/*
* CPUFreq driver should support CPUFREQ_ASYNC_NOTIFICATION in order
* to allow asynchronous notifications. This means we can't block
--
2.22.0
^ permalink raw reply related
* [PATCH v4 20/24] PM / devfreq: tegra30: Optimize upper average watermark selection
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
I noticed that CPU may be crossing the dependency threshold very
frequently for some workloads and this results in a lot of interrupts
which could be avoided if MCALL client is keeping actual EMC frequency
at a higher rate.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index c3cf87231d25..4d582809acb6 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -314,7 +314,8 @@ 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)
+ struct tegra_devfreq_device *dev,
+ unsigned long freq)
{
unsigned long avg_threshold, lower, upper;
@@ -323,6 +324,15 @@ static void tegra_devfreq_update_avg_wmark(struct tegra_devfreq *tegra,
avg_threshold = dev->config->avg_dependency_threshold;
avg_threshold = avg_threshold * ACTMON_SAMPLING_PERIOD;
+ /*
+ * If cumulative EMC frequency selection is higher than the
+ * device's, then there is no need to set upper watermark to
+ * a lower value because it will result in unnecessary upper
+ * interrupts.
+ */
+ if (freq * ACTMON_SAMPLING_PERIOD > upper)
+ upper = freq * ACTMON_SAMPLING_PERIOD;
+
/*
* We want to get interrupts when MCCPU client crosses the
* dependency threshold in order to take into / out of account
@@ -392,6 +402,7 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
u32 intr_status, dev_ctrl, avg_intr_mask, avg_count;
+ unsigned long freq;
trace_device_isr_enter(tegra->regs, dev->config->offset,
dev->boost_freq, cpufreq_get(0));
@@ -405,8 +416,10 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
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);
+ if (intr_status & avg_intr_mask) {
+ freq = clk_get_rate(tegra->emc_clock) / KHZ;
+ tegra_devfreq_update_avg_wmark(tegra, dev, freq);
+ }
if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_UPPER) {
/*
@@ -525,7 +538,7 @@ static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
dev = &tegra->devices[i];
- tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev, freq);
tegra_devfreq_update_wmark(tegra, dev, freq);
}
@@ -630,7 +643,7 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
device_writel(dev, dev->avg_freq * ACTMON_SAMPLING_PERIOD,
ACTMON_DEV_INIT_AVG);
- tegra_devfreq_update_avg_wmark(tegra, dev);
+ tegra_devfreq_update_avg_wmark(tegra, dev, dev->avg_freq);
tegra_devfreq_update_wmark(tegra, dev, dev->avg_freq);
device_writel(dev, ACTMON_COUNT_WEIGHT, ACTMON_DEV_COUNT_WEIGHT);
--
2.22.0
^ permalink raw reply related
* [PATCH v4 21/24] PM / devfreq: tegra30: Synchronize average count on target's update
From: Dmitry Osipenko @ 2019-07-07 22:33 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: <20190707223303.6755-1-digetx@gmail.com>
The average count may get out of sync if interrupt was disabled / avoided
for a long time due to upper watermark optimization, hence it should be
re-synced on each target's update to ensure that watermarks are set up
correctly on EMC rate-change notification and that a correct frequency
is selected for device.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 4d582809acb6..8a674fad26be 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -466,11 +466,41 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
dev->boost_freq, cpufreq_get(0));
}
+static void tegra_devfreq_sync_avg_count(struct tegra_devfreq *tegra,
+ struct tegra_devfreq_device *dev)
+{
+ u32 avg_count, avg_freq, old_upper, new_upper;
+
+ avg_count = device_readl(dev, ACTMON_DEV_AVG_COUNT);
+ avg_freq = avg_count / ACTMON_SAMPLING_PERIOD;
+
+ old_upper = tegra_actmon_upper_freq(tegra, dev->avg_freq);
+ new_upper = tegra_actmon_upper_freq(tegra, avg_freq);
+
+ /* similar to ISR, see comments in actmon_isr_device() */
+ if (old_upper != new_upper) {
+ dev->avg_freq = avg_freq;
+ dev->boost_freq = 0;
+ }
+}
+
static unsigned long actmon_update_target(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
unsigned long target_freq;
+ /*
+ * The avg_count / avg_freq is getting snapshoted on device's
+ * interrupt, but there are cases where actual value need to
+ * be utilized on target's update, like CPUFreq boosting and
+ * overriding the min freq via /sys/class/devfreq/devfreq0/min_freq
+ * because we're optimizing the upper watermark based on the
+ * actual EMC frequency. This means that interrupt may be
+ * inactive for a long time and thus making snapshoted value
+ * outdated.
+ */
+ tegra_devfreq_sync_avg_count(tegra, dev);
+
target_freq = min(dev->avg_freq + dev->boost_freq, KHZ_MAX);
target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq);
--
2.22.0
^ permalink raw reply related
* [PATCH v4 22/24] PM / devfreq: tegra30: Include appropriate header
From: Dmitry Osipenko @ 2019-07-07 22:33 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: <20190707223303.6755-1-digetx@gmail.com>
It's not very correct to include mod_devicetable.h for the OF device
drivers and of_device.h should be included instead.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 8a674fad26be..19e872a64148 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -13,7 +13,7 @@
#include <linux/io.h>
#include <linux/irq.h>
#include <linux/module.h>
-#include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/reset.h>
--
2.22.0
^ permalink raw reply related
* [PATCH v4 23/24] PM / devfreq: tegra30: Increase sampling period to 16ms
From: Dmitry Osipenko @ 2019-07-07 22:33 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: <20190707223303.6755-1-digetx@gmail.com>
Increase sampling period by 4ms to get a nicer pow2 value, converting
diving into shifts in the code. That's more preferable for Tegra30 that
doesn't have hardware divider instructions because of older Cortex-A9 CPU.
In a result boosting events are delayed by 4ms, which is not sensible in
practice at all.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 19e872a64148..bde19b758643 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -71,7 +71,7 @@
* 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_SAMPLING_PERIOD 16 /* ms */
#define KHZ 1000
--
2.22.0
^ permalink raw reply related
* [PATCH v4 24/24] PM / devfreq: tegra20/30: Add Dmitry as a maintainer
From: Dmitry Osipenko @ 2019-07-07 22:33 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: <20190707223303.6755-1-digetx@gmail.com>
I was contributing to the NVIDIA Tegra20+ devfreq drivers recently and
want to help keep them working and evolving in the future.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
MAINTAINERS | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 95d4bd85df44..4e47ce737376 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10356,6 +10356,15 @@ F: include/linux/memblock.h
F: mm/memblock.c
F: Documentation/core-api/boot-time-mm.rst
+MEMORY FREQUENCY SCALING DRIVERS FOR NVIDIA TEGRA
+M: Dmitry Osipenko <digetx@gmail.com>
+L: linux-pm@vger.kernel.org
+L: linux-tegra@vger.kernel.org
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git
+S: Maintained
+F: drivers/devfreq/tegra20-devfreq.c
+F: drivers/devfreq/tegra30-devfreq.c
+
MEMORY MANAGEMENT
L: linux-mm@kvack.org
W: http://www.linux-mm.org
--
2.22.0
^ permalink raw reply related
* [PATCH v4 13/24] PM / devfreq: tegra30: Constify structs
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-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 1a10df5dbbed..2f59c78930bd 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 v4 07/24] PM / devfreq: tegra30: Use CPUFreq notifier
From: Dmitry Osipenko @ 2019-07-07 22: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: <20190707223303.6755-1-digetx@gmail.com>
The CPU's client need to take into account that CPUFreq may change
while memory activity not, staying high. Thus an appropriate frequency
notifier should be used in addition to the clk-notifier.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/devfreq/tegra30-devfreq.c | 105 +++++++++++++++++++++++++-----
1 file changed, 88 insertions(+), 17 deletions(-)
diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 2bf65409ddd8..48a799fa5f63 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -17,6 +17,7 @@
#include <linux/platform_device.h>
#include <linux/pm_opp.h>
#include <linux/reset.h>
+#include <linux/workqueue.h>
#include "governor.h"
@@ -154,7 +155,10 @@ struct tegra_devfreq {
struct clk *emc_clock;
unsigned long max_freq;
- struct notifier_block rate_change_nb;
+ struct notifier_block clk_rate_change_nb;
+
+ struct work_struct update_work;
+ struct notifier_block cpu_rate_change_nb;
struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
@@ -456,8 +460,8 @@ static irqreturn_t actmon_thread_isr(int irq, void *data)
return handled ? IRQ_HANDLED : IRQ_NONE;
}
-static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
- unsigned long action, void *ptr)
+static int tegra_actmon_clk_notify_cb(struct notifier_block *nb,
+ unsigned long action, void *ptr)
{
struct clk_notifier_data *data = ptr;
struct tegra_devfreq_device *dev;
@@ -467,7 +471,7 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
if (action != POST_RATE_CHANGE)
return NOTIFY_OK;
- tegra = container_of(nb, struct tegra_devfreq, rate_change_nb);
+ tegra = container_of(nb, struct tegra_devfreq, clk_rate_change_nb);
/*
* EMC rate could change due to three reasons:
@@ -496,6 +500,37 @@ static int tegra_actmon_rate_notify_cb(struct notifier_block *nb,
return NOTIFY_OK;
}
+static void tegra_actmon_delayed_update(struct work_struct *work)
+{
+ struct tegra_devfreq *tegra = container_of(work, struct tegra_devfreq,
+ update_work);
+
+ mutex_lock(&tegra->devfreq->lock);
+ update_devfreq(tegra->devfreq);
+ mutex_unlock(&tegra->devfreq->lock);
+}
+
+static int tegra_actmon_cpu_notify_cb(struct notifier_block *nb,
+ unsigned long action, void *ptr)
+{
+ struct tegra_devfreq *tegra;
+
+ if (action != CPUFREQ_POSTCHANGE)
+ return NOTIFY_OK;
+
+ tegra = container_of(nb, struct tegra_devfreq, cpu_rate_change_nb);
+
+ /*
+ * CPUFreq driver should support CPUFREQ_ASYNC_NOTIFICATION in order
+ * to allow asynchronous notifications. This means we can't block
+ * here for too long, otherwise CPUFreq's core will complain with a
+ * warning splat.
+ */
+ schedule_work(&tegra->update_work);
+
+ return NOTIFY_OK;
+}
+
static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
struct tegra_devfreq_device *dev)
{
@@ -527,9 +562,16 @@ static void tegra_actmon_configure_device(struct tegra_devfreq *tegra,
device_writel(dev, val, ACTMON_DEV_CTRL);
}
-static void tegra_actmon_start(struct tegra_devfreq *tegra)
+static void tegra_actmon_stop_device(struct tegra_devfreq_device *dev)
+{
+ device_writel(dev, 0x00000000, ACTMON_DEV_CTRL);
+ device_writel(dev, ACTMON_INTR_STATUS_CLEAR, ACTMON_DEV_INTR_STATUS);
+}
+
+static int tegra_actmon_start(struct tegra_devfreq *tegra)
{
unsigned int i;
+ int err;
actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
ACTMON_GLB_PERIOD_CTRL);
@@ -537,7 +579,30 @@ 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]);
+ /*
+ * We are estimating CPU's memory bandwidth requirement based on
+ * amount of memory accesses and system's load, judging by CPU's
+ * frequency. We also don't want to receive events about CPU's
+ * frequency transaction when governor is stopped, hence notifier
+ * is registered dynamically.
+ */
+ err = cpufreq_register_notifier(&tegra->cpu_rate_change_nb,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ if (err) {
+ dev_err(tegra->devfreq->dev.parent,
+ "Failed to register rate change notifier: %d\n", err);
+ goto err_stop;
+ }
+
enable_irq(tegra->irq);
+
+ return 0;
+
+err_stop:
+ for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
+ tegra_actmon_stop_device(&tegra->devices[i]);
+
+ return err;
}
static void tegra_actmon_stop(struct tegra_devfreq *tegra)
@@ -546,11 +611,13 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
disable_irq(tegra->irq);
- for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
- device_writel(&tegra->devices[i], 0x00000000, ACTMON_DEV_CTRL);
- device_writel(&tegra->devices[i], ACTMON_INTR_STATUS_CLEAR,
- ACTMON_DEV_INTR_STATUS);
- }
+ cpufreq_unregister_notifier(&tegra->cpu_rate_change_nb,
+ CPUFREQ_TRANSITION_NOTIFIER);
+
+ cancel_work_sync(&tegra->update_work);
+
+ for (i = 0; i < ARRAY_SIZE(tegra->devices); i++)
+ tegra_actmon_stop_device(&tegra->devices[i]);
}
static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -659,6 +726,7 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
unsigned int event, void *data)
{
struct tegra_devfreq *tegra = dev_get_drvdata(devfreq->dev.parent);
+ int ret = 0;
/*
* Couple device with the governor early as it is needed at
@@ -669,7 +737,7 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
switch (event) {
case DEVFREQ_GOV_START:
devfreq_monitor_start(devfreq);
- tegra_actmon_start(tegra);
+ ret = tegra_actmon_start(tegra);
break;
case DEVFREQ_GOV_STOP:
@@ -684,11 +752,11 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
case DEVFREQ_GOV_RESUME:
devfreq_monitor_resume(devfreq);
- tegra_actmon_start(tegra);
+ ret = tegra_actmon_start(tegra);
break;
}
- return 0;
+ return ret;
}
static struct devfreq_governor tegra_devfreq_governor = {
@@ -794,9 +862,12 @@ 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;
+ tegra->cpu_rate_change_nb.notifier_call = tegra_actmon_cpu_notify_cb;
+ INIT_WORK(&tegra->update_work, tegra_actmon_delayed_update);
+
+ tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb;
err = clk_notifier_register(tegra->emc_clock,
- &tegra->rate_change_nb);
+ &tegra->clk_rate_change_nb);
if (err) {
dev_err(&pdev->dev,
"Failed to register rate change notifier\n");
@@ -823,7 +894,7 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
devfreq_remove_governor(&tegra_devfreq_governor);
unreg_notifier:
- clk_notifier_unregister(tegra->emc_clock, &tegra->rate_change_nb);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
remove_opps:
dev_pm_opp_remove_all_dynamic(&pdev->dev);
@@ -841,7 +912,7 @@ static int tegra_devfreq_remove(struct platform_device *pdev)
devfreq_remove_device(tegra->devfreq);
devfreq_remove_governor(&tegra_devfreq_governor);
- clk_notifier_unregister(tegra->emc_clock, &tegra->rate_change_nb);
+ clk_notifier_unregister(tegra->emc_clock, &tegra->clk_rate_change_nb);
dev_pm_opp_remove_all_dynamic(&pdev->dev);
reset_control_reset(tegra->reset);
--
2.22.0
^ permalink raw reply related
* Re: [PATCH v3 6/6] interconnect: Add OPP table support for interconnects
From: Saravana Kannan @ 2019-07-07 21:48 UTC (permalink / raw)
To: Vincent Guittot
Cc: Georgi Djakov, Rob Herring, Mark Rutland, Viresh Kumar,
Nishanth Menon, Stephen Boyd, Rafael J. Wysocki, Sweeney, Sean,
daidavid1, Rajendra Nayak, sibis, Bjorn Andersson, Evan Green,
Android Kernel Team, open list:THERMAL,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
linux-kernel
In-Reply-To: <CAKfTPtBHrXG1QZzcaStWCtL3nx+vE_-WKtOhjiHbjFQiw9Yk8w@mail.gmail.com>
On Thu, Jul 4, 2019 at 12:12 AM Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
> On Wed, 3 Jul 2019 at 23:33, Saravana Kannan <saravanak@google.com> wrote:
> >
> > On Tue, Jul 2, 2019 at 11:45 PM Vincent Guittot
> > <vincent.guittot@linaro.org> wrote:
> > >
> > > On Wed, 3 Jul 2019 at 03:10, Saravana Kannan <saravanak@google.com> wrote:
> > > >
> > > > Interconnect paths can have different performance points. Now that OPP
> > > > framework supports bandwidth OPP tables, add OPP table support for
> > > > interconnects.
> > > >
> > > > Devices can use the interconnect-opp-table DT property to specify OPP
> > > > tables for interconnect paths. And the driver can obtain the OPP table for
> > > > an interconnect path by calling icc_get_opp_table().
> > >
> > > The opp table of a path must come from the aggregation of OPP tables
> > > of the interconnect providers.
> >
> > The aggregation of OPP tables of the providers is certainly the
> > superset of what a path can achieve, but to say that OPPs for
> > interconnect path should match that superset is an oversimplification
> > of the reality in hardware.
> >
> > There are lots of reasons an interconnect path might not want to use
> > all the available bandwidth options across all the interconnects in
> > the route.
> >
> > 1. That particular path might not have been validated or verified
> > during the HW design process for some of the frequencies/bandwidth
> > combinations of the providers.
>
> All these constraint are provider's constraints and not consumer's one
>
> The consumer asks for a bandwidth according to its needs and then the
> providers select the optimal bandwidth of each interconnect after
> aggregating all the request and according to what OPP have been
> validated
Not really. The screening can be a consumer specific issue. The
consumer IP itself might have some issue with using too low of a
bandwidth or bandwidth that's not within some range. It should not be
the provider's job to take into account all the IP that might be
connected to the interconnects. If the interconnect HW itself didn't
change, the provider driver shouldn't need to change. By your
definition, a provider driver will have to account for all the
possible bus masters that might be connected to it across all SoCs.
That's not good design nor is it scalable.
> >
> > 2. Similarly during parts screening in the factory, some of the
> > combinations might not have been screened and can't be guaranteed
> > to work.
>
> As above, it's the provider's job to select the final bandwidth
> according to its constraint
Same reply as above.
> >
> > 3. Only a certain set of bandwidth levels might make sense to use from
> > a power/performance balance given the device using it. For example:
> > - The big CPU might not want to use some of the lower bandwidths
> > but the little CPU might want to.
> > - The big CPU might not want to use some intermediate bandwidth
> > points if they don't save a lot of power compared to a higher
> > bandwidth levels, but the little CPU might want to.
> > - The little CPU might never want to use the higher set of
> > bandwidth levels since they won't be power efficient for the use
> > cases that might run on it.
>
> These example are quite vague about the reasons why little might never
> want to use higher bandwidth.
How is it vague? I just said because of power/performance balance.
> But then, if little doesn't ask high bandwidth it will not use them.
If you are running a heuristics based algorithm to pick bandwidth,
this is how it'll know NOT to use some of the bandwidth levels.
> >
> > 4. It might not make sense from a system level power perspective.
> > Let's take an example of a path S (source) -> A -> B -> C -> D
> > (destination).
> > - A supports only 2, 5, 7 and 10 GB/s. B supports 1, 2 ... 10 GB/s.
> > C supports 5 and 10 GB/s
> > - If you combine and list the superset of bandwidth levels
> > supported in that path, that'd be 1, 2, 3, ... 10 GB/s.
> > - Which set of bandwidth levels make sense will depend on the
> > hardware characteristics of the interconnects.
> > - If B is the biggest power sink, then you might want to use all 10
> > levels.
> > - If A is the biggest power sink, then you might want to use all 2,
> > 5 and 10 GB/s of the levels.
> > - If C is the biggest power sink then you might only want to use 5
> > and 10 GB/s
> > - The more hops and paths you get the more convoluted this gets.
> >
> > 5. The design of the interconnects themselves might have an impact on
> > which bandwidth levels are used.
> > - For example, the FIFO depth between two specific interconnects
> > might affect the valid bandwidth levels for a specific path.
> > - Say S1 -> A -> B -> D1, S2 -> C -> B -> D1 and S2 -> C -> D2 are
> > three paths.
> > - If C <-> B FIFO depth is small, then there might be a requirement
> > that C and B be closely performance matched to avoid system level
> > congestion due to back pressure.
> > - So S2 -> D1 path can't use all the bandwidth levels supported by
> > C-B combination.
> > - But S2 -> D2 can use all the bandwidth levels supported by C.
> > - And S1 -> D1 can use all the levels supported by A-B combination.
> >
>
> All the examples above makes sense but have to be handle by the
> provider not the consumer. The consumer asks for a bandwidth according
> to its constraints. Then the provider which is the driver that manages
> the interconnect IP, should manage all this hardware and platform
> specific stuff related to the interconnect IP in order to set the
> optimal bandwidth that fit both consumer constraint and platform
> specific configuration.
Sure, but the provider itself can have interconnect properties to
indicate which other interconnects it's tied to. And the provider will
still need the interconnect-opp-table to denote which bandwidth levels
are sensible to use with each of its connections.
So in some instances the interconnect-opp-table covers the needs of
purely consumers and in some instances purely providers. But in either
case, it's still needed to describe the hardware properly.
-Saravana
> > These are just some of the reasons I could recollect in a few minutes.
> > These are all real world cases I had to deal with in the past several
> > years of dealing with scaling interconnects. I'm sure vendors and SoCs
> > I'm not familiar with have other good reasons I'm not aware of.
> >
> > Trying to figure this all out by aggregating OPP tables of
> > interconnect providers just isn't feasible nor is it efficient. The
> > OPP tables for an interconnect path is describing the valid BW levels
> > supported by that path and verified in hardware and makes a lot of
> > sense to capture it clearly in DT.
> >
> > > So such kind of OPP table should be at
> > > provider level but not at path level.
> >
> > They can also use it if they want to, but they'll probably want to use
> > a frequency OPP table.
> >
> >
> > -Saravana
> >
> > >
> > > >
> > > > Signed-off-by: Saravana Kannan <saravanak@google.com>
> > > > ---
> > > > drivers/interconnect/core.c | 27 ++++++++++++++++++++++++++-
> > > > include/linux/interconnect.h | 7 +++++++
> > > > 2 files changed, 33 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> > > > index 871eb4bc4efc..881bac80bc1e 100644
> > > > --- a/drivers/interconnect/core.c
> > > > +++ b/drivers/interconnect/core.c
> > > > @@ -47,6 +47,7 @@ struct icc_req {
> > > > */
> > > > struct icc_path {
> > > > size_t num_nodes;
> > > > + struct opp_table *opp_table;
> > > > struct icc_req reqs[];
> > > > };
> > > >
> > > > @@ -313,7 +314,7 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
> > > > {
> > > > struct icc_path *path = ERR_PTR(-EPROBE_DEFER);
> > > > struct icc_node *src_node, *dst_node;
> > > > - struct device_node *np = NULL;
> > > > + struct device_node *np = NULL, *opp_node;
> > > > struct of_phandle_args src_args, dst_args;
> > > > int idx = 0;
> > > > int ret;
> > > > @@ -381,10 +382,34 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)
> > > > dev_err(dev, "%s: invalid path=%ld\n", __func__, PTR_ERR(path));
> > > > mutex_unlock(&icc_lock);
> > > >
> > > > + opp_node = of_parse_phandle(np, "interconnect-opp-table", idx);
> > > > + if (opp_node) {
> > > > + path->opp_table = dev_pm_opp_of_find_table_from_node(opp_node);
> > > > + of_node_put(opp_node);
> > > > + }
> > > > +
> > > > +
> > > > return path;
> > > > }
> > > > EXPORT_SYMBOL_GPL(of_icc_get);
> > > >
> > > > +/**
> > > > + * icc_get_opp_table() - Get the OPP table that corresponds to a path
> > > > + * @path: reference to the path returned by icc_get()
> > > > + *
> > > > + * This function will return the OPP table that corresponds to a path handle.
> > > > + * If the interconnect API is disabled, NULL is returned and the consumer
> > > > + * drivers will still build. Drivers are free to handle this specifically, but
> > > > + * they don't have to.
> > > > + *
> > > > + * Return: opp_table pointer on success. NULL is returned when the API is
> > > > + * disabled or the OPP table is missing.
> > > > + */
> > > > +struct opp_table *icc_get_opp_table(struct icc_path *path)
> > > > +{
> > > > + return path->opp_table;
> > > > +}
> > > > +
> > > > /**
> > > > * icc_set_bw() - set bandwidth constraints on an interconnect path
> > > > * @path: reference to the path returned by icc_get()
> > > > diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> > > > index dc25864755ba..0c0bc55f0e89 100644
> > > > --- a/include/linux/interconnect.h
> > > > +++ b/include/linux/interconnect.h
> > > > @@ -9,6 +9,7 @@
> > > >
> > > > #include <linux/mutex.h>
> > > > #include <linux/types.h>
> > > > +#include <linux/pm_opp.h>
> > > >
> > > > /* macros for converting to icc units */
> > > > #define Bps_to_icc(x) ((x) / 1000)
> > > > @@ -28,6 +29,7 @@ struct device;
> > > > struct icc_path *icc_get(struct device *dev, const int src_id,
> > > > const int dst_id);
> > > > struct icc_path *of_icc_get(struct device *dev, const char *name);
> > > > +struct opp_table *icc_get_opp_table(struct icc_path *path);
> > > > void icc_put(struct icc_path *path);
> > > > int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
> > > >
> > > > @@ -49,6 +51,11 @@ static inline void icc_put(struct icc_path *path)
> > > > {
> > > > }
> > > >
> > > > +static inline struct opp_table *icc_get_opp_table(struct icc_path *path)
> > > > +{
> > > > + return NULL;
> > > > +}
> > > > +
> > > > static inline int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
> > > > {
> > > > return 0;
> > > > --
> > > > 2.22.0.410.gd8fdbe21b5-goog
> > > >
>
> --
> To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com.
>
^ permalink raw reply
* RE: [PATCH] cpuidle/drivers/mobile: Add new governor for mobile/embedded systems
From: Doug Smythies @ 2019-07-07 17:02 UTC (permalink / raw)
To: 'Daniel Lezcano'
Cc: linux-kernel, 'Rafael J. Wysocki',
'Thomas Gleixner', 'Greg Kroah-Hartman',
'open list:CPU IDLE TIME MANAGEMENT FRAMEWORK', rafael
In-Reply-To: <000a01d531d3$3471a060$9d54e120$@net>
On 2019.07.03 12:12 Doug Smythies wrote:
> On 2019.07.03 08:16 Daniel Lezcano wrote:
>> On 03/07/2019 16:23, Doug Smythies wrote:
>>> On 2019.06.20 04:58 Daniel Lezcano wrote:
> ...
>>> Anyway, I did a bunch of tests and such, but have deleted
>>> most from this e-mail, because it's just noise. I'll
>>> include just one set:
>>>
>>> For a work load that would normally result in a lot of use
>>> of shallow idle states (single core pipe-test * 2 cores).
>>
>> Can you share the tests and the command lines?
>
> Yes, give me a few days to repeat the tests and write
> it up properly. I am leaving town in an hour and for a day.
O.K. I re-did the tests and made a new web page with, I think,
everything I used.
...
>> The governor can be better by selecting the shallow states, the
>> scheduler has to interact with the governor to give clues about the
>> load, that is identified and will be the next step.
>>
>> Is it possible to check with the schedutil governor instead?
>
> Oh, I already have some data, just didn't include it before:
>
> Idle governor, teo; CPU frequency scaling: intel-cpufreq/schedutil;
> Processor package power: 40.4 watts; 4.9 uSec/loop
>
> Idle governor, mobile; CPU frequency scaling: intel-cpufreq/schedutil;
> Processor package power: 12.7 watts; 19.7 uSec/loop
>
> Idle governor, teo; CPU frequency scaling: intel-cpufreq/schedutil;
> Idle states 0-3 disabled (note: Idle state 4 is the deepest on my system)
> Processor package power: 36.9 watts; 8.3 uSec/loop
> In my notes I wrote: "Huh?? I do not understand this result, as I had
> expected more similar to the mobile governor". But I did not investigate.
The reason for the big difference was/is that with the "mobile"
governor the CPU frequency never scales up for this test. It can be
sluggish to scale up with the teo and the menu governors, but always
eventually does. I also tried the acpi-cpufreq driver with similar results.
> Anyway, the schedutil test is the one I'll repeat and write up better.
New summary (similar to old):
governor usec/loop watts
mobile 19.8 12.67
teo 4.87 40.28
menu 4.85 40.25
teo-idle-0-3-dis 8.30 36.85
Graphs, details and source codes:
http://www.smythies.com/~doug/linux/idle/mobile/index.html
... Doug
^ permalink raw reply
* Re: [PATCH v3 1/3] cpuidle-powernv : forced wakeup for stop states
From: Nicholas Piggin @ 2019-07-07 10:13 UTC (permalink / raw)
To: Abhishek Goel, linux-kernel, linux-pm, linuxppc-dev
Cc: daniel.lezcano, dja, ego, mpe, rjw
In-Reply-To: <20190704091827.19555-2-huntbag@linux.vnet.ibm.com>
Abhishek Goel's on July 4, 2019 7:18 pm:
> Currently, the cpuidle governors determine what idle state a idling CPU
> should enter into based on heuristics that depend on the idle history on
> that CPU. Given that no predictive heuristic is perfect, there are cases
> where the governor predicts a shallow idle state, hoping that the CPU will
> be busy soon. However, if no new workload is scheduled on that CPU in the
> near future, the CPU may end up in the shallow state.
>
> This is problematic, when the predicted state in the aforementioned
> scenario is a shallow stop state on a tickless system. As we might get
> stuck into shallow states for hours, in absence of ticks or interrupts.
>
> To address this, We forcefully wakeup the cpu by setting the
> decrementer. The decrementer is set to a value that corresponds with the
> residency of the next available state. Thus firing up a timer that will
> forcefully wakeup the cpu. Few such iterations will essentially train the
> governor to select a deeper state for that cpu, as the timer here
> corresponds to the next available cpuidle state residency. Thus, cpu will
> eventually end up in the deepest possible state.
>
> Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
> ---
>
> Auto-promotion
> v1 : started as auto promotion logic for cpuidle states in generic
> driver
> v2 : Removed timeout_needed and rebased the code to upstream kernel
> Forced-wakeup
> v1 : New patch with name of forced wakeup started
> v2 : Extending the forced wakeup logic for all states. Setting the
> decrementer instead of queuing up a hrtimer to implement the logic.
> v3 : Cleanly handle setting/resetting of decrementer so as to not break
> irq work
>
> arch/powerpc/include/asm/time.h | 2 ++
> arch/powerpc/kernel/time.c | 40 +++++++++++++++++++++++++++++++
> drivers/cpuidle/cpuidle-powernv.c | 32 +++++++++++++++++++++++++
> 3 files changed, 74 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index 54f4ec1f9..a3bd4f3c0 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -188,6 +188,8 @@ static inline unsigned long tb_ticks_since(unsigned long tstamp)
> extern u64 mulhdu(u64, u64);
> #endif
>
> +extern int set_dec_before_idle(u64 timeout);
> +extern void reset_dec_after_idle(void);
> extern void div128_by_32(u64 dividend_high, u64 dividend_low,
> unsigned divisor, struct div_result *dr);
>
> diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
> index 694522308..814de3469 100644
> --- a/arch/powerpc/kernel/time.c
> +++ b/arch/powerpc/kernel/time.c
> @@ -576,6 +576,46 @@ void arch_irq_work_raise(void)
>
> #endif /* CONFIG_IRQ_WORK */
>
> +/*
> + * Returns 1 if we have reprogrammed the decrementer for idle.
> + * Returns 0 if the decrementer is unchanged.
> + */
> +int set_dec_before_idle(u64 timeout)
> +{
> + u64 *next_tb = this_cpu_ptr(&decrementers_next_tb);
> + u64 now = get_tb_or_rtc();
> +
> + /*
> + * Ensure that the timeout is at least one microsecond
> + * before the current decrement value. Else, we will
> + * unnecesarily wakeup again within a microsecond.
> + */
> + if (now + timeout + 512 > *next_tb)
I would pass this 512 in as a parameter and put the comment in the
idle code. Timer code does not know/care.
Maybe return bool and call it try_set_dec_before_idle.
> + return 0;
> +
> + set_dec(timeout);
This needs to have
if (test_irq_work_pending())
set_dec(1);
here AFAIKS
> +
> + return 1;
> +}
> +
> +void reset_dec_after_idle(void)
> +{
> + u64 now;
> + u64 *next_tb;
> +
> + if (test_irq_work_pending())
> + return;
> +
> + now = get_tb_or_rtc();
> + next_tb = this_cpu_ptr(&decrementers_next_tb);
> + if (now >= *next_tb)
> + return;
Are you sure it's okay to escape early in this case?
Thanks,
Nick
^ permalink raw reply
* cpufreq notifiers break suspend -- Re: suspend broken in next-20190704 on Thinkpad X60
From: Pavel Machek @ 2019-07-06 20:30 UTC (permalink / raw)
To: Rafael J. Wysocki, viresh.kumar, mka, ulf.hansson
Cc: Rafael J. Wysocki, Linux-pm mailing list, kernel list,
Stephen Rothwell
In-Reply-To: <CAJZ5v0irbn-Xd47KExw=h7On7KShCm6rThCo0q4-zn=o_x6_HQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2666 bytes --]
Hi!
> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
>
> I would try
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
>
> to narrow down the scope somewhat.
Bisect says:
572542c81dec533b7dd3778ea9f5949a00595f68 is the first bad commit
Author: Viresh Kumar <viresh.kumar@linaro.org>
cpufreq: Register notifiers with the PM QoS framework
This registers the notifiers for min/max frequency constraints
with the
Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
Unfortunately, it does not revert cleanly:
pavel@duo:/data/l/linux-next-32$ git show
572542c81dec533b7dd3778ea9f5949a00595f68 | patch -REsp1
6 out of 11 hunks FAILED -- saving rejects to file
drivers/cpufreq/cpufreq.c.rej
pavel@duo:/data/l/linux-next-32$ git bisect log
# bad: [1e2a4c9019eb53f62790fadf86c14a54f4cf4888] Merge branch
'pm-cpufreq-new' into linux-next
# good: [f012a132824fc870b90980540f727c76fc72e244] Merge branches
'acpica', 'acpi-osl', 'acpi-tables', 'acpi-misc' and 'acpi-tools' into
linux-next
git bisect start '1e2a4c9019eb53f62790fadf86c14a54f4cf4888'
'f012a132824fc870b90980540f727c76fc72e244'
# good: [48a8a5f9a326d1c1a5505d51fb98086e5003f37e] Add linux-next
specific files for 20190701
git bisect good 48a8a5f9a326d1c1a5505d51fb98086e5003f37e
# good: [96021e491dbf30bd1c5c1a753992838c8d8d00cb] Merge branches
'acpi-apei', 'acpi-doc', 'acpi-soc' and 'acpi-pmic' into linux-next
git bisect good 96021e491dbf30bd1c5c1a753992838c8d8d00cb
# bad: [141467868c1f7bf1c4e8394a39d47d4db38cd2f1] cpufreq:
intel_pstate: Reuse refresh_frequency_limits()
git bisect bad 141467868c1f7bf1c4e8394a39d47d4db38cd2f1
# good: [2a79ea5ec53973c8711b54d33ace5c77659dc8f8] PM / QOS: Pass
request type to dev_pm_qos_read_value()
git bisect good 2a79ea5ec53973c8711b54d33ace5c77659dc8f8
# bad: [572542c81dec533b7dd3778ea9f5949a00595f68] cpufreq: Register
notifiers with the PM QoS framework
git bisect bad 572542c81dec533b7dd3778ea9f5949a00595f68
# good: [208637b37824c8956fe28d277835a403ee35fa84] PM / QoS: Add
support for MIN/MAX frequency constraints
git bisect good 208637b37824c8956fe28d277835a403ee35fa84
# first bad commit: [572542c81dec533b7dd3778ea9f5949a00595f68]
cpufreq: Register notifiers with the PM QoS framework
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: Re: Re: [PATCH v2 3/7] rtc: mt6397: improvements of rtc driver
From: Alexandre Belloni @ 2019-07-06 20:04 UTC (permalink / raw)
To: Frank Wunderlich
Cc: Lee Jones, Rob Herring, Mark Rutland, Matthias Brugger, Sean Wang,
Sebastian Reichel, Alessandro Zummo, devicetree, linux-arm-kernel,
linux-mediatek, linux-kernel, linux-pm, linux-rtc, Eddie Huang,
Thomas Gleixner, Richard Fontana, Allison Randal,
David S . Miller, Mauro Carvalho Chehab, Greg Kroah-Hartman,
Rob Herring, Linus Walleij, Nicolas Ferre, Paul E . McKenney,
Josef Friedl
In-Reply-To: <trinity-a4e5f99f-00bc-4e90-9a48-64dbc6ba9c08-1562429720701@3c-app-gmx-bs42>
On 06/07/2019 18:15:20+0200, Frank Wunderlich wrote:
> > Gesendet: Freitag, 05. Juli 2019 um 23:24 Uhr
> > Von: "Alexandre Belloni" <alexandre.belloni@bootlin.com>
>
> > Let's say the RTC has been used to start your platform, then the irq
> > handler will be called as soon as the irq is requested, leading to a
> > null pointer dereference.
>
> i cannot test this with my platform, but i have changed it in my repo
>
> https://github.com/frank-w/BPI-R2-4.14/commits/5.2-poweroff-mainline
>
> > Yes and IIRC, I did comment that the rtc change also had to be separated
> > from 1/7.
>
> also this is put in separate commit, can you take a look before i post v3?
>
> > Also, I really doubt this new compatible is necessary at all as you
> > could simply directly use mediatek,mt6397-rtc.
>
> imho this can confuse because the wrong chip-name is used in dts
>
This is not true, we do that all the time and the immediate benefit of
using the mt6397 compatible is that then there is no need to
synchronize between subsystems. If you want to be absolutely
conservative, you could use
compatible = "mediatek,mt6323-rtc", "mediatek,mt6397-rtc";
in your DT.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] Documentation: cpu-freq: Convert core.txt file to ReST format
From: Shreeya Patel @ 2019-07-06 19:56 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: skhan, Rafael J. Wysocki, Viresh Kumar, Jonathan Corbet, Linux PM,
Linux Kernel Mailing List, open list:DOCUMENTATION,
linux-kernel-mentees
In-Reply-To: <CAJZ5v0hovK+BY0kozGvkyCgR5CFHpZUu71BZRjUUdCYV8fM5Hg@mail.gmail.com>
On Sat, 2019-07-06 at 09:58 +0200, Rafael J. Wysocki wrote:
> On Fri, Jul 5, 2019 at 11:04 PM Shreeya Patel
> <shreeya.patel23498@gmail.com> wrote:
> >
> > Convert core file to ReST format, in order to allow it to
> > be parsed by Sphinx. Make a minor change of correcting the wrong
> > function name cpufreq_put_cpu to cpufreq_cpu_put.
> > Also create an index.rst file in cpu-freq and add it's entry
> > in the main Documentation/index.rst file.
> >
> > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
>
> I've said "no" no three previous attempts and this one is not
> different.
>
Sorry, I was not knowing about it.
> I don't want to have anything .rst in Documentation/cpu-freq/.
>
> There is a *new* admin-guide doc for cpufreq already and what is
> missing is a *new* driver-api one.
>
Yes I saw that but it didn't include all the details given in
Documentation/cpu-freq hence I thought of sending this initial patch.
Thanks
> Thanks!
^ permalink raw reply
* Re: suspend broken in next-20190704 on Thinkpad X60
From: Pavel Machek @ 2019-07-06 19:01 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Linux-pm mailing list, kernel list,
Stephen Rothwell
In-Reply-To: <CAJZ5v0irbn-Xd47KExw=h7On7KShCm6rThCo0q4-zn=o_x6_HQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1774 bytes --]
Hi!
> > > > Suspend is broken in next-20190704 on Thinkpad X60.
> > >
> > > Broken in what way? Any details?
> > >
> > > > It very very probably worked ok in 20190701.
> > >
> > > Well, please try the linux-next branch from linux-pm.git
> > > (git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git)
> > > alone and see if that fails.
> >
> > So... let me try this one?
> >
> > commit 1e2a4c9019eb53f62790fadf86c14a54f4cf4888 (patch)
...
> I'm not sure what you mean here. I'm guessing that you don't get back
> to the console from which you ran the pm-suspend command, but is X
> restored, for example? Is there any way to get into the system in
> that state?
>
> Anyway, if 5.2-rc7 is OK, something in this branch causes the problem
> to happen for you.
>
> I would try
>
> https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=f012a132824fc870b90980540f727c76fc72e244
>
> to narrow down the scope somewhat.
pavel@amd:/data/l/linux-next-32$ git bisect log
# bad: [1e2a4c9019eb53f62790fadf86c14a54f4cf4888] Merge branch
'pm-cpufreq-new' into linux-next
# good: [f012a132824fc870b90980540f727c76fc72e244] Merge branches
'acpica', 'acpi-osl', 'acpi-tables', 'acpi-misc' and 'acpi-tools' into
linux-next
git bisect start '1e2a4c9019eb53f62790fadf86c14a54f4cf4888'
'f012a132824fc870b90980540f727c76fc72e244'
# good: [48a8a5f9a326d1c1a5505d51fb98086e5003f37e] Add linux-next
specific files for 20190701
git bisect good 48a8a5f9a326d1c1a5505d51fb98086e5003f37e
I think I can handle this... when I'm near the AC power.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ 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