[RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

This RFC series aims to extend cpu based scaling support to L3/DDR on
SDM845 and SC7180 SoCs.

Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
Patches [5-7] - Hack in a way to add/remove multiple opp tables to
                a single device. I am yet to fix the debugfs to
		support multiple opp_tables per device but wanted to
		send what was working upstream to get an idea if multiple
		opp tables per device is a feature that will be useful
		upstream.
Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
                 and SC7180 SoCs.

v3:
 * Migrated to using Saravana's opp-kBps bindings [1]
 * Fixed some misc comments from Rajendra
 * Added support for SC7180

v2:
 * Incorporated Viresh's comments from:
 https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
 https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
 * Dropped cpufreq-map passive governor

Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420

Some alternate ways of hosting the opp-tables:
https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
Viresh didn't really like ^^ bindings and they dont really scale well. Just
including them here for completeness.

Depends on the following series:
[1] https://patchwork.kernel.org/cover/11277199/
[2] https://patchwork.kernel.org/cover/11055499/ 
[3] https://patchwork.kernel.org/cover/11326381/

Sibi Sankar (10):
  arm64: dts: qcom: sdm845: Add SoC compatible to MTP
  cpufreq: blacklist SDM845 in cpufreq-dt-platdev
  cpufreq: blacklist SC7180 in cpufreq-dt-platdev
  OPP: Add and export helper to update voltage
  opp: of: export _opp_of_get_opp_desc_node
  opp: Allow multiple opp_tables to be mapped to a single device
  opp: Remove multiple attached opp tables from a device
  cpufreq: qcom: Update the bandwidth levels on frequency change
  arm64: dts: qcom: sdm845: Add cpu OPP tables
  arm64: dts: qcom: sc7180: Add cpu OPP tables

 arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
 arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
 drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
 drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
 drivers/opp/core.c                      | 111 +++++-
 drivers/opp/of.c                        |   3 +-
 drivers/opp/opp.h                       |   2 +
 include/linux/pm_opp.h                  |  10 +
 9 files changed, 1083 insertions(+), 33 deletions(-)

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

[RFC v3 01/10] arm64: dts: qcom: sdm845: Add SoC compatible to MTP

[Sibi Sankar]

Add missing SoC compatible to SDM845 MTP board file.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
index 09ad37b0dd71d..54087847794aa 100644
--- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
@@ -13,7 +13,7 @@
 
 / {
 	model = "Qualcomm Technologies, Inc. SDM845 MTP";
-	compatible = "qcom,sdm845-mtp";
+	compatible = "qcom,sdm845-mtp", "qcom,sdm845";
 
 	aliases {
 		serial0 = &uart9;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev

[Sibi Sankar]

Add SDM845 to cpufreq-dt-platdev blacklist.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index f2ae9cd455c17..5492cf3c9dc18 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -130,6 +130,7 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "qcom,apq8096", },
 	{ .compatible = "qcom,msm8996", },
 	{ .compatible = "qcom,qcs404", },
+	{ .compatible = "qcom,sdm845", },
 
 	{ .compatible = "st,stih407", },
 	{ .compatible = "st,stih410", },
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 03/10] cpufreq: blacklist SC7180 in cpufreq-dt-platdev

[Sibi Sankar]

Add SC7180 to cpufreq-dt-platdev blacklist.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
index 5492cf3c9dc18..580abc777d9d8 100644
--- a/drivers/cpufreq/cpufreq-dt-platdev.c
+++ b/drivers/cpufreq/cpufreq-dt-platdev.c
@@ -130,6 +130,7 @@ static const struct of_device_id blacklist[] __initconst = {
 	{ .compatible = "qcom,apq8096", },
 	{ .compatible = "qcom,msm8996", },
 	{ .compatible = "qcom,qcs404", },
+	{ .compatible = "qcom,sc7180", },
 	{ .compatible = "qcom,sdm845", },
 
 	{ .compatible = "st,stih407", },
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 04/10] OPP: Add and export helper to update voltage

[Sibi Sankar]

Add and export 'dev_pm_opp_update_voltage' to find and update voltage
of an opp for a given frequency. This will be useful to update the opps
with voltages read back from firmware.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/opp/core.c     | 55 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm_opp.h | 10 ++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9aa2a44a5d638..f241e83ec926a 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq)
 }
 EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
 
+/**
+ * dev_pm_opp_update_voltage() - Find and update voltage
+ * @dev:	device for which we do this operation
+ * @freq:	OPP frequency to update voltage
+ * @u_volt:	voltage requested for this opp
+ *
+ * Find and update voltage of a disabled opp corresponding to the given
+ * frequency. This is useful only for devices with single power supply.
+ *
+ * Return: 0 if modification was successful or a negative error value.
+ */
+int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
+			      unsigned long u_volt)
+{
+	struct dev_pm_opp *opp = ERR_PTR(-ENODEV);
+	struct opp_table *opp_table;
+	unsigned long tol;
+	int ret = 0;
+
+	/* Find the opp_table */
+	opp_table = _find_opp_table(dev);
+	if (IS_ERR(opp_table)) {
+		ret = PTR_ERR(opp_table);
+		dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret);
+		return PTR_ERR(opp_table);
+	}
+
+	opp = dev_pm_opp_find_freq_exact(dev, freq, false);
+	if (IS_ERR(opp)) {
+		ret = PTR_ERR(opp);
+		goto put_table;
+	}
+
+	mutex_lock(&opp_table->lock);
+
+	/* update only if the opp is disabled */
+	if (opp->available) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
+	opp->supplies[0].u_volt_min = u_volt - tol;
+	opp->supplies[0].u_volt = u_volt;
+	opp->supplies[0].u_volt_min = u_volt + tol;
+
+unlock:
+	mutex_unlock(&opp_table->lock);
+	dev_pm_opp_put(opp);
+put_table:
+	dev_pm_opp_put_opp_table(opp_table);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(dev_pm_opp_update_voltage);
+
 /**
  * dev_pm_opp_register_notifier() - Register OPP notifier for the device
  * @dev:	Device for which notifier needs to be registered
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 62c88898bae46..b26d492cbd635 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -132,6 +132,9 @@ int dev_pm_opp_enable(struct device *dev, unsigned long freq);
 
 int dev_pm_opp_disable(struct device *dev, unsigned long freq);
 
+int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
+			      unsigned long u_volt);
+
 int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb);
 int dev_pm_opp_unregister_notifier(struct device *dev, struct notifier_block *nb);
 
@@ -311,6 +314,13 @@ static inline int dev_pm_opp_disable(struct device *dev, unsigned long freq)
 	return 0;
 }
 
+static inline int dev_pm_opp_update_voltage(struct device *dev,
+					    unsigned long freq,
+					    unsigned long u_volt)
+{
+	return 0;
+}
+
 static inline int dev_pm_opp_register_notifier(struct device *dev, struct notifier_block *nb)
 {
 	return -ENOTSUPP;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 05/10] opp: of: export _opp_of_get_opp_desc_node

[Sibi Sankar]

Export _opp_of_get_opp_desc_node in preparation for attaching multiple
opp_table per device.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/opp/of.c  | 3 +--
 drivers/opp/opp.h | 2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index 43acf54934be7..015ee9ab09485 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -25,8 +25,7 @@
  * Returns opp descriptor node for a device node, caller must
  * do of_node_put().
  */
-static struct device_node *_opp_of_get_opp_desc_node(struct device_node *np,
-						     int index)
+struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index)
 {
 	/* "operating-points-v2" can be an array for power domain providers */
 	return of_parse_phandle(np, "operating-points-v2", index);
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index 9e021f819acca..6f56ff6038605 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -226,12 +226,14 @@ void _put_opp_list_kref(struct opp_table *opp_table);
 void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index);
 void _of_clear_opp_table(struct opp_table *opp_table);
 struct opp_table *_managed_opp(struct device *dev, int index);
+struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index);
 void _of_opp_free_required_opps(struct opp_table *opp_table,
 				struct dev_pm_opp *opp);
 #else
 static inline void _of_init_opp_table(struct opp_table *opp_table, struct device *dev, int index) {}
 static inline void _of_clear_opp_table(struct opp_table *opp_table) {}
 static inline struct opp_table *_managed_opp(struct device *dev, int index) { return NULL; }
+static inline struct device_node *_opp_of_get_opp_desc_node(struct device_node *np, int index) { return NULL; }
 static inline void _of_opp_free_required_opps(struct opp_table *opp_table,
 					      struct dev_pm_opp *opp) {}
 #endif
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device

[Sibi Sankar]

Introduce _find_opp_table_indexed and its unlocked variant to allow for
multiple distinct opp_tables to be linked to a single device.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/opp/core.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index f241e83ec926a..e9d633c9e40b1 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -46,19 +46,26 @@ static struct opp_device *_find_opp_dev(const struct device *dev,
 	return NULL;
 }
 
-static struct opp_table *_find_opp_table_unlocked(struct device *dev)
+static struct opp_table *_find_opp_table_indexed_unlocked(struct device *dev,
+							  int index)
 {
 	struct opp_table *opp_table;
+	struct device_node *np;
 	bool found;
 
+	np = _opp_of_get_opp_desc_node(dev->of_node, index);
+	of_node_put(np);
+
 	list_for_each_entry(opp_table, &opp_tables, node) {
 		mutex_lock(&opp_table->lock);
 		found = !!_find_opp_dev(dev, opp_table);
 		mutex_unlock(&opp_table->lock);
 
 		if (found) {
-			_get_opp_table_kref(opp_table);
+			if (np && opp_table->np != np)
+				continue;
 
+			_get_opp_table_kref(opp_table);
 			return opp_table;
 		}
 	}
@@ -67,17 +74,19 @@ static struct opp_table *_find_opp_table_unlocked(struct device *dev)
 }
 
 /**
- * _find_opp_table() - find opp_table struct using device pointer
+ * _find_opp_table_indexed() - find opp_table struct using device pointer
  * @dev:	device pointer used to lookup OPP table
+ * @index:	Index number.
  *
- * Search OPP table for one containing matching device.
+ * Search OPP table for one containing matching device at the given
+ * index.
  *
  * Return: pointer to 'struct opp_table' if found, otherwise -ENODEV or
  * -EINVAL based on type of error.
  *
  * The callers must call dev_pm_opp_put_opp_table() after the table is used.
  */
-struct opp_table *_find_opp_table(struct device *dev)
+struct opp_table *_find_opp_table_indexed(struct device *dev, int index)
 {
 	struct opp_table *opp_table;
 
@@ -87,12 +96,28 @@ struct opp_table *_find_opp_table(struct device *dev)
 	}
 
 	mutex_lock(&opp_table_lock);
-	opp_table = _find_opp_table_unlocked(dev);
+	opp_table = _find_opp_table_indexed_unlocked(dev, index);
 	mutex_unlock(&opp_table_lock);
 
 	return opp_table;
 }
 
+/**
+ * _find_opp_table() - find opp_table struct using device pointer
+ * @dev:	device pointer used to lookup OPP table
+ *
+ * Search OPP table for one containing matching device.
+ *
+ * Return: pointer to 'struct opp_table' if found, otherwise -ENODEV or
+ * -EINVAL based on type of error.
+ *
+ * The callers must call dev_pm_opp_put_opp_table() after the table is used.
+ */
+struct opp_table *_find_opp_table(struct device *dev)
+{
+	return _find_opp_table_indexed(dev, 0);
+}
+
 /**
  * dev_pm_opp_get_voltage() - Gets the voltage corresponding to an opp
  * @opp:	opp for which voltage has to be returned for
@@ -1238,7 +1263,7 @@ static struct opp_table *_opp_get_opp_table(struct device *dev, int index)
 	/* Hold our table modification lock here */
 	mutex_lock(&opp_table_lock);
 
-	opp_table = _find_opp_table_unlocked(dev);
+	opp_table = _find_opp_table_indexed_unlocked(dev, index);
 	if (!IS_ERR(opp_table))
 		goto unlock;
 
@@ -2612,7 +2637,7 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev)
 	struct opp_table *opp_table;
 
 	/* Check for existing table for 'dev' */
-	opp_table = _find_opp_table(dev);
+	opp_table = _find_opp_table_indexed(dev, 0);
 	if (IS_ERR(opp_table)) {
 		int error = PTR_ERR(opp_table);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 07/10] opp: Remove multiple attached opp tables from a device

[Sibi Sankar]

Introduce _dev_pm_opp_find_and_remove_table_indexed to remove
all the linked opp tables from a device.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/opp/core.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index e9d633c9e40b1..cc2f156db7fda 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2632,12 +2632,13 @@ int dev_pm_opp_unregister_notifier(struct device *dev,
 }
 EXPORT_SYMBOL(dev_pm_opp_unregister_notifier);
 
-void _dev_pm_opp_find_and_remove_table(struct device *dev)
+static void _dev_pm_opp_find_and_remove_table_indexed(struct device *dev,
+						      int index)
 {
 	struct opp_table *opp_table;
 
 	/* Check for existing table for 'dev' */
-	opp_table = _find_opp_table_indexed(dev, 0);
+	opp_table = _find_opp_table_indexed(dev, index);
 	if (IS_ERR(opp_table)) {
 		int error = PTR_ERR(opp_table);
 
@@ -2658,6 +2659,18 @@ void _dev_pm_opp_find_and_remove_table(struct device *dev)
 	dev_pm_opp_put_opp_table(opp_table);
 }
 
+void _dev_pm_opp_find_and_remove_table(struct device *dev)
+{
+	int count, i;
+
+	count = of_count_phandle_with_args(dev->of_node,
+					   "operating-points-v2", NULL);
+	count = max(count, 1);
+
+	for (i = 0; i < count; i++)
+		_dev_pm_opp_find_and_remove_table_indexed(dev, i);
+}
+
 /**
  * dev_pm_opp_remove_table() - Free all OPPs associated with the device
  * @dev:	device pointer used to lookup OPP table.
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 08/10] cpufreq: qcom: Update the bandwidth levels on frequency change

[Sibi Sankar]

Add support to parse and update OPP tables attached to the cpu device
when the required OPP bandwidth values are populated to enable scaling
of DDR/L3 bandwidth levels with frequency change.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++++++++++++++++++---
 1 file changed, 225 insertions(+), 21 deletions(-)

diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
index fc92a8842e252..348eb2fdaccaf 100644
--- a/drivers/cpufreq/qcom-cpufreq-hw.c
+++ b/drivers/cpufreq/qcom-cpufreq-hw.c
@@ -6,6 +6,7 @@
 #include <linux/bitfield.h>
 #include <linux/cpufreq.h>
 #include <linux/init.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of_address.h>
@@ -28,17 +29,194 @@
 #define REG_VOLT_LUT			0x114
 #define REG_PERF_STATE			0x920
 
+#define QCOM_ICC_TAG_ACTIVE_ONLY	1
+
 static unsigned long cpu_hw_rate, xo_rate;
 static struct platform_device *global_pdev;
 
+/* opp table indices */
+enum {
+	QCOM_CPU_OPP_TABLE_INDEX,
+	QCOM_CPU_DDR_OPP_TABLE_INDEX,
+	QCOM_CPU_L3_OPP_TABLE_INDEX,
+};
+
+/* icc path indices */
+enum {
+	ICC_DDR_PATH,
+	ICC_L3_PATH,
+};
+
+struct qcom_cpufreq_hw_res {
+	void __iomem *base;
+
+	struct device *cpu_dev;
+
+	/* ddr/l3 icc paths */
+	struct icc_path *path[2];
+
+	/* cpu/ddr/l3 opp tables */
+	struct opp_table *opp_table[3];
+};
+
+struct cpufreq_hw_icc_info {
+	const char *icc_path_name;
+	u8 table_index;
+	u32 tag;
+};
+
+static const struct cpufreq_hw_icc_info icc_info[] = {
+	{
+		.icc_path_name = "cpu-ddr",
+		.table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX,
+		.tag = QCOM_ICC_TAG_ACTIVE_ONLY,
+	},
+	{
+		.icc_path_name = "cpu-l3",
+		.table_index = QCOM_CPU_L3_OPP_TABLE_INDEX,
+		.tag = 0,
+	},
+};
+
+static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res *res)
+{
+	struct opp_table **table = res->opp_table;
+	struct device_node *opp_table_np, *np;
+	struct device *dev = res->cpu_dev;
+	int ret, i;
+	u64 rate;
+
+	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
+		ret = dev_pm_opp_of_add_table_indexed(dev, i);
+		if (ret) {
+			dev_dbg(dev, "Add OPP table failed index %d: %d\n",
+				i, ret);
+			goto err;
+		}
+
+		table[i] = dev_pm_opp_get_opp_table_indexed(dev, i);
+		if (!table[i]) {
+			dev_dbg(dev, "Get OPP table failed index %d\n", i);
+			ret = -EINVAL;
+			goto err;
+		}
+	}
+
+	/* Disable all cpu opps and cross-validate against LUT */
+	opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev);
+	for_each_available_child_of_node(opp_table_np, np) {
+		ret = of_property_read_u64(np, "opp-hz", &rate);
+		if (ret)
+			continue;
+
+		dev_pm_opp_disable(dev, rate);
+	}
+	of_node_put(opp_table_np);
+
+	return 0;
+
+err:
+	for (; i >= 0; i--) {
+		if (table[i]) {
+			dev_pm_opp_put_opp_table(table[i]);
+			table[i] = NULL;
+		}
+	}
+
+	dev_pm_opp_remove_table(dev);
+	return ret;
+}
+
+static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, u8 index)
+{
+	const struct cpufreq_hw_icc_info *info = &icc_info[index];
+	struct icc_path **path = res->path;
+	struct device *dev = res->cpu_dev;
+
+	path[index] = of_icc_get(dev, info->icc_path_name);
+	if (IS_ERR(path[index])) {
+		dev_dbg(dev, "ICC %s path get failed ret: %ld\n",
+			info->icc_path_name, PTR_ERR(path[index]));
+		return PTR_ERR(path[index]);
+	}
+
+	icc_set_tag(path[index], info->tag);
+
+	return 0;
+}
+
+static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res,
+				    unsigned long freq)
+{
+	struct opp_table **table = res->opp_table;
+	const struct cpufreq_hw_icc_info *info;
+	unsigned long freq_hz = freq * 1000;
+	struct icc_path **path = res->path;
+	struct device *dev = res->cpu_dev;
+	struct dev_pm_opp *cpu_opp, *opp;
+	struct opp_table *cpu_opp_table;
+	unsigned long peak_bw, avg_bw;
+	int ret;
+	int i;
+
+	cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX];
+	if (!cpu_opp_table)
+		return;
+
+	cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
+	if (IS_ERR_OR_NULL(cpu_opp))
+		return;
+
+	for (i = 0; i <= ICC_L3_PATH; i++) {
+		if (IS_ERR(path[i])) {
+			if (PTR_ERR(path[i]) != -EPROBE_DEFER)
+				continue;
+
+			ret = qcom_cpufreq_hw_icc_init(res, i);
+			if (ret)
+				continue;
+		}
+
+		info = &icc_info[i];
+
+		opp = dev_pm_opp_xlate_required_opp(cpu_opp_table,
+						    table[info->table_index],
+						    cpu_opp);
+		if (IS_ERR_OR_NULL(opp))
+			continue;
+
+		peak_bw = dev_pm_opp_get_bw(opp, &avg_bw);
+		dev_pm_opp_put(opp);
+
+		icc_set_bw(res->path[i], avg_bw, peak_bw);
+	}
+	dev_pm_opp_put(cpu_opp);
+}
+
+static int qcom_cpufreq_update_opp(struct device *dev,
+				   unsigned long freq_khz,
+				   unsigned long volt)
+{
+	unsigned long freq_hz = freq_khz * 1000;
+
+	if (dev_pm_opp_update_voltage(dev, freq_hz, volt))
+		return dev_pm_opp_add(dev, freq_hz, volt);
+
+	/* Enable the opp after voltage update*/
+	return dev_pm_opp_enable(dev, freq_hz);
+}
+
 static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 					unsigned int index)
 {
-	void __iomem *perf_state_reg = policy->driver_data;
+	struct qcom_cpufreq_hw_res *res = policy->driver_data;
+	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
 	unsigned long freq = policy->freq_table[index].frequency;
 
 	writel_relaxed(index, perf_state_reg);
 
+	qcom_cpufreq_hw_icc_set(res, freq);
+
 	arch_set_freq_scale(policy->related_cpus, freq,
 			    policy->cpuinfo.max_freq);
 	return 0;
@@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
 
 static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
 {
+	struct qcom_cpufreq_hw_res *res;
 	void __iomem *perf_state_reg;
 	struct cpufreq_policy *policy;
 	unsigned int index;
@@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
 	if (!policy)
 		return 0;
 
-	perf_state_reg = policy->driver_data;
+	res = policy->driver_data;
+	perf_state_reg = res->base + REG_PERF_STATE;
 
 	index = readl_relaxed(perf_state_reg);
 	index = min(index, LUT_MAX_ENTRIES - 1);
@@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
 static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
 						unsigned int target_freq)
 {
-	void __iomem *perf_state_reg = policy->driver_data;
+	struct qcom_cpufreq_hw_res *res = policy->driver_data;
+	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
 	int index;
 	unsigned long freq;
 
@@ -82,18 +263,24 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
 	return freq;
 }
 
-static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
-				    struct cpufreq_policy *policy,
-				    void __iomem *base)
+static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy,
+				    struct qcom_cpufreq_hw_res *res)
 {
 	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
 	u32 volt;
 	struct cpufreq_frequency_table	*table;
+	struct device *cpu_dev = res->cpu_dev;
+	void __iomem *base = res->base;
+	int ret;
 
 	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
 	if (!table)
 		return -ENOMEM;
 
+	ret = qcom_cpufreq_hw_add_opp_table(res);
+	if (ret)
+		dev_dbg(cpu_dev, "Add OPP tables failed from dt\n");
+
 	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
 		data = readl_relaxed(base + REG_FREQ_LUT +
 				      i * LUT_ROW_SIZE);
@@ -112,7 +299,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 
 		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
 			table[i].frequency = freq;
-			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
+			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
 			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
 				freq, core_count);
 		} else if (core_count == LUT_TURBO_IND) {
@@ -133,7 +320,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
 			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
 				prev->frequency = prev_freq;
 				prev->flags = CPUFREQ_BOOST_FREQ;
-				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
+				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
+							volt);
 			}
 
 			break;
@@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
 static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 {
 	struct device *dev = &global_pdev->dev;
+	struct qcom_cpufreq_hw_res *res;
 	struct of_phandle_args args;
 	struct device_node *cpu_np;
 	struct device *cpu_dev;
-	struct resource *res;
-	void __iomem *base;
 	int ret, index;
 
 	cpu_dev = get_cpu_device(policy->cpu);
@@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 	index = args.args[0];
 
-	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
+	res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
 	if (!res)
-		return -ENODEV;
-
-	base = devm_ioremap(dev, res->start, resource_size(res));
-	if (!base)
 		return -ENOMEM;
 
+	res->cpu_dev = cpu_dev;
+	res->base = devm_platform_ioremap_resource(global_pdev, index);
+	if (IS_ERR(res->base))
+		return PTR_ERR(res->base);
+
 	/* HW should be in enabled state to proceed */
-	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
+	if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) {
 		dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
 		ret = -ENODEV;
 		goto error;
@@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 		goto error;
 	}
 
-	policy->driver_data = base + REG_PERF_STATE;
+	policy->driver_data = res;
 
-	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base);
+	ret = qcom_cpufreq_hw_read_lut(policy, res);
 	if (ret) {
 		dev_err(dev, "Domain-%d failed to read LUT\n", index);
 		goto error;
@@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
 
 	dev_pm_opp_of_register_em(policy->cpus);
 
-	policy->fast_switch_possible = true;
+	if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) {
+		policy->fast_switch_possible = true;
+	} else {
+		qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH);
+		qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH);
+	}
 
 	return 0;
 error:
-	devm_iounmap(dev, base);
+	devm_iounmap(dev, res->base);
+	devm_kfree(&global_pdev->dev, res);
 	return ret;
 }
 
 static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
 {
+	struct qcom_cpufreq_hw_res *res = policy->driver_data;
 	struct device *cpu_dev = get_cpu_device(policy->cpu);
-	void __iomem *base = policy->driver_data - REG_PERF_STATE;
+	struct opp_table **table = res->opp_table;
+	void __iomem *base = res->base;
+	int i;
+
+	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
+		if (table[i])
+			dev_pm_opp_put_opp_table(table[i]);
+	}
 
 	dev_pm_opp_remove_all_dynamic(cpu_dev);
 	kfree(policy->freq_table);
+	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
 	devm_iounmap(&global_pdev->dev, base);
+	devm_kfree(&global_pdev->dev, res);
 
 	return 0;
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 09/10] arm64: dts: qcom: sdm845: Add cpu OPP tables

[Sibi Sankar]

Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 +++++++++++++++++++++++++++
 1 file changed, 453 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index c036bab49fc03..8cb976118407b 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -199,6 +199,12 @@
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_0>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_0: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -221,6 +227,12 @@
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_100>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_100: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -240,6 +252,12 @@
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_200>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_200: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -259,6 +277,12 @@
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_300>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_300: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -278,6 +302,12 @@
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_400>;
+			operating-points-v2 = <&cpu4_opp_table>,
+					      <&cpu4_ddr_bw_opp_table>,
+					      <&cpu4_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_400: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -297,6 +327,12 @@
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_500>;
+			operating-points-v2 = <&cpu4_opp_table>,
+					      <&cpu4_ddr_bw_opp_table>,
+					      <&cpu4_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_500: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -316,6 +352,12 @@
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_600>;
+			operating-points-v2 = <&cpu4_opp_table>,
+					      <&cpu4_ddr_bw_opp_table>,
+					      <&cpu4_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_600: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -335,6 +377,12 @@
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			#cooling-cells = <2>;
 			next-level-cache = <&L2_700>;
+			operating-points-v2 = <&cpu4_opp_table>,
+					      <&cpu4_ddr_bw_opp_table>,
+					      <&cpu4_l3_bw_opp_table>;
+			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			L2_700: l2-cache {
 				compatible = "cache";
 				next-level-cache = <&L3_0>;
@@ -432,6 +480,231 @@
 		};
 	};
 
+	cpu0_opp_table: cpu0_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu0_opp1: opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>;
+		};
+
+		cpu0_opp2: opp-403200000 {
+			opp-hz = /bits/ 64 <403200000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>;
+		};
+
+		cpu0_opp3: opp-480000000 {
+			opp-hz = /bits/ 64 <480000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp2>;
+		};
+
+		cpu0_opp4: opp-576000000 {
+			opp-hz = /bits/ 64 <576000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp2>;
+		};
+
+		cpu0_opp5: opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp3>;
+		};
+
+		cpu0_opp6: opp-748800000 {
+			opp-hz = /bits/ 64 <748800000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp4>;
+		};
+
+		cpu0_opp7: opp-825600000 {
+			opp-hz = /bits/ 64 <825600000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp4>;
+		};
+
+		cpu0_opp8: opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp5>;
+		};
+
+		cpu0_opp9: opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp6>;
+		};
+
+		cpu0_opp10: opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp6>;
+		};
+
+		cpu0_opp11: opp-1132800000 {
+			opp-hz = /bits/ 64 <1132800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp7>;
+		};
+
+		cpu0_opp12: opp-1228800000 {
+			opp-hz = /bits/ 64 <1228800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp8>;
+		};
+
+		cpu0_opp13: opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp9>;
+		};
+
+		cpu0_opp14: opp-1420800000 {
+			opp-hz = /bits/ 64 <1420800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp10>;
+		};
+
+		cpu0_opp15: opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			required-opps = <&cpu0_ddr_bw_opp5>, <&cpu0_l3_bw_opp11>;
+		};
+
+		cpu0_opp16: opp-1612800000 {
+			opp-hz = /bits/ 64 <1612800000>;
+			required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp11>;
+		};
+
+		cpu0_opp17: opp-1689600000 {
+			opp-hz = /bits/ 64 <1689600000>;
+			required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp12>;
+		};
+
+		cpu0_opp18: opp-1766400000 {
+			opp-hz = /bits/ 64 <1766400000>;
+			required-opps = <&cpu0_ddr_bw_opp7>, <&cpu0_l3_bw_opp13>;
+		};
+	};
+
+	cpu4_opp_table: cpu4_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu4_opp1: opp-825600000 {
+			opp-hz = /bits/ 64 <825600000>;
+			required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>;
+		};
+
+		cpu4_opp2: opp-902400000 {
+			opp-hz = /bits/ 64 <902400000>;
+			required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>;
+		};
+
+		cpu4_opp3: opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			required-opps = <&cpu4_ddr_bw_opp1>, <&cpu4_l3_bw_opp1>;
+		};
+
+		cpu4_opp4: opp-1056000000 {
+			opp-hz = /bits/ 64 <1056000000>;
+			required-opps = <&cpu4_ddr_bw_opp5>, <&cpu4_l3_bw_opp3>;
+		};
+
+		cpu4_opp5: opp-1209600000 {
+			opp-hz = /bits/ 64 <1209600000>;
+			required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp3>;
+		};
+
+		cpu4_opp6: opp-1286400000 {
+			opp-hz = /bits/ 64 <1286400000>;
+			required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp3>;
+		};
+
+		cpu4_opp7: opp-1363200000 {
+			opp-hz = /bits/ 64 <1363200000>;
+			required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>;
+		};
+
+		cpu4_opp8: opp-1459200000 {
+			opp-hz = /bits/ 64 <1459200000>;
+			required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>;
+		};
+
+		cpu4_opp9: opp-1536000000 {
+			opp-hz = /bits/ 64 <1536000000>;
+			required-opps = <&cpu4_ddr_bw_opp6>, <&cpu4_l3_bw_opp5>;
+		};
+
+		cpu4_opp10: opp-1612800000 {
+			opp-hz = /bits/ 64 <1612800000>;
+			required-opps = <&cpu4_ddr_bw_opp7>, <&cpu4_l3_bw_opp5>;
+		};
+
+		cpu4_opp11: opp-1689600000 {
+			opp-hz = /bits/ 64 <1689600000>;
+			required-opps = <&cpu4_ddr_bw_opp7>, <&cpu4_l3_bw_opp8>;
+		};
+
+		cpu4_opp12: opp-1766400000 {
+			opp-hz = /bits/ 64 <1766400000>;
+			required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>;
+		};
+
+		cpu4_opp13: opp-1843200000 {
+			opp-hz = /bits/ 64 <1843200000>;
+			required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>;
+		};
+
+		cpu4_opp14: opp-1920000000 {
+			opp-hz = /bits/ 64 <1920000000>;
+			required-opps = <&cpu4_ddr_bw_opp8>, <&cpu4_l3_bw_opp8>;
+		};
+
+		cpu4_opp15: opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>;
+		};
+
+		cpu4_opp16: opp-2092800000 {
+			opp-hz = /bits/ 64 <2092800000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>;
+		};
+
+		cpu4_opp17: opp-2169600000 {
+			opp-hz = /bits/ 64 <2169600000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>;
+		};
+
+		cpu4_opp18: opp-2246400000 {
+			opp-hz = /bits/ 64 <2246400000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>;
+		};
+
+		cpu4_opp19: opp-2323200000 {
+			opp-hz = /bits/ 64 <2323200000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp9>;
+		};
+
+		cpu4_opp20: opp-2400000000 {
+			opp-hz = /bits/ 64 <2400000000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>;
+		};
+
+		cpu4_opp21: opp-2476800000 {
+			opp-hz = /bits/ 64 <2476800000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>;
+		};
+
+		cpu4_opp22: opp-2553600000 {
+			opp-hz = /bits/ 64 <2553600000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>;
+		};
+
+		cpu4_opp23: opp-2649600000 {
+			opp-hz = /bits/ 64 <2649600000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp10>;
+		};
+
+		cpu4_opp24: opp-2745600000 {
+			opp-hz = /bits/ 64 <2745600000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp11>;
+		};
+
+		cpu4_opp25: opp-2803200000 {
+			opp-hz = /bits/ 64 <2803200000>;
+			required-opps = <&cpu4_ddr_bw_opp9>, <&cpu4_l3_bw_opp11>;
+		};
+	};
+
 	pmu {
 		compatible = "arm,armv8-pmuv3";
 		interrupts = <GIC_PPI 5 IRQ_TYPE_LEVEL_HIGH>;
@@ -3345,6 +3618,186 @@
 			#freq-domain-cells = <1>;
 		};
 
+		cpu0_ddr_bw_opp_table: cpu0-ddr-bw-opp-table {
+			compatible = "operating-points-v2";
+			opp-shared;
+
+			cpu0_ddr_bw_opp1: opp-200000000  {
+				opp-peak-kBps =/bits/ 32 <800000>;
+			};
+
+			cpu0_ddr_bw_opp2: opp-300000000 {
+				opp-peak-kBps = /bits/ 32 <1200000>;
+			};
+
+			cpu0_ddr_bw_opp3: opp-451000000 {
+				opp-peak-kBps = /bits/ 32 <1804000>;
+			};
+
+			cpu0_ddr_bw_opp4: opp-547000000 {
+				opp-peak-kBps = /bits/ 32 <2188000>;
+			};
+
+			cpu0_ddr_bw_opp5: opp-681000000 {
+				opp-peak-kBps = /bits/ 32 <2724000>;
+			};
+
+			cpu0_ddr_bw_opp6: opp-768000000 {
+				opp-peak-kBps = /bits/ 32 <3072000>;
+			};
+
+			cpu0_ddr_bw_opp7: opp-1017000000 {
+				opp-peak-kBps = /bits/ 32 <4068000>;
+			};
+		};
+
+		cpu0_l3_bw_opp_table: cpu0-l3-bw-opp-table {
+			compatible = "operating-points-v2";
+			opp-shared;
+
+			cpu0_l3_bw_opp1: opp-300000000 {
+				opp-peak-kBps = /bits/ 32 <4800000>;
+			};
+
+			cpu0_l3_bw_opp2: opp-403200000 {
+				opp-peak-kBps = /bits/ 32 <6451200>;
+			};
+
+			cpu0_l3_bw_opp3: opp-480000000 {
+				opp-peak-kBps = /bits/ 32 <7680000>;
+			};
+
+			cpu0_l3_bw_opp4: opp-576000000 {
+				opp-peak-kBps = /bits/ 32 <9216000>;
+			};
+
+			cpu0_l3_bw_opp5: opp-652800000 {
+				opp-peak-kBps = /bits/ 32 <10444800>;
+			};
+
+			cpu0_l3_bw_opp6: opp-748800000 {
+				opp-peak-kBps = /bits/ 32 <11980800>;
+			};
+
+			cpu0_l3_bw_opp7: opp-844800000 {
+				opp-peak-kBps = /bits/ 32 <13516800>;
+			};
+
+			cpu0_l3_bw_opp8: opp-940800000 {
+				opp-peak-kBps = /bits/ 32 <15052800>;
+			};
+
+			cpu0_l3_bw_opp9: opp-1036800000 {
+				opp-peak-kBps = /bits/ 32 <16588800>;
+			};
+
+			cpu0_l3_bw_opp10: opp-1132800000 {
+				opp-peak-kBps = /bits/ 32 <18124800>;
+			};
+
+			cpu0_l3_bw_opp11: opp-1209600000 {
+				opp-peak-kBps = /bits/ 32 <19353600>;
+			};
+
+			cpu0_l3_bw_opp12: opp-1305600000 {
+				opp-peak-kBps = /bits/ 32 <20889600>;
+			};
+
+			cpu0_l3_bw_opp13: opp-1401600000 {
+				opp-peak-kBps = /bits/ 32 <22425600>;
+			};
+		};
+
+		cpu4_ddr_bw_opp_table: cpu4-ddr-bw-opp-table {
+			compatible = "operating-points-v2";
+			opp-shared;
+
+			cpu4_ddr_bw_opp1: opp-300000000 {
+				opp-peak-kBps = /bits/ 32 <1200000>;
+			};
+
+			cpu4_ddr_bw_opp2: opp-451000000 {
+				opp-peak-kBps = /bits/ 32 <1804000>;
+			};
+
+			cpu4_ddr_bw_opp3: opp-547000000 {
+				opp-peak-kBps = /bits/ 32 <2188000>;
+			};
+
+			cpu4_ddr_bw_opp4: opp-681000000 {
+				opp-peak-kBps = /bits/ 32 <2724000>;
+			};
+
+			cpu4_ddr_bw_opp5: opp-768000000 {
+				opp-peak-kBps = /bits/ 32 <3072000>;
+			};
+
+			cpu4_ddr_bw_opp6: opp-1017000000 {
+				opp-peak-kBps = /bits/ 32 <4068000>;
+			};
+
+			cpu4_ddr_bw_opp7: opp-1296000000 {
+				opp-peak-kBps = /bits/ 32 <5184000>;
+			};
+
+			cpu4_ddr_bw_opp8: opp-1555000000 {
+				opp-peak-kBps = /bits/ 32 <6220000>;
+			};
+
+			cpu4_ddr_bw_opp9: opp-1804000000 {
+				opp-peak-kBps = /bits/ 32 <7216000>;
+			};
+		};
+
+		cpu4_l3_bw_opp_table: cpu4-l3-bw-opp-table {
+			compatible = "operating-points-v2";
+			opp-shared;
+
+			cpu4_l3_bw_opp1: opp-576000000 {
+				opp-peak-kBps = /bits/ 32 <9216000>;
+			};
+
+			cpu4_l3_bw_opp2: opp-652800000 {
+				opp-peak-kBps = /bits/ 32 <10444800>;
+			};
+
+			cpu4_l3_bw_opp3: opp-748800000 {
+				opp-peak-kBps = /bits/ 32 <11980800>;
+			};
+
+			cpu4_l3_bw_opp4: opp-844800000 {
+				opp-peak-kBps = /bits/ 32 <13516800>;
+			};
+
+			cpu4_l3_bw_opp5: opp-940800000 {
+				opp-peak-kBps = /bits/ 32 <15052800>;
+			};
+
+			cpu4_l3_bw_opp6: opp-1036800000 {
+				opp-peak-kBps = /bits/ 32 <16588800>;
+			};
+
+			cpu4_l3_bw_opp7: opp-1132800000 {
+				opp-peak-kBps = /bits/ 32 <18124800>;
+			};
+
+			cpu4_l3_bw_opp8: opp-1209600000 {
+				opp-peak-kBps = /bits/ 32 <19353600>;
+			};
+
+			cpu4_l3_bw_opp9: opp-1305600000 {
+				opp-peak-kBps = /bits/ 32 <20889600>;
+			};
+
+			cpu4_l3_bw_opp10: opp-1401600000 {
+				opp-peak-kBps = /bits/ 32 <22425600>;
+			};
+
+			cpu4_l3_bw_opp11: opp-1478400000 {
+				opp-peak-kBps = /bits/ 32 <23654400>;
+			};
+		};
+
 		wifi: wifi@18800000 {
 			compatible = "qcom,wcn3990-wifi";
 			status = "disabled";
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[RFC v3 10/10] arm64: dts: qcom: sc7180: Add cpu OPP tables

[Sibi Sankar]

Add OPP tables required to scale DDR/L3 per freq-domain on SC7180 SoCs.

Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---
 arch/arm64/boot/dts/qcom/sc7180.dtsi | 287 +++++++++++++++++++++++++++
 1 file changed, 287 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index ba53ddf17ee3a..699cafc1a727d 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -109,6 +109,12 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			next-level-cache = <&L2_0>;
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
@@ -129,6 +135,12 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
 			next-level-cache = <&L2_100>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			L2_100: l2-cache {
@@ -145,6 +157,12 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
 			next-level-cache = <&L2_200>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			L2_200: l2-cache {
@@ -161,6 +179,12 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
 			next-level-cache = <&L2_300>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			L2_300: l2-cache {
@@ -177,6 +201,12 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
 			next-level-cache = <&L2_400>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			L2_400: l2-cache {
@@ -193,6 +223,12 @@
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <100>;
 			next-level-cache = <&L2_500>;
+			operating-points-v2 = <&cpu0_opp_table>,
+					      <&cpu0_ddr_bw_opp_table>,
+					      <&cpu0_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 0>;
 			L2_500: l2-cache {
@@ -209,6 +245,12 @@
 			capacity-dmips-mhz = <1740>;
 			dynamic-power-coefficient = <405>;
 			next-level-cache = <&L2_600>;
+			operating-points-v2 = <&cpu6_opp_table>,
+					      <&cpu6_ddr_bw_opp_table>,
+					      <&cpu6_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			L2_600: l2-cache {
@@ -225,6 +267,12 @@
 			capacity-dmips-mhz = <1740>;
 			dynamic-power-coefficient = <405>;
 			next-level-cache = <&L2_700>;
+			operating-points-v2 = <&cpu6_opp_table>,
+					      <&cpu6_ddr_bw_opp_table>,
+					      <&cpu6_l3_bw_opp_table>;
+			interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>,
+					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
+			interconnect-names = "cpu-ddr", "cpu-l3";
 			#cooling-cells = <2>;
 			qcom,freq-domain = <&cpufreq_hw 1>;
 			L2_700: l2-cache {
@@ -270,6 +318,245 @@
 		};
 	};
 
+	cpu0_opp_table: cpu0_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu0_opp1: opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>;
+		};
+
+		cpu0_opp2: opp-576000000 {
+			opp-hz = /bits/ 64 <576000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>;
+		};
+
+		cpu0_opp3: opp-768000000 {
+			opp-hz = /bits/ 64 <768000000>;
+			required-opps = <&cpu0_ddr_bw_opp1>, <&cpu0_l3_bw_opp1>;
+		};
+
+		cpu0_opp4: opp-1017600000 {
+			opp-hz = /bits/ 64 <1017600000>;
+			required-opps = <&cpu0_ddr_bw_opp2>, <&cpu0_l3_bw_opp2>;
+		};
+
+		cpu0_opp5: opp-1248000000 {
+			opp-hz = /bits/ 64 <1248000000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp3>;
+		};
+
+		cpu0_opp6: opp-1324800000 {
+			opp-hz = /bits/ 64 <1324800000>;
+			required-opps = <&cpu0_ddr_bw_opp3>, <&cpu0_l3_bw_opp3>;
+		};
+
+		cpu0_opp7: opp-1516800000 {
+			opp-hz = /bits/ 64 <1516800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>;
+		};
+
+		cpu0_opp8: opp-1612800000 {
+			opp-hz = /bits/ 64 <1612800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>;
+		};
+
+		cpu0_opp9: opp-1708800000 {
+			opp-hz = /bits/ 64 <1708800000>;
+			required-opps = <&cpu0_ddr_bw_opp4>, <&cpu0_l3_bw_opp4>;
+		};
+
+		cpu0_opp10: opp-1804800000 {
+			opp-hz = /bits/ 64 <1804800000>;
+			required-opps = <&cpu0_ddr_bw_opp5>, <&cpu0_l3_bw_opp5>;
+		};
+	};
+
+	cpu6_opp_table: cpu6_opp_table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu6_opp1: opp-300000000 {
+			opp-hz = /bits/ 64 <300000000>;
+			required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>;
+		};
+
+		cpu6_opp2: opp-652800000 {
+			opp-hz = /bits/ 64 <652800000>;
+			required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>;
+		};
+
+		cpu6_opp3: opp-825600000 {
+			opp-hz = /bits/ 64 <825600000>;
+			required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>;
+		};
+
+		cpu6_opp4: opp-979200000 {
+			opp-hz = /bits/ 64 <979200000>;
+			required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>;
+		};
+
+		cpu6_opp5: opp-1113600000 {
+			opp-hz = /bits/ 64 <1113600000>;
+			required-opps = <&cpu6_ddr_bw_opp1>, <&cpu6_l3_bw_opp1>;
+		};
+
+		cpu6_opp6: opp-1267200000 {
+			opp-hz = /bits/ 64 <1267200000>;
+			required-opps = <&cpu6_ddr_bw_opp2>, <&cpu6_l3_bw_opp2>;
+		};
+
+		cpu6_opp7: opp-1555200000 {
+			opp-hz = /bits/ 64 <1555200000>;
+			required-opps = <&cpu6_ddr_bw_opp2>, <&cpu6_l3_bw_opp3>;
+		};
+
+		cpu6_opp8: opp-1708800000 {
+			opp-hz = /bits/ 64 <1708800000>;
+			required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp4>;
+		};
+
+		cpu6_opp9: opp-1843200000 {
+			opp-hz = /bits/ 64 <1843200000>;
+			required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp4>;
+		};
+
+		cpu6_opp10: opp-1900800000 {
+			opp-hz = /bits/ 64 <1900800000>;
+			required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>;
+		};
+
+		cpu6_opp11: opp-1996800000 {
+			opp-hz = /bits/ 64 <1996800000>;
+			required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>;
+		};
+
+		cpu6_opp12: opp-2112000000 {
+			opp-hz = /bits/ 64 <2112000000>;
+			required-opps = <&cpu6_ddr_bw_opp3>, <&cpu6_l3_bw_opp5>;
+		};
+
+		cpu6_opp13: opp-2208000000 {
+			opp-hz = /bits/ 64 <2208000000>;
+			required-opps = <&cpu6_ddr_bw_opp4>, <&cpu6_l3_bw_opp5>;
+		};
+
+		cpu6_opp14: opp-2323200000 {
+			opp-hz = /bits/ 64 <2323200000>;
+			required-opps = <&cpu6_ddr_bw_opp4>, <&cpu6_l3_bw_opp5>;
+		};
+
+		cpu6_opp15: opp-2400000000 {
+			opp-hz = /bits/ 64 <2400000000>;
+			required-opps = <&cpu6_ddr_bw_opp5>, <&cpu6_l3_bw_opp6>;
+		};
+	};
+
+	cpu0_ddr_bw_opp_table: cpu0-ddr-bw-opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu0_ddr_bw_opp1: opp-300000000  {
+			opp-peak-kBps =/bits/ 32 <1200000>;
+		};
+
+		cpu0_ddr_bw_opp2: opp-451000000  {
+			opp-peak-kBps =/bits/ 32 <1804000>;
+		};
+
+		cpu0_ddr_bw_opp3: opp-547000000  {
+			opp-peak-kBps =/bits/ 32 <2188000>;
+		};
+
+		cpu0_ddr_bw_opp4: opp-768000000  {
+			opp-peak-kBps =/bits/ 32 <3072000>;
+		};
+
+		cpu0_ddr_bw_opp5: opp-1017000000  {
+			opp-peak-kBps =/bits/ 32 <4068000>;
+		};
+	};
+
+	cpu0_l3_bw_opp_table: cpu0-l3-bw-opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu0_l3_bw_opp1: opp-300000000 {
+			opp-peak-kBps = /bits/ 32 <4800000>;
+		};
+
+		cpu0_l3_bw_opp2: opp-556800000 {
+			opp-peak-kBps = /bits/ 32 <8908800>;
+		};
+
+		cpu0_l3_bw_opp3: opp-806400000 {
+			opp-peak-kBps = /bits/ 32 <12902400>;
+		};
+
+		cpu0_l3_bw_opp4: opp-940800000 {
+			opp-peak-kBps = /bits/ 32 <15052800>;
+		};
+
+		cpu0_l3_bw_opp5: opp-1401000000 {
+			opp-peak-kBps = /bits/ 32 <22425600>;
+		};
+	};
+
+	cpu6_ddr_bw_opp_table: cpu6-ddr-bw-opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu6_ddr_bw_opp1: opp-547000000  {
+			opp-peak-kBps =/bits/ 32 <2188000>;
+		};
+
+		cpu6_ddr_bw_opp2: opp-1017000000  {
+			opp-peak-kBps =/bits/ 32 <4068000>;
+		};
+
+		cpu6_ddr_bw_opp3: opp-1555000000  {
+			opp-peak-kBps =/bits/ 32 <6220000>;
+		};
+
+		cpu6_ddr_bw_opp4: opp-1804000000  {
+			opp-peak-kBps =/bits/ 32 <7216000>;
+		};
+
+		cpu6_ddr_bw_opp5: opp-2133000000  {
+			opp-peak-kBps =/bits/ 32 <8532000>;
+		};
+	};
+
+	cpu6_l3_bw_opp_table: cpu6-l3-bw-opp-table {
+		compatible = "operating-points-v2";
+		opp-shared;
+
+		cpu6_l3_bw_opp1: opp-556800000 {
+			opp-peak-kBps = /bits/ 32 <8908800>;
+		};
+
+		cpu6_l3_bw_opp2: opp-806400000 {
+			opp-peak-kBps = /bits/ 32 <12902400>;
+		};
+
+		cpu6_l3_bw_opp3: opp-940800000 {
+			opp-peak-kBps = /bits/ 32 <15052800>;
+		};
+
+		cpu6_l3_bw_opp4: opp-1209600000 {
+			opp-peak-kBps = /bits/ 32 <19353600>;
+		};
+
+		cpu6_l3_bw_opp5: opp-1401000000 {
+			opp-peak-kBps = /bits/ 32 <22425600>;
+		};
+
+		cpu6_l3_bw_opp6: opp-1459000000 {
+			opp-peak-kBps = /bits/ 32 <23347200>;
+		};
+	};
+
 	memory@80000000 {
 		device_type = "memory";
 		/* We expect the bootloader to fill in the size */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [RFC v3 01/10] arm64: dts: qcom: sdm845: Add SoC compatible to MTP

[Matthias Kaehlcke]

Hi Sibi,

On Tue, Jan 28, 2020 at 01:33:41AM +0530, Sibi Sankar wrote:
> Add missing SoC compatible to SDM845 MTP board file.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index 09ad37b0dd71d..54087847794aa 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -13,7 +13,7 @@
>  
>  / {
>  	model = "Qualcomm Technologies, Inc. SDM845 MTP";
> -	compatible = "qcom,sdm845-mtp";
> +	compatible = "qcom,sdm845-mtp", "qcom,sdm845";

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Since this is v3 already I think you can omit the RFC tag in the next
revision of the series.

Re: [RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev

[Matthias Kaehlcke]

On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote:
> Add SDM845 to cpufreq-dt-platdev blacklist.

nit: you could mention that cpufreq is handled by the
'qcom-cpufreq-hw' driver.

> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index f2ae9cd455c17..5492cf3c9dc18 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -130,6 +130,7 @@ static const struct of_device_id blacklist[] __initconst = {
>  	{ .compatible = "qcom,apq8096", },
>  	{ .compatible = "qcom,msm8996", },
>  	{ .compatible = "qcom,qcs404", },
> +	{ .compatible = "qcom,sdm845", },
>  
>  	{ .compatible = "st,stih407", },
>  	{ .compatible = "st,stih410", },

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [RFC v3 03/10] cpufreq: blacklist SC7180 in cpufreq-dt-platdev

[Matthias Kaehlcke]

On Tue, Jan 28, 2020 at 01:33:43AM +0530, Sibi Sankar wrote:
> Add SC7180 to cpufreq-dt-platdev blacklist.

nit: you could mention that cpufreq is handled by the
'qcom-cpufreq-hw' driver.

> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c b/drivers/cpufreq/cpufreq-dt-platdev.c
> index 5492cf3c9dc18..580abc777d9d8 100644
> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
> @@ -130,6 +130,7 @@ static const struct of_device_id blacklist[] __initconst = {
>  	{ .compatible = "qcom,apq8096", },
>  	{ .compatible = "qcom,msm8996", },
>  	{ .compatible = "qcom,qcs404", },
> +	{ .compatible = "qcom,sc7180", },
>  	{ .compatible = "qcom,sdm845", },
>  
>  	{ .compatible = "st,stih407", },

Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

Re: [RFC v3 04/10] OPP: Add and export helper to update voltage

[Matthias Kaehlcke]

Hi Sibi,

On Tue, Jan 28, 2020 at 01:33:44AM +0530, Sibi Sankar wrote:
> Add and export 'dev_pm_opp_update_voltage' to find and update voltage
> of an opp for a given frequency. This will be useful to update the opps
> with voltages read back from firmware.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  drivers/opp/core.c     | 55 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pm_opp.h | 10 ++++++++
>  2 files changed, 65 insertions(+)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 9aa2a44a5d638..f241e83ec926a 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, unsigned long freq)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
>  
> +/**
> + * dev_pm_opp_update_voltage() - Find and update voltage

The comment should mention that this is done for an OPP.

Maybe omit the 'find' part here and just say 'Update the voltage of
an OPP'?

> + * @dev:	device for which we do this operation
> + * @freq:	OPP frequency to update voltage
> + * @u_volt:	voltage requested for this opp
> + *
> + * Find and update voltage of a disabled opp corresponding to the given
> + * frequency. This is useful only for devices with single power supply.
> + *
> + * Return: 0 if modification was successful or a negative error value.
> + */
> +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
> +			      unsigned long u_volt)
> +{
> +	struct dev_pm_opp *opp = ERR_PTR(-ENODEV);

initialization is not needed

> +	struct opp_table *opp_table;
> +	unsigned long tol;
> +	int ret = 0;
> +
> +	/* Find the opp_table */

Drop the comment, it's obvious from the code.

> +	opp_table = _find_opp_table(dev);
> +	if (IS_ERR(opp_table)) {
> +		ret = PTR_ERR(opp_table);
> +		dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret);
> +		return PTR_ERR(opp_table);

  		return ret;

> +	}
> +
> +	opp = dev_pm_opp_find_freq_exact(dev, freq, false);
> +	if (IS_ERR(opp)) {
> +		ret = PTR_ERR(opp);
> +		goto put_table;
> +	}
> +
> +	mutex_lock(&opp_table->lock);
> +
> +	/* update only if the opp is disabled */
> +	if (opp->available) {
> +		ret = -EBUSY;
> +		goto unlock;
> +	}
> +
> +	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
> +	opp->supplies[0].u_volt_min = u_volt - tol;
> +	opp->supplies[0].u_volt = u_volt;
> +	opp->supplies[0].u_volt_min = u_volt + tol;

  			.u_volt_max =

I suppose the assignments need to be done for all possible supplies,
i.e. 0 to (opp_table->regulator_count - 1).

Re: [RFC v3 09/10] arm64: dts: qcom: sdm845: Add cpu OPP tables

[Matthias Kaehlcke]

Hi Sibi,

On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote:
> Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 SoCs.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 +++++++++++++++++++++++++++
>  1 file changed, 453 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> index c036bab49fc03..8cb976118407b 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> @@ -199,6 +199,12 @@
>  			qcom,freq-domain = <&cpufreq_hw 0>;
>  			#cooling-cells = <2>;
>  			next-level-cache = <&L2_0>;
> +			operating-points-v2 = <&cpu0_opp_table>,
> +					      <&cpu0_ddr_bw_opp_table>,
> +					      <&cpu0_l3_bw_opp_table>;
> +			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc SLAVE_EBI1>,
> +					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;

This apparently depends on the 'Split SDM845 interconnect nodes and
consolidate RPMh support' series
(https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281),
which isn't mentioned in the cover letter.

I also couldn't find a patch on the lists that adds the 'osm_l3'
interconnect node for SDM845. The same is true for SC7180 (next
patch of this series). These patches may be available in custom trees,
but that isn't really helpful for upstream review.

I would suggest to focus on landing the dependencies of this series,
before proceding with it (or at least most of them), there are plenty
and without the dependencies this series isn't going to land, it also
makes it hard for testers and reviewers to get all the pieces
together. In particular the last post of the series 'Add
required-opps support to devfreq passive gov'
(https://patchwork.kernel.org/cover/11055499/) is from July 2019 ...

Thanks

Matthias

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Rob Clark]

On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
>
> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                 a single device. I am yet to fix the debugfs to
>                 support multiple opp_tables per device but wanted to
>                 send what was working upstream to get an idea if multiple
>                 opp tables per device is a feature that will be useful
>                 upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                  and SC7180 SoCs.
>
> v3:
>  * Migrated to using Saravana's opp-kBps bindings [1]
>  * Fixed some misc comments from Rajendra
>  * Added support for SC7180
>
> v2:
>  * Incorporated Viresh's comments from:
>  https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>  https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>  * Dropped cpufreq-map passive governor
>
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. Just
> including them here for completeness.
>
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/

So drive-by question, from the perspective of someone who cares about
a non-cpu device that frequently enjoys a lot of bandwidth (ie. the
GPU).. any thoughts on how ddr scaling would/should work for workloads
where the CPU is not particularly busy?

BR,
-R

>
> Sibi Sankar (10):
>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>   OPP: Add and export helper to update voltage
>   opp: of: export _opp_of_get_opp_desc_node
>   opp: Allow multiple opp_tables to be mapped to a single device
>   opp: Remove multiple attached opp tables from a device
>   cpufreq: qcom: Update the bandwidth levels on frequency change
>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>   arm64: dts: qcom: sc7180: Add cpu OPP tables
>
>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>  drivers/opp/core.c                      | 111 +++++-
>  drivers/opp/of.c                        |   3 +-
>  drivers/opp/opp.h                       |   2 +
>  include/linux/pm_opp.h                  |  10 +
>  9 files changed, 1083 insertions(+), 33 deletions(-)
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [RFC v3 08/10] cpufreq: qcom: Update the bandwidth levels on frequency change

[Lukasz Luba]

Hi Sibi,

On 1/27/20 8:03 PM, Sibi Sankar wrote:
> Add support to parse and update OPP tables attached to the cpu device
> when the required OPP bandwidth values are populated to enable scaling
> of DDR/L3 bandwidth levels with frequency change.
> 
> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
>   drivers/cpufreq/qcom-cpufreq-hw.c | 246 +++++++++++++++++++++++++++---
>   1 file changed, 225 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index fc92a8842e252..348eb2fdaccaf 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -6,6 +6,7 @@
>   #include <linux/bitfield.h>
>   #include <linux/cpufreq.h>
>   #include <linux/init.h>
> +#include <linux/interconnect.h>
>   #include <linux/kernel.h>
>   #include <linux/module.h>
>   #include <linux/of_address.h>
> @@ -28,17 +29,194 @@
>   #define REG_VOLT_LUT			0x114
>   #define REG_PERF_STATE			0x920
>   
> +#define QCOM_ICC_TAG_ACTIVE_ONLY	1
> +
>   static unsigned long cpu_hw_rate, xo_rate;
>   static struct platform_device *global_pdev;
>   
> +/* opp table indices */
> +enum {
> +	QCOM_CPU_OPP_TABLE_INDEX,
> +	QCOM_CPU_DDR_OPP_TABLE_INDEX,
> +	QCOM_CPU_L3_OPP_TABLE_INDEX,
> +};
> +
> +/* icc path indices */
> +enum {
> +	ICC_DDR_PATH,
> +	ICC_L3_PATH,
> +};
> +
> +struct qcom_cpufreq_hw_res {
> +	void __iomem *base;
> +
> +	struct device *cpu_dev;
> +
> +	/* ddr/l3 icc paths */
> +	struct icc_path *path[2];
> +
> +	/* cpu/ddr/l3 opp tables */
> +	struct opp_table *opp_table[3];
> +};
> +
> +struct cpufreq_hw_icc_info {
> +	const char *icc_path_name;
> +	u8 table_index;
> +	u32 tag;
> +};
> +
> +static const struct cpufreq_hw_icc_info icc_info[] = {
> +	{
> +		.icc_path_name = "cpu-ddr",
> +		.table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX,
> +		.tag = QCOM_ICC_TAG_ACTIVE_ONLY,
> +	},
> +	{
> +		.icc_path_name = "cpu-l3",
> +		.table_index = QCOM_CPU_L3_OPP_TABLE_INDEX,
> +		.tag = 0,
> +	},
> +};
> +
> +static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res *res)
> +{
> +	struct opp_table **table = res->opp_table;
> +	struct device_node *opp_table_np, *np;
> +	struct device *dev = res->cpu_dev;
> +	int ret, i;
> +	u64 rate;
> +
> +	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
> +		ret = dev_pm_opp_of_add_table_indexed(dev, i);
> +		if (ret) {
> +			dev_dbg(dev, "Add OPP table failed index %d: %d\n",
> +				i, ret);
> +			goto err;
> +		}
> +
> +		table[i] = dev_pm_opp_get_opp_table_indexed(dev, i);
> +		if (!table[i]) {
> +			dev_dbg(dev, "Get OPP table failed index %d\n", i);
> +			ret = -EINVAL;
> +			goto err;
> +		}
> +	}
> +
> +	/* Disable all cpu opps and cross-validate against LUT */
> +	opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev);
> +	for_each_available_child_of_node(opp_table_np, np) {
> +		ret = of_property_read_u64(np, "opp-hz", &rate);
> +		if (ret)
> +			continue;
> +
> +		dev_pm_opp_disable(dev, rate);
> +	}
> +	of_node_put(opp_table_np);
> +
> +	return 0;
> +
> +err:
> +	for (; i >= 0; i--) {
> +		if (table[i]) {
> +			dev_pm_opp_put_opp_table(table[i]);
> +			table[i] = NULL;
> +		}
> +	}
> +
> +	dev_pm_opp_remove_table(dev);
> +	return ret;
> +}
> +
> +static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, u8 index)
> +{
> +	const struct cpufreq_hw_icc_info *info = &icc_info[index];
> +	struct icc_path **path = res->path;
> +	struct device *dev = res->cpu_dev;
> +
> +	path[index] = of_icc_get(dev, info->icc_path_name);
> +	if (IS_ERR(path[index])) {
> +		dev_dbg(dev, "ICC %s path get failed ret: %ld\n",
> +			info->icc_path_name, PTR_ERR(path[index]));
> +		return PTR_ERR(path[index]);
> +	}
> +
> +	icc_set_tag(path[index], info->tag);
> +
> +	return 0;
> +}
> +
> +static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res,
> +				    unsigned long freq)
> +{
> +	struct opp_table **table = res->opp_table;
> +	const struct cpufreq_hw_icc_info *info;
> +	unsigned long freq_hz = freq * 1000;
> +	struct icc_path **path = res->path;
> +	struct device *dev = res->cpu_dev;
> +	struct dev_pm_opp *cpu_opp, *opp;
> +	struct opp_table *cpu_opp_table;
> +	unsigned long peak_bw, avg_bw;
> +	int ret;
> +	int i;
> +
> +	cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX];
> +	if (!cpu_opp_table)
> +		return;
> +
> +	cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
> +	if (IS_ERR_OR_NULL(cpu_opp))
> +		return;
> +
> +	for (i = 0; i <= ICC_L3_PATH; i++) {
> +		if (IS_ERR(path[i])) {
> +			if (PTR_ERR(path[i]) != -EPROBE_DEFER)
> +				continue;
> +
> +			ret = qcom_cpufreq_hw_icc_init(res, i);
> +			if (ret)
> +				continue;
> +		}
> +
> +		info = &icc_info[i];
> +
> +		opp = dev_pm_opp_xlate_required_opp(cpu_opp_table,
> +						    table[info->table_index],
> +						    cpu_opp);
> +		if (IS_ERR_OR_NULL(opp))
> +			continue;
> +
> +		peak_bw = dev_pm_opp_get_bw(opp, &avg_bw);
> +		dev_pm_opp_put(opp);
> +
> +		icc_set_bw(res->path[i], avg_bw, peak_bw);
> +	}
> +	dev_pm_opp_put(cpu_opp);
> +}
> +
> +static int qcom_cpufreq_update_opp(struct device *dev,
> +				   unsigned long freq_khz,
> +				   unsigned long volt)
> +{
> +	unsigned long freq_hz = freq_khz * 1000;
> +
> +	if (dev_pm_opp_update_voltage(dev, freq_hz, volt))
> +		return dev_pm_opp_add(dev, freq_hz, volt);
> +
> +	/* Enable the opp after voltage update*/
> +	return dev_pm_opp_enable(dev, freq_hz);
> +}
> +
>   static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
>   					unsigned int index)
>   {
> -	void __iomem *perf_state_reg = policy->driver_data;
> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
> +	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
>   	unsigned long freq = policy->freq_table[index].frequency;
>   
>   	writel_relaxed(index, perf_state_reg);
>   
> +	qcom_cpufreq_hw_icc_set(res, freq);
> +
>   	arch_set_freq_scale(policy->related_cpus, freq,
>   			    policy->cpuinfo.max_freq);
>   	return 0;
> @@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct cpufreq_policy *policy,
>   
>   static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>   {
> +	struct qcom_cpufreq_hw_res *res;
>   	void __iomem *perf_state_reg;
>   	struct cpufreq_policy *policy;
>   	unsigned int index;
> @@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>   	if (!policy)
>   		return 0;
>   
> -	perf_state_reg = policy->driver_data;
> +	res = policy->driver_data;
> +	perf_state_reg = res->base + REG_PERF_STATE;
>   
>   	index = readl_relaxed(perf_state_reg);
>   	index = min(index, LUT_MAX_ENTRIES - 1);
> @@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>   static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>   						unsigned int target_freq)
>   {
> -	void __iomem *perf_state_reg = policy->driver_data;
> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
> +	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
>   	int index;
>   	unsigned long freq;
>   
> @@ -82,18 +263,24 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>   	return freq;
>   }
>   
> -static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
> -				    struct cpufreq_policy *policy,
> -				    void __iomem *base)
> +static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy,
> +				    struct qcom_cpufreq_hw_res *res)
>   {
>   	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
>   	u32 volt;
>   	struct cpufreq_frequency_table	*table;
> +	struct device *cpu_dev = res->cpu_dev;
> +	void __iomem *base = res->base;
> +	int ret;
>   
>   	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
>   	if (!table)
>   		return -ENOMEM;
>   
> +	ret = qcom_cpufreq_hw_add_opp_table(res);
> +	if (ret)
> +		dev_dbg(cpu_dev, "Add OPP tables failed from dt\n");
> +
>   	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>   		data = readl_relaxed(base + REG_FREQ_LUT +
>   				      i * LUT_ROW_SIZE);
> @@ -112,7 +299,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>   
>   		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
>   			table[i].frequency = freq;
> -			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
> +			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
>   			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
>   				freq, core_count);
>   		} else if (core_count == LUT_TURBO_IND) {
> @@ -133,7 +320,8 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>   			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
>   				prev->frequency = prev_freq;
>   				prev->flags = CPUFREQ_BOOST_FREQ;
> -				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
> +				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
> +							volt);
>   			}
>   
>   			break;
> @@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, struct cpumask *m)
>   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   {
>   	struct device *dev = &global_pdev->dev;
> +	struct qcom_cpufreq_hw_res *res;
>   	struct of_phandle_args args;
>   	struct device_node *cpu_np;
>   	struct device *cpu_dev;
> -	struct resource *res;
> -	void __iomem *base;
>   	int ret, index;
>   
>   	cpu_dev = get_cpu_device(policy->cpu);
> @@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   
>   	index = args.args[0];
>   
> -	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
> +	res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
>   	if (!res)
> -		return -ENODEV;
> -
> -	base = devm_ioremap(dev, res->start, resource_size(res));
> -	if (!base)
>   		return -ENOMEM;
>   
> +	res->cpu_dev = cpu_dev;
> +	res->base = devm_platform_ioremap_resource(global_pdev, index);
> +	if (IS_ERR(res->base))
> +		return PTR_ERR(res->base);
> +
>   	/* HW should be in enabled state to proceed */
> -	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
> +	if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) {
>   		dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
>   		ret = -ENODEV;
>   		goto error;
> @@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   		goto error;
>   	}
>   
> -	policy->driver_data = base + REG_PERF_STATE;
> +	policy->driver_data = res;
>   
> -	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base);
> +	ret = qcom_cpufreq_hw_read_lut(policy, res);
>   	if (ret) {
>   		dev_err(dev, "Domain-%d failed to read LUT\n", index);
>   		goto error;
> @@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>   
>   	dev_pm_opp_of_register_em(policy->cpus);
>   
> -	policy->fast_switch_possible = true;
> +	if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) {
> +		policy->fast_switch_possible = true;
> +	} else {
> +		qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH);
> +		qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH);
> +	}

This patch tries to disable silently the fast_switch. The fast_switch
is used by SchedUtil governor.

Regards,
Lukasz

>   
>   	return 0;
>   error:
> -	devm_iounmap(dev, base);
> +	devm_iounmap(dev, res->base);
> +	devm_kfree(&global_pdev->dev, res);
>   	return ret;
>   }
>   
>   static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
>   {
> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
>   	struct device *cpu_dev = get_cpu_device(policy->cpu);
> -	void __iomem *base = policy->driver_data - REG_PERF_STATE;
> +	struct opp_table **table = res->opp_table;
> +	void __iomem *base = res->base;
> +	int i;
> +
> +	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
> +		if (table[i])
> +			dev_pm_opp_put_opp_table(table[i]);
> +	}
>   
>   	dev_pm_opp_remove_all_dynamic(cpu_dev);
>   	kfree(policy->freq_table);
> +	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
>   	devm_iounmap(&global_pdev->dev, base);
> +	devm_kfree(&global_pdev->dev, res);
>   
>   	return 0;
>   }
> 

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Lukasz Luba]

Hi Sibi,

In my opinion this solution depends on not always true assumption that
CPUFreq notification chain will be triggered when there is a frequency
switch. Extending devfreq governor (as in one of the dependent patch
series that you have referred) by attaching to this notification
chain makes sense only when the SchedUtil and fast_switch is not in use.
The Schedutil CPUFreq governor might use the fast_switch from this
driver and the notifications will not be triggered. I have also
commented patch 08/10 which tries to disable it.

Regards,
Lukasz

On 1/27/20 8:03 PM, Sibi Sankar wrote:
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
> 
> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                  a single device. I am yet to fix the debugfs to
> 		support multiple opp_tables per device but wanted to
> 		send what was working upstream to get an idea if multiple
> 		opp tables per device is a feature that will be useful
> 		upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                   and SC7180 SoCs.
> 
> v3:
>   * Migrated to using Saravana's opp-kBps bindings [1]
>   * Fixed some misc comments from Rajendra
>   * Added support for SC7180
> 
> v2:
>   * Incorporated Viresh's comments from:
>   https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>   https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>   * Dropped cpufreq-map passive governor
> 
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
> 
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. Just
> including them here for completeness.
> 
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/
> 
> Sibi Sankar (10):
>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>    OPP: Add and export helper to update voltage
>    opp: of: export _opp_of_get_opp_desc_node
>    opp: Allow multiple opp_tables to be mapped to a single device
>    opp: Remove multiple attached opp tables from a device
>    cpufreq: qcom: Update the bandwidth levels on frequency change
>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>    arm64: dts: qcom: sc7180: Add cpu OPP tables
> 
>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>   drivers/opp/core.c                      | 111 +++++-
>   drivers/opp/of.c                        |   3 +-
>   drivers/opp/opp.h                       |   2 +
>   include/linux/pm_opp.h                  |  10 +
>   9 files changed, 1083 insertions(+), 33 deletions(-)
> 

Re: [RFC v3 01/10] arm64: dts: qcom: sdm845: Add SoC compatible to MTP

[Sibi Sankar]

Hey Matthias,
Thanks for the review!

On 2020-01-29 02:10, Matthias Kaehlcke wrote:
> Hi Sibi,
> 
> On Tue, Jan 28, 2020 at 01:33:41AM +0530, Sibi Sankar wrote:
>> Add missing SoC compatible to SDM845 MTP board file.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts 
>> b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> index 09ad37b0dd71d..54087847794aa 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
>> @@ -13,7 +13,7 @@
>> 
>>  / {
>>  	model = "Qualcomm Technologies, Inc. SDM845 MTP";
>> -	compatible = "qcom,sdm845-mtp";
>> +	compatible = "qcom,sdm845-mtp", "qcom,sdm845";
> 
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>
> 
> Since this is v3 already I think you can omit the RFC tag in the next
> revision of the series.

Yeah would like to drop the RFC
but every version seems to have
a feature which we havn't reached
a consensus on.

v1: CPUfreq passive governor
v2: New bandwidth bindings
v3: Multiple opp tables per device

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

Re: [RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev

[Sibi Sankar]

Thanks for the review!

On 2020-01-29 02:14, Matthias Kaehlcke wrote:
> On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote:
>> Add SDM845 to cpufreq-dt-platdev blacklist.
> 
> nit: you could mention that cpufreq is handled by the
> 'qcom-cpufreq-hw' driver.

will add it in the next
re-spin.

> 
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  drivers/cpufreq/cpufreq-dt-platdev.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/cpufreq/cpufreq-dt-platdev.c 
>> b/drivers/cpufreq/cpufreq-dt-platdev.c
>> index f2ae9cd455c17..5492cf3c9dc18 100644
>> --- a/drivers/cpufreq/cpufreq-dt-platdev.c
>> +++ b/drivers/cpufreq/cpufreq-dt-platdev.c
>> @@ -130,6 +130,7 @@ static const struct of_device_id blacklist[] 
>> __initconst = {
>>  	{ .compatible = "qcom,apq8096", },
>>  	{ .compatible = "qcom,msm8996", },
>>  	{ .compatible = "qcom,qcs404", },
>> +	{ .compatible = "qcom,sdm845", },
>> 
>>  	{ .compatible = "st,stih407", },
>>  	{ .compatible = "st,stih410", },
> 
> Reviewed-by: Matthias Kaehlcke <mka@chromium.org>

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

Re: [RFC v3 04/10] OPP: Add and export helper to update voltage

[Sibi Sankar]

On 2020-01-29 03:03, Matthias Kaehlcke wrote:
> Hi Sibi,
> 
> On Tue, Jan 28, 2020 at 01:33:44AM +0530, Sibi Sankar wrote:
>> Add and export 'dev_pm_opp_update_voltage' to find and update voltage
>> of an opp for a given frequency. This will be useful to update the 
>> opps
>> with voltages read back from firmware.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  drivers/opp/core.c     | 55 
>> ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pm_opp.h | 10 ++++++++
>>  2 files changed, 65 insertions(+)
>> 
>> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
>> index 9aa2a44a5d638..f241e83ec926a 100644
>> --- a/drivers/opp/core.c
>> +++ b/drivers/opp/core.c
>> @@ -2503,6 +2503,61 @@ int dev_pm_opp_disable(struct device *dev, 
>> unsigned long freq)
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_opp_disable);
>> 
>> +/**
>> + * dev_pm_opp_update_voltage() - Find and update voltage
> 
> The comment should mention that this is done for an OPP.
> 
> Maybe omit the 'find' part here and just say 'Update the voltage of
> an OPP'?

sure makes sense

> 
>> + * @dev:	device for which we do this operation
>> + * @freq:	OPP frequency to update voltage
>> + * @u_volt:	voltage requested for this opp
>> + *
>> + * Find and update voltage of a disabled opp corresponding to the 
>> given
>> + * frequency. This is useful only for devices with single power 
>> supply.
>> + *
>> + * Return: 0 if modification was successful or a negative error 
>> value.
>> + */
>> +int dev_pm_opp_update_voltage(struct device *dev, unsigned long freq,
>> +			      unsigned long u_volt)
>> +{
>> +	struct dev_pm_opp *opp = ERR_PTR(-ENODEV);
> 
> initialization is not needed
> 
>> +	struct opp_table *opp_table;
>> +	unsigned long tol;
>> +	int ret = 0;
>> +
>> +	/* Find the opp_table */
> 
> Drop the comment, it's obvious from the code.
> 
>> +	opp_table = _find_opp_table(dev);
>> +	if (IS_ERR(opp_table)) {
>> +		ret = PTR_ERR(opp_table);
>> +		dev_err(dev, "%s: OPP table not found (%d)\n", __func__, ret);
>> +		return PTR_ERR(opp_table);
> 
>   		return ret;

missed that :(

> 
>> +	}
>> +
>> +	opp = dev_pm_opp_find_freq_exact(dev, freq, false);
>> +	if (IS_ERR(opp)) {
>> +		ret = PTR_ERR(opp);
>> +		goto put_table;
>> +	}
>> +
>> +	mutex_lock(&opp_table->lock);
>> +
>> +	/* update only if the opp is disabled */
>> +	if (opp->available) {
>> +		ret = -EBUSY;
>> +		goto unlock;
>> +	}
>> +
>> +	tol = u_volt * opp_table->voltage_tolerance_v1 / 100;
>> +	opp->supplies[0].u_volt_min = u_volt - tol;
>> +	opp->supplies[0].u_volt = u_volt;
>> +	opp->supplies[0].u_volt_min = u_volt + tol;
> 
>   			.u_volt_max =
> 
> I suppose the assignments need to be done for all possible supplies,
> i.e. 0 to (opp_table->regulator_count - 1).

a single value for all possible
supplies seems wrong. Anyway
will wait to see what Viresh
thinks about it.

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

Re: [RFC v3 09/10] arm64: dts: qcom: sdm845: Add cpu OPP tables

[Sibi Sankar]

Hey Matthias,

Thanks for the review!

On 2020-01-29 06:54, Matthias Kaehlcke wrote:
> Hi Sibi,
> 
> On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote:
>> Add OPP tables required to scale DDR/L3 per freq-domain on SDM845 
>> SoCs.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>  arch/arm64/boot/dts/qcom/sdm845.dtsi | 453 
>> +++++++++++++++++++++++++++
>>  1 file changed, 453 insertions(+)
>> 
>> diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi 
>> b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> index c036bab49fc03..8cb976118407b 100644
>> --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
>> @@ -199,6 +199,12 @@
>>  			qcom,freq-domain = <&cpufreq_hw 0>;
>>  			#cooling-cells = <2>;
>>  			next-level-cache = <&L2_0>;
>> +			operating-points-v2 = <&cpu0_opp_table>,
>> +					      <&cpu0_ddr_bw_opp_table>,
>> +					      <&cpu0_l3_bw_opp_table>;
>> +			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc 
>> SLAVE_EBI1>,
>> +					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
> 
> This apparently depends on the 'Split SDM845 interconnect nodes and
> consolidate RPMh support' series
> (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281),
> which isn't mentioned in the cover letter.
> 
> I also couldn't find a patch on the lists that adds the 'osm_l3'
> interconnect node for SDM845. The same is true for SC7180 (next
> patch of this series). These patches may be available in custom trees,
> but that isn't really helpful for upstream review.

yeah I missed adding the interconnect
refactor dependency and the nodes.

> 
> I would suggest to focus on landing the dependencies of this series,
> before proceding with it (or at least most of them), there are plenty
> and without the dependencies this series isn't going to land, it also
> makes it hard for testers and reviewers to get all the pieces

yes I understand but wanted the series
out asap because since there are a few
points where we still havn't reached
a consensus on.

> together. In particular the last post of the series 'Add
> required-opps support to devfreq passive gov'
> (https://patchwork.kernel.org/cover/11055499/) is from July 2019 ...

https://lore.kernel.org/lkml/CAGTfZH37ALwUHd8SpRRrBzZ6x1-++YtzS60_yRQvN-TN6rOzaA@mail.gmail.com/

The pending patch for lazy linking
was posted a while back. Now that
it has a tested-by, majority of the
series should go in since the devfreq
maintainers wanted the series pulled
in.

> 
> Thanks
> 
> Matthias

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

Hey Rob,

On 2020-01-29 08:24, Rob Clark wrote:
> On Mon, Jan 27, 2020 at 12:05 PM Sibi Sankar <sibis@codeaurora.org> 
> wrote:
>> 
>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>> SDM845 and SC7180 SoCs.
>> 
>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>                 a single device. I am yet to fix the debugfs to
>>                 support multiple opp_tables per device but wanted to
>>                 send what was working upstream to get an idea if 
>> multiple
>>                 opp tables per device is a feature that will be useful
>>                 upstream.
>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>                  and SC7180 SoCs.
>> 
>> v3:
>>  * Migrated to using Saravana's opp-kBps bindings [1]
>>  * Fixed some misc comments from Rajendra
>>  * Added support for SC7180
>> 
>> v2:
>>  * Incorporated Viresh's comments from:
>>  
>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>  
>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>  * Dropped cpufreq-map passive governor
>> 
>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>> 
>> Some alternate ways of hosting the opp-tables:
>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
>> Viresh didn't really like ^^ bindings and they dont really scale well. 
>> Just
>> including them here for completeness.
>> 
>> Depends on the following series:
>> [1] https://patchwork.kernel.org/cover/11277199/
>> [2] https://patchwork.kernel.org/cover/11055499/
>> [3] https://patchwork.kernel.org/cover/11326381/
> 
> So drive-by question, from the perspective of someone who cares about
> a non-cpu device that frequently enjoys a lot of bandwidth (ie. the
> GPU).. any thoughts on how ddr scaling would/should work for workloads
> where the CPU is not particularly busy?

I guess Jordan would have more info
about ^^. From what I can see from
the CAF trees there is a similar
device freq -> DDR Freq mapping that
you can achieve with required-opps
bindings and a custom governor ;)

> 
> BR,
> -R
> 
>> 
>> Sibi Sankar (10):
>>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>   OPP: Add and export helper to update voltage
>>   opp: of: export _opp_of_get_opp_desc_node
>>   opp: Allow multiple opp_tables to be mapped to a single device
>>   opp: Remove multiple attached opp tables from a device
>>   cpufreq: qcom: Update the bandwidth levels on frequency change
>>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>>   arm64: dts: qcom: sc7180: Add cpu OPP tables
>> 
>>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 
>> ++++++++++++++++++++++++
>>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>  drivers/opp/core.c                      | 111 +++++-
>>  drivers/opp/of.c                        |   3 +-
>>  drivers/opp/opp.h                       |   2 +
>>  include/linux/pm_opp.h                  |  10 +
>>  9 files changed, 1083 insertions(+), 33 deletions(-)
>> 
>> --
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
>> Forum,
>> a Linux Foundation Collaborative Project

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

Re: [RFC v3 08/10] cpufreq: qcom: Update the bandwidth levels on frequency change

[Sibi Sankar]

Hey Lukasz,
Thanks for taking time to review
the series!

On 2020-01-29 15:05, Lukasz Luba wrote:
> Hi Sibi,
> 
> On 1/27/20 8:03 PM, Sibi Sankar wrote:
>> Add support to parse and update OPP tables attached to the cpu device
>> when the required OPP bandwidth values are populated to enable scaling
>> of DDR/L3 bandwidth levels with frequency change.
>> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>   drivers/cpufreq/qcom-cpufreq-hw.c | 246 
>> +++++++++++++++++++++++++++---
>>   1 file changed, 225 insertions(+), 21 deletions(-)
>> 
>> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c 
>> b/drivers/cpufreq/qcom-cpufreq-hw.c
>> index fc92a8842e252..348eb2fdaccaf 100644
>> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
>> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/bitfield.h>
>>   #include <linux/cpufreq.h>
>>   #include <linux/init.h>
>> +#include <linux/interconnect.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>>   #include <linux/of_address.h>
>> @@ -28,17 +29,194 @@
>>   #define REG_VOLT_LUT			0x114
>>   #define REG_PERF_STATE			0x920
>>   +#define QCOM_ICC_TAG_ACTIVE_ONLY	1
>> +
>>   static unsigned long cpu_hw_rate, xo_rate;
>>   static struct platform_device *global_pdev;
>>   +/* opp table indices */
>> +enum {
>> +	QCOM_CPU_OPP_TABLE_INDEX,
>> +	QCOM_CPU_DDR_OPP_TABLE_INDEX,
>> +	QCOM_CPU_L3_OPP_TABLE_INDEX,
>> +};
>> +
>> +/* icc path indices */
>> +enum {
>> +	ICC_DDR_PATH,
>> +	ICC_L3_PATH,
>> +};
>> +
>> +struct qcom_cpufreq_hw_res {
>> +	void __iomem *base;
>> +
>> +	struct device *cpu_dev;
>> +
>> +	/* ddr/l3 icc paths */
>> +	struct icc_path *path[2];
>> +
>> +	/* cpu/ddr/l3 opp tables */
>> +	struct opp_table *opp_table[3];
>> +};
>> +
>> +struct cpufreq_hw_icc_info {
>> +	const char *icc_path_name;
>> +	u8 table_index;
>> +	u32 tag;
>> +};
>> +
>> +static const struct cpufreq_hw_icc_info icc_info[] = {
>> +	{
>> +		.icc_path_name = "cpu-ddr",
>> +		.table_index = QCOM_CPU_DDR_OPP_TABLE_INDEX,
>> +		.tag = QCOM_ICC_TAG_ACTIVE_ONLY,
>> +	},
>> +	{
>> +		.icc_path_name = "cpu-l3",
>> +		.table_index = QCOM_CPU_L3_OPP_TABLE_INDEX,
>> +		.tag = 0,
>> +	},
>> +};
>> +
>> +static int qcom_cpufreq_hw_add_opp_table(struct qcom_cpufreq_hw_res 
>> *res)
>> +{
>> +	struct opp_table **table = res->opp_table;
>> +	struct device_node *opp_table_np, *np;
>> +	struct device *dev = res->cpu_dev;
>> +	int ret, i;
>> +	u64 rate;
>> +
>> +	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
>> +		ret = dev_pm_opp_of_add_table_indexed(dev, i);
>> +		if (ret) {
>> +			dev_dbg(dev, "Add OPP table failed index %d: %d\n",
>> +				i, ret);
>> +			goto err;
>> +		}
>> +
>> +		table[i] = dev_pm_opp_get_opp_table_indexed(dev, i);
>> +		if (!table[i]) {
>> +			dev_dbg(dev, "Get OPP table failed index %d\n", i);
>> +			ret = -EINVAL;
>> +			goto err;
>> +		}
>> +	}
>> +
>> +	/* Disable all cpu opps and cross-validate against LUT */
>> +	opp_table_np = dev_pm_opp_of_get_opp_desc_node(dev);
>> +	for_each_available_child_of_node(opp_table_np, np) {
>> +		ret = of_property_read_u64(np, "opp-hz", &rate);
>> +		if (ret)
>> +			continue;
>> +
>> +		dev_pm_opp_disable(dev, rate);
>> +	}
>> +	of_node_put(opp_table_np);
>> +
>> +	return 0;
>> +
>> +err:
>> +	for (; i >= 0; i--) {
>> +		if (table[i]) {
>> +			dev_pm_opp_put_opp_table(table[i]);
>> +			table[i] = NULL;
>> +		}
>> +	}
>> +
>> +	dev_pm_opp_remove_table(dev);
>> +	return ret;
>> +}
>> +
>> +static int qcom_cpufreq_hw_icc_init(struct qcom_cpufreq_hw_res *res, 
>> u8 index)
>> +{
>> +	const struct cpufreq_hw_icc_info *info = &icc_info[index];
>> +	struct icc_path **path = res->path;
>> +	struct device *dev = res->cpu_dev;
>> +
>> +	path[index] = of_icc_get(dev, info->icc_path_name);
>> +	if (IS_ERR(path[index])) {
>> +		dev_dbg(dev, "ICC %s path get failed ret: %ld\n",
>> +			info->icc_path_name, PTR_ERR(path[index]));
>> +		return PTR_ERR(path[index]);
>> +	}
>> +
>> +	icc_set_tag(path[index], info->tag);
>> +
>> +	return 0;
>> +}
>> +
>> +static void qcom_cpufreq_hw_icc_set(struct qcom_cpufreq_hw_res *res,
>> +				    unsigned long freq)
>> +{
>> +	struct opp_table **table = res->opp_table;
>> +	const struct cpufreq_hw_icc_info *info;
>> +	unsigned long freq_hz = freq * 1000;
>> +	struct icc_path **path = res->path;
>> +	struct device *dev = res->cpu_dev;
>> +	struct dev_pm_opp *cpu_opp, *opp;
>> +	struct opp_table *cpu_opp_table;
>> +	unsigned long peak_bw, avg_bw;
>> +	int ret;
>> +	int i;
>> +
>> +	cpu_opp_table = table[QCOM_CPU_OPP_TABLE_INDEX];
>> +	if (!cpu_opp_table)
>> +		return;
>> +
>> +	cpu_opp = dev_pm_opp_find_freq_exact(dev, freq_hz, true);
>> +	if (IS_ERR_OR_NULL(cpu_opp))
>> +		return;
>> +
>> +	for (i = 0; i <= ICC_L3_PATH; i++) {
>> +		if (IS_ERR(path[i])) {
>> +			if (PTR_ERR(path[i]) != -EPROBE_DEFER)
>> +				continue;
>> +
>> +			ret = qcom_cpufreq_hw_icc_init(res, i);
>> +			if (ret)
>> +				continue;
>> +		}
>> +
>> +		info = &icc_info[i];
>> +
>> +		opp = dev_pm_opp_xlate_required_opp(cpu_opp_table,
>> +						    table[info->table_index],
>> +						    cpu_opp);
>> +		if (IS_ERR_OR_NULL(opp))
>> +			continue;
>> +
>> +		peak_bw = dev_pm_opp_get_bw(opp, &avg_bw);
>> +		dev_pm_opp_put(opp);
>> +
>> +		icc_set_bw(res->path[i], avg_bw, peak_bw);
>> +	}
>> +	dev_pm_opp_put(cpu_opp);
>> +}
>> +
>> +static int qcom_cpufreq_update_opp(struct device *dev,
>> +				   unsigned long freq_khz,
>> +				   unsigned long volt)
>> +{
>> +	unsigned long freq_hz = freq_khz * 1000;
>> +
>> +	if (dev_pm_opp_update_voltage(dev, freq_hz, volt))
>> +		return dev_pm_opp_add(dev, freq_hz, volt);
>> +
>> +	/* Enable the opp after voltage update*/
>> +	return dev_pm_opp_enable(dev, freq_hz);
>> +}
>> +
>>   static int qcom_cpufreq_hw_target_index(struct cpufreq_policy 
>> *policy,
>>   					unsigned int index)
>>   {
>> -	void __iomem *perf_state_reg = policy->driver_data;
>> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
>> +	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
>>   	unsigned long freq = policy->freq_table[index].frequency;
>>     	writel_relaxed(index, perf_state_reg);
>>   +	qcom_cpufreq_hw_icc_set(res, freq);
>> +
>>   	arch_set_freq_scale(policy->related_cpus, freq,
>>   			    policy->cpuinfo.max_freq);
>>   	return 0;
>> @@ -46,6 +224,7 @@ static int qcom_cpufreq_hw_target_index(struct 
>> cpufreq_policy *policy,
>>     static unsigned int qcom_cpufreq_hw_get(unsigned int cpu)
>>   {
>> +	struct qcom_cpufreq_hw_res *res;
>>   	void __iomem *perf_state_reg;
>>   	struct cpufreq_policy *policy;
>>   	unsigned int index;
>> @@ -54,7 +233,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned 
>> int cpu)
>>   	if (!policy)
>>   		return 0;
>>   -	perf_state_reg = policy->driver_data;
>> +	res = policy->driver_data;
>> +	perf_state_reg = res->base + REG_PERF_STATE;
>>     	index = readl_relaxed(perf_state_reg);
>>   	index = min(index, LUT_MAX_ENTRIES - 1);
>> @@ -65,7 +245,8 @@ static unsigned int qcom_cpufreq_hw_get(unsigned 
>> int cpu)
>>   static unsigned int qcom_cpufreq_hw_fast_switch(struct 
>> cpufreq_policy *policy,
>>   						unsigned int target_freq)
>>   {
>> -	void __iomem *perf_state_reg = policy->driver_data;
>> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
>> +	void __iomem *perf_state_reg = res->base + REG_PERF_STATE;
>>   	int index;
>>   	unsigned long freq;
>>   @@ -82,18 +263,24 @@ static unsigned int 
>> qcom_cpufreq_hw_fast_switch(struct cpufreq_policy *policy,
>>   	return freq;
>>   }
>>   -static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>> -				    struct cpufreq_policy *policy,
>> -				    void __iomem *base)
>> +static int qcom_cpufreq_hw_read_lut(struct cpufreq_policy *policy,
>> +				    struct qcom_cpufreq_hw_res *res)
>>   {
>>   	u32 data, src, lval, i, core_count, prev_freq = 0, freq;
>>   	u32 volt;
>>   	struct cpufreq_frequency_table	*table;
>> +	struct device *cpu_dev = res->cpu_dev;
>> +	void __iomem *base = res->base;
>> +	int ret;
>>     	table = kcalloc(LUT_MAX_ENTRIES + 1, sizeof(*table), GFP_KERNEL);
>>   	if (!table)
>>   		return -ENOMEM;
>>   +	ret = qcom_cpufreq_hw_add_opp_table(res);
>> +	if (ret)
>> +		dev_dbg(cpu_dev, "Add OPP tables failed from dt\n");
>> +
>>   	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
>>   		data = readl_relaxed(base + REG_FREQ_LUT +
>>   				      i * LUT_ROW_SIZE);
>> @@ -112,7 +299,7 @@ static int qcom_cpufreq_hw_read_lut(struct device 
>> *cpu_dev,
>>     		if (freq != prev_freq && core_count != LUT_TURBO_IND) {
>>   			table[i].frequency = freq;
>> -			dev_pm_opp_add(cpu_dev, freq * 1000, volt);
>> +			qcom_cpufreq_update_opp(cpu_dev, freq, volt);
>>   			dev_dbg(cpu_dev, "index=%d freq=%d, core_count %d\n", i,
>>   				freq, core_count);
>>   		} else if (core_count == LUT_TURBO_IND) {
>> @@ -133,7 +320,8 @@ static int qcom_cpufreq_hw_read_lut(struct device 
>> *cpu_dev,
>>   			if (prev->frequency == CPUFREQ_ENTRY_INVALID) {
>>   				prev->frequency = prev_freq;
>>   				prev->flags = CPUFREQ_BOOST_FREQ;
>> -				dev_pm_opp_add(cpu_dev,	prev_freq * 1000, volt);
>> +				qcom_cpufreq_update_opp(cpu_dev, prev_freq,
>> +							volt);
>>   			}
>>     			break;
>> @@ -175,11 +363,10 @@ static void qcom_get_related_cpus(int index, 
>> struct cpumask *m)
>>   static int qcom_cpufreq_hw_cpu_init(struct cpufreq_policy *policy)
>>   {
>>   	struct device *dev = &global_pdev->dev;
>> +	struct qcom_cpufreq_hw_res *res;
>>   	struct of_phandle_args args;
>>   	struct device_node *cpu_np;
>>   	struct device *cpu_dev;
>> -	struct resource *res;
>> -	void __iomem *base;
>>   	int ret, index;
>>     	cpu_dev = get_cpu_device(policy->cpu);
>> @@ -201,16 +388,17 @@ static int qcom_cpufreq_hw_cpu_init(struct 
>> cpufreq_policy *policy)
>>     	index = args.args[0];
>>   -	res = platform_get_resource(global_pdev, IORESOURCE_MEM, index);
>> +	res = devm_kzalloc(dev, sizeof(*res), GFP_KERNEL);
>>   	if (!res)
>> -		return -ENODEV;
>> -
>> -	base = devm_ioremap(dev, res->start, resource_size(res));
>> -	if (!base)
>>   		return -ENOMEM;
>>   +	res->cpu_dev = cpu_dev;
>> +	res->base = devm_platform_ioremap_resource(global_pdev, index);
>> +	if (IS_ERR(res->base))
>> +		return PTR_ERR(res->base);
>> +
>>   	/* HW should be in enabled state to proceed */
>> -	if (!(readl_relaxed(base + REG_ENABLE) & 0x1)) {
>> +	if (!(readl_relaxed(res->base + REG_ENABLE) & 0x1)) {
>>   		dev_err(dev, "Domain-%d cpufreq hardware not enabled\n", index);
>>   		ret = -ENODEV;
>>   		goto error;
>> @@ -223,9 +411,9 @@ static int qcom_cpufreq_hw_cpu_init(struct 
>> cpufreq_policy *policy)
>>   		goto error;
>>   	}
>>   -	policy->driver_data = base + REG_PERF_STATE;
>> +	policy->driver_data = res;
>>   -	ret = qcom_cpufreq_hw_read_lut(cpu_dev, policy, base);
>> +	ret = qcom_cpufreq_hw_read_lut(policy, res);
>>   	if (ret) {
>>   		dev_err(dev, "Domain-%d failed to read LUT\n", index);
>>   		goto error;
>> @@ -240,22 +428,38 @@ static int qcom_cpufreq_hw_cpu_init(struct 
>> cpufreq_policy *policy)
>>     	dev_pm_opp_of_register_em(policy->cpus);
>>   -	policy->fast_switch_possible = true;
>> +	if (!res->opp_table[QCOM_CPU_OPP_TABLE_INDEX]) {
>> +		policy->fast_switch_possible = true;
>> +	} else {
>> +		qcom_cpufreq_hw_icc_init(res, ICC_DDR_PATH);
>> +		qcom_cpufreq_hw_icc_init(res, ICC_L3_PATH);
>> +	}
> 
> This patch tries to disable silently the fast_switch. The fast_switch
> is used by SchedUtil governor.

I agree that the series would benefit
from splitting this patch into more
chunks. SchedUtil will still be used
as the governor but will operate in
the slow path. The main reason is that
the icc bandwidth votes through rpmh
on SDM845/SC7180 cant be done in the
fast path.

> 
> Regards,
> Lukasz
> 
>>     	return 0;
>>   error:
>> -	devm_iounmap(dev, base);
>> +	devm_iounmap(dev, res->base);
>> +	devm_kfree(&global_pdev->dev, res);
>>   	return ret;
>>   }
>>     static int qcom_cpufreq_hw_cpu_exit(struct cpufreq_policy *policy)
>>   {
>> +	struct qcom_cpufreq_hw_res *res = policy->driver_data;
>>   	struct device *cpu_dev = get_cpu_device(policy->cpu);
>> -	void __iomem *base = policy->driver_data - REG_PERF_STATE;
>> +	struct opp_table **table = res->opp_table;
>> +	void __iomem *base = res->base;
>> +	int i;
>> +
>> +	for (i = 0; i <= QCOM_CPU_L3_OPP_TABLE_INDEX; i++) {
>> +		if (table[i])
>> +			dev_pm_opp_put_opp_table(table[i]);
>> +	}
>>     	dev_pm_opp_remove_all_dynamic(cpu_dev);
>>   	kfree(policy->freq_table);
>> +	dev_pm_opp_of_cpumask_remove_table(policy->related_cpus);
>>   	devm_iounmap(&global_pdev->dev, base);
>> +	devm_kfree(&global_pdev->dev, res);
>>     	return 0;
>>   }
>> 

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

Hey Lukasz,
Thanks for taking time to review
the series!

On 2020-01-29 15:16, Lukasz Luba wrote:
> Hi Sibi,
> 
> In my opinion this solution depends on not always true assumption that
> CPUFreq notification chain will be triggered when there is a frequency

This series does not create any
devfreq devices nor use the cpufreq
notification chain. It just relies
on the opening up of required-opps
from being exclusive to gen-pd using
patches 1,2,3 from patch series[1].
With the fast path disabled and
schedutil enabled, this series will
not miss any cpufreq changes.

[1] https://patchwork.kernel.org/cover/11055499/

> switch. Extending devfreq governor (as in one of the dependent patch
> series that you have referred) by attaching to this notification
> chain makes sense only when the SchedUtil and fast_switch is not in 
> use.

fast_switch and cpu notifier chains
are mutually exclusive but schedutil
will still operate in the slow path
IIRC.

> The Schedutil CPUFreq governor might use the fast_switch from this
> driver and the notifications will not be triggered. I have also
> commented patch 08/10 which tries to disable it.
> 
> Regards,
> Lukasz
> 
> On 1/27/20 8:03 PM, Sibi Sankar wrote:
>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>> SDM845 and SC7180 SoCs.
>> 
>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>                  a single device. I am yet to fix the debugfs to
>> 		support multiple opp_tables per device but wanted to
>> 		send what was working upstream to get an idea if multiple
>> 		opp tables per device is a feature that will be useful
>> 		upstream.
>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>                   and SC7180 SoCs.
>> 
>> v3:
>>   * Migrated to using Saravana's opp-kBps bindings [1]
>>   * Fixed some misc comments from Rajendra
>>   * Added support for SC7180
>> 
>> v2:
>>   * Incorporated Viresh's comments from:
>>   
>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>   
>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>   * Dropped cpufreq-map passive governor
>> 
>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>> 
>> Some alternate ways of hosting the opp-tables:
>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
>> Viresh didn't really like ^^ bindings and they dont really scale well. 
>> Just
>> including them here for completeness.
>> 
>> Depends on the following series:
>> [1] https://patchwork.kernel.org/cover/11277199/
>> [2] https://patchwork.kernel.org/cover/11055499/
>> [3] https://patchwork.kernel.org/cover/11326381/
>> 
>> Sibi Sankar (10):
>>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>    OPP: Add and export helper to update voltage
>>    opp: of: export _opp_of_get_opp_desc_node
>>    opp: Allow multiple opp_tables to be mapped to a single device
>>    opp: Remove multiple attached opp tables from a device
>>    cpufreq: qcom: Update the bandwidth levels on frequency change
>>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>>    arm64: dts: qcom: sc7180: Add cpu OPP tables
>> 
>>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 
>> ++++++++++++++++++++++++
>>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>   drivers/opp/core.c                      | 111 +++++-
>>   drivers/opp/of.c                        |   3 +-
>>   drivers/opp/opp.h                       |   2 +
>>   include/linux/pm_opp.h                  |  10 +
>>   9 files changed, 1083 insertions(+), 33 deletions(-)
>> 

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Lukasz Luba]

On 1/29/20 2:37 PM, Sibi Sankar wrote:
> Hey Lukasz,
> Thanks for taking time to review
> the series!
> 
> On 2020-01-29 15:16, Lukasz Luba wrote:
>> Hi Sibi,
>>
>> In my opinion this solution depends on not always true assumption that
>> CPUFreq notification chain will be triggered when there is a frequency
> 
> This series does not create any
> devfreq devices nor use the cpufreq
> notification chain. It just relies
> on the opening up of required-opps
> from being exclusive to gen-pd using
> patches 1,2,3 from patch series[1].
> With the fast path disabled and
> schedutil enabled, this series will
> not miss any cpufreq changes.

Thank you Sibi for clarifying this. I spotted in patch 08/10
that the fast_switch is removed and the normal path is in use,
which also triggers the notification chain in CPUFreq. Then maybe other
machinery which you have put in your 'depends on' list accidentally
works thanks to this.

> 
> [1] https://patchwork.kernel.org/cover/11055499/
> 
>> switch. Extending devfreq governor (as in one of the dependent patch
>> series that you have referred) by attaching to this notification
>> chain makes sense only when the SchedUtil and fast_switch is not in use.
> 
> fast_switch and cpu notifier chains
> are mutually exclusive but schedutil
> will still operate in the slow path
> IIRC.

True, SchedUtil would work in slow_path. The driver and SoC support
'fast_switch', your solution when is properly initialized, disables it.
I would suggest to put this information in the commit message.
I don't know the side effects on the performance, though. The other
side effect would be: the CPUFreq notifications will be triggered.

The fast_switch is now the preferred way, any new ideas should
consider also this path.

> 
>> The Schedutil CPUFreq governor might use the fast_switch from this
>> driver and the notifications will not be triggered. I have also
>> commented patch 08/10 which tries to disable it.
>>
>> Regards,
>> Lukasz
>>
>> On 1/27/20 8:03 PM, Sibi Sankar wrote:
>>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>>> SDM845 and SC7180 SoCs.
>>>
>>> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
>>> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>>>                  a single device. I am yet to fix the debugfs to
>>>         support multiple opp_tables per device but wanted to
>>>         send what was working upstream to get an idea if multiple
>>>         opp tables per device is a feature that will be useful
>>>         upstream.
>>> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>>>                   and SC7180 SoCs.
>>>
>>> v3:
>>>   * Migrated to using Saravana's opp-kBps bindings [1]
>>>   * Fixed some misc comments from Rajendra
>>>   * Added support for SC7180
>>>
>>> v2:
>>>   * Incorporated Viresh's comments from:
>>> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>>> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>>>   * Dropped cpufreq-map passive governor
>>>
>>> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
>>>
>>> Some alternate ways of hosting the opp-tables:
>>> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3 
>>>
>>> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98 
>>>
>>> Viresh didn't really like ^^ bindings and they dont really scale 
>>> well. Just
>>> including them here for completeness.
>>>
>>> Depends on the following series:
>>> [1] https://patchwork.kernel.org/cover/11277199/
>>> [2] https://patchwork.kernel.org/cover/11055499/
>>> [3] https://patchwork.kernel.org/cover/11326381/
>>>
>>> Sibi Sankar (10):
>>>    arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>>>    cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>>>    cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>>>    OPP: Add and export helper to update voltage
>>>    opp: of: export _opp_of_get_opp_desc_node
>>>    opp: Allow multiple opp_tables to be mapped to a single device
>>>    opp: Remove multiple attached opp tables from a device
>>>    cpufreq: qcom: Update the bandwidth levels on frequency change
>>>    arm64: dts: qcom: sdm845: Add cpu OPP tables
>>>    arm64: dts: qcom: sc7180: Add cpu OPP tables
>>>
>>>   arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>>>   arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>>>   arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>>>   drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>>>   drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>>>   drivers/opp/core.c                      | 111 +++++-
>>>   drivers/opp/of.c                        |   3 +-
>>>   drivers/opp/opp.h                       |   2 +
>>>   include/linux/pm_opp.h                  |  10 +
>>>   9 files changed, 1083 insertions(+), 33 deletions(-)
>>>
> 

Re: [RFC v3 09/10] arm64: dts: qcom: sdm845: Add cpu OPP tables

[Matthias Kaehlcke]

Hi Sibi,

On Wed, Jan 29, 2020 at 07:35:21PM +0530, Sibi Sankar wrote:
> Hey Matthias,
> 
> Thanks for the review!
> 
> On 2020-01-29 06:54, Matthias Kaehlcke wrote:
> > Hi Sibi,
> > 
> > On Tue, Jan 28, 2020 at 01:33:49AM +0530, Sibi Sankar wrote:
> > > Add OPP tables required to scale DDR/L3 per freq-domain on SDM845
> > > SoCs.
> > > 
> > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sdm845.dtsi | 453
> > > +++++++++++++++++++++++++++
> > >  1 file changed, 453 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > index c036bab49fc03..8cb976118407b 100644
> > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
> > > @@ -199,6 +199,12 @@
> > >  			qcom,freq-domain = <&cpufreq_hw 0>;
> > >  			#cooling-cells = <2>;
> > >  			next-level-cache = <&L2_0>;
> > > +			operating-points-v2 = <&cpu0_opp_table>,
> > > +					      <&cpu0_ddr_bw_opp_table>,
> > > +					      <&cpu0_l3_bw_opp_table>;
> > > +			interconnects = <&gladiator_noc MASTER_APPSS_PROC &mem_noc
> > > SLAVE_EBI1>,
> > > +					<&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>;
> > 
> > This apparently depends on the 'Split SDM845 interconnect nodes and
> > consolidate RPMh support' series
> > (https://patchwork.kernel.org/project/linux-arm-msm/list/?series=226281),
> > which isn't mentioned in the cover letter.
> > 
> > I also couldn't find a patch on the lists that adds the 'osm_l3'
> > interconnect node for SDM845. The same is true for SC7180 (next
> > patch of this series). These patches may be available in custom trees,
> > but that isn't really helpful for upstream review.
> 
> yeah I missed adding the interconnect
> refactor dependency and the nodes.
> 
> > 
> > I would suggest to focus on landing the dependencies of this series,
> > before proceding with it (or at least most of them), there are plenty
> > and without the dependencies this series isn't going to land, it also
> > makes it hard for testers and reviewers to get all the pieces
> 
> yes I understand but wanted the series
> out asap because since there are a few
> points where we still havn't reached
> a consensus on.

Ok, I just wanted to make sure we are not burning the limited
maintainer/reviewer bandwidth on code with hard dependencies on
things that aren't moving forward.

> > together. In particular the last post of the series 'Add
> > required-opps support to devfreq passive gov'
> > (https://patchwork.kernel.org/cover/11055499/) is from July 2019 ...
> 
> https://lore.kernel.org/lkml/CAGTfZH37ALwUHd8SpRRrBzZ6x1-++YtzS60_yRQvN-TN6rOzaA@mail.gmail.com/
> 
> The pending patch for lazy linking
> was posted a while back. Now that
> it has a tested-by, majority of the
> series should go in since the devfreq
> maintainers wanted the series pulled
> in.

Thanks for the clarification. For reference the post is
https://patchwork.kernel.org/patch/11048277/#23020727

It sems the series will require at least another re-spin:

  "So once that's (lazy linking) added, I should be able to drop a few
  patches in this series, do some minor updates and then this will be
  good to go."

  https://patchwork.kernel.org/cover/11055499/#23001445

So it looks like we are waiting for the lazy linking patch to
land in the PM/Linus' tree and then a re-spin of the 'Add
required-opps support to devfreq passive gov' series.

Re: [RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev

[Sudeep Holla]

On Tue, Jan 28, 2020 at 12:44:23PM -0800, Matthias Kaehlcke wrote:
> On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote:
> > Add SDM845 to cpufreq-dt-platdev blacklist.
>
> nit: you could mention that cpufreq is handled by the
> 'qcom-cpufreq-hw' driver.
>

IIUC, these platforms get the OPP table from the firmware and there shouldn't
be OPP entries in the DT. If not, why not fix that to avoid more confusion.
Can we make cpu0_node_has_opp_v2_prop return false in short.

--
Regards,
Sudeep

Re: [RFC v3 02/10] cpufreq: blacklist SDM845 in cpufreq-dt-platdev

[Sibi Sankar]

Hey Sudeep,

Thanks for the review!

On 2020-01-30 17:10, Sudeep Holla wrote:
> On Tue, Jan 28, 2020 at 12:44:23PM -0800, Matthias Kaehlcke wrote:
>> On Tue, Jan 28, 2020 at 01:33:42AM +0530, Sibi Sankar wrote:
>> > Add SDM845 to cpufreq-dt-platdev blacklist.
>> 
>> nit: you could mention that cpufreq is handled by the
>> 'qcom-cpufreq-hw' driver.
>> 
> 
> IIUC, these platforms get the OPP table from the firmware and there 
> shouldn't
> be OPP entries in the DT. If not, why not fix that to avoid more 
> confusion.
> Can we make cpu0_node_has_opp_v2_prop return false in short.

The entire point of the series is to
define a way of identifying required
opp-kBps values for L3/DDR at different
cpu frequencies. The required-opps is
chosen as the way of showcasing the
dependencies. Hence the need to for a
superset cpu opp-table consisting of
all possible cpu freq values supported
by the family of SoCs and enabling only
the opps that are supported by the board
from the values read back from firmware.

> 
> --
> Regards,
> Sudeep

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

On 2020-01-28 01:33, Sibi Sankar wrote:
> This RFC series aims to extend cpu based scaling support to L3/DDR on
> SDM845 and SC7180 SoCs.
> 

Hey Viresh/Saravana,

Ping! Can you take a stab at reviewing
the series, it has been on the list for
a while now.

> Patches [1-3] - Blacklist SDM845 and SC7180 in cpufreq-dt-platdev
> Patches [5-7] - Hack in a way to add/remove multiple opp tables to
>                 a single device. I am yet to fix the debugfs to
> 		support multiple opp_tables per device but wanted to
> 		send what was working upstream to get an idea if multiple
> 		opp tables per device is a feature that will be useful
> 		upstream.
> Patches [9-10] - Add the cpu/cpu-ddr/cpu-l3 opp tables for SDM845
>                  and SC7180 SoCs.
> 
> v3:
>  * Migrated to using Saravana's opp-kBps bindings [1]
>  * Fixed some misc comments from Rajendra
>  * Added support for SC7180
> 
> v2:
>  * Incorporated Viresh's comments from:
>  
> https://lore.kernel.org/lkml/20190410102429.r6j6brm5kspmqxc3@vireshk-i7/
>  
> https://lore.kernel.org/lkml/20190410112516.gnh77jcwawvld6et@vireshk-i7/
>  * Dropped cpufreq-map passive governor
> 
> Git-branch: https://github.com/QuinAsura/linux/tree/lnext-012420
> 
> Some alternate ways of hosting the opp-tables:
> https://github.com/QuinAsura/linux/commit/50b92bfaadc8f9a0d1e12249646e018bd6d1a9d3
> https://github.com/QuinAsura/linux/commit/3d23d1eefd16ae6d9e3ef91e93e78749d8844e98
> Viresh didn't really like ^^ bindings and they dont really scale well. 
> Just
> including them here for completeness.
> 
> Depends on the following series:
> [1] https://patchwork.kernel.org/cover/11277199/
> [2] https://patchwork.kernel.org/cover/11055499/
> [3] https://patchwork.kernel.org/cover/11326381/
> 
> Sibi Sankar (10):
>   arm64: dts: qcom: sdm845: Add SoC compatible to MTP
>   cpufreq: blacklist SDM845 in cpufreq-dt-platdev
>   cpufreq: blacklist SC7180 in cpufreq-dt-platdev
>   OPP: Add and export helper to update voltage
>   opp: of: export _opp_of_get_opp_desc_node
>   opp: Allow multiple opp_tables to be mapped to a single device
>   opp: Remove multiple attached opp tables from a device
>   cpufreq: qcom: Update the bandwidth levels on frequency change
>   arm64: dts: qcom: sdm845: Add cpu OPP tables
>   arm64: dts: qcom: sc7180: Add cpu OPP tables
> 
>  arch/arm64/boot/dts/qcom/sc7180.dtsi    | 287 +++++++++++++++
>  arch/arm64/boot/dts/qcom/sdm845-mtp.dts |   2 +-
>  arch/arm64/boot/dts/qcom/sdm845.dtsi    | 453 ++++++++++++++++++++++++
>  drivers/cpufreq/cpufreq-dt-platdev.c    |   2 +
>  drivers/cpufreq/qcom-cpufreq-hw.c       | 246 +++++++++++--
>  drivers/opp/core.c                      | 111 +++++-
>  drivers/opp/of.c                        |   3 +-
>  drivers/opp/opp.h                       |   2 +
>  include/linux/pm_opp.h                  |  10 +
>  9 files changed, 1083 insertions(+), 33 deletions(-)

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Viresh Kumar]

On 18-03-20, 02:13, Sibi Sankar wrote:
> On 2020-01-28 01:33, Sibi Sankar wrote:
> > This RFC series aims to extend cpu based scaling support to L3/DDR on
> > SDM845 and SC7180 SoCs.
> > 
> 
> Hey Viresh/Saravana,
> 
> Ping! Can you take a stab at reviewing
> the series, it has been on the list for
> a while now.

I believe this depends on Saravana's series on which I have raised
some doubts few weeks back ? I am still waiting for them to get
clarified by him.

-- 
viresh

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Rajendra Nayak]

On 3/18/2020 9:12 AM, Viresh Kumar wrote:
> On 18-03-20, 02:13, Sibi Sankar wrote:
>> On 2020-01-28 01:33, Sibi Sankar wrote:
>>> This RFC series aims to extend cpu based scaling support to L3/DDR on
>>> SDM845 and SC7180 SoCs.
>>>
>>
>> Hey Viresh/Saravana,
>>
>> Ping! Can you take a stab at reviewing
>> the series, it has been on the list for
>> a while now.
> 
> I believe this depends on Saravana's series on which I have raised
> some doubts few weeks back ? I am still waiting for them to get
> clarified by him.

Could you please post a link to the discussion that you are referring to here?
I looked at a few links posted in the cover letter as dependencies and it seems
like the discussions are pending for *months* and not weeks but I might have looked
at the wrong ones.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

On 2020-03-19 15:12, Rajendra Nayak wrote:
> On 3/18/2020 9:12 AM, Viresh Kumar wrote:
>> On 18-03-20, 02:13, Sibi Sankar wrote:
>>> On 2020-01-28 01:33, Sibi Sankar wrote:
>>>> This RFC series aims to extend cpu based scaling support to L3/DDR 
>>>> on
>>>> SDM845 and SC7180 SoCs.
>>>> 
>>> 
>>> Hey Viresh/Saravana,
>>> 
>>> Ping! Can you take a stab at reviewing
>>> the series, it has been on the list for
>>> a while now.
>> 
>> I believe this depends on Saravana's series on which I have raised
>> some doubts few weeks back ? I am still waiting for them to get
>> clarified by him.

Viresh,
Saravana's example does show a device
with multiple opp tables but doesn't
need multiple opp table support to
land though (since it works fine with
the current implementation). I am more
interested  in understanding your/
Stephen's/Saravana's stance on adding
multiple opp-table support. Personally
I feel its inevitable, since multiple
qc drivers using interconnect opp-tables,
routinely need vote on multiple paths in
a non-trivial manner.

> 
> Could you please post a link to the discussion that you are referring 
> to here?
> I looked at a few links posted in the cover letter as dependencies and 
> it seems
> like the discussions are pending for *months* and not weeks but I
> might have looked
> at the wrong ones.

https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/

Rajendra,
Viresh is referring to ^^ one

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Viresh Kumar]

On 19-03-20, 15:41, Sibi Sankar wrote:
> Viresh,
> Saravana's example does show a device
> with multiple opp tables but doesn't
> need multiple opp table support to
> land though (since it works fine with
> the current implementation). I am more
> interested  in understanding your/
> Stephen's/Saravana's stance on adding
> multiple opp-table support. Personally
> I feel its inevitable, since multiple
> qc drivers using interconnect opp-tables,
> routinely need vote on multiple paths in
> a non-trivial manner.

The OPP core doesn't support multiple OPP tables for a device and I
don't understand how it will. And so I have been waiting for a reply.

> > 
> > Could you please post a link to the discussion that you are referring to
> > here?
> > I looked at a few links posted in the cover letter as dependencies and
> > it seems
> > like the discussions are pending for *months* and not weeks but I
> > might have looked
> > at the wrong ones.
> 
> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
> 
> Rajendra,
> Viresh is referring to ^^ one

Right, thanks.

-- 
viresh

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Rajendra Nayak]

On 3/19/2020 3:54 PM, Viresh Kumar wrote:
> On 19-03-20, 15:41, Sibi Sankar wrote:
>> Viresh,
>> Saravana's example does show a device
>> with multiple opp tables but doesn't
>> need multiple opp table support to
>> land though (since it works fine with
>> the current implementation). I am more
>> interested  in understanding your/
>> Stephen's/Saravana's stance on adding
>> multiple opp-table support. Personally
>> I feel its inevitable, since multiple
>> qc drivers using interconnect opp-tables,
>> routinely need vote on multiple paths in
>> a non-trivial manner.
> 
> The OPP core doesn't support multiple OPP tables for a device and I
> don't understand how it will. And so I have been waiting for a reply.

I thought this series indeed is proposing to add that support in OPP core?
a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"

> 
>>>
>>> Could you please post a link to the discussion that you are referring to
>>> here?
>>> I looked at a few links posted in the cover letter as dependencies and
>>> it seems
>>> like the discussions are pending for *months* and not weeks but I
>>> might have looked
>>> at the wrong ones.
>>
>> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
>>
>> Rajendra,
>> Viresh is referring to ^^ one
> 
> Right, thanks.

These discussions are stalled for over 2 months now waiting on a response from Saravana.
Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?


-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Sibi Sankar]

On 2020-03-19 15:54, Viresh Kumar wrote:
> On 19-03-20, 15:41, Sibi Sankar wrote:
>> Viresh,
>> Saravana's example does show a device
>> with multiple opp tables but doesn't
>> need multiple opp table support to
>> land though (since it works fine with
>> the current implementation). I am more
>> interested  in understanding your/
>> Stephen's/Saravana's stance on adding
>> multiple opp-table support. Personally
>> I feel its inevitable, since multiple
>> qc drivers using interconnect opp-tables,
>> routinely need vote on multiple paths in
>> a non-trivial manner.
> 
> The OPP core doesn't support multiple OPP tables for a device and I
> don't understand how it will. And so I have been waiting for a reply.

This series tries to add minimal support
for multiple opp-tables per device and
can be tested on db845c/sdm845mtp/sc7180.
Debugfs still needs to be fixed though and
fixing it did feel non-trivial (I can get
to it if we reach a consensus). Perhaps we
can fork out icc oppp-tables from regular
opp-tables and allow only multiple instances
of icc opp-tables per device(though Rob
didn't like the bindings associated with it)
that way all the current users wont be
affected but from what I've tested the
series shouldn't cause any regression.

> 
>> >
>> > Could you please post a link to the discussion that you are referring to
>> > here?
>> > I looked at a few links posted in the cover letter as dependencies and
>> > it seems
>> > like the discussions are pending for *months* and not weeks but I
>> > might have looked
>> > at the wrong ones.
>> 
>> https://lore.kernel.org/lkml/20200114103448.odnvqawnqb3twst5@vireshk-i7/
>> 
>> Rajendra,
>> Viresh is referring to ^^ one
> 
> Right, thanks.

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

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Viresh Kumar]

On 19-03-20, 16:23, Rajendra Nayak wrote:
> 
> 
> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
> I thought this series indeed is proposing to add that support in OPP core?
> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
> 
> These discussions are stalled for over 2 months now waiting on a response from Saravana.
> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?

I agree and I am equally worried about it. So lets clear the air a bit
first. Can someone answer following :

- This series depends on the series from Saravana ? Right, so that
  needs to get merged/accepted first ?

- If yes, then what is the way forward as Saravana isn't responding
  right now ..

-- 
viresh

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Rajendra Nayak]

On 3/19/2020 4:38 PM, Viresh Kumar wrote:
> On 19-03-20, 16:23, Rajendra Nayak wrote:
>>
>>
>> On 3/19/2020 3:54 PM, Viresh Kumar wrote:
>> I thought this series indeed is proposing to add that support in OPP core?
>> a.k.a "[RFC v3 06/10] opp: Allow multiple opp_tables to be mapped to a single device"
>>
>> These discussions are stalled for over 2 months now waiting on a response from Saravana.
>> Viresh, whats the way forward here and how long do we plan on waiting for Saravanas response?
> 
> I agree and I am equally worried about it. So lets clear the air a bit
> first. Can someone answer following :
> 
> - This series depends on the series from Saravana ? Right, so that
>    needs to get merged/accepted first ?
> 
> - If yes, then what is the way forward as Saravana isn't responding
>    right now ..

sure, I understand there is a dependency, however refusing to review the approach
(to add multiple OPPS tables per device) that this series is taking because of an outstanding
question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
seems odd.

Its fine if you are not happy with the approach taken here and you can propose something else,
but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
and hence the request to review the patches.

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [RFC v3 00/10] DDR/L3 Scaling support on SDM845 and SC7180 SoCs

[Viresh Kumar]

On 19-03-20, 17:03, Rajendra Nayak wrote:
> sure, I understand there is a dependency, however refusing to review the approach
> (to add multiple OPPS tables per device) that this series is taking because of an outstanding
> question which, if I read it right is "We can not add multiple OPP tables for a single device right now"
> seems odd.
> 
> Its fine if you are not happy with the approach taken here and you can propose something else,
> but it looks inevitable that we would need something like this to be supported (multiple OPP tables per device)
> and hence the request to review the patches.

I understand the frustration you have right now, but honestly most of
the delay is not from my part. I normally try to be very quick in
giving reviews or even accepting material for next cycle. Still, I am
part of the chain which has blocked this series and I am really sorry
for that.

Coming back to reviews. I never liked the idea of keeping separate OPP
tables just for the relation that a peripheral share with the
interconnect, i.e. the b/w tables getting discussed right now.

And I am not at all sure if such tables should be added as device
tables anyway, it was present in cover letter of Saravana for sometime
but not in the real patches and so I missed it until the time Sibi
asked me something on IRC.

To be clear, I don't think we will allow multiple active OPP tables
for a device for now, unless we have a very good reason to do that. It
just doesn't make sense to me to have one table for keeping
frequency/voltage thing, and another set of tables for b/w
requirements from interconnect.

So, the changes proposed by this patchset even doesn't matter as I am
not agreeing on the binding itself. And we need a binding change for
that, which Saravana never sent, i.e. to have multiple active tables
for a device.

Yes, we need a solution for this and a quick one. I am fine to discuss
this all on a hangout session (anyway, everyone is working from home
now anyways) anytime next week with the interested parties. I think
/me and Vincent can join it.

-- 
viresh