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 <[email protected]>
Reported-by: kernel test robot <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>
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
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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Reviewed-by: Ezequiel Garcia <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
>
> 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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
It doesn't seem correct to have the Reported-by on this commit.
Thanks,
Ezequiel
> Reviewed-by: Ezequiel Garcia <[email protected]>
>
> 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
<[email protected]> 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 <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
>
> It doesn't seem correct to have the Reported-by on this commit.
Applied with Reported-by tag dropped, thanks!
On Mon, Apr 25, 2022 at 10:34 AM Lucas Stach <[email protected]> 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 <[email protected]>
> > > Reported-by: kernel test robot <[email protected]>
> > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > >
> > > 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 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 <[email protected]>
> Reported-by: kernel test robot <[email protected]>
> Reviewed-by: Ezequiel Garcia <[email protected]>
>
> 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 <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
> > Reviewed-by: Ezequiel Garcia <[email protected]>
> >
> > 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
Am Montag, dem 25.04.2022 um 10:47 -0500 schrieb Adam Ford:
> On Mon, Apr 25, 2022 at 10:34 AM Lucas Stach <[email protected]>
> 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 <[email protected]>
> > > > Reported-by: kernel test robot <[email protected]>
> > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > >
> > > > 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/[email protected]/
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 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 <[email protected]>
> > > > Reported-by: kernel test robot <[email protected]>
> > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > >
> > > > > 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 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 <[email protected]>
> > > Reported-by: kernel test robot <[email protected]>
> > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > >
> > > 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 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 <[email protected]>
> > 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 <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > >
> > > > > 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/[email protected]/
>
> 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 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 <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > > > Reported-by: kernel test robot <[email protected]>
> > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > >
> > > > > 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/[email protected]/T/#t
thank you for you help so far,
martin
Hi Martin,
On Mon, Jul 11, 2022 at 6:53 AM Martin Kepplinger
<[email protected]> 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 <[email protected]>
> > > > > > Reported-by: kernel test robot <[email protected]>
> > > > > > Reviewed-by: Ezequiel Garcia <[email protected]>
> > > > > >
> > > > > > 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/[email protected]/T/#t
>
> thank you for you help so far,
>
Thanks a lot for keeping us posted. The fix for suspend/resume
looks great!
Ezequiel