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

[Liviu Dudau]

On Fri, Feb 28, 2025 at 06:43:53PM +0100, Marek Vasut wrote:
> 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 at 0,0 {
> > >   			};
> > >   		};
> > > +		gpu_blk_ctrl: reset-controller at 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 at 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 ?

The 'coregroup' clock does indeed control the MMU and L2$ blocks as well as the AXI interface,
so if that is indeed a separate external clock source it should be defined. Could it be why
you're seeing issues with L2$ resume on the fast reset path?

Best regards,
Liviu

> 
> > > +			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.

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯