[PATCH v4 14/18] arm64: dts: imx8mm: add GPC node
Tim Harvey
tharvey at gateworks.com
Fri Oct 1 19:43:46 PDT 2021
'On Fri, Oct 1, 2021 at 5:25 PM Lucas Stach <l.stach at pengutronix.de> wrote:
>
> Am Freitag, dem 01.10.2021 um 17:15 -0700 schrieb Tim Harvey:
> > On Fri, Oct 1, 2021 at 5:06 PM Tim Harvey <tharvey at gateworks.com> wrote:
> > >
> > > On Fri, Oct 1, 2021 at 4:20 PM Lucas Stach <l.stach at pengutronix.de> wrote:
> > > >
> > > > Hi Tim,
> > > >
> > > > Am Freitag, dem 01.10.2021 um 16:07 -0700 schrieb Tim Harvey:
> > > > > On Fri, Sep 10, 2021 at 1:26 PM Lucas Stach <l.stach at pengutronix.de> wrote:
> > > > > >
> > > > > > 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: Lucas Stach <l.stach at pengutronix.de>
> > > > > > ---
> > > > > > arch/arm64/boot/dts/freescale/imx8mm.dtsi | 107 ++++++++++++++++++++++
> > > > > > 1 file changed, 107 insertions(+)
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > > index e7648c3b8390..3922f26f8fd4 100644
> > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> > > > > > @@ -7,6 +7,8 @@
> > > > > > #include <dt-bindings/gpio/gpio.h>
> > > > > > #include <dt-bindings/input/input.h>
> > > > > > #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > > > > +#include <dt-bindings/power/imx8mm-power.h>
> > > > > > +#include <dt-bindings/reset/imx8mq-reset.h>
> > > > > > #include <dt-bindings/thermal/thermal.h>
> > > > > >
> > > > > > #include "imx8mm-pinfunc.h"
> > > > > > @@ -609,6 +611,111 @@ src: reset-controller at 30390000 {
> > > > > > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > #reset-cells = <1>;
> > > > > > };
> > > > > > +
> > > > > > + gpc: gpc at 303a0000 {
> > > > > > + compatible = "fsl,imx8mm-gpc";
> > > > > > + reg = <0x303a0000 0x10000>;
> > > > > > + interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
> > > > > > + interrupt-parent = <&gic>;
> > > > > > + interrupt-controller;
> > > > > > + #interrupt-cells = <3>;
> > > > > > +
> > > > > > + pgc {
> > > > > > + #address-cells = <1>;
> > > > > > + #size-cells = <0>;
> > > > > > +
> > > > > > + pgc_hsiomix: power-domain at 0 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_HSIOMIX>;
> > > > > > + clocks = <&clk IMX8MM_CLK_USB_BUS>;
> > > > > > + assigned-clocks = <&clk IMX8MM_CLK_USB_BUS>;
> > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_500M>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_pcie: power-domain at 1 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> > > > > > + power-domains = <&pgc_hsiomix>;
> > > > > > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_otg1: power-domain at 2 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> > > > > > + power-domains = <&pgc_hsiomix>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_otg2: power-domain at 3 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> > > > > > + power-domains = <&pgc_hsiomix>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_gpumix: power-domain at 4 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_GPUMIX>;
> > > > > > + clocks = <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
> > > > > > + <&clk IMX8MM_CLK_GPU_AHB>;
> > > > > > + assigned-clocks = <&clk IMX8MM_CLK_GPU_AXI>,
> > > > > > + <&clk IMX8MM_CLK_GPU_AHB>;
> > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>,
> > > > > > + <&clk IMX8MM_SYS_PLL1_800M>;
> > > > > > + assigned-clock-rates = <800000000>, <400000000>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_gpu: power-domain at 5 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_GPU>;
> > > > > > + clocks = <&clk IMX8MM_CLK_GPU_AHB>,
> > > > > > + <&clk IMX8MM_CLK_GPU_BUS_ROOT>,
> > > > > > + <&clk IMX8MM_CLK_GPU2D_ROOT>,
> > > > > > + <&clk IMX8MM_CLK_GPU3D_ROOT>;
> > > > > > + resets = <&src IMX8MQ_RESET_GPU_RESET>;
> > > > > > + power-domains = <&pgc_gpumix>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_vpumix: power-domain at 6 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUMIX>;
> > > > > > + clocks = <&clk IMX8MM_CLK_VPU_DEC_ROOT>;
> > > > > > + assigned-clocks = <&clk IMX8MM_CLK_VPU_BUS>;
> > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_800M>;
> > > > > > + resets = <&src IMX8MQ_RESET_VPU_RESET>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_vpu_g1: power-domain at 7 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUG1>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_vpu_g2: power-domain at 8 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_vpu_h1: power-domain at 9 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_dispmix: power-domain at 10 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_DISPMIX>;
> > > > > > + clocks = <&clk IMX8MM_CLK_DISP_APB_ROOT>,
> > > > > > + <&clk IMX8MM_CLK_DISP_AXI_ROOT>;
> > > > > > + assigned-clocks = <&clk IMX8MM_CLK_DISP_AXI>,
> > > > > > + <&clk IMX8MM_CLK_DISP_APB>;
> > > > > > + assigned-clock-parents = <&clk IMX8MM_SYS_PLL2_1000M>,
> > > > > > + <&clk IMX8MM_SYS_PLL1_800M>;
> > > > > > + assigned-clock-rates = <500000000>, <200000000>;
> > > > > > + };
> > > > > > +
> > > > > > + pgc_mipi: power-domain at 11 {
> > > > > > + #power-domain-cells = <0>;
> > > > > > + reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> > > > > > + };
> > > > > > + };
> > > > > > + };
> > > > > > };
> > > > > >
> > > > > > aips2: bus at 30400000 {
> > > > > > --
> > > > > > 2.30.2
> > > > > >
> > > > >
> > > > > Lucas,
> > > > >
> > > > > I've been using your 'i.MX8MM GPC improvements and BLK_CTRL driver'
> > > > > series for imx8mm-venice* and imx8mn-venice* boards. Thank you for
> > > > > this work and I hope to see it merged soon!
> > > > >
> > > > > I have found that on the imx8mm-venice-gw7901 board which does not use
> > > > > MIPI and thus does not connect VDD_MIPI_1P8, VDD_MIPI_1P2,
> > > > > VDD_MIPI_0P9, MIPI_VREG_CAP pins on the IMX8MM hangs with this
> > > > > particular patch. If I comment out the pgc_mipi domain and subsequent
> > > > > disp_blk_ctrl node from a later patch it resolves the hang. Is this
> > > > > behavior expected and what would your recommendation be to work around
> > > > > it?
> > > >
> > > > No, this isn't expected. If there are no active devices in the MIPI
> > > > domain, the power domain should not be touched, as we treat all of them
> > > > as disabled initially. If we don't touch the domain I would expect that
> > > > the power supply not being present shouldn't be an issue.
> > > >
> > > > Can you check if something in your system causes this power domain to
> > > > be powered up? Easiest way is probably to sprinkle a
> > > > printk("%s\n, genpd->name) in both imx8m_blk_ctrl_power_on() and
> > > > imx_gpc_power_on().
> > > >
> > >
> > > Lucas,
> > >
> > > Here's what I see before I hang (debug print on both power on/off
> > > followed by a msleep(1000) to make sure I see it before I hang):
> > > [ 0.518319] imx_pgc_power_up hsiomix
> > > [ 0.624031] imx_pgc_power_down hsiomix
> > > [ 0.731879] imx_pgc_power_up hsiomix
> > > [ 0.839906] imx_pgc_power_down hsiomix
> > > [ 0.947875] imx_pgc_power_up hsiomix
> > > [ 1.055859] imx_pgc_power_down hsiomix
> > > [ 1.057296] imx_pgc_power_up gpumix
> > > [ 1.167884] imx_pgc_power_down gpumix
> > > [ 0.518513] imx_pgc_power_up hsiomix
> > > [ 0.519933] imx_pgc_power_up gpumix
> > >
> >
> > The board also has IMX8MM VDD_GPU pins not connected so it makes sense
> > that we hang here I suppose. Yet if I add the folloiwng to
> > imx8mm-venice-gw7901.dts it still tries to enable them and hangs:
> > &gpu_2d {
> > status = "disabled";
> > };
> >
> > &gpu_3d {
> > status = "disabled";
> > };
> >
> > &vpu_blk_ctrl {
> > status = "disabled";
> > };
>
> The pgc_gpu is a "active" consumer of the pgc_gpumix domain while the
> driver gets probed, so the driver core will power up the gpumix domain
> for a moment during kernel init. To avoid this you must at least set
> the status of the pgc_gpu node to disabled.
>
I've tried that and it doesn't work either.
&gpu_2d {
status = "disabled";
};
&gpu_3d {
status = "disabled";
};
&vpu_blk_ctrl {
status = "disabled";
};
&pgc_gpumix {
status = "disabled";
};
&pgc_gpu {
status = "disabled";
};
The interesting thing is that the first patch that causes this is
'arm64: dts: imx8mm: add GPC node' which just adds the gpc node and is
before the addition of the other nodes. Therefore these are being
enabled by default regardless of the above nodes being disabled (or
not even added yet to imx8mm.dtsi).
Tim
More information about the linux-arm-kernel
mailing list