linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "David E. Box" <david.e.box@linux.intel.com>
To: nirmal.patel@linux.intel.com, jonathan.derrick@linux.dev,
	lorenzo.pieralisi@arm.com, hch@infradead.org, kw@linux.com,
	robh@kernel.org, bhelgaas@google.com,
	david.e.box@linux.intel.com, michael.a.bottini@linux.intel.com,
	rafael@kernel.org, me@adhityamohan.in
Cc: linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH V6 3/3] PCI: vmd: Configure PCIe ASPM and LTR
Date: Mon, 28 Feb 2022 20:19:43 -0800	[thread overview]
Message-ID: <20220301041943.2935892-4-david.e.box@linux.intel.com> (raw)
In-Reply-To: <20220301041943.2935892-1-david.e.box@linux.intel.com>

PCIe ports reserved for VMD use are not visible to BIOS and therefore not
configured to enable PCIE ASPM. Additionally, PCIE LTR values may be left
unset since BIOS will set a default maximum LTR value on endpoints to
ensure that they don't block SoC power management. Lack of this programming
results in high power consumption on laptops as reported in bugzilla [1].
For currently affected products, use pci_enable_default_link_state to set
the allowed link states for devices on the root ports. Also set the LTR
value to the maximum value needed for the SoC. Per the VMD hardware team
future products using VMD will enable BIOS configuration of these
capabilities. This solution is a workaround for current products that
mainly targets laptops. Support is not provided if a switch used nor for
hotplug.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=213717

Signed-off-by: Michael Bottini <michael.a.bottini@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
 V6
  - Set ASPM first before setting LTR. This is needed because some
    devices may only have LTR set by BIOS and not ASPM
  - Skip setting the LTR if the current LTR in non-zero.
 V5
  - Provide the LTR value as driver data.
  - Use DWORD for the config space write to avoid PCI WORD access bug.
  - Set ASPM links firsts, enabling all link states, before setting a
    default LTR if the capability is present
  - Add kernel message that VMD is setting the device LTR.
 V4
  - Refactor vmd_enable_apsm() to exit early, making the lines shorter
    and more readable. Suggested by Christoph.
 V3
  - No changes
 V2
  - Use return status to print pci_info message if ASPM cannot be enabled.
  - Add missing static declaration, caught by lkp@intel.com

 drivers/pci/controller/vmd.c | 66 +++++++++++++++++++++++++++++++++---
 1 file changed, 62 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index cde6e2cba210..8525bb8312f2 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -67,10 +67,19 @@ enum vmd_features {
 	 * interrupt handling.
 	 */
 	VMD_FEAT_CAN_BYPASS_MSI_REMAP		= (1 << 4),
+
+	/*
+	 * Enable ASPM on the PCIE root ports and set the default LTR of the
+	 * storage devices on platforms where these values are not configured by
+	 * BIOS. This is needed for laptops, which require these settings for
+	 * proper power management of the SoC.
+	 */
+	VMD_FEAT_BIOS_PM_QUIRK		= (1 << 5),
 };
 
 struct vmd_device_data {
 	enum vmd_features features;
+	u16 ltr;
 };
 
 static DEFINE_IDA(vmd_instance_ida);
@@ -714,6 +723,45 @@ static void vmd_copy_host_bridge_flags(struct pci_host_bridge *root_bridge,
 	vmd_bridge->native_dpc = root_bridge->native_dpc;
 }
 
+/*
+ * Enable ASPM and LTR settings on devices that aren't configured by BIOS.
+ */
+static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
+{
+	struct vmd_device_data *info = userdata;
+	u32 ltr_reg;
+	int pos;
+
+	if (!(info->features & VMD_FEAT_BIOS_PM_QUIRK))
+		return 0;
+
+	pci_enable_default_link_state(pdev, PCIE_LINK_STATE_ALL);
+
+	pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_LTR);
+	if (!pos)
+		return 0;
+
+	/*
+	 * Skip if the max snoop LTR is non-zero, indicating BIOS has set it
+	 * so the LTR quirk is not needed.
+	 */
+	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;
+
+	/*
+	 * Set the default values to the maximum required by the platform to
+	 * allow the deepest power management savings. Write as a DWORD where
+	 * the lower word is the max snoop latency and the upper word is the
+	 * max non-snoop latency.
+	 */
+	ltr_reg = (info->ltr << 16) | info->ltr;
+	pci_write_config_dword(pdev, pos + PCI_LTR_MAX_SNOOP_LAT, ltr_reg);
+	pci_info(pdev, "VMD: Default LTR set\n");
+
+	return 0;
+}
+
 static int vmd_enable_domain(struct vmd_dev *vmd, struct vmd_device_data *info)
 {
 	struct pci_sysdata *sd = &vmd->sysdata;
@@ -867,6 +915,8 @@ static int vmd_enable_domain(struct vmd_dev *vmd, struct vmd_device_data *info)
 		pci_reset_bus(child->self);
 	pci_assign_unassigned_bus_resources(vmd->bus);
 
+	pci_walk_bus(vmd->bus, vmd_pm_enable_quirk, info);
+
 	/*
 	 * VMD root buses are virtual and don't return true on pci_is_pcie()
 	 * and will fail pcie_bus_configure_settings() early. It can instead be
@@ -1016,28 +1066,36 @@ static const struct pci_device_id vmd_ids[] = {
 		(kernel_ulong_t)&(struct vmd_device_data) {
 			.features = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				    VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				    VMD_FEAT_OFFSET_FIRST_VECTOR,
+				    VMD_FEAT_OFFSET_FIRST_VECTOR |
+				    VMD_FEAT_BIOS_PM_QUIRK,
+			.ltr = 0x1003, /* 3145728 ns */
 		},
 	},
 	{ PCI_VDEVICE(INTEL, 0x4c3d),
 		(kernel_ulong_t)&(struct vmd_device_data) {
 			.features = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				    VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				    VMD_FEAT_OFFSET_FIRST_VECTOR,
+				    VMD_FEAT_OFFSET_FIRST_VECTOR |
+				    VMD_FEAT_BIOS_PM_QUIRK,
+			.ltr = 0x1003, /* 3145728 ns */
 		},
 	},
 	{ PCI_VDEVICE(INTEL, 0xa77f),
 		(kernel_ulong_t)&(struct vmd_device_data) {
 			.features = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				    VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				    VMD_FEAT_OFFSET_FIRST_VECTOR,
+				    VMD_FEAT_OFFSET_FIRST_VECTOR |
+				    VMD_FEAT_BIOS_PM_QUIRK,
+			.ltr = 0x1003, /* 3145728 ns */
 		},
 	},
 	{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_VMD_9A0B),
 		(kernel_ulong_t)&(struct vmd_device_data) {
 			.features = VMD_FEAT_HAS_MEMBAR_SHADOW_VSCAP |
 				    VMD_FEAT_HAS_BUS_RESTRICTIONS |
-				    VMD_FEAT_OFFSET_FIRST_VECTOR,
+				    VMD_FEAT_OFFSET_FIRST_VECTOR |
+				    VMD_FEAT_BIOS_PM_QUIRK,
+			.ltr = 0x1003, /* 3145728 ns */
 		},
 	},
 	{ }
-- 
2.25.1


  parent reply	other threads:[~2022-03-01  4:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-01  4:19 [PATCH V6 0/3] PCI: vmd: Enable PCIe ASPM and LTR David E. Box
2022-03-01  4:19 ` [PATCH V6 1/3] PCI/ASPM: Add pci_enable_default_link_state() David E. Box
2022-03-01  8:13   ` Christoph Hellwig
2022-03-01 13:31     ` David E. Box
2022-03-01 13:38       ` Christoph Hellwig
2022-08-26  9:16   ` Lorenzo Pieralisi
2022-08-26 17:01   ` Bjorn Helgaas
2022-03-01  4:19 ` [PATCH V6 2/3] PCI: vmd: Add vmd_device_data David E. Box
2022-08-26  9:15   ` Lorenzo Pieralisi
2022-03-01  4:19 ` David E. Box [this message]
2022-08-26 17:01   ` [PATCH V6 3/3] PCI: vmd: Configure PCIe ASPM and LTR Bjorn Helgaas
2022-09-14 20:59     ` David E. Box
2022-03-01 19:19 ` [PATCH V6 0/3] PCI: vmd: Enable " Jonathan Derrick

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=20220301041943.2935892-4-david.e.box@linux.intel.com \
    --to=david.e.box@linux.intel.com \
    --cc=bhelgaas@google.com \
    --cc=hch@infradead.org \
    --cc=jonathan.derrick@linux.dev \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=me@adhityamohan.in \
    --cc=michael.a.bottini@linux.intel.com \
    --cc=nirmal.patel@linux.intel.com \
    --cc=rafael@kernel.org \
    --cc=robh@kernel.org \
    /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;
as well as URLs for NNTP newsgroup(s).