On Tue 08 Jun 17:15 CDT 2021, Stephen Boyd wrote: > Quoting Bjorn Andersson (2021-06-07 16:31:47) > > On Mon 07 Jun 12:48 CDT 2021, khsieh@xxxxxxxxxxxxxx wrote: > > > > > On 2021-06-05 22:07, Bjorn Andersson wrote: > > > > On Thu 03 Jun 16:56 CDT 2021, khsieh@xxxxxxxxxxxxxx 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(). > 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
