Re: [PATCH 9/9] arm64: dts: imx95: Describe Mali G310 GPU

[Marek Vasut]

On 2/28/25 11:36 AM, Alexander Stein wrote:
Hi Marek,

Hi,

diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi 
b/arch/arm64/boot/dts/freescale/imx95.dtsi
index 3af13173de4bd..36bad211e5558 100644
--- a/arch/arm64/boot/dts/freescale/imx95.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
@@ -249,6 +249,37 @@ dummy: clock-dummy {
                clock-output-names = "dummy";
        };
  
+	gpu_fixed_reg: fixed-gpu-reg {
+               compatible = "regulator-fixed";
+               regulator-min-microvolt = <920000>;
+               regulator-max-microvolt = <920000>;
+               regulator-name = "vdd_gpu";
+               regulator-always-on;
+               regulator-boot-on;
+       };

Is this an internal voltage?

I think so.

+
+       gpu_opp_table: opp_table {

Node-Names use dash instead of underscore.

Fixed, thanks.

[...]

@@ -1846,6 +1877,37 @@ netc_emdio: mdio@0,0 {
                        };
                };
  
+		gpu_blk_ctrl: reset-controller@4d810000 {
+                       compatible = "fsl,imx95-gpu-blk-ctrl";
+                       reg = <0x0 0x4d810000 0x0 0xc>;

Mh, GPU_BLK_CTRL is /just a bit) more than the GPU reset. Does it make sense
to make this an gpu-reset-only node, located at 0x4d810008?

The block controller itself is larger, it spans 3 or 4 registers, so 
this should describe the entire block controller here.

+                       #reset-cells = <1>;
+                       clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
+                       assigned-clocks = <&scmi_clk IMX95_CLK_GPUAPB>;
+                       assigned-clock-parents = <&scmi_clk 
IMX95_CLK_SYSPLL1_PFD1_DIV2>;
+                       assigned-clock-rates = <133333333>;
+                       power-domains = <&scmi_devpd IMX95_PD_GPU>;
+                       status = "disabled";
+               };
+
+               gpu: gpu@4d900000 {
+                       compatible = "fsl,imx95-mali", "arm,mali-valhall-csf";
+                       reg = <0 0x4d900000 0 0x480000>;
+                       clocks = <&scmi_clk IMX95_CLK_GPU>;

There is also IMX95_CLK_GPUAPB. Is this only required for the rese control 
above?

I think I have to describe those clock here too, possibly as 'coregroup' 
clock ?

+                       clock-names = "core";
+                       interrupts = <GIC_SPI 288 IRQ_TYPE_LEVEL_HIGH>,
+                                    <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+                                    <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>;
+                       interrupt-names = "gpu", "job", "mmu";

DT bindings say this order: job, mmu, gpu
Yes, currently it is sorted by IRQ number, fixed.