* [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state()
@ 2021-06-19 17:08 Odelu Kukatla
2021-06-21 16:33 ` Matthias Kaehlcke
2021-06-25 21:20 ` Mike Tipton
0 siblings, 2 replies; 3+ messages in thread
From: Odelu Kukatla @ 2021-06-19 17:08 UTC (permalink / raw)
To: georgi.djakov, bjorn.andersson, evgreen, Georgi Djakov, linux-pm,
linux-kernel
Cc: sboyd, mdtipton, sibis, saravanak, okukatla, seansw, elder,
linux-arm-msm, linux-arm-msm-owner
When removing the initial bandwidth votes in sync_state(), make sure
to call the aggregate() function for nodes which don't have any
clients yet. aggregate_requests() does not invoke aggregate()
for unused nodes.
Signed-off-by: Odelu Kukatla <okukatla@codeaurora.org>
---
drivers/interconnect/core.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
index 8a1e70e..1d9a00a 100644
--- a/drivers/interconnect/core.c
+++ b/drivers/interconnect/core.c
@@ -1106,7 +1106,16 @@ void icc_sync_state(struct device *dev)
dev_dbg(p->dev, "interconnect provider is in synced state\n");
list_for_each_entry(n, &p->nodes, node_list) {
if (n->init_avg || n->init_peak) {
- aggregate_requests(n);
+ if (hlist_empty(&n->req_list)) {
+ if (p->pre_aggregate)
+ p->pre_aggregate(n);
+
+ p->aggregate(n, 0, 0, 0, &n->avg_bw,
+ &n->peak_bw);
+ } else {
+ aggregate_requests(n);
+ }
+
p->set(n, n);
}
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state()
2021-06-19 17:08 [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state() Odelu Kukatla
@ 2021-06-21 16:33 ` Matthias Kaehlcke
2021-06-25 21:20 ` Mike Tipton
1 sibling, 0 replies; 3+ messages in thread
From: Matthias Kaehlcke @ 2021-06-21 16:33 UTC (permalink / raw)
To: Odelu Kukatla
Cc: georgi.djakov, bjorn.andersson, evgreen, Georgi Djakov, linux-pm,
linux-kernel, sboyd, mdtipton, sibis, saravanak, seansw, elder,
linux-arm-msm, linux-arm-msm-owner
On Sat, Jun 19, 2021 at 10:38:29PM +0530, Odelu Kukatla wrote:
> When removing the initial bandwidth votes in sync_state(), make sure
> to call the aggregate() function for nodes which don't have any
> clients yet. aggregate_requests() does not invoke aggregate()
> for unused nodes.
>
> Signed-off-by: Odelu Kukatla <okukatla@codeaurora.org>
It seems this should have a 'Fixes' tag for b1d681d8d324 ("interconnect:
Add sync state support")', to make sure the change makes it into the
stable trees.
> ---
> drivers/interconnect/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 8a1e70e..1d9a00a 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -1106,7 +1106,16 @@ void icc_sync_state(struct device *dev)
> dev_dbg(p->dev, "interconnect provider is in synced state\n");
> list_for_each_entry(n, &p->nodes, node_list) {
> if (n->init_avg || n->init_peak) {
> - aggregate_requests(n);
> + if (hlist_empty(&n->req_list)) {
nit: consider handling the common case in the 'if' branch and the exception of
the initial votes in 'else'.
> + if (p->pre_aggregate)
> + p->pre_aggregate(n);
> +
> + p->aggregate(n, 0, 0, 0, &n->avg_bw,
> + &n->peak_bw);
> + } else {
> + aggregate_requests(n);
> + }
> +
> p->set(n, n);
> }
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state()
2021-06-19 17:08 [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state() Odelu Kukatla
2021-06-21 16:33 ` Matthias Kaehlcke
@ 2021-06-25 21:20 ` Mike Tipton
1 sibling, 0 replies; 3+ messages in thread
From: Mike Tipton @ 2021-06-25 21:20 UTC (permalink / raw)
To: Odelu Kukatla, georgi.djakov, bjorn.andersson, evgreen,
Georgi Djakov, linux-pm, linux-kernel
Cc: sboyd, sibis, saravanak, seansw, elder, linux-arm-msm,
linux-arm-msm-owner
On 6/19/2021 10:08 AM, Odelu Kukatla wrote:
> When removing the initial bandwidth votes in sync_state(), make sure
> to call the aggregate() function for nodes which don't have any
> clients yet. aggregate_requests() does not invoke aggregate()
> for unused nodes.
>
> Signed-off-by: Odelu Kukatla <okukatla@codeaurora.org>
> ---
> drivers/interconnect/core.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 8a1e70e..1d9a00a 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -1106,7 +1106,16 @@ void icc_sync_state(struct device *dev)
> dev_dbg(p->dev, "interconnect provider is in synced state\n");
> list_for_each_entry(n, &p->nodes, node_list) {
> if (n->init_avg || n->init_peak) {
> - aggregate_requests(n);
> + if (hlist_empty(&n->req_list)) {
> + if (p->pre_aggregate)
> + p->pre_aggregate(n);
> +
> + p->aggregate(n, 0, 0, 0, &n->avg_bw,
> + &n->peak_bw);
> + } else {
> + aggregate_requests(n);
> + }
> +
> p->set(n, n);
> }
> }
>
I ran into this issue myself last week. There is an alternative fix that
I think is a little cleaner. The reason we need the aggregate() call
here is that the icc-rpmh providers only add BCMs to the commit list in
the aggregate() callback. If aggregate() isn't called, then we don't
commit anything to HW. But, if we instead add BCMs to the commit list in
pre_aggregate(), then the existing aggregate_requests() call is
sufficient, since that always calls pre_aggregate() even when req_list
is empty. That means we'd need to update icc_node_add() to call
pre_aggregate() before aggregate(), otherwise we wouldn't commit the
initial floors. But I think always calling pre_aggregate() before
aggregate() is a reasonable requirement.
I've posted these changes in a patch series along with a separate
sync-state-related fix. The solutions could co-exist, but they are
largely redundant.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-06-25 21:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-19 17:08 [PATCH] interconnect: Aggregate bandwidth votes for unused nodes in sync_state() Odelu Kukatla
2021-06-21 16:33 ` Matthias Kaehlcke
2021-06-25 21:20 ` Mike Tipton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.