mbox series

[0/4] interconnect: Fix sync-state issues

Message ID 20210625212839.24155-1-mdtipton@codeaurora.org
Headers show
Series interconnect: Fix sync-state issues | expand

Message

Mike Tipton June 25, 2021, 9:28 p.m. UTC 
These patches fix a couple of sync-state bugs that either cause the initial BW
floors to be ignored entirely, or to be never removed after sync-state is
called.

Mike Tipton (4):
  interconnect: Zero initial BW after sync-state
  interconnect: Always call pre_aggregate before aggregate
  interconnect: qcom: icc-rpmh: Ensure floor BW is enforced for all nodes
  interconnect: qcom: icc-rpmh: Add BCMs to commit list in pre_aggregate

 drivers/interconnect/core.c          |  7 ++++++-
 drivers/interconnect/qcom/icc-rpmh.c | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 11 deletions(-)

Comments

Odelu Kukatla July 1, 2021, 4:56 p.m. UTC | #1 
On 2021-06-26 02:58, Mike Tipton wrote:
> The initial BW values may be used by providers to enforce floors. Zero
> these values after sync-state so that providers know when to stop
> enforcing them.
> 
> Fixes: b1d681d8d324 ("interconnect: Add sync state support")
> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>
> ---
>  drivers/interconnect/core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c
> index 8a1e70e00876..945121e18b5c 100644
> --- a/drivers/interconnect/core.c
> +++ b/drivers/interconnect/core.c
> @@ -1106,6 +1106,8 @@ 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) {
> +				n->init_avg = 0;
> +				n->init_peak = 0;
nit: It is good to reset init/floor levels back to zero, but we don't 
need to do this as we have sync_state flag to let providers know when to 
stop enforcing.
>  				aggregate_requests(n);
>  				p->set(n, n);
>  			}
Mike Tipton July 12, 2021, 3:41 p.m. UTC | #2 
On 7/1/2021 9:56 AM, okukatla@codeaurora.org wrote:
> On 2021-06-26 02:58, Mike Tipton wrote:

>> The initial BW values may be used by providers to enforce floors. Zero

>> these values after sync-state so that providers know when to stop

>> enforcing them.

>>

>> Fixes: b1d681d8d324 ("interconnect: Add sync state support")

>> Signed-off-by: Mike Tipton <mdtipton@codeaurora.org>

>> ---

>>  drivers/interconnect/core.c | 2 ++

>>  1 file changed, 2 insertions(+)

>>

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

>> index 8a1e70e00876..945121e18b5c 100644

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

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

>> @@ -1106,6 +1106,8 @@ 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) {

>> +                n->init_avg = 0;

>> +                n->init_peak = 0;

> nit: It is good to reset init/floor levels back to zero, but we don't 

> need to do this as we have sync_state flag to let providers know when to 

> stop enforcing.


The synced_state variable is static to this file. It's not exposed to 
providers. In fact, we could entirely remove synced_state with this 
patch since it's unnecessary after zeroing the initial floors.

>>                  aggregate_requests(n);

>>                  p->set(n, n);

>>              }