12.08.2021 14:17, Ulf Hansson пишет:

On Thu, 12 Aug 2021 at 03:40, Dmitry Osipenko [off-list ref] wrote:

quoted

12.08.2021 01:41, Dmitry Osipenko пишет:

quoted

quoted

quoted

I am not saying you should change the clock rate. The current code
path that runs via devm_tegra_core_dev_init_opp_table() just calls
clk_get_rate and then dev_pm_opp_set_rate() with the current rate to
vote for the corresponding OPP level. Right?

Isn't this exactly what you want? No?

I see now what you meant, it's actually a simpler variant and it works
too. Thank you for the suggestion, I'll prepare v8.

My bad, it doesn't work at all. I actually need to use the rpm_pstate or
something else because performance state is coupled with the enable
state of the device. If device is never rpm-suspended by consumer
driver, then the initialized performance state is never dropped. Hence I
want to initialize the state which is set only when device is resumed.

I'll need to think more about it.

GENPD core has these false assumptions:

1. It assumes that by default all devices are at zero performance level
at a boot time. This is not true for Tegra because hardware is
pre-initialized independently from GENPD.

Right, which is similar to other SoCs.

quoted

2. It assumes that nothing depends on performance level and devices can
operate at any level at any time. Not true for Tegra and other platforms
where performance level is coupled with clocks state of attached
devices. OPP framework glues clock and performance level together for
us, which works good so far.

Right, OPPs need to be managed differently depending on the SoC.
That's why genpd is there to help and to model this as "performance
states" and to allow operations to be set through SoC specific
callabacks, for example.

More importantly, the assumption is that in general, consumer drivers
should use the OPP library to vote/set OPP levels, they shouldn't call
dev_pm_genpd_set_performance_state() - unless they know exactly what
they are doing.

quoted

Hence I either need to patch every driver to use dev_pm_opp_set_rate in
order to sync clk rate with the perf level at device runtime, or I need
to preset the rpm perf level to allow GENPD core to set the level before
device is resumed.

When the device is getting attached to its genpd (during the probe
sequence and for a single PM domain case), runtime PM is disabled for
the device. If you would call dev_pm_opp_set_rate() from a genpd
callback during attach (without changing the rate), this means you
would update/synchronize the vote. In this way, the vote is set before
the device is runtime resumed by the driver, right?

I was doing that previously, but then realized that for some devices the
state needs to be synced only once device is rpm-resumed. Making RPM
driver to perform the syncing worked well.

On the other hand, patching the driver should also be quite simple and
you need to do that anyways, don't you?

It will be extra boilerplate code in the drivers, but perhaps it's the
best option for today, I'll consider it for v8. Thank you for helping
with the review.