[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAd53p6DX2C7KVRV=uu_mmPTTjE7=RsXfNPxjbOBLRbf-pXi5A@mail.gmail.com>
Date: Fri, 15 Apr 2022 22:26:19 +0800
From: Kai-Heng Feng <kai.heng.feng@...onical.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Vidya Sagar <vidyas@...dia.com>,
"Kenneth R. Crudup" <kenny@...ix.com>, bhelgaas@...gle.com,
lorenzo.pieralisi@....com, hkallweit1@...il.com,
wangxiongfeng2@...wei.com, mika.westerberg@...ux.intel.com,
chris.packham@...iedtelesis.co.nz, yangyicong@...ilicon.com,
treding@...dia.com, jonathanh@...dia.com, abhsahu@...dia.com,
sagupta@...dia.com, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, kthota@...dia.com,
mmaddireddy@...dia.com, sagar.tv@...il.com,
Ricky Wu <ricky_wu@...ltek.com>,
Rajat Jain <rajatja@...gle.com>,
Prasad Malisetty <quic_pmaliset@...cinc.com>,
Victor Ding <victording@...gle.com>
Subject: Re: [PATCH V1] PCI/ASPM: Save/restore L1SS Capability for suspend/resume
On Fri, Apr 15, 2022 at 12:41 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> On Wed, Apr 13, 2022 at 08:19:26AM +0800, Kai-Heng Feng wrote:
> > On Wed, Apr 13, 2022 at 6:50 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
> > > ...
>
> > > - For L1 PM Substates configuration, sec 5.5.4 says that both ports
> > > must be configured while ASPM L1 is disabled, but I don't think we
> > > currently guarantee this: we restore all the upstream component
> > > state first, and we don't know the ASPM state of the downstream
> > > one. Maybe we need to:
> > >
> > > * When restoring upstream component,
> > > + disable its ASPM
> > >
> > > * When restoring downstream component,
> > > + disable its ASPM
> > > + restore upstream component's LTR, L1SS
> > > + restore downstream component's LTR, L1SS
> > > + restore upstream component's ASPM
> > > + restore downstream component's ASPM
> >
> > Right now L1SS isn't reprogrammed after S3, and that causes WD NVMe
> > starts to spew lots of AER errors.
>
> Right now we don't fully restore L1SS-related state after S3, so maybe
> there's some inconsistency that leads to the AER errors.
>
> Could you collect the "lspci -vv" state before and after S3 so we can
> compare them?
>
> > So yes please restore L1SS upon resume. Meanwhile I am asking vendor
> > _why_ restoring L1SS is crucial for it to work.
> >
> > I also wonder what's the purpose of pcie_aspm_pm_state_change()? Can't
> > we just restore ASPM bits like other configs?
>
> Good question. What's the context? This is in the
> pci_raw_set_power_state() path, not the pci_restore_state() path, so
> seems like a separate discussion.
Because this patch alone doesn't restore T_PwrOn and LTR1.2_Threshold.
So I forced the pcie_aspm_pm_state_change() calling path to eventually
call aspm_calc_l1ss_info() which solved the problem for me.
Let me investigate a bit further.
Kai-Heng
>
> Bjorn
Powered by blists - more mailing lists