Re: [PATCH 2/2] OPP/pmdomain: Fix the assignment of the required-devs

From: Ulf Hansson
Date: Wed Sep 11 2024 - 10:16:15 EST


On Wed, 11 Sept 2024 at 08:03, Viresh Kumar <viresh.kumar@xxxxxxxxxx> wrote:
>
> FYI, I am on holidays now :)

Oh, nice! Enjoy!

>
> On Fri, 6 Sept 2024 at 14:19, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> > > How do we differentiate between two cases where the required-opps can
> > > be defined as either of these:
> > >
> > > required-opps = <&opp_pd_50, &opp_pd_51>; //corresponds to pd_perf1 and pd_perf0 (in reverse order)
> > >
> > > OR
> > >
> > > required-opps = <&opp_pd_51, &opp_pd_50>; //corresponds to pd_perf0 and pd_perf1
> > >
> > > I thought this can't be fixed without some platform code telling how
> > > the DT is really configured, i.e. order of the power domains in the
> > > required-opps.
> >
> > I don't think we need platform code for this.
> >
> > When registering a genpd provider, an OPP table gets assigned to it.
>
> So we will create a real OPP table in code, which will point to the common
> OPP table in DT. Fine.
>
> > When hooking up a device to one of its genpd providers, that virtual
> > device then also gets a handle to its genpd's OPP table.
>
> Right.
>
> If there are two genpds required for a device from the same genpd provider, the
> picture isn't very clear at this point. i.e. which required OPP
> belongs to which genpd,
> as both have same table in DT.

I agree that it's not very clear.

But to me, this seems like an orthogonal problem that really should
not be managed by platform specific code in consumer drivers.
Moreover, unless I am mistaken, I believe this isn't really a problem
for the currently supported use cases we have for required-opps. Or is
it?

That said, we already have two methods that helps us to deal with this issue:

1)
For a genpd OF provider that provides multiple genpds, the genpd/OPP
core tries to assign an OPP table for each genpd, based on the
power-domain index. In other words, if corresponding OPP-tables are
specified in the operating-points-v2 list, those would get assigned
accordingly.

2)
The genpd OF provider can control on a per genpd basis, whether there
should be an OPP table assigned to it. This is managed by assigning
the ->set_performance_state() callback for the genpd or leaving it
unassigned. Typically this works well, when there is one OPP-table
specified in the operating-points-v2 list for the provider - and only
one of the genpds that should use it.

If it turns out that we need something more flexible, I think we need
to look at extending the OPP/power-domain DT bindings. We would
probably need a "by-names" DT property, allowing us to specify the
mapping between the OPP-tables and the power-domains.

>
> > Each of the phandles in the required-opps points to another OPP table,
> > which OPP table should be associated with a specific genpd.
>
> Yes, but a simple order reversal in DT (which I sent in my last
> email), will not be picked
> by code at all. i.e. DT doesn't give the order in which required OPPs
> are present.

Assuming genpd OF providers are following 1) or 2), I don't think this
should be an issue.

>
> > In other words, the information is there, we should not need anything
> > additional in DT.

Kind regards
Uffe