Re: [RFCv2 0/8] Add imx8mm bus frequency switching

[Leonard Crestez <leonard.crestez@nxp.com>]

On 7/4/2019 1:20 AM, Saravana Kannan wrote:

>> The interconnect and devfreq parts do not communicate explicitly: they both
>> just call clk_set_min_rate and the clk core picks the minimum value that can
>> satisfy both. They are thus completely independent.
> 
> Two different parts not talking to each other and just setting min
> rate can cause problems for some concurrency use cases. ICC framework
> is explicitly designed to handle cases like this and aggregate the
> needs correctly. You might want to look into that more closely.

The clk framework aggregates the min_rate requests from multiple 
consumers under a big "prepare_lock" so I expect it will deal with 
concurrent requests correctly. As for performance: frequency switching 
shouldn't be a fast path.

>> The clk_set_min_rate approach does not mesh very well with the OPP framework.
>> Some of interconnect nodes on imx8m can run at different voltages: OPP can
>> handle this well but not in response to a clk_set_min_rate from an unrelated
>> subsystem. Maybe set voltage on a clk notifier?
> 
> I think if you design it something like below, it might make your life
> a whole lot easier.
> Hopefully the formatting looks okay on your end. The arrow going up is
> just connecting devfreq to ICC.
> 
>                          Proactive -> ICC -> clk/OPP API to set freq/volt
>                                        ^
>                                        |
> HW measure -> governor -> devfreq ----+
> 
> That way, all voltage/frequency requirements are managed cleanly by
> clk/OPP frameworks. The ICC deals with aggregating all the
> requirements and devfreq lets you handle reactive scaling and policy.

If icc and devfreq are to directly communicate it makes more sense to do 
it backwards: ICC should set a min_freq on nodes which have a devfreq 
instance attached and devfreq should implement actual freq switching.

HW measurement is done on individual nodes while ICC deals with requests 
along a path. In particular on imx we have a performance monitor 
attached to the ddr controller and I doubt it can distinguish between 
masters so how could this be mapped usefully to an interconnect request?

As far as I understand with devfreq the ddrc node could use "ondemand" 
while the other nodes would default to the "passive" governor and run at 
predefined default ratios relative to DDRC.

> If all of this makes sense, please take a look at [2] and provide your
> thoughts. I've dropped a few patches from [1] to avoid confusion (too
> much going on at once). I think BW OPP tables and having OPP tables
> for interconnect paths will be something you'll need (if not now,
> eventually) and something you can build on top of nicely.

I found it very confusing that you're assigning BW OPP tables to 
devices. My initial understanding was that BW OPP would map "bandwidth" 
to "frequency" so BW OPPs should be assigned to links along the 
interconnect graph. But maybe what you want is to have OPPs for the BW 
values requested by devices?

Looking at the sdm845 icc provider source and it seems that those 
"peak/avg" values are actually parameters which go into a firmware 
command!? It makes sense that you want interconnect to be "below" 
devfreq since icc_provider.set maps very closely to what firmware exposes.

 > Interconnects and interconnect paths quantify their performance 
levels > in terms of bandwidth and not in terms of frequency.

On i.MX we just have a bunch of interconnect IPs for which frequencies 
can be adjusted (in hz) so the above statement doesn't really hold. It 
is up to an icc provider to convert aggregate bandwidth values to 
frequencies along the path.

--
Regards,
Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel