Message-ID: <c2bba35e-ea83-0f82-992e-a8ddeb106276@quicinc.com>
Date:   Mon, 30 Jan 2023 13:55:08 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.5.1
Subject: Re: [PATCH] interconnect: Skip call into provider if initial bw is
 zero
To:     Abel Vesa <abel.vesa@linaro.org>, Peng Fan <peng.fan@nxp.com>,
        <djakov@kernel.org>
CC:     Vivek Aknurwar <quic_viveka@quicinc.com>,
        <quic_okukatla@quicinc.com>, <linux-pm@vger.kernel.org>,
        <linux-arm-msm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        "abel >> Philipp Zabel" <p.zabel@pengutronix.de>,
        <abelvesa@kernel.org>,
        Bryan O'Donoghue <bryan.odonoghue@linaro.org>
References: <1673647679-15216-1-git-send-email-quic_viveka@quicinc.com>
 <83a7bfed-3b16-3d01-b1b2-f197252bd0b1@linaro.org>
 <5e1f37ba-494a-19d2-e412-7631508ab142@linaro.org>
 <151790dd-02e5-a1f5-aab5-360f39e21c57@quicinc.com>
 <35dcb764-e340-5fe7-6637-cdb5f84266ce@linaro.org>
 <6dd7b0b0-f6fb-9de4-c365-d6cbfe04f2c0@quicinc.com>
 <742ff9cd-e7be-11b8-3805-5b60aba6b2f1@linaro.org>
 <Y9fZ6GUzWsCMXzba@linaro.org>
Content-Language: en-US
From:   Mike Tipton <quic_mdtipton@quicinc.com>
In-Reply-To: <Y9fZ6GUzWsCMXzba@linaro.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 1/30/2023 6:53 AM, Abel Vesa wrote:
> On 23-01-23 22:58:49, Bryan O'Donoghue wrote:
>> On 23/01/2023 20:37, Mike Tipton wrote:
>>>
>>> This isn't actually changing it for all providers. Only for those that
>>> define the get_bw() callback. Right now that's only qcom/msm8974 and
>>> imx/imx. If get_bw() isn't defined, then icc_node_add() defaults to
>>> INT_MAX. So, the logical behavior in that case is unchanged. Which means
>>> this isn't even changing the behavior for rpmh yet, either.
>>
>> Yes that adds up.
>>
>> Looking at the commit for get_bw() for the 8974, I think this change would
>> be OK with the intent of this commit
>>
>> commit 9caf2d956cfa254c6d89c5f4d7b3f8235d75b28f
>> Author: Georgi Djakov <georgi.djakov@linaro.org>
>> Date:   Mon Nov 9 14:45:12 2020 +0200
>>
>> @Abel what effect will skipping pre->aggregation() have on i.MX ?
> 
> I don't think there is any impact on i.MX platforms.
> 
> Peng, any input?

It should only have an impact if there are nodes left enabled from 
bootloaders that nobody votes for and need to be turned off.

The imx get_bw() callback returns zero for everything. So, the previous 
icc_node_add() behavior would call set() with zero for everything and 
give the provider an opportunity to disable all nodes by default. After 
this change, set() won't be called from icc_node_add() anymore. And 
because init_bw is zero, set() won't be called in icc_sync_state() 
either. So, it's possible for certain nodes to be left enabled whereas 
previously they were disabled during imx probe.

If this change does result in nodes being left enabled, then the ideal 
fix would be for get_bw() to return non-zero for nodes enabled from 
boot. That would result in them being disabled in icc_sync_state().

Should be same possible impact for qcom/msm8974.

> 
>>
>> ---
>> bod