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@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 ?
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! /
---------------
¯\_(ツ)_/¯
