Re: [PATCH v4 3/4] PCI: Decouple D3Hot and D3Cold handling for bridges

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Mon, Aug 05, 2024 at 12:17:13PM -0700, Hsin-Yi Wang wrote:
> On Mon, Aug 5, 2024 at 8:35 AM Manivannan Sadhasivam
> <manivannan.sadhasivam@linaro.org> wrote:
> >
> > On Fri, Aug 02, 2024 at 12:53:42PM -0700, Hsin-Yi Wang wrote:
> >
> > [...]
> >
> > > > > [   42.202016] mt7921e 0000:01:00.0: PM: calling
> > > > > pci_pm_suspend_noirq+0x0/0x300 @ 77, parent: 0000:00:00.0
> > > > > [   42.231681] mt7921e 0000:01:00.0: PCI PM: Suspend power state: D3hot
> > > >
> > > > Here I can see that the port entered D3hot
> > > >
> > > This one is the wifi device connected to the port.
> > >
> >
> > Ah, okay. You could've just shared the logs for the bridge/rootport.
> >
> > > > > [   42.238048] mt7921e 0000:01:00.0: PM:
> > > > > pci_pm_suspend_noirq+0x0/0x300 returned 0 after 26583 usecs
> > > > > [   42.247083] pcieport 0000:00:00.0: PM: calling
> > > > > pci_pm_suspend_noirq+0x0/0x300 @ 3196, parent: pci0000:00
> > > > > [   42.296325] pcieport 0000:00:00.0: PCI PM: Suspend power state: D0
> > > >
> > > This is the port suspended with D0. If we hack power_manageable to
> > > only consider D3hot, the state here for pcieport will become D3hot
> > > (compared in below).
> > >
> > > If it's D0 (and s2idle), in resume it won't restore config:
> > > https://elixir.bootlin.com/linux/v6.10/source/drivers/pci/pci-driver.c#L959,
> > > and in resume it would be an issue.
> > >
> > > Comparison:
> > > 1. pcieport can go to D3:
> > > (suspend)
> > > [   61.645809] mt7921e 0000:01:00.0: PM: calling
> > > pci_pm_suspend_noirq+0x0/0x2f8 @ 1139, parent: 0000:00:00.0
> > > [   61.675562] mt7921e 0000:01:00.0: PCI PM: Suspend power state: D3hot
> > > [   61.681931] mt7921e 0000:01:00.0: PM:
> > > pci_pm_suspend_noirq+0x0/0x2f8 returned 0 after 26502 usecs
> > > [   61.690959] pcieport 0000:00:00.0: PM: calling
> > > pci_pm_suspend_noirq+0x0/0x2f8 @ 3248, parent: pci0000:00
> > > [   61.755359] pcieport 0000:00:00.0: PCI PM: Suspend power state: D3hot
> > > [   61.761832] pcieport 0000:00:00.0: PM:
> > > pci_pm_suspend_noirq+0x0/0x2f8 returned 0 after 61345 usecs
> > >
> >
> > Why the device state is not saved? Did you skip those logs?
> >
> Right, I only showed the power state of pcieport and the device here
> to show the difference of 1 and 2.
> 
> > > (resume)
> > > [   65.243981] pcieport 0000:00:00.0: PM: calling
> > > pci_pm_resume_noirq+0x0/0x190 @ 3258, parent: pci0000:00
> > > [   65.253122] mtk-pcie-phy 16930000.phy: CKM_38=0x13040500,
> > > GLB_20=0x0, GLB_30=0x0, GLB_38=0x30453fc, GLB_F4=0x1453b000
> > > [   65.262725] pcieport 0000:00:00.0: PM:
> > > pci_pm_resume_noirq+0x0/0x190 returned 0 after 175 usecs
> > > [   65.273159] mtk-pcie-phy 16930000.phy: No calibration info
> > > [   65.281903] mt7921e 0000:01:00.0: PM: calling
> > > pci_pm_resume_noirq+0x0/0x190 @ 3259, parent: 0000:00:00.0
> > > [   65.297108] mt7921e 0000:01:00.0: PM: pci_pm_resume_noirq+0x0/0x190
> > > returned 0 after 329 usecs
> > >
> > >
> > > 2. pcieport stays at D0 due to power_manageable returns false:
> > > (suspend)
> > > [   52.435375] mt7921e 0000:01:00.0: PM: calling
> > > pci_pm_suspend_noirq+0x0/0x300 @ 2040, parent: 0000:00:00.0
> > > [   52.465235] mt7921e 0000:01:00.0: PCI PM: Suspend power state: D3hot
> > > [   52.471610] mt7921e 0000:01:00.0: PM:
> > > pci_pm_suspend_noirq+0x0/0x300 returned 0 after 26602 usecs
> > > [   52.480674] pcieport 0000:00:00.0: PM: calling
> > > pci_pm_suspend_noirq+0x0/0x300 @ 143, parent: pci0000:00
> > > [   52.529876] pcieport 0000:00:00.0: PCI PM: Suspend power state: D0
> > >                 <-- port is still D0
> > > [   52.536056] pcieport 0000:00:00.0: PCI PM: Skipped
> > >
> > > (resume)
> > > [   56.026298] pcieport 0000:00:00.0: PM: calling
> > > pci_pm_resume_noirq+0x0/0x190 @ 3243, parent: pci0000:00
> > > [   56.035379] mtk-pcie-phy 16930000.phy: CKM_38=0x13040500,
> > > GLB_20=0x0, GLB_30=0x0, GLB_38=0x30453fc, GLB_F4=0x1453b000
> > > [   56.044776] pcieport 0000:00:00.0: PM:
> > > pci_pm_resume_noirq+0x0/0x190 returned 0 after 13 usecs
> > > [   56.055409] mtk-pcie-phy 16930000.phy: No calibration info
> > > [   56.064098] mt7921e 0000:01:00.0: PM: calling
> > > pci_pm_resume_noirq+0x0/0x190 @ 3244, parent: 0000:00:00.0
> > > [   56.078962] mt7921e 0000:01:00.0: Unable to change power state from
> > > D3hot to D0, device inaccessible                    <-- resume failed.
> >
> > This means the port entered D3Cold? This is not expected during s2idle. During
> > s2idle, devices should be put into low power state and their power should be
> > preserved.
> >
> > Who is pulling the plug here?
> 
> In our system's use case, after the kernel enters s2idle then ATF (arm
> trusted firmware) will turn off the power (similar to suspend to ram).
> 

This is not acceptable IMO. S2IDLE != S2RAM. Even if you fix the portdrv, rest
of the PCIe client drivers may fail (hint: have you checked the NVMe driver)?

- Mani

-- 
மணிவண்ணன் சதாசிவம்