Re: [PATCH 2/6] arm64: dts: renesas: r8a774a1: Enable GPU

[Matt Coster <Matt.Coster@imgtec.com>]

[-- 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 --]