Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Oct 2022 16:08:20 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Lukasz Majczak <lma@semihalf.com>
Cc:     bhelgaas@google.com, Rajat Jain <rajatja@google.com>,
        Vidya Sagar <vidyas@nvidia.com>, upstream@semihalf.com,
        linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        =?utf-8?B?UmFkb3PFgmF3?= Biernacki <rad@semihalf.com>
Subject: Re: [BUG] Intel Apollolake: PCIe bridge "loses" capabilities after
 entering D3Cold state
Message-ID: <20221021210820.GA308037@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAFJ_xbq0cxcH-cgpXLU4Mjk30+muWyWm1aUZGK7iG53yaLBaQg@mail.gmail.com>
Precedence: bulk

[+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@nvidia.com/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