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
