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

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

Quoting Bjorn Andersson (2021-06-07 16:31:47)

quoted

On Mon 07 Jun 12:48 CDT 2021, khsieh@codeaurora.org wrote:

quoted

On 2021-06-05 22:07, Bjorn Andersson wrote:

quoted

On Thu 03 Jun 16:56 CDT 2021, khsieh@codeaurora.org wrote:

quoted

On 2021-06-03 09:53, Bjorn Andersson wrote:

quoted

On Thu 03 Jun 11:09 CDT 2021, Kuogee Hsieh wrote:

[..]

quoted

quoted

quoted

diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi

[..]

quoted

quoted

quoted

+                             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().

If I understand the logic correctly *_configure_dp_phy() will both
configure the vco clock and "request" the clock framework to change the
rate.

So I presume what you're suggesting is that that would be the place to
cast the CX corner vote?

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.

On the bright side, it's wonderful that we're at a point where this is
not only a theoretical problem :)

Regards,
Bjorn