From: Jian-Hong Pan <jhp@endlessos.org>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "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>,
"Mika Westerberg" <mika.westerberg@linux.intel.com>,
"Damien Le Moal" <dlemoal@kernel.org>,
"Nirmal Patel" <nirmal.patel@linux.intel.com>,
"Jonathan Derrick" <jonathan.derrick@linux.dev>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
"Jian-Hong Pan" <jhp@endlessos.org>
Subject: [PATCH v5 4/4] PCI/ASPM: Fix L1.2 parameters when enable link state
Date: Wed, 24 Apr 2024 19:02:24 +0800 [thread overview]
Message-ID: <20240424110223.21799-2-jhp@endlessos.org> (raw)
Currently, when enable link's L1.2 features with __pci_enable_link_state(),
it configs the link directly without ensuring related L1.2 parameters, such
as T_POWER_ON, Common_Mode_Restore_Time, and LTR_L1.2_THRESHOLD have been
programmed.
This leads the link's L1.2 between PCIe Root Port and child device gets
wrong configs when a caller tries to enabled it.
Here is a failed example on ASUS B1400CEAE with enabled VMD:
10000:e0:06.0 PCI bridge: Intel Corporation 11th Gen Core Processor PCIe Controller (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=45us LTR1.2_Threshold=101376ns
L1SubCtl2: T_PwrOn=50us
10000:e1:00.0 Non-Volatile memory controller: Sandisk Corp WD Blue SN550 NVMe SSD (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=0ns
L1SubCtl2: T_PwrOn=10us
According to "PCIe r6.0, sec 5.5.4", before enabling ASPM L1.2 on the PCIe
Root Port and the child NVMe, they should be programmed with the same
LTR1.2_Threshold value. However, they have different values in this case.
Invoke aspm_calc_l12_info() to program the L1.2 parameters properly before
enable L1.2 bits of L1 PM Substates Control Register in
__pci_enable_link_state().
Link: https://bugzilla.kernel.org/show_bug.cgi?id=218394
Signed-off-by: Jian-Hong Pan <jhp@endlessos.org>
---
v2:
- Prepare the PCIe LTR parameters before enable L1 Substates
v3:
- Only enable supported features for the L1 Substates part
v4:
- Focus on fixing L1.2 parameters, instead of re-initializing whole L1SS
v5:
- Fix typo and commit message
- Split introducing aspm_get_l1ss_cap() to "PCI/ASPM: Introduce
aspm_get_l1ss_cap()"
drivers/pci/pcie/aspm.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index c55ac11faa73..553327dee991 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1402,6 +1402,8 @@ EXPORT_SYMBOL(pci_disable_link_state);
static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
{
struct pcie_link_state *link = pcie_aspm_get_link(pdev);
+ struct pci_dev *child = link->downstream, *parent = link->pdev;
+ u32 parent_l1ss_cap, child_l1ss_cap;
if (!link)
return -EINVAL;
@@ -1433,6 +1435,16 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
link->aspm_default |= ASPM_STATE_L1_1_PCIPM | ASPM_STATE_L1;
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
link->aspm_default |= ASPM_STATE_L1_2_PCIPM | ASPM_STATE_L1;
+ /*
+ * Ensure L1.2 parameters: Common_Mode_Restore_Times, T_POWER_ON and
+ * LTR_L1.2_THRESHOLD are programmed properly before enable bits for
+ * L1.2, per PCIe r6.0, sec 5.5.4.
+ */
+ if (state & link->aspm_capable & ASPM_STATE_L1_2_MASK) {
+ parent_l1ss_cap = aspm_get_l1ss_cap(parent);
+ child_l1ss_cap = aspm_get_l1ss_cap(child);
+ aspm_calc_l12_info(link, parent_l1ss_cap, child_l1ss_cap);
+ }
pcie_config_aspm_link(link, policy_to_aspm_state(link));
link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
--
2.44.0
next reply other threads:[~2024-04-24 11:03 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-24 11:02 Jian-Hong Pan [this message]
2024-04-27 0:03 ` [PATCH v5 4/4] PCI/ASPM: Fix L1.2 parameters when enable link state David E. Box
2024-04-30 7:46 ` Jian-Hong Pan
2024-04-30 18:26 ` David E. Box
2024-05-03 9:45 ` Jian-Hong Pan
2024-05-03 19:15 ` David E. Box
2024-05-03 22:28 ` 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=20240424110223.21799-2-jhp@endlessos.org \
--to=jhp@endlessos.org \
--cc=david.e.box@linux.intel.com \
--cc=dlemoal@kernel.org \
--cc=helgaas@kernel.org \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=johan@kernel.org \
--cc=jonathan.derrick@linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--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.