lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 25 Apr 2022 17:34:34 +0200
From:   Lucas Stach <l.stach@...gutronix.de>
To:     Martin Kepplinger <martin.kepplinger@...i.sm>,
        Adam Ford <aford173@...il.com>, linux-media@...r.kernel.org
Cc:     aford@...conembedded.com, cphealy@...il.com,
        kernel test robot <lkp@...el.com>,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Rob Herring <robh@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Pengutronix Kernel Team <kernel@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        NXP Linux Team <linux-imx@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-rockchip@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-staging@...ts.linux.dev
Subject: Re: [PATCH V4 07/11] arm64: dts: imx8mq: Enable both G1 and G2
 VPU's with vpu-blk-ctrl

Hi Martin,

Am Montag, dem 25.04.2022 um 17:22 +0200 schrieb Martin Kepplinger:
> Am Dienstag, dem 25.01.2022 um 11:11 -0600 schrieb Adam Ford:
> > With the Hantro G1 and G2 now setup to run independently, update
> > the device tree to allow both to operate.  This requires the
> > vpu-blk-ctrl node to be configured.  Since vpu-blk-ctrl needs
> > certain clock enabled to handle the gating of the G1 and G2
> > fuses, the clock-parents and clock-rates for the various VPU's
> > to be moved into the pgc_vpu because they cannot get re-parented
> > once enabled, and the pgc_vpu is the highest in the chain.
> > 
> > Signed-off-by: Adam Ford <aford173@...il.com>
> > Reported-by: kernel test robot <lkp@...el.com>
> > Reviewed-by: Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > index 2df2510d0118..549b2440f55d 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > @@ -737,7 +737,21 @@ pgc_gpu: power-domain@5 {
> >                                         pgc_vpu: power-domain@6 {
> >                                                 #power-domain-cells =
> > <0>;
> >                                                 reg =
> > <IMX8M_POWER_DOMAIN_VPU>;
> > -                                               clocks = <&clk
> > IMX8MQ_CLK_VPU_DEC_ROOT>;
> > +                                               clocks = <&clk
> > IMX8MQ_CLK_VPU_DEC_ROOT>,
> > +                                                        <&clk
> > IMX8MQ_CLK_VPU_G1_ROOT>,
> > +                                                        <&clk
> > IMX8MQ_CLK_VPU_G2_ROOT>;
> > +                                               assigned-clocks =
> > <&clk IMX8MQ_CLK_VPU_G1>,
> > +                                                                
> > <&clk IMX8MQ_CLK_VPU_G2>,
> > +                                                                
> > <&clk IMX8MQ_CLK_VPU_BUS>,
> > +                                                                
> > <&clk IMX8MQ_VPU_PLL_BYPASS>;
> > +                                               assigned-clock-
> > parents = <&clk IMX8MQ_VPU_PLL_OUT>,
> > +                                                                    
> >     <&clk IMX8MQ_VPU_PLL_OUT>,
> > +                                                                    
> >     <&clk IMX8MQ_SYS1_PLL_800M>,
> > +                                                                    
> >     <&clk IMX8MQ_VPU_PLL>;
> > +                                               assigned-clock-rates
> > = <600000000>,
> > +                                                                    
> >   <600000000>,
> > +                                                                    
> >   <800000000>,
> > +                                                                    
> >   <0>;
> >                                         };
> >  
> >                                         pgc_disp: power-domain@7 {
> > @@ -1457,30 +1471,31 @@ usb3_phy1: usb-phy@...f0040 {
> >                         status = "disabled";
> >                 };
> >  
> > -               vpu: video-codec@...00000 {
> > -                       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@...00000 {
> > +                       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@...10000 {
> > +                       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@...20000 {
> > +                       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@...00000 {
> 
> 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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ