public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jian-Hong Pan <jhp@endlessos.org>
To: Bjorn Helgaas <helgaas@kernel.org>, Johan Hovold <johan@kernel.org>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	David Box <david.e.box@linux.intel.com>,
	Damien Le Moal <dlemoal@kernel.org>,
	Niklas Cassel <cassel@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,
	linux@endlessos.org, Jian-Hong Pan <jhp@endlessos.org>
Subject: [PATCH v2] PCI: vmd: Enable PCI PM's L1 substates of remapped PCIe Root Port and NVMe
Date: Fri,  2 Feb 2024 15:11:12 +0800	[thread overview]
Message-ID: <20240202071110.8515-3-jhp@endlessos.org> (raw)

The remapped PCIe Root Port and NVMe have PCI PM L1 substates
capability, but they are disabled originally:

Here is an example on ASUS B1400CEAE:

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

Power on all of the VMD remapped PCI devices and quirk max snoop LTR
before enable PCI-PM L1 PM Substates by following "Section 5.5.4 of PCIe
Base Spec Revision 6.0". Then, PCI PM's L1 substates control are
initialized & enabled accordingly. Also, update the comments of
pci_enable_link_state() and pci_enable_link_state_locked() for
kernel-doc, too.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=218394
Signed-off-by: Jian-Hong Pan <jhp@endlessos.org>
---
v2:
  - Power on the VMD remapped devices with pci_set_power_state_locked()
  - Prepare the PCIe LTR parameters before enable L1 Substates
  - Add note into the comments of both pci_enable_link_state() and
    pci_enable_link_state_locked() for kernel-doc.
  - The original patch set can be split as individual patches.

 drivers/pci/controller/vmd.c | 15 ++++++++++-----
 drivers/pci/pcie/aspm.c      | 10 ++++++++++
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 87b7856f375a..66e47a0dbf1a 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -751,11 +751,9 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	if (!(features & VMD_FEAT_BIOS_PM_QUIRK))
 		return 0;
 
-	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
-
 	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
 	if (!pos)
-		return 0;
+		goto out_enable_link_state;
 
 	/*
 	 * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
@@ -763,7 +761,7 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	 */
 	pci_read_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, &ltr_reg);
 	if (!!(ltr_reg & (PCI_LTR_VALUE_MASK | PCI_LTR_SCALE_MASK)))
-		return 0;
+		goto out_enable_link_state;
 
 	/*
 	 * Set the default values to the maximum required by the platform to
@@ -775,6 +773,14 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
 	pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
 	pci_info(pdev, "VMD: Default LTR value set by driver\n");
 
+out_enable_link_state:
+	/*
+	 * Make PCI devices at D0 when enable PCI-PM L1 PM Substates from
+	 * Section 5.5.4 of PCIe Base Spec Revision 6.0
+	 */
+	pci_set_power_state_locked(pdev, PCI_D0);
+	pci_enable_link_state_locked(pdev, PCIE_LINK_STATE_ALL);
+
 	return 0;
 }
 
@@ -926,7 +932,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
 				   dev_get_msi_domain(&vmd->dev->dev));
 
 	vmd_acpi_begin();
-
 	pci_scan_child_bus(vmd->bus);
 	vmd_domain_reset(vmd);
 
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index bc0bd86695ec..5f902c5552ca 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1164,6 +1164,8 @@ 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;
+	if (state & ASPM_STATE_L1_2_MASK)
+		aspm_l1ss_init(link);
 	pcie_config_aspm_link(link, policy_to_aspm_state(link));
 
 	link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
@@ -1182,6 +1184,10 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
  * touch the LNKCTL register. Also note that this does not enable states
  * disabled by pci_disable_link_state(). Return 0 or a negative errno.
  *
+ * Note: The PCIe devices of the link must be in D0, if the PCI-PM L1 PM
+ * substates are going to be enabled. From Section 5.5.4 of PCIe Base Spec
+ * Revision 6.0.
+ *
  * @pdev: PCI device
  * @state: Mask of ASPM link states to enable
  */
@@ -1198,6 +1204,10 @@ EXPORT_SYMBOL(pci_enable_link_state);
  * can't touch the LNKCTL register. Also note that this does not enable states
  * disabled by pci_disable_link_state(). Return 0 or a negative errno.
  *
+ * Note: The PCIe devices of the link must be in D0, if the PCI-PM L1 PM
+ * substates are going to be enabled. From Section 5.5.4 of PCIe Base Spec
+ * Revision 6.0.
+ *
  * @pdev: PCI device
  * @state: Mask of ASPM link states to enable
  *
-- 
2.43.0


             reply	other threads:[~2024-02-02  7:25 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02  7:11 Jian-Hong Pan [this message]
2024-02-03  0:05 ` [PATCH v2] PCI: vmd: Enable PCI PM's L1 substates of remapped PCIe Root Port and NVMe Bjorn Helgaas
2024-02-05 19:37   ` David E. Box
2024-02-05 22:42     ` Bjorn Helgaas
2024-02-05 23:05       ` David E. Box
2024-02-06 21:25         ` David E. Box
2024-02-06 23:30           ` Bjorn Helgaas
2024-02-07 19:51             ` David E. Box
2024-02-06 12:29 ` Ilpo Järvinen
2024-02-06 16:00   ` 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=20240202071110.8515-3-jhp@endlessos.org \
    --to=jhp@endlessos.org \
    --cc=cassel@kernel.org \
    --cc=david.e.box@linux.intel.com \
    --cc=dlemoal@kernel.org \
    --cc=helgaas@kernel.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=mika.westerberg@linux.intel.com \
    --cc=nirmal.patel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox