linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/11] More improvements for Tegra30 devfreq driver
@ 2019-06-23 21:46 Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int Dmitry Osipenko
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

Hello,

This series is a followup to [1] which addresses some additional review
comments that were made by Thierry Reding to [1] and makes several
important changes to the driver, fixing excessive interrupts activity.
In the end I'm proposing myself as a maintainer for the driver.

[1] https://lore.kernel.org/lkml/0fb50eb1-a173-1756-6889-2526a10ac707@gmail.com/T/

Dmitry Osipenko (11):
  PM / devfreq: tegra30: Change irq type to unsigned int
  PM / devfreq: tegra30: Keep interrupt disabled while governor is
    stopped
  PM / devfreq: tegra30: Handle possible round-rate error
  PM / devfreq: tegra30: Drop write-barrier
  PM / devfreq: tegra30: Rework frequency management logic
  PM / devfreq: tegra30: Reset boosting on startup
  PM / devfreq: tegra30: Reset boosting if clock rate changed
  PM / devfreq: tegra30: Stop de-boosting once it's finished
  PM / devfreq: tegra30: Don't enable consecutive-down interrupt on
    startup
  PM / devfreq: tegra30: Add debug messages
  PM / devfreq: tegra30: Add Dmitry as maintainer

 MAINTAINERS                       |   8 +
 drivers/devfreq/tegra30-devfreq.c | 542 ++++++++++++++++++++++--------
 2 files changed, 409 insertions(+), 141 deletions(-)

-- 
2.22.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 02/11] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped Dmitry Osipenko
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

IRQ numbers are always positive, hence the corresponding variable should
be unsigned to keep types consistent. This is a minor change that cleans
up code a tad more.

Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index a6ba75f4106d..a27300f40b0b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -160,7 +160,7 @@ struct tegra_devfreq {
 
 	struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
 
-	int irq;
+	unsigned int		irq;
 };
 
 struct tegra_actmon_emc_ratio {
@@ -618,12 +618,12 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 		return PTR_ERR(tegra->emc_clock);
 	}
 
-	tegra->irq = platform_get_irq(pdev, 0);
-	if (tegra->irq < 0) {
-		err = tegra->irq;
+	err = platform_get_irq(pdev, 0);
+	if (err < 0) {
 		dev_err(&pdev->dev, "Failed to get IRQ: %d\n", err);
 		return err;
 	}
+	tegra->irq = err;
 
 	reset_control_assert(tegra->reset);
 
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 02/11] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 03/11] PM / devfreq: tegra30: Handle possible round-rate error Dmitry Osipenko
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

There is no real need to keep interrupt always-enabled, will be nicer
to keep it disabled where appropriate.

Suggested-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 43 ++++++++++++++++---------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index a27300f40b0b..5e2b133babdd 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -11,6 +11,7 @@
 #include <linux/devfreq.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/irq.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/platform_device.h>
@@ -416,8 +417,6 @@ static void tegra_actmon_start(struct tegra_devfreq *tegra)
 {
 	unsigned int i;
 
-	disable_irq(tegra->irq);
-
 	actmon_writel(tegra, ACTMON_SAMPLING_PERIOD - 1,
 		      ACTMON_GLB_PERIOD_CTRL);
 
@@ -442,8 +441,6 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
 	}
 
 	actmon_write_barrier(tegra);
-
-	enable_irq(tegra->irq);
 }
 
 static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
@@ -552,6 +549,12 @@ static int tegra_governor_event_handler(struct devfreq *devfreq,
 {
 	struct tegra_devfreq *tegra = dev_get_drvdata(devfreq->dev.parent);
 
+	/*
+	 * Couple device with the governor early as it is needed at
+	 * the moment of governor's start (used by ISR).
+	 */
+	tegra->devfreq = devfreq;
+
 	switch (event) {
 	case DEVFREQ_GOV_START:
 		devfreq_monitor_start(devfreq);
@@ -586,10 +589,11 @@ static struct devfreq_governor tegra_devfreq_governor = {
 
 static int tegra_devfreq_probe(struct platform_device *pdev)
 {
-	struct tegra_devfreq *tegra;
 	struct tegra_devfreq_device *dev;
-	unsigned int i;
+	struct tegra_devfreq *tegra;
+	struct devfreq *devfreq;
 	unsigned long rate;
+	unsigned int i;
 	int err;
 
 	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
@@ -625,6 +629,16 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 	tegra->irq = err;
 
+	irq_set_status_flags(tegra->irq, IRQ_NOAUTOEN);
+
+	err = devm_request_threaded_irq(&pdev->dev, tegra->irq, NULL,
+					actmon_thread_isr, IRQF_ONESHOT,
+					"tegra-devfreq", tegra);
+	if (err) {
+		dev_err(&pdev->dev, "Interrupt request failed: %d\n", err);
+		return err;
+	}
+
 	reset_control_assert(tegra->reset);
 
 	err = clk_prepare_enable(tegra->clock);
@@ -672,28 +686,15 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	}
 
 	tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock);
-	tegra->devfreq = devfreq_add_device(&pdev->dev,
-					    &tegra_devfreq_profile,
-					    "tegra_actmon",
-					    NULL);
+	devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile,
+				     "tegra_actmon", NULL);
 	if (IS_ERR(tegra->devfreq)) {
 		err = PTR_ERR(tegra->devfreq);
 		goto remove_governor;
 	}
 
-	err = devm_request_threaded_irq(&pdev->dev, tegra->irq, NULL,
-					actmon_thread_isr, IRQF_ONESHOT,
-					"tegra-devfreq", tegra);
-	if (err) {
-		dev_err(&pdev->dev, "Interrupt request failed: %d\n", err);
-		goto remove_devfreq;
-	}
-
 	return 0;
 
-remove_devfreq:
-	devfreq_remove_device(tegra->devfreq);
-
 remove_governor:
 	devfreq_remove_governor(&tegra_devfreq_governor);
 
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 03/11] PM / devfreq: tegra30: Handle possible round-rate error
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 02/11] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 04/11] PM / devfreq: tegra30: Drop write-barrier Dmitry Osipenko
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

The EMC clock rate rounding technically could fail, hence let's handle
the error cases properly.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 5e2b133babdd..5e606ae3f238 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -592,8 +592,8 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	struct tegra_devfreq_device *dev;
 	struct tegra_devfreq *tegra;
 	struct devfreq *devfreq;
-	unsigned long rate;
 	unsigned int i;
+	long rate;
 	int err;
 
 	tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL);
@@ -650,8 +650,14 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 
 	reset_control_deassert(tegra->reset);
 
-	tegra->max_freq = clk_round_rate(tegra->emc_clock, ULONG_MAX) / KHZ;
+	rate = clk_round_rate(tegra->emc_clock, ULONG_MAX);
+	if (rate < 0) {
+		dev_err(&pdev->dev, "Failed to round clock rate: %ld\n", rate);
+		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++) {
 		dev = tegra->devices + i;
@@ -662,6 +668,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev)
 	for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) {
 		rate = clk_round_rate(tegra->emc_clock, rate);
 
+		if (rate < 0) {
+			dev_err(&pdev->dev,
+				"Failed to round clock rate: %ld\n", rate);
+			err = rate;
+			goto remove_opps;
+		}
+
 		err = dev_pm_opp_add(&pdev->dev, rate, 0);
 		if (err) {
 			dev_err(&pdev->dev, "Failed to add OPP: %d\n", err);
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 04/11] PM / devfreq: tegra30: Drop write-barrier
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (2 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 03/11] PM / devfreq: tegra30: Handle possible round-rate error Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 05/11] PM / devfreq: tegra30: Rework frequency management logic Dmitry Osipenko
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

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	[flat|nested] 13+ messages in thread

* [PATCH v1 05/11] PM / devfreq: tegra30: Rework frequency management logic
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (3 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 04/11] PM / devfreq: tegra30: Drop write-barrier Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 06/11] PM / devfreq: tegra30: Reset boosting on startup Dmitry Osipenko
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

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. Secondly, the CPU's client need to
take into account that CPUFreq may change while memory activity not,
thus an appropriate frequency notifier should be used instead of the
clk-notifier. 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.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 411 ++++++++++++++++++++++--------
 1 file changed, 301 insertions(+), 110 deletions(-)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 4be7858c33bc..813b0f490b90 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"
 
@@ -40,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
 
@@ -47,6 +49,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 +67,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 +145,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 +156,8 @@ struct tegra_devfreq {
 
 	struct clk		*emc_clock;
 	unsigned long		max_freq;
-	unsigned long		cur_freq;
+
+	struct work_struct	update_work;
 	struct notifier_block	rate_change_nb;
 
 	struct tegra_devfreq_device devices[ARRAY_SIZE(actmon_device_configs)];
@@ -205,42 +206,216 @@ 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 count)
+{
+	unsigned long static_cpu_emc_freq;
+
+	if (dev->config->avg_dependency_threshold &&
+	    dev->config->avg_dependency_threshold < 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 count,
+					 unsigned long *lower,
+					 unsigned long *upper)
+{
+	unsigned long target_freq, min;
+
+	target_freq = count / ACTMON_SAMPLING_PERIOD * KHZ;
+
+	/*
+	 * 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);
+
+	*lower = tegra_actmon_lower_freq(tegra, target_freq);
+
+	/*
+	 * When memory activity is below the minimum bandwidth that EMC
+	 * driver could provide, the upper watermark should be set for
+	 * the second OPP to avoid unnecessary interrupts activity when
+	 * the minimum threshold is crossed towards zero bandwidth.
+	 *
+	 *         ------ target_freq
+	 *         |        ----------------- minimum possible freq
+	 *         v        v
+	 * |0|----------|min freq|----|second freq|----|third freq|---...
+	 *  ^                                ^
+	 *  |                                |
+	 *  ---- lower watermark             ---- upper watermark
+	 */
+	min = tegra_actmon_upper_freq(tegra, 0);
+
+	if (target_freq < min)
+		*upper = tegra_actmon_upper_freq(tegra, min);
+	else
+		*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, count);
+
+	*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 count = dev->avg_count;
+	unsigned long lower, upper;
+
+	tegra_actmon_get_lower_upper(tegra, dev, count, &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 into / out of account the
+	 * CPU's freq.
+	 */
+	if (lower == 0 && dev->config->avg_dependency_threshold) {
+		if (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)
 {
-	u32 val = tegra->cur_freq * ACTMON_SAMPLING_PERIOD;
+	unsigned long count = device_readl(dev, ACTMON_DEV_COUNT);
+	unsigned long lower, upper, delta;
 
-	device_writel(dev, do_percent(val, dev->config->boost_up_threshold),
-		      ACTMON_DEV_UPPER_WMARK);
+	/*
+	 * Boosting logic kicks-in once lower / upper watermark is hit.
+	 *
+	 *                  count --------       --- upper delta
+	 *                               v       v
+	 * ...--|OPP N|--|lower|<------->-<--------->|upper|--|OPP N+1|--...
+	 *                          ^        ^
+	 *                          |        |
+	 *             lower delta---        |
+	 *                                   |
+	 * average count somewhere there------
+	 */
+
+	tegra_actmon_get_lower_upper(tegra, dev, count, &lower, &upper);
+
+	/* don't forget to take boosting into account */
+	upper += dev->boost_freq;
+
+	/* the lesser memory activity goes, the harder is boost-down */
+	delta = do_percent(count - lower, dev->config->boost_down_threshold);
+	device_writel(dev, count - delta, ACTMON_DEV_LOWER_WMARK);
 
-	device_writel(dev, do_percent(val, dev->config->boost_down_threshold),
-		      ACTMON_DEV_LOWER_WMARK);
+	/* the higher memory activity goes, the softer is boost-up */
+	delta = do_percent(upper - count, dev->config->boost_up_threshold);
+	device_writel(dev, count + delta, ACTMON_DEV_UPPER_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;
+	bool low_activity = true;
 
 	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);
+
+	/*
+	 * Instantly cutoff boosting if CPU isn't touching memory nearly
+	 * at all for awhile, this result in a more interactive governing.
+	 *
+	 * MCALL doesn't have the minimum activity threshold.
+	 *
+	 * Note that it's important to disabled boosting before calling
+	 * tegra_devfreq_update_wmark() because it takes boosting into
+	 * account.
+	 */
+	if (dev->config->avg_dependency_threshold &&
+	    dev->avg_count < dev->config->avg_dependency_threshold)
+		dev->boost_freq = 0;
+	else
+		low_activity = false;
+
 	if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_UPPER) {
 		/*
 		 * new_boost = min(old_boost * up_coef + step, max_freq)
@@ -249,12 +424,10 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
 					     dev->config->boost_up_coeff);
 		dev->boost_freq += ACTMON_BOOST_FREQ_STEP;
 
-		dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
-
-		if (dev->boost_freq >= tegra->max_freq)
+		if (dev->boost_freq >= tegra->max_freq && !low_activity)
 			dev->boost_freq = tegra->max_freq;
-		else
-			dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_ABOVE_WMARK_EN;
+
+		tegra_devfreq_update_wmark(tegra, dev);
 	} else if (intr_status & ACTMON_DEV_INTR_CONSECUTIVE_LOWER) {
 		/*
 		 * new_boost = old_boost * down_coef
@@ -263,19 +436,10 @@ 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;
+		tegra_devfreq_update_wmark(tegra, dev);
 	}
 
 	device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
@@ -283,43 +447,20 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
 	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)
+static unsigned long 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);
-	}
+	unsigned long avg_sustain_coef, 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;
 
-	if (dev->avg_count >= dev->config->avg_dependency_threshold)
-		dev->target_freq = max(dev->target_freq, static_cpu_emc_freq);
+	target_freq = dev->avg_count / ACTMON_SAMPLING_PERIOD;
+	target_freq = do_percent(target_freq, avg_sustain_coef);
+	target_freq += dev->boost_freq;
+	target_freq = tegra_actmon_account_cpu_freq(tegra, dev, target_freq,
+						    dev->avg_count);
+
+	return target_freq;
 }
 
 static irqreturn_t actmon_thread_isr(int irq, void *data)
@@ -347,26 +488,53 @@ static irqreturn_t actmon_thread_isr(int irq, void *data)
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
+static void tegra_actmon_update_watermarks(struct tegra_devfreq *tegra)
+{
+	struct tegra_devfreq_device *dev = tegra->devices;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(tegra->devices); i++, dev++) {
+		tegra_devfreq_update_avg_wmark(tegra, dev);
+		tegra_devfreq_update_wmark(tegra, dev);
+	}
+}
+
+static void tegra_actmon_delayed_update(struct work_struct *work)
+{
+	struct tegra_devfreq *tegra = container_of(work, struct tegra_devfreq,
+						   update_work);
+	unsigned long previous_freq;
+
+	mutex_lock(&tegra->devfreq->lock);
+
+	previous_freq = tegra->devfreq->previous_freq;
+
+	update_devfreq(tegra->devfreq);
+
+	/*
+	 * CPU freq changed and this may result in a change of a target
+	 * memory  rate, hence the watermarks may need to be updated too
+	 * because they depend on the CPU's freq and interrupt events will
+	 * stop if the watermark's range is now larger than it should be.
+	 */
+	if (tegra->devfreq->previous_freq != previous_freq)
+		tegra_actmon_update_watermarks(tegra);
+
+	mutex_unlock(&tegra->devfreq->lock);
+}
+
 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;
-	unsigned int i;
 
-	if (action != POST_RATE_CHANGE)
+	if (action != CPUFREQ_POSTCHANGE)
 		return NOTIFY_OK;
 
 	tegra = container_of(nb, struct tegra_devfreq, rate_change_nb);
 
-	tegra->cur_freq = data->new_rate / KHZ;
-
-	for (i = 0; i < ARRAY_SIZE(tegra->devices); i++) {
-		dev = &tegra->devices[i];
-
-		tegra_devfreq_update_wmark(tegra, dev);
-	}
+	/* notifier runs with a disabled preemption */
+	schedule_work(&tegra->update_work);
 
 	return NOTIFY_OK;
 }
@@ -374,11 +542,10 @@ 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);
@@ -403,9 +570,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);
@@ -413,7 +587,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->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)
@@ -422,11 +619,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->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,
@@ -469,13 +668,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 +685,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 +704,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 +720,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;
@@ -534,6 +734,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
@@ -544,7 +745,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:
@@ -559,11 +760,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 = {
@@ -642,7 +843,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,17 +871,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;
-	err = clk_notifier_register(tegra->emc_clock, &tegra->rate_change_nb);
-	if (err) {
-		dev_err(&pdev->dev,
-			"Failed to register rate change notifier\n");
-		goto remove_opps;
-	}
+	INIT_WORK(&tegra->update_work, tegra_actmon_delayed_update);
 
 	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);
@@ -697,9 +892,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->rate_change_nb);
-
 remove_opps:
 	dev_pm_opp_remove_all_dynamic(&pdev->dev);
 
@@ -716,7 +908,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->rate_change_nb);
 	dev_pm_opp_remove_all_dynamic(&pdev->dev);
 
 	reset_control_reset(tegra->reset);
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 06/11] PM / devfreq: tegra30: Reset boosting on startup
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (4 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 05/11] PM / devfreq: tegra30: Rework frequency management logic Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 07/11] PM / devfreq: tegra30: Reset boosting if clock rate changed Dmitry Osipenko
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

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 813b0f490b90..fc278f2f1b62 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -544,6 +544,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	[flat|nested] 13+ messages in thread

* [PATCH v1 07/11] PM / devfreq: tegra30: Reset boosting if clock rate changed
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (5 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 06/11] PM / devfreq: tegra30: Reset boosting on startup Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 08/11] PM / devfreq: tegra30: Stop de-boosting once it's finished Dmitry Osipenko
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

There is a situation when memory activity is going up, hence boosting up
starts to happen, and then governor ramps memory clock rate up. In this
case consecutive events may be stopped if new "COUNT" is within watermarks
range, meanwhile old boosting value remains, which is plainly wrong and
results in unneeded "go down" events after ramping up. In a result of this
change unnecessary interrupts activity goes even lower.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index fc278f2f1b62..6fb3ca125438 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -631,6 +631,24 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra)
 		tegra_actmon_stop_device(&tegra->devices[i]);
 }
 
+static void tegra_actmon_stop_boosting(struct tegra_devfreq *tegra)
+{
+	struct tegra_devfreq_device *dev = tegra->devices;
+	unsigned int i;
+	u32 dev_ctrl;
+
+	for (i = 0; i < ARRAY_SIZE(tegra->devices); i++, dev++) {
+		if (!dev->boost_freq)
+			continue;
+
+		dev_ctrl = device_readl(dev, ACTMON_DEV_CTRL);
+		dev_ctrl &= ~ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
+		device_writel(dev, dev_ctrl, ACTMON_DEV_CTRL);
+
+		dev->boost_freq = 0;
+	}
+}
+
 static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
 				u32 flags)
 {
@@ -656,6 +674,16 @@ static int tegra_devfreq_target(struct device *dev, unsigned long *freq,
 	if (err)
 		goto restore_min_rate;
 
+	/*
+	 * Hence boosting-up could be active at the moment of the rate-change
+	 * and in this case boosting should be reset because it doesn't relate
+	 * to the new state. If average won't follow shortly in a case of going
+	 * UP, then clock rate will drop back on next update due to the missed
+	 * boosting.
+	 */
+	if (rate != devfreq->previous_freq)
+		tegra_actmon_stop_boosting(tegra);
+
 	return 0;
 
 restore_min_rate:
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 08/11] PM / devfreq: tegra30: Stop de-boosting once it's finished
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (6 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 07/11] PM / devfreq: tegra30: Reset boosting if clock rate changed Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 09/11] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup Dmitry Osipenko
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

Once boosting value is 0, then further consecutive-down events won't do
anything useful since the only purpose of those consecutive events to
boost the freq and de-boosting is over in the case of 0. Note that upper
watermark is infinitely high in a case of frequency max out and thus the
upper events are stopping by themselves. In a result of this change all
parasite interrupts are fixed now and interrupts activity is nearly
non-existent now!

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/devfreq/tegra30-devfreq.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 6fb3ca125438..81449cc1392b 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -424,6 +424,8 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
 					     dev->config->boost_up_coeff);
 		dev->boost_freq += ACTMON_BOOST_FREQ_STEP;
 
+		dev_ctrl |= ACTMON_DEV_CTRL_CONSECUTIVE_BELOW_WMARK_EN;
+
 		if (dev->boost_freq >= tegra->max_freq && !low_activity)
 			dev->boost_freq = tegra->max_freq;
 
@@ -442,6 +444,10 @@ static void actmon_isr_device(struct tegra_devfreq *tegra,
 		tegra_devfreq_update_wmark(tegra, dev);
 	}
 
+	/* 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);
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v1 09/11] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (7 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 08/11] PM / devfreq: tegra30: Stop de-boosting once it's finished Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 10/11] PM / devfreq: tegra30: Add debug messages Dmitry Osipenko
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

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 81449cc1392b..168bfe78e525 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -572,7 +572,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	[flat|nested] 13+ messages in thread

* [PATCH v1 10/11] PM / devfreq: tegra30: Add debug messages
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (8 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 09/11] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
  2019-06-23 21:46 ` [PATCH v1 11/11] PM / devfreq: tegra30: Add Dmitry as maintainer Dmitry Osipenko
       [not found] ` <CGME20190623214841epcas5p161cceab51cb10d5146e57d49a696d5e5@epcms1p5>
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

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 | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c
index 168bfe78e525..452bc10d2d72 100644
--- a/drivers/devfreq/tegra30-devfreq.c
+++ b/drivers/devfreq/tegra30-devfreq.c
@@ -277,6 +277,8 @@ 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;
 	unsigned long target_freq, min;
 
 	target_freq = count / ACTMON_SAMPLING_PERIOD * KHZ;
@@ -312,6 +314,9 @@ static void tegra_actmon_get_lower_upper(struct tegra_devfreq *tegra,
 	else
 		*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;
 
@@ -384,12 +389,32 @@ static void tegra_devfreq_update_wmark(struct tegra_devfreq *tegra,
 	device_writel(dev, count + delta, ACTMON_DEV_UPPER_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;
 	bool low_activity = true;
 
+	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);
@@ -451,6 +476,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,
@@ -737,6 +764,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;
@@ -759,6 +787,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	[flat|nested] 13+ messages in thread

* [PATCH v1 11/11] PM / devfreq: tegra30: Add Dmitry as maintainer
  2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
                   ` (9 preceding siblings ...)
  2019-06-23 21:46 ` [PATCH v1 10/11] PM / devfreq: tegra30: Add debug messages Dmitry Osipenko
@ 2019-06-23 21:46 ` Dmitry Osipenko
       [not found] ` <CGME20190623214841epcas5p161cceab51cb10d5146e57d49a696d5e5@epcms1p5>
  11 siblings, 0 replies; 13+ messages in thread
From: Dmitry Osipenko @ 2019-06-23 21:46 UTC (permalink / raw)
  To: Thierry Reding, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

I was contributing to the NVIDIA Tegra30+ devfreq driver recently and want
to help keep it working and evolving in the future.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6c203278700f..ac347278f1fc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10214,6 +10214,14 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mzx/devfreq.git
 S:	Maintained
 F:	drivers/devfreq/tegra20-devfreq.c
 
+MEMORY FREQUENCY SCALING DRIVER FOR NVIDIA TEGRA30-TEGRA210
+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/tegra30-devfreq.c
+
 MEMORY MANAGEMENT
 L:	linux-mm@kvack.org
 W:	http://www.linux-mm.org
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int
       [not found] ` <CGME20190623214841epcas5p161cceab51cb10d5146e57d49a696d5e5@epcms1p5>
@ 2019-06-24  2:12   ` MyungJoo Ham
  0 siblings, 0 replies; 13+ messages in thread
From: MyungJoo Ham @ 2019-06-24  2:12 UTC (permalink / raw)
  To: Dmitry Osipenko, Thierry Reding, Kyungmin Park, Chanwoo Choi,
	Jonathan Hunter, Tomeu Vizoso
  Cc: linux-pm, linux-tegra, linux-kernel

>IRQ numbers are always positive, hence the corresponding variable should
>be unsigned to keep types consistent. This is a minor change that cleans
>up code a tad more.
>
>Suggested-by: Thierry Reding <thierry.reding@gmail.com>
>Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>


Cheers,
MyungJoo

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-06-24  3:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-23 21:46 [PATCH v1 00/11] More improvements for Tegra30 devfreq driver Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 02/11] PM / devfreq: tegra30: Keep interrupt disabled while governor is stopped Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 03/11] PM / devfreq: tegra30: Handle possible round-rate error Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 04/11] PM / devfreq: tegra30: Drop write-barrier Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 05/11] PM / devfreq: tegra30: Rework frequency management logic Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 06/11] PM / devfreq: tegra30: Reset boosting on startup Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 07/11] PM / devfreq: tegra30: Reset boosting if clock rate changed Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 08/11] PM / devfreq: tegra30: Stop de-boosting once it's finished Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 09/11] PM / devfreq: tegra30: Don't enable consecutive-down interrupt on startup Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 10/11] PM / devfreq: tegra30: Add debug messages Dmitry Osipenko
2019-06-23 21:46 ` [PATCH v1 11/11] PM / devfreq: tegra30: Add Dmitry as maintainer Dmitry Osipenko
     [not found] ` <CGME20190623214841epcas5p161cceab51cb10d5146e57d49a696d5e5@epcms1p5>
2019-06-24  2:12   ` [PATCH v1 01/11] PM / devfreq: tegra30: Change irq type to unsigned int MyungJoo Ham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).