Re: [PATCH V3 2/8] soc: qcom: geni: Support for ICC voting

[Georgi Djakov <georgi.djakov@xxxxxxxxxx>]

Hi,

On 4/7/20 09:46, Akash Asthana wrote:
> Hi Bjorn, Evan,
> 
>>> Given that these two functions only switch the bandwidth request between
>>> some value and 0, I really think we should carry a "bool enabled" on the
>>> path and replace these two functions with
>>> icc_bulk_enable()/icc_bulk_disable().
> So, if above is implementation "bool enabled" on path can be used directly in
> aggregation of ICC votes on particular node without using icc_set_bw call, if
> yes then I am not aware how? or we'll be using icc_set_bw API indirectly inside
> icc_bulk APIs?

If there is a repeated pattern to switch between some bandwidth value and zero,
it really makes sense to introduce such functions in the framework core. I think
that this might be very useful especially for suspend and resume cases.
Something like icc_{enable,disable}(struct icc_path *path) functions and also
the bulk versions, that will flag the path as disabled, re-aggregate and do
icc_set_bw().

>>> The added benefit of this would be that you call icc_set_bw() instead of
>>> changing the geni_icc_path->{avg_bw,peak_bw} and don't need to keep
>>> track of them here.
> 
> Ok IIUC, we need to call icc_set_bw() from GENI driver only if we change (avg_bw
> | peak_bw)?

Yes, exactly.

Thanks,
Georgi

> 
> Regards,
> 
> Akash
> 
>> Yes yes! I had the same thought here [1].
>>
>> Georgi, what do you think?
>> -Evan
>>
>> [1]
>> https://lore.kernel.org/linux-arm-msm/CAE=gft58QsgTCUHMHKJhcM9ZxAeMiY16CrbNv2HaTCRqwtmt7A@xxxxxxxxxxxxxx/
>>
>