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

[Ulf Hansson]

On Wed, 11 Sept 2024 at 16:15, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
> 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?

Answering my own question. After some further investigation, I am
afraid that your concern was correct.

One sm8250, venus is using three power-domains,"venus", "vcodec0",
"mx", but there is only one phandle in the required-opp.
"venus" and "vcodec0" correspond to the "videocc" power-domain, which
has a parent-domain that is the "rpmhpd".
"mx" corresponds to the "rpmhpd".
The rpmhpd power-domain has one shared OPP table used for all the
power-domains it provides. :-(

Because we also need to look for a matching OPP table for the
required-opp by walking the power-domain parents (needed on Tegra), we
simply can't tell what power-domain the required-opp belongs to.

>
> 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.

It looks like I was wrong.

This whole problem boils down to that we are allowing the OPP table to
be shared for a genpd OF provider for multiple power-domains. We could
consider adding some new DT property to point out at what power-domain
the required-opps belongs to, but it doesn't really matter as we need
to keep supporting the current DTS.

Oh well, back to the drawing table to re-work this again. It looks
like we need to make it possible for consumer drivers to provide
additional information to dev_pm_domain_attach_list(), allowing it to
understand how the required-devs should be assigned.

Unless you have some better ideas?

[...]

Kind regards
Uffe