linux-kernel - Re: [PATCH RFC 24/24] arm64: dts: qcom: x1e80100: Describe GPU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b903628-9abf-4b9e-971e-e9338308d693@oss.qualcomm.com>
Date: Tue, 29 Jul 2025 10:23:13 +0200
From: Konrad Dybcio <konrad.dybcio@....qualcomm.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
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 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.

Konrad