[v2,1/2] interconnect: Add support for path tags

Message ID 20190618091724.28232-2-georgi.djakov@linaro.org
State Superseded
Headers show
Series
  • interconnect: Add path tagging support
Related show

Commit Message

Georgi Djakov June 18, 2019, 9:17 a.m.
Consumers may have use cases with different bandwidth requirements based
on the system or driver state. The consumer driver can append a specific
tag to the path and pass this information to the interconnect platform
driver to do the aggregation based on this state.

Introduce icc_set_tag() function that will allow the consumers to append
an optional tag to each path. The aggregation of these tagged paths is
platform specific.

Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

---
 drivers/interconnect/core.c           | 24 +++++++++++++++++++++++-
 drivers/interconnect/qcom/sdm845.c    |  2 +-
 include/linux/interconnect-provider.h |  4 ++--
 include/linux/interconnect.h          |  5 +++++
 4 files changed, 31 insertions(+), 4 deletions(-)

Comments

Evan Green July 11, 2019, 5:06 p.m. | #1
Hi Georgi and David,

On Tue, Jun 18, 2019 at 2:17 AM Georgi Djakov <georgi.djakov@linaro.org> wrote:
>

> Consumers may have use cases with different bandwidth requirements based

> on the system or driver state. The consumer driver can append a specific

> tag to the path and pass this information to the interconnect platform

> driver to do the aggregation based on this state.

>

> Introduce icc_set_tag() function that will allow the consumers to append

> an optional tag to each path. The aggregation of these tagged paths is

> platform specific.

>

> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>

> ---

>  drivers/interconnect/core.c           | 24 +++++++++++++++++++++++-

>  drivers/interconnect/qcom/sdm845.c    |  2 +-

>  include/linux/interconnect-provider.h |  4 ++--

>  include/linux/interconnect.h          |  5 +++++

>  4 files changed, 31 insertions(+), 4 deletions(-)

>

> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c

> index 871eb4bc4efc..251354bb7fdc 100644

> --- a/drivers/interconnect/core.c

> +++ b/drivers/interconnect/core.c

> @@ -29,6 +29,7 @@ static struct dentry *icc_debugfs_dir;

>   * @req_node: entry in list of requests for the particular @node

>   * @node: the interconnect node to which this constraint applies

>   * @dev: reference to the device that sets the constraints

> + * @tag: path tag (optional)

>   * @avg_bw: an integer describing the average bandwidth in kBps

>   * @peak_bw: an integer describing the peak bandwidth in kBps

>   */

> @@ -36,6 +37,7 @@ struct icc_req {

>         struct hlist_node req_node;

>         struct icc_node *node;

>         struct device *dev;

> +       u32 tag;

>         u32 avg_bw;

>         u32 peak_bw;

>  };

> @@ -204,7 +206,7 @@ static int aggregate_requests(struct icc_node *node)

>         node->peak_bw = 0;

>

>         hlist_for_each_entry(r, &node->req_list, req_node)

> -               p->aggregate(node, r->avg_bw, r->peak_bw,

> +               p->aggregate(node, r->tag, r->avg_bw, r->peak_bw,

>                              &node->avg_bw, &node->peak_bw);

>

>         return 0;

> @@ -385,6 +387,26 @@ struct icc_path *of_icc_get(struct device *dev, const char *name)

>  }

>  EXPORT_SYMBOL_GPL(of_icc_get);

>

> +/**

> + * icc_set_tag() - set an optional tag on a path

> + * @path: the path we want to tag

> + * @tag: the tag value

> + *

> + * This function allows consumers to append a tag to the requests associated

> + * with a path, so that a different aggregation could be done based on this tag.

> + */

> +void icc_set_tag(struct icc_path *path, u32 tag)

> +{

> +       int i;

> +

> +       if (!path)

> +               return;

> +

> +       for (i = 0; i < path->num_nodes; i++)

> +               path->reqs[i].tag = tag;


It's a little unfortunate to have this tag sprayed across all the
request nodes in the path, even though it's really a single value. If
we thought there were likely to be more attributes common to a path
that a provider might want access to, we could add a pointer to the
path in icc_req, then stick the tag in the path. But if the tag ends
up being the only thing worth looking at, then I guess what you have
now is slightly better.

Patch

diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 871eb4bc4efc..251354bb7fdc 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -29,6 +29,7 @@  static struct dentry *icc_debugfs_dir;
  * @req_node: entry in list of requests for the particular @node
  * @node: the interconnect node to which this constraint applies
  * @dev: reference to the device that sets the constraints
+ * @tag: path tag (optional)
  * @avg_bw: an integer describing the average bandwidth in kBps
  * @peak_bw: an integer describing the peak bandwidth in kBps
  */
@@ -36,6 +37,7 @@  struct icc_req {
 	struct hlist_node req_node;
 	struct icc_node *node;
 	struct device *dev;
+	u32 tag;
 	u32 avg_bw;
 	u32 peak_bw;
 };
@@ -204,7 +206,7 @@  static int aggregate_requests(struct icc_node *node)
 	node->peak_bw = 0;
 
 	hlist_for_each_entry(r, &node->req_list, req_node)
-		p->aggregate(node, r->avg_bw, r->peak_bw,
+		p->aggregate(node, r->tag, r->avg_bw, r->peak_bw,
 			     &node->avg_bw, &node->peak_bw);
 
 	return 0;
@@ -385,6 +387,26 @@  struct icc_path *of_icc_get(struct device *dev, const char *name)
 }
 EXPORT_SYMBOL_GPL(of_icc_get);
 
+/**
+ * icc_set_tag() - set an optional tag on a path
+ * @path: the path we want to tag
+ * @tag: the tag value
+ *
+ * This function allows consumers to append a tag to the requests associated
+ * with a path, so that a different aggregation could be done based on this tag.
+ */
+void icc_set_tag(struct icc_path *path, u32 tag)
+{
+	int i;
+
+	if (!path)
+		return;
+
+	for (i = 0; i < path->num_nodes; i++)
+		path->reqs[i].tag = tag;
+}
+EXPORT_SYMBOL_GPL(icc_set_tag);
+
 /**
  * icc_set_bw() - set bandwidth constraints on an interconnect path
  * @path: reference to the path returned by icc_get()
diff --git a/drivers/interconnect/qcom/sdm845.c b/drivers/interconnect/qcom/sdm845.c
index 4915b78da673..fb526004c82e 100644
--- a/drivers/interconnect/qcom/sdm845.c
+++ b/drivers/interconnect/qcom/sdm845.c
@@ -626,7 +626,7 @@  static void bcm_aggregate(struct qcom_icc_bcm *bcm)
 	bcm->dirty = false;
 }
 
-static int qcom_icc_aggregate(struct icc_node *node, u32 avg_bw,
+static int qcom_icc_aggregate(struct icc_node *node, u32 tag, u32 avg_bw,
 			      u32 peak_bw, u32 *agg_avg, u32 *agg_peak)
 {
 	size_t i;
diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
index 63caccadc2db..4ee19fd41568 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -45,8 +45,8 @@  struct icc_provider {
 	struct list_head	provider_list;
 	struct list_head	nodes;
 	int (*set)(struct icc_node *src, struct icc_node *dst);
-	int (*aggregate)(struct icc_node *node, u32 avg_bw, u32 peak_bw,
-			 u32 *agg_avg, u32 *agg_peak);
+	int (*aggregate)(struct icc_node *node, u32 tag, u32 avg_bw,
+			 u32 peak_bw, u32 *agg_avg, u32 *agg_peak);
 	struct icc_node* (*xlate)(struct of_phandle_args *spec, void *data);
 	struct device		*dev;
 	int			users;
diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
index dc25864755ba..d70a914cba11 100644
--- a/include/linux/interconnect.h
+++ b/include/linux/interconnect.h
@@ -30,6 +30,7 @@  struct icc_path *icc_get(struct device *dev, const int src_id,
 struct icc_path *of_icc_get(struct device *dev, const char *name);
 void icc_put(struct icc_path *path);
 int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw);
+void icc_set_tag(struct icc_path *path, u32 tag);
 
 #else
 
@@ -54,6 +55,10 @@  static inline int icc_set_bw(struct icc_path *path, u32 avg_bw, u32 peak_bw)
 	return 0;
 }
 
+static inline void icc_set_tag(struct icc_path *path, u32 tag)
+{
+}
+
 #endif /* CONFIG_INTERCONNECT */
 
 #endif /* __LINUX_INTERCONNECT_H */