From: Mario Limonciello <mario.limonciello@amd.com>
To: "Bjorn Helgaas" <bhelgaas@google.com>,
"Hans de Goede" <hdegoede@redhat.com>,
"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: "Rafael J . Wysocki" <rjw@rjwysocki.net>,
Shyam Sundar S K <Shyam-sundar.S-k@amd.com>,
"open list:PCI SUBSYSTEM" <linux-pci@vger.kernel.org>,
Lukas Wunner <lukas@wunner.de>,
Kai-Heng Feng <kai.heng.feng@canonical.com>,
Mario Limonciello <mario.limonciello@amd.com>
Subject: [RFC v1 2/4] PCI: Add support for drivers to decide bridge D3 policy
Date: Mon, 9 Oct 2023 17:56:51 -0500 [thread overview]
Message-ID: <20231009225653.36030-3-mario.limonciello@amd.com> (raw)
In-Reply-To: <20231009225653.36030-1-mario.limonciello@amd.com>
commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
changed pci_bridge_d3_possible() so that any vendor's PCIe ports
from modern machines (>=2015) are allowed to be put into D3.
This policy change has worked for most machines, but the behavior
is improved with `pcie_port_pm=off` on some others.
Add support for drivers to register a callback that they can optionally
use to decide the policy on supported machines.
A priority is used so that if multiple drivers register a callback and
support deciding for a device the highest priority driver will return
the value.
Suggested-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/pci/pci.c | 119 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pci.h | 54 ++++++++++++++++++++
2 files changed, 173 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 59c01d68c6d5..3b8e7b936908 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -21,6 +21,7 @@
#include <linux/module.h>
#include <linux/spinlock.h>
#include <linux/string.h>
+#include <linux/list_sort.h>
#include <linux/log2.h>
#include <linux/logic_pio.h>
#include <linux/pm_wakeup.h>
@@ -54,6 +55,8 @@ unsigned int pci_pm_d3hot_delay;
static void pci_pme_list_scan(struct work_struct *work);
static LIST_HEAD(pci_pme_list);
+static LIST_HEAD(d3_possible_list);
+static DEFINE_MUTEX(d3_possible_list_mutex);
static DEFINE_MUTEX(pci_pme_list_mutex);
static DECLARE_DELAYED_WORK(pci_pme_work, pci_pme_list_scan);
@@ -3031,6 +3034,16 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
if (pci_bridge_d3_force)
return true;
+ /* check for any preferences for the bridge */
+ switch (bridge->driver_d3) {
+ case BRIDGE_PREF_DRIVER_D3:
+ return true;
+ case BRIDGE_PREF_DRIVER_D0:
+ return false;
+ default:
+ break;
+ }
+
/* Even the oldest 2010 Thunderbolt controller supports D3. */
if (bridge->is_thunderbolt)
return true;
@@ -3168,6 +3181,112 @@ void pci_d3cold_disable(struct pci_dev *dev)
}
EXPORT_SYMBOL_GPL(pci_d3cold_disable);
+static struct pci_dev *pci_get_upstream_port(struct pci_dev *pci_dev)
+{
+ struct pci_dev *bridge;
+
+ bridge = pci_upstream_bridge(pci_dev);
+ if (!bridge)
+ return NULL;
+
+ if (!pci_is_pcie(bridge))
+ return NULL;
+
+ switch (pci_pcie_type(bridge)) {
+ case PCI_EXP_TYPE_ROOT_PORT:
+ case PCI_EXP_TYPE_UPSTREAM:
+ case PCI_EXP_TYPE_DOWNSTREAM:
+ return bridge;
+ default:
+ break;
+ };
+
+ return NULL;
+}
+
+static void pci_refresh_driver_d3(void)
+{
+ struct pci_d3_driver_ops *driver;
+ struct pci_dev *pci_dev = NULL;
+ struct pci_dev *bridge;
+
+ /* 1st pass: unset any preferences set a previous invocation */
+ while ((pci_dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pci_dev))) {
+ bridge = pci_get_upstream_port(pci_dev);
+ if (!bridge)
+ continue;
+
+ if (bridge->driver_d3 != BRIDGE_PREF_UNSET)
+ bridge->driver_d3 = BRIDGE_PREF_UNSET;
+ }
+
+ pci_dev = NULL;
+
+ /* 2nd pass: update the preference and refresh bridge_d3 */
+ while ((pci_dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, pci_dev))) {
+ bridge = pci_get_upstream_port(pci_dev);
+ if (!bridge)
+ continue;
+
+ /* don't make multiple passes on the same device */
+ if (bridge->driver_d3 != BRIDGE_PREF_UNSET)
+ continue;
+
+ /* the list is pre-sorted by highest priority */
+ mutex_lock(&d3_possible_list_mutex);
+ list_for_each_entry(driver, &d3_possible_list, list_node) {
+ /* another higher priority driver already set preference */
+ if (bridge->driver_d3 != BRIDGE_PREF_UNSET)
+ break;
+ if (!driver->check)
+ continue;
+ bridge->driver_d3 = driver->check(bridge);
+ }
+ mutex_unlock(&d3_possible_list_mutex);
+
+ /* no driver set a preference */
+ if (bridge->driver_d3 == BRIDGE_PREF_UNSET)
+ bridge->driver_d3 = BRIDGE_PREF_NONE;
+
+ /* update bridge_d3 */
+ pci_bridge_d3_update(pci_dev);
+ }
+}
+
+static int pci_d3_driver_cmp(void *priv, const struct list_head *_a,
+ const struct list_head *_b)
+{
+ struct pci_d3_driver_ops *a = container_of(_a, typeof(*a), list_node);
+ struct pci_d3_driver_ops *b = container_of(_b, typeof(*b), list_node);
+
+ if (a->priority < b->priority)
+ return -1;
+ else if (a->priority > b->priority)
+ return 1;
+ return 0;
+}
+
+int pci_register_driver_d3_policy_cb(struct pci_d3_driver_ops *arg)
+{
+ mutex_lock(&d3_possible_list_mutex);
+ list_add(&arg->list_node, &d3_possible_list);
+ list_sort(NULL, &d3_possible_list, pci_d3_driver_cmp);
+ mutex_unlock(&d3_possible_list_mutex);
+ pci_refresh_driver_d3();
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pci_register_driver_d3_policy_cb);
+
+void pci_unregister_driver_d3_policy_cb(struct pci_d3_driver_ops *arg)
+{
+ mutex_lock(&d3_possible_list_mutex);
+ list_del(&arg->list_node);
+ list_sort(NULL, &d3_possible_list, pci_d3_driver_cmp);
+ mutex_unlock(&d3_possible_list_mutex);
+ pci_refresh_driver_d3();
+}
+EXPORT_SYMBOL_GPL(pci_unregister_driver_d3_policy_cb);
+
/**
* pci_pm_init - Initialize PM functions of given PCI device
* @dev: PCI device to handle.
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 8c7c2c3c6c65..c36903a4e351 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -389,6 +389,7 @@ struct pci_dev {
bit manually */
unsigned int d3hot_delay; /* D3hot->D0 transition time in ms */
unsigned int d3cold_delay; /* D3cold->D0 transition time in ms */
+ unsigned int driver_d3; /* Driver D3 request preference */
#ifdef CONFIG_PCIEASPM
struct pcie_link_state *link_state; /* ASPM link state */
@@ -1405,6 +1406,59 @@ bool pcie_relaxed_ordering_enabled(struct pci_dev *dev);
void pci_resume_bus(struct pci_bus *bus);
void pci_bus_set_current_state(struct pci_bus *bus, pci_power_t state);
+/**
+ * enum bridge_d3_pref - D3 preference of a bridge
+ * @BRIDGE_PREF_UNSET: Not configured by driver
+ * @BRIDGE_PREF_NONE: Driver does not care
+ * @BRIDGE_PREF_DRIVER_D3: Driver prefers D3
+ * @BRIDGE_PREF_DRIVER_D0: Driver prefers D0
+ */
+enum bridge_d3_pref {
+ BRIDGE_PREF_UNSET,
+ BRIDGE_PREF_NONE,
+ BRIDGE_PREF_DRIVER_D3,
+ BRIDGE_PREF_DRIVER_D0,
+};
+
+/**
+ * pci_d3_driver_ops - Operations provided by a driver to evaluate D3 policy
+ * @list_node: the linked list node
+ * @priority: the priority of the callback
+ * @check: the callback to evaluate D3 policy
+ *
+ * For the callback drivers are expected to return:
+ * BRIDGE_PREF_NONE if they can't evaluate the decision whether to support D3
+ * BRIDGE_PREF_DRIVER_D0 if they decide the device should not support D3
+ * BRIDGE_PREF_DRIVER_D3 if they decide the device should support D3
+ */
+struct pci_d3_driver_ops {
+ struct list_head list_node;
+ int priority;
+ enum bridge_d3_pref (*check)(struct pci_dev *pdev);
+};
+
+/**
+ * pci_register_driver_d3_policy_cb - Register a driver callback for configuring D3 policy
+ * @arg: driver provided structure with function pointer and priority
+ *
+ * This function can be used by drivers to register a callback that can be used
+ * to delegate the decision of whether a device should be used for D3 to that
+ * driver.
+ *
+ * Returns 0 on success, error code on failure.
+ */
+int pci_register_driver_d3_policy_cb(struct pci_d3_driver_ops *arg);
+
+/**
+ * pci_unregister_driver_d3_policy_cb - Unregister a driver callback for configuring D3 policy
+ * @arg: driver provided structure with function pointer and priority
+ *
+ * This function can be used by drivers to unregister a callback that can be used
+ * to delegate the decision of whether a device should be used for D3 to that
+ * driver.
+ */
+void pci_unregister_driver_d3_policy_cb(struct pci_d3_driver_ops *arg);
+
/* For use by arch with custom probe code */
void set_pcie_port_type(struct pci_dev *pdev);
void set_pcie_hotplug_bridge(struct pci_dev *pdev);
--
2.34.1
next prev parent reply other threads:[~2023-10-09 22:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-09 22:56 [RFC v1 0/4] Add support for drivers to decide bridge D3 policy Mario Limonciello
2023-10-09 22:56 ` [RFC v1 1/4] ACPI: x86: s2idle: Export symbol for fetching constraints for module use Mario Limonciello
2023-10-09 22:56 ` Mario Limonciello [this message]
2023-10-14 10:53 ` [RFC v1 2/4] PCI: Add support for drivers to decide bridge D3 policy Lukas Wunner
2023-10-15 18:55 ` Mario Limonciello
2023-10-09 22:56 ` [RFC v1 3/4] PCI: Check for changes in pci_bridge_d3_possible() when updating D3 Mario Limonciello
2023-10-09 22:56 ` [RFC v1 4/4] platform/x86/amd: pmc: Add support for using constraints to decide D3 policy Mario Limonciello
2023-10-16 2:11 ` Kai-Heng Feng
2023-10-16 21:34 ` Mario Limonciello
2023-10-24 7:05 ` Kai-Heng Feng
2023-10-24 19:45 ` Mario Limonciello
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=20231009225653.36030-3-mario.limonciello@amd.com \
--to=mario.limonciello@amd.com \
--cc=Shyam-sundar.S-k@amd.com \
--cc=bhelgaas@google.com \
--cc=hdegoede@redhat.com \
--cc=ilpo.jarvinen@linux.intel.com \
--cc=kai.heng.feng@canonical.com \
--cc=linux-pci@vger.kernel.org \
--cc=lukas@wunner.de \
--cc=rjw@rjwysocki.net \
/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