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