On Tue, Nov 23, 2021 at 8:08 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > 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? > > > > > > > > I don't mind at all submitting the above patch to fix my board after > > > > your series is accepted as I think that having an IMX8MN with 'no gpu' > > > > is perhaps less likely than having one with a GPU and thus we probably > > > > shouldn't mark the node as disabled and force everyone that has a GPU > > > > to go and enable it. > > > > > > > > I wonder however if we should think about adding something to etnaviv > > > > to check the capability so that the same dt could be used with both > > > > CPU variants? > > > > > > etnaviv or really the kernel at all is not the place to handle this. > > > The DT is supposed to describe the hardware and the kernel should be > > > able to trust this description. > > > > > > If there is some way to read the chip capabilities and avoid having too > > > much DT variants in the kernel, the right place to handle this is the > > > software running before the kernel is started, i.e. your bootloader. > > > Barebox for example reads the SCU fuses on i.MX6 and removes the DT > > > nodes for the fused off CPU cores on i.MX6S and i.MX6D. > > > > > > > Lucas, > > > > I agree - the boot firmware is an appropriate place for this. I > > believe the correct course of action in the case of the IMX8M Nano > > would be to do the following for no GPU: > > - disable disp_blk_ctrl node > > I don't think it's necessary to remove the disp_blk_ctrl node or > change it. The GPU doesn't directly interact with it. LCD, CSI, and > DSI do, but I don't think they are removed. The gpu only interacts > with the pgc_gpumix and neither the gpu nor gpumix currently interact > with the disp_blk_ctrl. > I went back and reviewed the datasheet, and the UltraLite models remove the DSI, but I think it's still safe to leave the disp_blk_ctrl untouched, because FWICT it's only activated from the corresponding power-domain is requested, and without a MIPI_DSI the DSI power-domain won't be requested. The CSI appears to remain intact on the Lite and UltraLite versions, so we still need some form of the disp_blk_ctrl to manage it. > adam > > > > > - disable gpu node > > - disable pgc_gpumix node > > > > What would you propose to do for detection of this in boot firmware? > > The DIGPROG register is currently used in U-Boot to determine IMX8M > > variants including Nano > > Qad/Dual/Solo/QaudLite/DualLite/SoloLite/UltraLite Quad/UltraLite > > Dual/UltraLite Solo. It would appear all the 'Lite' and 'UltraLite' > > variants have no GPU. > > > > Best regards, > > > > Tim