| Message ID | 20211206150025.15703-2-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Mali-G31 GPU support for RZ/G2L SoC | expand |
On 2021-12-06 15:00, Biju Das wrote: > The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost Mali-G31 GPU, > add a compatible string for it. > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > v1->v2: > * Updated minItems for resets as 2 > * Documented optional property reset-names > * Documented reset-names as required property for RZ/G2L SoC. > --- > .../bindings/gpu/arm,mali-bifrost.yaml | 39 ++++++++++++++++++- > 1 file changed, 37 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > index 6f98dd55fb4c..c3b2f4ddd520 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > @@ -19,6 +19,7 @@ properties: > - amlogic,meson-g12a-mali > - mediatek,mt8183-mali > - realtek,rtd1619-mali > + - renesas,r9a07g044-mali > - rockchip,px30-mali > - rockchip,rk3568-mali > - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable > @@ -27,19 +28,30 @@ properties: > maxItems: 1 > > interrupts: > + minItems: 3 > items: > - description: Job interrupt > - description: MMU interrupt > - description: GPU interrupt > + - description: EVENT interrupt I believe we haven't bothered with the "Event" interrupt so far since there's no real meaningful use for it - it seems the downstream binding for Arm's kbase driver doesn't mention it either. > interrupt-names: > + minItems: 3 > items: > - const: job > - const: mmu > - const: gpu > + - const: event > > clocks: > - maxItems: 1 > + minItems: 1 > + maxItems: 3 > + > + clock-names: > + items: > + - const: gpu > + - const: bus > + - const: bus_ace Note that the Bifrost GPUs themselves all only have a single external clock and reset (unexcitingly named "CLK" and "RESETn" respectively, FWIW). I can't help feeling wary that defining additional names for vendor integration details in the core binding may quickly grow into a mess of mutually-incompatible sets of values, for no great benefit. At the very least, it would seem more sensible to put them in the SoC-specific conditional schemas. Robin. > > mali-supply: true > > @@ -52,7 +64,14 @@ properties: > maxItems: 3 > > resets: > - maxItems: 2 > + minItems: 2 > + maxItems: 3 > + > + reset-names: > + items: > + - const: rst > + - const: axi_rst > + - const: ace_rst > > "#cooling-cells": > const: 2 > @@ -113,6 +132,22 @@ allOf: > - sram-supply > - power-domains > - power-domain-names > + - if: > + properties: > + compatible: > + contains: > + const: renesas,r9a07g044-mali > + then: > + properties: > + interrupt-names: > + minItems: 4 > + clock-names: > + minItems: 3 > + required: > + - clock-names > + - power-domains > + - resets > + - reset-names > else: > properties: > power-domains: >
Hi Robin, Thanks for the feedback. > Subject: Re: [PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document > RZ/G2L support > > On 2021-12-06 15:00, Biju Das wrote: > > The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost Mali-G31 > > GPU, add a compatible string for it. > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > v1->v2: > > * Updated minItems for resets as 2 > > * Documented optional property reset-names > > * Documented reset-names as required property for RZ/G2L SoC. > > --- > > .../bindings/gpu/arm,mali-bifrost.yaml | 39 ++++++++++++++++++- > > 1 file changed, 37 insertions(+), 2 deletions(-) > > > > diff --git > > a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > index 6f98dd55fb4c..c3b2f4ddd520 100644 > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > @@ -19,6 +19,7 @@ properties: > > - amlogic,meson-g12a-mali > > - mediatek,mt8183-mali > > - realtek,rtd1619-mali > > + - renesas,r9a07g044-mali > > - rockchip,px30-mali > > - rockchip,rk3568-mali > > - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is > > fully discoverable @@ -27,19 +28,30 @@ properties: > > maxItems: 1 > > > > interrupts: > > + minItems: 3 > > items: > > - description: Job interrupt > > - description: MMU interrupt > > - description: GPU interrupt > > + - description: EVENT interrupt > > I believe we haven't bothered with the "Event" interrupt so far since > there's no real meaningful use for it - it seems the downstream binding > for Arm's kbase driver doesn't mention it either. I agree. But DT binding describes the H/W. Our SoC, mention about Event interrupt. That is the reason I have documented it. I am ok for keeping it or removing it. Please let me know. > > > interrupt-names: > > + minItems: 3 > > items: > > - const: job > > - const: mmu > > - const: gpu > > + - const: event > > > > clocks: > > - maxItems: 1 > > + minItems: 1 > > + maxItems: 3 > > + > > + clock-names: > > + items: > > + - const: gpu > > + - const: bus > > + - const: bus_ace > > Note that the Bifrost GPUs themselves all only have a single external > clock and reset (unexcitingly named "CLK" and "RESETn" respectively, > FWIW). I can't help feeling wary that defining additional names for vendor > integration details in the core binding may quickly grow into a mess of > mutually-incompatible sets of values, for no great benefit. At the very > least, it would seem more sensible to put them in the SoC-specific > conditional schemas. Initially GPU was not working on our platform. Then after debugging found that it needs, bus clock to make it work. This information is missing in dt binding and I need to find this info from source code. That is the reason, even if it is optional, I have documented with same name here. Regards, Biju > > Robin. > > > > > mali-supply: true > > > > @@ -52,7 +64,14 @@ properties: > > maxItems: 3 > > > > resets: > > - maxItems: 2 > > + minItems: 2 > > + maxItems: 3 > > + > > + reset-names: > > + items: > > + - const: rst > > + - const: axi_rst > > + - const: ace_rst > > > > "#cooling-cells": > > const: 2 > > @@ -113,6 +132,22 @@ allOf: > > - sram-supply > > - power-domains > > - power-domain-names > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: renesas,r9a07g044-mali > > + then: > > + properties: > > + interrupt-names: > > + minItems: 4 > > + clock-names: > > + minItems: 3 > > + required: > > + - clock-names > > + - power-domains > > + - resets > > + - reset-names > > else: > > properties: > > power-domains: > >
Hi, > Subject: RE: [PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document > RZ/G2L support > > Hi Robin, > > Thanks for the feedback. > > > Subject: Re: [PATCH v2 1/3] dt-bindings: gpu: mali-bifrost: Document > > RZ/G2L support > > > > On 2021-12-06 15:00, Biju Das wrote: > > > The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost > > > Mali-G31 GPU, add a compatible string for it. > > > > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > v1->v2: > > > * Updated minItems for resets as 2 > > > * Documented optional property reset-names > > > * Documented reset-names as required property for RZ/G2L SoC. > > > --- > > > .../bindings/gpu/arm,mali-bifrost.yaml | 39 > ++++++++++++++++++- > > > 1 file changed, 37 insertions(+), 2 deletions(-) > > > > > > diff --git > > > a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > index 6f98dd55fb4c..c3b2f4ddd520 100644 > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > @@ -19,6 +19,7 @@ properties: > > > - amlogic,meson-g12a-mali > > > - mediatek,mt8183-mali > > > - realtek,rtd1619-mali > > > + - renesas,r9a07g044-mali > > > - rockchip,px30-mali > > > - rockchip,rk3568-mali > > > - const: arm,mali-bifrost # Mali Bifrost GPU model/revision > > > is fully discoverable @@ -27,19 +28,30 @@ properties: > > > maxItems: 1 > > > > > > interrupts: > > > + minItems: 3 > > > items: > > > - description: Job interrupt > > > - description: MMU interrupt > > > - description: GPU interrupt > > > + - description: EVENT interrupt > > > > I believe we haven't bothered with the "Event" interrupt so far since > > there's no real meaningful use for it - it seems the downstream > > binding for Arm's kbase driver doesn't mention it either. > > I agree. > But DT binding describes the H/W. Our SoC, mention about Event interrupt. > That is the reason I have documented it. > > > > > interrupt-names: > > > + minItems: 3 > > > items: > > > - const: job > > > - const: mmu > > > - const: gpu > > > + - const: event > > > > > > clocks: > > > - maxItems: 1 > > > + minItems: 1 > > > + maxItems: 3 > > > + > > > + clock-names: > > > + items: > > > + - const: gpu > > > + - const: bus > > > + - const: bus_ace > > > > Note that the Bifrost GPUs themselves all only have a single external > > clock and reset (unexcitingly named "CLK" and "RESETn" respectively, > > FWIW). I can't help feeling wary that defining additional names for > > vendor integration details in the core binding may quickly grow into a > > mess of mutually-incompatible sets of values, for no great benefit. At > > the very least, it would seem more sensible to put them in the > > SoC-specific conditional schemas. > I agree, All optional properties like clock-names and reset-names should go in the SoC-specific conditional schemas. I will make clock-names and reset-names to true and handle it in the SoC-specific conditional schemas. I will send V3, incorporating the above. Regards, Biju > > > > > Robin. > > > > > > > > mali-supply: true > > > > > > @@ -52,7 +64,14 @@ properties: > > > maxItems: 3 > > > > > > resets: > > > - maxItems: 2 > > > + minItems: 2 > > > + maxItems: 3 > > > + > > > + reset-names: > > > + items: > > > + - const: rst > > > + - const: axi_rst > > > + - const: ace_rst > > > > > > "#cooling-cells": > > > const: 2 > > > @@ -113,6 +132,22 @@ allOf: > > > - sram-supply > > > - power-domains > > > - power-domain-names > > > + - if: > > > + properties: > > > + compatible: > > > + contains: > > > + const: renesas,r9a07g044-mali > > > + then: > > > + properties: > > > + interrupt-names: > > > + minItems: 4 > > > + clock-names: > > > + minItems: 3 > > > + required: > > > + - clock-names > > > + - power-domains > > > + - resets > > > + - reset-names > > > else: > > > properties: > > > power-domains: > > >
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 6f98dd55fb4c..c3b2f4ddd520 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -19,6 +19,7 @@ properties: - amlogic,meson-g12a-mali - mediatek,mt8183-mali - realtek,rtd1619-mali + - renesas,r9a07g044-mali - rockchip,px30-mali - rockchip,rk3568-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -27,19 +28,30 @@ properties: maxItems: 1 interrupts: + minItems: 3 items: - description: Job interrupt - description: MMU interrupt - description: GPU interrupt + - description: EVENT interrupt interrupt-names: + minItems: 3 items: - const: job - const: mmu - const: gpu + - const: event clocks: - maxItems: 1 + minItems: 1 + maxItems: 3 + + clock-names: + items: + - const: gpu + - const: bus + - const: bus_ace mali-supply: true @@ -52,7 +64,14 @@ properties: maxItems: 3 resets: - maxItems: 2 + minItems: 2 + maxItems: 3 + + reset-names: + items: + - const: rst + - const: axi_rst + - const: ace_rst "#cooling-cells": const: 2 @@ -113,6 +132,22 @@ allOf: - sram-supply - power-domains - power-domain-names + - if: + properties: + compatible: + contains: + const: renesas,r9a07g044-mali + then: + properties: + interrupt-names: + minItems: 4 + clock-names: + minItems: 3 + required: + - clock-names + - power-domains + - resets + - reset-names else: properties: power-domains:
The Renesas RZ/G2{L, LC} SoC (a.k.a R9A07G044) has a Bifrost Mali-G31 GPU, add a compatible string for it. Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- v1->v2: * Updated minItems for resets as 2 * Documented optional property reset-names * Documented reset-names as required property for RZ/G2L SoC. --- .../bindings/gpu/arm,mali-bifrost.yaml | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-)