Message-ID: <aIjsTgA7O7UqS-Oz@linaro.org>
Date: Tue, 29 Jul 2025 17:44:17 +0200
From: Stephan Gerhold <stephan.gerhold@...aro.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
Cc: Konrad Dybcio <konradybcio@...nel.org>,
	Ulf Hansson <ulf.hansson@...aro.org>,
	Johan Hovold <johan+linaro@...nel.org>,
	Bjorn Andersson <andersson@...nel.org>,
	Taniya Das <taniya.das@....qualcomm.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Taniya Das <quic_tdas@...cinc.com>,
	Imran Shaik <quic_imrashai@...cinc.com>,
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
	Dmitry Baryshkov <lumag@...nel.org>,
	cros-qcom-dts-watchers@...omium.org,
	Douglas Anderson <dianders@...omium.org>,
	Vinod Koul <vkoul@...nel.org>,
	Richard Acayan <mailingradian@...il.com>,
	Ajit Pandey <quic_ajipan@...cinc.com>,
	Luca Weiss <luca.weiss@...rphone.com>,
	Jonathan Marek <jonathan@...ek.ca>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Jagadeesh Kona <quic_jkona@...cinc.com>,
	Akhil P Oommen <akhilpo@....qualcomm.com>,
	Marijn Suijten <marijn.suijten@...ainline.org>,
	linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
	devicetree@...r.kernel.org,
	Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Subject: Re: [PATCH RFC 24/24] arm64: dts: qcom: x1e80100: Describe GPU_CC
 power plumbing requirements

On Tue, Jul 29, 2025 at 03:28:55PM +0200, Konrad Dybcio wrote:
> On 7/29/25 10:23 AM, Konrad Dybcio wrote:
> > On 7/29/25 8:34 AM, Stephan Gerhold wrote:
> >> On Mon, Jul 28, 2025 at 11:31:10PM +0200, Konrad Dybcio wrote:
> >>> On 7/28/25 7:10 PM, Stephan Gerhold wrote:
> >>>> On Mon, Jul 28, 2025 at 06:16:24PM +0200, Konrad Dybcio wrote:
> >>>>> From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> >>>>>
> >>>>> A number of power rails must be powered on in order for GPU_CC to
> >>>>> function. Ensure that's conveyed to the OS.
> >>>>>
> >>>>> Fixes: 721e38301b79 ("arm64: dts: qcom: x1e80100: Add gpu support")
> >>>>> Signed-off-by: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
> >>>>> ---
> >>>>>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 6 ++++++
> >>>>>  1 file changed, 6 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> index 5e9a8fa3cf96468b12775f91192cbd779d5ce946..6620517fbb0f3ed715c4901ec53dcbc6235be88f 100644
> >>>>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> >>>>> @@ -3928,6 +3928,12 @@ gpucc: clock-controller@...0000 {
> >>>>>  			clocks = <&bi_tcxo_div2>,
> >>>>>  				 <&gcc GCC_GPU_GPLL0_CPH_CLK_SRC>,
> >>>>>  				 <&gcc GCC_GPU_GPLL0_DIV_CPH_CLK_SRC>;
> >>>>> +
> >>>>> +			power-domains = <&rpmhpd RPMHPD_CX>,
> >>>>> +					<&rpmhpd RPMHPD_MX>,
> >>>>> +					<&rpmhpd RPMHPD_GFX>,
> >>>>> +					<&rpmhpd RPMHPD_GMXC>;
> >>>>> +
> >>>>>  			#clock-cells = <1>;
> >>>>>  			#reset-cells = <1>;
> >>>>>  			#power-domain-cells = <1>;
> >>>>>
> >>>>
> >>>> To repeat your own message from a couple of months back [1]:
> >>>>
> >>>>> You shouldn't be messing with VDD_GFX on platforms with a GMU.
> >>>>>
> >>>>> Parts of the clock controller are backed by one of the MX rails,
> >>>>> with some logic depending on CX/GFX, but handling of the latter is
> >>>>> fully deferred to the GMU firmware.
> >>>>>
> >>>>> Konrad
> >>>>
> >>>> Please describe somewhere in the cover letter or the individual patches
> >>>> how this relates to the responsibilities of the GMU. I searched for
> >>>> "GMU" in the patch series and couldn't find any note about this.
> >>>>
> >>>> Also: How much is a plain "power on" votes (without a corresponding
> >>>> "required-opps") really worth nowadays? An arbitrary low voltage level
> >>>> on those rails won't be sufficient to make the GPU_CC actually
> >>>> "function". Do you need "required-opps" here? In the videocc/camcc case
> >>>> we have those.
> >>>
> >>> Right, I failed to capture this.
> >>>
> >>> The GFX rail should be powered on before unclamping the GX_GDSC (as
> >>> per the programming guide). The clock controller HPG however doesn't
> >>> seem to have a concept of RPMh, so it says something that amounts to
> >>> "tell the PMIC to supply power on this rail". In Linux, since Commit
> >>> e3e56c050ab6 ("soc: qcom: rpmhpd: Make power_on actually enable the
> >>> domain") we don't really need a defined level for this (perhaps it's
> >>> more ""portable"" across potential fuse-bins if we don't hardcode the
> >>> lowest level anyway?).
> >>
> >> Thanks, I forgot that we have this commit.
> >>
> >>>
> >>> However after that happens, the level scaling is done by the GMU
> >>> firmware. This holds for allOf CX/MX/GFX. I'm not super sure if
> >>> both MX and (G)MXC need to both be captured together - downstream
> >>> seems to describe MXC as a child of MX (in socname-regulators.dtsi),
> >>> but I'm not really sure this is true in hardware.
> >>>
> >>> The GPU driver currently first enables the GX_GDSC and only then
> >>> does it kickstart the GMU firmware. Downstream seems to do that as
> >>> well. So on a second thought, since we've not seen any errors so
> >>> far, it calls into question what role the GFX rail plays in the
> >>> GX_GDSC's powering up..
> >>>
> >>
> >> It might play a role, but we wouldn't know since AFAICT we don't support
> >> enabling the GX_GDSC. Look at the beautiful gdsc_gx_do_nothing_enable()
> >> function, it basically just defers the entire task to the GMU. The GDSC
> >> just exists in Linux so we can turn it *off* during GMU crashes. :D
> > 
> > OHHHHH snap! I, on the other hand, forgot we have *that* commit..
> > 
> >> I think we should identify precisely which votes we are missing, instead
> >> of making blanket votes for all the power rails somehow related to the
> >> GPU. In this case this means: Which rails do we need to vote for to make
> >> the GMU turn on? If there are no votes necessary after the GMU is on,
> >> it's better to have none IMO.
> > 
> > The GMU pokes at RPMh directly (see a6xx_hfi.c), so we indeed just
> > need to make sure that it can turn on.. Which in short means the
> > *C*X_GDSC must be able to power up, which doesn't have any special
> > requirements. The only question that's left is basically whether
> > MX_C must be on. I'll try testing that in practice.
> 
> So this is apparently difficult, at least on SC8280XP, where something
> seems to be voting on MXC and it only seems to shut down when entering
> CXPC. I would imagine/hope this is not the case on newer platforms, but
> I don't have a way to fully confirm this at the moment..
> 

If in doubt, I would suggest to leave everything as-is for now until
someone actually runs into an issue caused by this (if this is even
possible). There are plenty other actual gaps to address. ;)

Stephan

Powered by blists - more mailing lists