[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f677f8ee-d635-4131-8ee2-8ca6ead43095@oss.qualcomm.com>
Date: Fri, 24 Oct 2025 04:33:28 +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/22/2025 12:49 AM, Krzysztof Kozlowski wrote:
> On 21/10/2025 17:51, Akhil P Oommen wrote:
>> 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.
>
> Yes, like that.
>
>>
>> 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?
>
> Yes.
>
>>
>>>
>>>>
>>>> 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.
>
> Maybe the binding is getting to complicated and RGMU should have its own.
There is just a single chipset with RGMU and we haven't seen another one
in the last 8 yrs. So it is very unlikely we will see another one again.
So I feel it is not worth splitting this file just for RGMU.
Let me send another revision and let's take a call after that.
-Akhil.
>
>
> Best regards,
> Krzysztof
Powered by blists - more mailing lists