Message-ID: <8f3f4874-2e82-473e-87bd-e3bd58089b90@kernel.org>
Date: Sun, 19 Oct 2025 11:13:03 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Akhil P Oommen <akhilpo@....qualcomm.com>,
 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>
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
Subject: Re: [PATCH 3/6] dt-bindings: display/msm/gmu: Document A612 RGMU

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.

> +    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.

>  
>    power-domain-names:
> -    items:
> -      - const: cx
> -      - const: gx
> +    minItems: 2
> +    maxItems: 3


No. Why?

>  
>    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.

> +        required:
> +          - clocks
> +          - clock-names
> +          - interrupts
> +          - interrupt-names
> +          - iommus
> +          - operating-points-v2
>  
>  examples:
>    - |
> 


Best regards,
Krzysztof

Powered by blists - more mailing lists