A612 GPU has a new IP called RGMU (Reduced Graphics Management Unit)
which replaces GMU. But it doesn't do clock or voltage scaling. So we
need the gpu core clock in the GPU node along with the power domain to
do clock and voltage scaling from the kernel. Update the bindings to
describe this GPU.
Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
---
.../devicetree/bindings/display/msm/gpu.yaml | 24 ++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.yaml b/Documentation/devicetree/bindings/display/msm/gpu.yaml
index 826aafdcc20b..19507bfb6004 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/gpu.yaml
@@ -45,11 +45,11 @@ properties:
- const: amd,imageon
clocks:
- minItems: 2
+ minItems: 1
maxItems: 7
clock-names:
- minItems: 2
+ minItems: 1
maxItems: 7
reg:
@@ -387,6 +387,26 @@ allOf:
required:
- clocks
- clock-names
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,adreno-612.0
+ then:
+ properties:
+ clocks:
+ items:
+ - description: GPU Core clock
+
+ clock-names:
+ items:
+ - const: core
+
+ required:
+ - clocks
+ - clock-names
+
else:
if:
properties:
--
2.51.0On Sat, Nov 22, 2025 at 03:22:16AM +0530, Akhil P Oommen wrote: > + > + - if: > + properties: > + compatible: > + contains: > + const: qcom,adreno-612.0 > + then: > + properties: > + clocks: > + items: > + - description: GPU Core clock > + > + clock-names: > + items: > + - const: core > + > + required: > + - clocks > + - clock-names > + > else: I am pretty sure you break not only intention/logic behindi this else, but actually cause real warnings to appear. The else was intentional, right? So the pattern further will not match some of devices defined in if:. Now else is for different part, so only 612 out of these devices is excluded. There is a reason we do not want ever else:if: in bindings. If it appeared, sure, maybe there is some benefit of it, but it means you need to be more careful now. > if: > properties: > > -- > 2.51.0 >
On 11/22/2025 4:32 PM, Krzysztof Kozlowski wrote: > On Sat, Nov 22, 2025 at 03:22:16AM +0530, Akhil P Oommen wrote: >> + >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: qcom,adreno-612.0 >> + then: >> + properties: >> + clocks: >> + items: >> + - description: GPU Core clock >> + >> + clock-names: >> + items: >> + - const: core >> + >> + required: >> + - clocks >> + - clock-names >> + >> else: > > I am pretty sure you break not only intention/logic behindi this else, > but actually cause real warnings to appear. > > The else was intentional, right? So the pattern further will not match > some of devices defined in if:. Now else is for different part, so only > 612 out of these devices is excluded. > > There is a reason we do not want ever else:if: in bindings. If it > appeared, sure, maybe there is some benefit of it, but it means you need > to be more careful now. Aah! I missed that this comes under an 'allOf'. Not an expert in this syntax, does moving this entire block under an 'else' make sense? Or is there a saner alternative? Regrettably, I tested dtbs-check only for the talos-ride dtb. -Akhil. > >> if: >> properties: >> >> -- >> 2.51.0 >>
On 24/11/2025 22:39, Akhil P Oommen wrote: > On 11/22/2025 4:32 PM, Krzysztof Kozlowski wrote: >> On Sat, Nov 22, 2025 at 03:22:16AM +0530, Akhil P Oommen wrote: >>> + >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: qcom,adreno-612.0 >>> + then: >>> + properties: >>> + clocks: >>> + items: >>> + - description: GPU Core clock >>> + >>> + clock-names: >>> + items: >>> + - const: core >>> + >>> + required: >>> + - clocks >>> + - clock-names >>> + >>> else: >> >> I am pretty sure you break not only intention/logic behindi this else, >> but actually cause real warnings to appear. >> >> The else was intentional, right? So the pattern further will not match >> some of devices defined in if:. Now else is for different part, so only >> 612 out of these devices is excluded. >> >> There is a reason we do not want ever else:if: in bindings. If it >> appeared, sure, maybe there is some benefit of it, but it means you need >> to be more careful now. > > Aah! I missed that this comes under an 'allOf'. Not an expert in this The allOf does not matter here. If these were separate if:then: then it would be the same. > syntax, does moving this entire block under an 'else' make sense? Or is No, never nest blocks. > there a saner alternative? Not sure, I don't remember the code. Original code was not easy to read, with your changes it will not be easier. So the only alternative I see is to make it simple and obvious. Best regards, Krzysztof
On 11/25/2025 1:28 PM, Krzysztof Kozlowski wrote:
> On 24/11/2025 22:39, Akhil P Oommen wrote:
>> On 11/22/2025 4:32 PM, Krzysztof Kozlowski wrote:
>>> On Sat, Nov 22, 2025 at 03:22:16AM +0530, Akhil P Oommen wrote:
>>>> +
>>>> + - if:
>>>> + properties:
>>>> + compatible:
>>>> + contains:
>>>> + const: qcom,adreno-612.0
>>>> + then:
>>>> + properties:
>>>> + clocks:
>>>> + items:
>>>> + - description: GPU Core clock
>>>> +
>>>> + clock-names:
>>>> + items:
>>>> + - const: core
>>>> +
>>>> + required:
>>>> + - clocks
>>>> + - clock-names
>>>> +
>>>> else:
>>>
>>> I am pretty sure you break not only intention/logic behindi this else,
>>> but actually cause real warnings to appear.
>>>
>>> The else was intentional, right? So the pattern further will not match
>>> some of devices defined in if:. Now else is for different part, so only
>>> 612 out of these devices is excluded.
>>>
>>> There is a reason we do not want ever else:if: in bindings. If it
>>> appeared, sure, maybe there is some benefit of it, but it means you need
>>> to be more careful now.
>>
>> Aah! I missed that this comes under an 'allOf'. Not an expert in this
>
> The allOf does not matter here. If these were separate if:then: then it
> would be the same.
>
>> syntax, does moving this entire block under an 'else' make sense? Or is
>
> No, never nest blocks.
>
>> there a saner alternative?
>
> Not sure, I don't remember the code. Original code was not easy to read,
> with your changes it will not be easier. So the only alternative I see
> is to make it simple and obvious.
Could you please confirm if the below change would be okay?
@@ -384,6 +384,31 @@ allOf:
- if:
properties:
compatible:
contains:
enum:
- qcom,adreno-610.0
- qcom,adreno-619.1
- qcom,adreno-07000200
then:
properties:
clocks:
minItems: 6
maxItems: 6
clock-names:
items:
- const: core
description: GPU Core clock
- const: iface
description: GPU Interface clock
- const: mem_iface
description: GPU Memory Interface clock
- const: alt_mem_iface
description: GPU Alternative Memory Interface clock
- const: gmu
description: CX GMU clock
- const: xo
description: GPUCC clocksource clock
reg-names:
minItems: 1
items:
- const: kgsl_3d0_reg_memory
- const: cx_dbgc
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,adreno-612.0
+ then:
+ properties:
+ clocks:
+ items:
+ - description: GPU Core clock
+
+ clock-names:
+ items:
+ - const: core
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,adreno-610.0
+ - qcom,adreno-612.0
+ - qcom,adreno-619.1
+ - qcom,adreno-07000200
+ then:
required:
- clocks
- clock-names
else:
if:
properties:
compatible:
contains:
oneOf:
- pattern: '^qcom,adreno-[67][0-9][0-9]\.[0-9]+$'
- pattern: '^qcom,adreno-[0-9a-f]{8}$'
then: # Starting with A6xx, the clocks are usually defined in the
properties:
clocks: false
clock-names: false
reg-names:
minItems: 1
items:
- const: kgsl_3d0_reg_memory
- const: cx_mem
- const: cx_dbgc
-Akhil
>
>
> Best regards,
> Krzysztof
On 28/11/2025 11:29, Akhil P Oommen wrote:
> On 11/25/2025 1:28 PM, Krzysztof Kozlowski wrote:
>> On 24/11/2025 22:39, Akhil P Oommen wrote:
>>> On 11/22/2025 4:32 PM, Krzysztof Kozlowski wrote:
>>>> On Sat, Nov 22, 2025 at 03:22:16AM +0530, Akhil P Oommen wrote:
>>>>> +
>>>>> + - if:
>>>>> + properties:
>>>>> + compatible:
>>>>> + contains:
>>>>> + const: qcom,adreno-612.0
>>>>> + then:
>>>>> + properties:
>>>>> + clocks:
>>>>> + items:
>>>>> + - description: GPU Core clock
>>>>> +
>>>>> + clock-names:
>>>>> + items:
>>>>> + - const: core
>>>>> +
>>>>> + required:
>>>>> + - clocks
>>>>> + - clock-names
>>>>> +
>>>>> else:
>>>>
>>>> I am pretty sure you break not only intention/logic behindi this else,
>>>> but actually cause real warnings to appear.
>>>>
>>>> The else was intentional, right? So the pattern further will not match
>>>> some of devices defined in if:. Now else is for different part, so only
>>>> 612 out of these devices is excluded.
>>>>
>>>> There is a reason we do not want ever else:if: in bindings. If it
>>>> appeared, sure, maybe there is some benefit of it, but it means you need
>>>> to be more careful now.
>>>
>>> Aah! I missed that this comes under an 'allOf'. Not an expert in this
>>
>> The allOf does not matter here. If these were separate if:then: then it
>> would be the same.
>>
>>> syntax, does moving this entire block under an 'else' make sense? Or is
>>
>> No, never nest blocks.
>>
>>> there a saner alternative?
>>
>> Not sure, I don't remember the code. Original code was not easy to read,
>> with your changes it will not be easier. So the only alternative I see
>> is to make it simple and obvious.
>
> Could you please confirm if the below change would be okay?
>
> @@ -384,6 +384,31 @@ allOf:
>
> - if:
> properties:
> compatible:
> contains:
> enum:
> - qcom,adreno-610.0
> - qcom,adreno-619.1
> - qcom,adreno-07000200
> then:
> properties:
> clocks:
> minItems: 6
> maxItems: 6
>
> clock-names:
> items:
> - const: core
> description: GPU Core clock
> - const: iface
> description: GPU Interface clock
> - const: mem_iface
> description: GPU Memory Interface clock
> - const: alt_mem_iface
> description: GPU Alternative Memory Interface clock
> - const: gmu
> description: CX GMU clock
> - const: xo
> description: GPUCC clocksource clock
>
> reg-names:
> minItems: 1
> items:
> - const: kgsl_3d0_reg_memory
> - const: cx_dbgc
>
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: qcom,adreno-612.0
> + then:
> + properties:
> + clocks:
> + items:
> + - description: GPU Core clock
> +
> + clock-names:
> + items:
> + - const: core
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,adreno-610.0
> + - qcom,adreno-612.0
> + - qcom,adreno-619.1
> + - qcom,adreno-07000200
> + then:
> required:
> - clocks
> - clock-names
Yes, this should work, but I think it is better to require clocks in the
same block which defines them. You also need to restrict reg/reg-names
for the new device.
>
> else:
> if:
> properties:
> compatible:
> contains:
> oneOf:
> - pattern: '^qcom,adreno-[67][0-9][0-9]\.[0-9]+$'
> - pattern: '^qcom,adreno-[0-9a-f]{8}$'
This if:else:if: should be just removed and written in a way to choose
specific devices affected here. The code is not readable and your
mistake was a proof of that.
>
> then: # Starting with A6xx, the clocks are usually defined in the
> properties:
Best regards,
Krzysztof
© 2016 - 2025 Red Hat, Inc.