On Tue 26 Oct 19:48 CDT 2021, Stephen Boyd wrote:
+Rajendra
Quoting Bjorn Andersson (2021-10-25 19:48:02)
On Mon 25 Oct 15:41 PDT 2021, Stephen Boyd wrote:
When the binding was introduced I recall we punted on the parent
child
conversion stuff. One problem at a time. There's also the
possibility
for a power domain to be parented by multiple power domains so
translation tables need to account for that.
But for this case - and below display case - the subdomain (the
device's
power-domain) is just a dumb gate. So there is no translation, the
given
performance_state applies to the parent. Or perhaps such implicitness
will come back and bite us?
In the gate case I don't see how the implicitness will ever be a
problem.
I don't think we allow a power-domain to be a subdomain of two
power-domains - and again it's not applicable to USB or display
afaict.
Ah maybe. I always confuse power domains and genpd.
Or we may need to make another part of the OPP binding to
indicate the
relationship between the power domain and the OPP and the
parent of
the power domain.
I suspect this would be useful if a power-domain provider needs to
translate a performance_state into a different
supply-performance_state.
Not sure if we have such case currently; these examples are all an
adjustable power-domain with "gating" subdomains.
Even for this case, we should be able to have the GDSC map the on
state
to some performance state in the parent domain. Maybe we need to add
some code to the gdsc.c file to set a performance state on the
parent
domain when it is turned on. I'm not sure where the value for
that perf
state comes from. I guess we can hardcode it in the driver for
now and
if it needs to be multiple values based on the clk frequency we
can push
it out to an OPP table or something like that.
For the GDSC I believe we only have 1:1 mapping, so implementing
set_performance_state to just pass that on to the parent might do the
trick (although I haven't thought this through).
Conceptually I guess this would be like calling clk_set_rate() on a
clock gate, relying on it being propagated upwards. The problem
here is
that the performance_state is just a "random" integer without a well
defined unit.
Right. Ideally it would be in the core code somehow so that if there
isn't a set_performance_state function we go to the parent or some
special return value from the function says "call it on my parent".
The
translation scheme could come later so we can translate the "random"
integer between parent-child domains.
As a proof of concept it should be sufficient to just add an
implementation of sc->pd.set_performance_state in gdsc.c. But I agree
that it would be nice to push this into some framework code, perhaps
made opt-in by some GENPD_FLAG_xyz.
At the end of the day the device
driver wants to set a frequency or runtime pm get the device and
let the
OPP table or power domain code figure out what the level is
supposed to
be.
Yes and this is already working for the non-nested case - where the
single power-domain jumps between performance states as the opp code
switches from one opp to another.
So if we can list only the child power-domain (i.e. the GDSC) and have
the performance_stat requests propagate up to the parent rpmhpd
resource
I think we're good.
Let's give this a spin and confirm that this is the case...
The one case where I believe we talked about having different mapping
between the performance_state levels was in the relationship
between CX
and MX. But I don't think we ever did anything about that...
Hmm alright. I think there's a constraint but otherwise nobody really
wants to change both at the same time.
Yes, a GDSC is really a gate on a parent power domain like CX or
MMCX,
etc. Is the display subsystem an example of different clk
frequencies
wanting to change the perf state of CX? If so it's a good place
to work
out the translation scheme for devices that aren't listing the CX
power
domain in DT.
Yes, the various display components sits in MDSS_GDSC but the
opp-tables
needs to change the performance_state of MDSS_GDSC->parent (i.e.
CX or
MMCX, depending on platform).
As I said, today we hack this by trusting that the base drm/msm
driver
will keep MDSS_GDSC on and listing MMCX (or CX) as power-domain
for each
of these components.
So if we solve this, then that seems to directly map to the static
case
for USB as well.
Got it. So in this case we could have the various display components
that are in the mdss gdsc domain set their frequency via OPP and then
have that translate to a level in CX or MMCX. How do we parent the
power
domains outside of DT? I'm thinking that we'll need to do that if MMCX
is parented by CX or something like that and the drivers for those two
power domains are different. Is it basic string matching?
In one way or another we need to invoke pm_genpd_add_subdomain() to
link
the two power-domains (actually genpds) together, like what was done in
3652265514f5 ("clk: qcom: gdsc: enable optional power domain support").
In the case of MMCX and CX, my impression of the documentation is that
they are independent - but if we need to express that CX is parent of
MMCX, they are both provided by rpmhpd which already supports this by
just specifying .parent on mmcx to point to cx.
