All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Krzysztof Wilczyński" <kw@linux.com>
To: Jian-Hong Pan <jhp@endlessos.org>
Cc: "Bjorn Helgaas" <helgaas@kernel.org>,
	"Johan Hovold" <johan@kernel.org>,
	"David Box" <david.e.box@linux.intel.com>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	"Kuppuswamy Sathyanarayanan"
	<sathyanarayanan.kuppuswamy@linux.intel.com>,
	"Nirmal Patel" <nirmal.patel@linux.intel.com>,
	"Jonathan Derrick" <jonathan.derrick@linux.dev>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux@endlessos.org
Subject: Re: [PATCH v12 3/3] PCI/ASPM: Make pci_save_aspm_l1ss_state save both child and parent's L1SS configuration
Date: Mon, 4 Nov 2024 05:47:16 +0900	[thread overview]
Message-ID: <20241103204716.GE237624@rocinante> (raw)
In-Reply-To: <20241001083438.10070-8-jhp@endlessos.org>

Hello,

> PCI devices' parameters on the VMD bus have been programmed properly
> originally. But, cleared after pci_reset_bus() and have not been restored
> correctly. This leads the link's L1.2 between PCIe Root Port and child
> device gets wrong configs.
> 
> Here is a failed example on ASUS B1400CEAE with enabled VMD. Both PCIe
> bridge and NVMe device should have the same LTR1.2_Threshold value.
> However, they are configured as different values in this case:
> 
> 10000:e0:06.0 PCI bridge [0604]: Intel Corporation 11th Gen Core Processor PCIe Controller [8086:9a09] (rev 01) (prog-if 00 [Normal decode])
>   ...
>   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=45us PortTPowerOnTime=50us
>     L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1-
>       T_CommonMode=0us LTR1.2_Threshold=0ns
>     L1SubCtl2: T_PwrOn=0us
> 
> 10000:e1:00.0 Non-Volatile memory controller [0108]: Sandisk Corp WD Blue SN550 NVMe SSD [15b7:5009] (rev 01) (prog-if 02 [NVM Express])
>   ...
>   Capabilities: [900 v1] L1 PM Substates
>     L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1- L1_PM_Substates+
>       PortCommonModeRestoreTime=32us PortTPowerOnTime=10us
>     L1SubCtl1: PCI-PM_L1.2- PCI-PM_L1.1- ASPM_L1.2+ ASPM_L1.1-
>       T_CommonMode=0us LTR1.2_Threshold=101376ns
>     L1SubCtl2: T_PwrOn=50us
> 
> Here is VMD mapped PCI device tree:
> 
> -+-[0000:00]-+-00.0  Intel Corporation Device 9a04
>  | ...
>  \-[10000:e0]-+-06.0-[e1]----00.0  Sandisk Corp WD Blue SN550 NVMe SSD
>               \-17.0  Intel Corporation Tiger Lake-LP SATA Controller
> 
> When pci_reset_bus() resets the bus [e1] of the NVMe, it only saves and
> restores NVMe's state before and after reset. Then, when it restores the
> NVMe's state, ASPM code restores L1SS for both the parent bridge and the
> NVMe in pci_restore_aspm_l1ss_state(). The NVMe's L1SS is restored
> correctly. But, the parent bridge's L1SS is restored with a wrong value 0x0
> because the parent bridge's L1SS wasn't saved by pci_save_aspm_l1ss_state()
> before reset.
> 
> So, if the PCI device has a parent, make pci_save_aspm_l1ss_state() save
> the parent's L1SS configuration, too. This is symmetric on
> pci_restore_aspm_l1ss_state().

Applied to aspm, thank you!

[01/02] PCI/ASPM: Add notes about enabling PCI-PM L1SS to pci_enable_link_state(_locked)
        https://git.kernel.org/pci/pci/c/4681da23786a

[02/02] PCI/ASPM: Make pci_save_aspm_l1ss_state save both child and parent's L1SS configuration
        https://git.kernel.org/pci/pci/c/1f37e72d586f

	Krzysztof

  parent reply	other threads:[~2024-11-03 20:47 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-01  8:34 [PATCH v12 0/3] PCI: vmd: Enable PCI PM's L1 substates of remapped PCIe Root Port and NVMe Jian-Hong Pan
2024-10-01  8:34 ` [PATCH v12 1/3] PCI: vmd: Set PCI devices to D0 before enable PCI PM's L1 substates Jian-Hong Pan
2024-11-03 20:40   ` Krzysztof Wilczyński
2024-10-01  8:34 ` [PATCH v12 2/3] PCI/ASPM: Add notes about enabling PCI-PM L1SS to pci_enable_link_state(_locked) Jian-Hong Pan
2024-10-01  8:34 ` [PATCH v12 3/3] PCI/ASPM: Make pci_save_aspm_l1ss_state save both child and parent's L1SS configuration Jian-Hong Pan
2024-10-01 15:00   ` Ilpo Järvinen
2024-10-02  0:02   ` David E. Box
2024-10-02  8:02     ` Ilpo Järvinen
2024-11-03 20:47   ` Krzysztof Wilczyński [this message]
2024-11-05 22:59   ` Bjorn Helgaas
2024-11-06 10:54     ` Ilpo Järvinen
2024-11-06 17:16       ` Bjorn Helgaas
2024-11-07  9:19     ` Jian-Hong Pan
2024-11-07 15:41       ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241103204716.GE237624@rocinante \
    --to=kw@linux.com \
    --cc=david.e.box@linux.intel.com \
    --cc=helgaas@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jhp@endlessos.org \
    --cc=johan@kernel.org \
    --cc=jonathan.derrick@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux@endlessos.org \
    --cc=nirmal.patel@linux.intel.com \
    --cc=sathyanarayanan.kuppuswamy@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.