Re: [PATCH v7 4/7] OPP: Add support for parsing interconnect bandwidth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Georgi,

Apart from the Matthias's comments
ran into a few issues during testing.

On 2020-04-24 21:24, Georgi Djakov wrote:
The OPP bindings now support bandwidth values, so add support to parse it from device tree and store it into the new dev_pm_opp_icc_bw struct, which
is part of the dev_pm_opp.

Signed-off-by: Georgi Djakov <georgi.djakov@xxxxxxxxxx>
---
v7:
* Addressed some review comments from Viresh and Sibi.
* Various other changes.

v2:
https://lore.kernel.org/linux-arm-msm/20190423132823.7915-4-georgi.djakov@xxxxxxxxxx/

 drivers/opp/Kconfig    |   1 +
 drivers/opp/core.c     |  16 +++++-
 drivers/opp/of.c       | 119 ++++++++++++++++++++++++++++++++++++++++-
 drivers/opp/opp.h      |   9 ++++
 include/linux/pm_opp.h |  12 +++++
 5 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/drivers/opp/Kconfig b/drivers/opp/Kconfig
index 35dfc7e80f92..230d2b84436c 100644
--- a/drivers/opp/Kconfig
+++ b/drivers/opp/Kconfig
@@ -2,6 +2,7 @@
 config PM_OPP
 	bool
 	select SRCU
+	depends on INTERCONNECT || !INTERCONNECT
 	---help---
 	  SOCs have a standard set of tuples consisting of frequency and
 	  voltage pairs that the device will support per voltage domain. This
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index c9c1bbe6ae27..8e86811eb7b2 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -985,6 +985,12 @@ static struct opp_table
*_allocate_opp_table(struct device *dev, int index)
 				ret);
 	}

+	/* Find interconnect path(s) for the device */
+	ret = _of_find_paths(opp_table, dev);
+	if (ret)
+		dev_dbg(dev, "%s: Error finding interconnect paths: %d\n",
+			__func__, ret);
+
 	BLOCKING_INIT_NOTIFIER_HEAD(&opp_table->head);
 	INIT_LIST_HEAD(&opp_table->opp_list);
 	kref_init(&opp_table->kref);
@@ -1229,19 +1235,22 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_remove_all_dynamic);
 struct dev_pm_opp *_opp_allocate(struct opp_table *table)
 {
 	struct dev_pm_opp *opp;
-	int count, supply_size;
+	int count, supply_size, icc_size;

 	/* Allocate space for at least one supply */
 	count = table->regulator_count > 0 ? table->regulator_count : 1;
 	supply_size = sizeof(*opp->supplies) * count;
+	icc_size = sizeof(*opp->bandwidth) * table->path_count;

 	/* allocate new OPP node and supplies structures */
-	opp = kzalloc(sizeof(*opp) + supply_size, GFP_KERNEL);
+	opp = kzalloc(sizeof(*opp) + supply_size + icc_size, GFP_KERNEL);
+
 	if (!opp)
 		return NULL;

/* Put the supplies at the end of the OPP structure as an empty array */
 	opp->supplies = (struct dev_pm_opp_supply *)(opp + 1);
+	opp->bandwidth = (struct dev_pm_opp_icc_bw *)(opp->supplies + 1);
 	INIT_LIST_HEAD(&opp->node);

 	return opp;
@@ -1276,6 +1285,9 @@ int _opp_compare_key(struct dev_pm_opp *opp1,
struct dev_pm_opp *opp2)
 {
 	if (opp1->rate != opp2->rate)
 		return opp1->rate < opp2->rate ? -1 : 1;
+	if (opp1->bandwidth && opp2->bandwidth &&
+	    opp1->bandwidth[0].peak != opp2->bandwidth[0].peak)
+		return opp1->bandwidth[0].peak < opp2->bandwidth[0].peak ? -1 : 1;
 	if (opp1->level != opp2->level)
 		return opp1->level < opp2->level ? -1 : 1;
 	return 0;
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index e33169c7e045..978e445b0cdb 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -332,6 +332,59 @@ static int _of_opp_alloc_required_opps(struct
opp_table *opp_table,
 	return ret;
 }

+int _of_find_paths(struct opp_table *opp_table, struct device *dev)
+{
+	struct device_node *np;
+	int ret, i, count, num_paths;
+
+	np = of_node_get(dev->of_node);
+	if (!np)
+		return 0;
+
+	count = of_count_phandle_with_args(np, "interconnects",
+					   "#interconnect-cells");
+	of_node_put(np);
+	if (count < 0)
+		return 0;
+
+	/* two phandles when #interconnect-cells = <1> */
+	if (count % 2) {
+		dev_err(dev, "%s: Invalid interconnects values\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	num_paths = count / 2;
+	opp_table->paths = kcalloc(num_paths, sizeof(*opp_table->paths),
+				   GFP_KERNEL);
+	if (!opp_table->paths)
+		return -ENOMEM;
+
+	for (i = 0; i < num_paths; i++) {
+		opp_table->paths[i] = of_icc_get_by_index(dev, i);

now that icc_get is part of the
opp core framework shouldn't we
do a put on table remove as well?

+		if (IS_ERR(opp_table->paths[i])) {
+			ret = PTR_ERR(opp_table->paths[i]);
+			if (ret != -EPROBE_DEFER) {
+				dev_err(dev, "%s: Unable to get path%d: %d\n",
+					__func__, i, ret);
+			}
+			goto err;
+		}
+	}
+	opp_table->path_count = num_paths;
+
+	return 0;
+
+err:
+	while (i--)
+		icc_put(opp_table->paths[i]);
+
+	kfree(opp_table->paths);
+	opp_table->paths = NULL;
+
+	return ret;
+}
+
static bool _opp_is_supported(struct device *dev, struct opp_table *opp_table,
 			      struct device_node *np)
 {
@@ -524,8 +577,11 @@ EXPORT_SYMBOL_GPL(dev_pm_opp_of_remove_table);
static int _read_opp_key(struct dev_pm_opp *new_opp, struct device_node *np,
 			 bool *rate_not_available)
 {
+	struct property *peak, *avg;
+	u32 *peak_bw, *avg_bw;
 	u64 rate;
-	int ret;
+	int ret, i, count;
+	bool found = false;

 	ret = of_property_read_u64(np, "opp-hz", &rate);
 	if (!ret) {
@@ -535,10 +591,69 @@ static int _read_opp_key(struct dev_pm_opp
*new_opp, struct device_node *np,
 		 * bit guaranteed in clk API.
 		 */
 		new_opp->rate = (unsigned long)rate;
+		found = true;
 	}
 	*rate_not_available = !!ret;

-	of_property_read_u32(np, "opp-level", &new_opp->level);
+	peak = of_find_property(np, "opp-peak-kBps", NULL);
+	if (peak) {
+		/*
+		 * Bandwidth consists of peak and average (optional) values:
+		 * opp-peak-kBps = <path1_value path2_value>;
+		 * opp-avg-kBps = <path1_value path2_value>;
+		 */
+		count = peak->length / sizeof(u32);
+		peak_bw = kmalloc_array(count, sizeof(*peak_bw), GFP_KERNEL);
+		if (!peak_bw)
+			return -ENOMEM;
+
+		ret = of_property_read_u32_array(np, "opp-peak-kBps", peak_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-peak-kBps: %d\n",
+			       __func__, ret);
+			goto free_peak_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].peak = kBps_to_icc(peak_bw[i]);
+
+		found = true;
+	}
+
+	avg = of_find_property(np, "opp-avg-kBps", NULL);
+	if (peak && avg) {
+		count = avg->length / sizeof(u32);
+		avg_bw = kmalloc_array(count, sizeof(*avg_bw), GFP_KERNEL);
+		if (!avg_bw) {
+			ret = -ENOMEM;
+			goto free_peak_bw;
+		}
+
+		ret = of_property_read_u32_array(np, "opp-avg-kBps", avg_bw,
+						 count);
+		if (ret) {
+			pr_err("%s: Error parsing opp-avg-kBps: %d\n",
+			       __func__, ret);
+			goto free_avg_bw;
+		}
+
+		for (i = 0; i < count; i++)
+			new_opp->bandwidth[i].avg = kBps_to_icc(avg_bw[i]);
+	}
+
+	if (of_property_read_u32(np, "opp-level", &new_opp->level))

of_property_read_u32 returns 0 on
success so should use logical not
instead ^^

+		found = true;
+
+	if (found)
+		return 0;
+
+	return ret;
+
+free_avg_bw:
+	kfree(avg_bw);
+free_peak_bw:
+	kfree(peak_bw);

 	return ret;
 }
diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h
index bcadb1e328a4..2e0113360297 100644
--- a/drivers/opp/opp.h
+++ b/drivers/opp/opp.h
@@ -12,6 +12,7 @@
 #define __DRIVER_OPP_H__

 #include <linux/device.h>
+#include <linux/interconnect.h>
 #include <linux/kernel.h>
 #include <linux/kref.h>
 #include <linux/list.h>
@@ -59,6 +60,7 @@ extern struct list_head opp_tables;
  * @rate:	Frequency in hertz
  * @level:	Performance level
  * @supplies:	Power supplies voltage/current values
+ * @bandwidth:	Interconnect bandwidth values
* @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's
  *		frequency from any other OPP's frequency.
  * @required_opps: List of OPPs that are required by this OPP.
@@ -81,6 +83,7 @@ struct dev_pm_opp {
 	unsigned int level;

 	struct dev_pm_opp_supply *supplies;
+	struct dev_pm_opp_icc_bw *bandwidth;

 	unsigned long clock_latency_ns;

@@ -146,6 +149,8 @@ enum opp_table_access {
* @regulator_count: Number of power supply regulators. Its value can be -1 * (uninitialized), 0 (no opp-microvolt property) or > 0 (has opp-microvolt
  * property).
+ * @paths: Interconnect path handles
+ * @path_count: Number of interconnect paths
* @genpd_performance_state: Device's power domain support performance state.
  * @is_genpd: Marks if the OPP table belongs to a genpd.
  * @set_opp: Platform specific set_opp callback
@@ -189,6 +194,8 @@ struct opp_table {
 	struct clk *clk;
 	struct regulator **regulators;
 	int regulator_count;
+	struct icc_path **paths;
+	unsigned int path_count;
 	bool genpd_performance_state;
 	bool is_genpd;

@@ -224,12 +231,14 @@ void _of_clear_opp_table(struct opp_table *opp_table);
 struct opp_table *_managed_opp(struct device *dev, int index);
 void _of_opp_free_required_opps(struct opp_table *opp_table,
 				struct dev_pm_opp *opp);
+int _of_find_paths(struct opp_table *opp_table, struct device *dev);
 #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 void _of_opp_free_required_opps(struct opp_table *opp_table,
 					      struct dev_pm_opp *opp) {}
+static inline int _of_find_paths(struct opp_table *opp_table, struct
device *dev) { return 0; }
 #endif

 #ifdef CONFIG_DEBUG_FS
diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
index 747861816f4f..cfceb0290401 100644
--- a/include/linux/pm_opp.h
+++ b/include/linux/pm_opp.h
@@ -41,6 +41,18 @@ struct dev_pm_opp_supply {
 	unsigned long u_amp;
 };

+/**
+ * struct dev_pm_opp_icc_bw - Interconnect bandwidth values
+ * @avg:	Average bandwidth corresponding to this OPP (in icc units)
+ * @peak:	Peak bandwidth corresponding to this OPP (in icc units)
+ *
+ * This structure stores the bandwidth values for a single interconnect path.
+ */
+struct dev_pm_opp_icc_bw {
+	u32 avg;
+	u32 peak;
+};
+
 /**
  * struct dev_pm_opp_info - OPP freq/voltage/current values
  * @rate:	Target clk rate in hz

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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux