From: "Saheed O. Bolarinwa" <refactormyself@gmail.com>
To: helgaas@kernel.org
Cc: "Saheed O. Bolarinwa" <refactormyself@gmail.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFC PATCH v1 6/6] PCI/ASPM: Don't cache struct pcie_link_state->aspm_enabled
Date: Sat, 6 Nov 2021 18:53:53 +0100 [thread overview]
Message-ID: <20211106175353.26248-7-refactormyself@gmail.com> (raw)
In-Reply-To: <20211106175353.26248-1-refactormyself@gmail.com>
aspm_enabled in struct pcie_link_state usually holds the current
enabled states of the link. This value is set in two places:
1. pcie_aspm_cap_init(): if the passed in blacklist value holds true,
then `link->aspm_enabled = ASPM_STATE_ALL` otherwise values are
read from the register and link->aspm_enabled is calculated.
This calculation has been extracted into aspm_calc_enabled_states()
in an earlier patch in this series.
2. pcie_config_aspm_link(): when a valid state is calculated from the
value passed in. The result is later written into the register. The
calculated valid state is then store in struct
pcie_link_state->aspm_enabled.
The calculations in aspm_calc_enabled_states() reads and uses the
current state in the register. This can be called whenever
link->aspm_enabled is needed. We don't need to store the state.
For the case when blacklist value holds in pcie_aspm_cap_init(), this
value comes from pcie_aspm_init_link_state(). We can obtain this value
whenever link->aspm_enabled is needed and determine if the current
enabled states should be set to "ASPM_STATE_ALL". So also in this case
we don't need to store the enabled states, it can be obtained on the
fly.
- Remove aspm_enabled from struct pcie_link_state.
- Create a wrapper function arround aspm_calc_enabled_states().
- Replace references to aspm_enabled with a function call.
Signed-off-by: Saheed O. Bolarinwa <refactormyself@gmail.com>
---
drivers/pci/pcie/aspm.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 05ca165380e1..dce0851c6ab5 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -54,7 +54,6 @@ struct pcie_link_state {
struct list_head sibling; /* node in link_list */
/* ASPM state */
- u32 aspm_enabled:7; /* Enabled ASPM state */
u32 aspm_capable:7; /* Capable ASPM state with latency */
u32 aspm_default:7; /* Default ASPM state by BIOS */
u32 aspm_disable:7; /* Disabled ASPM state */
@@ -676,6 +675,18 @@ static u32 aspm_calc_enabled_states(struct pcie_link_state *link,
return aspm_enabled;
}
+static u32 aspm_get_enabled_states(struct pcie_link_state *link)
+{
+ u32 up_l1ss_cap, dwn_l1ss_cap;
+
+ if (pcie_aspm_sanity_check(link->pdev))
+ return ASPM_STATE_ALL;
+
+ aspm_calc_both_l1ss_caps(link, &up_l1ss_cap, &dwn_l1ss_cap);
+
+ return aspm_calc_enabled_states(link, up_l1ss_cap, dwn_l1ss_cap);
+}
+
static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
{
struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -684,8 +695,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
struct pci_bus *linkbus = parent->subordinate;
if (blacklist) {
- /* Set enabled/disable so that we will disable ASPM later */
- link->aspm_enabled = ASPM_STATE_ALL;
+ /* Set disable so that we will disable ASPM later */
link->aspm_disable = ASPM_STATE_ALL;
return;
}
@@ -719,11 +729,9 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
/* Setup L1 substate */
aspm_calc_both_l1ss_caps(link, &parent_l1ss_cap, &child_l1ss_cap);
- link->aspm_enabled = aspm_calc_enabled_states(link, parent_l1ss_cap,
- child_l1ss_cap);
-
/* Save default state */
- link->aspm_default = link->aspm_enabled;
+ link->aspm_default = aspm_calc_enabled_states(link, parent_l1ss_cap,
+ child_l1ss_cap);
aspm_support = aspm_calc_init_linkcap(parent_lnkcap, child_lnkcap,
parent_l1ss_cap, child_l1ss_cap);
@@ -762,7 +770,7 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
u32 val, enable_req;
struct pci_dev *child = link->downstream, *parent = link->pdev;
- enable_req = (link->aspm_enabled ^ state) & state;
+ enable_req = (aspm_get_enabled_states(link) ^ state) & state;
/*
* Here are the rules specified in the PCIe spec for enabling L1SS:
@@ -821,6 +829,7 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
u32 upstream = 0, dwstream = 0;
struct pci_dev *child = link->downstream, *parent = link->pdev;
struct pci_bus *linkbus = parent->subordinate;
+ u32 aspm_enabled = aspm_get_enabled_states(link);
/* Enable only the states that were not explicitly disabled */
state &= (link->aspm_capable & ~link->aspm_disable);
@@ -832,11 +841,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
/* Spec says both ports must be in D0 before enabling PCI PM substates*/
if (parent->current_state != PCI_D0 || child->current_state != PCI_D0) {
state &= ~ASPM_STATE_L1_SS_PCIPM;
- state |= (link->aspm_enabled & ASPM_STATE_L1_SS_PCIPM);
+ state |= (aspm_enabled & ASPM_STATE_L1_SS_PCIPM);
}
/* Nothing to do if the link is already in the requested state */
- if (link->aspm_enabled == state)
+ if (aspm_enabled == state)
return;
/* Convert ASPM state to upstream/downstream ASPM register state */
if (state & ASPM_STATE_L0S_UP)
@@ -863,8 +872,6 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
pcie_config_aspm_dev(child, dwstream);
if (!(state & ASPM_STATE_L1))
pcie_config_aspm_dev(parent, upstream);
-
- link->aspm_enabled = state;
}
static void pcie_config_aspm_path(struct pcie_link_state *link)
@@ -1238,7 +1245,7 @@ bool pcie_aspm_enabled(struct pci_dev *pdev)
if (!link)
return false;
- return link->aspm_enabled;
+ return aspm_get_enabled_states(link);
}
EXPORT_SYMBOL_GPL(pcie_aspm_enabled);
@@ -1249,7 +1256,8 @@ static ssize_t aspm_attr_show_common(struct device *dev,
struct pci_dev *pdev = to_pci_dev(dev);
struct pcie_link_state *link = pcie_aspm_get_link(pdev);
- return sysfs_emit(buf, "%d\n", (link->aspm_enabled & state) ? 1 : 0);
+ return sysfs_emit(buf, "%d\n",
+ (aspm_get_enabled_states(link) & state) ? 1 : 0);
}
static ssize_t aspm_attr_store_common(struct device *dev,
--
2.20.1
prev parent reply other threads:[~2021-11-06 17:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-06 17:53 [RFC PATCH v1 0/6] Remove struct pcie_link_state.aspm_* Saheed O. Bolarinwa
2021-11-06 17:53 ` [RFC PATCH v1 1/6] PCI/ASPM: Extract out L1SS_CAP calculations Saheed O. Bolarinwa
2021-11-06 17:53 ` [RFC PATCH v1 2/6] PCI/ASPM: Extract the calculation of link->aspm_support Saheed O. Bolarinwa
2021-11-06 17:53 ` [RFC PATCH v1 3/6] PCI/ASPM: Extract the calculation of link->aspm_enabled Saheed O. Bolarinwa
2021-11-06 17:53 ` [RFC PATCH v1 4/6] PCI/ASPM: Don't cache struct pcie_link_state->aspm_support Saheed O. Bolarinwa
2021-11-06 17:53 ` [RFC PATCH v1 5/6] PCI/ASPM: Move pcie_aspm_sanity_check() upwards Saheed O. Bolarinwa
2021-11-06 17:53 ` Saheed O. Bolarinwa [this message]
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=20211106175353.26248-7-refactormyself@gmail.com \
--to=refactormyself@gmail.com \
--cc=helgaas@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.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 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.