Re: [PATCH V3 0/9] arm64: imx8mn: Enable more imx8m Nano functions

On Tue, Nov 23, 2021 at 8:08 AM Adam Ford [off-list ref] wrote:

On Mon, Nov 22, 2021 at 3:52 PM Tim Harvey [off-list ref] wrote:

quoted

On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach [off-list ref] wrote:

quoted

Am Montag, dem 22.11.2021 um 09:59 -0800 schrieb Tim Harvey:

quoted

On Sun, Nov 21, 2021 at 7:25 AM Adam Ford [off-list ref] wrote:

quoted

On Sun, Nov 21, 2021 at 8:34 AM Adam Ford [off-list ref] wrote:

quoted

On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam [off-list ref] wrote:

quoted

Hi Adam,

On Sun, Nov 21, 2021 at 11:17 AM Adam Ford [off-list ref] wrote:

quoted

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?

quoted

quoted

quoted

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

quoted

- 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel