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

On Wed, 11 Sept 2024 at 16:15, Ulf Hansson [off-list ref] wrote:

On Wed, 11 Sept 2024 at 08:03, Viresh Kumar [off-list ref] wrote:

quoted

FYI, I am on holidays now :)

Oh, nice! Enjoy!

quoted

On Fri, 6 Sept 2024 at 14:19, Ulf Hansson [off-list ref] wrote:

quoted

quoted

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.

quoted

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.

quoted

quoted

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