On 7/4/2019 6:03 AM, Saravana Kannan wrote:

On Wed, Jul 3, 2019 at 4:30 PM Leonard Crestez [off-list ref] wrote:

quoted

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

quoted

quoted

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.

Sorry I wasn't clear. I was not talking about locking issues or race
conditions when I said concurrency use cases. What I meant was if GPU
wants 5 GB/s and at the same time (concurrent) camera wants 5 GB/s
you'll need to configure the interconnect for 10 GB/s. If both of them
just try to set the min freq equivalent for 5 GB/s the performance
would be bad or functionality might break.

I'm not calling clk_set_min_rate independently for each icc path, that 
would be obviously broken. The interconnect framework is still used to 
aggregate bandwith requests and in your scenario clk_set_min_rate for 
the main NOC would be called in a way that meets the combined 10 GB/s 
requests.

It is devfreq which calls clk_set_min_rate independently of 
interconnect, this results in CLK performing the final aggregation 
between the "proactive" and "reactive" scaling.

quoted

quoted

quoted

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?

Ah, that was the missing piece for me -- you are trying to use a
central performance monitor. I see what you are trying to do.

So you are looking at system wide traffic at DDR and then using that
to scale the interconnect/DDRC. I don't know how complicated or not
the IMX interconnect topology is, so please pardon my questions. If
you are using a performance monitor at the DDR controller, why do you
need the "proactive" requests from other clients? Wouldn't the
performance monitor account for all the traffic to DDR?

Reactive scaling is too slow to ramp-up in media playback scenarios and 
first few frames would fail.

quoted

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.

Yes, that approach would also work but I'm not sure why you need the
ICC framework in that case.

For proactive scaling: to ensure bandwidth *before* traffic starts. In 
imx vendor tree that's all that's implemented; for reactive scaling we 
just set busfreq to high as soon as cpu leaves min opp.

IMX interconnect topology is not very complex so mechanisms other than 
interconnect could be used. But ICC is the most powerful and expressive 
subsystem for proactive requests.

quoted

quoted

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?

I want to have OPPs for bandwidths requested for paths.

Right, this was not very obvious.

Each interconnect node can also use BW OPPs if that makes sense for them,
but I think they'd be better served by using frequency OPPs.

Each interconnect node is asked by the framework to ensure a certain BW 
is available in "bytes". The nodes could use OPPs with BW values to map 
the icc request to a frequency in "hz".

quoted

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.

Even without the firmware (it's mainly there to aggregate requests for
some system wide resources) or if interconnects are scaled directly
using clock APIs (older chips), sdm845 would still want ICC to be
below devfreq. It's because 845 doesn't try to do ICC scaling by
measuring at the DDR. Each master makes separate requests and then the
ICC aggregates and sets the frequency. They have their reasons (good
ones) for doing that.

Maybe I'm confused about how devfreq is used in your scenario: you have 
devices which have their own OPPs (like a GPU) and expose this via 
devfreq. Then for each GPU OPP you want to pick a the BW value to 
request from interconnect, right?

My idea was to use devfreq *after* icc so that you can do stuff like 
force a certain NOC to "max" via echo "performance" > $sysfs/governor. 
It also allows encapsulating complex freq switching (clk maintainers 
don't seem to like my dram clk).

On a second examination there is no actual incompatibility here, devfreq 
could be used both below and above ICC.

--
Regards,
Leonard

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