Re: [PATCH] OPP: Fix support for required OPPs for multiple PM domains

[Viresh Kumar]

On 25-07-24, 11:21, Ulf Hansson wrote:
> Right.
> 
> The main issue in regards to the above, is that we may end up trying
> to vote for different devices, which votes correspond to the same
> OPP/OPP-table. The one that comes first will request the OPP, the
> other ones will be ignored as the OPP core thinks there is no reason
> to already set the current OPP.

Right, but that won't happen with the diff I shared earlier where we set
"forced" to true. Isn't it ?

> > I think that design is rather correct, just like other frameworks. Just that we
> > need to do only set-level for genpds and nothing else. That will have exactly
> > the same behavior that you want.
> 
> I don't quite understand what you are proposing. Do you want to add a
> separate path for opp-levels?

Not separate paths, but ignore clk/regulator changes if the table belongs to a
genpd.

> The problem with that would be that platforms (Tegra at least) are
> already using a combination of opp-level and clocks.

If they are using both for a genpd's OPP table (and changes are made for both
opp-level and clock by the OPP core), then it should already be wrong, isn't it?
Two simultaneous calls to dev_pm_opp_set_opp() would set the level correctly (as
aggregation happens in the genpd core), but clock setting would always reflect
the second caller. This should be fixed too, isn't it ?

> To be able to call dev_pm_opp_set_opp() on the required-dev (which
> would be the real device in this case), we need to add it to genpd's
> OPP table by calling _add_opp_dev() on it. See _opp_attach_genpd().
> 
> The problem with this, is that the real device already has its own OPP
> table (with the required-OPPs pointing to genpd's OPP table), which
> means that we would end up adding the device to two different OPP
> tables.

I was terrified for a minute after reading this and the current code, as I also
thought there is an issue there. But I was confident that we used to take care
of this case separately earlier. A short dive into git logs got me to this:

commit 6d366d0e5446 ("OPP: Use _set_opp_level() for single genpd case")

This should be working just fine I guess.

-- 
viresh