SoCs that have multiple coexisting CPUs and DSPs, may have shared
interconnect buses between them. In such cases, each CPU/DSP may have
different bandwidth needs, depending on whether it is active or sleeping.
This means that we have to keep different bandwidth configurations for
the CPU (active/sleep). In such systems, usually there is a way to
communicate and synchronize this information with some firmware or pass
it to another processor responsible for monitoring and switching the
interconnect configurations based on the state of each CPU/DSP.
The above problem can be solved by introducing the path tagging concept,
that allows consumers to optionally attach a tag to each path they use.
This tag is used to differentiate between the aggregated bandwidth values
for each state. The tag is generic and how it's handled is up to the
platform specific interconnect provider drivers.
v3:
- New patch to add a pre_aggregate() callback.
v2: https://lore.kernel.org/lkml/[email protected]/
- Store tag with the request. (Evan)
- Reorganize the code to save bandwidth values into buckets and use the
tag as a bitfield. (Evan)
- Clear the aggregated values after icc_set().
v1: https://lore.kernel.org/lkml/[email protected]/
David Dai (1):
interconnect: qcom: Add tagging and wake/sleep support for sdm845
Georgi Djakov (2):
interconnect: Add support for path tags
interconnect: Add pre_aggregate() callback
drivers/interconnect/core.c | 27 ++++-
drivers/interconnect/qcom/sdm845.c | 136 ++++++++++++++++++++------
include/linux/interconnect-provider.h | 7 +-
include/linux/interconnect.h | 5 +
4 files changed, 140 insertions(+), 35 deletions(-)
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 <[email protected]>
---
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;
+}
+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 */
Introduce an optional callback in interconnect provider drivers. It can be
used for implementing actions, that need to be executed before the actual
aggregation of the bandwidth requests has started.
The benefit of this for now is that it will significantly simplify the code
in provider drivers.
Suggested-by: Evan Green <[email protected]>
Signed-off-by: Georgi Djakov <[email protected]>
---
drivers/interconnect/core.c | 3 +++
include/linux/interconnect-provider.h | 3 +++
2 files changed, 6 insertions(+)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 251354bb7fdc..7b971228df38 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -205,6 +205,9 @@ static int aggregate_requests(struct icc_node *node)
node->avg_bw = 0;
node->peak_bw = 0;
+ if (p->pre_aggregate)
+ p->pre_aggregate(node);
+
hlist_for_each_entry(r, &node->req_list, req_node)
p->aggregate(node, r->tag, r->avg_bw, r->peak_bw,
&node->avg_bw, &node->peak_bw);
diff --git a/include/linux/interconnect-provider.h b/include/linux/interconnect-provider.h
index 4ee19fd41568..b16f9effa555 100644
--- a/include/linux/interconnect-provider.h
+++ b/include/linux/interconnect-provider.h
@@ -36,6 +36,8 @@ struct icc_node *of_icc_xlate_onecell(struct of_phandle_args *spec,
* @nodes: internal list of the interconnect provider nodes
* @set: pointer to device specific set operation function
* @aggregate: pointer to device specific aggregate operation function
+ * @pre_aggregate: pointer to device specific function that is called
+ * before the aggregation begins (optional)
* @xlate: provider-specific callback for mapping nodes from phandle arguments
* @dev: the device this interconnect provider belongs to
* @users: count of active users
@@ -47,6 +49,7 @@ struct icc_provider {
int (*set)(struct icc_node *src, struct icc_node *dst);
int (*aggregate)(struct icc_node *node, u32 tag, u32 avg_bw,
u32 peak_bw, u32 *agg_avg, u32 *agg_peak);
+ void (*pre_aggregate)(struct icc_node *node);
struct icc_node* (*xlate)(struct of_phandle_args *spec, void *data);
struct device *dev;
int users;
From: David Dai <[email protected]>
Add support for wake and sleep commands by using a tag to indicate
whether or not the aggregate and set requests fall into execution
state specific bucket.
Signed-off-by: David Dai <[email protected]>
Signed-off-by: Georgi Djakov <[email protected]>
---
drivers/interconnect/qcom/sdm845.c | 134 ++++++++++++++++++++++-------
1 file changed, 103 insertions(+), 31 deletions(-)
diff --git a/drivers/interconnect/qcom/sdm845.c b/drivers/interconnect/qcom/sdm845.c
index fb526004c82e..b2047d1c6d84 100644
--- a/drivers/interconnect/qcom/sdm845.c
+++ b/drivers/interconnect/qcom/sdm845.c
@@ -66,6 +66,17 @@ struct bcm_db {
#define SDM845_MAX_BCM_PER_NODE 2
#define SDM845_MAX_VCD 10
+#define QCOM_ICC_BUCKET_AMC 0
+#define QCOM_ICC_BUCKET_WAKE 1
+#define QCOM_ICC_BUCKET_SLEEP 2
+#define QCOM_ICC_NUM_BUCKETS 3
+#define QCOM_ICC_TAG_AMC BIT(QCOM_ICC_BUCKET_AMC)
+#define QCOM_ICC_TAG_WAKE BIT(QCOM_ICC_BUCKET_WAKE)
+#define QCOM_ICC_TAG_SLEEP BIT(QCOM_ICC_BUCKET_SLEEP)
+#define QCOM_ICC_TAG_ACTIVE_ONLY (QCOM_ICC_TAG_AMC | QCOM_ICC_TAG_WAKE)
+#define QCOM_ICC_TAG_ALWAYS (QCOM_ICC_TAG_AMC | QCOM_ICC_TAG_WAKE |\
+ QCOM_ICC_TAG_SLEEP)
+
/**
* struct qcom_icc_node - Qualcomm specific interconnect nodes
* @name: the node name used in debugfs
@@ -86,8 +97,8 @@ struct qcom_icc_node {
u16 num_links;
u16 channels;
u16 buswidth;
- u64 sum_avg;
- u64 max_peak;
+ u64 sum_avg[QCOM_ICC_NUM_BUCKETS];
+ u64 max_peak[QCOM_ICC_NUM_BUCKETS];
struct qcom_icc_bcm *bcms[SDM845_MAX_BCM_PER_NODE];
size_t num_bcms;
};
@@ -112,8 +123,8 @@ struct qcom_icc_bcm {
const char *name;
u32 type;
u32 addr;
- u64 vote_x;
- u64 vote_y;
+ u64 vote_x[QCOM_ICC_NUM_BUCKETS];
+ u64 vote_y[QCOM_ICC_NUM_BUCKETS];
bool dirty;
bool keepalive;
struct bcm_db aux_data;
@@ -555,7 +566,7 @@ inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y,
cmd->wait = true;
}
-static void tcs_list_gen(struct list_head *bcm_list,
+static void tcs_list_gen(struct list_head *bcm_list, int bucket,
struct tcs_cmd tcs_list[SDM845_MAX_VCD],
int n[SDM845_MAX_VCD])
{
@@ -573,8 +584,8 @@ static void tcs_list_gen(struct list_head *bcm_list,
commit = true;
cur_vcd_size = 0;
}
- tcs_cmd_gen(&tcs_list[idx], bcm->vote_x, bcm->vote_y,
- bcm->addr, commit);
+ tcs_cmd_gen(&tcs_list[idx], bcm->vote_x[bucket],
+ bcm->vote_y[bucket], bcm->addr, commit);
idx++;
n[batch]++;
/*
@@ -595,51 +606,76 @@ static void tcs_list_gen(struct list_head *bcm_list,
static void bcm_aggregate(struct qcom_icc_bcm *bcm)
{
- size_t i;
- u64 agg_avg = 0;
- u64 agg_peak = 0;
+ size_t i, bucket;
+ u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
+ u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
u64 temp;
- for (i = 0; i < bcm->num_nodes; i++) {
- temp = bcm->nodes[i]->sum_avg * bcm->aux_data.width;
- do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
- agg_avg = max(agg_avg, temp);
+ for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
+ for (i = 0; i < bcm->num_nodes; i++) {
+ temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width;
+ do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
+ agg_avg[bucket] = max(agg_avg[bucket], temp);
- temp = bcm->nodes[i]->max_peak * bcm->aux_data.width;
- do_div(temp, bcm->nodes[i]->buswidth);
- agg_peak = max(agg_peak, temp);
- }
+ temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width;
+ do_div(temp, bcm->nodes[i]->buswidth);
+ agg_peak[bucket] = max(agg_peak[bucket], temp);
+ }
- temp = agg_avg * 1000ULL;
- do_div(temp, bcm->aux_data.unit);
- bcm->vote_x = temp;
+ temp = agg_avg[bucket] * 1000ULL;
+ do_div(temp, bcm->aux_data.unit);
+ bcm->vote_x[bucket] = temp;
- temp = agg_peak * 1000ULL;
- do_div(temp, bcm->aux_data.unit);
- bcm->vote_y = temp;
+ temp = agg_peak[bucket] * 1000ULL;
+ do_div(temp, bcm->aux_data.unit);
+ bcm->vote_y[bucket] = temp;
+ }
- if (bcm->keepalive && bcm->vote_x == 0 && bcm->vote_y == 0) {
- bcm->vote_x = 1;
- bcm->vote_y = 1;
+ if (bcm->keepalive && bcm->vote_x[0] == 0 && bcm->vote_y[0] == 0) {
+ bcm->vote_x[QCOM_ICC_BUCKET_AMC] = 1;
+ bcm->vote_x[QCOM_ICC_BUCKET_WAKE] = 1;
+ bcm->vote_y[QCOM_ICC_BUCKET_AMC] = 1;
+ bcm->vote_y[QCOM_ICC_BUCKET_WAKE] = 1;
}
bcm->dirty = false;
}
+static void qcom_icc_pre_aggregate(struct icc_node *node)
+{
+ size_t i;
+ struct qcom_icc_node *qn;
+
+ qn = node->data;
+
+ for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
+ qn->sum_avg[i] = 0;
+ qn->max_peak[i] = 0;
+ }
+}
+
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;
struct qcom_icc_node *qn;
+ unsigned long tag_word = (unsigned long)tag;
qn = node->data;
+ if (!tag)
+ tag_word = QCOM_ICC_TAG_ALWAYS;
+
+ for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
+ if (test_bit(i, &tag_word)) {
+ qn->sum_avg[i] += avg_bw;
+ qn->max_peak[i] = max_t(u32, qn->max_peak[i], peak_bw);
+ }
+ }
+
*agg_avg += avg_bw;
*agg_peak = max_t(u32, *agg_peak, peak_bw);
- qn->sum_avg = *agg_avg;
- qn->max_peak = *agg_peak;
-
for (i = 0; i < qn->num_bcms; i++)
qn->bcms[i]->dirty = true;
@@ -675,7 +711,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
* Construct the command list based on a pre ordered list of BCMs
* based on VCD.
*/
- tcs_list_gen(&commit_list, cmds, commit_idx);
+ tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_AMC, cmds, commit_idx);
if (!commit_idx[0])
return ret;
@@ -693,6 +729,41 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
return ret;
}
+ INIT_LIST_HEAD(&commit_list);
+
+ for (i = 0; i < qp->num_bcms; i++) {
+ /*
+ * Only generate WAKE and SLEEP commands if a resource's
+ * requirements change as the execution environment transitions
+ * between different power states.
+ */
+ if (qp->bcms[i]->vote_x[QCOM_ICC_BUCKET_WAKE] !=
+ qp->bcms[i]->vote_x[QCOM_ICC_BUCKET_SLEEP] ||
+ qp->bcms[i]->vote_y[QCOM_ICC_BUCKET_WAKE] !=
+ qp->bcms[i]->vote_y[QCOM_ICC_BUCKET_SLEEP]) {
+ list_add_tail(&qp->bcms[i]->list, &commit_list);
+ }
+ }
+
+ if (list_empty(&commit_list))
+ return ret;
+
+ tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_WAKE, cmds, commit_idx);
+
+ ret = rpmh_write_batch(qp->dev, RPMH_WAKE_ONLY_STATE, cmds, commit_idx);
+ if (ret) {
+ pr_err("Error sending WAKE RPMH requests (%d)\n", ret);
+ return ret;
+ }
+
+ tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_SLEEP, cmds, commit_idx);
+
+ ret = rpmh_write_batch(qp->dev, RPMH_SLEEP_STATE, cmds, commit_idx);
+ if (ret) {
+ pr_err("Error sending SLEEP RPMH requests (%d)\n", ret);
+ return ret;
+ }
+
return ret;
}
@@ -738,6 +809,7 @@ static int qnoc_probe(struct platform_device *pdev)
provider = &qp->provider;
provider->dev = &pdev->dev;
provider->set = qcom_icc_set;
+ provider->pre_aggregate = qcom_icc_pre_aggregate;
provider->aggregate = qcom_icc_aggregate;
provider->xlate = of_icc_xlate_onecell;
INIT_LIST_HEAD(&provider->nodes);
On Fri, Aug 9, 2019 at 5:13 AM Georgi Djakov <[email protected]> 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 <[email protected]>
Reviewed-by: Evan Green <[email protected]>
On Fri, Aug 9, 2019 at 5:13 AM Georgi Djakov <[email protected]> wrote:
>
> Introduce an optional callback in interconnect provider drivers. It can be
> used for implementing actions, that need to be executed before the actual
> aggregation of the bandwidth requests has started.
>
> The benefit of this for now is that it will significantly simplify the code
> in provider drivers.
>
> Suggested-by: Evan Green <[email protected]>
> Signed-off-by: Georgi Djakov <[email protected]>
Reviewed-by: Evan Green <[email protected]>
On Fri, Aug 9, 2019 at 5:13 AM Georgi Djakov <[email protected]> wrote:
>
> From: David Dai <[email protected]>
>
> Add support for wake and sleep commands by using a tag to indicate
> whether or not the aggregate and set requests fall into execution
> state specific bucket.
>
> Signed-off-by: David Dai <[email protected]>
> Signed-off-by: Georgi Djakov <[email protected]>
> ---
> drivers/interconnect/qcom/sdm845.c | 134 ++++++++++++++++++++++-------
> 1 file changed, 103 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/interconnect/qcom/sdm845.c b/drivers/interconnect/qcom/sdm845.c
> index fb526004c82e..b2047d1c6d84 100644
> --- a/drivers/interconnect/qcom/sdm845.c
> +++ b/drivers/interconnect/qcom/sdm845.c
> @@ -66,6 +66,17 @@ struct bcm_db {
> #define SDM845_MAX_BCM_PER_NODE 2
> #define SDM845_MAX_VCD 10
>
> +#define QCOM_ICC_BUCKET_AMC 0
> +#define QCOM_ICC_BUCKET_WAKE 1
> +#define QCOM_ICC_BUCKET_SLEEP 2
> +#define QCOM_ICC_NUM_BUCKETS 3
> +#define QCOM_ICC_TAG_AMC BIT(QCOM_ICC_BUCKET_AMC)
> +#define QCOM_ICC_TAG_WAKE BIT(QCOM_ICC_BUCKET_WAKE)
> +#define QCOM_ICC_TAG_SLEEP BIT(QCOM_ICC_BUCKET_SLEEP)
> +#define QCOM_ICC_TAG_ACTIVE_ONLY (QCOM_ICC_TAG_AMC | QCOM_ICC_TAG_WAKE)
> +#define QCOM_ICC_TAG_ALWAYS (QCOM_ICC_TAG_AMC | QCOM_ICC_TAG_WAKE |\
> + QCOM_ICC_TAG_SLEEP)
> +
> /**
> * struct qcom_icc_node - Qualcomm specific interconnect nodes
> * @name: the node name used in debugfs
> @@ -86,8 +97,8 @@ struct qcom_icc_node {
> u16 num_links;
> u16 channels;
> u16 buswidth;
> - u64 sum_avg;
> - u64 max_peak;
> + u64 sum_avg[QCOM_ICC_NUM_BUCKETS];
> + u64 max_peak[QCOM_ICC_NUM_BUCKETS];
> struct qcom_icc_bcm *bcms[SDM845_MAX_BCM_PER_NODE];
> size_t num_bcms;
> };
> @@ -112,8 +123,8 @@ struct qcom_icc_bcm {
> const char *name;
> u32 type;
> u32 addr;
> - u64 vote_x;
> - u64 vote_y;
> + u64 vote_x[QCOM_ICC_NUM_BUCKETS];
> + u64 vote_y[QCOM_ICC_NUM_BUCKETS];
> bool dirty;
> bool keepalive;
> struct bcm_db aux_data;
> @@ -555,7 +566,7 @@ inline void tcs_cmd_gen(struct tcs_cmd *cmd, u64 vote_x, u64 vote_y,
> cmd->wait = true;
> }
>
> -static void tcs_list_gen(struct list_head *bcm_list,
> +static void tcs_list_gen(struct list_head *bcm_list, int bucket,
> struct tcs_cmd tcs_list[SDM845_MAX_VCD],
> int n[SDM845_MAX_VCD])
> {
> @@ -573,8 +584,8 @@ static void tcs_list_gen(struct list_head *bcm_list,
> commit = true;
> cur_vcd_size = 0;
> }
> - tcs_cmd_gen(&tcs_list[idx], bcm->vote_x, bcm->vote_y,
> - bcm->addr, commit);
> + tcs_cmd_gen(&tcs_list[idx], bcm->vote_x[bucket],
> + bcm->vote_y[bucket], bcm->addr, commit);
> idx++;
> n[batch]++;
> /*
> @@ -595,51 +606,76 @@ static void tcs_list_gen(struct list_head *bcm_list,
>
> static void bcm_aggregate(struct qcom_icc_bcm *bcm)
> {
> - size_t i;
> - u64 agg_avg = 0;
> - u64 agg_peak = 0;
> + size_t i, bucket;
> + u64 agg_avg[QCOM_ICC_NUM_BUCKETS] = {0};
> + u64 agg_peak[QCOM_ICC_NUM_BUCKETS] = {0};
> u64 temp;
>
> - for (i = 0; i < bcm->num_nodes; i++) {
> - temp = bcm->nodes[i]->sum_avg * bcm->aux_data.width;
> - do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
> - agg_avg = max(agg_avg, temp);
> + for (bucket = 0; bucket < QCOM_ICC_NUM_BUCKETS; bucket++) {
> + for (i = 0; i < bcm->num_nodes; i++) {
> + temp = bcm->nodes[i]->sum_avg[bucket] * bcm->aux_data.width;
> + do_div(temp, bcm->nodes[i]->buswidth * bcm->nodes[i]->channels);
> + agg_avg[bucket] = max(agg_avg[bucket], temp);
>
> - temp = bcm->nodes[i]->max_peak * bcm->aux_data.width;
> - do_div(temp, bcm->nodes[i]->buswidth);
> - agg_peak = max(agg_peak, temp);
> - }
> + temp = bcm->nodes[i]->max_peak[bucket] * bcm->aux_data.width;
> + do_div(temp, bcm->nodes[i]->buswidth);
> + agg_peak[bucket] = max(agg_peak[bucket], temp);
> + }
>
> - temp = agg_avg * 1000ULL;
> - do_div(temp, bcm->aux_data.unit);
> - bcm->vote_x = temp;
> + temp = agg_avg[bucket] * 1000ULL;
> + do_div(temp, bcm->aux_data.unit);
> + bcm->vote_x[bucket] = temp;
>
> - temp = agg_peak * 1000ULL;
> - do_div(temp, bcm->aux_data.unit);
> - bcm->vote_y = temp;
> + temp = agg_peak[bucket] * 1000ULL;
> + do_div(temp, bcm->aux_data.unit);
> + bcm->vote_y[bucket] = temp;
> + }
>
> - if (bcm->keepalive && bcm->vote_x == 0 && bcm->vote_y == 0) {
> - bcm->vote_x = 1;
> - bcm->vote_y = 1;
> + if (bcm->keepalive && bcm->vote_x[0] == 0 && bcm->vote_y[0] == 0) {
Consider using QCOM_ICC_BUCKET_AMC instead of 0.
> + bcm->vote_x[QCOM_ICC_BUCKET_AMC] = 1;
> + bcm->vote_x[QCOM_ICC_BUCKET_WAKE] = 1;
> + bcm->vote_y[QCOM_ICC_BUCKET_AMC] = 1;
> + bcm->vote_y[QCOM_ICC_BUCKET_WAKE] = 1;
> }
>
> bcm->dirty = false;
> }
>
> +static void qcom_icc_pre_aggregate(struct icc_node *node)
> +{
> + size_t i;
> + struct qcom_icc_node *qn;
> +
> + qn = node->data;
> +
> + for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
> + qn->sum_avg[i] = 0;
> + qn->max_peak[i] = 0;
> + }
> +}
> +
> 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;
> struct qcom_icc_node *qn;
> + unsigned long tag_word = (unsigned long)tag;
Is this local still needed? Can't we just use tag directly?
>
> qn = node->data;
>
> + if (!tag)
> + tag_word = QCOM_ICC_TAG_ALWAYS;
> +
> + for (i = 0; i < QCOM_ICC_NUM_BUCKETS; i++) {
> + if (test_bit(i, &tag_word)) {
> + qn->sum_avg[i] += avg_bw;
> + qn->max_peak[i] = max_t(u32, qn->max_peak[i], peak_bw);
> + }
> + }
> +
> *agg_avg += avg_bw;
> *agg_peak = max_t(u32, *agg_peak, peak_bw);
>
> - qn->sum_avg = *agg_avg;
> - qn->max_peak = *agg_peak;
> -
> for (i = 0; i < qn->num_bcms; i++)
> qn->bcms[i]->dirty = true;
>
> @@ -675,7 +711,7 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> * Construct the command list based on a pre ordered list of BCMs
> * based on VCD.
> */
> - tcs_list_gen(&commit_list, cmds, commit_idx);
> + tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_AMC, cmds, commit_idx);
>
> if (!commit_idx[0])
> return ret;
> @@ -693,6 +729,41 @@ static int qcom_icc_set(struct icc_node *src, struct icc_node *dst)
> return ret;
> }
>
> + INIT_LIST_HEAD(&commit_list);
> +
> + for (i = 0; i < qp->num_bcms; i++) {
> + /*
> + * Only generate WAKE and SLEEP commands if a resource's
> + * requirements change as the execution environment transitions
> + * between different power states.
> + */
> + if (qp->bcms[i]->vote_x[QCOM_ICC_BUCKET_WAKE] !=
> + qp->bcms[i]->vote_x[QCOM_ICC_BUCKET_SLEEP] ||
> + qp->bcms[i]->vote_y[QCOM_ICC_BUCKET_WAKE] !=
> + qp->bcms[i]->vote_y[QCOM_ICC_BUCKET_SLEEP]) {
> + list_add_tail(&qp->bcms[i]->list, &commit_list);
> + }
> + }
> +
> + if (list_empty(&commit_list))
> + return ret;
> +
> + tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_WAKE, cmds, commit_idx);
> +
> + ret = rpmh_write_batch(qp->dev, RPMH_WAKE_ONLY_STATE, cmds, commit_idx);
> + if (ret) {
> + pr_err("Error sending WAKE RPMH requests (%d)\n", ret);
> + return ret;
> + }
> +
> + tcs_list_gen(&commit_list, QCOM_ICC_BUCKET_SLEEP, cmds, commit_idx);
> +
> + ret = rpmh_write_batch(qp->dev, RPMH_SLEEP_STATE, cmds, commit_idx);
> + if (ret) {
> + pr_err("Error sending SLEEP RPMH requests (%d)\n", ret);
> + return ret;
> + }
> +
> return ret;
> }
>
> @@ -738,6 +809,7 @@ static int qnoc_probe(struct platform_device *pdev)
> provider = &qp->provider;
> provider->dev = &pdev->dev;
> provider->set = qcom_icc_set;
> + provider->pre_aggregate = qcom_icc_pre_aggregate;
> provider->aggregate = qcom_icc_aggregate;
> provider->xlate = of_icc_xlate_onecell;
> INIT_LIST_HEAD(&provider->nodes);