| Message ID | 20220125171129.472775-8-aford173@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: hantro: imx8mq/imx8mm: Let VPU decoders get controlled by vpu-blk-ctrl | expand |
Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > With the Hantro G1 and G2 now setup to run independently, update > the device tree to allow both to operate. This requires the > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > certain clock enabled to handle the gating of the G1 and G2 > fuses, the clock-parents and clock-rates for the various VPU's > to be moved into the pgc_vpu because they cannot get re-parented > once enabled, and the pgc_vpu is the highest in the chain. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > index 2df2510d0118..549b2440f55d 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > pgc_vpu: power-domain@6 { > #power-domain-cells = <0>; > reg = <IMX8M_POWER_DOMAIN_VPU>; > - clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > + clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>, > + <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > + <&clk IMX8MQ_CLK_VPU_G2>, > + <&clk IMX8MQ_CLK_VPU_BUS>, > + <&clk IMX8MQ_VPU_PLL_BYPASS>; > + assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > + <&clk IMX8MQ_VPU_PLL_OUT>, > + <&clk IMX8MQ_SYS1_PLL_800M>, > + <&clk IMX8MQ_VPU_PLL>; > + assigned-clock-rates = <600000000>, > + <600000000>, > + <800000000>, > + <0>; > }; > > pgc_disp: power-domain@7 { > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > status = "disabled"; > }; > > - vpu: video-codec@38300000 { > - compatible = "nxp,imx8mq-vpu"; > - reg = <0x38300000 0x10000>, > - <0x38310000 0x10000>, > - <0x38320000 0x10000>; > - reg-names = "g1", "g2", "ctrl"; > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-names = "g1", "g2"; > + vpu_g1: video-codec@38300000 { > + compatible = "nxp,imx8mq-vpu-g1"; > + reg = <0x38300000 0x10000>; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>; > + }; > + > + vpu_g2: video-codec@38310000 { > + compatible = "nxp,imx8mq-vpu-g2"; > + reg = <0x38310000 0x10000>; > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>; > + }; > + > + vpu_blk_ctrl: blk-ctrl@38320000 { > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > + reg = <0x38320000 0x100>; > + power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>; > + power-domain-names = "bus", "g1", "g2"; > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > - clock-names = "g1", "g2", "bus"; > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > - <&clk IMX8MQ_CLK_VPU_G2>, > - <&clk IMX8MQ_CLK_VPU_BUS>, > - <&clk IMX8MQ_VPU_PLL_BYPASS>; > - assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > - <&clk IMX8MQ_VPU_PLL_OUT>, > - <&clk IMX8MQ_SYS1_PLL_800M>, > - <&clk IMX8MQ_VPU_PLL>; > - assigned-clock-rates = <600000000>, <600000000>, > - <800000000>, <0>; > - power-domains = <&pgc_vpu>; > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + clock-names = "g1", "g2"; > + #power-domain-cells = <1>; > }; > > pcie0: pcie@33800000 {
On Tue, Jan 25, 2022 at 11:11:24AM -0600, Adam Ford wrote: > With the Hantro G1 and G2 now setup to run independently, update > the device tree to allow both to operate. This requires the > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > certain clock enabled to handle the gating of the G1 and G2 > fuses, the clock-parents and clock-rates for the various VPU's > to be moved into the pgc_vpu because they cannot get re-parented > once enabled, and the pgc_vpu is the highest in the chain. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> It doesn't seem correct to have the Reported-by on this commit. Thanks, Ezequiel > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > index 2df2510d0118..549b2440f55d 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > pgc_vpu: power-domain@6 { > #power-domain-cells = <0>; > reg = <IMX8M_POWER_DOMAIN_VPU>; > - clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > + clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>, > + <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > + <&clk IMX8MQ_CLK_VPU_G2>, > + <&clk IMX8MQ_CLK_VPU_BUS>, > + <&clk IMX8MQ_VPU_PLL_BYPASS>; > + assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > + <&clk IMX8MQ_VPU_PLL_OUT>, > + <&clk IMX8MQ_SYS1_PLL_800M>, > + <&clk IMX8MQ_VPU_PLL>; > + assigned-clock-rates = <600000000>, > + <600000000>, > + <800000000>, > + <0>; > }; > > pgc_disp: power-domain@7 { > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > status = "disabled"; > }; > > - vpu: video-codec@38300000 { > - compatible = "nxp,imx8mq-vpu"; > - reg = <0x38300000 0x10000>, > - <0x38310000 0x10000>, > - <0x38320000 0x10000>; > - reg-names = "g1", "g2", "ctrl"; > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-names = "g1", "g2"; > + vpu_g1: video-codec@38300000 { > + compatible = "nxp,imx8mq-vpu-g1"; > + reg = <0x38300000 0x10000>; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>; > + }; > + > + vpu_g2: video-codec@38310000 { > + compatible = "nxp,imx8mq-vpu-g2"; > + reg = <0x38310000 0x10000>; > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>; > + }; > + > + vpu_blk_ctrl: blk-ctrl@38320000 { > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > + reg = <0x38320000 0x100>; > + power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>; > + power-domain-names = "bus", "g1", "g2"; > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > - clock-names = "g1", "g2", "bus"; > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > - <&clk IMX8MQ_CLK_VPU_G2>, > - <&clk IMX8MQ_CLK_VPU_BUS>, > - <&clk IMX8MQ_VPU_PLL_BYPASS>; > - assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > - <&clk IMX8MQ_VPU_PLL_OUT>, > - <&clk IMX8MQ_SYS1_PLL_800M>, > - <&clk IMX8MQ_VPU_PLL>; > - assigned-clock-rates = <600000000>, <600000000>, > - <800000000>, <0>; > - power-domains = <&pgc_vpu>; > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + clock-names = "g1", "g2"; > + #power-domain-cells = <1>; > }; > > pcie0: pcie@33800000 { > -- > 2.32.0 >
On Tue, Jan 25, 2022 at 1:04 PM Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > > On Tue, Jan 25, 2022 at 11:11:24AM -0600, Adam Ford wrote: > > With the Hantro G1 and G2 now setup to run independently, update > > the device tree to allow both to operate. This requires the > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > certain clock enabled to handle the gating of the G1 and G2 > > fuses, the clock-parents and clock-rates for the various VPU's > > to be moved into the pgc_vpu because they cannot get re-parented > > once enabled, and the pgc_vpu is the highest in the chain. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Reported-by: kernel test robot <lkp@intel.com> > > It doesn't seem correct to have the Reported-by on this commit. I didn't put it here, because I fixed it in a whole different patch (Patch 1/11). This patch remains unchanged. I probably should have put in the other patch, but I didn't think it was essential. Sorry about that. Do I need to resend to just add the r-b tag? adam > > Thanks, > Ezequiel > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > index 2df2510d0118..549b2440f55d 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > pgc_vpu: power-domain@6 { > > #power-domain-cells = <0>; > > reg = <IMX8M_POWER_DOMAIN_VPU>; > > - clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > + clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>, > > + <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > > + <&clk IMX8MQ_CLK_VPU_G2>, > > + <&clk IMX8MQ_CLK_VPU_BUS>, > > + <&clk IMX8MQ_VPU_PLL_BYPASS>; > > + assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > + <&clk IMX8MQ_VPU_PLL_OUT>, > > + <&clk IMX8MQ_SYS1_PLL_800M>, > > + <&clk IMX8MQ_VPU_PLL>; > > + assigned-clock-rates = <600000000>, > > + <600000000>, > > + <800000000>, > > + <0>; > > }; > > > > pgc_disp: power-domain@7 { > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > status = "disabled"; > > }; > > > > - vpu: video-codec@38300000 { > > - compatible = "nxp,imx8mq-vpu"; > > - reg = <0x38300000 0x10000>, > > - <0x38310000 0x10000>, > > - <0x38320000 0x10000>; > > - reg-names = "g1", "g2", "ctrl"; > > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > - interrupt-names = "g1", "g2"; > > + vpu_g1: video-codec@38300000 { > > + compatible = "nxp,imx8mq-vpu-g1"; > > + reg = <0x38300000 0x10000>; > > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>; > > + }; > > + > > + vpu_g2: video-codec@38310000 { > > + compatible = "nxp,imx8mq-vpu-g2"; > > + reg = <0x38310000 0x10000>; > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>; > > + }; > > + > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > + reg = <0x38320000 0x100>; > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>; > > + power-domain-names = "bus", "g1", "g2"; > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > - clock-names = "g1", "g2", "bus"; > > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > > - <&clk IMX8MQ_CLK_VPU_G2>, > > - <&clk IMX8MQ_CLK_VPU_BUS>, > > - <&clk IMX8MQ_VPU_PLL_BYPASS>; > > - assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > - <&clk IMX8MQ_VPU_PLL_OUT>, > > - <&clk IMX8MQ_SYS1_PLL_800M>, > > - <&clk IMX8MQ_VPU_PLL>; > > - assigned-clock-rates = <600000000>, <600000000>, > > - <800000000>, <0>; > > - power-domains = <&pgc_vpu>; > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > + clock-names = "g1", "g2"; > > + #power-domain-cells = <1>; > > }; > > > > pcie0: pcie@33800000 { > > -- > > 2.32.0 > >
On Tue, Jan 25, 2022 at 04:04:38PM -0300, Ezequiel Garcia wrote: > On Tue, Jan 25, 2022 at 11:11:24AM -0600, Adam Ford wrote: > > With the Hantro G1 and G2 now setup to run independently, update > > the device tree to allow both to operate. This requires the > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > certain clock enabled to handle the gating of the G1 and G2 > > fuses, the clock-parents and clock-rates for the various VPU's > > to be moved into the pgc_vpu because they cannot get re-parented > > once enabled, and the pgc_vpu is the highest in the chain. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Reported-by: kernel test robot <lkp@intel.com> > > It doesn't seem correct to have the Reported-by on this commit. Applied with Reported-by tag dropped, thanks!
Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > With the Hantro G1 and G2 now setup to run independently, update > the device tree to allow both to operate. This requires the > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > certain clock enabled to handle the gating of the G1 and G2 > fuses, the clock-parents and clock-rates for the various VPU's > to be moved into the pgc_vpu because they cannot get re-parented > once enabled, and the pgc_vpu is the highest in the chain. > > Signed-off-by: Adam Ford <aford173@gmail.com> > Reported-by: kernel test robot <lkp@intel.com> > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > index 2df2510d0118..549b2440f55d 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > pgc_vpu: power-domain@6 { > #power-domain-cells = > <0>; > reg = > <IMX8M_POWER_DOMAIN_VPU>; > - clocks = <&clk > IMX8MQ_CLK_VPU_DEC_ROOT>; > + clocks = <&clk > IMX8MQ_CLK_VPU_DEC_ROOT>, > + <&clk > IMX8MQ_CLK_VPU_G1_ROOT>, > + <&clk > IMX8MQ_CLK_VPU_G2_ROOT>; > + assigned-clocks = > <&clk IMX8MQ_CLK_VPU_G1>, > + > <&clk IMX8MQ_CLK_VPU_G2>, > + > <&clk IMX8MQ_CLK_VPU_BUS>, > + > <&clk IMX8MQ_VPU_PLL_BYPASS>; > + assigned-clock- > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > + > <&clk IMX8MQ_VPU_PLL_OUT>, > + > <&clk IMX8MQ_SYS1_PLL_800M>, > + > <&clk IMX8MQ_VPU_PLL>; > + assigned-clock-rates > = <600000000>, > + > <600000000>, > + > <800000000>, > + > <0>; > }; > > pgc_disp: power-domain@7 { > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > status = "disabled"; > }; > > - vpu: video-codec@38300000 { > - compatible = "nxp,imx8mq-vpu"; > - reg = <0x38300000 0x10000>, > - <0x38310000 0x10000>, > - <0x38320000 0x10000>; > - reg-names = "g1", "g2", "ctrl"; > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > - interrupt-names = "g1", "g2"; > + vpu_g1: video-codec@38300000 { > + compatible = "nxp,imx8mq-vpu-g1"; > + reg = <0x38300000 0x10000>; > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > + power-domains = <&vpu_blk_ctrl > IMX8MQ_VPUBLK_PD_G1>; > + }; > + > + vpu_g2: video-codec@38310000 { > + compatible = "nxp,imx8mq-vpu-g2"; > + reg = <0x38310000 0x10000>; > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + power-domains = <&vpu_blk_ctrl > IMX8MQ_VPUBLK_PD_G2>; > + }; > + > + vpu_blk_ctrl: blk-ctrl@38320000 { > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > + reg = <0x38320000 0x100>; > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > <&pgc_vpu>; > + power-domain-names = "bus", "g1", "g2"; > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > - clock-names = "g1", "g2", "bus"; > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > - <&clk IMX8MQ_CLK_VPU_G2>, > - <&clk IMX8MQ_CLK_VPU_BUS>, > - <&clk > IMX8MQ_VPU_PLL_BYPASS>; > - assigned-clock-parents = <&clk > IMX8MQ_VPU_PLL_OUT>, > - <&clk > IMX8MQ_VPU_PLL_OUT>, > - <&clk > IMX8MQ_SYS1_PLL_800M>, > - <&clk > IMX8MQ_VPU_PLL>; > - assigned-clock-rates = <600000000>, > <600000000>, > - <800000000>, <0>; > - power-domains = <&pgc_vpu>; > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > + clock-names = "g1", "g2"; > + #power-domain-cells = <1>; > }; > > pcie0: pcie@33800000 { With this update, when testing suspend to ram on imx8mq, I get: buck4: failed to disable: -ETIMEDOUT where buck4 is power-supply of pgc_vpu. And thus the transition to suspend (and resuming) fails. Have you tested system suspend after the imx8m-blk-ctrl update on imx8mq? thank you, martin
Hi Martin, Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > With the Hantro G1 and G2 now setup to run independently, update > > the device tree to allow both to operate. This requires the > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > certain clock enabled to handle the gating of the G1 and G2 > > fuses, the clock-parents and clock-rates for the various VPU's > > to be moved into the pgc_vpu because they cannot get re-parented > > once enabled, and the pgc_vpu is the highest in the chain. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > Reported-by: kernel test robot <lkp@intel.com> > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > index 2df2510d0118..549b2440f55d 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > pgc_vpu: power-domain@6 { > > #power-domain-cells = > > <0>; > > reg = > > <IMX8M_POWER_DOMAIN_VPU>; > > - clocks = <&clk > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > + clocks = <&clk > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > + <&clk > > IMX8MQ_CLK_VPU_G1_ROOT>, > > + <&clk > > IMX8MQ_CLK_VPU_G2_ROOT>; > > + assigned-clocks = > > <&clk IMX8MQ_CLK_VPU_G1>, > > + > > <&clk IMX8MQ_CLK_VPU_G2>, > > + > > <&clk IMX8MQ_CLK_VPU_BUS>, > > + > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > + assigned-clock- > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > + > > <&clk IMX8MQ_VPU_PLL_OUT>, > > + > > <&clk IMX8MQ_SYS1_PLL_800M>, > > + > > <&clk IMX8MQ_VPU_PLL>; > > + assigned-clock-rates > > = <600000000>, > > + > > <600000000>, > > + > > <800000000>, > > + > > <0>; > > }; > > > > pgc_disp: power-domain@7 { > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > status = "disabled"; > > }; > > > > - vpu: video-codec@38300000 { > > - compatible = "nxp,imx8mq-vpu"; > > - reg = <0x38300000 0x10000>, > > - <0x38310000 0x10000>, > > - <0x38320000 0x10000>; > > - reg-names = "g1", "g2", "ctrl"; > > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > - interrupt-names = "g1", "g2"; > > + vpu_g1: video-codec@38300000 { > > + compatible = "nxp,imx8mq-vpu-g1"; > > + reg = <0x38300000 0x10000>; > > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > + power-domains = <&vpu_blk_ctrl > > IMX8MQ_VPUBLK_PD_G1>; > > + }; > > + > > + vpu_g2: video-codec@38310000 { > > + compatible = "nxp,imx8mq-vpu-g2"; > > + reg = <0x38310000 0x10000>; > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > + power-domains = <&vpu_blk_ctrl > > IMX8MQ_VPUBLK_PD_G2>; > > + }; > > + > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > + reg = <0x38320000 0x100>; > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > <&pgc_vpu>; > > + power-domain-names = "bus", "g1", "g2"; > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > - clock-names = "g1", "g2", "bus"; > > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > > - <&clk IMX8MQ_CLK_VPU_G2>, > > - <&clk IMX8MQ_CLK_VPU_BUS>, > > - <&clk > > IMX8MQ_VPU_PLL_BYPASS>; > > - assigned-clock-parents = <&clk > > IMX8MQ_VPU_PLL_OUT>, > > - <&clk > > IMX8MQ_VPU_PLL_OUT>, > > - <&clk > > IMX8MQ_SYS1_PLL_800M>, > > - <&clk > > IMX8MQ_VPU_PLL>; > > - assigned-clock-rates = <600000000>, > > <600000000>, > > - <800000000>, <0>; > > - power-domains = <&pgc_vpu>; > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > + clock-names = "g1", "g2"; > > + #power-domain-cells = <1>; > > }; > > > > pcie0: pcie@33800000 { > > With this update, when testing suspend to ram on imx8mq, I get: > > buck4: failed to disable: -ETIMEDOUT > > where buck4 is power-supply of pgc_vpu. And thus the transition to > suspend (and resuming) fails. > > Have you tested system suspend after the imx8m-blk-ctrl update on > imx8mq? I haven't tested system suspend, don't know if anyone else did. However I guess that this is just uncovering a preexisting issue in the system suspend sequencing, which you would also hit if the video decoders were active at system suspend time. My guess is that the regulator disable fails, due to the power domains being disabled quite late in the suspend sequence, where i2c communication with the PMIC is no longer possible due to i2c being suspended already or something like that. Maybe you can dig in a bit on the actual sequence on your system and we can see how we can rework things to suspend the power domains at a time where communication with the PMIC is still possible? Regards, Lucas
On Mon, Apr 25, 2022 at 10:34 AM Lucas Stach <l.stach@pengutronix.de> wrote: > > Hi Martin, > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > With the Hantro G1 and G2 now setup to run independently, update > > > the device tree to allow both to operate. This requires the > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > certain clock enabled to handle the gating of the G1 and G2 > > > fuses, the clock-parents and clock-rates for the various VPU's > > > to be moved into the pgc_vpu because they cannot get re-parented > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > index 2df2510d0118..549b2440f55d 100644 > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > pgc_vpu: power-domain@6 { > > > #power-domain-cells = > > > <0>; > > > reg = > > > <IMX8M_POWER_DOMAIN_VPU>; > > > - clocks = <&clk > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > + clocks = <&clk > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > + <&clk > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > + <&clk > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > + assigned-clocks = > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > + > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > + > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > + > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > + assigned-clock- > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > + > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > + > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > + > > > <&clk IMX8MQ_VPU_PLL>; > > > + assigned-clock-rates > > > = <600000000>, > > > + > > > <600000000>, > > > + > > > <800000000>, > > > + > > > <0>; > > > }; > > > > > > pgc_disp: power-domain@7 { > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > status = "disabled"; > > > }; > > > > > > - vpu: video-codec@38300000 { > > > - compatible = "nxp,imx8mq-vpu"; > > > - reg = <0x38300000 0x10000>, > > > - <0x38310000 0x10000>, > > > - <0x38320000 0x10000>; > > > - reg-names = "g1", "g2", "ctrl"; > > > - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, > > > - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > > - interrupt-names = "g1", "g2"; > > > + vpu_g1: video-codec@38300000 { > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > + reg = <0x38300000 0x10000>; > > > + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > > + power-domains = <&vpu_blk_ctrl > > > IMX8MQ_VPUBLK_PD_G1>; > > > + }; > > > + > > > + vpu_g2: video-codec@38310000 { > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > + reg = <0x38310000 0x10000>; > > > + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > + power-domains = <&vpu_blk_ctrl > > > IMX8MQ_VPUBLK_PD_G2>; > > > + }; > > > + > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > > + reg = <0x38320000 0x100>; > > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > > <&pgc_vpu>; > > > + power-domain-names = "bus", "g1", "g2"; > > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > > - clock-names = "g1", "g2", "bus"; > > > - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, > > > - <&clk IMX8MQ_CLK_VPU_G2>, > > > - <&clk IMX8MQ_CLK_VPU_BUS>, > > > - <&clk > > > IMX8MQ_VPU_PLL_BYPASS>; > > > - assigned-clock-parents = <&clk > > > IMX8MQ_VPU_PLL_OUT>, > > > - <&clk > > > IMX8MQ_VPU_PLL_OUT>, > > > - <&clk > > > IMX8MQ_SYS1_PLL_800M>, > > > - <&clk > > > IMX8MQ_VPU_PLL>; > > > - assigned-clock-rates = <600000000>, > > > <600000000>, > > > - <800000000>, <0>; > > > - power-domains = <&pgc_vpu>; > > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > + clock-names = "g1", "g2"; > > > + #power-domain-cells = <1>; > > > }; > > > > > > pcie0: pcie@33800000 { > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > buck4: failed to disable: -ETIMEDOUT > > > > where buck4 is power-supply of pgc_vpu. And thus the transition to > > suspend (and resuming) fails. > > > > Have you tested system suspend after the imx8m-blk-ctrl update on > > imx8mq? > > I haven't tested system suspend, don't know if anyone else did. However > I guess that this is just uncovering a preexisting issue in the system > suspend sequencing, which you would also hit if the video decoders were > active at system suspend time. I have not tested it either. > > My guess is that the regulator disable fails, due to the power domains > being disabled quite late in the suspend sequence, where i2c > communication with the PMIC is no longer possible due to i2c being > suspended already or something like that. Maybe you can dig in a bit on > the actual sequence on your system and we can see how we can rework > things to suspend the power domains at a time where communication with > the PMIC is still possible? In the meantime, should we mark the regulator with regulator-always-on so it doesn't attempt to power it down? It might not be ideal,but it might be enough to let it suspend. adam > > Regards, > Lucas >
Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > Hi Martin, > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > With the Hantro G1 and G2 now setup to run independently, update > > > the device tree to allow both to operate. This requires the > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > certain clock enabled to handle the gating of the G1 and G2 > > > fuses, the clock-parents and clock-rates for the various VPU's > > > to be moved into the pgc_vpu because they cannot get re-parented > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > Reported-by: kernel test robot <lkp@intel.com> > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > index 2df2510d0118..549b2440f55d 100644 > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > pgc_vpu: power-domain@6 { > > > #power-domain- > > > cells = > > > <0>; > > > reg = > > > <IMX8M_POWER_DOMAIN_VPU>; > > > - clocks = <&clk > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > + clocks = <&clk > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > + <&clk > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > + <&clk > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > + assigned-clocks = > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > + > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > + > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > + > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > + assigned-clock- > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > + > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > + > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > + > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > + assigned-clock- > > > rates > > > = <600000000>, > > > + > > > > > > <600000000>, > > > + > > > > > > <800000000>, > > > + > > > > > > <0>; > > > }; > > > > > > pgc_disp: power-domain@7 > > > { > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > status = "disabled"; > > > }; > > > > > > - vpu: video-codec@38300000 { > > > - compatible = "nxp,imx8mq-vpu"; > > > - reg = <0x38300000 0x10000>, > > > - <0x38310000 0x10000>, > > > - <0x38320000 0x10000>; > > > - reg-names = "g1", "g2", "ctrl"; > > > - interrupts = <GIC_SPI 7 > > > IRQ_TYPE_LEVEL_HIGH>, > > > - <GIC_SPI 8 > > > IRQ_TYPE_LEVEL_HIGH>; > > > - interrupt-names = "g1", "g2"; > > > + vpu_g1: video-codec@38300000 { > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > + reg = <0x38300000 0x10000>; > > > + interrupts = <GIC_SPI 7 > > > IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > > + power-domains = <&vpu_blk_ctrl > > > IMX8MQ_VPUBLK_PD_G1>; > > > + }; > > > + > > > + vpu_g2: video-codec@38310000 { > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > + reg = <0x38310000 0x10000>; > > > + interrupts = <GIC_SPI 8 > > > IRQ_TYPE_LEVEL_HIGH>; > > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > + power-domains = <&vpu_blk_ctrl > > > IMX8MQ_VPUBLK_PD_G2>; > > > + }; > > > + > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > > + reg = <0x38320000 0x100>; > > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > > <&pgc_vpu>; > > > + power-domain-names = "bus", "g1", "g2"; > > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > > - clock-names = "g1", "g2", "bus"; > > > - assigned-clocks = <&clk > > > IMX8MQ_CLK_VPU_G1>, > > > - <&clk > > > IMX8MQ_CLK_VPU_G2>, > > > - <&clk > > > IMX8MQ_CLK_VPU_BUS>, > > > - <&clk > > > IMX8MQ_VPU_PLL_BYPASS>; > > > - assigned-clock-parents = <&clk > > > IMX8MQ_VPU_PLL_OUT>, > > > - <&clk > > > IMX8MQ_VPU_PLL_OUT>, > > > - <&clk > > > IMX8MQ_SYS1_PLL_800M>, > > > - <&clk > > > IMX8MQ_VPU_PLL>; > > > - assigned-clock-rates = <600000000>, > > > <600000000>, > > > - <800000000>, <0>; > > > - power-domains = <&pgc_vpu>; > > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > + clock-names = "g1", "g2"; > > > + #power-domain-cells = <1>; > > > }; > > > > > > pcie0: pcie@33800000 { > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > buck4: failed to disable: -ETIMEDOUT > > > > where buck4 is power-supply of pgc_vpu. And thus the transition to > > suspend (and resuming) fails. > > > > Have you tested system suspend after the imx8m-blk-ctrl update on > > imx8mq? > > I haven't tested system suspend, don't know if anyone else did. > However > I guess that this is just uncovering a preexisting issue in the > system > suspend sequencing, which you would also hit if the video decoders > were > active at system suspend time. > > My guess is that the regulator disable fails, due to the power > domains > being disabled quite late in the suspend sequence, where i2c > communication with the PMIC is no longer possible due to i2c being > suspended already or something like that. Maybe you can dig in a bit > on > the actual sequence on your system and we can see how we can rework > things to suspend the power domains at a time where communication > with > the PMIC is still possible? What exactly would you like to see? Here's all gpcv2 regulators disabling on suspend. (gpu (domain 5) is disabled by runtime pm often): [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator [ 47.298071] Freezing user space processes ... (elapsed 0.008 seconds) done. [ 47.313432] OOM killer disabled. [ 47.316670] Freezing remaining freezable tasks ... (elapsed 2.221 seconds) done. [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: imx8m_blk_ctrl_suspend start [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator [ 49.819064] buck4: failed to disable: -ETIMEDOUT The stack looks pretty much the same for all of them, from pm_suspend() over genpd_suspend_noiry(). And as a reminder, power-domain nr 0=mipi, 5=gpu, 6=vpu. > > Regards, > Lucas >
Am Montag, dem 25.04.2022 um 10:47 -0500 schrieb Adam Ford: > On Mon, Apr 25, 2022 at 10:34 AM Lucas Stach <l.stach@pengutronix.de> > wrote: > > > > Hi Martin, > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > With the Hantro G1 and G2 now setup to run independently, > > > > update > > > > the device tree to allow both to operate. This requires the > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > fuses, the clock-parents and clock-rates for the various VPU's > > > > to be moved into the pgc_vpu because they cannot get re- > > > > parented > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > index 2df2510d0118..549b2440f55d 100644 > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > pgc_vpu: power-domain@6 > > > > { > > > > #power-domain- > > > > cells = > > > > <0>; > > > > reg = > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > - clocks = <&clk > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > + clocks = <&clk > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > + <&clk > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > + <&clk > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + assigned-clocks > > > > = > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > + > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > + > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > + > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > + assigned-clock- > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > + > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > + > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > + > > > > <&clk IMX8MQ_VPU_PLL>; > > > > + assigned-clock- > > > > rates > > > > = <600000000>, > > > > + > > > > <600000000>, > > > > + > > > > <800000000>, > > > > + > > > > <0>; > > > > }; > > > > > > > > pgc_disp: power- > > > > domain@7 { > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > status = "disabled"; > > > > }; > > > > > > > > - vpu: video-codec@38300000 { > > > > - compatible = "nxp,imx8mq-vpu"; > > > > - reg = <0x38300000 0x10000>, > > > > - <0x38310000 0x10000>, > > > > - <0x38320000 0x10000>; > > > > - reg-names = "g1", "g2", "ctrl"; > > > > - interrupts = <GIC_SPI 7 > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > - <GIC_SPI 8 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > - interrupt-names = "g1", "g2"; > > > > + vpu_g1: video-codec@38300000 { > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > + reg = <0x38300000 0x10000>; > > > > + interrupts = <GIC_SPI 7 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > > > + power-domains = <&vpu_blk_ctrl > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > + }; > > > > + > > > > + vpu_g2: video-codec@38310000 { > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > + reg = <0x38310000 0x10000>; > > > > + interrupts = <GIC_SPI 8 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + power-domains = <&vpu_blk_ctrl > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > + }; > > > > + > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > > > + reg = <0x38320000 0x100>; > > > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > > > <&pgc_vpu>; > > > > + power-domain-names = "bus", "g1", "g2"; > > > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > > > - <&clk > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > - clock-names = "g1", "g2", "bus"; > > > > - assigned-clocks = <&clk > > > > IMX8MQ_CLK_VPU_G1>, > > > > - <&clk > > > > IMX8MQ_CLK_VPU_G2>, > > > > - <&clk > > > > IMX8MQ_CLK_VPU_BUS>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > - assigned-clock-parents = <&clk > > > > IMX8MQ_VPU_PLL_OUT>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL_OUT>, > > > > - <&clk > > > > IMX8MQ_SYS1_PLL_800M>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL>; > > > > - assigned-clock-rates = <600000000>, > > > > <600000000>, > > > > - <800000000>, > > > > <0>; > > > > - power-domains = <&pgc_vpu>; > > > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + clock-names = "g1", "g2"; > > > > + #power-domain-cells = <1>; > > > > }; > > > > > > > > pcie0: pcie@33800000 { > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > to > > > suspend (and resuming) fails. > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update on > > > imx8mq? > > > > I haven't tested system suspend, don't know if anyone else did. > > However > > I guess that this is just uncovering a preexisting issue in the > > system > > suspend sequencing, which you would also hit if the video decoders > > were > > active at system suspend time. > > I have not tested it either. > > > > > My guess is that the regulator disable fails, due to the power > > domains > > being disabled quite late in the suspend sequence, where i2c > > communication with the PMIC is no longer possible due to i2c being > > suspended already or something like that. Maybe you can dig in a > > bit on > > the actual sequence on your system and we can see how we can rework > > things to suspend the power domains at a time where communication > > with > > the PMIC is still possible? > > In the meantime, should we mark the regulator with regulator-always- > on > so it doesn't attempt to power it down? It might not be ideal,but it > might be enough to let it suspend. > it would be a temporary workaround, but I want to remind you that it wouldn't help much: even if suspending "works" again, system resume is broken on imx8mq since https://lore.kernel.org/all/a20ecd639f8e8b7fa4a9bed7a8e9590225262784.camel@puri.sm/ Of course I did the current tests on v5.18-rc4 without any gpcv2 changes to mainline. But for resume to work I need the one revert from the above link (plus a minor additional hack) already. If we'd have that working in mainline I could make sure it stays that way :) martin
Am Dienstag, dem 26.04.2022 um 12:28 +0200 schrieb Martin Kepplinger: > Am Montag, dem 25.04.2022 um 10:47 -0500 schrieb Adam Ford: > > On Mon, Apr 25, 2022 at 10:34 AM Lucas Stach <l.stach@pengutronix.de> > > wrote: > > > > > > Hi Martin, > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > update > > > > > the device tree to allow both to operate. This requires the > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > fuses, the clock-parents and clock-rates for the various VPU's > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > parented > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > pgc_vpu: power-domain@6 > > > > > { > > > > > #power-domain- > > > > > cells = > > > > > <0>; > > > > > reg = > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > - clocks = <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + assigned-clocks > > > > > = > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > + > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > + > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > + > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > + assigned-clock- > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > + > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > + assigned-clock- > > > > > rates > > > > > = <600000000>, > > > > > + > > > > > <600000000>, > > > > > + > > > > > <800000000>, > > > > > + > > > > > <0>; > > > > > }; > > > > > > > > > > pgc_disp: power- > > > > > domain@7 { > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > status = "disabled"; > > > > > }; > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > - reg = <0x38300000 0x10000>, > > > > > - <0x38310000 0x10000>, > > > > > - <0x38320000 0x10000>; > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > - interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > - <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > - interrupt-names = "g1", "g2"; > > > > > + vpu_g1: video-codec@38300000 { > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > + reg = <0x38300000 0x10000>; > > > > > + interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > + }; > > > > > + > > > > > + vpu_g2: video-codec@38310000 { > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > + reg = <0x38310000 0x10000>; > > > > > + interrupts = <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > + }; > > > > > + > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > > > > + reg = <0x38320000 0x100>; > > > > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > > > > <&pgc_vpu>; > > > > > + power-domain-names = "bus", "g1", "g2"; > > > > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > - clock-names = "g1", "g2", "bus"; > > > > > - assigned-clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > - assigned-clock-parents = <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL>; > > > > > - assigned-clock-rates = <600000000>, > > > > > <600000000>, > > > > > - <800000000>, > > > > > <0>; > > > > > - power-domains = <&pgc_vpu>; > > > > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + clock-names = "g1", "g2"; > > > > > + #power-domain-cells = <1>; > > > > > }; > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > to > > > > suspend (and resuming) fails. > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update on > > > > imx8mq? > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > However > > > I guess that this is just uncovering a preexisting issue in the > > > system > > > suspend sequencing, which you would also hit if the video decoders > > > were > > > active at system suspend time. > > > > I have not tested it either. > > > > > > > > My guess is that the regulator disable fails, due to the power > > > domains > > > being disabled quite late in the suspend sequence, where i2c > > > communication with the PMIC is no longer possible due to i2c being > > > suspended already or something like that. Maybe you can dig in a > > > bit on > > > the actual sequence on your system and we can see how we can rework > > > things to suspend the power domains at a time where communication > > > with > > > the PMIC is still possible? > > > > In the meantime, should we mark the regulator with regulator-always- > > on > > so it doesn't attempt to power it down? It might not be ideal,but it > > might be enough to let it suspend. > > > > it would be a temporary workaround, but I want to remind you that it > wouldn't help much: even if suspending "works" again, system resume is > broken on imx8mq since > https://lore.kernel.org/all/a20ecd639f8e8b7fa4a9bed7a8e9590225262784.camel@puri.sm/ > > Of course I did the current tests on v5.18-rc4 without any gpcv2 > changes to mainline. But for resume to work I need the one revert from > the above link (plus a minor additional hack) already. > > If we'd have that working in mainline I could make sure it stays that > way :) > This looks like the same underlying issue to me. We need to suspend the power domains before i2c (or whatever is causing the regulator disable fail) and only resume them after this subsystem is resumed. Regards, Lucas
Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > Hi Martin, > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger: > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > With the Hantro G1 and G2 now setup to run independently, update > > > > the device tree to allow both to operate. This requires the > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > fuses, the clock-parents and clock-rates for the various VPU's > > > > to be moved into the pgc_vpu because they cannot get re-parented > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > index 2df2510d0118..549b2440f55d 100644 > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > pgc_vpu: power-domain@6 { > > > > #power-domain- > > > > cells = > > > > <0>; > > > > reg = > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > - clocks = <&clk > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > + clocks = <&clk > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > + <&clk > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > + <&clk > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + assigned-clocks = > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > + > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > + > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > + > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > + assigned-clock- > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > + > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > + > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > + > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > + assigned-clock- > > > > rates > > > > = <600000000>, > > > > + > > > > > > > > <600000000>, > > > > + > > > > > > > > <800000000>, > > > > + > > > > > > > > <0>; > > > > }; > > > > > > > > pgc_disp: power-domain@7 > > > > { > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > status = "disabled"; > > > > }; > > > > > > > > - vpu: video-codec@38300000 { > > > > - compatible = "nxp,imx8mq-vpu"; > > > > - reg = <0x38300000 0x10000>, > > > > - <0x38310000 0x10000>, > > > > - <0x38320000 0x10000>; > > > > - reg-names = "g1", "g2", "ctrl"; > > > > - interrupts = <GIC_SPI 7 > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > - <GIC_SPI 8 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > - interrupt-names = "g1", "g2"; > > > > + vpu_g1: video-codec@38300000 { > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > + reg = <0x38300000 0x10000>; > > > > + interrupts = <GIC_SPI 7 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; > > > > + power-domains = <&vpu_blk_ctrl > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > + }; > > > > + > > > > + vpu_g2: video-codec@38310000 { > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > + reg = <0x38310000 0x10000>; > > > > + interrupts = <GIC_SPI 8 > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + power-domains = <&vpu_blk_ctrl > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > + }; > > > > + > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > + compatible = "fsl,imx8mq-vpu-blk-ctrl"; > > > > + reg = <0x38320000 0x100>; > > > > + power-domains = <&pgc_vpu>, <&pgc_vpu>, > > > > <&pgc_vpu>; > > > > + power-domain-names = "bus", "g1", "g2"; > > > > clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, > > > > - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, > > > > - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > - clock-names = "g1", "g2", "bus"; > > > > - assigned-clocks = <&clk > > > > IMX8MQ_CLK_VPU_G1>, > > > > - <&clk > > > > IMX8MQ_CLK_VPU_G2>, > > > > - <&clk > > > > IMX8MQ_CLK_VPU_BUS>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > - assigned-clock-parents = <&clk > > > > IMX8MQ_VPU_PLL_OUT>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL_OUT>, > > > > - <&clk > > > > IMX8MQ_SYS1_PLL_800M>, > > > > - <&clk > > > > IMX8MQ_VPU_PLL>; > > > > - assigned-clock-rates = <600000000>, > > > > <600000000>, > > > > - <800000000>, <0>; > > > > - power-domains = <&pgc_vpu>; > > > > + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; > > > > + clock-names = "g1", "g2"; > > > > + #power-domain-cells = <1>; > > > > }; > > > > > > > > pcie0: pcie@33800000 { > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition to > > > suspend (and resuming) fails. > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update on > > > imx8mq? > > > > I haven't tested system suspend, don't know if anyone else did. > > However > > I guess that this is just uncovering a preexisting issue in the > > system > > suspend sequencing, which you would also hit if the video decoders > > were > > active at system suspend time. > > > > My guess is that the regulator disable fails, due to the power > > domains > > being disabled quite late in the suspend sequence, where i2c > > communication with the PMIC is no longer possible due to i2c being > > suspended already or something like that. Maybe you can dig in a bit > > on > > the actual sequence on your system and we can see how we can rework > > things to suspend the power domains at a time where communication > > with > > the PMIC is still possible? > > What exactly would you like to see? Here's all gpcv2 regulators > disabling on suspend. (gpu (domain 5) is disabled by runtime pm often): > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > seconds) done. > [ 47.313432] OOM killer disabled. > [ 47.316670] Freezing remaining freezable tasks ... (elapsed 2.221 > seconds) done. > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: imx8m_blk_ctrl_suspend > start > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > The stack looks pretty much the same for all of them, from pm_suspend() > over genpd_suspend_noiry(). So the GPU domain is already suspended before the system suspend, probably due to short runtime PM timeouts. Can you please check at which point the i2c subsystem is suspended? I think we are already past that point when running the PM domain suspend from a _noirq callback. I'll take a look on how we can properly change this ordering. Regards, Lucas
Am Dienstag, dem 26.04.2022 um 12:43 +0200 schrieb Lucas Stach: > Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > > Hi Martin, > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin > > > Kepplinger: > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > update > > > > > the device tree to allow both to operate. This requires the > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > fuses, the clock-parents and clock-rates for the various > > > > > VPU's > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > parented > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > pgc_vpu: power- > > > > > domain@6 { > > > > > #power- > > > > > domain- > > > > > cells = > > > > > <0>; > > > > > reg = > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > - clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > + clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + assigned- > > > > > clocks = > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > + assigned- > > > > > clock- > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > + assigned- > > > > > clock- > > > > > rates > > > > > = <600000000>, > > > > > + > > > > > > > > > > > > > > > <600000000>, > > > > > + > > > > > > > > > > > > > > > <800000000>, > > > > > + > > > > > > > > > > > > > > > <0>; > > > > > }; > > > > > > > > > > pgc_disp: > > > > > power-domain@7 > > > > > { > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > status = "disabled"; > > > > > }; > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > - reg = <0x38300000 0x10000>, > > > > > - <0x38310000 0x10000>, > > > > > - <0x38320000 0x10000>; > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > - interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > - <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > - interrupt-names = "g1", "g2"; > > > > > + vpu_g1: video-codec@38300000 { > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > + reg = <0x38300000 0x10000>; > > > > > + interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > + }; > > > > > + > > > > > + vpu_g2: video-codec@38310000 { > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > + reg = <0x38310000 0x10000>; > > > > > + interrupts = <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > + }; > > > > > + > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > + compatible = "fsl,imx8mq-vpu-blk- > > > > > ctrl"; > > > > > + reg = <0x38320000 0x100>; > > > > > + power-domains = <&pgc_vpu>, > > > > > <&pgc_vpu>, > > > > > <&pgc_vpu>; > > > > > + power-domain-names = "bus", "g1", > > > > > "g2"; > > > > > clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > - clock-names = "g1", "g2", "bus"; > > > > > - assigned-clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > - assigned-clock-parents = <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL>; > > > > > - assigned-clock-rates = <600000000>, > > > > > <600000000>, > > > > > - <800000000>, > > > > > <0>; > > > > > - power-domains = <&pgc_vpu>; > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + clock-names = "g1", "g2"; > > > > > + #power-domain-cells = <1>; > > > > > }; > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > to > > > > suspend (and resuming) fails. > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update > > > > on > > > > imx8mq? > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > However > > > I guess that this is just uncovering a preexisting issue in the > > > system > > > suspend sequencing, which you would also hit if the video > > > decoders > > > were > > > active at system suspend time. > > > > > > My guess is that the regulator disable fails, due to the power > > > domains > > > being disabled quite late in the suspend sequence, where i2c > > > communication with the PMIC is no longer possible due to i2c > > > being > > > suspended already or something like that. Maybe you can dig in a > > > bit > > > on > > > the actual sequence on your system and we can see how we can > > > rework > > > things to suspend the power domains at a time where communication > > > with > > > the PMIC is still possible? > > > > What exactly would you like to see? Here's all gpcv2 regulators > > disabling on suspend. (gpu (domain 5) is disabled by runtime pm > > often): > > > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > > seconds) done. > > [ 47.313432] OOM killer disabled. > > [ 47.316670] Freezing remaining freezable tasks ... (elapsed > > 2.221 > > seconds) done. > > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: > > imx8m_blk_ctrl_suspend > > start > > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > > > The stack looks pretty much the same for all of them, from > > pm_suspend() > > over genpd_suspend_noiry(). > > So the GPU domain is already suspended before the system suspend, > probably due to short runtime PM timeouts. > > Can you please check at which point the i2c subsystem is suspended? I > think we are already past that point when running the PM domain > suspend > from a _noirq callback. I'll take a look on how we can properly > change > this ordering. > > Regards, > Lucas > not sure whether I correctly check that, but the last i2c_imx_runtime_suspend() (here for i2c1 and i2c3) are executed before the power-domain disable, so that makes sense(?): [ 40.774853] imx-pgc imx-pgc-domain.5: disable regulator now [ 40.786211] imx-i2c 30a20000.i2c: i2c_imx_runtime_suspend [ 40.910189] imx-i2c 30a40000.i2c: i2c_imx_runtime_suspend [ 40.940895] Freezing user space processes ... [ 40.968914] rfkill: input handler enabled [ 40.979955] (elapsed 0.007 seconds) done. [ 40.984033] OOM killer disabled. [ 40.987297] Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done. [ 41.264683] imx-pgc imx-pgc-domain.0: disable regulator now [ 41.271679] imx-pgc imx-pgc-domain.6: disable regulator now [ 41.378759] buck4: failed to disable: -ETIMEDOUT [ 41.383405] imx-pgc imx-pgc-domain.6: failed to disable regulator: - 110 And you can see that I have the domain.0 regulator set to always-on. Otherwise it would time out as well in this case: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi#n900 thank a lot for taking the time to look at this! martin
Am Dienstag, dem 26.04.2022 um 12:43 +0200 schrieb Lucas Stach: > Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > > Hi Martin, > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin > > > Kepplinger: > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > update > > > > > the device tree to allow both to operate. This requires the > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > fuses, the clock-parents and clock-rates for the various > > > > > VPU's > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > parented > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > pgc_vpu: power- > > > > > domain@6 { > > > > > #power- > > > > > domain- > > > > > cells = > > > > > <0>; > > > > > reg = > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > - clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > + clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + assigned- > > > > > clocks = > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > + assigned- > > > > > clock- > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > + assigned- > > > > > clock- > > > > > rates > > > > > = <600000000>, > > > > > + > > > > > > > > > > > > > > > <600000000>, > > > > > + > > > > > > > > > > > > > > > <800000000>, > > > > > + > > > > > > > > > > > > > > > <0>; > > > > > }; > > > > > > > > > > pgc_disp: > > > > > power-domain@7 > > > > > { > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > status = "disabled"; > > > > > }; > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > - reg = <0x38300000 0x10000>, > > > > > - <0x38310000 0x10000>, > > > > > - <0x38320000 0x10000>; > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > - interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > - <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > - interrupt-names = "g1", "g2"; > > > > > + vpu_g1: video-codec@38300000 { > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > + reg = <0x38300000 0x10000>; > > > > > + interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > + }; > > > > > + > > > > > + vpu_g2: video-codec@38310000 { > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > + reg = <0x38310000 0x10000>; > > > > > + interrupts = <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > + }; > > > > > + > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > + compatible = "fsl,imx8mq-vpu-blk- > > > > > ctrl"; > > > > > + reg = <0x38320000 0x100>; > > > > > + power-domains = <&pgc_vpu>, > > > > > <&pgc_vpu>, > > > > > <&pgc_vpu>; > > > > > + power-domain-names = "bus", "g1", > > > > > "g2"; > > > > > clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > - clock-names = "g1", "g2", "bus"; > > > > > - assigned-clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > - assigned-clock-parents = <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL>; > > > > > - assigned-clock-rates = <600000000>, > > > > > <600000000>, > > > > > - <800000000>, > > > > > <0>; > > > > > - power-domains = <&pgc_vpu>; > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + clock-names = "g1", "g2"; > > > > > + #power-domain-cells = <1>; > > > > > }; > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > to > > > > suspend (and resuming) fails. > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update > > > > on > > > > imx8mq? > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > However > > > I guess that this is just uncovering a preexisting issue in the > > > system > > > suspend sequencing, which you would also hit if the video > > > decoders > > > were > > > active at system suspend time. > > > > > > My guess is that the regulator disable fails, due to the power > > > domains > > > being disabled quite late in the suspend sequence, where i2c > > > communication with the PMIC is no longer possible due to i2c > > > being > > > suspended already or something like that. Maybe you can dig in a > > > bit > > > on > > > the actual sequence on your system and we can see how we can > > > rework > > > things to suspend the power domains at a time where communication > > > with > > > the PMIC is still possible? > > > > What exactly would you like to see? Here's all gpcv2 regulators > > disabling on suspend. (gpu (domain 5) is disabled by runtime pm > > often): > > > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > > seconds) done. > > [ 47.313432] OOM killer disabled. > > [ 47.316670] Freezing remaining freezable tasks ... (elapsed > > 2.221 > > seconds) done. > > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: > > imx8m_blk_ctrl_suspend > > start > > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > > > The stack looks pretty much the same for all of them, from > > pm_suspend() > > over genpd_suspend_noiry(). > > So the GPU domain is already suspended before the system suspend, > probably due to short runtime PM timeouts. > > Can you please check at which point the i2c subsystem is suspended? I > think we are already past that point when running the PM domain > suspend > from a _noirq callback. I'll take a look on how we can properly > change > this ordering. hi Lucas, when I just add the same system suspend hook you added to gpcv2 (pm_runtime_get()), I get the same timeout: [ 366.100154] Freezing remaining freezable tasks ... (elapsed 2.863 seconds) done. [ 369.228387] imx-i2c 30a50000.i2c: pm_runtime get. [ 369.234204] imx-i2c 30a40000.i2c: pm_runtime get. [ 369.239834] imx-i2c 30a30000.i2c: pm_runtime get. [ 369.247417] imx-i2c 30a20000.i2c: pm_runtime get. [ 369.277838] imx-pgc imx-pgc-domain.0: disable regulator now [ 369.284758] imx-pgc imx-pgc-domain.6: disable regulator now [ 369.394954] buck4: failed to disable: -ETIMEDOUT do you have an idea? martin
Am Dienstag, dem 26.04.2022 um 12:43 +0200 schrieb Lucas Stach: > Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > > Hi Martin, > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin > > > Kepplinger: > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > update > > > > > the device tree to allow both to operate. This requires the > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > fuses, the clock-parents and clock-rates for the various > > > > > VPU's > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > parented > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > pgc_vpu: power- > > > > > domain@6 { > > > > > #power- > > > > > domain- > > > > > cells = > > > > > <0>; > > > > > reg = > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > - clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > + clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + assigned- > > > > > clocks = > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > + assigned- > > > > > clock- > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > + assigned- > > > > > clock- > > > > > rates > > > > > = <600000000>, > > > > > + > > > > > > > > > > > > > > > <600000000>, > > > > > + > > > > > > > > > > > > > > > <800000000>, > > > > > + > > > > > > > > > > > > > > > <0>; > > > > > }; > > > > > > > > > > pgc_disp: > > > > > power-domain@7 > > > > > { > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > status = "disabled"; > > > > > }; > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > - reg = <0x38300000 0x10000>, > > > > > - <0x38310000 0x10000>, > > > > > - <0x38320000 0x10000>; > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > - interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > - <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > - interrupt-names = "g1", "g2"; > > > > > + vpu_g1: video-codec@38300000 { > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > + reg = <0x38300000 0x10000>; > > > > > + interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > + }; > > > > > + > > > > > + vpu_g2: video-codec@38310000 { > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > + reg = <0x38310000 0x10000>; > > > > > + interrupts = <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > + }; > > > > > + > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > + compatible = "fsl,imx8mq-vpu-blk- > > > > > ctrl"; > > > > > + reg = <0x38320000 0x100>; > > > > > + power-domains = <&pgc_vpu>, > > > > > <&pgc_vpu>, > > > > > <&pgc_vpu>; > > > > > + power-domain-names = "bus", "g1", > > > > > "g2"; > > > > > clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > - clock-names = "g1", "g2", "bus"; > > > > > - assigned-clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > - assigned-clock-parents = <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL>; > > > > > - assigned-clock-rates = <600000000>, > > > > > <600000000>, > > > > > - <800000000>, > > > > > <0>; > > > > > - power-domains = <&pgc_vpu>; > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + clock-names = "g1", "g2"; > > > > > + #power-domain-cells = <1>; > > > > > }; > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > to > > > > suspend (and resuming) fails. > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update > > > > on > > > > imx8mq? > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > However > > > I guess that this is just uncovering a preexisting issue in the > > > system > > > suspend sequencing, which you would also hit if the video > > > decoders > > > were > > > active at system suspend time. > > > > > > My guess is that the regulator disable fails, due to the power > > > domains > > > being disabled quite late in the suspend sequence, where i2c > > > communication with the PMIC is no longer possible due to i2c > > > being > > > suspended already or something like that. Maybe you can dig in a > > > bit > > > on > > > the actual sequence on your system and we can see how we can > > > rework > > > things to suspend the power domains at a time where communication > > > with > > > the PMIC is still possible? > > > > What exactly would you like to see? Here's all gpcv2 regulators > > disabling on suspend. (gpu (domain 5) is disabled by runtime pm > > often): > > > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > > seconds) done. > > [ 47.313432] OOM killer disabled. > > [ 47.316670] Freezing remaining freezable tasks ... (elapsed > > 2.221 > > seconds) done. > > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: > > imx8m_blk_ctrl_suspend > > start > > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > > > The stack looks pretty much the same for all of them, from > > pm_suspend() > > over genpd_suspend_noiry(). > > So the GPU domain is already suspended before the system suspend, > probably due to short runtime PM timeouts. > > Can you please check at which point the i2c subsystem is suspended? I > think we are already past that point when running the PM domain > suspend > from a _noirq callback. I'll take a look on how we can properly > change > this ordering. > > Regards, > Lucas > hi Lucas, just to update a bit: (I rmmod hantro_vpu because it adds another bug but let's ignore for now) when I use a mainline kernel and for testing just ignore ETIMEDOUT in gpcv2 on regulator_disable(): - return ret; + if (ret == -ETIMEDOUT) + ret = 0; + else + return ret; I see the error message the first time I suspend. Not the consecutive times (and the systems seems to be working after resuming). Now what's weird is when I suspend and resume often, after some time, resume will definitely fail. Not sure yet where, but when I add debug output for the kernel/ directory, that's usually about where it hangs: [ 969.620825] OOM killer enabled. [ 969.623971] Restarting tasks ... [ 969.624230] systemd-udevd left refrigerator [ 969.624535] khugepaged left refrigerator [ 969.624568] gdbus left refrigerator [ 969.624795] alsa-sink-308b0 left refrigerator [ 969.626864] pipewire left refrigerator [ 969.626880] alsa-source-308 left refrigerator [ 969.626913] alsa-source-300 left refrigerator [ 969.626920] gmain left refrigerator [ 969.626949] gvfs-afc-volume left refrigerator [ 969.626949] systemd left refrigerator [ 969.626977] gmain left refrigerator [ 969.627009] chatty left refrigerator [ 969.627036] gmain left refrigerator [ 969.627060] gmain left refrigerator [ 969.627087] gmain left refrigerator And the above hang ("sometimes") happens for "platform" in pm_test already, so I guess it's purely a linux bug. just so you know, in case that tells you anything more... thanks, martin
Am Dienstag, dem 26.04.2022 um 12:43 +0200 schrieb Lucas Stach: > Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > > Hi Martin, > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin > > > Kepplinger: > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > update > > > > > the device tree to allow both to operate. This requires the > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > fuses, the clock-parents and clock-rates for the various > > > > > VPU's > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > parented > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > pgc_vpu: power- > > > > > domain@6 { > > > > > #power- > > > > > domain- > > > > > cells = > > > > > <0>; > > > > > reg = > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > - clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > + clocks = > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > + > > > > > <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + assigned- > > > > > clocks = > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > + > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > + assigned- > > > > > clock- > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > + > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > + assigned- > > > > > clock- > > > > > rates > > > > > = <600000000>, > > > > > + > > > > > > > > > > > > > > > <600000000>, > > > > > + > > > > > > > > > > > > > > > <800000000>, > > > > > + > > > > > > > > > > > > > > > <0>; > > > > > }; > > > > > > > > > > pgc_disp: > > > > > power-domain@7 > > > > > { > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > status = "disabled"; > > > > > }; > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > - reg = <0x38300000 0x10000>, > > > > > - <0x38310000 0x10000>, > > > > > - <0x38320000 0x10000>; > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > - interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > - <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > - interrupt-names = "g1", "g2"; > > > > > + vpu_g1: video-codec@38300000 { > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > + reg = <0x38300000 0x10000>; > > > > > + interrupts = <GIC_SPI 7 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > + }; > > > > > + > > > > > + vpu_g2: video-codec@38310000 { > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > + reg = <0x38310000 0x10000>; > > > > > + interrupts = <GIC_SPI 8 > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > + clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + power-domains = <&vpu_blk_ctrl > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > + }; > > > > > + > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > + compatible = "fsl,imx8mq-vpu-blk- > > > > > ctrl"; > > > > > + reg = <0x38320000 0x100>; > > > > > + power-domains = <&pgc_vpu>, > > > > > <&pgc_vpu>, > > > > > <&pgc_vpu>; > > > > > + power-domain-names = "bus", "g1", > > > > > "g2"; > > > > > clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > - clock-names = "g1", "g2", "bus"; > > > > > - assigned-clocks = <&clk > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > - <&clk > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > - assigned-clock-parents = <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > - <&clk > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > - <&clk > > > > > IMX8MQ_VPU_PLL>; > > > > > - assigned-clock-rates = <600000000>, > > > > > <600000000>, > > > > > - <800000000>, > > > > > <0>; > > > > > - power-domains = <&pgc_vpu>; > > > > > + <&clk > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > + clock-names = "g1", "g2"; > > > > > + #power-domain-cells = <1>; > > > > > }; > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > to > > > > suspend (and resuming) fails. > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update > > > > on > > > > imx8mq? > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > However > > > I guess that this is just uncovering a preexisting issue in the > > > system > > > suspend sequencing, which you would also hit if the video > > > decoders > > > were > > > active at system suspend time. > > > > > > My guess is that the regulator disable fails, due to the power > > > domains > > > being disabled quite late in the suspend sequence, where i2c > > > communication with the PMIC is no longer possible due to i2c > > > being > > > suspended already or something like that. Maybe you can dig in a > > > bit > > > on > > > the actual sequence on your system and we can see how we can > > > rework > > > things to suspend the power domains at a time where communication > > > with > > > the PMIC is still possible? > > > > What exactly would you like to see? Here's all gpcv2 regulators > > disabling on suspend. (gpu (domain 5) is disabled by runtime pm > > often): > > > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > > seconds) done. > > [ 47.313432] OOM killer disabled. > > [ 47.316670] Freezing remaining freezable tasks ... (elapsed > > 2.221 > > seconds) done. > > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: > > imx8m_blk_ctrl_suspend > > start > > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > > > The stack looks pretty much the same for all of them, from > > pm_suspend() > > over genpd_suspend_noiry(). > > So the GPU domain is already suspended before the system suspend, > probably due to short runtime PM timeouts. > > Can you please check at which point the i2c subsystem is suspended? I > think we are already past that point when running the PM domain > suspend > from a _noirq callback. I'll take a look on how we can properly > change > this ordering. > > Regards, > Lucas > hi Lucas, sorry for not following up on this for so long. This fixes suspend/resume for me: https://lore.kernel.org/linux-arm-kernel/20220711094549.3445566-1-martin.kepplinger@puri.sm/T/#t thank you for you help so far, martin
Hi Martin, On Mon, Jul 11, 2022 at 6:53 AM Martin Kepplinger <martin.kepplinger@puri.sm> wrote: > > Am Dienstag, dem 26.04.2022 um 12:43 +0200 schrieb Lucas Stach: > > Am Dienstag, dem 26.04.2022 um 09:38 +0200 schrieb Martin Kepplinger: > > > Am Montag, dem 25.04.2022 um 17:34 +0200 schrieb Lucas Stach: > > > > Hi Martin, > > > > > > > > Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin > > > > Kepplinger: > > > > > Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford: > > > > > > With the Hantro G1 and G2 now setup to run independently, > > > > > > update > > > > > > the device tree to allow both to operate. This requires the > > > > > > vpu-blk-ctrl node to be configured. Since vpu-blk-ctrl needs > > > > > > certain clock enabled to handle the gating of the G1 and G2 > > > > > > fuses, the clock-parents and clock-rates for the various > > > > > > VPU's > > > > > > to be moved into the pgc_vpu because they cannot get re- > > > > > > parented > > > > > > once enabled, and the pgc_vpu is the highest in the chain. > > > > > > > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > > Reported-by: kernel test robot <lkp@intel.com> > > > > > > Reviewed-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > > > > > > > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > > index 2df2510d0118..549b2440f55d 100644 > > > > > > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi > > > > > > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { > > > > > > pgc_vpu: power- > > > > > > domain@6 { > > > > > > #power- > > > > > > domain- > > > > > > cells = > > > > > > <0>; > > > > > > reg = > > > > > > <IMX8M_POWER_DOMAIN_VPU>; > > > > > > - clocks = > > > > > > <&clk > > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > > + clocks = > > > > > > <&clk > > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>, > > > > > > + > > > > > > <&clk > > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > > + > > > > > > <&clk > > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > > + assigned- > > > > > > clocks = > > > > > > <&clk IMX8MQ_CLK_VPU_G1>, > > > > > > + > > > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_G2>, > > > > > > + > > > > > > > > > > > > <&clk IMX8MQ_CLK_VPU_BUS>, > > > > > > + > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_BYPASS>; > > > > > > + assigned- > > > > > > clock- > > > > > > parents = <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > > + > > > > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL_OUT>, > > > > > > + > > > > > > > > > > > > > > > > > > <&clk IMX8MQ_SYS1_PLL_800M>, > > > > > > + > > > > > > > > > > > > > > > > > > <&clk IMX8MQ_VPU_PLL>; > > > > > > + assigned- > > > > > > clock- > > > > > > rates > > > > > > = <600000000>, > > > > > > + > > > > > > > > > > > > > > > > > > <600000000>, > > > > > > + > > > > > > > > > > > > > > > > > > <800000000>, > > > > > > + > > > > > > > > > > > > > > > > > > <0>; > > > > > > }; > > > > > > > > > > > > pgc_disp: > > > > > > power-domain@7 > > > > > > { > > > > > > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { > > > > > > status = "disabled"; > > > > > > }; > > > > > > > > > > > > - vpu: video-codec@38300000 { > > > > > > - compatible = "nxp,imx8mq-vpu"; > > > > > > - reg = <0x38300000 0x10000>, > > > > > > - <0x38310000 0x10000>, > > > > > > - <0x38320000 0x10000>; > > > > > > - reg-names = "g1", "g2", "ctrl"; > > > > > > - interrupts = <GIC_SPI 7 > > > > > > IRQ_TYPE_LEVEL_HIGH>, > > > > > > - <GIC_SPI 8 > > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > > - interrupt-names = "g1", "g2"; > > > > > > + vpu_g1: video-codec@38300000 { > > > > > > + compatible = "nxp,imx8mq-vpu-g1"; > > > > > > + reg = <0x38300000 0x10000>; > > > > > > + interrupts = <GIC_SPI 7 > > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > > + clocks = <&clk > > > > > > IMX8MQ_CLK_VPU_G1_ROOT>; > > > > > > + power-domains = <&vpu_blk_ctrl > > > > > > IMX8MQ_VPUBLK_PD_G1>; > > > > > > + }; > > > > > > + > > > > > > + vpu_g2: video-codec@38310000 { > > > > > > + compatible = "nxp,imx8mq-vpu-g2"; > > > > > > + reg = <0x38310000 0x10000>; > > > > > > + interrupts = <GIC_SPI 8 > > > > > > IRQ_TYPE_LEVEL_HIGH>; > > > > > > + clocks = <&clk > > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > > + power-domains = <&vpu_blk_ctrl > > > > > > IMX8MQ_VPUBLK_PD_G2>; > > > > > > + }; > > > > > > + > > > > > > + vpu_blk_ctrl: blk-ctrl@38320000 { > > > > > > + compatible = "fsl,imx8mq-vpu-blk- > > > > > > ctrl"; > > > > > > + reg = <0x38320000 0x100>; > > > > > > + power-domains = <&pgc_vpu>, > > > > > > <&pgc_vpu>, > > > > > > <&pgc_vpu>; > > > > > > + power-domain-names = "bus", "g1", > > > > > > "g2"; > > > > > > clocks = <&clk > > > > > > IMX8MQ_CLK_VPU_G1_ROOT>, > > > > > > - <&clk > > > > > > IMX8MQ_CLK_VPU_G2_ROOT>, > > > > > > - <&clk > > > > > > IMX8MQ_CLK_VPU_DEC_ROOT>; > > > > > > - clock-names = "g1", "g2", "bus"; > > > > > > - assigned-clocks = <&clk > > > > > > IMX8MQ_CLK_VPU_G1>, > > > > > > - <&clk > > > > > > IMX8MQ_CLK_VPU_G2>, > > > > > > - <&clk > > > > > > IMX8MQ_CLK_VPU_BUS>, > > > > > > - <&clk > > > > > > IMX8MQ_VPU_PLL_BYPASS>; > > > > > > - assigned-clock-parents = <&clk > > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > > - <&clk > > > > > > IMX8MQ_VPU_PLL_OUT>, > > > > > > - <&clk > > > > > > IMX8MQ_SYS1_PLL_800M>, > > > > > > - <&clk > > > > > > IMX8MQ_VPU_PLL>; > > > > > > - assigned-clock-rates = <600000000>, > > > > > > <600000000>, > > > > > > - <800000000>, > > > > > > <0>; > > > > > > - power-domains = <&pgc_vpu>; > > > > > > + <&clk > > > > > > IMX8MQ_CLK_VPU_G2_ROOT>; > > > > > > + clock-names = "g1", "g2"; > > > > > > + #power-domain-cells = <1>; > > > > > > }; > > > > > > > > > > > > pcie0: pcie@33800000 { > > > > > > > > > > With this update, when testing suspend to ram on imx8mq, I get: > > > > > > > > > > buck4: failed to disable: -ETIMEDOUT > > > > > > > > > > where buck4 is power-supply of pgc_vpu. And thus the transition > > > > > to > > > > > suspend (and resuming) fails. > > > > > > > > > > Have you tested system suspend after the imx8m-blk-ctrl update > > > > > on > > > > > imx8mq? > > > > > > > > I haven't tested system suspend, don't know if anyone else did. > > > > However > > > > I guess that this is just uncovering a preexisting issue in the > > > > system > > > > suspend sequencing, which you would also hit if the video > > > > decoders > > > > were > > > > active at system suspend time. > > > > > > > > My guess is that the regulator disable fails, due to the power > > > > domains > > > > being disabled quite late in the suspend sequence, where i2c > > > > communication with the PMIC is no longer possible due to i2c > > > > being > > > > suspended already or something like that. Maybe you can dig in a > > > > bit > > > > on > > > > the actual sequence on your system and we can see how we can > > > > rework > > > > things to suspend the power domains at a time where communication > > > > with > > > > the PMIC is still possible? > > > > > > What exactly would you like to see? Here's all gpcv2 regulators > > > disabling on suspend. (gpu (domain 5) is disabled by runtime pm > > > often): > > > > > > [ 47.138700] imx-pgc imx-pgc-domain.5: disabling regulator > > > [ 47.298071] Freezing user space processes ... (elapsed 0.008 > > > seconds) done. > > > [ 47.313432] OOM killer disabled. > > > [ 47.316670] Freezing remaining freezable tasks ... (elapsed > > > 2.221 > > > seconds) done. > > > [ 49.672052] imx8m-blk-ctrl 38320000.blk-ctrl: > > > imx8m_blk_ctrl_suspend > > > start > > > [ 49.704417] imx-pgc imx-pgc-domain.0: disabling regulator > > > [ 49.711114] imx-pgc imx-pgc-domain.6: disabling regulator > > > [ 49.819064] buck4: failed to disable: -ETIMEDOUT > > > > > > The stack looks pretty much the same for all of them, from > > > pm_suspend() > > > over genpd_suspend_noiry(). > > > > So the GPU domain is already suspended before the system suspend, > > probably due to short runtime PM timeouts. > > > > Can you please check at which point the i2c subsystem is suspended? I > > think we are already past that point when running the PM domain > > suspend > > from a _noirq callback. I'll take a look on how we can properly > > change > > this ordering. > > > > Regards, > > Lucas > > > > hi Lucas, sorry for not following up on this for so long. This fixes > suspend/resume for me: > > https://lore.kernel.org/linux-arm-kernel/20220711094549.3445566-1-martin.kepplinger@puri.sm/T/#t > > thank you for you help so far, > Thanks a lot for keeping us posted. The fix for suspend/resume looks great! Ezequiel
diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi index 2df2510d0118..549b2440f55d 100644 --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 { pgc_vpu: power-domain@6 { #power-domain-cells = <0>; reg = <IMX8M_POWER_DOMAIN_VPU>; - clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; + clocks = <&clk IMX8MQ_CLK_VPU_DEC_ROOT>, + <&clk IMX8MQ_CLK_VPU_G1_ROOT>, + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; + assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, + <&clk IMX8MQ_CLK_VPU_G2>, + <&clk IMX8MQ_CLK_VPU_BUS>, + <&clk IMX8MQ_VPU_PLL_BYPASS>; + assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, + <&clk IMX8MQ_VPU_PLL_OUT>, + <&clk IMX8MQ_SYS1_PLL_800M>, + <&clk IMX8MQ_VPU_PLL>; + assigned-clock-rates = <600000000>, + <600000000>, + <800000000>, + <0>; }; pgc_disp: power-domain@7 { @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@382f0040 { status = "disabled"; }; - vpu: video-codec@38300000 { - compatible = "nxp,imx8mq-vpu"; - reg = <0x38300000 0x10000>, - <0x38310000 0x10000>, - <0x38320000 0x10000>; - reg-names = "g1", "g2", "ctrl"; - interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>, - <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; - interrupt-names = "g1", "g2"; + vpu_g1: video-codec@38300000 { + compatible = "nxp,imx8mq-vpu-g1"; + reg = <0x38300000 0x10000>; + interrupts = <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>; + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G1>; + }; + + vpu_g2: video-codec@38310000 { + compatible = "nxp,imx8mq-vpu-g2"; + reg = <0x38310000 0x10000>; + interrupts = <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk IMX8MQ_CLK_VPU_G2_ROOT>; + power-domains = <&vpu_blk_ctrl IMX8MQ_VPUBLK_PD_G2>; + }; + + vpu_blk_ctrl: blk-ctrl@38320000 { + compatible = "fsl,imx8mq-vpu-blk-ctrl"; + reg = <0x38320000 0x100>; + power-domains = <&pgc_vpu>, <&pgc_vpu>, <&pgc_vpu>; + power-domain-names = "bus", "g1", "g2"; clocks = <&clk IMX8MQ_CLK_VPU_G1_ROOT>, - <&clk IMX8MQ_CLK_VPU_G2_ROOT>, - <&clk IMX8MQ_CLK_VPU_DEC_ROOT>; - clock-names = "g1", "g2", "bus"; - assigned-clocks = <&clk IMX8MQ_CLK_VPU_G1>, - <&clk IMX8MQ_CLK_VPU_G2>, - <&clk IMX8MQ_CLK_VPU_BUS>, - <&clk IMX8MQ_VPU_PLL_BYPASS>; - assigned-clock-parents = <&clk IMX8MQ_VPU_PLL_OUT>, - <&clk IMX8MQ_VPU_PLL_OUT>, - <&clk IMX8MQ_SYS1_PLL_800M>, - <&clk IMX8MQ_VPU_PLL>; - assigned-clock-rates = <600000000>, <600000000>, - <800000000>, <0>; - power-domains = <&pgc_vpu>; + <&clk IMX8MQ_CLK_VPU_G2_ROOT>; + clock-names = "g1", "g2"; + #power-domain-cells = <1>; }; pcie0: pcie@33800000 {