Re: [PATCH V4 2/9] interconnect: Set peak requirement as twice of average

Re: [PATCH V4 2/9] interconnect: Set peak requirement as twice of average

[Georgi Djakov]

Hi Akash,

On 4/15/20 13:23, Akash Asthana wrote:
> Lot of ICC clients are not aware of their actual peak requirement,
> most commonly they tend to guess their peak requirement as
> (some factor) * avg_bw.
> 
> Centralize random peak guess as twice of average, out into the core
> to maintain consistency across the clients. Client can always
> override this setting if they got a better idea.

I am still not convinced that this is a good idea. If the factor is a random
value, then i think that the default factor should be 1.

According to your previous reply, it seems that from geni we are requesting
double peak bandwidth to compensate for other clients which are not requesting
bandwidth for themselves. IMO, this is a bit hacky.

Instead of requesting double peak bandwidth, IIUC the correct thing to do here
is to request peak_bw = avg_bw for geni. And instead of trying to compensate for
other clients "stealing" bandwidth, can't we make these clients vote for their
own bandwidth? Or if they really can't, this should be handled elsewhere - maybe
in the interconnect platform driver we can reserve some amount of minimum
bandwidth for such cases?

Thanks,
Georgi

Re: [PATCH V4 2/9] interconnect: Set peak requirement as twice of average

[Akash Asthana]

Hi Georgi,

On 4/23/2020 3:01 PM, Georgi Djakov wrote:
> Hi Akash,
>
> On 4/15/20 13:23, Akash Asthana wrote:
>> Lot of ICC clients are not aware of their actual peak requirement,
>> most commonly they tend to guess their peak requirement as
>> (some factor) * avg_bw.
>>
>> Centralize random peak guess as twice of average, out into the core
>> to maintain consistency across the clients. Client can always
>> override this setting if they got a better idea.
> I am still not convinced that this is a good idea. If the factor is a random
> value, then i think that the default factor should be 1.
>
> According to your previous reply, it seems that from geni we are requesting
> double peak bandwidth to compensate for other clients which are not requesting
> bandwidth for themselves. IMO, this is a bit hacky.
>
> Instead of requesting double peak bandwidth, IIUC the correct thing to do here
> is to request peak_bw = avg_bw for geni. And instead of trying to compensate for
> other clients "stealing" bandwidth, can't we make these clients vote for their
> own bandwidth? Or if they really can't, this should be handled elsewhere - maybe
> in the interconnect platform driver we can reserve some amount of minimum
> bandwidth for such cases?

Okay, probably we can correct clients vote for their own bandwidth or 
reserve some minimum BW from interconnect platform driver is case of any 
latency issue observed.

I will drop this change in next version.

Will it create any difference if  peak_bw = 0 instead of peak_bw = 
avg_bw? In my understanding peak_bw <= avg_bw is no-ops, it won't impact 
the NOC speed.


Regards,

Akash

>
> Thanks,
> Georgi

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project

Re: [PATCH V4 2/9] interconnect: Set peak requirement as twice of average

[Georgi Djakov]

Hi Akash,

On 4/28/20 12:46, Akash Asthana wrote:
> Hi Georgi,
> 
> On 4/23/2020 3:01 PM, Georgi Djakov wrote:
>> Hi Akash,
>>
>> On 4/15/20 13:23, Akash Asthana wrote:
>>> Lot of ICC clients are not aware of their actual peak requirement,
>>> most commonly they tend to guess their peak requirement as
>>> (some factor) * avg_bw.
>>>
>>> Centralize random peak guess as twice of average, out into the core
>>> to maintain consistency across the clients. Client can always
>>> override this setting if they got a better idea.
>> I am still not convinced that this is a good idea. If the factor is a random
>> value, then i think that the default factor should be 1.
>>
>> According to your previous reply, it seems that from geni we are requesting
>> double peak bandwidth to compensate for other clients which are not requesting
>> bandwidth for themselves. IMO, this is a bit hacky.
>>
>> Instead of requesting double peak bandwidth, IIUC the correct thing to do here
>> is to request peak_bw = avg_bw for geni. And instead of trying to compensate for
>> other clients "stealing" bandwidth, can't we make these clients vote for their
>> own bandwidth? Or if they really can't, this should be handled elsewhere - maybe
>> in the interconnect platform driver we can reserve some amount of minimum
>> bandwidth for such cases?
> 
> Okay, probably we can correct clients vote for their own bandwidth or reserve
> some minimum BW from interconnect platform driver is case of any latency issue
> observed.

Yes, this sounds like the correct thing to do.

> 
> I will drop this change in next version.
> 
> Will it create any difference if  peak_bw = 0 instead of peak_bw = avg_bw? In my
> understanding peak_bw <= avg_bw is no-ops, it won't impact the NOC speed.

It will not have impact on the NOC speed, but it does not make much logical
sense to have peak_bw = 0 or peak_bw < avg_bw. In the geni case, i think what
we want to do is peak_bw = avg_bw.

Thanks,
Georgi