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

From: Stephen Boyd <swboyd@chromium.org>
To: Bjorn Andersson <bjorn.andersson@linaro.org>, khsieh@codeaurora.org
Cc: 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: Tue, 8 Jun 2021 15:15:29 -0700	[thread overview]
Message-ID: <CAE-0n50-X03sMyJdsw7s=Ue0dWXBo=iHOc0HxDQm5yh2J-uS3A@mail.gmail.com> (raw)
In-Reply-To: <YL6sY/1E5wLzMiP/@yoga>

Quoting Bjorn Andersson (2021-06-07 16:31:47)
> On Mon 07 Jun 12:48 CDT 2021, khsieh@codeaurora.org wrote:
>
> > On 2021-06-05 22:07, Bjorn Andersson wrote:
> > > On Thu 03 Jun 16:56 CDT 2021, khsieh@codeaurora.org wrote:
> > >
> > > > On 2021-06-03 09:53, Bjorn Andersson wrote:
> > > > > On Thu 03 Jun 11:09 CDT 2021, Kuogee Hsieh wrote:
> > > [..]
> > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
> > > [..]
> > > > > > +                             power-domains = <&rpmhpd SC7180_CX>;
> > > > >
> > > > > Just curious, but isn't the DP block in the MDSS_GDCS? Or do we need to
> > > > > mention CX here in order for the opp framework to apply required-opps
> > > > > of CX?
> > > >
> > > > yes,
> > >
> > > If you want me, or other maintainers, to spend any time reviewing or
> > > applying your patches going forward then you need to actually bother
> > > replying properly to the questions asked.
> > >
> > > Thanks,
> > > Bjorn
> >
> > Sorry about the confusion. What I meant is that even though DP controller is
> > in the MDSS_GDSC
> > power domain, DP PHY/PLL sources out of CX. The DP link clocks have a direct
> > impact
> > on the CX voltage corners. Therefore, we need to mention the CX power domain
> > here. And, since
> > we can associate only one OPP table with one device, we picked the DP link
> > clock over other
> > clocks.
>
> Thank you, that's a much more useful answer.
>
> Naturally I would think it would make more sense for the PHY/PLL driver
> to ensure that CX is appropriately voted for then, but I think that
> would result in it being the clock driver performing such vote and I'm
> unsure how the opp table for that would look.
>
> @Stephen, what do you say?
>

Wouldn't the PHY be the one that sets some vote? So it wouldn't be the
clk driver, and probably not from the clk ops, but instead come from the
phy ops via phy_enable() and phy_configure().

By the way, there's nothing wrong with a clk device doing power domain
"stuff", except for that we haven't plumbed it into the clk framework
properly and I'm fairly certain our usage of runtime PM in the clk
framework today underneath the prepare_lock is getting us into trouble
or will get us there soon.