dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices
@ 2024-02-27  3:45 Adam Ford
  2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
                   ` (5 more replies)
  0 siblings, 6 replies; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

Renesas has used a few variants of the Power VR GPU in their R-Car
and RZ/G2 families.  While there is still some work to do at the Mesa
level,  adding these device tree nodes allows the Power VR driver
to enumerate and load the respective firmware located:

https://gitlab.freedesktop.org/imagination/linux-firmware/-/tree/powervr/powervr?ref_type=heads

Adam Ford (6):
  dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas
    GPUs
  arm64: dts: renesas: r8a774a1: Enable GPU
  arm64: dts: renesas: r8a774e1: Enable GPU
  arm64: dts: renesas: r8a77951: Enable GPU
  arm64: dts: renesas: r8a77960: Enable GPU
  arm64: dts: renesas: r8a77961: Enable GPU

 .../devicetree/bindings/gpu/img,powervr-rogue.yaml  | 13 ++++++++++++-
 arch/arm64/boot/dts/renesas/r8a774a1.dtsi           | 10 ++++++++++
 arch/arm64/boot/dts/renesas/r8a774e1.dtsi           | 10 ++++++++++
 arch/arm64/boot/dts/renesas/r8a77951.dtsi           | 10 ++++++++++
 arch/arm64/boot/dts/renesas/r8a77960.dtsi           | 10 ++++++++++
 arch/arm64/boot/dts/renesas/r8a77961.dtsi           | 10 ++++++++++
 6 files changed, 62 insertions(+), 1 deletion(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  7:48   ` Geert Uytterhoeven
  2024-03-07 12:40   ` Frank Binns
  2024-02-27  3:45 ` [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU Adam Ford
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

Update the binding to add support for various Renesas SoC's with PowerVR
Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
the table to indicate such like what was done for the ti,am62-gpu.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
index 256e252f8087..7c75104df09f 100644
--- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
+++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
@@ -14,6 +14,11 @@ properties:
   compatible:
     items:
       - enum:
+          - renesas,r8a774a1-gpu
+          - renesas,r8a774e1-gpu
+          - renesas,r8a77951-gpu
+          - renesas,r8a77960-gpu
+          - renesas,r8a77961-gpu
           - ti,am62-gpu
       - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
 
@@ -51,7 +56,13 @@ allOf:
       properties:
         compatible:
           contains:
-            const: ti,am62-gpu
+            enum:
+              - ti,am62-gpu
+              - renesas,r8a774a1-gpu
+              - renesas,r8a774e1-gpu
+              - renesas,r8a77951-gpu
+              - renesas,r8a77960-gpu
+              - renesas,r8a77961-gpu
     then:
       properties:
         clocks:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
  2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  8:09   ` Geert Uytterhoeven
  2024-02-27  9:31   ` Matt Coster
  2024-02-27  3:45 ` [PATCH 3/6] arm64: dts: renesas: r8a774e1: " Adam Ford
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
rogue_4.45.2.58_v1.fw available from Imagination.

When enumerated, it appears as:
  powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
  powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
index a8a44fe5e83b..8923d9624b39 100644
--- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
@@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
 			resets = <&cpg 408>;
 		};
 
+		gpu: gpu@fd000000 {
+			compatible = "renesas,r8a774a1-gpu", "img,img-axe";
+			reg = <0 0xfd000000 0 0x20000>;
+			clocks = <&cpg CPG_MOD 112>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&sysc R8A774A1_PD_3DG_B>;
+			resets = <&cpg 112>;
+		};
+
 		pciec0: pcie@fe000000 {
 			compatible = "renesas,pcie-r8a774a1",
 				     "renesas,pcie-rcar-gen3";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 3/6] arm64: dts: renesas: r8a774e1: Enable GPU
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
  2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
  2024-02-27  3:45 ` [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  8:10   ` Geert Uytterhoeven
  2024-02-27  3:45 ` [PATCH 4/6] arm64: dts: renesas: r8a77951: " Adam Ford
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, David Airlie,
	Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

The GPU on the RZ/G2H is a Rogue GX6650 which uses firmware
rogue_4.46.6.62_v1.fw available from Imagination.

When enumerated, it appears as:
 powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
 powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
index be55ae83944c..398c9df1577b 100644
--- a/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a774e1.dtsi
@@ -2464,6 +2464,16 @@ gic: interrupt-controller@f1010000 {
 			resets = <&cpg 408>;
 		};
 
+		gpu: gpu@fd000000 {
+			compatible = "renesas,r8a774e1-gpu", "img,img-axe";
+			reg = <0 0xfd000000 0 0x20000>;
+			clocks = <&cpg CPG_MOD 112>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&sysc R8A774E1_PD_3DG_E>;
+			resets = <&cpg 112>;
+		};
+
 		pciec0: pcie@fe000000 {
 			compatible = "renesas,pcie-r8a774e1",
 				     "renesas,pcie-rcar-gen3";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 4/6] arm64: dts: renesas: r8a77951: Enable GPU
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
                   ` (2 preceding siblings ...)
  2024-02-27  3:45 ` [PATCH 3/6] arm64: dts: renesas: r8a774e1: " Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  8:11   ` Geert Uytterhoeven
  2024-02-27  3:45 ` [PATCH 5/6] arm64: dts: renesas: r8a77960: " Adam Ford
  2024-02-27  3:45 ` [PATCH 6/6] arm64: dts: renesas: r8a77961: " Adam Ford
  5 siblings, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel

The GPU on the R-Car H3 is a Rogue GX6650 which uses firmware
rogue_4.46.6.61_v1.fw available from Imagination.

When enumerated, it appears as:
powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/renesas/r8a77951.dtsi b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
index bea4edd17d53..3e9defaeb00f 100644
--- a/arch/arm64/boot/dts/renesas/r8a77951.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
@@ -2771,6 +2771,16 @@ gic: interrupt-controller@f1010000 {
 			resets = <&cpg 408>;
 		};
 
+		gpu: gpu@fd000000 {
+			compatible = "renesas,r8a77951-gpu", "img,img-axe";
+			reg = <0 0xfd000000 0 0x20000>;
+			clocks = <&cpg CPG_MOD 112>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&sysc R8A7795_PD_3DG_E>;
+			resets = <&cpg 112>;
+		};
+
 		pciec0: pcie@fe000000 {
 			compatible = "renesas,pcie-r8a7795",
 				     "renesas,pcie-rcar-gen3";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 5/6] arm64: dts: renesas: r8a77960: Enable GPU
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
                   ` (3 preceding siblings ...)
  2024-02-27  3:45 ` [PATCH 4/6] arm64: dts: renesas: r8a77951: " Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  8:12   ` Geert Uytterhoeven
  2024-02-27  3:45 ` [PATCH 6/6] arm64: dts: renesas: r8a77961: " Adam Ford
  5 siblings, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel

The GPU on the R-Car M3-W is a Rogue GX6250 which uses firmware
rogue_4.45.2.58_v1.fw available from Imagination.

When enumerated, it appears as:
powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/renesas/r8a77960.dtsi b/arch/arm64/boot/dts/renesas/r8a77960.dtsi
index 7846fea8e40d..0f17bc3f2d9a 100644
--- a/arch/arm64/boot/dts/renesas/r8a77960.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77960.dtsi
@@ -2558,6 +2558,16 @@ gic: interrupt-controller@f1010000 {
 			resets = <&cpg 408>;
 		};
 
+		gpu: gpu@fd000000 {
+			compatible = "renesas,r8a77960-gpu", "img,img-axe";
+			reg = <0 0xfd000000 0 0x20000>;
+			clocks = <&cpg CPG_MOD 112>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&sysc R8A7796_PD_3DG_B>;
+			resets = <&cpg 112>;
+		};
+
 		pciec0: pcie@fe000000 {
 			compatible = "renesas,pcie-r8a7796",
 				     "renesas,pcie-rcar-gen3";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH 6/6] arm64: dts: renesas: r8a77961: Enable GPU
  2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
                   ` (4 preceding siblings ...)
  2024-02-27  3:45 ` [PATCH 5/6] arm64: dts: renesas: r8a77960: " Adam Ford
@ 2024-02-27  3:45 ` Adam Ford
  2024-02-27  8:12   ` Geert Uytterhoeven
  5 siblings, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-02-27  3:45 UTC (permalink / raw)
  To: dri-devel, linux-renesas-soc
  Cc: aford, Adam Ford, Frank Binns, Matt Coster, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Daniel Vetter,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, devicetree, linux-kernel

The GPU on the R-Car M3-W+ is a Rogue GX6250 which uses firmware
rogue_4.45.2.58_v1.fw available from Imagination.

When enumerated, it appears as:
powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/renesas/r8a77961.dtsi b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
index 58f9286a5ab5..cc17e624c069 100644
--- a/arch/arm64/boot/dts/renesas/r8a77961.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a77961.dtsi
@@ -2438,6 +2438,16 @@ gic: interrupt-controller@f1010000 {
 			resets = <&cpg 408>;
 		};
 
+		gpu: gpu@fd000000 {
+			compatible = "renesas,r8a77961-gpu", "img,img-axe";
+			reg = <0 0xfd000000 0 0x20000>;
+			clocks = <&cpg CPG_MOD 112>;
+			clock-names = "core";
+			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
+			power-domains = <&sysc R8A77961_PD_3DG_B>;
+			resets = <&cpg 112>;
+		};
+
 		pciec0: pcie@fe000000 {
 			compatible = "renesas,pcie-r8a77961",
 				     "renesas,pcie-rcar-gen3";
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
@ 2024-02-27  7:48   ` Geert Uytterhoeven
  2024-02-27  8:03     ` Geert Uytterhoeven
  2024-02-27  8:09     ` Geert Uytterhoeven
  2024-03-07 12:40   ` Frank Binns
  1 sibling, 2 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  7:48 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, devicetree, linux-kernel

Hi Adam,

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> Update the binding to add support for various Renesas SoC's with PowerVR
> Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> the table to indicate such like what was done for the ti,am62-gpu.
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -14,6 +14,11 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - renesas,r8a774a1-gpu

I would add a comment like this:

    - renesas,r8a774a1-gpu # PowerVR Series 6XT GX6650 on RZ/G2M

> +          - renesas,r8a774e1-gpu

    .. # PowerVR Series 6XT GX6650 on RZ/G2H

> +          - renesas,r8a77951-gpu

    ... # PowerVR Series 6XT GX6650 on R-Car H3 ES2.0+

> +          - renesas,r8a77960-gpu

    ... # PowerVR Series 6XT GX6250 on R-Car M3-W

> +          - renesas,r8a77961-gpu

    ... # PowerVR Series 6XT GX6250 on R-Car M3-W+

>            - ti,am62-gpu
>        - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
>
> @@ -51,7 +56,13 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: ti,am62-gpu
> +            enum:
> +              - ti,am62-gpu
> +              - renesas,r8a774a1-gpu
> +              - renesas,r8a774e1-gpu
> +              - renesas,r8a77951-gpu
> +              - renesas,r8a77960-gpu
> +              - renesas,r8a77961-gpu

Please preserve alphabetical sort order.

>      then:
>        properties:
>          clocks:
> --
> 2.43.0

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  7:48   ` Geert Uytterhoeven
@ 2024-02-27  8:03     ` Geert Uytterhoeven
  2024-02-27  8:09     ` Geert Uytterhoeven
  1 sibling, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:03 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, devicetree, linux-kernel

On Tue, Feb 27, 2024 at 8:48 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> > Update the binding to add support for various Renesas SoC's with PowerVR
> > Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> > the table to indicate such like what was done for the ti,am62-gpu.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Thanks for your patch!
>
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -14,6 +14,11 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > +          - renesas,r8a774a1-gpu
>
> I would add a comment like this:
>
>     - renesas,r8a774a1-gpu # PowerVR Series 6XT GX6650 on RZ/G2M

After reading [1], s/Series 6XT/Series6XT/g.

[1] "[PATCH 00/11] Device tree support for Imagination Series5 GPU"
    https://lore.kernel.org/all/20240109171950.31010-1-afd@ti.com/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  7:48   ` Geert Uytterhoeven
  2024-02-27  8:03     ` Geert Uytterhoeven
@ 2024-02-27  8:09     ` Geert Uytterhoeven
  2024-02-27 10:38       ` Geert Uytterhoeven
  1 sibling, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:09 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, devicetree, linux-kernel

Hi Adam,

On Tue, Feb 27, 2024 at 8:48 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> > Update the binding to add support for various Renesas SoC's with PowerVR
> > Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> > the table to indicate such like what was done for the ti,am62-gpu.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>

> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml

> > +          - renesas,r8a77951-gpu
>
>     ... # PowerVR Series 6XT GX6650 on R-Car H3 ES2.0+

All compatible values for R-Car H3 variants use the r8a7795 "base" value,
so that should be:

     - renesas,r8a7795-gpu # PowerVR Series 6XT GX6650 on R-Car H3

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27  3:45 ` [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU Adam Ford
@ 2024-02-27  8:09   ` Geert Uytterhoeven
  2024-02-27  9:31   ` Matt Coster
  1 sibling, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:09 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> rogue_4.45.2.58_v1.fw available from Imagination.
>
> When enumerated, it appears as:
>   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
>   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 3/6] arm64: dts: renesas: r8a774e1: Enable GPU
  2024-02-27  3:45 ` [PATCH 3/6] arm64: dts: renesas: r8a774e1: " Adam Ford
@ 2024-02-27  8:10   ` Geert Uytterhoeven
  0 siblings, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:10 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, devicetree, linux-kernel

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> The GPU on the RZ/G2H is a Rogue GX6650 which uses firmware
> rogue_4.46.6.62_v1.fw available from Imagination.
>
> When enumerated, it appears as:
>  powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
>  powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/6] arm64: dts: renesas: r8a77951: Enable GPU
  2024-02-27  3:45 ` [PATCH 4/6] arm64: dts: renesas: r8a77951: " Adam Ford
@ 2024-02-27  8:11   ` Geert Uytterhoeven
  2024-02-27 15:15     ` Adam Ford
  0 siblings, 1 reply; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:11 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

Hi Adam,

Thanks for your patch!

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> The GPU on the R-Car H3 is a Rogue GX6650 which uses firmware
> rogue_4.46.6.61_v1.fw available from Imagination.

s/61/62/

>
> When enumerated, it appears as:
> powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
> powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

Do you have a different build number?

On Salvator-XS with R-Car H3 ES2.0:

    powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
    powervr fd000000.gpu: [drm] FW version v1.0 (build 6538781 OS)
    [drm] Initialized powervr 1.0.0 20230904 for fd000000.gpu on minor 1

>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- a/arch/arm64/boot/dts/renesas/r8a77951.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
> @@ -2771,6 +2771,16 @@ gic: interrupt-controller@f1010000 {
>                         resets = <&cpg 408>;
>                 };
>
> +               gpu: gpu@fd000000 {
> +                       compatible = "renesas,r8a77951-gpu", "img,img-axe";

renesas,r8a7795-gpu

> +                       reg = <0 0xfd000000 0 0x20000>;
> +                       clocks = <&cpg CPG_MOD 112>;
> +                       clock-names = "core";
> +                       interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> +                       power-domains = <&sysc R8A7795_PD_3DG_E>;
> +                       resets = <&cpg 112>;
> +               };
> +
>                 pciec0: pcie@fe000000 {
>                         compatible = "renesas,pcie-r8a7795",
>                                      "renesas,pcie-rcar-gen3";

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 5/6] arm64: dts: renesas: r8a77960: Enable GPU
  2024-02-27  3:45 ` [PATCH 5/6] arm64: dts: renesas: r8a77960: " Adam Ford
@ 2024-02-27  8:12   ` Geert Uytterhoeven
  0 siblings, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:12 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> The GPU on the R-Car M3-W is a Rogue GX6250 which uses firmware
> rogue_4.45.2.58_v1.fw available from Imagination.
>
> When enumerated, it appears as:
> powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

On Salvator-X with R-Car M3-W ES1.0:

    powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
    powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
    [drm] Initialized powervr 1.0.0 20230904 for fd000000.gpu on minor 1

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 6/6] arm64: dts: renesas: r8a77961: Enable GPU
  2024-02-27  3:45 ` [PATCH 6/6] arm64: dts: renesas: r8a77961: " Adam Ford
@ 2024-02-27  8:12   ` Geert Uytterhoeven
  0 siblings, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27  8:12 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> The GPU on the R-Car M3-W+ is a Rogue GX6250 which uses firmware
> rogue_4.45.2.58_v1.fw available from Imagination.
>
> When enumerated, it appears as:
> powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> Signed-off-by: Adam Ford <aford173@gmail.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27  3:45 ` [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU Adam Ford
  2024-02-27  8:09   ` Geert Uytterhoeven
@ 2024-02-27  9:31   ` Matt Coster
  2024-02-27 11:04     ` Geert Uytterhoeven
  2024-02-27 11:50     ` Adam Ford
  1 sibling, 2 replies; 28+ messages in thread
From: Matt Coster @ 2024-02-27  9:31 UTC (permalink / raw)
  To: Adam Ford, dri-devel, linux-renesas-soc
  Cc: Adam Ford, Frank Binns, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, devicetree, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2932 bytes --]

Hi Adam,

Thanks for these patches! I'll just reply to this one patch, but my
comments apply to them all.

On 27/02/2024 03:45, Adam Ford wrote:
> The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> rogue_4.45.2.58_v1.fw available from Imagination.
> 
> When enumerated, it appears as:
>   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
>   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)

These messages are printed after verifying the firmware blob’s headers,
*before* attempting to upload it to the device. Just because they appear
in dmesg does *not* imply the device is functional beyond the handful of
register reads in pvr_load_gpu_id().

Since Mesa does not yet have support for this GPU, there’s not a lot
that can be done to actually test these bindings.

When we added upstream support for the first GPU (the AXE core in TI’s
AM62), we opted to wait until userspace was sufficiently progressed to
the point it could be used for testing. This thought process still
applies when adding new GPUs.

Our main concern is that adding bindings for GPUs implies a level of
support that cannot be tested. That in turn may make it challenging to
justify UAPI changes if/when they’re needed to actually make these GPUs
functional.

> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> index a8a44fe5e83b..8923d9624b39 100644
> --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
>  			resets = <&cpg 408>;
>  		};
>  
> +		gpu: gpu@fd000000 {
> +			compatible = "renesas,r8a774a1-gpu", "img,img-axe";

The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
with one. For prior art, see [1] where we added support for the MT8173
found in Elm Chromebooks R13 (also a Series6XT GPU).

> +			reg = <0 0xfd000000 0 0x20000>;
> +			clocks = <&cpg CPG_MOD 112>;
> +			clock-names = "core";

Series6XT cores have three clocks (see [1] again). I don’t have a
Renesas TRM to hand – do you know if their docs go into detail on the
GPU integration?

> +			interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> +			power-domains = <&sysc R8A774A1_PD_3DG_B>;
> +			resets = <&cpg 112>;
> +		};
> +
>  		pciec0: pcie@fe000000 {
>  			compatible = "renesas,pcie-r8a774a1",
>  				     "renesas,pcie-rcar-gen3";

As you probably expect by this point, I have to nack this series for
now. I appreciate your effort here and I’ll be happy to help you land
these once Mesa gains some form of usable support to allow testing.

Cheers,
Matt

[1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  8:09     ` Geert Uytterhoeven
@ 2024-02-27 10:38       ` Geert Uytterhoeven
  0 siblings, 0 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27 10:38 UTC (permalink / raw)
  To: Adam Ford
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Magnus Damm, devicetree, linux-kernel

On Tue, Feb 27, 2024 at 9:09 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Tue, Feb 27, 2024 at 8:48 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> > > Update the binding to add support for various Renesas SoC's with PowerVR
> > > Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> > > the table to indicate such like what was done for the ti,am62-gpu.
> > >
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> > > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
>
> > > +          - renesas,r8a77951-gpu
> >
> >     ... # PowerVR Series 6XT GX6650 on R-Car H3 ES2.0+
>
> All compatible values for R-Car H3 variants use the r8a7795 "base" value,
> so that should be:
>
>      - renesas,r8a7795-gpu # PowerVR Series 6XT GX6650 on R-Car H3

Same for R-Car M3-W, so

  - renesas,r8a77960-gpu # PowerVR Series 6XT GX6250 on R-Car M3-W

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27  9:31   ` Matt Coster
@ 2024-02-27 11:04     ` Geert Uytterhoeven
  2024-02-27 11:54       ` Adam Ford
  2024-03-07 12:01       ` Frank Binns
  2024-02-27 11:50     ` Adam Ford
  1 sibling, 2 replies; 28+ messages in thread
From: Geert Uytterhoeven @ 2024-02-27 11:04 UTC (permalink / raw)
  To: Matt Coster
  Cc: Adam Ford, dri-devel, linux-renesas-soc, Adam Ford, Frank Binns,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

Hi Matt,

On Tue, Feb 27, 2024 at 10:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
>
> Hi Adam,
>
> Thanks for these patches! I'll just reply to this one patch, but my
> comments apply to them all.
>
> On 27/02/2024 03:45, Adam Ford wrote:
> > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > rogue_4.45.2.58_v1.fw available from Imagination.
> >
> > When enumerated, it appears as:
> >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> These messages are printed after verifying the firmware blob’s headers,
> *before* attempting to upload it to the device. Just because they appear
> in dmesg does *not* imply the device is functional beyond the handful of
> register reads in pvr_load_gpu_id().
>
> Since Mesa does not yet have support for this GPU, there’s not a lot
> that can be done to actually test these bindings.

OK.

> When we added upstream support for the first GPU (the AXE core in TI’s
> AM62), we opted to wait until userspace was sufficiently progressed to
> the point it could be used for testing. This thought process still
> applies when adding new GPUs.
>
> Our main concern is that adding bindings for GPUs implies a level of
> support that cannot be tested. That in turn may make it challenging to
> justify UAPI changes if/when they’re needed to actually make these GPUs
> functional.

I guess that applies to "[PATCH 00/11] Device tree support for
Imagination Series5 GPU", too, which has been in linux-next for about
a month?
https://lore.kernel.org/all/20240109171950.31010-1-afd@ti.com/

> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > index a8a44fe5e83b..8923d9624b39 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> >                       resets = <&cpg 408>;
> >               };
> >
> > +             gpu: gpu@fd000000 {
> > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
>
> The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> with one. For prior art, see [1] where we added support for the MT8173
> found in Elm Chromebooks R13 (also a Series6XT GPU).

IC. And the bindings in [2].

>
> > +                     reg = <0 0xfd000000 0 0x20000>;
> > +                     clocks = <&cpg CPG_MOD 112>;
> > +                     clock-names = "core";
>
> Series6XT cores have three clocks (see [1] again). I don’t have a
> Renesas TRM to hand – do you know if their docs go into detail on the
> GPU integration?

Not really. The diagram in the Hardware User's Manual just shows the
following clock inputs:
  - Clock (ZGϕ) from CPG,
  - Clock (S3D1ϕ) from CPG,
  - MSTP (ST112) from CPG.

ZG is the main (programmable) 3DGE clock, running at up to 600 MHz.
S3D1 is the fixed 266 MHz AXI bus clock.
MSTP112 is the gateable module clock (part of the SYSC/CPG clock
domain), and its parent is ZG.

According to the sources:
  - "core" is the primary clock used by the entire GPU core, so we use
    MSTP112 for that.
  - "sys" is the optional system bus clock, so that could be S3D1,
  - "mem" is the optional memory clock, no idea what that would map to.

But IMHO the two optional clocks do not matter at all (the driver
doesn't care about their rates, and just enables them together with
the core clock), and S3D1 is always-on, so I'd just limit clocks to
a single item.

Just wondering: is the availability of 1 clock specific to AXE, or to
the AXE integration on AM62x?

> +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> +                     resets = <&cpg 112>;
> +             };

> [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

[2] https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/Documentation/devicetree/bindings/gpu/img,powervr.yaml


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27  9:31   ` Matt Coster
  2024-02-27 11:04     ` Geert Uytterhoeven
@ 2024-02-27 11:50     ` Adam Ford
  2024-03-07 12:26       ` Frank Binns
  1 sibling, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-02-27 11:50 UTC (permalink / raw)
  To: Matt Coster
  Cc: dri-devel, linux-renesas-soc, Adam Ford, Frank Binns,
	David Airlie, Daniel Vetter, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

On Tue, Feb 27, 2024 at 3:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
>
> Hi Adam,
>
> Thanks for these patches! I'll just reply to this one patch, but my
> comments apply to them all.
>
> On 27/02/2024 03:45, Adam Ford wrote:
> > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > rogue_4.45.2.58_v1.fw available from Imagination.
> >
> > When enumerated, it appears as:
> >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> These messages are printed after verifying the firmware blob’s headers,
> *before* attempting to upload it to the device. Just because they appear
> in dmesg does *not* imply the device is functional beyond the handful of
> register reads in pvr_load_gpu_id().
>
> Since Mesa does not yet have support for this GPU, there’s not a lot
> that can be done to actually test these bindings.
>
> When we added upstream support for the first GPU (the AXE core in TI’s
> AM62), we opted to wait until userspace was sufficiently progressed to
> the point it could be used for testing. This thought process still
> applies when adding new GPUs.
>
> Our main concern is that adding bindings for GPUs implies a level of
> support that cannot be tested. That in turn may make it challenging to
> justify UAPI changes if/when they’re needed to actually make these GPUs
> functional.

I wrongly assumed that when the firmware was ready, there was some
preliminary functionality, but it sounds like we need to work for
Series6XT support to be added to the driver.  I only used the AXE
compatible since it appeared to the be the only one and the existing
binding document stated "model/revision is fully discoverable" which I
interpreted to mean that the AXE compatible was sufficient.
>
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > index a8a44fe5e83b..8923d9624b39 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> >                       resets = <&cpg 408>;
> >               };
> >
> > +             gpu: gpu@fd000000 {
> > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
>
> The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> with one. For prior art, see [1] where we added support for the MT8173
> found in Elm Chromebooks R13 (also a Series6XT GPU).
>
> > +                     reg = <0 0xfd000000 0 0x20000>;
> > +                     clocks = <&cpg CPG_MOD 112>;
> > +                     clock-names = "core";
>
> Series6XT cores have three clocks (see [1] again). I don’t have a
> Renesas TRM to hand – do you know if their docs go into detail on the
> GPU integration?
>
> > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > +                     resets = <&cpg 112>;
> > +             };
> > +
> >               pciec0: pcie@fe000000 {
> >                       compatible = "renesas,pcie-r8a774a1",
> >                                    "renesas,pcie-rcar-gen3";
>
> As you probably expect by this point, I have to nack this series for
> now. I appreciate your effort here and I’ll be happy to help you land

I get that.  I wasn't sure if I should have even pushed this, but I
wanted to get a little traction, because I know there are people like
myself who want to use the 3D in the Renesas boards, but don't want to
use the closed-source blobs tied to EULA and NDA documents.

> these once Mesa gains some form of usable support to allow testing.

Is there a way for your group to add me to the CC list when future
updates are submitted?  I'd like to follow this and resubmit when it's
ready.
>
> Cheers,
> Matt
>
> [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27 11:04     ` Geert Uytterhoeven
@ 2024-02-27 11:54       ` Adam Ford
  2024-03-07 12:01       ` Frank Binns
  1 sibling, 0 replies; 28+ messages in thread
From: Adam Ford @ 2024-02-27 11:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Matt Coster, dri-devel, linux-renesas-soc, Adam Ford,
	Frank Binns, David Airlie, Daniel Vetter, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, devicetree, linux-kernel

On Tue, Feb 27, 2024 at 5:04 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Matt,
>
> On Tue, Feb 27, 2024 at 10:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> >
> > Hi Adam,
> >
> > Thanks for these patches! I'll just reply to this one patch, but my
> > comments apply to them all.
> >
> > On 27/02/2024 03:45, Adam Ford wrote:
> > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > rogue_4.45.2.58_v1.fw available from Imagination.
> > >
> > > When enumerated, it appears as:
> > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> >
> > These messages are printed after verifying the firmware blob’s headers,
> > *before* attempting to upload it to the device. Just because they appear
> > in dmesg does *not* imply the device is functional beyond the handful of
> > register reads in pvr_load_gpu_id().
> >
> > Since Mesa does not yet have support for this GPU, there’s not a lot
> > that can be done to actually test these bindings.
>
> OK.
>
> > When we added upstream support for the first GPU (the AXE core in TI’s
> > AM62), we opted to wait until userspace was sufficiently progressed to
> > the point it could be used for testing. This thought process still
> > applies when adding new GPUs.
> >
> > Our main concern is that adding bindings for GPUs implies a level of
> > support that cannot be tested. That in turn may make it challenging to
> > justify UAPI changes if/when they’re needed to actually make these GPUs
> > functional.
>
> I guess that applies to "[PATCH 00/11] Device tree support for
> Imagination Series5 GPU", too, which has been in linux-next for about
> a month?
> https://lore.kernel.org/all/20240109171950.31010-1-afd@ti.com/
>
> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > >
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > index a8a44fe5e83b..8923d9624b39 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > >                       resets = <&cpg 408>;
> > >               };
> > >
> > > +             gpu: gpu@fd000000 {
> > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> >
> > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > with one. For prior art, see [1] where we added support for the MT8173
> > found in Elm Chromebooks R13 (also a Series6XT GPU).
>
> IC. And the bindings in [2].
>
> >
> > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > +                     clocks = <&cpg CPG_MOD 112>;
> > > +                     clock-names = "core";
> >
> > Series6XT cores have three clocks (see [1] again). I don’t have a
> > Renesas TRM to hand – do you know if their docs go into detail on the
> > GPU integration?
>
> Not really. The diagram in the Hardware User's Manual just shows the
> following clock inputs:
>   - Clock (ZGϕ) from CPG,
>   - Clock (S3D1ϕ) from CPG,
>   - MSTP (ST112) from CPG.
>
> ZG is the main (programmable) 3DGE clock, running at up to 600 MHz.
> S3D1 is the fixed 266 MHz AXI bus clock.
> MSTP112 is the gateable module clock (part of the SYSC/CPG clock
> domain), and its parent is ZG.
>
> According to the sources:
>   - "core" is the primary clock used by the entire GPU core, so we use
>     MSTP112 for that.
>   - "sys" is the optional system bus clock, so that could be S3D1,
>   - "mem" is the optional memory clock, no idea what that would map to.
>
> But IMHO the two optional clocks do not matter at all (the driver
> doesn't care about their rates, and just enables them together with
> the core clock), and S3D1 is always-on, so I'd just limit clocks to
> a single item.

Matt,

When the time is right, and the driver is ready for Series 6XT-based
systems, would Geert's rationale for supporting one clock be
acceptable if I added his clock description to the commit message?

>
> Just wondering: is the availability of 1 clock specific to AXE, or to
> the AXE integration on AM62x?
>
> > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > +                     resets = <&cpg 112>;
> > +             };
>
> > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006
>
> [2] https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/Documentation/devicetree/bindings/gpu/img,powervr.yaml
>
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 4/6] arm64: dts: renesas: r8a77951: Enable GPU
  2024-02-27  8:11   ` Geert Uytterhoeven
@ 2024-02-27 15:15     ` Adam Ford
  0 siblings, 0 replies; 28+ messages in thread
From: Adam Ford @ 2024-02-27 15:15 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: dri-devel, linux-renesas-soc, aford, Frank Binns, Matt Coster,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
	David Airlie, Daniel Vetter, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Geert Uytterhoeven, Magnus Damm, devicetree,
	linux-kernel

On Tue, Feb 27, 2024 at 2:11 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Adam,
>
> Thanks for your patch!
>
> On Tue, Feb 27, 2024 at 4:46 AM Adam Ford <aford173@gmail.com> wrote:
> > The GPU on the R-Car H3 is a Rogue GX6650 which uses firmware
> > rogue_4.46.6.61_v1.fw available from Imagination.
>
> s/61/62/
>
> >
> > When enumerated, it appears as:
> > powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
> > powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
>
> Do you have a different build number?

I don't actually have this board.  I just copy-pasted what I thought
it was.  If you have  build number that's more appropriate, I can use
that in the future.

adam
>
> On Salvator-XS with R-Car H3 ES2.0:
>
>     powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.46.6.62_v1.fw
>     powervr fd000000.gpu: [drm] FW version v1.0 (build 6538781 OS)
>     [drm] Initialized powervr 1.0.0 20230904 for fd000000.gpu on minor 1
>
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
>
> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> > --- a/arch/arm64/boot/dts/renesas/r8a77951.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a77951.dtsi
> > @@ -2771,6 +2771,16 @@ gic: interrupt-controller@f1010000 {
> >                         resets = <&cpg 408>;
> >                 };
> >
> > +               gpu: gpu@fd000000 {
> > +                       compatible = "renesas,r8a77951-gpu", "img,img-axe";
>
> renesas,r8a7795-gpu
>
> > +                       reg = <0 0xfd000000 0 0x20000>;
> > +                       clocks = <&cpg CPG_MOD 112>;
> > +                       clock-names = "core";
> > +                       interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > +                       power-domains = <&sysc R8A7795_PD_3DG_E>;
> > +                       resets = <&cpg 112>;
> > +               };
> > +
> >                 pciec0: pcie@fe000000 {
> >                         compatible = "renesas,pcie-r8a7795",
> >                                      "renesas,pcie-rcar-gen3";
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27 11:04     ` Geert Uytterhoeven
  2024-02-27 11:54       ` Adam Ford
@ 2024-03-07 12:01       ` Frank Binns
  1 sibling, 0 replies; 28+ messages in thread
From: Frank Binns @ 2024-03-07 12:01 UTC (permalink / raw)
  To: Matt Coster, geert
  Cc: robh, aford, krzysztof.kozlowski+dt, tzimmermann, mripard,
	daniel, linux-kernel, dri-devel, conor+dt, aford173, devicetree,
	linux-renesas-soc, maarten.lankhorst, geert+renesas, magnus.damm,
	airlied

Hi Geert,

On Tue, 2024-02-27 at 12:04 +0100, Geert Uytterhoeven wrote:
> Hi Matt,
> 
> On Tue, Feb 27, 2024 at 10:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> > Hi Adam,
> > 
> > Thanks for these patches! I'll just reply to this one patch, but my
> > comments apply to them all.
> > 
> > On 27/02/2024 03:45, Adam Ford wrote:
> > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > rogue_4.45.2.58_v1.fw available from Imagination.
> > > 
> > > When enumerated, it appears as:
> > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> > 
> > These messages are printed after verifying the firmware blob’s headers,
> > *before* attempting to upload it to the device. Just because they appear
> > in dmesg does *not* imply the device is functional beyond the handful of
> > register reads in pvr_load_gpu_id().
> > 
> > Since Mesa does not yet have support for this GPU, there’s not a lot
> > that can be done to actually test these bindings.
> 
> OK.
> 
> > When we added upstream support for the first GPU (the AXE core in TI’s
> > AM62), we opted to wait until userspace was sufficiently progressed to
> > the point it could be used for testing. This thought process still
> > applies when adding new GPUs.
> > 
> > Our main concern is that adding bindings for GPUs implies a level of
> > support that cannot be tested. That in turn may make it challenging to
> > justify UAPI changes if/when they’re needed to actually make these GPUs
> > functional.
> 
> I guess that applies to "[PATCH 00/11] Device tree support for
> Imagination Series5 GPU", too, which has been in linux-next for about
> a month?
> https://lore.kernel.org/all/20240109171950.31010-1-afd@ti.com/
> 

The concern Matt has expressed is mostly right. However, adding DT bindings
doesn't prevent us from making UAPI changes. The thing that would prevent this
is adding the compatible(s) to the driver's `struct of_device_id` table, so this
is what we need to avoid doing until sufficient testing has been done.

> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > index a8a44fe5e83b..8923d9624b39 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > >                       resets = <&cpg 408>;
> > >               };
> > > 
> > > +             gpu: gpu@fd000000 {
> > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> > 
> > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > with one. For prior art, see [1] where we added support for the MT8173
> > found in Elm Chromebooks R13 (also a Series6XT GPU).
> 
> IC. And the bindings in [2].
> 
> > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > +                     clocks = <&cpg CPG_MOD 112>;
> > > +                     clock-names = "core";
> > 
> > Series6XT cores have three clocks (see [1] again). I don’t have a
> > Renesas TRM to hand – do you know if their docs go into detail on the
> > GPU integration?
> 
> Not really. The diagram in the Hardware User's Manual just shows the
> following clock inputs:
>   - Clock (ZGϕ) from CPG,
>   - Clock (S3D1ϕ) from CPG,
>   - MSTP (ST112) from CPG.
> 
> ZG is the main (programmable) 3DGE clock, running at up to 600 MHz.
> S3D1 is the fixed 266 MHz AXI bus clock.
> MSTP112 is the gateable module clock (part of the SYSC/CPG clock
> domain), and its parent is ZG.
> 
> According to the sources:
>   - "core" is the primary clock used by the entire GPU core, so we use
>     MSTP112 for that.
>   - "sys" is the optional system bus clock, so that could be S3D1,
>   - "mem" is the optional memory clock, no idea what that would map to.
> 

The sys and mem clocks are optional in the driver as AXE can be configured at
integration time to operate with a single clock ("core") or three clocks
("core", "sys" and "mem). Series6XT doesn't have this configurability and
expects all three clocks.

> But IMHO the two optional clocks do not matter at all (the driver
> doesn't care about their rates, and just enables them together with
> the core clock), and S3D1 is always-on, so I'd just limit clocks to
> a single item.
> 

They matter in that, if present, we may be able to make additional power
savings. The driver doesn't attempt to take advantage of this at present (and
can't for AM62x anyway), but this shouldn't influence the DT bindings.

> Just wondering: is the availability of 1 clock specific to AXE, or to
> the AXE integration on AM62x?
> 

This is something TI have configured as part of their integration for AM62x.

Thanks
Frank

> > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > +                     resets = <&cpg 112>;
> > +             };
> > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006
> 
> [2] https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/Documentation/devicetree/bindings/gpu/img,powervr.yaml
> 
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-02-27 11:50     ` Adam Ford
@ 2024-03-07 12:26       ` Frank Binns
  2024-03-07 12:37         ` Frank Binns
  0 siblings, 1 reply; 28+ messages in thread
From: Frank Binns @ 2024-03-07 12:26 UTC (permalink / raw)
  To: aford173, Matt Coster
  Cc: magnus.damm, linux-kernel, tzimmermann, devicetree, dri-devel,
	robh, linux-renesas-soc, airlied, geert+renesas,
	maarten.lankhorst, krzysztof.kozlowski+dt, aford, daniel,
	conor+dt, mripard

On Tue, 2024-02-27 at 05:50 -0600, Adam Ford wrote:
> On Tue, Feb 27, 2024 at 3:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> > Hi Adam,
> > 
> > Thanks for these patches! I'll just reply to this one patch, but my
> > comments apply to them all.
> > 
> > On 27/02/2024 03:45, Adam Ford wrote:
> > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > rogue_4.45.2.58_v1.fw available from Imagination.
> > > 
> > > When enumerated, it appears as:
> > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> > 
> > These messages are printed after verifying the firmware blob’s headers,
> > *before* attempting to upload it to the device. Just because they appear
> > in dmesg does *not* imply the device is functional beyond the handful of
> > register reads in pvr_load_gpu_id().
> > 
> > Since Mesa does not yet have support for this GPU, there’s not a lot
> > that can be done to actually test these bindings.
> > 
> > When we added upstream support for the first GPU (the AXE core in TI’s
> > AM62), we opted to wait until userspace was sufficiently progressed to
> > the point it could be used for testing. This thought process still
> > applies when adding new GPUs.
> > 
> > Our main concern is that adding bindings for GPUs implies a level of
> > support that cannot be tested. That in turn may make it challenging to
> > justify UAPI changes if/when they’re needed to actually make these GPUs
> > functional.
> 
> I wrongly assumed that when the firmware was ready, there was some
> preliminary functionality, but it sounds like we need to work for
> Series6XT support to be added to the driver.  I only used the AXE
> compatible since it appeared to the be the only one and the existing
> binding document stated "model/revision is fully discoverable" which I
> interpreted to mean that the AXE compatible was sufficient.

The comment is related to there being a few models/revisions of AXE [1][2][3],
which we can distinguish between by reading a register.

> > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > 
> > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > index a8a44fe5e83b..8923d9624b39 100644
> > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > >                       resets = <&cpg 408>;
> > >               };
> > > 
> > > +             gpu: gpu@fd000000 {
> > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> > 
> > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > with one. For prior art, see [1] where we added support for the MT8173
> > found in Elm Chromebooks R13 (also a Series6XT GPU).
> > 
> > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > +                     clocks = <&cpg CPG_MOD 112>;
> > > +                     clock-names = "core";
> > 
> > Series6XT cores have three clocks (see [1] again). I don’t have a
> > Renesas TRM to hand – do you know if their docs go into detail on the
> > GPU integration?
> > 
> > > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > > +                     resets = <&cpg 112>;
> > > +             };
> > > +
> > >               pciec0: pcie@fe000000 {
> > >                       compatible = "renesas,pcie-r8a774a1",
> > >                                    "renesas,pcie-rcar-gen3";
> > 
> > As you probably expect by this point, I have to nack this series for
> > now. I appreciate your effort here and I’ll be happy to help you land
> 
> I get that.  I wasn't sure if I should have even pushed this, but I
> wanted to get a little traction, because I know there are people like
> myself who want to use the 3D in the Renesas boards, but don't want to
> use the closed-source blobs tied to EULA and NDA documents.
> 
> > these once Mesa gains some form of usable support to allow testing.
> 
> Is there a way for your group to add me to the CC list when future
> updates are submitted?  I'd like to follow this and resubmit when it's
> ready.

Sure, we'll keep you updated as things progress.

Thanks
Frank

[1] https://www.imaginationtech.com/product/img-axe-1-16m/
[2] https://www.imaginationtech.com/product/img-axe-1-16/
[3] https://www.imaginationtech.com/product/img-axe-2-16/

> > Cheers,
> > Matt
> > 
> > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-03-07 12:26       ` Frank Binns
@ 2024-03-07 12:37         ` Frank Binns
  2024-03-07 13:31           ` Adam Ford
  0 siblings, 1 reply; 28+ messages in thread
From: Frank Binns @ 2024-03-07 12:37 UTC (permalink / raw)
  To: aford173, Matt Coster
  Cc: maarten.lankhorst, daniel, tzimmermann, mripard, dri-devel, robh,
	linux-renesas-soc, airlied, geert+renesas, magnus.damm,
	devicetree, krzysztof.kozlowski+dt, linux-kernel, conor+dt,
	aford

On Thu, 2024-03-07 at 12:26 +0000, Frank Binns wrote:
> On Tue, 2024-02-27 at 05:50 -0600, Adam Ford wrote:
> > On Tue, Feb 27, 2024 at 3:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> > > Hi Adam,
> > > 
> > > Thanks for these patches! I'll just reply to this one patch, but my
> > > comments apply to them all.
> > > 
> > > On 27/02/2024 03:45, Adam Ford wrote:
> > > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > > rogue_4.45.2.58_v1.fw available from Imagination.
> > > > 
> > > > When enumerated, it appears as:
> > > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> > > 
> > > These messages are printed after verifying the firmware blob’s headers,
> > > *before* attempting to upload it to the device. Just because they appear
> > > in dmesg does *not* imply the device is functional beyond the handful of
> > > register reads in pvr_load_gpu_id().
> > > 
> > > Since Mesa does not yet have support for this GPU, there’s not a lot
> > > that can be done to actually test these bindings.
> > > 
> > > When we added upstream support for the first GPU (the AXE core in TI’s
> > > AM62), we opted to wait until userspace was sufficiently progressed to
> > > the point it could be used for testing. This thought process still
> > > applies when adding new GPUs.
> > > 
> > > Our main concern is that adding bindings for GPUs implies a level of
> > > support that cannot be tested. That in turn may make it challenging to
> > > justify UAPI changes if/when they’re needed to actually make these GPUs
> > > functional.
> > 
> > I wrongly assumed that when the firmware was ready, there was some
> > preliminary functionality, but it sounds like we need to work for
> > Series6XT support to be added to the driver.  I only used the AXE
> > compatible since it appeared to the be the only one and the existing
> > binding document stated "model/revision is fully discoverable" which I
> > interpreted to mean that the AXE compatible was sufficient.
> 
> The comment is related to there being a few models/revisions of AXE [1][2][3],
> which we can distinguish between by reading a register.
> 
> > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > index a8a44fe5e83b..8923d9624b39 100644
> > > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > > >                       resets = <&cpg 408>;
> > > >               };
> > > > 
> > > > +             gpu: gpu@fd000000 {
> > > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> > > 
> > > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > > with one. For prior art, see [1] where we added support for the MT8173
> > > found in Elm Chromebooks R13 (also a Series6XT GPU).
> > > 
> > > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > > +                     clocks = <&cpg CPG_MOD 112>;
> > > > +                     clock-names = "core";
> > > 
> > > Series6XT cores have three clocks (see [1] again). I don’t have a
> > > Renesas TRM to hand – do you know if their docs go into detail on the
> > > GPU integration?
> > > 
> > > > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > > > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > > > +                     resets = <&cpg 112>;
> > > > +             };
> > > > +
> > > >               pciec0: pcie@fe000000 {
> > > >                       compatible = "renesas,pcie-r8a774a1",
> > > >                                    "renesas,pcie-rcar-gen3";
> > > 
> > > As you probably expect by this point, I have to nack this series for
> > > now. I appreciate your effort here and I’ll be happy to help you land
> > 
> > I get that.  I wasn't sure if I should have even pushed this, but I
> > wanted to get a little traction, because I know there are people like
> > myself who want to use the 3D in the Renesas boards, but don't want to
> > use the closed-source blobs tied to EULA and NDA documents.
> > 
> > > these once Mesa gains some form of usable support to allow testing.
> > 
> > Is there a way for your group to add me to the CC list when future
> > updates are submitted?  I'd like to follow this and resubmit when it's
> > ready.
> 
> Sure, we'll keep you updated as things progress.
> 

Oh, I forgot to add, in the meantime, would you find it useful for us to create
a Series6XT branch on GitLab where we can include these patches? We can create a
corresponding Mesa branch that we'll update as we progress support for GX6250.
This should make it easier for you (and others) to test and hopefully make it
easier for others to contribute while we work to get support into a good state.

> Thanks
> Frank
> 
> [1] https://www.imaginationtech.com/product/img-axe-1-16m/
> [2] https://www.imaginationtech.com/product/img-axe-1-16/
> [3] https://www.imaginationtech.com/product/img-axe-2-16/
> 
> > > Cheers,
> > > Matt
> > > 
> > > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
  2024-02-27  7:48   ` Geert Uytterhoeven
@ 2024-03-07 12:40   ` Frank Binns
  2024-03-07 14:11     ` Adam Ford
  1 sibling, 1 reply; 28+ messages in thread
From: Frank Binns @ 2024-03-07 12:40 UTC (permalink / raw)
  To: aford173, dri-devel, linux-renesas-soc
  Cc: magnus.damm, tzimmermann, Matt Coster, devicetree, robh, airlied,
	linux-kernel, geert+renesas, maarten.lankhorst,
	krzysztof.kozlowski+dt, daniel, aford, conor+dt, mripard

Hi Adam,

On Mon, 2024-02-26 at 21:45 -0600, Adam Ford wrote:
> Update the binding to add support for various Renesas SoC's with PowerVR
> Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> the table to indicate such like what was done for the ti,am62-gpu.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> index 256e252f8087..7c75104df09f 100644
> --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> @@ -14,6 +14,11 @@ properties:
>    compatible:
>      items:
>        - enum:
> +          - renesas,r8a774a1-gpu
> +          - renesas,r8a774e1-gpu
> +          - renesas,r8a77951-gpu
> +          - renesas,r8a77960-gpu
> +          - renesas,r8a77961-gpu
>            - ti,am62-gpu
>        - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable

A new set of items should be added for 'img,powervr-series6xt' and the Renesas
models along the lines of [1].

Thanks
Frank

[1] 
https://gitlab.freedesktop.org/imagination/linux/-/blob/powervr-next/Documentation/devicetree/bindings/gpu/img,powervr.yaml?ref_type=heads#L16-19

>  
> @@ -51,7 +56,13 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: ti,am62-gpu
> +            enum:
> +              - ti,am62-gpu
> +              - renesas,r8a774a1-gpu
> +              - renesas,r8a774e1-gpu
> +              - renesas,r8a77951-gpu
> +              - renesas,r8a77960-gpu
> +              - renesas,r8a77961-gpu
>      then:
>        properties:
>          clocks:

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-03-07 12:37         ` Frank Binns
@ 2024-03-07 13:31           ` Adam Ford
  2024-03-11  9:03             ` Frank Binns
  0 siblings, 1 reply; 28+ messages in thread
From: Adam Ford @ 2024-03-07 13:31 UTC (permalink / raw)
  To: Frank Binns
  Cc: Matt Coster, maarten.lankhorst, daniel, tzimmermann, mripard,
	dri-devel, robh, linux-renesas-soc, airlied, geert+renesas,
	magnus.damm, devicetree, krzysztof.kozlowski+dt, linux-kernel,
	conor+dt, aford

On Thu, Mar 7, 2024 at 6:37 AM Frank Binns <Frank.Binns@imgtec.com> wrote:
>
> On Thu, 2024-03-07 at 12:26 +0000, Frank Binns wrote:
> > On Tue, 2024-02-27 at 05:50 -0600, Adam Ford wrote:
> > > On Tue, Feb 27, 2024 at 3:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> > > > Hi Adam,
> > > >
> > > > Thanks for these patches! I'll just reply to this one patch, but my
> > > > comments apply to them all.
> > > >
> > > > On 27/02/2024 03:45, Adam Ford wrote:
> > > > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > > > rogue_4.45.2.58_v1.fw available from Imagination.
> > > > >
> > > > > When enumerated, it appears as:
> > > > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > > > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> > > >
> > > > These messages are printed after verifying the firmware blob’s headers,
> > > > *before* attempting to upload it to the device. Just because they appear
> > > > in dmesg does *not* imply the device is functional beyond the handful of
> > > > register reads in pvr_load_gpu_id().
> > > >
> > > > Since Mesa does not yet have support for this GPU, there’s not a lot
> > > > that can be done to actually test these bindings.
> > > >
> > > > When we added upstream support for the first GPU (the AXE core in TI’s
> > > > AM62), we opted to wait until userspace was sufficiently progressed to
> > > > the point it could be used for testing. This thought process still
> > > > applies when adding new GPUs.
> > > >
> > > > Our main concern is that adding bindings for GPUs implies a level of
> > > > support that cannot be tested. That in turn may make it challenging to
> > > > justify UAPI changes if/when they’re needed to actually make these GPUs
> > > > functional.
> > >
> > > I wrongly assumed that when the firmware was ready, there was some
> > > preliminary functionality, but it sounds like we need to work for
> > > Series6XT support to be added to the driver.  I only used the AXE
> > > compatible since it appeared to the be the only one and the existing
> > > binding document stated "model/revision is fully discoverable" which I
> > > interpreted to mean that the AXE compatible was sufficient.
> >
> > The comment is related to there being a few models/revisions of AXE [1][2][3],
> > which we can distinguish between by reading a register.
> >
> > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > index a8a44fe5e83b..8923d9624b39 100644
> > > > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > > > >                       resets = <&cpg 408>;
> > > > >               };
> > > > >
> > > > > +             gpu: gpu@fd000000 {
> > > > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> > > >
> > > > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > > > with one. For prior art, see [1] where we added support for the MT8173
> > > > found in Elm Chromebooks R13 (also a Series6XT GPU).
> > > >
> > > > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > > > +                     clocks = <&cpg CPG_MOD 112>;
> > > > > +                     clock-names = "core";
> > > >
> > > > Series6XT cores have three clocks (see [1] again). I don’t have a
> > > > Renesas TRM to hand – do you know if their docs go into detail on the
> > > > GPU integration?
> > > >
> > > > > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > > > > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > > > > +                     resets = <&cpg 112>;
> > > > > +             };
> > > > > +
> > > > >               pciec0: pcie@fe000000 {
> > > > >                       compatible = "renesas,pcie-r8a774a1",
> > > > >                                    "renesas,pcie-rcar-gen3";
> > > >
> > > > As you probably expect by this point, I have to nack this series for
> > > > now. I appreciate your effort here and I’ll be happy to help you land
> > >
> > > I get that.  I wasn't sure if I should have even pushed this, but I
> > > wanted to get a little traction, because I know there are people like
> > > myself who want to use the 3D in the Renesas boards, but don't want to
> > > use the closed-source blobs tied to EULA and NDA documents.
> > >
> > > > these once Mesa gains some form of usable support to allow testing.
> > >
> > > Is there a way for your group to add me to the CC list when future
> > > updates are submitted?  I'd like to follow this and resubmit when it's
> > > ready.
> >
> > Sure, we'll keep you updated as things progress.
> >
>
> Oh, I forgot to add, in the meantime, would you find it useful for us to create
> a Series6XT branch on GitLab where we can include these patches? We can create a
> corresponding Mesa branch that we'll update as we progress support for GX6250.
> This should make it easier for you (and others) to test and hopefully make it
> easier for others to contribute while we work to get support into a good state.

That works for me.  Do you want me to make any changes to the series?
I know there was some discussion about the number of clocks for the
Renesas variants.

adam
>
> > Thanks
> > Frank
> >
> > [1] https://www.imaginationtech.com/product/img-axe-1-16m/
> > [2] https://www.imaginationtech.com/product/img-axe-1-16/
> > [3] https://www.imaginationtech.com/product/img-axe-2-16/
> >
> > > > Cheers,
> > > > Matt
> > > >
> > > > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs
  2024-03-07 12:40   ` Frank Binns
@ 2024-03-07 14:11     ` Adam Ford
  0 siblings, 0 replies; 28+ messages in thread
From: Adam Ford @ 2024-03-07 14:11 UTC (permalink / raw)
  To: Frank Binns
  Cc: dri-devel, linux-renesas-soc, magnus.damm, tzimmermann,
	Matt Coster, devicetree, robh, airlied, linux-kernel,
	geert+renesas, maarten.lankhorst, krzysztof.kozlowski+dt, daniel,
	aford, conor+dt, mripard

On Thu, Mar 7, 2024 at 6:41 AM Frank Binns <Frank.Binns@imgtec.com> wrote:
>
> Hi Adam,
>
> On Mon, 2024-02-26 at 21:45 -0600, Adam Ford wrote:
> > Update the binding to add support for various Renesas SoC's with PowerVR
> > Rogue GX6250 and GX6650 GPUs.  These devices only need one clock, so update
> > the table to indicate such like what was done for the ti,am62-gpu.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > index 256e252f8087..7c75104df09f 100644
> > --- a/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > +++ b/Documentation/devicetree/bindings/gpu/img,powervr-rogue.yaml
> > @@ -14,6 +14,11 @@ properties:
> >    compatible:
> >      items:
> >        - enum:
> > +          - renesas,r8a774a1-gpu
> > +          - renesas,r8a774e1-gpu
> > +          - renesas,r8a77951-gpu
> > +          - renesas,r8a77960-gpu
> > +          - renesas,r8a77961-gpu
> >            - ti,am62-gpu
> >        - const: img,img-axe # IMG AXE GPU model/revision is fully discoverable
>
> A new set of items should be added for 'img,powervr-series6xt' and the Renesas
> models along the lines of [1].

Should I rebase my binding off the one below, so it applies to your
branch or should I attempt to base it off the mainline?
>
> Thanks
> Frank
>
> [1]
> https://gitlab.freedesktop.org/imagination/linux/-/blob/powervr-next/Documentation/devicetree/bindings/gpu/img,powervr.yaml?ref_type=heads#L16-19
>
> >
> > @@ -51,7 +56,13 @@ allOf:
> >        properties:
> >          compatible:
> >            contains:
> > -            const: ti,am62-gpu
> > +            enum:
> > +              - ti,am62-gpu
> > +              - renesas,r8a774a1-gpu
> > +              - renesas,r8a774e1-gpu
> > +              - renesas,r8a77951-gpu
> > +              - renesas,r8a77960-gpu
> > +              - renesas,r8a77961-gpu
> >      then:
> >        properties:
> >          clocks:

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU
  2024-03-07 13:31           ` Adam Ford
@ 2024-03-11  9:03             ` Frank Binns
  0 siblings, 0 replies; 28+ messages in thread
From: Frank Binns @ 2024-03-11  9:03 UTC (permalink / raw)
  To: aford173
  Cc: aford, robh, krzysztof.kozlowski+dt, tzimmermann, mripard,
	daniel, linux-kernel, Matt Coster, maarten.lankhorst,
	linux-renesas-soc, dri-devel, devicetree, geert+renesas,
	conor+dt, magnus.damm, airlied

On Thu, 2024-03-07 at 07:31 -0600, Adam Ford wrote:
> On Thu, Mar 7, 2024 at 6:37 AM Frank Binns <Frank.Binns@imgtec.com> wrote:
> > On Thu, 2024-03-07 at 12:26 +0000, Frank Binns wrote:
> > > On Tue, 2024-02-27 at 05:50 -0600, Adam Ford wrote:
> > > > On Tue, Feb 27, 2024 at 3:31 AM Matt Coster <Matt.Coster@imgtec.com> wrote:
> > > > > Hi Adam,
> > > > > 
> > > > > Thanks for these patches! I'll just reply to this one patch, but my
> > > > > comments apply to them all.
> > > > > 
> > > > > On 27/02/2024 03:45, Adam Ford wrote:
> > > > > > The GPU on the RZ/G2M is a Rogue GX6250 which uses firmware
> > > > > > rogue_4.45.2.58_v1.fw available from Imagination.
> > > > > > 
> > > > > > When enumerated, it appears as:
> > > > > >   powervr fd000000.gpu: [drm] loaded firmware powervr/rogue_4.45.2.58_v1.fw
> > > > > >   powervr fd000000.gpu: [drm] FW version v1.0 (build 6513336 OS)
> > > > > 
> > > > > These messages are printed after verifying the firmware blob’s headers,
> > > > > *before* attempting to upload it to the device. Just because they appear
> > > > > in dmesg does *not* imply the device is functional beyond the handful of
> > > > > register reads in pvr_load_gpu_id().
> > > > > 
> > > > > Since Mesa does not yet have support for this GPU, there’s not a lot
> > > > > that can be done to actually test these bindings.
> > > > > 
> > > > > When we added upstream support for the first GPU (the AXE core in TI’s
> > > > > AM62), we opted to wait until userspace was sufficiently progressed to
> > > > > the point it could be used for testing. This thought process still
> > > > > applies when adding new GPUs.
> > > > > 
> > > > > Our main concern is that adding bindings for GPUs implies a level of
> > > > > support that cannot be tested. That in turn may make it challenging to
> > > > > justify UAPI changes if/when they’re needed to actually make these GPUs
> > > > > functional.
> > > > 
> > > > I wrongly assumed that when the firmware was ready, there was some
> > > > preliminary functionality, but it sounds like we need to work for
> > > > Series6XT support to be added to the driver.  I only used the AXE
> > > > compatible since it appeared to the be the only one and the existing
> > > > binding document stated "model/revision is fully discoverable" which I
> > > > interpreted to mean that the AXE compatible was sufficient.
> > > 
> > > The comment is related to there being a few models/revisions of AXE [1][2][3],
> > > which we can distinguish between by reading a register.
> > > 
> > > > > > Signed-off-by: Adam Ford <aford173@gmail.com>
> > > > > > 
> > > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > > index a8a44fe5e83b..8923d9624b39 100644
> > > > > > --- a/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/renesas/r8a774a1.dtsi
> > > > > > @@ -2352,6 +2352,16 @@ gic: interrupt-controller@f1010000 {
> > > > > >                       resets = <&cpg 408>;
> > > > > >               };
> > > > > > 
> > > > > > +             gpu: gpu@fd000000 {
> > > > > > +                     compatible = "renesas,r8a774a1-gpu", "img,img-axe";
> > > > > 
> > > > > The GX6250 is *not* an AXE core - it shouldn’t be listed as compatible
> > > > > with one. For prior art, see [1] where we added support for the MT8173
> > > > > found in Elm Chromebooks R13 (also a Series6XT GPU).
> > > > > 
> > > > > > +                     reg = <0 0xfd000000 0 0x20000>;
> > > > > > +                     clocks = <&cpg CPG_MOD 112>;
> > > > > > +                     clock-names = "core";
> > > > > 
> > > > > Series6XT cores have three clocks (see [1] again). I don’t have a
> > > > > Renesas TRM to hand – do you know if their docs go into detail on the
> > > > > GPU integration?
> > > > > 
> > > > > > +                     interrupts = <GIC_SPI 119 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > +                     power-domains = <&sysc R8A774A1_PD_3DG_B>;
> > > > > > +                     resets = <&cpg 112>;
> > > > > > +             };
> > > > > > +
> > > > > >               pciec0: pcie@fe000000 {
> > > > > >                       compatible = "renesas,pcie-r8a774a1",
> > > > > >                                    "renesas,pcie-rcar-gen3";
> > > > > 
> > > > > As you probably expect by this point, I have to nack this series for
> > > > > now. I appreciate your effort here and I’ll be happy to help you land
> > > > 
> > > > I get that.  I wasn't sure if I should have even pushed this, but I
> > > > wanted to get a little traction, because I know there are people like
> > > > myself who want to use the 3D in the Renesas boards, but don't want to
> > > > use the closed-source blobs tied to EULA and NDA documents.
> > > > 
> > > > > these once Mesa gains some form of usable support to allow testing.
> > > > 
> > > > Is there a way for your group to add me to the CC list when future
> > > > updates are submitted?  I'd like to follow this and resubmit when it's
> > > > ready.
> > > 
> > > Sure, we'll keep you updated as things progress.
> > > 
> > 
> > Oh, I forgot to add, in the meantime, would you find it useful for us to create
> > a Series6XT branch on GitLab where we can include these patches? We can create a
> > corresponding Mesa branch that we'll update as we progress support for GX6250.
> > This should make it easier for you (and others) to test and hopefully make it
> > easier for others to contribute while we work to get support into a good state.
> 
> That works for me.  Do you want me to make any changes to the series?
> I know there was some discussion about the number of clocks for the
> Renesas variants.

I'd say leave it as is for now. We'll add a cleaned up version of our DT
bindings patch adding 'img,powervr-series6xt' to the branch. You can then rebase
your series on top of that and send out a GitLab merge request for inclusion in
the branch. Does that sound okay?

I'll let you know once we've created the kernel and Mesa branches.

Thanks
Frank

> 
> adam
> > > Thanks
> > > Frank
> > > 
> > > [1] https://www.imaginationtech.com/product/img-axe-1-16m/
> > > [2] https://www.imaginationtech.com/product/img-axe-1-16/
> > > [3] https://www.imaginationtech.com/product/img-axe-2-16/
> > > 
> > > > > Cheers,
> > > > > Matt
> > > > > 
> > > > > [1]: https://gitlab.freedesktop.org/imagination/linux/-/blob/b3506b8bc45ed6d4005eb32a994df0e33d6613f1/arch/arm64/boot/dts/mediatek/mt8173.dtsi#L993-1006

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2024-03-11  9:03 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-27  3:45 [PATCH 0/6] gpu: powervr-rogue: Add PowerVR support for some Renesas devices Adam Ford
2024-02-27  3:45 ` [PATCH 1/6] dt-bindings: gpu: powervr-rogue: Add PowerVR support for some Renesas GPUs Adam Ford
2024-02-27  7:48   ` Geert Uytterhoeven
2024-02-27  8:03     ` Geert Uytterhoeven
2024-02-27  8:09     ` Geert Uytterhoeven
2024-02-27 10:38       ` Geert Uytterhoeven
2024-03-07 12:40   ` Frank Binns
2024-03-07 14:11     ` Adam Ford
2024-02-27  3:45 ` [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU Adam Ford
2024-02-27  8:09   ` Geert Uytterhoeven
2024-02-27  9:31   ` Matt Coster
2024-02-27 11:04     ` Geert Uytterhoeven
2024-02-27 11:54       ` Adam Ford
2024-03-07 12:01       ` Frank Binns
2024-02-27 11:50     ` Adam Ford
2024-03-07 12:26       ` Frank Binns
2024-03-07 12:37         ` Frank Binns
2024-03-07 13:31           ` Adam Ford
2024-03-11  9:03             ` Frank Binns
2024-02-27  3:45 ` [PATCH 3/6] arm64: dts: renesas: r8a774e1: " Adam Ford
2024-02-27  8:10   ` Geert Uytterhoeven
2024-02-27  3:45 ` [PATCH 4/6] arm64: dts: renesas: r8a77951: " Adam Ford
2024-02-27  8:11   ` Geert Uytterhoeven
2024-02-27 15:15     ` Adam Ford
2024-02-27  3:45 ` [PATCH 5/6] arm64: dts: renesas: r8a77960: " Adam Ford
2024-02-27  8:12   ` Geert Uytterhoeven
2024-02-27  3:45 ` [PATCH 6/6] arm64: dts: renesas: r8a77961: " Adam Ford
2024-02-27  8:12   ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).