Re: [PATCH RFC 24/24] arm64: dts: qcom: x1e80100: Describe GPU_CC power plumbing requirements

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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@oss.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@oss.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@3d90000 {
>>>>  			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