[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <181af756-09a1-4694-98c4-53cea556e172@oss.qualcomm.com>
Date: Tue, 21 Oct 2025 21:21:37 +0530
From: Akhil P Oommen <akhilpo@....qualcomm.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, Rob Clark <robin.clark@....qualcomm.com>,
Sean Paul <sean@...rly.run>, Konrad Dybcio <konradybcio@...nel.org>,
Dmitry Baryshkov <lumag@...nel.org>,
Abhinav Kumar
<abhinav.kumar@...ux.dev>,
Jessica Zhang <jessica.zhang@....qualcomm.com>,
Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Bjorn Andersson <andersson@...nel.org>
Subject: Re: [PATCH 3/6] dt-bindings: display/msm/gmu: Document A612 RGMU
On 10/19/2025 2:43 PM, Krzysztof Kozlowski wrote:
> On 17/10/2025 19:08, Akhil P Oommen wrote:
>> RGMU a.k.a Reduced Graphics Management Unit is a small state machine
>> with the sole purpose of providing IFPC (Inter Frame Power Collapse)
>> support. Compared to GMU, it doesn't manage GPU clock, voltage
>> scaling, bw voting or any other functionalities. All it does is detect
>> an idle GPU and toggle the GDSC switch. As it doesn't access DDR space,
>> it doesn't require iommu.
>>
>> So far, only Adreno 612 GPU has an RGMU core. Document RGMU in the GMU's
>> schema.
>>
>> Signed-off-by: Akhil P Oommen <akhilpo@....qualcomm.com>
>> ---
>> .../devicetree/bindings/display/msm/gmu.yaml | 98 +++++++++++++++++-----
>> 1 file changed, 79 insertions(+), 19 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml
>> index afc1879357440c137cadeb2d9a74ae8459570a25..a262d41755f09f21f607bf7a1fd567f386595f39 100644
>> --- a/Documentation/devicetree/bindings/display/msm/gmu.yaml
>> +++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml
>> @@ -26,6 +26,9 @@ properties:
>> - items:
>> - pattern: '^qcom,adreno-gmu-x[1-9][0-9][0-9]\.[0-9]$'
>> - const: qcom,adreno-gmu
>> + - items:
>> + - const: qcom,adreno-rgmu-612.0
>> + - const: qcom,adreno-rgmu
>> - const: qcom,adreno-gmu-wrapper
>>
>> reg:
>> @@ -45,24 +48,30 @@ properties:
>> maxItems: 7
>>
>> interrupts:
>> - items:
>> - - description: GMU HFI interrupt
>> - - description: GMU interrupt
>
>
> Both stay, just explain what is the first interrupt. You should not drop
> descriptions here. Look at every other binding - of course except that
> terrible Adreno GPU which is anti-example.
Do you mean we should use a OneOf and list both combo? Or elaborate the
description of the first interrupt to include OOB too? Something like:
- description: HFI interrupt on GMU and OOB interrupt on RGMU.
Sorry, I am a bit confused.
>
>> + minItems: 2
>> + maxItems: 2
>>
>> interrupt-names:
>> - items:
>> - - const: hfi
>> - - const: gmu
>> + oneOf:
>> + - items:
>> + - const: hfi
>> + description: GMU HFI interrupt
>
> No, descriptions never go to xxx-names, but to xxx.
>
>> + - const: gmu
>> + description: GMU interrupt
>> + - items:
>> + - const: oob
>> + description: GMU OOB interrupt
>> + - const: gmu
>> + description: GMU interrupt
>> +
>>
>> power-domains:
>> - items:
>> - - description: CX power domain
>> - - description: GX power domain
>> + minItems: 2
>> + maxItems: 3
>
> No.
I will keep the 'description'. Here, RGMU has 3 power domains instead of
2. I suppose we should add the description for the 3rd power domain here
and keep 'minItems: 2' property to override the default 3?
>
>>
>> power-domain-names:
>> - items:
>> - - const: cx
>> - - const: gx
>> + minItems: 2
>> + maxItems: 3
>
>
> No. Why?
Same as above.
>
>>
>> iommus:
>> maxItems: 1
>> @@ -86,6 +95,44 @@ required:
>> additionalProperties: false
>>
>> allOf:
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: qcom,adreno-rgmu-612.0
>> + then:
>> + properties:
>> + reg:
>> + items:
>> + - description: Core RGMU registers
>> + reg-names:
>> + items:
>> + - const: gmu
>> + clocks:
>> + items:
>> + - description: GMU clock
>> + - description: GPU CX clock
>> + - description: GPU AXI clock
>> + - description: GPU MEMNOC clock
>> + - description: GPU SMMU vote clock
>> + clock-names:
>> + items:
>> + - const: gmu
>> + - const: cxo
>> + - const: axi
>> + - const: memnoc
>> + - const: smmu_vote
>> + power-domains:
>> + items:
>> + - description: CX power domain
>> + - description: GX power domain
>> + - description: VDD_CX power domain
>> + power-domain-names:
>> + items:
>> + - const: cx
>> + - const: gx
>> + - const: vdd_cx
>
> This does not make even sense. Why did you remove the the common list
> from power-domain-names?
>
>> +
>> - if:
>> properties:
>> compatible:
>> @@ -313,13 +360,26 @@ allOf:
>> items:
>> - const: gmu
>> else:
>> - required:
>> - - clocks
>> - - clock-names
>> - - interrupts
>> - - interrupt-names
>> - - iommus
>> - - operating-points-v2
>> + if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: qcom,adreno-rgmu
>> + then:
>> + required:
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - interrupt-names
>> + - operating-points-v2
>> + else:
>
> No. Don't nest multiple ifs.
I guess we should split this. I will add a 'required' constraint to the
rgmu constraints above. And apply the below 'required' constraint
specifically to 'qcom,adreno-gmu' instead of the 'else' fallback case.
Please let me know if you have any suggestion.
-Akhil>
>> + required:
>> + - clocks
>> + - clock-names
>> + - interrupts
>> + - interrupt-names
>> + - iommus
>> + - operating-points-v2
>>
>> examples:
>> - |
>>
>
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists