On Tue, Nov 23, 2021 at 8:30 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > On Tue, Nov 23, 2021 at 8:24 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote: > > > > Am Dienstag, dem 23.11.2021 um 08:08 -0600 schrieb Adam Ford: > > > On Mon, Nov 22, 2021 at 3:52 PM Tim Harvey <tharvey@xxxxxxxxxxxxx> wrote: > > > > > > > > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@xxxxxxxxxxxxxx> wrote: > > > > > > > > > > Am Montag, dem 22.11.2021 um 09:59 -0800 schrieb Tim Harvey: > > > > > > On Sun, Nov 21, 2021 at 7:25 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > > > > > > > > > > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > > > > > > > > > > > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@xxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > Hi Adam, > > > > > > > > > > > > > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > > > > > > > > > > > > > > > > > > I am using https://source.codeaurora.org/external/imx/imx-atf/log/?h=lf_v2.4 > > > > > > > > > > > > > > > > > > > > Since the driver sending SMCC commands to ATF isn't doing that, I > > > > > > > > > > assume it's safe to use the linux power-domain drivers with the ATF > > > > > > > > > > from NXP's kernel. > > > > > > > > > > > > > > > > > > > > If you can point me to the repo you think I should be using, I'll give it a try. > > > > > > > > > > > > > > > > > > Do you know if the mainline TF-A repo v2.5 works too? > > > > > > > > > https://github.com/ARM-software/arm-trusted-firmware/tree/v2.5 > > > > > > > > > > > > > > > > That's good to know. > > > > > > > > > > > > > > > > I just built it into U-Boot: > > > > > > > > > > > > > > > > NOTICE: BL31: v2.5(release):v2.5 > > > > > > > > NOTICE: BL31: Built : 08:24:13, Nov 21 2021 > > > > > > > > > > > > > > > > The Etnaviv driver is still loading without hanging > > > > > > > > > > > > > > > > root@beacon-imx8mn-kit:~# dmesg |grep -i etna > > > > > > > > [ 12.393936] etnaviv etnaviv: bound 38000000.gpu (ops gpu_ops [etnaviv]) > > > > > > > > [ 12.400676] etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6203 > > > > > > > > [ 12.641297] [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Tim, > > > > > > > > > > > > > > Which version of Nano do you have? Not all Nano SoC's have a GPU from > > > > > > > looking at the datasheet [1] . I am using MIMX8MN2CVTIZAA (Nano Solo) > > > > > > > > > > > > > > [1] - https://www.nxp.com/docs/en/data-sheet/IMX8MNIEC.pdf > > > > > > > > > > > > > > > > > > > Adam, > > > > > > > > > > > > The board I have here has MIMX8MN5CVTIZAA so i.MX 8M Nano QuadLite > > > > > > with 'No GPU' as you expected. > > > > > > > > > > > > So I have to add the following to keep my board from hanging after your series: > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn-venice-gw7902.dts > > > > > > b/arch/arm64/boot/dts/freescale/imx8mn-venice-gw7902.dts > > > > > > index 236f425e1570..0d256a607b7c 100644 > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mn-venice-gw7902.dts > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn-venice-gw7902.dts > > > > > > @@ -251,6 +251,10 @@ > > > > > > }; > > > > > > }; > > > > > > > > > > > > +&gpu { > > > > > > + status = "disabled"; > > > > > > +}; > > > > > > + > > > > > > &i2c1 { > > > > > > clock-frequency = <100000>; > > > > > > pinctrl-names = "default"; > > > > > > > > > > > > This situation is similar to the one I encountered with the > > > > > > imx8mm-venice-gw7901 where adding the GPC node caused my board (which > > > > > > did not power the GPU) to hang until I added disables to the > > > > > > device-tree with commit 7973009235e2 ("arm64: dts: > > > > > > imx8mm-venice-gw7901.dts: disable pgc_gpumix"). It feels painful to > > > > > > have to add patches to keep things from hanging after additional > > > > > > functionality is added to dt but perhaps that is more common than I > > > > > > think esp for SoC's like IMX8M which have a lot of lingering support > > > > > > still coming in. > > > > > > > > > > > Yea, it's unfortunate that those patches break your board, but I guess > > > > > we need to accept this, while there is still a lot of feature work > > > > > going on. > > > > > > There are a significant number of peripherals which are defined and > > > marked as 'disabled' by default, so I don't think it's unreasonable to > > > do that here. > > > I'd like to propose we keep the default disabled and people who > > > need/want the GPU enabled can turn it on. Why waste the power if it's > > > not needed? > > > > > Sure, if a significant number of chips has the GPU disabled, we might > > want to keep it disabled in the base dtsi. With those variants it's > > always a tradeoff, for example there are SKUs of the i.MX6 that had the > > VPU disabled, but very few of those were in the field, so the VPUs are > > enabled in the SoC base dtsi and only users of those special SKUs would > > need to disable them in the board DT. > > > > The power argument isn't valid, as the kernel driver will suspend the > > device when not needed, so there is no wasted power (aside from the > > sort moment while the driver probes) with the GPU enabled. > > > > The rule of thumb for when a device is default enabled in the SoC dsti > > has always been (at least for i.MX) that the peripheral must not have a > > board level dependency. While a i2c controller obviously needs a i2c > > bus connected on the board to fulfill its purpose, a GPU can be used as > > color space converter or something like that with no board level > > interaction. Now the line is a bit blurred by having multiple power > > rails into the SoC, so one could argue that the GPUs and VPUs now have > > some board level dependency on the i.MX8M*. > > That makes sense. > > Do we defer to Shawn as the final arbiter as to whether or not it's > enabled/disabled? It would be nice to get Nano caught up in > functionality as much as possible. We could add two more device trees, one for 8mnl (lite) and 8mnul (ulta-lite) imx8mnl: #include imx8mn.dtsi &gpu { status = "disabled"; }; imx8mnul: #include imx8mnl &dsi { status = "disabled"; }; Then the boards using either lite or ultralite just include their respective SoC.dtsi instead of imx8mn.dtsi. This is similar to what we do with the plethora of i.mx6 options. Just a thought. Although, I really like the idea of the bootloader disabling the unavailable nodes. adam > > adam > > > > > Regards, > > Lucas > >