linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add OSM L3 Interconnect Provider
@ 2019-08-07 11:24 Sibi Sankar
  2019-08-07 11:24 ` [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings Sibi Sankar
  2019-08-07 11:24 ` [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support Sibi Sankar
  0 siblings, 2 replies; 11+ messages in thread
From: Sibi Sankar @ 2019-08-07 11:24 UTC (permalink / raw)
  To: robh+dt, georgi.djakov
  Cc: bjorn.andersson, agross, linux-kernel, devicetree, linux-arm-msm,
	mark.rutland, evgreen, daidavid1, saravanak, Sibi Sankar

This patch series aims to add Operating State Manager (OSM) L3
interconnect provider support on SDM845 SoCs to handle bandwidth
requests from CPU/GPU to scale L3 caches.

Sibi Sankar (2):
  dt-bindings: interconnect: Add OSM L3 DT bindings
  interconnect: qcom: Add OSM L3 interconnect provider support

 .../bindings/interconnect/qcom,osm-l3.yaml    |  55 ++++
 drivers/interconnect/qcom/Kconfig             |   7 +
 drivers/interconnect/qcom/Makefile            |   2 +
 drivers/interconnect/qcom/osm-l3.c            | 292 ++++++++++++++++++
 .../dt-bindings/interconnect/qcom,osm-l3.h    |  13 +
 5 files changed, 369 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
 create mode 100644 drivers/interconnect/qcom/osm-l3.c
 create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings
  2019-08-07 11:24 [PATCH 0/2] Add OSM L3 Interconnect Provider Sibi Sankar
@ 2019-08-07 11:24 ` Sibi Sankar
  2019-08-14 11:34   ` Sibi Sankar
  2019-08-07 11:24 ` [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support Sibi Sankar
  1 sibling, 1 reply; 11+ messages in thread
From: Sibi Sankar @ 2019-08-07 11:24 UTC (permalink / raw)
  To: robh+dt, georgi.djakov
  Cc: bjorn.andersson, agross, linux-kernel, devicetree, linux-arm-msm,
	mark.rutland, evgreen, daidavid1, saravanak, Sibi Sankar

Add bindings for Operating State Manager (OSM) L3 interconnect provider
on SDM845 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 .../bindings/interconnect/qcom,osm-l3.yaml    | 55 +++++++++++++++++++
 .../dt-bindings/interconnect/qcom,osm-l3.h    | 13 +++++
 2 files changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
 create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h

diff --git a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
new file mode 100644
index 0000000000000..51a4722e1a69f
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/qcom,osm-l3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Operating State Manager (OSM) L3 Interconnect Provider
+
+maintainers:
+  - Sibi Sankar <sibis@codeaurora.org>
+
+description:
+  L3 cache bandwidth requirements on Qualcomm SoCs is serviced by the OSM.
+  The OSM L3 interconnect provider aggregates the L3 bandwidth requests
+  from CPU/GPU and relays it to the OSM.
+
+properties:
+  compatible:
+    const: "qcom,sdm845-osm-l3"
+
+  reg:
+    maxItems: 1
+    description: OSM L3 registers
+
+  clocks:
+    items:
+      - description: xo clock
+      - description: alternate clock
+
+  clock-names:
+    items:
+      - const: xo
+      - const: alternate
+
+  '#interconnect-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clocks-names
+  - '#interconnect-cells'
+
+examples:
+  - |
+    osm_l3: interconnect@17d41000 {
+      compatible = "qcom,sdm845-osm-l3";
+      reg = <0 0x17d41000 0 0x1400>;
+
+      clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;
+      clock-names = "xo", "alternate";
+
+      #interconnect-cells = <1>;
+    };
diff --git a/include/dt-bindings/interconnect/qcom,osm-l3.h b/include/dt-bindings/interconnect/qcom,osm-l3.h
new file mode 100644
index 0000000000000..6662134c84248
--- /dev/null
+++ b/include/dt-bindings/interconnect/qcom,osm-l3.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 The Linux Foundation. All rights reserved.
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
+#define __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
+
+#define MASTER_OSM_L3_APPS	0
+#define MASTER_OSM_L3_GPU	1
+#define SLAVE_OSM_L3		2
+
+#endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-07 11:24 [PATCH 0/2] Add OSM L3 Interconnect Provider Sibi Sankar
  2019-08-07 11:24 ` [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings Sibi Sankar
@ 2019-08-07 11:24 ` Sibi Sankar
  2019-08-07 21:21   ` Saravana Kannan
  2019-08-13 20:32   ` Evan Green
  1 sibling, 2 replies; 11+ messages in thread
From: Sibi Sankar @ 2019-08-07 11:24 UTC (permalink / raw)
  To: robh+dt, georgi.djakov
  Cc: bjorn.andersson, agross, linux-kernel, devicetree, linux-arm-msm,
	mark.rutland, evgreen, daidavid1, saravanak, Sibi Sankar

On some Qualcomm SoCs, Operating State Manager (OSM) controls the
resources of scaling L3 caches. Add a driver to handle bandwidth
requests to OSM L3 from CPU/GPU.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/interconnect/qcom/Kconfig  |   7 +
 drivers/interconnect/qcom/Makefile |   2 +
 drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
 3 files changed, 301 insertions(+)
 create mode 100644 drivers/interconnect/qcom/osm-l3.c

diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
index d5e70ebc24108..f6c2a11a1a2c9 100644
--- a/drivers/interconnect/qcom/Kconfig
+++ b/drivers/interconnect/qcom/Kconfig
@@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
 	help
 	  Support for Qualcomm's Network-on-Chip interconnect hardware.
 
+config INTERCONNECT_QCOM_OSM_L3
+	tristate "Qualcomm OSM L3 interconnect driver"
+	depends on INTERCONNECT_QCOM || COMPILE_TEST
+	help
+	  Say y here to support the Operating State Manager (OSM) interconnect
+	  driver which controls the scaling of L3 caches on Qualcomm SoCs.
+
 config INTERCONNECT_QCOM_SDM845
 	tristate "Qualcomm SDM845 interconnect driver"
 	depends on INTERCONNECT_QCOM
diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
index 1c1cea690f922..9078af5fed109 100644
--- a/drivers/interconnect/qcom/Makefile
+++ b/drivers/interconnect/qcom/Makefile
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
+icc-osm-l3-objs				:= osm-l3.o
 qnoc-sdm845-objs			:= sdm845.o
 
+obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
 obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
new file mode 100644
index 0000000000000..1e7dfce6f4f9b
--- /dev/null
+++ b/drivers/interconnect/qcom/osm-l3.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019, The Linux Foundation. All rights reserved.
+ *
+ */
+
+#include <dt-bindings/interconnect/qcom,osm-l3.h>
+#include <dt-bindings/interconnect/qcom,sdm845.h>
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/interconnect-provider.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+
+#define LUT_MAX_ENTRIES			40U
+#define LUT_SRC				GENMASK(31, 30)
+#define LUT_L_VAL			GENMASK(7, 0)
+#define LUT_ROW_SIZE			32
+#define CLK_HW_DIV			2
+
+/* Register offsets */
+#define REG_ENABLE			0x0
+#define REG_FREQ_LUT			0x110
+#define REG_PERF_STATE			0x920
+
+#define OSM_L3_MAX_LINKS		1
+
+#define to_qcom_provider(_provider) \
+	container_of(_provider, struct qcom_icc_provider, provider)
+
+enum {
+	SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
+	SDM845_MASTER_OSM_L3_GPU,
+	SDM845_SLAVE_OSM_L3,
+};
+
+struct qcom_icc_provider {
+	void __iomem *base;
+	unsigned int max_state;
+	unsigned long lut_tables[LUT_MAX_ENTRIES];
+	struct icc_provider provider;
+};
+
+/**
+ * struct qcom_icc_node - Qualcomm specific interconnect nodes
+ * @name: the node name used in debugfs
+ * @links: an array of nodes where we can go next while traversing
+ * @id: a unique node identifier
+ * @num_links: the total number of @links
+ * @buswidth: width of the interconnect between a node and the bus
+ */
+struct qcom_icc_node {
+	const char *name;
+	u16 links[OSM_L3_MAX_LINKS];
+	u16 id;
+	u16 num_links;
+	u16 buswidth;
+};
+
+struct qcom_icc_desc {
+	struct qcom_icc_node **nodes;
+	size_t num_nodes;
+};
+
+#define DEFINE_QNODE(_name, _id, _buswidth, ...)			\
+		static struct qcom_icc_node _name = {			\
+		.name = #_name,						\
+		.id = _id,						\
+		.buswidth = _buswidth,					\
+		.num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),	\
+		.links = { __VA_ARGS__ },				\
+	}
+
+DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
+DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
+DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
+
+static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
+	[MASTER_OSM_L3_APPS] = &osm_apps_l3,
+	[MASTER_OSM_L3_GPU] = &osm_gpu_l3,
+	[SLAVE_OSM_L3] = &osm_l3,
+};
+
+static struct qcom_icc_desc sdm845_osm_l3 = {
+	.nodes = sdm845_osm_l3_nodes,
+	.num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
+};
+
+static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
+			      u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
+{
+	*agg_avg += avg_bw;
+	*agg_peak = max_t(u32, *agg_peak, peak_bw);
+
+	return 0;
+}
+
+static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
+{
+	struct icc_provider *provider;
+	struct qcom_icc_provider *qp;
+	struct qcom_icc_node *qn;
+	struct icc_node *n;
+	unsigned int index;
+	u32 agg_peak = 0;
+	u32 agg_avg = 0;
+	u64 rate;
+
+	qn = src->data;
+	provider = src->provider;
+	qp = to_qcom_provider(provider);
+
+	list_for_each_entry(n, &provider->nodes, node_list)
+		qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
+				   &agg_avg, &agg_peak);
+
+	rate = max(agg_avg, agg_peak);
+	rate = icc_units_to_bps(rate);
+	do_div(rate, qn->buswidth);
+
+	for (index = 0; index < qp->max_state; index++) {
+		if (qp->lut_tables[index] >= rate)
+			break;
+	}
+
+	writel_relaxed(index, qp->base + REG_PERF_STATE);
+
+	return 0;
+}
+
+static int qcom_osm_l3_probe(struct platform_device *pdev)
+{
+	u32 info, src, lval, i, prev_freq = 0, freq;
+	static unsigned long hw_rate, xo_rate;
+	const struct qcom_icc_desc *desc;
+	struct icc_onecell_data *data;
+	struct icc_provider *provider;
+	struct qcom_icc_node **qnodes;
+	struct qcom_icc_provider *qp;
+	struct icc_node *node;
+	size_t num_nodes;
+	struct clk *clk;
+	int ret;
+
+	clk = clk_get(&pdev->dev, "xo");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	xo_rate = clk_get_rate(clk);
+	clk_put(clk);
+
+	clk = clk_get(&pdev->dev, "alternate");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
+	clk_put(clk);
+
+	qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
+	if (!qp)
+		return -ENOMEM;
+
+	qp->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(qp->base))
+		return PTR_ERR(qp->base);
+
+	/* HW should be in enabled state to proceed */
+	if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
+		dev_err(&pdev->dev, "error hardware not enabled\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
+		info = readl_relaxed(qp->base + REG_FREQ_LUT +
+				     i * LUT_ROW_SIZE);
+		src = FIELD_GET(LUT_SRC, info);
+		lval = FIELD_GET(LUT_L_VAL, info);
+		if (src)
+			freq = xo_rate * lval;
+		else
+			freq = hw_rate;
+
+		/*
+		 * Two of the same frequencies with the same core counts means
+		 * end of table
+		 */
+		if (i > 0 && prev_freq == freq)
+			break;
+
+		qp->lut_tables[i] = freq;
+		prev_freq = freq;
+	}
+	qp->max_state = i;
+
+	desc = of_device_get_match_data(&pdev->dev);
+	if (!desc)
+		return -EINVAL;
+
+	qnodes = desc->nodes;
+	num_nodes = desc->num_nodes;
+
+	data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	provider = &qp->provider;
+	provider->dev = &pdev->dev;
+	provider->set = qcom_icc_set;
+	provider->aggregate = qcom_icc_aggregate;
+	provider->xlate = of_icc_xlate_onecell;
+	INIT_LIST_HEAD(&provider->nodes);
+	provider->data = data;
+
+	ret = icc_provider_add(provider);
+	if (ret) {
+		dev_err(&pdev->dev, "error adding interconnect provider\n");
+		return ret;
+	}
+
+	for (i = 0; i < num_nodes; i++) {
+		size_t j;
+
+		node = icc_node_create(qnodes[i]->id);
+		if (IS_ERR(node)) {
+			ret = PTR_ERR(node);
+			goto err;
+		}
+
+		node->name = qnodes[i]->name;
+		node->data = qnodes[i];
+		icc_node_add(node, provider);
+
+		dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
+			qnodes[i]->name, node->id);
+
+		/* populate links */
+		for (j = 0; j < qnodes[i]->num_links; j++)
+			icc_link_create(node, qnodes[i]->links[j]);
+
+		data->nodes[i] = node;
+	}
+	data->num_nodes = num_nodes;
+
+	platform_set_drvdata(pdev, qp);
+
+	return ret;
+err:
+	list_for_each_entry(node, &provider->nodes, node_list) {
+		icc_node_del(node);
+		icc_node_destroy(node->id);
+	}
+
+	icc_provider_del(provider);
+	return ret;
+}
+
+static int qcom_osm_l3_remove(struct platform_device *pdev)
+{
+	struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
+	struct icc_provider *provider = &qp->provider;
+	struct icc_node *n;
+
+	list_for_each_entry(n, &provider->nodes, node_list) {
+		icc_node_del(n);
+		icc_node_destroy(n->id);
+	}
+
+	return icc_provider_del(provider);
+}
+
+static const struct of_device_id osm_l3_of_match[] = {
+	{ .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, osm_l3_of_match);
+
+static struct platform_driver osm_l3_driver = {
+	.probe = qcom_osm_l3_probe,
+	.remove = qcom_osm_l3_remove,
+	.driver = {
+		.name = "osm-l3",
+		.of_match_table = osm_l3_of_match,
+	},
+};
+module_platform_driver(osm_l3_driver);
+
+MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
+MODULE_LICENSE("GPL v2");
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-07 11:24 ` [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support Sibi Sankar
@ 2019-08-07 21:21   ` Saravana Kannan
  2019-08-08 17:36     ` Sibi Sankar
  2019-08-13 20:32   ` Evan Green
  1 sibling, 1 reply; 11+ messages in thread
From: Saravana Kannan @ 2019-08-07 21:21 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, Evan Green, David Dai

On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> resources of scaling L3 caches. Add a driver to handle bandwidth
> requests to OSM L3 from CPU/GPU.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/interconnect/qcom/Kconfig  |   7 +
>  drivers/interconnect/qcom/Makefile |   2 +
>  drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
>  3 files changed, 301 insertions(+)
>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index d5e70ebc24108..f6c2a11a1a2c9 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
>         help
>           Support for Qualcomm's Network-on-Chip interconnect hardware.
>
> +config INTERCONNECT_QCOM_OSM_L3
> +       tristate "Qualcomm OSM L3 interconnect driver"
> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> +       help
> +         Say y here to support the Operating State Manager (OSM) interconnect
> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> +
>  config INTERCONNECT_QCOM_SDM845
>         tristate "Qualcomm SDM845 interconnect driver"
>         depends on INTERCONNECT_QCOM
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 1c1cea690f922..9078af5fed109 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -1,5 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> +icc-osm-l3-objs                                := osm-l3.o
>  qnoc-sdm845-objs                       := sdm845.o
>
> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> new file mode 100644
> index 0000000000000..1e7dfce6f4f9b
> --- /dev/null
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -0,0 +1,292 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + *
> + */
> +
> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> +#include <dt-bindings/interconnect/qcom,sdm845.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define LUT_MAX_ENTRIES                        40U
> +#define LUT_SRC                                GENMASK(31, 30)
> +#define LUT_L_VAL                      GENMASK(7, 0)
> +#define LUT_ROW_SIZE                   32
> +#define CLK_HW_DIV                     2
> +
> +/* Register offsets */
> +#define REG_ENABLE                     0x0
> +#define REG_FREQ_LUT                   0x110
> +#define REG_PERF_STATE                 0x920
> +
> +#define OSM_L3_MAX_LINKS               1
> +
> +#define to_qcom_provider(_provider) \
> +       container_of(_provider, struct qcom_icc_provider, provider)
> +
> +enum {
> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
> +       SDM845_MASTER_OSM_L3_GPU,
> +       SDM845_SLAVE_OSM_L3,
> +};
> +
> +struct qcom_icc_provider {
> +       void __iomem *base;
> +       unsigned int max_state;
> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
> +       struct icc_provider provider;
> +};
> +
> +/**
> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> + * @name: the node name used in debugfs
> + * @links: an array of nodes where we can go next while traversing
> + * @id: a unique node identifier
> + * @num_links: the total number of @links
> + * @buswidth: width of the interconnect between a node and the bus
> + */
> +struct qcom_icc_node {
> +       const char *name;
> +       u16 links[OSM_L3_MAX_LINKS];
> +       u16 id;
> +       u16 num_links;
> +       u16 buswidth;
> +};
> +
> +struct qcom_icc_desc {
> +       struct qcom_icc_node **nodes;
> +       size_t num_nodes;
> +};
> +
> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
> +               static struct qcom_icc_node _name = {                   \
> +               .name = #_name,                                         \
> +               .id = _id,                                              \
> +               .buswidth = _buswidth,                                  \
> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
> +               .links = { __VA_ARGS__ },                               \
> +       }
> +
> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
> +
> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
> +       [SLAVE_OSM_L3] = &osm_l3,
> +};
> +
> +static struct qcom_icc_desc sdm845_osm_l3 = {
> +       .nodes = sdm845_osm_l3_nodes,
> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> +};
> +
> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
> +       *agg_avg += avg_bw;
> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
> +
> +       return 0;
> +}
> +
> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +       struct icc_provider *provider;
> +       struct qcom_icc_provider *qp;
> +       struct qcom_icc_node *qn;
> +       struct icc_node *n;
> +       unsigned int index;
> +       u32 agg_peak = 0;
> +       u32 agg_avg = 0;
> +       u64 rate;
> +
> +       qn = src->data;
> +       provider = src->provider;
> +       qp = to_qcom_provider(provider);
> +
> +       list_for_each_entry(n, &provider->nodes, node_list)
> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
> +                                  &agg_avg, &agg_peak);
> +
> +       rate = max(agg_avg, agg_peak);
> +       rate = icc_units_to_bps(rate);
> +       do_div(rate, qn->buswidth);
> +
> +       for (index = 0; index < qp->max_state; index++) {
> +               if (qp->lut_tables[index] >= rate)
> +                       break;
> +       }
> +
> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
> +
> +       return 0;
> +}
> +
> +static int qcom_osm_l3_probe(struct platform_device *pdev)
> +{
> +       u32 info, src, lval, i, prev_freq = 0, freq;
> +       static unsigned long hw_rate, xo_rate;
> +       const struct qcom_icc_desc *desc;
> +       struct icc_onecell_data *data;
> +       struct icc_provider *provider;
> +       struct qcom_icc_node **qnodes;
> +       struct qcom_icc_provider *qp;
> +       struct icc_node *node;
> +       size_t num_nodes;
> +       struct clk *clk;
> +       int ret;
> +
> +       clk = clk_get(&pdev->dev, "xo");
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       xo_rate = clk_get_rate(clk);
> +       clk_put(clk);
> +
> +       clk = clk_get(&pdev->dev, "alternate");
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +       clk_put(clk);
> +
> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> +       if (!qp)
> +               return -ENOMEM;
> +
> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(qp->base))
> +               return PTR_ERR(qp->base);
> +
> +       /* HW should be in enabled state to proceed */
> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
> +               dev_err(&pdev->dev, "error hardware not enabled\n");
> +               return -ENODEV;
> +       }
> +
> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
> +                                    i * LUT_ROW_SIZE);
> +               src = FIELD_GET(LUT_SRC, info);
> +               lval = FIELD_GET(LUT_L_VAL, info);
> +               if (src)
> +                       freq = xo_rate * lval;
> +               else
> +                       freq = hw_rate;
> +
> +               /*
> +                * Two of the same frequencies with the same core counts means
> +                * end of table
> +                */
> +               if (i > 0 && prev_freq == freq)
> +                       break;
> +
> +               qp->lut_tables[i] = freq;
> +               prev_freq = freq;
> +       }
> +       qp->max_state = i;
> +
> +       desc = of_device_get_match_data(&pdev->dev);
> +       if (!desc)
> +               return -EINVAL;
> +
> +       qnodes = desc->nodes;
> +       num_nodes = desc->num_nodes;
> +
> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       provider = &qp->provider;
> +       provider->dev = &pdev->dev;
> +       provider->set = qcom_icc_set;
> +       provider->aggregate = qcom_icc_aggregate;
> +       provider->xlate = of_icc_xlate_onecell;
> +       INIT_LIST_HEAD(&provider->nodes);
> +       provider->data = data;
> +
> +       ret = icc_provider_add(provider);
> +       if (ret) {
> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < num_nodes; i++) {
> +               size_t j;
> +
> +               node = icc_node_create(qnodes[i]->id);
> +               if (IS_ERR(node)) {
> +                       ret = PTR_ERR(node);
> +                       goto err;
> +               }
> +
> +               node->name = qnodes[i]->name;
> +               node->data = qnodes[i];
> +               icc_node_add(node, provider);
> +
> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> +                       qnodes[i]->name, node->id);
> +
> +               /* populate links */
> +               for (j = 0; j < qnodes[i]->num_links; j++)
> +                       icc_link_create(node, qnodes[i]->links[j]);
> +
> +               data->nodes[i] = node;
> +       }
> +       data->num_nodes = num_nodes;
> +
> +       platform_set_drvdata(pdev, qp);
> +
> +       return ret;
> +err:
> +       list_for_each_entry(node, &provider->nodes, node_list) {
> +               icc_node_del(node);
> +               icc_node_destroy(node->id);
> +       }
> +
> +       icc_provider_del(provider);
> +       return ret;
> +}
> +
> +static int qcom_osm_l3_remove(struct platform_device *pdev)
> +{
> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
> +       struct icc_provider *provider = &qp->provider;
> +       struct icc_node *n;
> +
> +       list_for_each_entry(n, &provider->nodes, node_list) {
> +               icc_node_del(n);
> +               icc_node_destroy(n->id);
> +       }
> +
> +       return icc_provider_del(provider);
> +}
> +
> +static const struct of_device_id osm_l3_of_match[] = {
> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
> +
> +static struct platform_driver osm_l3_driver = {
> +       .probe = qcom_osm_l3_probe,
> +       .remove = qcom_osm_l3_remove,
> +       .driver = {
> +               .name = "osm-l3",
> +               .of_match_table = osm_l3_of_match,
> +       },
> +};
> +module_platform_driver(osm_l3_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
> +MODULE_LICENSE("GPL v2");

Did a quick scan of the code and it's not clear how you connect the L3
interconnect provider to the rest of the interconnect. I don't see any
DT or code references to the rest of the interconnects in the system.
If GPU is making a bandwidth request all the way to L3, how do you
make sure the interconnects between GPU and L3 also scale up properly.

-Saravana

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-07 21:21   ` Saravana Kannan
@ 2019-08-08 17:36     ` Sibi Sankar
  2019-08-14  0:43       ` Saravana Kannan
  0 siblings, 1 reply; 11+ messages in thread
From: Sibi Sankar @ 2019-08-08 17:36 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, Evan Green, David Dai

Hey Saravana,

Thanks for the review!

On 8/8/19 2:51 AM, Saravana Kannan wrote:
> On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>>
>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>> resources of scaling L3 caches. Add a driver to handle bandwidth
>> requests to OSM L3 from CPU/GPU.
>>
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>   drivers/interconnect/qcom/Kconfig  |   7 +
>>   drivers/interconnect/qcom/Makefile |   2 +
>>   drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
>>   3 files changed, 301 insertions(+)
>>   create mode 100644 drivers/interconnect/qcom/osm-l3.c
>>
>> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
>> index d5e70ebc24108..f6c2a11a1a2c9 100644
>> --- a/drivers/interconnect/qcom/Kconfig
>> +++ b/drivers/interconnect/qcom/Kconfig
>> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
>>          help
>>            Support for Qualcomm's Network-on-Chip interconnect hardware.
>>
>> +config INTERCONNECT_QCOM_OSM_L3
>> +       tristate "Qualcomm OSM L3 interconnect driver"
>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>> +       help
>> +         Say y here to support the Operating State Manager (OSM) interconnect
>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
>> +
>>   config INTERCONNECT_QCOM_SDM845
>>          tristate "Qualcomm SDM845 interconnect driver"
>>          depends on INTERCONNECT_QCOM
>> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
>> index 1c1cea690f922..9078af5fed109 100644
>> --- a/drivers/interconnect/qcom/Makefile
>> +++ b/drivers/interconnect/qcom/Makefile
>> @@ -1,5 +1,7 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>
>> +icc-osm-l3-objs                                := osm-l3.o
>>   qnoc-sdm845-objs                       := sdm845.o
>>
>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>>   obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
>> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
>> new file mode 100644
>> index 0000000000000..1e7dfce6f4f9b
>> --- /dev/null
>> +++ b/drivers/interconnect/qcom/osm-l3.c
>> @@ -0,0 +1,292 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> + *
>> + */
>> +
>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>> +#include <dt-bindings/interconnect/qcom,sdm845.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/interconnect-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define LUT_MAX_ENTRIES                        40U
>> +#define LUT_SRC                                GENMASK(31, 30)
>> +#define LUT_L_VAL                      GENMASK(7, 0)
>> +#define LUT_ROW_SIZE                   32
>> +#define CLK_HW_DIV                     2
>> +
>> +/* Register offsets */
>> +#define REG_ENABLE                     0x0
>> +#define REG_FREQ_LUT                   0x110
>> +#define REG_PERF_STATE                 0x920
>> +
>> +#define OSM_L3_MAX_LINKS               1
>> +
>> +#define to_qcom_provider(_provider) \
>> +       container_of(_provider, struct qcom_icc_provider, provider)
>> +
>> +enum {
>> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
>> +       SDM845_MASTER_OSM_L3_GPU,
>> +       SDM845_SLAVE_OSM_L3,
>> +};
>> +
>> +struct qcom_icc_provider {
>> +       void __iomem *base;
>> +       unsigned int max_state;
>> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
>> +       struct icc_provider provider;
>> +};
>> +
>> +/**
>> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
>> + * @name: the node name used in debugfs
>> + * @links: an array of nodes where we can go next while traversing
>> + * @id: a unique node identifier
>> + * @num_links: the total number of @links
>> + * @buswidth: width of the interconnect between a node and the bus
>> + */
>> +struct qcom_icc_node {
>> +       const char *name;
>> +       u16 links[OSM_L3_MAX_LINKS];
>> +       u16 id;
>> +       u16 num_links;
>> +       u16 buswidth;
>> +};
>> +
>> +struct qcom_icc_desc {
>> +       struct qcom_icc_node **nodes;
>> +       size_t num_nodes;
>> +};
>> +
>> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
>> +               static struct qcom_icc_node _name = {                   \
>> +               .name = #_name,                                         \
>> +               .id = _id,                                              \
>> +               .buswidth = _buswidth,                                  \
>> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
>> +               .links = { __VA_ARGS__ },                               \
>> +       }
>> +
>> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
>> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
>> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
>> +
>> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
>> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
>> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
>> +       [SLAVE_OSM_L3] = &osm_l3,
>> +};
>> +
>> +static struct qcom_icc_desc sdm845_osm_l3 = {
>> +       .nodes = sdm845_osm_l3_nodes,
>> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
>> +};
>> +
>> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
>> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
>> +{
>> +       *agg_avg += avg_bw;
>> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> +       struct icc_provider *provider;
>> +       struct qcom_icc_provider *qp;
>> +       struct qcom_icc_node *qn;
>> +       struct icc_node *n;
>> +       unsigned int index;
>> +       u32 agg_peak = 0;
>> +       u32 agg_avg = 0;
>> +       u64 rate;
>> +
>> +       qn = src->data;
>> +       provider = src->provider;
>> +       qp = to_qcom_provider(provider);
>> +
>> +       list_for_each_entry(n, &provider->nodes, node_list)
>> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
>> +                                  &agg_avg, &agg_peak);
>> +
>> +       rate = max(agg_avg, agg_peak);
>> +       rate = icc_units_to_bps(rate);
>> +       do_div(rate, qn->buswidth);
>> +
>> +       for (index = 0; index < qp->max_state; index++) {
>> +               if (qp->lut_tables[index] >= rate)
>> +                       break;
>> +       }
>> +
>> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_osm_l3_probe(struct platform_device *pdev)
>> +{
>> +       u32 info, src, lval, i, prev_freq = 0, freq;
>> +       static unsigned long hw_rate, xo_rate;
>> +       const struct qcom_icc_desc *desc;
>> +       struct icc_onecell_data *data;
>> +       struct icc_provider *provider;
>> +       struct qcom_icc_node **qnodes;
>> +       struct qcom_icc_provider *qp;
>> +       struct icc_node *node;
>> +       size_t num_nodes;
>> +       struct clk *clk;
>> +       int ret;
>> +
>> +       clk = clk_get(&pdev->dev, "xo");
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       xo_rate = clk_get_rate(clk);
>> +       clk_put(clk);
>> +
>> +       clk = clk_get(&pdev->dev, "alternate");
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
>> +       clk_put(clk);
>> +
>> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
>> +       if (!qp)
>> +               return -ENOMEM;
>> +
>> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
>> +       if (IS_ERR(qp->base))
>> +               return PTR_ERR(qp->base);
>> +
>> +       /* HW should be in enabled state to proceed */
>> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
>> +               dev_err(&pdev->dev, "error hardware not enabled\n");
>> +               return -ENODEV;
>> +       }
>> +
>> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
>> +                                    i * LUT_ROW_SIZE);
>> +               src = FIELD_GET(LUT_SRC, info);
>> +               lval = FIELD_GET(LUT_L_VAL, info);
>> +               if (src)
>> +                       freq = xo_rate * lval;
>> +               else
>> +                       freq = hw_rate;
>> +
>> +               /*
>> +                * Two of the same frequencies with the same core counts means
>> +                * end of table
>> +                */
>> +               if (i > 0 && prev_freq == freq)
>> +                       break;
>> +
>> +               qp->lut_tables[i] = freq;
>> +               prev_freq = freq;
>> +       }
>> +       qp->max_state = i;
>> +
>> +       desc = of_device_get_match_data(&pdev->dev);
>> +       if (!desc)
>> +               return -EINVAL;
>> +
>> +       qnodes = desc->nodes;
>> +       num_nodes = desc->num_nodes;
>> +
>> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
>> +       if (!data)
>> +               return -ENOMEM;
>> +
>> +       provider = &qp->provider;
>> +       provider->dev = &pdev->dev;
>> +       provider->set = qcom_icc_set;
>> +       provider->aggregate = qcom_icc_aggregate;
>> +       provider->xlate = of_icc_xlate_onecell;
>> +       INIT_LIST_HEAD(&provider->nodes);
>> +       provider->data = data;
>> +
>> +       ret = icc_provider_add(provider);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
>> +               return ret;
>> +       }
>> +
>> +       for (i = 0; i < num_nodes; i++) {
>> +               size_t j;
>> +
>> +               node = icc_node_create(qnodes[i]->id);
>> +               if (IS_ERR(node)) {
>> +                       ret = PTR_ERR(node);
>> +                       goto err;
>> +               }
>> +
>> +               node->name = qnodes[i]->name;
>> +               node->data = qnodes[i];
>> +               icc_node_add(node, provider);
>> +
>> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
>> +                       qnodes[i]->name, node->id);
>> +
>> +               /* populate links */
>> +               for (j = 0; j < qnodes[i]->num_links; j++)
>> +                       icc_link_create(node, qnodes[i]->links[j]);
>> +
>> +               data->nodes[i] = node;
>> +       }
>> +       data->num_nodes = num_nodes;
>> +
>> +       platform_set_drvdata(pdev, qp);
>> +
>> +       return ret;
>> +err:
>> +       list_for_each_entry(node, &provider->nodes, node_list) {
>> +               icc_node_del(node);
>> +               icc_node_destroy(node->id);
>> +       }
>> +
>> +       icc_provider_del(provider);
>> +       return ret;
>> +}
>> +
>> +static int qcom_osm_l3_remove(struct platform_device *pdev)
>> +{
>> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>> +       struct icc_provider *provider = &qp->provider;
>> +       struct icc_node *n;
>> +
>> +       list_for_each_entry(n, &provider->nodes, node_list) {
>> +               icc_node_del(n);
>> +               icc_node_destroy(n->id);
>> +       }
>> +
>> +       return icc_provider_del(provider);
>> +}
>> +
>> +static const struct of_device_id osm_l3_of_match[] = {
>> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
>> +
>> +static struct platform_driver osm_l3_driver = {
>> +       .probe = qcom_osm_l3_probe,
>> +       .remove = qcom_osm_l3_remove,
>> +       .driver = {
>> +               .name = "osm-l3",
>> +               .of_match_table = osm_l3_of_match,
>> +       },
>> +};
>> +module_platform_driver(osm_l3_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
>> +MODULE_LICENSE("GPL v2");
> 
> Did a quick scan of the code and it's not clear how you connect the L3
> interconnect provider to the rest of the interconnect. I don't see any
> DT or code references to the rest of the interconnects in the system.
> If GPU is making a bandwidth request all the way to L3, how do you
> make sure the interconnects between GPU and L3 also scale up properly.

For SDM845 OSM L3 provider the icc nodes endpoints are isolated from
rsc icc node endpoints i.e GPU would need to vote on this path in
addition to voting for DDR. On future SoCs if the need to scale
interconnect between GPU rsc nodes along with the OSM l3 nodes arises,
it can be trivially extended by linking the osm icc nodes with global
icc node ids of the gpu rsc nodes.

> 
> -Saravana
> 

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-07 11:24 ` [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support Sibi Sankar
  2019-08-07 21:21   ` Saravana Kannan
@ 2019-08-13 20:32   ` Evan Green
  2019-08-14 11:27     ` Sibi Sankar
  1 sibling, 1 reply; 11+ messages in thread
From: Evan Green @ 2019-08-13 20:32 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, David Dai, Saravana Kannan

Hi Sibi,

On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> resources of scaling L3 caches. Add a driver to handle bandwidth
> requests to OSM L3 from CPU/GPU.
>
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>

This looks good to me, just a couple minor comments below.

> ---
>  drivers/interconnect/qcom/Kconfig  |   7 +
>  drivers/interconnect/qcom/Makefile |   2 +
>  drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
>  3 files changed, 301 insertions(+)
>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>
> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> index d5e70ebc24108..f6c2a11a1a2c9 100644
> --- a/drivers/interconnect/qcom/Kconfig
> +++ b/drivers/interconnect/qcom/Kconfig
> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
>         help
>           Support for Qualcomm's Network-on-Chip interconnect hardware.
>
> +config INTERCONNECT_QCOM_OSM_L3
> +       tristate "Qualcomm OSM L3 interconnect driver"
> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> +       help
> +         Say y here to support the Operating State Manager (OSM) interconnect
> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> +
>  config INTERCONNECT_QCOM_SDM845
>         tristate "Qualcomm SDM845 interconnect driver"
>         depends on INTERCONNECT_QCOM
> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> index 1c1cea690f922..9078af5fed109 100644
> --- a/drivers/interconnect/qcom/Makefile
> +++ b/drivers/interconnect/qcom/Makefile
> @@ -1,5 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> +icc-osm-l3-objs                                := osm-l3.o
>  qnoc-sdm845-objs                       := sdm845.o
>
> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>  obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> new file mode 100644
> index 0000000000000..1e7dfce6f4f9b
> --- /dev/null
> +++ b/drivers/interconnect/qcom/osm-l3.c
> @@ -0,0 +1,292 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> + *
> + */
> +
> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> +#include <dt-bindings/interconnect/qcom,sdm845.h>
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/interconnect-provider.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +#define LUT_MAX_ENTRIES                        40U
> +#define LUT_SRC                                GENMASK(31, 30)
> +#define LUT_L_VAL                      GENMASK(7, 0)
> +#define LUT_ROW_SIZE                   32
> +#define CLK_HW_DIV                     2
> +
> +/* Register offsets */
> +#define REG_ENABLE                     0x0
> +#define REG_FREQ_LUT                   0x110
> +#define REG_PERF_STATE                 0x920
> +
> +#define OSM_L3_MAX_LINKS               1
> +
> +#define to_qcom_provider(_provider) \
> +       container_of(_provider, struct qcom_icc_provider, provider)
> +
> +enum {
> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
> +       SDM845_MASTER_OSM_L3_GPU,
> +       SDM845_SLAVE_OSM_L3,
> +};
> +
> +struct qcom_icc_provider {

Maybe we should rename this and the ones below to avoid collisions
with drivers/interconnect/qcom/sdm845.c, even though those structures
are currently local.

> +       void __iomem *base;
> +       unsigned int max_state;
> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
> +       struct icc_provider provider;
> +};
> +
> +/**
> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> + * @name: the node name used in debugfs
> + * @links: an array of nodes where we can go next while traversing
> + * @id: a unique node identifier
> + * @num_links: the total number of @links
> + * @buswidth: width of the interconnect between a node and the bus
> + */
> +struct qcom_icc_node {
> +       const char *name;
> +       u16 links[OSM_L3_MAX_LINKS];
> +       u16 id;
> +       u16 num_links;
> +       u16 buswidth;
> +};
> +
> +struct qcom_icc_desc {
> +       struct qcom_icc_node **nodes;
> +       size_t num_nodes;
> +};
> +
> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
> +               static struct qcom_icc_node _name = {                   \
> +               .name = #_name,                                         \
> +               .id = _id,                                              \
> +               .buswidth = _buswidth,                                  \
> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
> +               .links = { __VA_ARGS__ },                               \
> +       }
> +
> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
> +
> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
> +       [SLAVE_OSM_L3] = &osm_l3,
> +};
> +
> +static struct qcom_icc_desc sdm845_osm_l3 = {
> +       .nodes = sdm845_osm_l3_nodes,
> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> +};
> +
> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> +{
> +       *agg_avg += avg_bw;
> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
> +
> +       return 0;
> +}
> +
> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> +{
> +       struct icc_provider *provider;
> +       struct qcom_icc_provider *qp;
> +       struct qcom_icc_node *qn;
> +       struct icc_node *n;
> +       unsigned int index;
> +       u32 agg_peak = 0;
> +       u32 agg_avg = 0;
> +       u64 rate;
> +
> +       qn = src->data;
> +       provider = src->provider;
> +       qp = to_qcom_provider(provider);
> +
> +       list_for_each_entry(n, &provider->nodes, node_list)
> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
> +                                  &agg_avg, &agg_peak);
> +
> +       rate = max(agg_avg, agg_peak);
> +       rate = icc_units_to_bps(rate);
> +       do_div(rate, qn->buswidth);
> +
> +       for (index = 0; index < qp->max_state; index++) {
> +               if (qp->lut_tables[index] >= rate)
> +                       break;
> +       }
> +
> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
> +
> +       return 0;
> +}
> +
> +static int qcom_osm_l3_probe(struct platform_device *pdev)
> +{
> +       u32 info, src, lval, i, prev_freq = 0, freq;
> +       static unsigned long hw_rate, xo_rate;
> +       const struct qcom_icc_desc *desc;
> +       struct icc_onecell_data *data;
> +       struct icc_provider *provider;
> +       struct qcom_icc_node **qnodes;
> +       struct qcom_icc_provider *qp;
> +       struct icc_node *node;
> +       size_t num_nodes;
> +       struct clk *clk;
> +       int ret;
> +
> +       clk = clk_get(&pdev->dev, "xo");
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       xo_rate = clk_get_rate(clk);
> +       clk_put(clk);
> +
> +       clk = clk_get(&pdev->dev, "alternate");
> +       if (IS_ERR(clk))
> +               return PTR_ERR(clk);
> +
> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> +       clk_put(clk);
> +
> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> +       if (!qp)
> +               return -ENOMEM;
> +
> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
> +       if (IS_ERR(qp->base))
> +               return PTR_ERR(qp->base);
> +
> +       /* HW should be in enabled state to proceed */
> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
> +               dev_err(&pdev->dev, "error hardware not enabled\n");
> +               return -ENODEV;
> +       }
> +
> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
> +                                    i * LUT_ROW_SIZE);
> +               src = FIELD_GET(LUT_SRC, info);
> +               lval = FIELD_GET(LUT_L_VAL, info);
> +               if (src)
> +                       freq = xo_rate * lval;
> +               else
> +                       freq = hw_rate;
> +
> +               /*
> +                * Two of the same frequencies with the same core counts means
> +                * end of table
> +                */
> +               if (i > 0 && prev_freq == freq)
> +                       break;
> +
> +               qp->lut_tables[i] = freq;
> +               prev_freq = freq;
> +       }
> +       qp->max_state = i;
> +
> +       desc = of_device_get_match_data(&pdev->dev);
> +       if (!desc)
> +               return -EINVAL;
> +
> +       qnodes = desc->nodes;
> +       num_nodes = desc->num_nodes;
> +
> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> +       if (!data)
> +               return -ENOMEM;
> +
> +       provider = &qp->provider;
> +       provider->dev = &pdev->dev;
> +       provider->set = qcom_icc_set;
> +       provider->aggregate = qcom_icc_aggregate;
> +       provider->xlate = of_icc_xlate_onecell;
> +       INIT_LIST_HEAD(&provider->nodes);
> +       provider->data = data;
> +
> +       ret = icc_provider_add(provider);
> +       if (ret) {
> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
> +               return ret;
> +       }
> +
> +       for (i = 0; i < num_nodes; i++) {
> +               size_t j;
> +
> +               node = icc_node_create(qnodes[i]->id);
> +               if (IS_ERR(node)) {
> +                       ret = PTR_ERR(node);
> +                       goto err;
> +               }
> +
> +               node->name = qnodes[i]->name;
> +               node->data = qnodes[i];
> +               icc_node_add(node, provider);
> +
> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> +                       qnodes[i]->name, node->id);
> +
> +               /* populate links */
> +               for (j = 0; j < qnodes[i]->num_links; j++)
> +                       icc_link_create(node, qnodes[i]->links[j]);
> +
> +               data->nodes[i] = node;
> +       }
> +       data->num_nodes = num_nodes;
> +
> +       platform_set_drvdata(pdev, qp);
> +
> +       return ret;
> +err:
> +       list_for_each_entry(node, &provider->nodes, node_list) {
> +               icc_node_del(node);
> +               icc_node_destroy(node->id);
> +       }
> +
> +       icc_provider_del(provider);

I looks like you could just call qcom_osm_l3_remove() here instead of the above.


> +       return ret;
> +}
> +
> +static int qcom_osm_l3_remove(struct platform_device *pdev)
> +{
> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
> +       struct icc_provider *provider = &qp->provider;
> +       struct icc_node *n;
> +
> +       list_for_each_entry(n, &provider->nodes, node_list) {
> +               icc_node_del(n);
> +               icc_node_destroy(n->id);
> +       }
> +
> +       return icc_provider_del(provider);
> +}
> +
> +static const struct of_device_id osm_l3_of_match[] = {
> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
> +
> +static struct platform_driver osm_l3_driver = {
> +       .probe = qcom_osm_l3_probe,
> +       .remove = qcom_osm_l3_remove,
> +       .driver = {
> +               .name = "osm-l3",
> +               .of_match_table = osm_l3_of_match,
> +       },
> +};
> +module_platform_driver(osm_l3_driver);
> +
> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
> +MODULE_LICENSE("GPL v2");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-08 17:36     ` Sibi Sankar
@ 2019-08-14  0:43       ` Saravana Kannan
  2019-08-14 11:30         ` Sibi Sankar
  0 siblings, 1 reply; 11+ messages in thread
From: Saravana Kannan @ 2019-08-14  0:43 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, Evan Green, David Dai

On Thu, Aug 8, 2019 at 10:37 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> Hey Saravana,
>
> Thanks for the review!
>
> On 8/8/19 2:51 AM, Saravana Kannan wrote:
> > On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
> >>
> >> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> >> resources of scaling L3 caches. Add a driver to handle bandwidth
> >> requests to OSM L3 from CPU/GPU.
> >>
> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >> ---
> >>   drivers/interconnect/qcom/Kconfig  |   7 +
> >>   drivers/interconnect/qcom/Makefile |   2 +
> >>   drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
> >>   3 files changed, 301 insertions(+)
> >>   create mode 100644 drivers/interconnect/qcom/osm-l3.c
> >>
> >> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> >> index d5e70ebc24108..f6c2a11a1a2c9 100644
> >> --- a/drivers/interconnect/qcom/Kconfig
> >> +++ b/drivers/interconnect/qcom/Kconfig
> >> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
> >>          help
> >>            Support for Qualcomm's Network-on-Chip interconnect hardware.
> >>
> >> +config INTERCONNECT_QCOM_OSM_L3
> >> +       tristate "Qualcomm OSM L3 interconnect driver"
> >> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> >> +       help
> >> +         Say y here to support the Operating State Manager (OSM) interconnect
> >> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> >> +
> >>   config INTERCONNECT_QCOM_SDM845
> >>          tristate "Qualcomm SDM845 interconnect driver"
> >>          depends on INTERCONNECT_QCOM
> >> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> >> index 1c1cea690f922..9078af5fed109 100644
> >> --- a/drivers/interconnect/qcom/Makefile
> >> +++ b/drivers/interconnect/qcom/Makefile
> >> @@ -1,5 +1,7 @@
> >>   # SPDX-License-Identifier: GPL-2.0
> >>
> >> +icc-osm-l3-objs                                := osm-l3.o
> >>   qnoc-sdm845-objs                       := sdm845.o
> >>
> >> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
> >>   obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
> >> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> >> new file mode 100644
> >> index 0000000000000..1e7dfce6f4f9b
> >> --- /dev/null
> >> +++ b/drivers/interconnect/qcom/osm-l3.c
> >> @@ -0,0 +1,292 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> >> + *
> >> + */
> >> +
> >> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> >> +#include <dt-bindings/interconnect/qcom,sdm845.h>
> >> +#include <linux/bitfield.h>
> >> +#include <linux/clk.h>
> >> +#include <linux/interconnect-provider.h>
> >> +#include <linux/io.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/of_device.h>
> >> +#include <linux/of_platform.h>
> >> +#include <linux/platform_device.h>
> >> +
> >> +#define LUT_MAX_ENTRIES                        40U
> >> +#define LUT_SRC                                GENMASK(31, 30)
> >> +#define LUT_L_VAL                      GENMASK(7, 0)
> >> +#define LUT_ROW_SIZE                   32
> >> +#define CLK_HW_DIV                     2
> >> +
> >> +/* Register offsets */
> >> +#define REG_ENABLE                     0x0
> >> +#define REG_FREQ_LUT                   0x110
> >> +#define REG_PERF_STATE                 0x920
> >> +
> >> +#define OSM_L3_MAX_LINKS               1
> >> +
> >> +#define to_qcom_provider(_provider) \
> >> +       container_of(_provider, struct qcom_icc_provider, provider)
> >> +
> >> +enum {
> >> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
> >> +       SDM845_MASTER_OSM_L3_GPU,
> >> +       SDM845_SLAVE_OSM_L3,
> >> +};
> >> +
> >> +struct qcom_icc_provider {
> >> +       void __iomem *base;
> >> +       unsigned int max_state;
> >> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
> >> +       struct icc_provider provider;
> >> +};
> >> +
> >> +/**
> >> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> >> + * @name: the node name used in debugfs
> >> + * @links: an array of nodes where we can go next while traversing
> >> + * @id: a unique node identifier
> >> + * @num_links: the total number of @links
> >> + * @buswidth: width of the interconnect between a node and the bus
> >> + */
> >> +struct qcom_icc_node {
> >> +       const char *name;
> >> +       u16 links[OSM_L3_MAX_LINKS];
> >> +       u16 id;
> >> +       u16 num_links;
> >> +       u16 buswidth;
> >> +};
> >> +
> >> +struct qcom_icc_desc {
> >> +       struct qcom_icc_node **nodes;
> >> +       size_t num_nodes;
> >> +};
> >> +
> >> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
> >> +               static struct qcom_icc_node _name = {                   \
> >> +               .name = #_name,                                         \
> >> +               .id = _id,                                              \
> >> +               .buswidth = _buswidth,                                  \
> >> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
> >> +               .links = { __VA_ARGS__ },                               \
> >> +       }
> >> +
> >> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> >> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
> >> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
> >> +
> >> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> >> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
> >> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
> >> +       [SLAVE_OSM_L3] = &osm_l3,
> >> +};
> >> +
> >> +static struct qcom_icc_desc sdm845_osm_l3 = {
> >> +       .nodes = sdm845_osm_l3_nodes,
> >> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> >> +};
> >> +
> >> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
> >> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> >> +{
> >> +       *agg_avg += avg_bw;
> >> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> >> +{
> >> +       struct icc_provider *provider;
> >> +       struct qcom_icc_provider *qp;
> >> +       struct qcom_icc_node *qn;
> >> +       struct icc_node *n;
> >> +       unsigned int index;
> >> +       u32 agg_peak = 0;
> >> +       u32 agg_avg = 0;
> >> +       u64 rate;
> >> +
> >> +       qn = src->data;
> >> +       provider = src->provider;
> >> +       qp = to_qcom_provider(provider);
> >> +
> >> +       list_for_each_entry(n, &provider->nodes, node_list)
> >> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
> >> +                                  &agg_avg, &agg_peak);
> >> +
> >> +       rate = max(agg_avg, agg_peak);
> >> +       rate = icc_units_to_bps(rate);
> >> +       do_div(rate, qn->buswidth);
> >> +
> >> +       for (index = 0; index < qp->max_state; index++) {
> >> +               if (qp->lut_tables[index] >= rate)
> >> +                       break;
> >> +       }
> >> +
> >> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int qcom_osm_l3_probe(struct platform_device *pdev)
> >> +{
> >> +       u32 info, src, lval, i, prev_freq = 0, freq;
> >> +       static unsigned long hw_rate, xo_rate;
> >> +       const struct qcom_icc_desc *desc;
> >> +       struct icc_onecell_data *data;
> >> +       struct icc_provider *provider;
> >> +       struct qcom_icc_node **qnodes;
> >> +       struct qcom_icc_provider *qp;
> >> +       struct icc_node *node;
> >> +       size_t num_nodes;
> >> +       struct clk *clk;
> >> +       int ret;
> >> +
> >> +       clk = clk_get(&pdev->dev, "xo");
> >> +       if (IS_ERR(clk))
> >> +               return PTR_ERR(clk);
> >> +
> >> +       xo_rate = clk_get_rate(clk);
> >> +       clk_put(clk);
> >> +
> >> +       clk = clk_get(&pdev->dev, "alternate");
> >> +       if (IS_ERR(clk))
> >> +               return PTR_ERR(clk);
> >> +
> >> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> >> +       clk_put(clk);
> >> +
> >> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> >> +       if (!qp)
> >> +               return -ENOMEM;
> >> +
> >> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
> >> +       if (IS_ERR(qp->base))
> >> +               return PTR_ERR(qp->base);
> >> +
> >> +       /* HW should be in enabled state to proceed */
> >> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
> >> +               dev_err(&pdev->dev, "error hardware not enabled\n");
> >> +               return -ENODEV;
> >> +       }
> >> +
> >> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> >> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
> >> +                                    i * LUT_ROW_SIZE);
> >> +               src = FIELD_GET(LUT_SRC, info);
> >> +               lval = FIELD_GET(LUT_L_VAL, info);
> >> +               if (src)
> >> +                       freq = xo_rate * lval;
> >> +               else
> >> +                       freq = hw_rate;
> >> +
> >> +               /*
> >> +                * Two of the same frequencies with the same core counts means
> >> +                * end of table
> >> +                */
> >> +               if (i > 0 && prev_freq == freq)
> >> +                       break;
> >> +
> >> +               qp->lut_tables[i] = freq;
> >> +               prev_freq = freq;
> >> +       }
> >> +       qp->max_state = i;
> >> +
> >> +       desc = of_device_get_match_data(&pdev->dev);
> >> +       if (!desc)
> >> +               return -EINVAL;
> >> +
> >> +       qnodes = desc->nodes;
> >> +       num_nodes = desc->num_nodes;
> >> +
> >> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> >> +       if (!data)
> >> +               return -ENOMEM;
> >> +
> >> +       provider = &qp->provider;
> >> +       provider->dev = &pdev->dev;
> >> +       provider->set = qcom_icc_set;
> >> +       provider->aggregate = qcom_icc_aggregate;
> >> +       provider->xlate = of_icc_xlate_onecell;
> >> +       INIT_LIST_HEAD(&provider->nodes);
> >> +       provider->data = data;
> >> +
> >> +       ret = icc_provider_add(provider);
> >> +       if (ret) {
> >> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
> >> +               return ret;
> >> +       }
> >> +
> >> +       for (i = 0; i < num_nodes; i++) {
> >> +               size_t j;
> >> +
> >> +               node = icc_node_create(qnodes[i]->id);
> >> +               if (IS_ERR(node)) {
> >> +                       ret = PTR_ERR(node);
> >> +                       goto err;
> >> +               }
> >> +
> >> +               node->name = qnodes[i]->name;
> >> +               node->data = qnodes[i];
> >> +               icc_node_add(node, provider);
> >> +
> >> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> >> +                       qnodes[i]->name, node->id);
> >> +
> >> +               /* populate links */
> >> +               for (j = 0; j < qnodes[i]->num_links; j++)
> >> +                       icc_link_create(node, qnodes[i]->links[j]);
> >> +
> >> +               data->nodes[i] = node;
> >> +       }
> >> +       data->num_nodes = num_nodes;
> >> +
> >> +       platform_set_drvdata(pdev, qp);
> >> +
> >> +       return ret;
> >> +err:
> >> +       list_for_each_entry(node, &provider->nodes, node_list) {
> >> +               icc_node_del(node);
> >> +               icc_node_destroy(node->id);
> >> +       }
> >> +
> >> +       icc_provider_del(provider);
> >> +       return ret;
> >> +}
> >> +
> >> +static int qcom_osm_l3_remove(struct platform_device *pdev)
> >> +{
> >> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
> >> +       struct icc_provider *provider = &qp->provider;
> >> +       struct icc_node *n;
> >> +
> >> +       list_for_each_entry(n, &provider->nodes, node_list) {
> >> +               icc_node_del(n);
> >> +               icc_node_destroy(n->id);
> >> +       }
> >> +
> >> +       return icc_provider_del(provider);
> >> +}
> >> +
> >> +static const struct of_device_id osm_l3_of_match[] = {
> >> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
> >> +       { },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
> >> +
> >> +static struct platform_driver osm_l3_driver = {
> >> +       .probe = qcom_osm_l3_probe,
> >> +       .remove = qcom_osm_l3_remove,
> >> +       .driver = {
> >> +               .name = "osm-l3",
> >> +               .of_match_table = osm_l3_of_match,
> >> +       },
> >> +};
> >> +module_platform_driver(osm_l3_driver);
> >> +
> >> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
> >> +MODULE_LICENSE("GPL v2");
> >
> > Did a quick scan of the code and it's not clear how you connect the L3
> > interconnect provider to the rest of the interconnect. I don't see any
> > DT or code references to the rest of the interconnects in the system.
> > If GPU is making a bandwidth request all the way to L3, how do you
> > make sure the interconnects between GPU and L3 also scale up properly.
>
> For SDM845 OSM L3 provider the icc nodes endpoints are isolated from
> rsc icc node endpoints i.e GPU would need to vote on this path in
> addition to voting for DDR. On future SoCs if the need to scale
> interconnect between GPU rsc nodes along with the OSM l3 nodes arises,
> it can be trivially extended by linking the osm icc nodes with global
> icc node ids of the gpu rsc nodes.

Sorry, I'm not sure we are talking about the same thing. To keep the
discussion focused, let's completely ignore GPU's DDR needs for now.

Talking about GPU <-> L3 traffic, unless the GPU is directly connected
to the L3 with no additional interconnects in between (unlikely), this
L3 provider need to connect the L3 icc node with the rest of the
interconnect nodes. Otherwise, even if the L3 itself is provisioned to
handle, say 10 GB/s, the interconnects connecting the GPU to L3 might
be left at 1 GB/s.

So, can we please connect this node to the rest of the interconnect
nodes in SDM845?

Thanks,
Saravana

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-13 20:32   ` Evan Green
@ 2019-08-14 11:27     ` Sibi Sankar
  0 siblings, 0 replies; 11+ messages in thread
From: Sibi Sankar @ 2019-08-14 11:27 UTC (permalink / raw)
  To: Evan Green
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, David Dai, Saravana Kannan,
	linux-kernel-owner

Hey Evan,

Thanks for the review!

On 2019-08-14 02:02, Evan Green wrote:
> Hi Sibi,
> 
> On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> 
> wrote:
>> 
>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>> resources of scaling L3 caches. Add a driver to handle bandwidth
>> requests to OSM L3 from CPU/GPU.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> 
> This looks good to me, just a couple minor comments below.
> 
>> ---
>>  drivers/interconnect/qcom/Kconfig  |   7 +
>>  drivers/interconnect/qcom/Makefile |   2 +
>>  drivers/interconnect/qcom/osm-l3.c | 292 
>> +++++++++++++++++++++++++++++
>>  3 files changed, 301 insertions(+)
>>  create mode 100644 drivers/interconnect/qcom/osm-l3.c
>> 
>> diff --git a/drivers/interconnect/qcom/Kconfig 
>> b/drivers/interconnect/qcom/Kconfig
>> index d5e70ebc24108..f6c2a11a1a2c9 100644
>> --- a/drivers/interconnect/qcom/Kconfig
>> +++ b/drivers/interconnect/qcom/Kconfig
>> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
>>         help
>>           Support for Qualcomm's Network-on-Chip interconnect 
>> hardware.
>> 
>> +config INTERCONNECT_QCOM_OSM_L3
>> +       tristate "Qualcomm OSM L3 interconnect driver"
>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>> +       help
>> +         Say y here to support the Operating State Manager (OSM) 
>> interconnect
>> +         driver which controls the scaling of L3 caches on Qualcomm 
>> SoCs.
>> +
>>  config INTERCONNECT_QCOM_SDM845
>>         tristate "Qualcomm SDM845 interconnect driver"
>>         depends on INTERCONNECT_QCOM
>> diff --git a/drivers/interconnect/qcom/Makefile 
>> b/drivers/interconnect/qcom/Makefile
>> index 1c1cea690f922..9078af5fed109 100644
>> --- a/drivers/interconnect/qcom/Makefile
>> +++ b/drivers/interconnect/qcom/Makefile
>> @@ -1,5 +1,7 @@
>>  # SPDX-License-Identifier: GPL-2.0
>> 
>> +icc-osm-l3-objs                                := osm-l3.o
>>  qnoc-sdm845-objs                       := sdm845.o
>> 
>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>>  obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
>> diff --git a/drivers/interconnect/qcom/osm-l3.c 
>> b/drivers/interconnect/qcom/osm-l3.c
>> new file mode 100644
>> index 0000000000000..1e7dfce6f4f9b
>> --- /dev/null
>> +++ b/drivers/interconnect/qcom/osm-l3.c
>> @@ -0,0 +1,292 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> + *
>> + */
>> +
>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>> +#include <dt-bindings/interconnect/qcom,sdm845.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/clk.h>
>> +#include <linux/interconnect-provider.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +
>> +#define LUT_MAX_ENTRIES                        40U
>> +#define LUT_SRC                                GENMASK(31, 30)
>> +#define LUT_L_VAL                      GENMASK(7, 0)
>> +#define LUT_ROW_SIZE                   32
>> +#define CLK_HW_DIV                     2
>> +
>> +/* Register offsets */
>> +#define REG_ENABLE                     0x0
>> +#define REG_FREQ_LUT                   0x110
>> +#define REG_PERF_STATE                 0x920
>> +
>> +#define OSM_L3_MAX_LINKS               1
>> +
>> +#define to_qcom_provider(_provider) \
>> +       container_of(_provider, struct qcom_icc_provider, provider)
>> +
>> +enum {
>> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
>> +       SDM845_MASTER_OSM_L3_GPU,
>> +       SDM845_SLAVE_OSM_L3,
>> +};
>> +
>> +struct qcom_icc_provider {
> 
> Maybe we should rename this and the ones below to avoid collisions
> with drivers/interconnect/qcom/sdm845.c, even though those structures
> are currently local.

sure I'll do that

> 
>> +       void __iomem *base;
>> +       unsigned int max_state;
>> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
>> +       struct icc_provider provider;
>> +};
>> +
>> +/**
>> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
>> + * @name: the node name used in debugfs
>> + * @links: an array of nodes where we can go next while traversing
>> + * @id: a unique node identifier
>> + * @num_links: the total number of @links
>> + * @buswidth: width of the interconnect between a node and the bus
>> + */
>> +struct qcom_icc_node {
>> +       const char *name;
>> +       u16 links[OSM_L3_MAX_LINKS];
>> +       u16 id;
>> +       u16 num_links;
>> +       u16 buswidth;
>> +};
>> +
>> +struct qcom_icc_desc {
>> +       struct qcom_icc_node **nodes;
>> +       size_t num_nodes;
>> +};
>> +
>> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                      
>>  \
>> +               static struct qcom_icc_node _name = {                  
>>  \
>> +               .name = #_name,                                        
>>  \
>> +               .id = _id,                                             
>>  \
>> +               .buswidth = _buswidth,                                 
>>  \
>> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),     
>>  \
>> +               .links = { __VA_ARGS__ },                              
>>  \
>> +       }
>> +
>> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, 
>> SDM845_SLAVE_OSM_L3);
>> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, 
>> SDM845_SLAVE_OSM_L3);
>> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
>> +
>> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
>> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
>> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
>> +       [SLAVE_OSM_L3] = &osm_l3,
>> +};
>> +
>> +static struct qcom_icc_desc sdm845_osm_l3 = {
>> +       .nodes = sdm845_osm_l3_nodes,
>> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
>> +};
>> +
>> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
>> +                             u32 peak_bw, u32 *agg_avg, u32 
>> *agg_peak)
>> +{
>> +       *agg_avg += avg_bw;
>> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
>> +{
>> +       struct icc_provider *provider;
>> +       struct qcom_icc_provider *qp;
>> +       struct qcom_icc_node *qn;
>> +       struct icc_node *n;
>> +       unsigned int index;
>> +       u32 agg_peak = 0;
>> +       u32 agg_avg = 0;
>> +       u64 rate;
>> +
>> +       qn = src->data;
>> +       provider = src->provider;
>> +       qp = to_qcom_provider(provider);
>> +
>> +       list_for_each_entry(n, &provider->nodes, node_list)
>> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
>> +                                  &agg_avg, &agg_peak);
>> +
>> +       rate = max(agg_avg, agg_peak);
>> +       rate = icc_units_to_bps(rate);
>> +       do_div(rate, qn->buswidth);
>> +
>> +       for (index = 0; index < qp->max_state; index++) {
>> +               if (qp->lut_tables[index] >= rate)
>> +                       break;
>> +       }
>> +
>> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
>> +
>> +       return 0;
>> +}
>> +
>> +static int qcom_osm_l3_probe(struct platform_device *pdev)
>> +{
>> +       u32 info, src, lval, i, prev_freq = 0, freq;
>> +       static unsigned long hw_rate, xo_rate;
>> +       const struct qcom_icc_desc *desc;
>> +       struct icc_onecell_data *data;
>> +       struct icc_provider *provider;
>> +       struct qcom_icc_node **qnodes;
>> +       struct qcom_icc_provider *qp;
>> +       struct icc_node *node;
>> +       size_t num_nodes;
>> +       struct clk *clk;
>> +       int ret;
>> +
>> +       clk = clk_get(&pdev->dev, "xo");
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       xo_rate = clk_get_rate(clk);
>> +       clk_put(clk);
>> +
>> +       clk = clk_get(&pdev->dev, "alternate");
>> +       if (IS_ERR(clk))
>> +               return PTR_ERR(clk);
>> +
>> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
>> +       clk_put(clk);
>> +
>> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
>> +       if (!qp)
>> +               return -ENOMEM;
>> +
>> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
>> +       if (IS_ERR(qp->base))
>> +               return PTR_ERR(qp->base);
>> +
>> +       /* HW should be in enabled state to proceed */
>> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
>> +               dev_err(&pdev->dev, "error hardware not enabled\n");
>> +               return -ENODEV;
>> +       }
>> +
>> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
>> +                                    i * LUT_ROW_SIZE);
>> +               src = FIELD_GET(LUT_SRC, info);
>> +               lval = FIELD_GET(LUT_L_VAL, info);
>> +               if (src)
>> +                       freq = xo_rate * lval;
>> +               else
>> +                       freq = hw_rate;
>> +
>> +               /*
>> +                * Two of the same frequencies with the same core 
>> counts means
>> +                * end of table
>> +                */
>> +               if (i > 0 && prev_freq == freq)
>> +                       break;
>> +
>> +               qp->lut_tables[i] = freq;
>> +               prev_freq = freq;
>> +       }
>> +       qp->max_state = i;
>> +
>> +       desc = of_device_get_match_data(&pdev->dev);
>> +       if (!desc)
>> +               return -EINVAL;
>> +
>> +       qnodes = desc->nodes;
>> +       num_nodes = desc->num_nodes;
>> +
>> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), 
>> GFP_KERNEL);
>> +       if (!data)
>> +               return -ENOMEM;
>> +
>> +       provider = &qp->provider;
>> +       provider->dev = &pdev->dev;
>> +       provider->set = qcom_icc_set;
>> +       provider->aggregate = qcom_icc_aggregate;
>> +       provider->xlate = of_icc_xlate_onecell;
>> +       INIT_LIST_HEAD(&provider->nodes);
>> +       provider->data = data;
>> +
>> +       ret = icc_provider_add(provider);
>> +       if (ret) {
>> +               dev_err(&pdev->dev, "error adding interconnect 
>> provider\n");
>> +               return ret;
>> +       }
>> +
>> +       for (i = 0; i < num_nodes; i++) {
>> +               size_t j;
>> +
>> +               node = icc_node_create(qnodes[i]->id);
>> +               if (IS_ERR(node)) {
>> +                       ret = PTR_ERR(node);
>> +                       goto err;
>> +               }
>> +
>> +               node->name = qnodes[i]->name;
>> +               node->data = qnodes[i];
>> +               icc_node_add(node, provider);
>> +
>> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", 
>> node,
>> +                       qnodes[i]->name, node->id);
>> +
>> +               /* populate links */
>> +               for (j = 0; j < qnodes[i]->num_links; j++)
>> +                       icc_link_create(node, qnodes[i]->links[j]);
>> +
>> +               data->nodes[i] = node;
>> +       }
>> +       data->num_nodes = num_nodes;
>> +
>> +       platform_set_drvdata(pdev, qp);
>> +
>> +       return ret;
>> +err:
>> +       list_for_each_entry(node, &provider->nodes, node_list) {
>> +               icc_node_del(node);
>> +               icc_node_destroy(node->id);
>> +       }
>> +
>> +       icc_provider_del(provider);
> 
> I looks like you could just call qcom_osm_l3_remove() here instead of 
> the above.

yeah

> 
> 
>> +       return ret;
>> +}
>> +
>> +static int qcom_osm_l3_remove(struct platform_device *pdev)
>> +{
>> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>> +       struct icc_provider *provider = &qp->provider;
>> +       struct icc_node *n;
>> +
>> +       list_for_each_entry(n, &provider->nodes, node_list) {
>> +               icc_node_del(n);
>> +               icc_node_destroy(n->id);
>> +       }
>> +
>> +       return icc_provider_del(provider);
>> +}
>> +
>> +static const struct of_device_id osm_l3_of_match[] = {
>> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 
>> },
>> +       { },
>> +};
>> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
>> +
>> +static struct platform_driver osm_l3_driver = {
>> +       .probe = qcom_osm_l3_probe,
>> +       .remove = qcom_osm_l3_remove,
>> +       .driver = {
>> +               .name = "osm-l3",
>> +               .of_match_table = osm_l3_of_match,
>> +       },
>> +};
>> +module_platform_driver(osm_l3_driver);
>> +
>> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project
>> 

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-14  0:43       ` Saravana Kannan
@ 2019-08-14 11:30         ` Sibi Sankar
  2019-08-14 15:21           ` Jordan Crouse
  0 siblings, 1 reply; 11+ messages in thread
From: Sibi Sankar @ 2019-08-14 11:30 UTC (permalink / raw)
  To: Saravana Kannan
  Cc: Rob Herring, Georgi Djakov, Bjorn Andersson, agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, Evan Green, David Dai,
	linux-kernel-owner

On 2019-08-14 06:13, Saravana Kannan wrote:
> On Thu, Aug 8, 2019 at 10:37 AM Sibi Sankar <sibis@codeaurora.org> 
> wrote:
>> 
>> Hey Saravana,
>> 
>> Thanks for the review!
>> 
>> On 8/8/19 2:51 AM, Saravana Kannan wrote:
>> > On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>> >>
>> >> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
>> >> resources of scaling L3 caches. Add a driver to handle bandwidth
>> >> requests to OSM L3 from CPU/GPU.
>> >>
>> >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> >> ---
>> >>   drivers/interconnect/qcom/Kconfig  |   7 +
>> >>   drivers/interconnect/qcom/Makefile |   2 +
>> >>   drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
>> >>   3 files changed, 301 insertions(+)
>> >>   create mode 100644 drivers/interconnect/qcom/osm-l3.c
>> >>
>> >> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
>> >> index d5e70ebc24108..f6c2a11a1a2c9 100644
>> >> --- a/drivers/interconnect/qcom/Kconfig
>> >> +++ b/drivers/interconnect/qcom/Kconfig
>> >> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
>> >>          help
>> >>            Support for Qualcomm's Network-on-Chip interconnect hardware.
>> >>
>> >> +config INTERCONNECT_QCOM_OSM_L3
>> >> +       tristate "Qualcomm OSM L3 interconnect driver"
>> >> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
>> >> +       help
>> >> +         Say y here to support the Operating State Manager (OSM) interconnect
>> >> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
>> >> +
>> >>   config INTERCONNECT_QCOM_SDM845
>> >>          tristate "Qualcomm SDM845 interconnect driver"
>> >>          depends on INTERCONNECT_QCOM
>> >> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
>> >> index 1c1cea690f922..9078af5fed109 100644
>> >> --- a/drivers/interconnect/qcom/Makefile
>> >> +++ b/drivers/interconnect/qcom/Makefile
>> >> @@ -1,5 +1,7 @@
>> >>   # SPDX-License-Identifier: GPL-2.0
>> >>
>> >> +icc-osm-l3-objs                                := osm-l3.o
>> >>   qnoc-sdm845-objs                       := sdm845.o
>> >>
>> >> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
>> >>   obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
>> >> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
>> >> new file mode 100644
>> >> index 0000000000000..1e7dfce6f4f9b
>> >> --- /dev/null
>> >> +++ b/drivers/interconnect/qcom/osm-l3.c
>> >> @@ -0,0 +1,292 @@
>> >> +// SPDX-License-Identifier: GPL-2.0
>> >> +/*
>> >> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
>> >> + *
>> >> + */
>> >> +
>> >> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
>> >> +#include <dt-bindings/interconnect/qcom,sdm845.h>
>> >> +#include <linux/bitfield.h>
>> >> +#include <linux/clk.h>
>> >> +#include <linux/interconnect-provider.h>
>> >> +#include <linux/io.h>
>> >> +#include <linux/kernel.h>
>> >> +#include <linux/module.h>
>> >> +#include <linux/of_device.h>
>> >> +#include <linux/of_platform.h>
>> >> +#include <linux/platform_device.h>
>> >> +
>> >> +#define LUT_MAX_ENTRIES                        40U
>> >> +#define LUT_SRC                                GENMASK(31, 30)
>> >> +#define LUT_L_VAL                      GENMASK(7, 0)
>> >> +#define LUT_ROW_SIZE                   32
>> >> +#define CLK_HW_DIV                     2
>> >> +
>> >> +/* Register offsets */
>> >> +#define REG_ENABLE                     0x0
>> >> +#define REG_FREQ_LUT                   0x110
>> >> +#define REG_PERF_STATE                 0x920
>> >> +
>> >> +#define OSM_L3_MAX_LINKS               1
>> >> +
>> >> +#define to_qcom_provider(_provider) \
>> >> +       container_of(_provider, struct qcom_icc_provider, provider)
>> >> +
>> >> +enum {
>> >> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
>> >> +       SDM845_MASTER_OSM_L3_GPU,
>> >> +       SDM845_SLAVE_OSM_L3,
>> >> +};
>> >> +
>> >> +struct qcom_icc_provider {
>> >> +       void __iomem *base;
>> >> +       unsigned int max_state;
>> >> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
>> >> +       struct icc_provider provider;
>> >> +};
>> >> +
>> >> +/**
>> >> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
>> >> + * @name: the node name used in debugfs
>> >> + * @links: an array of nodes where we can go next while traversing
>> >> + * @id: a unique node identifier
>> >> + * @num_links: the total number of @links
>> >> + * @buswidth: width of the interconnect between a node and the bus
>> >> + */
>> >> +struct qcom_icc_node {
>> >> +       const char *name;
>> >> +       u16 links[OSM_L3_MAX_LINKS];
>> >> +       u16 id;
>> >> +       u16 num_links;
>> >> +       u16 buswidth;
>> >> +};
>> >> +
>> >> +struct qcom_icc_desc {
>> >> +       struct qcom_icc_node **nodes;
>> >> +       size_t num_nodes;
>> >> +};
>> >> +
>> >> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
>> >> +               static struct qcom_icc_node _name = {                   \
>> >> +               .name = #_name,                                         \
>> >> +               .id = _id,                                              \
>> >> +               .buswidth = _buswidth,                                  \
>> >> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
>> >> +               .links = { __VA_ARGS__ },                               \
>> >> +       }
>> >> +
>> >> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
>> >> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
>> >> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
>> >> +
>> >> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
>> >> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
>> >> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
>> >> +       [SLAVE_OSM_L3] = &osm_l3,
>> >> +};
>> >> +
>> >> +static struct qcom_icc_desc sdm845_osm_l3 = {
>> >> +       .nodes = sdm845_osm_l3_nodes,
>> >> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
>> >> +};
>> >> +
>> >> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
>> >> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
>> >> +{
>> >> +       *agg_avg += avg_bw;
>> >> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
>> >> +
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
>> >> +{
>> >> +       struct icc_provider *provider;
>> >> +       struct qcom_icc_provider *qp;
>> >> +       struct qcom_icc_node *qn;
>> >> +       struct icc_node *n;
>> >> +       unsigned int index;
>> >> +       u32 agg_peak = 0;
>> >> +       u32 agg_avg = 0;
>> >> +       u64 rate;
>> >> +
>> >> +       qn = src->data;
>> >> +       provider = src->provider;
>> >> +       qp = to_qcom_provider(provider);
>> >> +
>> >> +       list_for_each_entry(n, &provider->nodes, node_list)
>> >> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
>> >> +                                  &agg_avg, &agg_peak);
>> >> +
>> >> +       rate = max(agg_avg, agg_peak);
>> >> +       rate = icc_units_to_bps(rate);
>> >> +       do_div(rate, qn->buswidth);
>> >> +
>> >> +       for (index = 0; index < qp->max_state; index++) {
>> >> +               if (qp->lut_tables[index] >= rate)
>> >> +                       break;
>> >> +       }
>> >> +
>> >> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
>> >> +
>> >> +       return 0;
>> >> +}
>> >> +
>> >> +static int qcom_osm_l3_probe(struct platform_device *pdev)
>> >> +{
>> >> +       u32 info, src, lval, i, prev_freq = 0, freq;
>> >> +       static unsigned long hw_rate, xo_rate;
>> >> +       const struct qcom_icc_desc *desc;
>> >> +       struct icc_onecell_data *data;
>> >> +       struct icc_provider *provider;
>> >> +       struct qcom_icc_node **qnodes;
>> >> +       struct qcom_icc_provider *qp;
>> >> +       struct icc_node *node;
>> >> +       size_t num_nodes;
>> >> +       struct clk *clk;
>> >> +       int ret;
>> >> +
>> >> +       clk = clk_get(&pdev->dev, "xo");
>> >> +       if (IS_ERR(clk))
>> >> +               return PTR_ERR(clk);
>> >> +
>> >> +       xo_rate = clk_get_rate(clk);
>> >> +       clk_put(clk);
>> >> +
>> >> +       clk = clk_get(&pdev->dev, "alternate");
>> >> +       if (IS_ERR(clk))
>> >> +               return PTR_ERR(clk);
>> >> +
>> >> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
>> >> +       clk_put(clk);
>> >> +
>> >> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
>> >> +       if (!qp)
>> >> +               return -ENOMEM;
>> >> +
>> >> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
>> >> +       if (IS_ERR(qp->base))
>> >> +               return PTR_ERR(qp->base);
>> >> +
>> >> +       /* HW should be in enabled state to proceed */
>> >> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
>> >> +               dev_err(&pdev->dev, "error hardware not enabled\n");
>> >> +               return -ENODEV;
>> >> +       }
>> >> +
>> >> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>> >> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
>> >> +                                    i * LUT_ROW_SIZE);
>> >> +               src = FIELD_GET(LUT_SRC, info);
>> >> +               lval = FIELD_GET(LUT_L_VAL, info);
>> >> +               if (src)
>> >> +                       freq = xo_rate * lval;
>> >> +               else
>> >> +                       freq = hw_rate;
>> >> +
>> >> +               /*
>> >> +                * Two of the same frequencies with the same core counts means
>> >> +                * end of table
>> >> +                */
>> >> +               if (i > 0 && prev_freq == freq)
>> >> +                       break;
>> >> +
>> >> +               qp->lut_tables[i] = freq;
>> >> +               prev_freq = freq;
>> >> +       }
>> >> +       qp->max_state = i;
>> >> +
>> >> +       desc = of_device_get_match_data(&pdev->dev);
>> >> +       if (!desc)
>> >> +               return -EINVAL;
>> >> +
>> >> +       qnodes = desc->nodes;
>> >> +       num_nodes = desc->num_nodes;
>> >> +
>> >> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
>> >> +       if (!data)
>> >> +               return -ENOMEM;
>> >> +
>> >> +       provider = &qp->provider;
>> >> +       provider->dev = &pdev->dev;
>> >> +       provider->set = qcom_icc_set;
>> >> +       provider->aggregate = qcom_icc_aggregate;
>> >> +       provider->xlate = of_icc_xlate_onecell;
>> >> +       INIT_LIST_HEAD(&provider->nodes);
>> >> +       provider->data = data;
>> >> +
>> >> +       ret = icc_provider_add(provider);
>> >> +       if (ret) {
>> >> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
>> >> +               return ret;
>> >> +       }
>> >> +
>> >> +       for (i = 0; i < num_nodes; i++) {
>> >> +               size_t j;
>> >> +
>> >> +               node = icc_node_create(qnodes[i]->id);
>> >> +               if (IS_ERR(node)) {
>> >> +                       ret = PTR_ERR(node);
>> >> +                       goto err;
>> >> +               }
>> >> +
>> >> +               node->name = qnodes[i]->name;
>> >> +               node->data = qnodes[i];
>> >> +               icc_node_add(node, provider);
>> >> +
>> >> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
>> >> +                       qnodes[i]->name, node->id);
>> >> +
>> >> +               /* populate links */
>> >> +               for (j = 0; j < qnodes[i]->num_links; j++)
>> >> +                       icc_link_create(node, qnodes[i]->links[j]);
>> >> +
>> >> +               data->nodes[i] = node;
>> >> +       }
>> >> +       data->num_nodes = num_nodes;
>> >> +
>> >> +       platform_set_drvdata(pdev, qp);
>> >> +
>> >> +       return ret;
>> >> +err:
>> >> +       list_for_each_entry(node, &provider->nodes, node_list) {
>> >> +               icc_node_del(node);
>> >> +               icc_node_destroy(node->id);
>> >> +       }
>> >> +
>> >> +       icc_provider_del(provider);
>> >> +       return ret;
>> >> +}
>> >> +
>> >> +static int qcom_osm_l3_remove(struct platform_device *pdev)
>> >> +{
>> >> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
>> >> +       struct icc_provider *provider = &qp->provider;
>> >> +       struct icc_node *n;
>> >> +
>> >> +       list_for_each_entry(n, &provider->nodes, node_list) {
>> >> +               icc_node_del(n);
>> >> +               icc_node_destroy(n->id);
>> >> +       }
>> >> +
>> >> +       return icc_provider_del(provider);
>> >> +}
>> >> +
>> >> +static const struct of_device_id osm_l3_of_match[] = {
>> >> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
>> >> +       { },
>> >> +};
>> >> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
>> >> +
>> >> +static struct platform_driver osm_l3_driver = {
>> >> +       .probe = qcom_osm_l3_probe,
>> >> +       .remove = qcom_osm_l3_remove,
>> >> +       .driver = {
>> >> +               .name = "osm-l3",
>> >> +               .of_match_table = osm_l3_of_match,
>> >> +       },
>> >> +};
>> >> +module_platform_driver(osm_l3_driver);
>> >> +
>> >> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
>> >> +MODULE_LICENSE("GPL v2");
>> >
>> > Did a quick scan of the code and it's not clear how you connect the L3
>> > interconnect provider to the rest of the interconnect. I don't see any
>> > DT or code references to the rest of the interconnects in the system.
>> > If GPU is making a bandwidth request all the way to L3, how do you
>> > make sure the interconnects between GPU and L3 also scale up properly.
>> 
>> For SDM845 OSM L3 provider the icc nodes endpoints are isolated from
>> rsc icc node endpoints i.e GPU would need to vote on this path in
>> addition to voting for DDR. On future SoCs if the need to scale
>> interconnect between GPU rsc nodes along with the OSM l3 nodes arises,
>> it can be trivially extended by linking the osm icc nodes with global
>> icc node ids of the gpu rsc nodes.
> 
> Sorry, I'm not sure we are talking about the same thing. To keep the
> discussion focused, let's completely ignore GPU's DDR needs for now.
> 
> Talking about GPU <-> L3 traffic, unless the GPU is directly connected
> to the L3 with no additional interconnects in between (unlikely), this
> L3 provider need to connect the L3 icc node with the rest of the
> interconnect nodes. Otherwise, even if the L3 itself is provisioned to
> handle, say 10 GB/s, the interconnects connecting the GPU to L3 might
> be left at 1 GB/s.
> 
> So, can we please connect this node to the rest of the interconnect
> nodes in SDM845?

Sure I'll double check if GPU has a l3
scaling requirement on SDM845 SoCs and
drop/link it to the rsc nodes appropriately.

> 
> Thanks,
> Saravana

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

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

* Re: [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings
  2019-08-07 11:24 ` [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings Sibi Sankar
@ 2019-08-14 11:34   ` Sibi Sankar
  0 siblings, 0 replies; 11+ messages in thread
From: Sibi Sankar @ 2019-08-14 11:34 UTC (permalink / raw)
  To: robh+dt, georgi.djakov
  Cc: bjorn.andersson, agross, linux-kernel, devicetree, linux-arm-msm,
	mark.rutland, evgreen, daidavid1, saravanak

on running dt_binding_check found a
few errors which I'll fix in the next
re-spin.


On 2019-08-07 16:54, Sibi Sankar wrote:
> Add bindings for Operating State Manager (OSM) L3 interconnect provider
> on SDM845 SoCs.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  .../bindings/interconnect/qcom,osm-l3.yaml    | 55 +++++++++++++++++++
>  .../dt-bindings/interconnect/qcom,osm-l3.h    | 13 +++++
>  2 files changed, 68 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
>  create mode 100644 include/dt-bindings/interconnect/qcom,osm-l3.h
> 
> diff --git
> a/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
> b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
> new file mode 100644
> index 0000000000000..51a4722e1a69f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/qcom,osm-l3.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interconnect/qcom,osm-l3.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Operating State Manager (OSM) L3 Interconnect Provider
> +
> +maintainers:
> +  - Sibi Sankar <sibis@codeaurora.org>
> +
> +description:
> +  L3 cache bandwidth requirements on Qualcomm SoCs is serviced by the 
> OSM.
> +  The OSM L3 interconnect provider aggregates the L3 bandwidth 
> requests
> +  from CPU/GPU and relays it to the OSM.
> +
> +properties:
> +  compatible:
> +    const: "qcom,sdm845-osm-l3"
> +
> +  reg:
> +    maxItems: 1
> +    description: OSM L3 registers

will correct the error ^^

> +
> +  clocks:
> +    items:
> +      - description: xo clock
> +      - description: alternate clock
> +
> +  clock-names:
> +    items:
> +      - const: xo
> +      - const: alternate
> +
> +  '#interconnect-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clocks-names

s/clocks-names/clock-names

> +  - '#interconnect-cells'
> +
> +examples:
> +  - |
> +    osm_l3: interconnect@17d41000 {
> +      compatible = "qcom,sdm845-osm-l3";
> +      reg = <0 0x17d41000 0 0x1400>;
> +
> +      clocks = <&rpmhcc RPMH_CXO_CLK>, <&gcc GPLL0>;

will replace rpmh_cxo_clk with 0
and GPLL0 with 165

> +      clock-names = "xo", "alternate";
> +
> +      #interconnect-cells = <1>;
> +    };
> diff --git a/include/dt-bindings/interconnect/qcom,osm-l3.h
> b/include/dt-bindings/interconnect/qcom,osm-l3.h
> new file mode 100644
> index 0000000000000..6662134c84248
> --- /dev/null
> +++ b/include/dt-bindings/interconnect/qcom,osm-l3.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2019 The Linux Foundation. All rights reserved.
> + */
> +
> +#ifndef __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
> +#define __DT_BINDINGS_INTERCONNECT_QCOM_OSM_L3_H
> +
> +#define MASTER_OSM_L3_APPS	0
> +#define MASTER_OSM_L3_GPU	1
> +#define SLAVE_OSM_L3		2
> +
> +#endif

-- 
-- Sibi Sankar --
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.

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

* Re: [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support
  2019-08-14 11:30         ` Sibi Sankar
@ 2019-08-14 15:21           ` Jordan Crouse
  0 siblings, 0 replies; 11+ messages in thread
From: Jordan Crouse @ 2019-08-14 15:21 UTC (permalink / raw)
  To: Sibi Sankar
  Cc: Saravana Kannan, Rob Herring, Georgi Djakov, Bjorn Andersson,
	agross, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-arm-msm, Mark Rutland, Evan Green, David Dai,
	linux-kernel-owner

On Wed, Aug 14, 2019 at 05:00:49PM +0530, Sibi Sankar wrote:
> On 2019-08-14 06:13, Saravana Kannan wrote:
> >On Thu, Aug 8, 2019 at 10:37 AM Sibi Sankar <sibis@codeaurora.org> wrote:
> >>
> >>Hey Saravana,
> >>
> >>Thanks for the review!
> >>
> >>On 8/8/19 2:51 AM, Saravana Kannan wrote:
> >>> On Wed, Aug 7, 2019 at 4:24 AM Sibi Sankar <sibis@codeaurora.org> wrote:
> >>>>
> >>>> On some Qualcomm SoCs, Operating State Manager (OSM) controls the
> >>>> resources of scaling L3 caches. Add a driver to handle bandwidth
> >>>> requests to OSM L3 from CPU/GPU.
> >>>>
> >>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >>>> ---
> >>>>   drivers/interconnect/qcom/Kconfig  |   7 +
> >>>>   drivers/interconnect/qcom/Makefile |   2 +
> >>>>   drivers/interconnect/qcom/osm-l3.c | 292 +++++++++++++++++++++++++++++
> >>>>   3 files changed, 301 insertions(+)
> >>>>   create mode 100644 drivers/interconnect/qcom/osm-l3.c
> >>>>
> >>>> diff --git a/drivers/interconnect/qcom/Kconfig b/drivers/interconnect/qcom/Kconfig
> >>>> index d5e70ebc24108..f6c2a11a1a2c9 100644
> >>>> --- a/drivers/interconnect/qcom/Kconfig
> >>>> +++ b/drivers/interconnect/qcom/Kconfig
> >>>> @@ -5,6 +5,13 @@ config INTERCONNECT_QCOM
> >>>>          help
> >>>>            Support for Qualcomm's Network-on-Chip interconnect hardware.
> >>>>
> >>>> +config INTERCONNECT_QCOM_OSM_L3
> >>>> +       tristate "Qualcomm OSM L3 interconnect driver"
> >>>> +       depends on INTERCONNECT_QCOM || COMPILE_TEST
> >>>> +       help
> >>>> +         Say y here to support the Operating State Manager (OSM) interconnect
> >>>> +         driver which controls the scaling of L3 caches on Qualcomm SoCs.
> >>>> +
> >>>>   config INTERCONNECT_QCOM_SDM845
> >>>>          tristate "Qualcomm SDM845 interconnect driver"
> >>>>          depends on INTERCONNECT_QCOM
> >>>> diff --git a/drivers/interconnect/qcom/Makefile b/drivers/interconnect/qcom/Makefile
> >>>> index 1c1cea690f922..9078af5fed109 100644
> >>>> --- a/drivers/interconnect/qcom/Makefile
> >>>> +++ b/drivers/interconnect/qcom/Makefile
> >>>> @@ -1,5 +1,7 @@
> >>>>   # SPDX-License-Identifier: GPL-2.0
> >>>>
> >>>> +icc-osm-l3-objs                                := osm-l3.o
> >>>>   qnoc-sdm845-objs                       := sdm845.o
> >>>>
> >>>> +obj-$(CONFIG_INTERCONNECT_QCOM_OSM_L3) += icc-osm-l3.o
> >>>>   obj-$(CONFIG_INTERCONNECT_QCOM_SDM845) += qnoc-sdm845.o
> >>>> diff --git a/drivers/interconnect/qcom/osm-l3.c b/drivers/interconnect/qcom/osm-l3.c
> >>>> new file mode 100644
> >>>> index 0000000000000..1e7dfce6f4f9b
> >>>> --- /dev/null
> >>>> +++ b/drivers/interconnect/qcom/osm-l3.c
> >>>> @@ -0,0 +1,292 @@
> >>>> +// SPDX-License-Identifier: GPL-2.0
> >>>> +/*
> >>>> + * Copyright (c) 2019, The Linux Foundation. All rights reserved.
> >>>> + *
> >>>> + */
> >>>> +
> >>>> +#include <dt-bindings/interconnect/qcom,osm-l3.h>
> >>>> +#include <dt-bindings/interconnect/qcom,sdm845.h>
> >>>> +#include <linux/bitfield.h>
> >>>> +#include <linux/clk.h>
> >>>> +#include <linux/interconnect-provider.h>
> >>>> +#include <linux/io.h>
> >>>> +#include <linux/kernel.h>
> >>>> +#include <linux/module.h>
> >>>> +#include <linux/of_device.h>
> >>>> +#include <linux/of_platform.h>
> >>>> +#include <linux/platform_device.h>
> >>>> +
> >>>> +#define LUT_MAX_ENTRIES                        40U
> >>>> +#define LUT_SRC                                GENMASK(31, 30)
> >>>> +#define LUT_L_VAL                      GENMASK(7, 0)
> >>>> +#define LUT_ROW_SIZE                   32
> >>>> +#define CLK_HW_DIV                     2
> >>>> +
> >>>> +/* Register offsets */
> >>>> +#define REG_ENABLE                     0x0
> >>>> +#define REG_FREQ_LUT                   0x110
> >>>> +#define REG_PERF_STATE                 0x920
> >>>> +
> >>>> +#define OSM_L3_MAX_LINKS               1
> >>>> +
> >>>> +#define to_qcom_provider(_provider) \
> >>>> +       container_of(_provider, struct qcom_icc_provider, provider)
> >>>> +
> >>>> +enum {
> >>>> +       SDM845_MASTER_OSM_L3_APPS = SLAVE_TCU + 1,
> >>>> +       SDM845_MASTER_OSM_L3_GPU,
> >>>> +       SDM845_SLAVE_OSM_L3,
> >>>> +};
> >>>> +
> >>>> +struct qcom_icc_provider {
> >>>> +       void __iomem *base;
> >>>> +       unsigned int max_state;
> >>>> +       unsigned long lut_tables[LUT_MAX_ENTRIES];
> >>>> +       struct icc_provider provider;
> >>>> +};
> >>>> +
> >>>> +/**
> >>>> + * struct qcom_icc_node - Qualcomm specific interconnect nodes
> >>>> + * @name: the node name used in debugfs
> >>>> + * @links: an array of nodes where we can go next while traversing
> >>>> + * @id: a unique node identifier
> >>>> + * @num_links: the total number of @links
> >>>> + * @buswidth: width of the interconnect between a node and the bus
> >>>> + */
> >>>> +struct qcom_icc_node {
> >>>> +       const char *name;
> >>>> +       u16 links[OSM_L3_MAX_LINKS];
> >>>> +       u16 id;
> >>>> +       u16 num_links;
> >>>> +       u16 buswidth;
> >>>> +};
> >>>> +
> >>>> +struct qcom_icc_desc {
> >>>> +       struct qcom_icc_node **nodes;
> >>>> +       size_t num_nodes;
> >>>> +};
> >>>> +
> >>>> +#define DEFINE_QNODE(_name, _id, _buswidth, ...)                       \
> >>>> +               static struct qcom_icc_node _name = {                   \
> >>>> +               .name = #_name,                                         \
> >>>> +               .id = _id,                                              \
> >>>> +               .buswidth = _buswidth,                                  \
> >>>> +               .num_links = ARRAY_SIZE(((int[]){ __VA_ARGS__ })),      \
> >>>> +               .links = { __VA_ARGS__ },                               \
> >>>> +       }
> >>>> +
> >>>> +DEFINE_QNODE(osm_apps_l3, SDM845_MASTER_OSM_L3_APPS, 16, SDM845_SLAVE_OSM_L3);
> >>>> +DEFINE_QNODE(osm_gpu_l3, SDM845_MASTER_OSM_L3_GPU, 16, SDM845_SLAVE_OSM_L3);
> >>>> +DEFINE_QNODE(osm_l3, SDM845_SLAVE_OSM_L3, 16);
> >>>> +
> >>>> +static struct qcom_icc_node *sdm845_osm_l3_nodes[] = {
> >>>> +       [MASTER_OSM_L3_APPS] = &osm_apps_l3,
> >>>> +       [MASTER_OSM_L3_GPU] = &osm_gpu_l3,
> >>>> +       [SLAVE_OSM_L3] = &osm_l3,
> >>>> +};
> >>>> +
> >>>> +static struct qcom_icc_desc sdm845_osm_l3 = {
> >>>> +       .nodes = sdm845_osm_l3_nodes,
> >>>> +       .num_nodes = ARRAY_SIZE(sdm845_osm_l3_nodes),
> >>>> +};
> >>>> +
> >>>> +static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
> >>>> +                             u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
> >>>> +{
> >>>> +       *agg_avg += avg_bw;
> >>>> +       *agg_peak = max_t(u32, *agg_peak, peak_bw);
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>> +
> >>>> +static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> >>>> +{
> >>>> +       struct icc_provider *provider;
> >>>> +       struct qcom_icc_provider *qp;
> >>>> +       struct qcom_icc_node *qn;
> >>>> +       struct icc_node *n;
> >>>> +       unsigned int index;
> >>>> +       u32 agg_peak = 0;
> >>>> +       u32 agg_avg = 0;
> >>>> +       u64 rate;
> >>>> +
> >>>> +       qn = src->data;
> >>>> +       provider = src->provider;
> >>>> +       qp = to_qcom_provider(provider);
> >>>> +
> >>>> +       list_for_each_entry(n, &provider->nodes, node_list)
> >>>> +               qcom_icc_aggregate(n, n->avg_bw, n->peak_bw,
> >>>> +                                  &agg_avg, &agg_peak);
> >>>> +
> >>>> +       rate = max(agg_avg, agg_peak);
> >>>> +       rate = icc_units_to_bps(rate);
> >>>> +       do_div(rate, qn->buswidth);
> >>>> +
> >>>> +       for (index = 0; index < qp->max_state; index++) {
> >>>> +               if (qp->lut_tables[index] >= rate)
> >>>> +                       break;
> >>>> +       }
> >>>> +
> >>>> +       writel_relaxed(index, qp->base + REG_PERF_STATE);
> >>>> +
> >>>> +       return 0;
> >>>> +}
> >>>> +
> >>>> +static int qcom_osm_l3_probe(struct platform_device *pdev)
> >>>> +{
> >>>> +       u32 info, src, lval, i, prev_freq = 0, freq;
> >>>> +       static unsigned long hw_rate, xo_rate;
> >>>> +       const struct qcom_icc_desc *desc;
> >>>> +       struct icc_onecell_data *data;
> >>>> +       struct icc_provider *provider;
> >>>> +       struct qcom_icc_node **qnodes;
> >>>> +       struct qcom_icc_provider *qp;
> >>>> +       struct icc_node *node;
> >>>> +       size_t num_nodes;
> >>>> +       struct clk *clk;
> >>>> +       int ret;
> >>>> +
> >>>> +       clk = clk_get(&pdev->dev, "xo");
> >>>> +       if (IS_ERR(clk))
> >>>> +               return PTR_ERR(clk);
> >>>> +
> >>>> +       xo_rate = clk_get_rate(clk);
> >>>> +       clk_put(clk);
> >>>> +
> >>>> +       clk = clk_get(&pdev->dev, "alternate");
> >>>> +       if (IS_ERR(clk))
> >>>> +               return PTR_ERR(clk);
> >>>> +
> >>>> +       hw_rate = clk_get_rate(clk) / CLK_HW_DIV;
> >>>> +       clk_put(clk);
> >>>> +
> >>>> +       qp = devm_kzalloc(&pdev->dev, sizeof(*qp), GFP_KERNEL);
> >>>> +       if (!qp)
> >>>> +               return -ENOMEM;
> >>>> +
> >>>> +       qp->base = devm_platform_ioremap_resource(pdev, 0);
> >>>> +       if (IS_ERR(qp->base))
> >>>> +               return PTR_ERR(qp->base);
> >>>> +
> >>>> +       /* HW should be in enabled state to proceed */
> >>>> +       if (!(readl_relaxed(qp->base + REG_ENABLE) & 0x1)) {
> >>>> +               dev_err(&pdev->dev, "error hardware not enabled\n");
> >>>> +               return -ENODEV;
> >>>> +       }
> >>>> +
> >>>> +       for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> >>>> +               info = readl_relaxed(qp->base + REG_FREQ_LUT +
> >>>> +                                    i * LUT_ROW_SIZE);
> >>>> +               src = FIELD_GET(LUT_SRC, info);
> >>>> +               lval = FIELD_GET(LUT_L_VAL, info);
> >>>> +               if (src)
> >>>> +                       freq = xo_rate * lval;
> >>>> +               else
> >>>> +                       freq = hw_rate;
> >>>> +
> >>>> +               /*
> >>>> +                * Two of the same frequencies with the same core counts means
> >>>> +                * end of table
> >>>> +                */
> >>>> +               if (i > 0 && prev_freq == freq)
> >>>> +                       break;
> >>>> +
> >>>> +               qp->lut_tables[i] = freq;
> >>>> +               prev_freq = freq;
> >>>> +       }
> >>>> +       qp->max_state = i;
> >>>> +
> >>>> +       desc = of_device_get_match_data(&pdev->dev);
> >>>> +       if (!desc)
> >>>> +               return -EINVAL;
> >>>> +
> >>>> +       qnodes = desc->nodes;
> >>>> +       num_nodes = desc->num_nodes;
> >>>> +
> >>>> +       data = devm_kcalloc(&pdev->dev, num_nodes, sizeof(*node), GFP_KERNEL);
> >>>> +       if (!data)
> >>>> +               return -ENOMEM;
> >>>> +
> >>>> +       provider = &qp->provider;
> >>>> +       provider->dev = &pdev->dev;
> >>>> +       provider->set = qcom_icc_set;
> >>>> +       provider->aggregate = qcom_icc_aggregate;
> >>>> +       provider->xlate = of_icc_xlate_onecell;
> >>>> +       INIT_LIST_HEAD(&provider->nodes);
> >>>> +       provider->data = data;
> >>>> +
> >>>> +       ret = icc_provider_add(provider);
> >>>> +       if (ret) {
> >>>> +               dev_err(&pdev->dev, "error adding interconnect provider\n");
> >>>> +               return ret;
> >>>> +       }
> >>>> +
> >>>> +       for (i = 0; i < num_nodes; i++) {
> >>>> +               size_t j;
> >>>> +
> >>>> +               node = icc_node_create(qnodes[i]->id);
> >>>> +               if (IS_ERR(node)) {
> >>>> +                       ret = PTR_ERR(node);
> >>>> +                       goto err;
> >>>> +               }
> >>>> +
> >>>> +               node->name = qnodes[i]->name;
> >>>> +               node->data = qnodes[i];
> >>>> +               icc_node_add(node, provider);
> >>>> +
> >>>> +               dev_dbg(&pdev->dev, "registered node %p %s %d\n", node,
> >>>> +                       qnodes[i]->name, node->id);
> >>>> +
> >>>> +               /* populate links */
> >>>> +               for (j = 0; j < qnodes[i]->num_links; j++)
> >>>> +                       icc_link_create(node, qnodes[i]->links[j]);
> >>>> +
> >>>> +               data->nodes[i] = node;
> >>>> +       }
> >>>> +       data->num_nodes = num_nodes;
> >>>> +
> >>>> +       platform_set_drvdata(pdev, qp);
> >>>> +
> >>>> +       return ret;
> >>>> +err:
> >>>> +       list_for_each_entry(node, &provider->nodes, node_list) {
> >>>> +               icc_node_del(node);
> >>>> +               icc_node_destroy(node->id);
> >>>> +       }
> >>>> +
> >>>> +       icc_provider_del(provider);
> >>>> +       return ret;
> >>>> +}
> >>>> +
> >>>> +static int qcom_osm_l3_remove(struct platform_device *pdev)
> >>>> +{
> >>>> +       struct qcom_icc_provider *qp = platform_get_drvdata(pdev);
> >>>> +       struct icc_provider *provider = &qp->provider;
> >>>> +       struct icc_node *n;
> >>>> +
> >>>> +       list_for_each_entry(n, &provider->nodes, node_list) {
> >>>> +               icc_node_del(n);
> >>>> +               icc_node_destroy(n->id);
> >>>> +       }
> >>>> +
> >>>> +       return icc_provider_del(provider);
> >>>> +}
> >>>> +
> >>>> +static const struct of_device_id osm_l3_of_match[] = {
> >>>> +       { .compatible = "qcom,sdm845-osm-l3", .data = &sdm845_osm_l3 },
> >>>> +       { },
> >>>> +};
> >>>> +MODULE_DEVICE_TABLE(of, osm_l3_of_match);
> >>>> +
> >>>> +static struct platform_driver osm_l3_driver = {
> >>>> +       .probe = qcom_osm_l3_probe,
> >>>> +       .remove = qcom_osm_l3_remove,
> >>>> +       .driver = {
> >>>> +               .name = "osm-l3",
> >>>> +               .of_match_table = osm_l3_of_match,
> >>>> +       },
> >>>> +};
> >>>> +module_platform_driver(osm_l3_driver);
> >>>> +
> >>>> +MODULE_DESCRIPTION("Qualcomm OSM L3 interconnect driver");
> >>>> +MODULE_LICENSE("GPL v2");
> >>>
> >>> Did a quick scan of the code and it's not clear how you connect the L3
> >>> interconnect provider to the rest of the interconnect. I don't see any
> >>> DT or code references to the rest of the interconnects in the system.
> >>> If GPU is making a bandwidth request all the way to L3, how do you
> >>> make sure the interconnects between GPU and L3 also scale up properly.
> >>
> >>For SDM845 OSM L3 provider the icc nodes endpoints are isolated from
> >>rsc icc node endpoints i.e GPU would need to vote on this path in
> >>addition to voting for DDR. On future SoCs if the need to scale
> >>interconnect between GPU rsc nodes along with the OSM l3 nodes arises,
> >>it can be trivially extended by linking the osm icc nodes with global
> >>icc node ids of the gpu rsc nodes.
> >
> >Sorry, I'm not sure we are talking about the same thing. To keep the
> >discussion focused, let's completely ignore GPU's DDR needs for now.
> >
> >Talking about GPU <-> L3 traffic, unless the GPU is directly connected
> >to the L3 with no additional interconnects in between (unlikely), this
> >L3 provider need to connect the L3 icc node with the rest of the
> >interconnect nodes. Otherwise, even if the L3 itself is provisioned to
> >handle, say 10 GB/s, the interconnects connecting the GPU to L3 might
> >be left at 1 GB/s.
> >
> >So, can we please connect this node to the rest of the interconnect
> >nodes in SDM845?
> 
> Sure I'll double check if GPU has a l3
> scaling requirement on SDM845 SoCs and
> drop/link it to the rsc nodes appropriately.

GPU doesn't use I/O coherency at the moment and I don't think we would need an
independent connection if we did.

Jordan

> >
> >Thanks,
> >Saravana
> 
> -- 
> -- Sibi Sankar --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project.

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

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

end of thread, other threads:[~2019-08-14 15:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-07 11:24 [PATCH 0/2] Add OSM L3 Interconnect Provider Sibi Sankar
2019-08-07 11:24 ` [PATCH 1/2] dt-bindings: interconnect: Add OSM L3 DT bindings Sibi Sankar
2019-08-14 11:34   ` Sibi Sankar
2019-08-07 11:24 ` [PATCH 2/2] interconnect: qcom: Add OSM L3 interconnect provider support Sibi Sankar
2019-08-07 21:21   ` Saravana Kannan
2019-08-08 17:36     ` Sibi Sankar
2019-08-14  0:43       ` Saravana Kannan
2019-08-14 11:30         ` Sibi Sankar
2019-08-14 15:21           ` Jordan Crouse
2019-08-13 20:32   ` Evan Green
2019-08-14 11:27     ` Sibi Sankar

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).