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

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

[Adam Ford]

The i.MX8M Nano is similar to the i.MX8M Mini in some ways, but very 
different in others.  With the blk-ctrl driver for Mini in place, 
this series expands the blk-ctrl driver to support the Nano which
opens the door for additional functions in the future.  As part of
this series, it also addresses some issues in the GPCv2 driver and
finally adds support for enabling USB and GPU.

V3:  Fixes an the yaml example
V2:  Fixes the clock count in the blk-ctrl

Adam Ford (9):
  soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled
  soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn
  dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains
  dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl
  soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl
  arm64: dts: imx8mn: add GPC node
  arm64: dts: imx8mn: put USB controller into power-domains
  arm64: dts: imx8mn: add DISP blk-ctrl
  arm64: dts: imx8mn: Enable GPU

 .../soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml     |  97 +++++++++++++++++
 arch/arm64/boot/dts/freescale/imx8mn.dtsi     | 103 ++++++++++++++++++
 drivers/soc/imx/gpcv2.c                       |  26 +++++
 drivers/soc/imx/imx8m-blk-ctrl.c              |  75 ++++++++++++-
 include/dt-bindings/power/imx8mn-power.h      |   5 +
 5 files changed, 305 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml

-- 
2.32.0

[PATCH V3 1/9] soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled

[Adam Ford]

Like the i.MX8MM, keep the gpumix clocks running when the
domain is active.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index b8d52d8d29db..95f05575f843 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -840,6 +840,7 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 			.hskack = IMX8MN_GPUMIX_HSK_PWRDNACKN,
 		},
 		.pgc   = BIT(IMX8MN_PGC_GPUMIX),
+		.keep_clocks = true,
 	},
 };
 
-- 
2.32.0

[PATCH V3 2/9] soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn

[Adam Ford]

The dispmix will be needed for the blkctl driver, so add it
to the gpcv2.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 95f05575f843..7b568cf39a5a 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -842,6 +842,31 @@ static const struct imx_pgc_domain imx8mn_pgc_domains[] = {
 		.pgc   = BIT(IMX8MN_PGC_GPUMIX),
 		.keep_clocks = true,
 	},
+
+	[IMX8MN_POWER_DOMAIN_DISPMIX] = {
+		.genpd = {
+			.name = "dispmix",
+		},
+			.bits  = {
+			.pxx = IMX8MN_DISPMIX_SW_Pxx_REQ,
+			.map = IMX8MN_DISPMIX_A53_DOMAIN,
+			.hskreq = IMX8MN_DISPMIX_HSK_PWRDNREQN,
+			.hskack = IMX8MN_DISPMIX_HSK_PWRDNACKN,
+		},
+		.pgc   = BIT(IMX8MN_PGC_DISPMIX),
+		.keep_clocks = true,
+	},
+
+	[IMX8MN_POWER_DOMAIN_MIPI] = {
+		.genpd = {
+			.name = "mipi",
+		},
+			.bits  = {
+			.pxx = IMX8MN_MIPI_SW_Pxx_REQ,
+			.map = IMX8MN_MIPI_A53_DOMAIN,
+		},
+		.pgc   = BIT(IMX8MN_PGC_MIPI),
+	},
 };
 
 static const struct regmap_range imx8mn_yes_ranges[] = {
-- 
2.32.0

[PATCH V3 3/9] dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains

[Adam Ford]

This adds the defines for the power domains provided by the DISP
blk-ctrl.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/include/dt-bindings/power/imx8mn-power.h b/include/dt-bindings/power/imx8mn-power.h
index 102ee85a9b62..eedd0e581939 100644
--- a/include/dt-bindings/power/imx8mn-power.h
+++ b/include/dt-bindings/power/imx8mn-power.h
@@ -12,4 +12,9 @@
 #define IMX8MN_POWER_DOMAIN_DISPMIX	3
 #define IMX8MN_POWER_DOMAIN_MIPI	4
 
+#define IMX8MN_DISPBLK_PD_MIPI_DSI	0
+#define IMX8MN_DISPBLK_PD_MIPI_CSI	1
+#define IMX8MN_DISPBLK_PD_LCDIF	2
+#define IMX8MN_DISPBLK_PD_ISI	3
+
 #endif
-- 
2.32.0

Re: [PATCH V3 3/9] dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains

[Rob Herring]

On Thu, 04 Nov 2021 11:17:58 -0500, Adam Ford wrote:
> This adds the defines for the power domains provided by the DISP
> blk-ctrl.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH V3 4/9] dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl

[Adam Ford]

Add the DT binding for the i.MX8MN DISP blk-ctrl.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
new file mode 100644
index 000000000000..fbeaac399c50
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8MN DISP blk-ctrl
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description:
+  The i.MX8MN DISP blk-ctrl is a top-level peripheral providing access to
+  the NoC and ensuring proper power sequencing of the display and MIPI CSI
+  peripherals located in the DISP domain of the SoC.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx8mn-disp-blk-ctrl
+      - const: syscon
+
+  reg:
+    maxItems: 1
+
+  '#power-domain-cells':
+    const: 1
+
+  power-domains:
+    minItems: 5
+    maxItems: 5
+
+  power-domain-names:
+    items:
+      - const: bus
+      - const: isi
+      - const: lcdif
+      - const: mipi-dsi
+      - const: mipi-csi
+
+  clocks:
+    minItems: 11
+    maxItems: 11
+
+  clock-names:
+    items:
+      - const: disp_axi
+      - const: disp_apb
+      - const: disp_axi_root
+      - const: disp_apb_root
+      - const: lcdif-axi
+      - const: lcdif-apb
+      - const: lcdif-pix
+      - const: dsi-pclk
+      - const: dsi-ref
+      - const: csi-aclk
+      - const: csi-pclk
+
+required:
+  - compatible
+  - reg
+  - power-domains
+  - power-domain-names
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mn-clock.h>
+    #include <dt-bindings/power/imx8mn-power.h>
+
+    disp_blk_ctl: blk_ctrl@32e28000 {
+      compatible = "fsl,imx8mn-disp-blk-ctrl", "syscon";
+      reg = <0x32e28000 0x100>;
+      power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
+                      <&pgc_dispmix>, <&pgc_mipi>,
+                      <&pgc_mipi>;
+      power-domain-names = "bus", "isi", "lcdif", "mipi-dsi",
+                           "mipi-csi";
+      clocks = <&clk IMX8MN_CLK_DISP_AXI>,
+               <&clk IMX8MN_CLK_DISP_APB>,
+               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
+               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
+               <&clk IMX8MN_CLK_DISP_PIXEL_ROOT>,
+               <&clk IMX8MN_CLK_DSI_CORE>,
+               <&clk IMX8MN_CLK_DSI_PHY_REF>,
+               <&clk IMX8MN_CLK_CSI1_PHY_REF>,
+               <&clk IMX8MN_CLK_CAMERA_PIXEL_ROOT>;
+       clock-names = "disp_axi", "disp_apb", "disp_axi_root", "disp_apb_root",
+                     "lcdif-axi", "lcdif-apb", "lcdif-pix", "dsi-pclk",
+                     "dsi-ref", "csi-aclk", "csi-pclk";
+       #power-domain-cells = <1>;
+    };
-- 
2.32.0

Re: [PATCH V3 4/9] dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl

[Rob Herring]

On Thu, Nov 04, 2021 at 11:17:59AM -0500, Adam Ford wrote:
> Add the DT binding for the i.MX8MN DISP blk-ctrl.
> 
> Signed-off-by: Adam Ford <aford173@gmail.com>
> 
> diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> new file mode 100644
> index 000000000000..fbeaac399c50
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8MN DISP blk-ctrl
> +
> +maintainers:
> +  - Lucas Stach <l.stach@pengutronix.de>
> +
> +description:
> +  The i.MX8MN DISP blk-ctrl is a top-level peripheral providing access to
> +  the NoC and ensuring proper power sequencing of the display and MIPI CSI
> +  peripherals located in the DISP domain of the SoC.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: fsl,imx8mn-disp-blk-ctrl
> +      - const: syscon

Are there other functions in this block? If so, what?

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#power-domain-cells':
> +    const: 1
> +
> +  power-domains:
> +    minItems: 5
> +    maxItems: 5
> +
> +  power-domain-names:
> +    items:
> +      - const: bus
> +      - const: isi
> +      - const: lcdif
> +      - const: mipi-dsi
> +      - const: mipi-csi
> +
> +  clocks:
> +    minItems: 11
> +    maxItems: 11
> +
> +  clock-names:
> +    items:
> +      - const: disp_axi
> +      - const: disp_apb
> +      - const: disp_axi_root
> +      - const: disp_apb_root
> +      - const: lcdif-axi
> +      - const: lcdif-apb
> +      - const: lcdif-pix
> +      - const: dsi-pclk
> +      - const: dsi-ref
> +      - const: csi-aclk
> +      - const: csi-pclk
> +
> +required:
> +  - compatible
> +  - reg
> +  - power-domains
> +  - power-domain-names
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mn-clock.h>
> +    #include <dt-bindings/power/imx8mn-power.h>
> +
> +    disp_blk_ctl: blk_ctrl@32e28000 {
> +      compatible = "fsl,imx8mn-disp-blk-ctrl", "syscon";
> +      reg = <0x32e28000 0x100>;
> +      power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
> +                      <&pgc_dispmix>, <&pgc_mipi>,
> +                      <&pgc_mipi>;
> +      power-domain-names = "bus", "isi", "lcdif", "mipi-dsi",
> +                           "mipi-csi";

This looks odd. These are the same power domains this node provides.

> +      clocks = <&clk IMX8MN_CLK_DISP_AXI>,
> +               <&clk IMX8MN_CLK_DISP_APB>,
> +               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
> +               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
> +               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
> +               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
> +               <&clk IMX8MN_CLK_DISP_PIXEL_ROOT>,
> +               <&clk IMX8MN_CLK_DSI_CORE>,
> +               <&clk IMX8MN_CLK_DSI_PHY_REF>,
> +               <&clk IMX8MN_CLK_CSI1_PHY_REF>,
> +               <&clk IMX8MN_CLK_CAMERA_PIXEL_ROOT>;
> +       clock-names = "disp_axi", "disp_apb", "disp_axi_root", "disp_apb_root",
> +                     "lcdif-axi", "lcdif-apb", "lcdif-pix", "dsi-pclk",
> +                     "dsi-ref", "csi-aclk", "csi-pclk";
> +       #power-domain-cells = <1>;
> +    };
> -- 
> 2.32.0
> 
> 

Re: [PATCH V3 4/9] dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl

[Adam Ford]

On Fri, Nov 12, 2021 at 5:02 PM Rob Herring <robh@kernel.org> wrote:
>
> On Thu, Nov 04, 2021 at 11:17:59AM -0500, Adam Ford wrote:
> > Add the DT binding for the i.MX8MN DISP blk-ctrl.
> >
> > Signed-off-by: Adam Ford <aford173@gmail.com>
> >
> > diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> > new file mode 100644
> > index 000000000000..fbeaac399c50
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> > @@ -0,0 +1,97 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NXP i.MX8MN DISP blk-ctrl
> > +
> > +maintainers:
> > +  - Lucas Stach <l.stach@pengutronix.de>
> > +
> > +description:
> > +  The i.MX8MN DISP blk-ctrl is a top-level peripheral providing access to
> > +  the NoC and ensuring proper power sequencing of the display and MIPI CSI
> > +  peripherals located in the DISP domain of the SoC.
> > +
> > +properties:
> > +  compatible:
> > +    items:
> > +      - const: fsl,imx8mn-disp-blk-ctrl
> > +      - const: syscon
>
> Are there other functions in this block? If so, what?

This is similar to the i.MX8M Mini.  From what I can tell, there are
some extra clock and reset registers that are not associated with
their respective blocks. The main power domain controller called GPCv2
partially enables the power domains, but there is some ping-pong
between that IP block and this one,

>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  '#power-domain-cells':
> > +    const: 1
> > +
> > +  power-domains:
> > +    minItems: 5
> > +    maxItems: 5
> > +
> > +  power-domain-names:
> > +    items:
> > +      - const: bus
> > +      - const: isi
> > +      - const: lcdif
> > +      - const: mipi-dsi
> > +      - const: mipi-csi
> > +
> > +  clocks:
> > +    minItems: 11
> > +    maxItems: 11
> > +
> > +  clock-names:
> > +    items:
> > +      - const: disp_axi
> > +      - const: disp_apb
> > +      - const: disp_axi_root
> > +      - const: disp_apb_root
> > +      - const: lcdif-axi
> > +      - const: lcdif-apb
> > +      - const: lcdif-pix
> > +      - const: dsi-pclk
> > +      - const: dsi-ref
> > +      - const: csi-aclk
> > +      - const: csi-pclk
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - power-domains
> > +  - power-domain-names
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/clock/imx8mn-clock.h>
> > +    #include <dt-bindings/power/imx8mn-power.h>
> > +
> > +    disp_blk_ctl: blk_ctrl@32e28000 {
> > +      compatible = "fsl,imx8mn-disp-blk-ctrl", "syscon";
> > +      reg = <0x32e28000 0x100>;
> > +      power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
> > +                      <&pgc_dispmix>, <&pgc_mipi>,
> > +                      <&pgc_mipi>;
> > +      power-domain-names = "bus", "isi", "lcdif", "mipi-dsi",
> > +                           "mipi-csi";
>
> This looks odd. These are the same power domains this node provides.

It is odd, but  I'll try to explain it the best I can.

When this SoC was developed, there were a few additional registers
placed into this IP block that control other IP blocks, and ping-pong
with the main power domain controller to make the various IP blocks
work.

GPCv2, the main power domain controller attempts to enable the domain,
but it has to enable the bus clock and clear the bus reset from this
IP block because neither the reset nor the clock enable were placed
into reset or clock IP blocks.

For example,  when the mipi-csi controller needs to come up, it needs
to request the power domain from disp-blk-ctrl IP block.  The blk-ctrl
block first requests the pgc_mipi power domain.  The GPC ping-pongs
the blk-ctrl which has to set or clear registers so the GPC can finish
its job.  One the GPC has finished its part the blk-ctrl then enables
the clock and reset that controls the csi.  This same is true for
several quasi-related items like DSI, CSI, and LCDIF.

The items listed in the power-domains are from the GPC, and the items
in the power-domain-names are inside the blk-ctrl.  When an item needs
a power domain from the blk-ctrl, the corresponding GPC power domain
must also be referenced.

In order to enable some of the registers, the blk-ctrl IP block also
needs to enable some additional clocks or the system can hang.

Lucas Stach wrote the original driver and might be able to better
explain how it works on the Mini, but the Nano behaves the same way
with different bits in the regisers.


>
> > +      clocks = <&clk IMX8MN_CLK_DISP_AXI>,
> > +               <&clk IMX8MN_CLK_DISP_APB>,
> > +               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
> > +               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
> > +               <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
> > +               <&clk IMX8MN_CLK_DISP_APB_ROOT>,
> > +               <&clk IMX8MN_CLK_DISP_PIXEL_ROOT>,
> > +               <&clk IMX8MN_CLK_DSI_CORE>,
> > +               <&clk IMX8MN_CLK_DSI_PHY_REF>,
> > +               <&clk IMX8MN_CLK_CSI1_PHY_REF>,
> > +               <&clk IMX8MN_CLK_CAMERA_PIXEL_ROOT>;
> > +       clock-names = "disp_axi", "disp_apb", "disp_axi_root", "disp_apb_root",
> > +                     "lcdif-axi", "lcdif-apb", "lcdif-pix", "dsi-pclk",
> > +                     "dsi-ref", "csi-aclk", "csi-pclk";
> > +       #power-domain-cells = <1>;
> > +    };
> > --
> > 2.32.0
> >
> >

[PATCH V3 5/9] soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl

[Adam Ford]

This adds the description for the i.MX8MN disp blk-ctrl.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 519b3651d1d9..6feeff5a3776 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -14,6 +14,7 @@
 #include <linux/clk.h>
 
 #include <dt-bindings/power/imx8mm-power.h>
+#include <dt-bindings/power/imx8mn-power.h>
 
 #define BLK_SFT_RSTN	0x0
 #define BLK_CLK_EN	0x4
@@ -498,6 +499,75 @@ static const struct imx8m_blk_ctrl_data imx8mm_disp_blk_ctl_dev_data = {
 	.num_domains = ARRAY_SIZE(imx8mm_disp_blk_ctl_domain_data),
 };
 
+
+static int imx8mn_disp_power_notifier(struct notifier_block *nb,
+				      unsigned long action, void *data)
+{
+	struct imx8m_blk_ctrl *bc = container_of(nb, struct imx8m_blk_ctrl,
+						 power_nb);
+
+	if (action != GENPD_NOTIFY_ON && action != GENPD_NOTIFY_PRE_OFF)
+		return NOTIFY_OK;
+
+	/* Enable bus clock and deassert bus reset */
+	regmap_set_bits(bc->regmap, BLK_CLK_EN, BIT(8));
+	regmap_set_bits(bc->regmap, BLK_SFT_RSTN, BIT(8));
+
+	/*
+	 * On power up we have no software backchannel to the GPC to
+	 * wait for the ADB handshake to happen, so we just delay for a
+	 * bit. On power down the GPC driver waits for the handshake.
+	 */
+	if (action == GENPD_NOTIFY_ON)
+		udelay(5);
+
+
+	return NOTIFY_OK;
+}
+
+static const struct imx8m_blk_ctrl_domain_data imx8mn_disp_blk_ctl_domain_data[] = {
+	[IMX8MN_DISPBLK_PD_MIPI_DSI] = {
+		.name = "dispblk-mipi-dsi",
+		.clk_names = (const char *[]){ "dsi-pclk", "dsi-ref", },
+		.num_clks = 2,
+		.gpc_name = "mipi-dsi",
+		.rst_mask = BIT(0) | BIT(1),
+		.clk_mask = BIT(0) | BIT(1),
+	},
+	[IMX8MN_DISPBLK_PD_MIPI_CSI] = {
+		.name = "dispblk-mipi-csi",
+		.clk_names = (const char *[]){ "csi-aclk", "csi-pclk" },
+		.num_clks = 2,
+		.gpc_name = "mipi-csi",
+		.rst_mask = BIT(2) | BIT(3),
+		.clk_mask = BIT(2) | BIT(3),
+	},
+	[IMX8MN_DISPBLK_PD_LCDIF] = {
+		.name = "dispblk-lcdif",
+		.clk_names = (const char *[]){ "lcdif-axi", "lcdif-apb", "lcdif-pix", },
+		.num_clks = 3,
+		.gpc_name = "lcdif",
+		.rst_mask = BIT(4) | BIT(5),
+		.clk_mask = BIT(4) | BIT(5),
+	},
+	[IMX8MN_DISPBLK_PD_ISI] = {
+		.name = "dispblk-isi",
+		.clk_names = (const char *[]){ "disp_axi", "disp_apb", "disp_axi_root",
+						"disp_apb_root"},
+		.num_clks = 4,
+		.gpc_name = "isi",
+		.rst_mask = BIT(6) | BIT(7),
+		.clk_mask = BIT(6) | BIT(7),
+	},
+};
+
+static const struct imx8m_blk_ctrl_data imx8mn_disp_blk_ctl_dev_data = {
+	.max_reg = 0x84,
+	.power_notifier_fn = imx8mn_disp_power_notifier,
+	.domains = imx8mn_disp_blk_ctl_domain_data,
+	.num_domains = ARRAY_SIZE(imx8mn_disp_blk_ctl_domain_data),
+};
+
 static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
 	{
 		.compatible = "fsl,imx8mm-vpu-blk-ctrl",
@@ -505,7 +575,10 @@ static const struct of_device_id imx8m_blk_ctrl_of_match[] = {
 	}, {
 		.compatible = "fsl,imx8mm-disp-blk-ctrl",
 		.data = &imx8mm_disp_blk_ctl_dev_data
-	} ,{
+	}, {
+		.compatible = "fsl,imx8mn-disp-blk-ctrl",
+		.data = &imx8mn_disp_blk_ctl_dev_data
+	}, {
 		/* Sentinel */
 	}
 };
-- 
2.32.0

[PATCH V3 6/9] arm64: dts: imx8mn: add GPC node

[Adam Ford]

Add the DT node for the GPC, including all the PGC power domains,
some of them are not fully functional yet, as they require interaction
with the blk-ctrls to properly power up/down the peripherals.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index da6c942fb7f9..f5bafd9db673 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -4,6 +4,8 @@
  */
 
 #include <dt-bindings/clock/imx8mn-clock.h>
+#include <dt-bindings/power/imx8mn-power.h>
+#include <dt-bindings/reset/imx8mq-reset.h>
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -612,6 +614,53 @@ src: reset-controller@30390000 {
 				interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 				#reset-cells = <1>;
 			};
+
+			gpc: gpc@303a0000 {
+				compatible = "fsl,imx8mn-gpc";
+				reg = <0x303a0000 0x10000>;
+				interrupt-parent = <&gic>;
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+
+				pgc {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					pgc_hsiomix: power-domain@0 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MN_POWER_DOMAIN_HSIOMIX>;
+						clocks = <&clk IMX8MN_CLK_USB_BUS>;
+					};
+
+					pgc_otg1: power-domain@1 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MN_POWER_DOMAIN_OTG1>;
+						power-domains = <&pgc_hsiomix>;
+					};
+
+					pgc_gpumix: power-domain@2 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MN_POWER_DOMAIN_GPUMIX>;
+						clocks = <&clk IMX8MN_CLK_GPU_CORE_ROOT>,
+							 <&clk IMX8MN_CLK_GPU_SHADER>,
+							 <&clk IMX8MN_CLK_GPU_BUS_ROOT>,
+							 <&clk IMX8MN_CLK_GPU_AHB>;
+						resets = <&src IMX8MQ_RESET_GPU_RESET>;
+					};
+
+					pgc_dispmix: power-domain@3 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MN_POWER_DOMAIN_DISPMIX>;
+						clocks = <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
+							 <&clk IMX8MN_CLK_DISP_APB_ROOT>;
+					};
+
+					pgc_mipi: power-domain@4 {
+						#power-domain-cells = <0>;
+						reg = <IMX8MN_POWER_DOMAIN_MIPI>;
+						power-domains = <&pgc_dispmix>;
+					};
+				};
+			};
 		};
 
 		aips2: bus@30400000 {
-- 
2.32.0

[PATCH V3 7/9] arm64: dts: imx8mn: put USB controller into power-domains

[Adam Ford]

Now that we have support for the power domain controller on the i.MX8MN,
we can put the USB controller in the respective power domain to allow
it to power down the PHY when possible.

Signed-off-by: Adam Ford <aford173@gmail.com>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index f5bafd9db673..4e9d7099bb4f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -1021,6 +1021,7 @@ usbotg1: usb@32e40000 {
 				assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_500M>;
 				phys = <&usbphynop1>;
 				fsl,usbmisc = <&usbmisc1 0>;
+				power-domains = <&pgc_otg1>;
 				status = "disabled";
 			};
 
-- 
2.32.0

[PATCH V3 8/9] arm64: dts: imx8mn: add DISP blk-ctrl

[Adam Ford]

Add the DT node for the DISP blk-ctrl. With this in place the
display/mipi power domains should be functional.

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 4e9d7099bb4f..6ac14903bcef 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -1011,6 +1011,34 @@ aips4: bus@32c00000 {
 			#size-cells = <1>;
 			ranges;
 
+			disp_blk_ctrl: blk-ctrl@32e28000 {
+				compatible = "fsl,imx8mn-disp-blk-ctrl", "syscon";
+				reg = <0x32e28000 0x100>;
+				power-domains = <&pgc_dispmix>, <&pgc_dispmix>,
+						<&pgc_dispmix>, <&pgc_mipi>,
+						<&pgc_mipi>;
+				power-domain-names = "bus", "isi",
+						     "lcdif", "mipi-dsi",
+						     "mipi-csi";
+				clocks = <&clk IMX8MN_CLK_DISP_AXI>,
+					 <&clk IMX8MN_CLK_DISP_APB>,
+					 <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MN_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MN_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MN_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MN_CLK_DISP_PIXEL_ROOT>,
+					 <&clk IMX8MN_CLK_DSI_CORE>,
+					 <&clk IMX8MN_CLK_DSI_PHY_REF>,
+					 <&clk IMX8MN_CLK_CSI1_PHY_REF>,
+					 <&clk IMX8MN_CLK_CAMERA_PIXEL_ROOT>;
+				clock-names = "disp_axi", "disp_apb",
+					      "disp_axi_root", "disp_apb_root",
+					      "lcdif-axi", "lcdif-apb", "lcdif-pix",
+					      "dsi-pclk", "dsi-ref",
+					      "csi-aclk", "csi-pclk";
+				#power-domain-cells = <1>;
+			};
+
 			usbotg1: usb@32e40000 {
 				compatible = "fsl,imx8mn-usb", "fsl,imx7d-usb";
 				reg = <0x32e40000 0x200>;
-- 
2.32.0

[PATCH V3 9/9] arm64: dts: imx8mn: Enable GPU

[Adam Ford]

The i.MX8M-Nano features a GC7000. The Etnaviv driver detects it as:

    etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6203

Signed-off-by: Adam Ford <aford173@gmail.com>

diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
index 6ac14903bcef..2aafc17d8aa3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi
@@ -1089,6 +1089,31 @@ gpmi: nand-controller@33002000 {
 			status = "disabled";
 		};
 
+		gpu: gpu@38000000 {
+			compatible = "vivante,gc";
+			reg = <0x38000000 0x8000>;
+			interrupts = <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&clk IMX8MN_CLK_GPU_AHB>,
+				<&clk IMX8MN_CLK_GPU_BUS_ROOT>,
+				<&clk IMX8MN_CLK_GPU_CORE_ROOT>,
+				<&clk IMX8MN_CLK_GPU_SHADER>;
+			clock-names = "reg", "bus", "core", "shader";
+			assigned-clocks = <&clk IMX8MN_CLK_GPU_CORE>,
+					  <&clk IMX8MN_CLK_GPU_SHADER>,
+					  <&clk IMX8MN_CLK_GPU_AXI>,
+					  <&clk IMX8MN_CLK_GPU_AHB>,
+					  <&clk IMX8MN_GPU_PLL>,
+					  <&clk IMX8MN_CLK_GPU_CORE>,
+					  <&clk IMX8MN_CLK_GPU_SHADER>;
+			assigned-clock-parents = <&clk IMX8MN_GPU_PLL_OUT>,
+						  <&clk IMX8MN_GPU_PLL_OUT>,
+						  <&clk IMX8MN_SYS_PLL1_800M>,
+						  <&clk IMX8MN_SYS_PLL1_800M>;
+			assigned-clock-rates = <0>, <0>, <800000000>, <400000000>, <1200000000>,
+				<400000000>, <400000000>;
+			power-domains = <&pgc_gpumix>;
+		};
+
 		gic: interrupt-controller@38800000 {
 			compatible = "arm,gic-v3";
 			reg = <0x38800000 0x10000>,
-- 
2.32.0

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

[Tim Harvey]

On Thu, Nov 4, 2021 at 9:18 AM Adam Ford <aford173@gmail.com> wrote:
>
> The i.MX8M Nano is similar to the i.MX8M Mini in some ways, but very
> different in others.  With the blk-ctrl driver for Mini in place,
> this series expands the blk-ctrl driver to support the Nano which
> opens the door for additional functions in the future.  As part of
> this series, it also addresses some issues in the GPCv2 driver and
> finally adds support for enabling USB and GPU.
>
> V3:  Fixes an the yaml example
> V2:  Fixes the clock count in the blk-ctrl
>
> Adam Ford (9):
>   soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled
>   soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn
>   dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains
>   dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl
>   soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl
>   arm64: dts: imx8mn: add GPC node
>   arm64: dts: imx8mn: put USB controller into power-domains
>   arm64: dts: imx8mn: add DISP blk-ctrl
>   arm64: dts: imx8mn: Enable GPU
>
>  .../soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml     |  97 +++++++++++++++++
>  arch/arm64/boot/dts/freescale/imx8mn.dtsi     | 103 ++++++++++++++++++
>  drivers/soc/imx/gpcv2.c                       |  26 +++++
>  drivers/soc/imx/imx8m-blk-ctrl.c              |  75 ++++++++++++-
>  include/dt-bindings/power/imx8mn-power.h      |   5 +
>  5 files changed, 305 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
>

Adam,

Thanks for the patches. I'm not sure how best to test this but on an
imx8mm-venice-gw7902 which has USB, but no display.

I find that if DRM_ETNAVIV is enabled I hang at 'etnaviv etnaviv:
bound 38000000.gpu (ops 0xffff800010964748)'.

If DRM_ETNAVIV is not enabled:
- boots fine
- usb works
- soft reboot works (does not hang)

Best regards,

Tim

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

[Adam Ford]

On Tue, Nov 16, 2021 at 11:57 AM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Thu, Nov 4, 2021 at 9:18 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > The i.MX8M Nano is similar to the i.MX8M Mini in some ways, but very
> > different in others.  With the blk-ctrl driver for Mini in place,
> > this series expands the blk-ctrl driver to support the Nano which
> > opens the door for additional functions in the future.  As part of
> > this series, it also addresses some issues in the GPCv2 driver and
> > finally adds support for enabling USB and GPU.
> >
> > V3:  Fixes an the yaml example
> > V2:  Fixes the clock count in the blk-ctrl
> >
> > Adam Ford (9):
> >   soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled
> >   soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn
> >   dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains
> >   dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl
> >   soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl
> >   arm64: dts: imx8mn: add GPC node
> >   arm64: dts: imx8mn: put USB controller into power-domains
> >   arm64: dts: imx8mn: add DISP blk-ctrl
> >   arm64: dts: imx8mn: Enable GPU
> >
> >  .../soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml     |  97 +++++++++++++++++
> >  arch/arm64/boot/dts/freescale/imx8mn.dtsi     | 103 ++++++++++++++++++
> >  drivers/soc/imx/gpcv2.c                       |  26 +++++
> >  drivers/soc/imx/imx8m-blk-ctrl.c              |  75 ++++++++++++-
> >  include/dt-bindings/power/imx8mn-power.h      |   5 +
> >  5 files changed, 305 insertions(+), 1 deletion(-)
> >  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> >
>
> Adam,
>
> Thanks for the patches. I'm not sure how best to test this but on an
> imx8mm-venice-gw7902 which has USB, but no display.
>
> I find that if DRM_ETNAVIV is enabled I hang at 'etnaviv etnaviv:
> bound 38000000.gpu (ops 0xffff800010964748)'.

Thanks for testing this.

Does your board send power to the GPU?  I recall someone somewhere
didn't power theirGPU, but I can't remember who and/or what board was
being discussed.

I'll run some more tests on the latest 5.16-rc1 to see if I can
replicate your issue.

adam
>
> If DRM_ETNAVIV is not enabled:
> - boots fine
> - usb works
> - soft reboot works (does not hang)

At least we have some progress.  :-)

adam
>
> Best regards,
>
> Tim

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

[Tim Harvey]

On Tue, Nov 16, 2021 at 10:04 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Nov 16, 2021 at 11:57 AM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Thu, Nov 4, 2021 at 9:18 AM Adam Ford <aford173@gmail.com> wrote:
> > >
> > > The i.MX8M Nano is similar to the i.MX8M Mini in some ways, but very
> > > different in others.  With the blk-ctrl driver for Mini in place,
> > > this series expands the blk-ctrl driver to support the Nano which
> > > opens the door for additional functions in the future.  As part of
> > > this series, it also addresses some issues in the GPCv2 driver and
> > > finally adds support for enabling USB and GPU.
> > >
> > > V3:  Fixes an the yaml example
> > > V2:  Fixes the clock count in the blk-ctrl
> > >
> > > Adam Ford (9):
> > >   soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled
> > >   soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn
> > >   dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains
> > >   dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl
> > >   soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl
> > >   arm64: dts: imx8mn: add GPC node
> > >   arm64: dts: imx8mn: put USB controller into power-domains
> > >   arm64: dts: imx8mn: add DISP blk-ctrl
> > >   arm64: dts: imx8mn: Enable GPU
> > >
> > >  .../soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml     |  97 +++++++++++++++++
> > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi     | 103 ++++++++++++++++++
> > >  drivers/soc/imx/gpcv2.c                       |  26 +++++
> > >  drivers/soc/imx/imx8m-blk-ctrl.c              |  75 ++++++++++++-
> > >  include/dt-bindings/power/imx8mn-power.h      |   5 +
> > >  5 files changed, 305 insertions(+), 1 deletion(-)
> > >  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> > >
> >
> > Adam,
> >
> > Thanks for the patches. I'm not sure how best to test this but on an
> > imx8mm-venice-gw7902 which has USB, but no display.
> >
> > I find that if DRM_ETNAVIV is enabled I hang at 'etnaviv etnaviv:
> > bound 38000000.gpu (ops 0xffff800010964748)'.
>
> Thanks for testing this.
>
> Does your board send power to the GPU?  I recall someone somewhere
> didn't power theirGPU, but I can't remember who and/or what board was
> being discussed.

Yes, the imx8mm-venice-gw7902 does have the GPU powered (the
imx8mm-venice-gw7901 was the one that did not).

Tim

>
> I'll run some more tests on the latest 5.16-rc1 to see if I can
> replicate your issue.
>
> adam
> >
> > If DRM_ETNAVIV is not enabled:
> > - boots fine
> > - usb works
> > - soft reboot works (does not hang)
>
> At least we have some progress.  :-)
>
> adam
> >
> > Best regards,
> >
> > Tim

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

[Adam Ford]

On Tue, Nov 16, 2021 at 12:27 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Tue, Nov 16, 2021 at 10:04 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Tue, Nov 16, 2021 at 11:57 AM Tim Harvey <tharvey@gateworks.com> wrote:
> > >
> > > On Thu, Nov 4, 2021 at 9:18 AM Adam Ford <aford173@gmail.com> wrote:
> > > >
> > > > The i.MX8M Nano is similar to the i.MX8M Mini in some ways, but very
> > > > different in others.  With the blk-ctrl driver for Mini in place,
> > > > this series expands the blk-ctrl driver to support the Nano which
> > > > opens the door for additional functions in the future.  As part of
> > > > this series, it also addresses some issues in the GPCv2 driver and
> > > > finally adds support for enabling USB and GPU.
> > > >
> > > > V3:  Fixes an the yaml example
> > > > V2:  Fixes the clock count in the blk-ctrl
> > > >
> > > > Adam Ford (9):
> > > >   soc: imx: gpcv2: keep i.MX8MN gpumix bus clock enabled
> > > >   soc: imx: gpcv2: Add dispmix and mipi domains to imx8mn
> > > >   dt-bindings: power: imx8mn: add defines for DISP blk-ctrl domains
> > > >   dt-bindings: soc: add binding for i.MX8MN DISP blk-ctrl
> > > >   soc: imx: imx8m-blk-ctrl: add i.MX8MN DISP blk-ctrl
> > > >   arm64: dts: imx8mn: add GPC node
> > > >   arm64: dts: imx8mn: put USB controller into power-domains
> > > >   arm64: dts: imx8mn: add DISP blk-ctrl
> > > >   arm64: dts: imx8mn: Enable GPU
> > > >
> > > >  .../soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml     |  97 +++++++++++++++++
> > > >  arch/arm64/boot/dts/freescale/imx8mn.dtsi     | 103 ++++++++++++++++++
> > > >  drivers/soc/imx/gpcv2.c                       |  26 +++++
> > > >  drivers/soc/imx/imx8m-blk-ctrl.c              |  75 ++++++++++++-
> > > >  include/dt-bindings/power/imx8mn-power.h      |   5 +
> > > >  5 files changed, 305 insertions(+), 1 deletion(-)
> > > >  create mode 100644 Documentation/devicetree/bindings/soc/imx/fsl,imx8mn-disp-blk-ctrl.yaml
> > > >
> > >
> > > Adam,
> > >
> > > Thanks for the patches. I'm not sure how best to test this but on an
> > > imx8mm-venice-gw7902 which has USB, but no display.
> > >
> > > I find that if DRM_ETNAVIV is enabled I hang at 'etnaviv etnaviv:
> > > bound 38000000.gpu (ops 0xffff800010964748)'.
> >
> > Thanks for testing this.
> >
> > Does your board send power to the GPU?  I recall someone somewhere
> > didn't power theirGPU, but I can't remember who and/or what board was
> > being discussed.
>
> Yes, the imx8mm-venice-gw7902 does have the GPU powered (the
> imx8mm-venice-gw7901 was the one that did not).

I cannot replicate your issue.  I applied the patch series to
5.16-rc1, and it's still working for me.

root@beacon-imx8mn-kit:~# dmesg |grep viv
[   11.323660] etnaviv etnaviv: bound 38000000.gpu (ops gpu_ops [etnaviv])
[   11.480543] etnaviv-gpu 38000000.gpu: model: GC7000, revision: 6203
[   11.747576] [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0

After booting, I connected a USB drive and it enumerated.

[  152.328228] ci_hdrc ci_hdrc.0: new USB bus registered, assigned bus number 1
[  152.351885] ci_hdrc ci_hdrc.0: USB 2.0 started, EHCI 1.00
[  152.363859] hub 1-0:1.0: USB hub found
[  152.371560] hub 1-0:1.0: 1 port detected
[  153.075902] usb 1-1: new high-speed USB device number 2 using ci_hdrc
[  153.244124] usb-storage 1-1:1.0: USB Mass Storage device detected
[  153.263651] scsi host0: usb-storage 1-1:1.0
[  154.277997] scsi 0:0:0:0: Direct-Access     SanDisk  Cruzer
  1.00 PQ: 0 ANSI: 6
[  154.292357] sd 0:0:0:0: [sda] 247529472 512-byte logical blocks:
(127 GB/118 GiB)
[  154.309246] sd 0:0:0:0: [sda] Write Protect is off
[  154.319237] sd 0:0:0:0: [sda] Write cache: disabled, read cache:
enabled, doesn't support DPO or FUA
[  154.359113]  sda: sda1
[  154.366975] sd 0:0:0:0: [sda] Attached SCSI removable disk

root@beacon-imx8mn-kit:~# lsusb
Bus 001 Device 002: ID 0781:5530 SanDisk Corp. Cruzer
Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

I am not sure where to go from here.

>
> Tim
>
> >
> > I'll run some more tests on the latest 5.16-rc1 to see if I can
> > replicate your issue.
> >
> > adam
> > >
> > > If DRM_ETNAVIV is not enabled:
> > > - boots fine
> > > - usb works
> > > - soft reboot works (does not hang)
> >
> > At least we have some progress.  :-)
> >
> > adam
> > >
> > > Best regards,
> > >
> > > Tim

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

[Fabio Estevam]

Hi Adam,

On Sun, Nov 21, 2021 at 10:07 AM Adam Ford <aford173@gmail.com> wrote:

> I cannot replicate your issue.  I applied the patch series to
> 5.16-rc1, and it's still working for me.

Could the different behavior be caused by different TF-A versions that
you and Tim used?

Which ATF version do you use? Is it TF-A v2.5?

Thanks

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

[Adam Ford]

On Sun, Nov 21, 2021 at 8:12 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Adam,
>
> On Sun, Nov 21, 2021 at 10:07 AM Adam Ford <aford173@gmail.com> wrote:
>
> > I cannot replicate your issue.  I applied the patch series to
> > 5.16-rc1, and it's still working for me.
>
> Could the different behavior be caused by different TF-A versions that
> you and Tim used?
>
> Which ATF version do you use? Is it TF-A v2.5?

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.

thanks,

adam

>
> Thanks

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

[Fabio Estevam]

Hi Adam,

On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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

Thanks

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

[Adam Ford]

On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Adam,
>
> On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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



>
> Thanks

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

[Adam Ford]

On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> >
> > Hi Adam,
> >
> > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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

>
> >
> > Thanks

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

[Tim Harvey]

On Sun, Nov 21, 2021 at 7:25 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> >
> > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > >
> > > Hi Adam,
> > >
> > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.

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?

At any rate for now let's keep the ball rolling!

For the series:
Reviewed-by: Tim Harvey <tharvey@gateworks.com>
Tested-by: Tim Harvey <tharvey@gateworks.com> (tested on imx8mm-venice-gw7902)

Best regards,

Tim

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

[Lucas Stach]

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@gmail.com> wrote:
> > 
> > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > 
> > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > 
> > > > Hi Adam,
> > > > 
> > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.

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

Regards,
Lucas

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

[Tim Harvey]

On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > >
> > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > >
> > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > >
> > > > > Hi Adam,
> > > > >
> > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.
>
> > 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
- 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

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

[Adam Ford]

On Mon, Nov 22, 2021 at 3:52 PM Tim Harvey <tharvey@gateworks.com> wrote:
>
> On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > >
> > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > >
> > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > >
> > > > > > Hi Adam,
> > > > > >
> > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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

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

[Adam Ford]

On Tue, Nov 23, 2021 at 8:08 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Mon, Nov 22, 2021 at 3:52 PM Tim Harvey <tharvey@gateworks.com> wrote:
> >
> > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > >
> > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > >
> > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > >
> > > > > > > Hi Adam,
> > > > > > >
> > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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

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

[Lucas Stach]

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@gateworks.com> wrote:
> > 
> > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > > 
> > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > > 
> > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > > 
> > > > > > > Hi Adam,
> > > > > > > 
> > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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*.

Regards,
Lucas

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

[Adam Ford]

On Tue, Nov 23, 2021 at 8:24 AM Lucas Stach <l.stach@pengutronix.de> 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@gateworks.com> wrote:
> > >
> > > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > > >
> > > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > > >
> > > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Adam,
> > > > > > > >
> > > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.

adam

>
> Regards,
> Lucas
>

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

[Adam Ford]

On Tue, Nov 23, 2021 at 8:30 AM Adam Ford <aford173@gmail.com> wrote:
>
> On Tue, Nov 23, 2021 at 8:24 AM Lucas Stach <l.stach@pengutronix.de> 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@gateworks.com> wrote:
> > > >
> > > > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > > > >
> > > > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > > > >
> > > > > > > > > Hi Adam,
> > > > > > > > >
> > > > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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
> >

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

[Lucas Stach]

Am Dienstag, dem 23.11.2021 um 10:40 -0600 schrieb Adam Ford:
> On Tue, Nov 23, 2021 at 8:30 AM Adam Ford <aford173@gmail.com> wrote:
> > 
> > On Tue, Nov 23, 2021 at 8:24 AM Lucas Stach <l.stach@pengutronix.de> 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@gateworks.com> wrote:
> > > > > 
> > > > > On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > > > > > 
> > > > > > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > > > > > 
> > > > > > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > > > > > 
> > > > > > > > > > Hi Adam,
> > > > > > > > > > 
> > > > > > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.
> 
Yes, that's an option but it quickly blows up in a combinatorial
explosion. As the chips are pin compatible there is a high probability
that hardware makers will start to offer boards with different feature
sets and then you need to have a number of board DTs just to include
the right dtsi for the chip.

> Just a thought.  Although, I really like the idea of the bootloader
> disabling the unavailable nodes.

Yea, it seems there are even fuses that allow to check those
capabilities, see my reply to Tim. So I think relying on the boot
firmware to fix things up would be the best option, as it allows to
keep the number of DTs small and does not place a big burden on the
boot firmware implementation.

Regards,
Lucas

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

[Lucas Stach]

Am Montag, dem 22.11.2021 um 13:52 -0800 schrieb Tim Harvey:
> On Mon, Nov 22, 2021 at 10:20 AM Lucas Stach <l.stach@pengutronix.de> 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@gmail.com> wrote:
> > > > 
> > > > On Sun, Nov 21, 2021 at 8:34 AM Adam Ford <aford173@gmail.com> wrote:
> > > > > 
> > > > > On Sun, Nov 21, 2021 at 8:21 AM Fabio Estevam <festevam@gmail.com> wrote:
> > > > > > 
> > > > > > Hi Adam,
> > > > > > 
> > > > > > On Sun, Nov 21, 2021 at 11:17 AM Adam Ford <aford173@gmail.com> 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.
> > 
> > > 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
> - 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.

According to the fusemap there are even individual fuses to indicate
disabled GPU, DSI, CSI and other peripherals (table 6-31 in the 8MN RM,
0x450 range). I would expect that those are burned correctly on the
stripped down SKUs. Maybe you can validate this on your side?

Regards,
Lucas