Re: [BUG] Intel Apollolake: PCIe bridge "loses" capabilities after entering D3Cold state

[Bjorn Helgaas]

[+cc Radosław]

On Fri, Oct 21, 2022 at 12:17:35PM +0200, Lukasz Majczak wrote:
> Hi,
> 
> This a follow-up from a discussion from “[PATCH V2] PCI/ASPM:
> Save/restore L1SS Capability for suspend/resume”
> (https://lore.kernel.org/lkml/d3228b1f-8d12-bfab-4cba-6d93a6869f20@xxxxxxxxxx/t/)
> 
> While working with Vidya’s patch I have noticed that after
> suspend/resume cycle on my Chromebook (Apollolake) PCIe bridge loses
> its capabilities - the missing part is:
> 
> Capabilities: [200 v1] L1 PM Substates
> L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+
>   PortCommonModeRestoreTime=40us PortTPowerOnTime=10us
> L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+
>    T_CommonMode=40us LTR1.2_Threshold=98304ns
> L1SubCtl2: T_PwrOn=60us
> 
> Digging more I’ve found out that entering D3Cold state causes this
> issue (D3Hot seems to work fine).
> 
> With Vidya’s patch (all versions form V1 to V3) on upstream kernels
> 5.10/5.15  it was causing underlying device unavailable (in my case -
> WiFi card) - the V4 (which was accepted and merged) works fine (I
> guess thanks to “PCI/ASPM: Refactor L1 PM Substates Control Register
> programming”) but the issue is still there - I mean now after
> suspend/resume the underlying deceive works fine but mentioned
> capabilities are still gone when using lspci -vvv.
> 
> I think with current code it does no harm to anyone, but just doing a
> heads up about this.

Thanks a lot for following up on this!  Tell me if I have this right:

  - After a fresh boot, the Root Port at 00:14.0 [8086:5ad6] has an L1
    PM Substates Capability [per 1,2].

  - You suspend and resume the system.

  - After resume, 00:14.0 no longer has an L1 PM Substates Capability,
    as in [2].

  - The 00:14.0 Root Port leads to an iwlwifi device at 01:00.0, and
    the wifi device works fine after resume.

  - On the 01:00.0 iwlwifi device, lspci -vv still shows L1.1 and L1.2
    enabled after resume, as it did in [2].

If substates are enabled at iwlwifi but not at the Root Port, that
would not be a valid scenario per spec.  Per PCIe r6.0, sec 5.5.4:

  An L1 PM Substate enable bit must only be Set in the Upstream and
  Downstream Ports on a Link when the corresponding supported
  capability bit is Set by both the Upstream and Downstream Ports on
  that Link, otherwise the behavior is undefined.

So I don't know whether the L1.s states would still actually work.
(Is there any way to tell whether the iwlwifi power consumption
changes after the suspend/resume?  Maybe powertop?)

And ASPM configuration, e.g., disabling/enabling substates via the
sysfs "l1_1_aspm" and "l1_2_aspm" files probably won't work right.

Bjorn

[1] https://lore.kernel.org/lkml/20220722174212.GA1911979@bhelgaas/
[2] https://gist.github.com/semihalf-majczak-lukasz/fb36dfa2eff22911109dfb91ab0fc0e3