diff mbox series

NOC (interconnect) lockup on i.MX8MP during resume

Message ID 2176690.usQuhbGJ8B@steina-w (mailing list archive)
State New, archived
Headers show
Series NOC (interconnect) lockup on i.MX8MP during resume | expand

Commit Message

Alexander Stein March 9, 2023, 2:57 p.m. UTC 
Hi,

while debugging another issue regarding suspend/resume I noticed that the imx
interconnect driver locks up the system when writing into registers. This
happens during resume of the 'g2' power domain. imx8m_blk_ctrl_power_on()
will eventually call down to imx_icc_node_set(). Using the debug diff furthe
down I get this output:

[   71.493062] imx_icc_node_set: node: VPU G2
[   71.497163] imx_icc_node_set: peak_bw: 1
[   71.501091] imx_icc_node_set: prio: 0x80000303
<freeze here>

Unfortunately there is absolutely no documentation regarding NOC in the
reference manual, so no idea whats happening here.
This is on next-20230309 using imx8mp-tqma8mpql-mba8mpxl.dts with USB
disabled (prevents suspend) and Ethernet-PHY-IRQ disabled (the issue I'm
tackling).
Is there some idea what do in order to support resume on i.MX8MP?

Best regards,
Alexander

---8<---
---8<---

Comments

Ahmad Fatoum March 9, 2023, 3:03 p.m. UTC | #1 
Hello Alexander,

On 09.03.23 15:57, Alexander Stein wrote:
> Hi,
> 
> while debugging another issue regarding suspend/resume I noticed that the imx
> interconnect driver locks up the system when writing into registers. This
> happens during resume of the 'g2' power domain. imx8m_blk_ctrl_power_on()
> will eventually call down to imx_icc_node_set(). Using the debug diff furthe
> down I get this output:
> 
> [   71.493062] imx_icc_node_set: node: VPU G2
> [   71.497163] imx_icc_node_set: peak_bw: 1
> [   71.501091] imx_icc_node_set: prio: 0x80000303
> <freeze here>
> 
> Unfortunately there is absolutely no documentation regarding NOC in the
> reference manual, so no idea whats happening here.
> This is on next-20230309 using imx8mp-tqma8mpql-mba8mpxl.dts with USB
> disabled (prevents suspend) and Ethernet-PHY-IRQ disabled (the issue I'm
> tackling).
> Is there some idea what do in order to support resume on i.MX8MP?

Could it be that you don't have a fully featured 8MP, but a 8MPUL
or 8MPL, which lacks VPUs? In that case, it's not enough to disable
the VPU nodes in the DT, but you need to disable the VPU power domains as well.

Cheers,
Ahmad

> 
> Best regards,
> Alexander
> 
> ---8<---
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> index 979ed610f704..93ec79cc7c06 100644
> --- a/drivers/interconnect/imx/imx.c
> +++ b/drivers/interconnect/imx/imx.c
> @@ -44,13 +44,18 @@ static int imx_icc_node_set(struct icc_node *node)
>         u32 prio;
>         u64 freq;
>  
> +       pr_info("%s: node: %s\n", __func__, node->name);
> +       pr_info("%s: peak_bw: %u\n", __func__, node->peak_bw);
>         if (node_data->setting && node->peak_bw) {
>                 base = node_data->setting->reg + node_data->imx_provider->noc_base;
>                 if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
>                         prio = node_data->setting->prio_level;
>                         prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
> +                       pr_info("%s: prio: %#x\n", __func__, prio);
>                         writel(prio, base + IMX_NOC_PRIO_REG);
> +                       pr_info("%s: mode: %#x\n", __func__, node_data->setting->mode);
>                         writel(node_data->setting->mode, base + IMX_NOC_MODE_REG);
> +                       pr_info("%s: ext_control: #%x\n", __func__, node_data->setting->ext_control);
>                         writel(node_data->setting->ext_control, base + IMX_NOC_EXT_CTL_REG);
>                         dev_dbg(dev, "%s: mode: 0x%x, prio: 0x%x, ext_control: 0x%x\n",
>                                 node_data->desc->name, node_data->setting->mode, prio,
> ---8<---
>
Alexander Stein March 9, 2023, 3:17 p.m. UTC | #2 
Hi Ahmad,

Am Donnerstag, 9. März 2023, 16:03:26 CET schrieb Ahmad Fatoum:
> Hello Alexander,
> 
> On 09.03.23 15:57, Alexander Stein wrote:
> > Hi,
> > 
> > while debugging another issue regarding suspend/resume I noticed that the
> > imx interconnect driver locks up the system when writing into registers.
> > This happens during resume of the 'g2' power domain.
> > imx8m_blk_ctrl_power_on() will eventually call down to
> > imx_icc_node_set(). Using the debug diff furthe down I get this output:
> > 
> > [   71.493062] imx_icc_node_set: node: VPU G2
> > [   71.497163] imx_icc_node_set: peak_bw: 1
> > [   71.501091] imx_icc_node_set: prio: 0x80000303
> > <freeze here>
> > 
> > Unfortunately there is absolutely no documentation regarding NOC in the
> > reference manual, so no idea whats happening here.
> > This is on next-20230309 using imx8mp-tqma8mpql-mba8mpxl.dts with USB
> > disabled (prevents suspend) and Ethernet-PHY-IRQ disabled (the issue I'm
> > tackling).
> > Is there some idea what do in order to support resume on i.MX8MP?
> 
> Could it be that you don't have a fully featured 8MP, but a 8MPUL
> or 8MPL, which lacks VPUs? In that case, it's not enough to disable
> the VPU nodes in the DT, but you need to disable the VPU power domains as
> well.

That's reasonable to question this, but in this case this is a full-fledged 
8MP. I just noticed that during boot those settings are done as well and there 
I get the (expected) output:

[   12.696371] imx_icc_node_set: node: VPU G2
[   12.700472] imx_icc_node_set: peak_bw: 1
[   12.704401] imx_icc_node_set: prio: 0x80000303
[   12.708850] imx_icc_node_set: mode: 0x0
[   12.712691] imx_icc_node_set: ext_control: #0
[   12.717053] imx_icc_node_set: node: NOC_VIDEO
[   12.721419] imx_icc_node_set: peak_bw: 1

So there is something wrong during resume. Both clk_ignore_unused 
pd_ignore_unused did not help.

Best regards,
Alexander

> Cheers,
> Ahmad
> 
> > Best regards,
> > Alexander
> > 
> > ---8<---
> > diff --git a/drivers/interconnect/imx/imx.c
> > b/drivers/interconnect/imx/imx.c index 979ed610f704..93ec79cc7c06 100644
> > --- a/drivers/interconnect/imx/imx.c
> > +++ b/drivers/interconnect/imx/imx.c
> > @@ -44,13 +44,18 @@ static int imx_icc_node_set(struct icc_node *node)
> > 
> >         u32 prio;
> >         u64 freq;
> > 
> > +       pr_info("%s: node: %s\n", __func__, node->name);
> > +       pr_info("%s: peak_bw: %u\n", __func__, node->peak_bw);
> > 
> >         if (node_data->setting && node->peak_bw) {
> >         
> >                 base = node_data->setting->reg +
> >                 node_data->imx_provider->noc_base;
> >                 if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
> >                 
> >                         prio = node_data->setting->prio_level;
> >                         prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
> > 
> > +                       pr_info("%s: prio: %#x\n", __func__, prio);
> > 
> >                         writel(prio, base + IMX_NOC_PRIO_REG);
> > 
> > +                       pr_info("%s: mode: %#x\n", __func__,
> > node_data->setting->mode);> 
> >                         writel(node_data->setting->mode, base +
> >                         IMX_NOC_MODE_REG);
> > 
> > +                       pr_info("%s: ext_control: #%x\n", __func__,
> > node_data->setting->ext_control);> 
> >                         writel(node_data->setting->ext_control, base +
> >                         IMX_NOC_EXT_CTL_REG);
> >                         dev_dbg(dev, "%s: mode: 0x%x, prio: 0x%x,
> >                         ext_control: 0x%x\n",
> >                         
> >                                 node_data->desc->name,
> >                                 node_data->setting->mode, prio,
> > 
> > ---8<---
Adam Ford March 13, 2023, 7:17 p.m. UTC | #3 
On Thu, Mar 9, 2023 at 9:11 AM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Hi,
>
> while debugging another issue regarding suspend/resume I noticed that the imx
> interconnect driver locks up the system when writing into registers. This
> happens during resume of the 'g2' power domain. imx8m_blk_ctrl_power_on()
> will eventually call down to imx_icc_node_set(). Using the debug diff furthe
> down I get this output:
>
> [   71.493062] imx_icc_node_set: node: VPU G2
> [   71.497163] imx_icc_node_set: peak_bw: 1
> [   71.501091] imx_icc_node_set: prio: 0x80000303
> <freeze here>
>
> Unfortunately there is absolutely no documentation regarding NOC in the
> reference manual, so no idea whats happening here.
> This is on next-20230309 using imx8mp-tqma8mpql-mba8mpxl.dts with USB
> disabled (prevents suspend) and Ethernet-PHY-IRQ disabled (the issue I'm
> tackling).
> Is there some idea what do in order to support resume on i.MX8MP?

I haven't played with suspend-resume myself, but there was a thread
[1] with discussions about fixing a race condition on the imx8mp
interconnect.

adam

[1] - https://lore.kernel.org/lkml/20230201101559.15529-5-johan+linaro@kernel.org/



>
> Best regards,
> Alexander
>
> ---8<---
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> index 979ed610f704..93ec79cc7c06 100644
> --- a/drivers/interconnect/imx/imx.c
> +++ b/drivers/interconnect/imx/imx.c
> @@ -44,13 +44,18 @@ static int imx_icc_node_set(struct icc_node *node)
>         u32 prio;
>         u64 freq;
>
> +       pr_info("%s: node: %s\n", __func__, node->name);
> +       pr_info("%s: peak_bw: %u\n", __func__, node->peak_bw);
>         if (node_data->setting && node->peak_bw) {
>                 base = node_data->setting->reg + node_data->imx_provider->noc_base;
>                 if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
>                         prio = node_data->setting->prio_level;
>                         prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
> +                       pr_info("%s: prio: %#x\n", __func__, prio);
>                         writel(prio, base + IMX_NOC_PRIO_REG);
> +                       pr_info("%s: mode: %#x\n", __func__, node_data->setting->mode);
>                         writel(node_data->setting->mode, base + IMX_NOC_MODE_REG);
> +                       pr_info("%s: ext_control: #%x\n", __func__, node_data->setting->ext_control);
>                         writel(node_data->setting->ext_control, base + IMX_NOC_EXT_CTL_REG);
>                         dev_dbg(dev, "%s: mode: 0x%x, prio: 0x%x, ext_control: 0x%x\n",
>                                 node_data->desc->name, node_data->setting->mode, prio,
> ---8<---
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>
Alexander Stein March 14, 2023, 7:39 a.m. UTC | #4 
Hi Adam,

Am Montag, 13. März 2023, 20:17:55 CET schrieb Adam Ford:
> On Thu, Mar 9, 2023 at 9:11 AM Alexander Stein
> 
> <alexander.stein@ew.tq-group.com> wrote:
> > Hi,
> > 
> > while debugging another issue regarding suspend/resume I noticed that the
> > imx interconnect driver locks up the system when writing into registers.
> > This happens during resume of the 'g2' power domain.
> > imx8m_blk_ctrl_power_on() will eventually call down to
> > imx_icc_node_set(). Using the debug diff furthe down I get this output:
> > 
> > [   71.493062] imx_icc_node_set: node: VPU G2
> > [   71.497163] imx_icc_node_set: peak_bw: 1
> > [   71.501091] imx_icc_node_set: prio: 0x80000303
> > <freeze here>
> > 
> > Unfortunately there is absolutely no documentation regarding NOC in the
> > reference manual, so no idea whats happening here.
> > This is on next-20230309 using imx8mp-tqma8mpql-mba8mpxl.dts with USB
> > disabled (prevents suspend) and Ethernet-PHY-IRQ disabled (the issue I'm
> > tackling).
> > Is there some idea what do in order to support resume on i.MX8MP?
> 
> I haven't played with suspend-resume myself, but there was a thread
> [1] with discussions about fixing a race condition on the imx8mp
> interconnect.

Thanks for this link, but I think this is addressing a race condition during 
(un)registering. This works for me so far. I'm dealing with a freezing 
hardware write upon resume.

Best regards,
Alexander


> adam
> 
> [1] -
> https://lore.kernel.org/lkml/20230201101559.15529-5-johan+linaro@kernel.org
> /
> > Best regards,
> > Alexander
> > 
> > ---8<---
> > diff --git a/drivers/interconnect/imx/imx.c
> > b/drivers/interconnect/imx/imx.c index 979ed610f704..93ec79cc7c06 100644
> > --- a/drivers/interconnect/imx/imx.c
> > +++ b/drivers/interconnect/imx/imx.c
> > @@ -44,13 +44,18 @@ static int imx_icc_node_set(struct icc_node *node)
> > 
> >         u32 prio;
> >         u64 freq;
> > 
> > +       pr_info("%s: node: %s\n", __func__, node->name);
> > +       pr_info("%s: peak_bw: %u\n", __func__, node->peak_bw);
> > 
> >         if (node_data->setting && node->peak_bw) {
> >         
> >                 base = node_data->setting->reg +
> >                 node_data->imx_provider->noc_base;
> >                 if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
> >                 
> >                         prio = node_data->setting->prio_level;
> >                         prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
> > 
> > +                       pr_info("%s: prio: %#x\n", __func__, prio);
> > 
> >                         writel(prio, base + IMX_NOC_PRIO_REG);
> > 
> > +                       pr_info("%s: mode: %#x\n", __func__,
> > node_data->setting->mode);> 
> >                         writel(node_data->setting->mode, base +
> >                         IMX_NOC_MODE_REG);
> > 
> > +                       pr_info("%s: ext_control: #%x\n", __func__,
> > node_data->setting->ext_control);> 
> >                         writel(node_data->setting->ext_control, base +
> >                         IMX_NOC_EXT_CTL_REG);
> >                         dev_dbg(dev, "%s: mode: 0x%x, prio: 0x%x,
> >                         ext_control: 0x%x\n",
> >                         
> >                                 node_data->desc->name,
> >                                 node_data->setting->mode, prio,
> > 
> > ---8<---
> > 
> > --
> > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> > Amtsgericht München, HRB 105018
> > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> > http://www.tq-group.com/
diff mbox series

Patch

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index 979ed610f704..93ec79cc7c06 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -44,13 +44,18 @@  static int imx_icc_node_set(struct icc_node *node)
        u32 prio;
        u64 freq;
 
+       pr_info("%s: node: %s\n", __func__, node->name);
+       pr_info("%s: peak_bw: %u\n", __func__, node->peak_bw);
        if (node_data->setting && node->peak_bw) {
                base = node_data->setting->reg + node_data->imx_provider->noc_base;
                if (node_data->setting->mode == IMX_NOC_MODE_FIXED) {
                        prio = node_data->setting->prio_level;
                        prio = PRIORITY_COMP_MARK | (prio << 8) | prio;
+                       pr_info("%s: prio: %#x\n", __func__, prio);
                        writel(prio, base + IMX_NOC_PRIO_REG);
+                       pr_info("%s: mode: %#x\n", __func__, node_data->setting->mode);
                        writel(node_data->setting->mode, base + IMX_NOC_MODE_REG);
+                       pr_info("%s: ext_control: #%x\n", __func__, node_data->setting->ext_control);
                        writel(node_data->setting->ext_control, base + IMX_NOC_EXT_CTL_REG);
                        dev_dbg(dev, "%s: mode: 0x%x, prio: 0x%x, ext_control: 0x%x\n",
                                node_data->desc->name, node_data->setting->mode, prio,