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@xxxxxxxxxxxxx> wrote:
> >
> > On Fri, Oct 1, 2021 at 4:20 PM Lucas Stach <l.stach@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > > > > ---
> > > > > 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@30390000 {
> > > > > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > > > > #reset-cells = <1>;
> > > > > };
> > > > > +
> > > > > + gpc: gpc@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@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@1 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_PCIE>;
> > > > > + power-domains = <&pgc_hsiomix>;
> > > > > + clocks = <&clk IMX8MM_CLK_PCIE1_ROOT>;
> > > > > + };
> > > > > +
> > > > > + pgc_otg1: power-domain@2 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_OTG1>;
> > > > > + power-domains = <&pgc_hsiomix>;
> > > > > + };
> > > > > +
> > > > > + pgc_otg2: power-domain@3 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_OTG2>;
> > > > > + power-domains = <&pgc_hsiomix>;
> > > > > + };
> > > > > +
> > > > > + pgc_gpumix: power-domain@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@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@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@7 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUG1>;
> > > > > + };
> > > > > +
> > > > > + pgc_vpu_g2: power-domain@8 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUG2>;
> > > > > + };
> > > > > +
> > > > > + pgc_vpu_h1: power-domain@9 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_VPUH1>;
> > > > > + };
> > > > > +
> > > > > + pgc_dispmix: power-domain@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@11 {
> > > > > + #power-domain-cells = <0>;
> > > > > + reg = <IMX8MM_POWER_DOMAIN_MIPI>;
> > > > > + };
> > > > > + };
> > > > > + };
> > > > > };
> > > > >
> > > > > aips2: bus@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.
Regards,
Lucas
