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

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: khsieh@codeaurora.org, robdclark@gmail.com, sean@poorly.run,
	vkoul@kernel.org, agross@kernel.org, robh+dt@kernel.org,
	devicetree@vger.kernel.org, abhinavk@codeaurora.org,
	aravindh@codeaurora.org, freedreno@lists.freedesktop.org,
	linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] arm64/dts/qcom/sc7180: Add Display Port dt node
Date: Fri, 18 Jun 2021 16:41:50 -0500	[thread overview]
Message-ID: <YM0THrlJlv7ADW8w@builder.lan> (raw)
In-Reply-To: <CAE-0n52J_mLsmXLS+skZn2u3k9dhn+GcHeXi0B2BeQyQxEUL9A@mail.gmail.com>

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

> Quoting khsieh@codeaurora.org (2021-06-10 09:54:05)
> > On 2021-06-08 16:10, Bjorn Andersson wrote:
> > > On Tue 08 Jun 17:44 CDT 2021, Stephen Boyd wrote:
> > >
> > >> 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.

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

Regards,
Bjorn

> > >
> > >
> > > PS. While this goes for the QMPs the DSI and eDP/DP PHYs (and PLLs)
> > > seems to live in MMCX.
> > >
> > > Regards,
> > > Bjorn
> >
> > Dp link clock rate is sourced from phy/pll (vco). However it is possible
> > that different link clock rate
> > are sourced from same vco (phy/pll) rate. Therefore I think CX rail
> > voltage level is more proper to
> > be decided base on link clock rate.
> >