[PATCH v3 0/3] PM QoS: Add support for memory bandwidth constraints

[PATCH v3 0/3] PM QoS: Add support for memory bandwidth constraints

[Tomeu Vizoso]

Hi,

have rebased on top of 3.17rc1 and changed the units to be megabits per second,
as suggested by Pavel. Follows the original blurb:

Some device drivers need to sustain transfers to or from external memory at
given fixed rates for a period of time. If we wait for the devfreq drivers to
react to this load, the user is likely going to perceive glitches in the user
experience.

Some common scenarios that would benefit from it are display controllers
setting a new mode, USB drivers starting isochronous transfers, camera drivers
starting to capture and hw media codecs doing their thing.

I'm proposing adding a new PM_QOS_MEMORY_BANDWIDTH class that device drivers
can use to register their memory bandwidth needs right when they become known,
so the external memory clock can be set at the optimum frequency, thus
preventing glitches.

There have been similar proposals before [0], the first main concern being that
constraints need to refer to a global resource such as CPUs. If they referred
to a network NIC or to a UART, there would be no way of knowing how to actually
constrain each device because there would be a single list of requests.

In this case, the memory bus is a global resource just like the CPUs are, and
all requests in this class will apply to a single device driver in any
particular system.

The second serious concern was that constraints need to be defined in a way
generic enough so they can be consumed in a wide array of particular systems
without placing the burden of taking into account hw specifics in the driver of
the memory client.

Here, the PM_QOS_MEMORY_BANDWIDTH constraints are specified always in kbs,
being the amount of data that the device driver wants to see transferred per
second. This allows for example for a camera driver to calculate the
appropriate value based on the size of each frame and the desired FPS and,
depending on the particular system where the camera block is used, the
appropriate external memory driver will convert that to the bus clock frequency
that makes sense for the hardware at hand.

The first patch adds the new class and also a new PM_QOS_SUM class type.

The second illustrates how the driver for such a memory client would request
its memory bandwidth requirement.

The third patch illustrates how an hypothetical driver for an external memory
controller would listen for changes in the aggregated bandwidth constraint and
would set a floor frequency rate on the appropriate clock. Depends on the
per-user clk series [1].

Most vendor trees carry their own solution for this specific issue, so it would
be pretty neat if mainline gained a similar mechanism so we get more power
management goodies in the right place.

[0] https://lkml.org/lkml/2012/3/10/165
[1] http://article.gmane.org/gmane.linux.ports.arm.kernel/349154

Thanks,

Tomeu

Tomeu Vizoso (3):
  PM / QoS: Add PM_QOS_MEMORY_BANDWIDTH class
  drm/tegra: Request memory bandwidth for the display controller
  memory: tegra124-emc: Add EMC driver

 Documentation/power/pm_qos_interface.txt |   4 +-
 drivers/gpu/drm/tegra/dc.c               |  13 ++++
 drivers/gpu/drm/tegra/drm.h              |   3 +
 drivers/memory/Kconfig                   |   8 +++
 drivers/memory/Makefile                  |   1 +
 drivers/memory/tegra124-emc.c            | 116 +++++++++++++++++++++++++++++++
 include/linux/pm_qos.h                   |   5 +-
 kernel/power/qos.c                       |  27 ++++++-
 8 files changed, 174 insertions(+), 3 deletions(-)
 create mode 100644 drivers/memory/tegra124-emc.c

-- 
1.9.3

[PATCH v3 1/3] PM / QoS: Add PM_QOS_MEMORY_BANDWIDTH class

[Tomeu Vizoso]

Also adds a class type PM_QOS_SUM that aggregates the values by summing them.

It can be used by memory controllers to calculate the optimum clock frequency
based on the bandwidth needs of the different memory clients.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
Acked-by: Pavel Machek <pavel@ucw.cz>

---

v3: * Changed units to be megabits per second
---
 Documentation/power/pm_qos_interface.txt |  4 +++-
 include/linux/pm_qos.h                   |  5 ++++-
 kernel/power/qos.c                       | 27 ++++++++++++++++++++++++++-
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index a5da5c7..129f7c0 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -5,7 +5,8 @@ performance expectations by drivers, subsystems and user space applications on
 one of the parameters.
 
 Two different PM QoS frameworks are available:
-1. PM QoS classes for cpu_dma_latency, network_latency, network_throughput.
+1. PM QoS classes for cpu_dma_latency, network_latency, network_throughput,
+memory_bandwidth.
 2. the per-device PM QoS framework provides the API to manage the per-device latency
 constraints and PM QoS flags.
 
@@ -13,6 +14,7 @@ Each parameters have defined units:
  * latency: usec
  * timeout: usec
  * throughput: kbs (kilo bit / sec)
+ * memory bandwidth: mbs (mega bit / sec)
 
 
 1. PM QoS framework
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 9ab4bf7..636e828 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -15,6 +15,7 @@ enum {
 	PM_QOS_CPU_DMA_LATENCY,
 	PM_QOS_NETWORK_LATENCY,
 	PM_QOS_NETWORK_THROUGHPUT,
+	PM_QOS_MEMORY_BANDWIDTH,
 
 	/* insert new class ID */
 	PM_QOS_NUM_CLASSES,
@@ -32,6 +33,7 @@ enum pm_qos_flags_status {
 #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE	(2000 * USEC_PER_SEC)
 #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE	(2000 * USEC_PER_SEC)
 #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE	0
+#define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE	0
 #define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE	0
 #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE	0
 #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT	(-1)
@@ -69,7 +71,8 @@ struct dev_pm_qos_request {
 enum pm_qos_type {
 	PM_QOS_UNITIALIZED,
 	PM_QOS_MAX,		/* return the largest value */
-	PM_QOS_MIN		/* return the smallest value */
+	PM_QOS_MIN,		/* return the smallest value */
+	PM_QOS_SUM		/* return the sum */
 };
 
 /*
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 884b770..5f4c006 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -105,11 +105,27 @@ static struct pm_qos_object network_throughput_pm_qos = {
 };
 
 
+static BLOCKING_NOTIFIER_HEAD(memory_bandwidth_notifier);
+static struct pm_qos_constraints memory_bw_constraints = {
+	.list = PLIST_HEAD_INIT(memory_bw_constraints.list),
+	.target_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
+	.default_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
+	.no_constraint_value = PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE,
+	.type = PM_QOS_SUM,
+	.notifiers = &memory_bandwidth_notifier,
+};
+static struct pm_qos_object memory_bandwidth_pm_qos = {
+	.constraints = &memory_bw_constraints,
+	.name = "memory_bandwidth",
+};
+
+
 static struct pm_qos_object *pm_qos_array[] = {
 	&null_pm_qos,
 	&cpu_dma_pm_qos,
 	&network_lat_pm_qos,
-	&network_throughput_pm_qos
+	&network_throughput_pm_qos,
+	&memory_bandwidth_pm_qos,
 };
 
 static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
@@ -130,6 +146,9 @@ static const struct file_operations pm_qos_power_fops = {
 /* unlocked internal variant */
 static inline int pm_qos_get_value(struct pm_qos_constraints *c)
 {
+	struct plist_node *node;
+	int total_value = 0;
+
 	if (plist_head_empty(&c->list))
 		return c->no_constraint_value;
 
@@ -140,6 +159,12 @@ static inline int pm_qos_get_value(struct pm_qos_constraints *c)
 	case PM_QOS_MAX:
 		return plist_last(&c->list)->prio;
 
+	case PM_QOS_SUM:
+		plist_for_each(node, &c->list)
+			total_value += node->prio;
+
+		return total_value;
+
 	default:
 		/* runtime check for not using enum */
 		BUG();
-- 
1.9.3

[PATCH v3 2/3] drm/tegra: Request memory bandwidth for the display controller

[Tomeu Vizoso]

For now the amount requested is based solely on the current mode's refresh
rate.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

---

v3: * Fixed the bandwidth calculation, and have the units be mbps
---
 drivers/gpu/drm/tegra/dc.c  | 13 +++++++++++++
 drivers/gpu/drm/tegra/drm.h |  3 +++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 6553fd2..83a6d4b 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
 
 #include <linux/clk.h>
 #include <linux/debugfs.h>
+#include <linux/pm_qos.h>
 #include <linux/reset.h>
 
 #include "dc.h"
@@ -753,6 +754,8 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
 	}
 
 	drm_vblank_off(drm, dc->pipe);
+
+	pm_qos_update_request(&dc->qos_request, PM_QOS_DEFAULT_VALUE);
 }
 
 static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -839,6 +842,7 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 	struct tegra_dc *dc = to_tegra_dc(crtc);
 	struct tegra_dc_window window;
 	u32 value;
+	unsigned long bandwidth;
 	int err;
 
 	drm_vblank_pre_modeset(crtc->dev, dc->pipe);
@@ -879,6 +883,10 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 	if (err < 0)
 		dev_err(dc->dev, "failed to enable root plane\n");
 
+	bandwidth = window.stride[0] * window.dst.h * 8;
+	bandwidth *= mode->clock * 1000 / 1024 / 1024;
+	pm_qos_update_request(&dc->qos_request, bandwidth);
+
 	return 0;
 }
 
@@ -1499,6 +1507,9 @@ static int tegra_dc_probe(struct platform_device *pdev)
 		return err;
 	}
 
+	pm_qos_add_request(&dc->qos_request, PM_QOS_MEMORY_BANDWIDTH,
+			   PM_QOS_DEFAULT_VALUE);
+
 	platform_set_drvdata(pdev, dc);
 
 	return 0;
@@ -1509,6 +1520,8 @@ static int tegra_dc_remove(struct platform_device *pdev)
 	struct tegra_dc *dc = platform_get_drvdata(pdev);
 	int err;
 
+	pm_qos_remove_request(&dc->qos_request);
+
 	err = host1x_client_unregister(&dc->client);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to unregister host1x client: %d\n",
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index e89c70f..a99e533d 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -12,6 +12,7 @@
 
 #include <uapi/drm/tegra_drm.h>
 #include <linux/host1x.h>
+#include <linux/pm_qos.h>
 
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
@@ -120,6 +121,8 @@ struct tegra_dc {
 	struct drm_pending_vblank_event *event;
 
 	const struct tegra_dc_soc_info *soc;
+
+	struct pm_qos_request qos_request;
 };
 
 static inline struct tegra_dc *
-- 
1.9.3

Re: [PATCH v3 2/3] drm/tegra: Request memory bandwidth for the display controller

[Pavel Machek]

On Tue 2014-08-19 10:38:42, Tomeu Vizoso wrote:
> For now the amount requested is based solely on the current mode's refresh
> rate.
> 
> Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

Acked-by: Pavel Machek <pavel@ucw.cz>

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[PATCH v3 3/3] memory: tegra124-emc: Add EMC driver

[Tomeu Vizoso]

Sets the EMC clock rate based on the bandwidth requirements registered by
memory clients through the PM_QOS_MEMORY_BANDWIDTH class.

Note: this is just an example and not a proper driver for a external memory
controller. Its only purpose is to illustrate how such a driver would set the
frequency of the external memory clock based on the bandwidth requirements of
memory clients.

Signed-off-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>
---
 drivers/memory/Kconfig        |   8 +++
 drivers/memory/Makefile       |   1 +
 drivers/memory/tegra124-emc.c | 116 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 125 insertions(+)
 create mode 100644 drivers/memory/tegra124-emc.c

diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index fab81a1..28c90c6 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -61,6 +61,14 @@ config TEGRA30_MC
 	  analysis, especially for IOMMU/SMMU(System Memory Management
 	  Unit) module.
 
+config TEGRA124_EMC
+	tristate "Tegra124 External Memory Controller (EMC) driver"
+	default y
+	depends on ARCH_TEGRA_124_SOC
+	help
+	  This driver is for the External Memory Controller (EMC) module
+	  available in Tegra124 SoCs.
+
 config FSL_CORENET_CF
 	tristate "Freescale CoreNet Error Reporting"
 	depends on FSL_SOC_BOOKE
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 4055c47..0a6d117 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_FSL_IFC)		+= fsl_ifc.o
 obj-$(CONFIG_MVEBU_DEVBUS)	+= mvebu-devbus.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
+obj-$(CONFIG_TEGRA124_EMC)	+= tegra124-emc.o
diff --git a/drivers/memory/tegra124-emc.c b/drivers/memory/tegra124-emc.c
new file mode 100644
index 0000000..2c4d61b
--- /dev/null
+++ b/drivers/memory/tegra124-emc.c
@@ -0,0 +1,116 @@
+/*
+ * Copyright (c) 2013, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <linux/clk.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_qos.h>
+
+#define DRV_NAME "tegra124-emc"
+#define EMC_FREQ_CUTOFF_USE_130_PERCENT	100000000
+#define EMC_FREQ_CUTOFF_USE_140_PERCENT	50000000
+#define BYTES_PER_EMC_CLOCK 16
+
+struct tegra124_emc {
+	struct clk *clk;
+	struct notifier_block memory_bw_notifier;
+};
+
+static struct platform_device *emc_pdev;
+
+static unsigned long tegra124_emc_bw_to_freq_req(unsigned long bw)
+{
+	return (bw + BYTES_PER_EMC_CLOCK - 1) / BYTES_PER_EMC_CLOCK;
+}
+
+static void tegra124_emc_update_rate(struct tegra124_emc *emc, unsigned long total_bandwidth)
+{
+	struct clk *emc_master;
+	unsigned long freq;
+
+	emc_master = clk_get_parent(emc->clk);
+	freq = tegra124_emc_bw_to_freq_req(total_bandwidth) * 1000;
+	freq = clk_round_rate(emc_master, freq);
+
+	/* Depending on frequency value, the amount of bandwidth usage % of
+	 * total we should use is different. Thus we should request a multiple of
+	 * original bandwidth on this.  Use 1.4 for < 50MHz, 1.3 for < 100MHz,
+	 * else 1.1 */
+	if (freq < EMC_FREQ_CUTOFF_USE_140_PERCENT)
+		total_bandwidth += 4 * total_bandwidth / 10; /* 1.4 */
+	else if (freq < EMC_FREQ_CUTOFF_USE_130_PERCENT)
+		total_bandwidth += 3 * total_bandwidth / 10; /* 1.3 */
+	else
+		total_bandwidth += total_bandwidth / 10; /* 1.1 */
+
+	freq = tegra124_emc_bw_to_freq_req(total_bandwidth) * 1000;
+
+	clk_set_floor_rate(emc->clk, freq);
+}
+
+int memory_bw_notify(struct notifier_block *nb, unsigned long total_bw, void *dummy)
+{
+	struct tegra124_emc *emc =
+		container_of(nb, struct tegra124_emc,
+			     memory_bw_notifier);
+
+	tegra124_emc_update_rate(emc, total_bw);
+
+	return NOTIFY_OK;
+}
+
+static int tegra124_emc_probe(struct platform_device *pdev)
+{
+	struct tegra124_emc *emc;
+
+	emc_pdev = pdev;
+
+	emc = devm_kzalloc(&pdev->dev, sizeof(*emc), GFP_KERNEL);
+	if (emc == NULL) {
+		dev_err(&pdev->dev, "Failed to allocate private memory\n");
+		return -ENOMEM;
+	}
+
+	emc->clk = clk_get(&pdev->dev, "emc");
+	if (IS_ERR(emc->clk)) {
+		devm_kfree(&pdev->dev, emc);
+		dev_err(&pdev->dev, "Can not find EMC clock\n");
+		return -EINVAL;
+	}
+
+	platform_set_drvdata(emc_pdev, emc);
+
+	emc->memory_bw_notifier.notifier_call = memory_bw_notify;
+	pm_qos_add_notifier(PM_QOS_MEMORY_BANDWIDTH, &emc->memory_bw_notifier);
+
+	return 0;
+}
+
+static struct platform_driver tegra124_emc_driver = {
+	.driver         = {
+		.name   = DRV_NAME,
+		.owner  = THIS_MODULE,
+	},
+	.probe          = tegra124_emc_probe,
+};
+
+module_platform_driver(tegra124_emc_driver);
+
+MODULE_AUTHOR("Tomeu Vizoso <tomeu.vizoso@collabora.com>");
+MODULE_DESCRIPTION("Tegra124 EMC driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
1.9.3

Re: [PATCH v3 3/3] memory: tegra124-emc: Add EMC driver

[Pavel Machek]

On Tue 2014-08-19 10:38:43, Tomeu Vizoso wrote:
> Sets the EMC clock rate based on the bandwidth requirements registered by
> memory clients through the PM_QOS_MEMORY_BANDWIDTH class.
> 
> Note: this is just an example and not a proper driver for a external memory
> controller. Its only purpose is to illustrate how such a driver
> would set the

So... you don't want this to be merged?

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH v3 3/3] memory: tegra124-emc: Add EMC driver

[Tomeu Vizoso]

On 19 August 2014 11:03, Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org> wrote:
> On Tue 2014-08-19 10:38:43, Tomeu Vizoso wrote:
>> Sets the EMC clock rate based on the bandwidth requirements registered by
>> memory clients through the PM_QOS_MEMORY_BANDWIDTH class.
>>
>> Note: this is just an example and not a proper driver for a external memory
>> controller. Its only purpose is to illustrate how such a driver
>> would set the
>
> So... you don't want this to be merged?

Not really, there's quite some work left to do until we have a proper
EMC driver for Tegra124.

Thanks,

Tomeu

Re: [PATCH v3 3/3] memory: tegra124-emc: Add EMC driver

[Stephen Warren]

On 08/19/2014 03:06 AM, Tomeu Vizoso wrote:
> On 19 August 2014 11:03, Pavel Machek <pavel@ucw.cz> wrote:
>> On Tue 2014-08-19 10:38:43, Tomeu Vizoso wrote:
>>> Sets the EMC clock rate based on the bandwidth requirements registered by
>>> memory clients through the PM_QOS_MEMORY_BANDWIDTH class.
>>>
>>> Note: this is just an example and not a proper driver for a external memory
>>> controller. Its only purpose is to illustrate how such a driver
>>> would set the
>>
>> So... you don't want this to be merged?
>
> Not really, there's quite some work left to do until we have a proper
> EMC driver for Tegra124.

Given that, the only patches in this series that should be merged touch 
(a) pm_qos, (b) tegradrm. As such, I would suggest that patch 1 be 
merged into the pm_qos tree in a standalone topic branch, and Thierry 
can merge this into the Tegra tree and apply patch 2 when he's ready.

Since patch 3 is the only patch that would normally go through the Tegra 
tree, and patch 3 isn't to be applied yet, I don't intend on applying 
any of these patches to the Tegra tree.