Re: [PATCH v2] arm64/dts/qcom/sc7180: Add Display Port dt node

Quoting Bjorn Andersson (2021-06-18 14:41:50)

On Fri 18 Jun 15:49 CDT 2021, Stephen Boyd wrote:

quoted

Quoting khsieh@codeaurora.org (2021-06-10 09:54:05)

quoted

On 2021-06-08 16:10, Bjorn Andersson wrote:

quoted

On Tue 08 Jun 17:44 CDT 2021, Stephen Boyd wrote:

quoted

Honestly I suspect the DP PHY is _not_ in the CX domain as CX is for
digital logic. Probably the PLL is the hardware that has some minimum
CX
requirement, and that flows down into the various display clks like
the
link clk that actually clock the DP controller hardware. The mdss_gdsc
probably gates CX for the display subsystem (mdss) so if we had proper
corner aggregation logic we could indicate that mdss_gdsc is a child
of
the CX domain and then make requests from the DP driver for particular
link frequencies on the mdss_gdsc and then have that bubble up to CX
appropriately. I don't think any of that sort of code is in place
though, right?

I haven't checked sc7180, but I'm guessing that it's following the
other
modern platforms, where all the MDSS related pieces (including e.g.
dispcc) lives in the MMCX domain, which is separate from CX.

So the parent of MDSS_GDSC should be MMCX, while Kuogee's answer (and
the dp-opp-table) tells us that the PLL lives in the CX domain.

Isn't MMCX a "child" of CX? At least my understanding is that MMCX is
basically a GDSC that clamps all of multimedia hardware block power
logic so that the leakage is minimized when multimedia isn't in use,
i.e. the device is suspended. In terms of bumping up the voltage we have
to pin that on CX though as far as I know because that's the only power
domain that can actually change voltage, while MMCX merely gates that
voltage for multimedia.

No, MMCX is a separate rail from CX, which powers the display blocks and
is parent of MDSS_GDSC. But I see in rpmhpd that sc7180 is not one of
these platforms, so I presume this means that the displayport controller
thereby sits in MDSS_GDSC parented by CX.

But in line with what you're saying, the naming of the supplies to the
QMP indicates that the power for the PLLs is static. As such the only
moving things would be the clock rates in the DP controller and as such
that's what needs to scale the voltage.

So if the resources we're scaling is the clocks in the DP controller
then the gist of the patch is correct. The only details I see is that
the DP controller actually sits in MDSS_GDSC - while it should control
the level of its parent (CX). Not sure if we can describe that in a
simple way.

Right. I'm not sure things could be described any better right now. If
we need to change this to be MDSS_GDSC power domain and control the
level of the parent then I suppose we'll have to make some sort of DT
change and pair that with a driver change. Maybe if that happens we can
just pick a new compatible and leave the old code in place.

Are you happy enough with this current patch?

PS. Why does the node name of the opp-table have to be globally unique?

Presumably the opp table node name can be 'opp-table' as long as it
lives under the node that's using it. If the opp table is at / or /soc
then it will need to be unique. I'd prefer just 'opp-table' if possible.