Skip to content

Commit

Permalink
PCI/ASPM: Consolidate link state defines
Browse files Browse the repository at this point in the history 
The linux/pci.h and aspm.c files define their own sets of link state
related defines which are almost the same.

Consolidate the use of defines into those defined by linux/pci.h and expand
PCIE_LINK_STATE_L0S to match earlier ASPM_STATE_L0S that includes both
upstream and downstream bits. Rename also the defines that are internal to
aspm.c to start with PCIE_LINK_STATE for consistency.

While the PCIE_LINK_STATE_L0S BIT(0) -> (BIT(0) | BIT(1)) transformation is
not 1:1, in practice aspm.c already used ASPM_STATE_L0S that has both bits
enabled except during mapping.

While at it, place the PCIE_LINK_STATE_CLKPM define last to have more
logical grouping.

Use static_assert() to ensure PCIE_LINK_STATE_L0S is strictly equal to the
combination of PCIE_LINK_STATE_L0S_UP/DW.

Link: https://lore.kernel.org/r/20240322123952.6384-2-ilpo.jarvinen@linux.intel.com
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
  • Loading branch information
@ij-intel @bjorn-helgaas
ij-intel authored and bjorn-helgaas committed May 2, 2024
1 parent 4cece76 commit b478e16
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 88 deletions.
155 changes: 77 additions & 78 deletions drivers/pci/pcie/aspm.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/

#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/build_bug.h>
#include <linux/kernel.h>
#include <linux/limits.h>
#include <linux/math.h>
Expand Down Expand Up @@ -189,21 +191,18 @@ void pci_restore_aspm_l1ss_state(struct pci_dev *pdev)
#endif
#define MODULE_PARAM_PREFIX "pcie_aspm."

/* Note: those are not register definitions */
#define ASPM_STATE_L0S_UP (1) /* Upstream direction L0s state */
#define ASPM_STATE_L0S_DW (2) /* Downstream direction L0s state */
#define ASPM_STATE_L1 (4) /* L1 state */
#define ASPM_STATE_L1_1 (8) /* ASPM L1.1 state */
#define ASPM_STATE_L1_2 (0x10) /* ASPM L1.2 state */
#define ASPM_STATE_L1_1_PCIPM (0x20) /* PCI PM L1.1 state */
#define ASPM_STATE_L1_2_PCIPM (0x40) /* PCI PM L1.2 state */
#define ASPM_STATE_L1_SS_PCIPM (ASPM_STATE_L1_1_PCIPM | ASPM_STATE_L1_2_PCIPM)
#define ASPM_STATE_L1_2_MASK (ASPM_STATE_L1_2 | ASPM_STATE_L1_2_PCIPM)
#define ASPM_STATE_L1SS (ASPM_STATE_L1_1 | ASPM_STATE_L1_1_PCIPM |\
ASPM_STATE_L1_2_MASK)
#define ASPM_STATE_L0S (ASPM_STATE_L0S_UP | ASPM_STATE_L0S_DW)
#define ASPM_STATE_ALL (ASPM_STATE_L0S | ASPM_STATE_L1 | \
ASPM_STATE_L1SS)
/* Note: these are not register definitions */
#define PCIE_LINK_STATE_L0S_UP BIT(0) /* Upstream direction L0s state */
#define PCIE_LINK_STATE_L0S_DW BIT(1) /* Downstream direction L0s state */
static_assert(PCIE_LINK_STATE_L0S == (PCIE_LINK_STATE_L0S_UP | PCIE_LINK_STATE_L0S_DW));

#define PCIE_LINK_STATE_L1_SS_PCIPM (PCIE_LINK_STATE_L1_1_PCIPM |\
PCIE_LINK_STATE_L1_2_PCIPM)
#define PCIE_LINK_STATE_L1_2_MASK (PCIE_LINK_STATE_L1_2 |\
PCIE_LINK_STATE_L1_2_PCIPM)
#define PCIE_LINK_STATE_L1SS (PCIE_LINK_STATE_L1_1 |\
PCIE_LINK_STATE_L1_1_PCIPM |\
PCIE_LINK_STATE_L1_2_MASK)

struct pcie_link_state {
struct pci_dev *pdev; /* Upstream component of the Link */
Expand Down Expand Up @@ -275,10 +274,10 @@ static int policy_to_aspm_state(struct pcie_link_state *link)
return 0;
case POLICY_POWERSAVE:
/* Enable ASPM L0s/L1 */
return (ASPM_STATE_L0S | ASPM_STATE_L1);
return PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1;
case POLICY_POWER_SUPERSAVE:
/* Enable Everything */
return ASPM_STATE_ALL;
return PCIE_LINK_STATE_ASPM_ALL;
case POLICY_DEFAULT:
return link->aspm_default;
}
Expand Down Expand Up @@ -581,14 +580,14 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
latency_dw_l1 = calc_l1_latency(lnkcap_dw);

/* Check upstream direction L0s latency */
if ((link->aspm_capable & ASPM_STATE_L0S_UP) &&
if ((link->aspm_capable & PCIE_LINK_STATE_L0S_UP) &&
(latency_up_l0s > acceptable_l0s))
link->aspm_capable &= ~ASPM_STATE_L0S_UP;
link->aspm_capable &= ~PCIE_LINK_STATE_L0S_UP;

/* Check downstream direction L0s latency */
if ((link->aspm_capable & ASPM_STATE_L0S_DW) &&
if ((link->aspm_capable & PCIE_LINK_STATE_L0S_DW) &&
(latency_dw_l0s > acceptable_l0s))
link->aspm_capable &= ~ASPM_STATE_L0S_DW;
link->aspm_capable &= ~PCIE_LINK_STATE_L0S_DW;
/*
* Check L1 latency.
* Every switch on the path to root complex need 1
Expand All @@ -603,9 +602,9 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint)
* substate latencies (and hence do not do any check).
*/
latency = max_t(u32, latency_up_l1, latency_dw_l1);
if ((link->aspm_capable & ASPM_STATE_L1) &&
if ((link->aspm_capable & PCIE_LINK_STATE_L1) &&
(latency + l1_switch_latency > acceptable_l1))
link->aspm_capable &= ~ASPM_STATE_L1;
link->aspm_capable &= ~PCIE_LINK_STATE_L1;
l1_switch_latency += NSEC_PER_USEC;

link = link->parent;
Expand Down Expand Up @@ -741,13 +740,13 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
child_l1ss_cap &= ~PCI_L1SS_CAP_ASPM_L1_2;

if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_1)
link->aspm_support |= ASPM_STATE_L1_1;
link->aspm_support |= PCIE_LINK_STATE_L1_1;
if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_ASPM_L1_2)
link->aspm_support |= ASPM_STATE_L1_2;
link->aspm_support |= PCIE_LINK_STATE_L1_2;
if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_1)
link->aspm_support |= ASPM_STATE_L1_1_PCIPM;
link->aspm_support |= PCIE_LINK_STATE_L1_1_PCIPM;
if (parent_l1ss_cap & child_l1ss_cap & PCI_L1SS_CAP_PCIPM_L1_2)
link->aspm_support |= ASPM_STATE_L1_2_PCIPM;
link->aspm_support |= PCIE_LINK_STATE_L1_2_PCIPM;

if (parent_l1ss_cap)
pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CTL1,
Expand All @@ -757,15 +756,15 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
&child_l1ss_ctl1);

if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_1)
link->aspm_enabled |= ASPM_STATE_L1_1;
link->aspm_enabled |= PCIE_LINK_STATE_L1_1;
if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_ASPM_L1_2)
link->aspm_enabled |= ASPM_STATE_L1_2;
link->aspm_enabled |= PCIE_LINK_STATE_L1_2;
if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_1)
link->aspm_enabled |= ASPM_STATE_L1_1_PCIPM;
link->aspm_enabled |= PCIE_LINK_STATE_L1_1_PCIPM;
if (parent_l1ss_ctl1 & child_l1ss_ctl1 & PCI_L1SS_CTL1_PCIPM_L1_2)
link->aspm_enabled |= ASPM_STATE_L1_2_PCIPM;
link->aspm_enabled |= PCIE_LINK_STATE_L1_2_PCIPM;

if (link->aspm_support & ASPM_STATE_L1_2_MASK)
if (link->aspm_support & PCIE_LINK_STATE_L1_2_MASK)
aspm_calc_l12_info(link, parent_l1ss_cap, child_l1ss_cap);
}

Expand All @@ -778,8 +777,8 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)

if (blacklist) {
/* Set enabled/disable so that we will disable ASPM later */
link->aspm_enabled = ASPM_STATE_ALL;
link->aspm_disable = ASPM_STATE_ALL;
link->aspm_enabled = PCIE_LINK_STATE_ASPM_ALL;
link->aspm_disable = PCIE_LINK_STATE_ASPM_ALL;
return;
}

Expand Down Expand Up @@ -814,19 +813,19 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
* support L0s.
*/
if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)
link->aspm_support |= ASPM_STATE_L0S;
link->aspm_support |= PCIE_LINK_STATE_L0S;

if (child_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
link->aspm_enabled |= ASPM_STATE_L0S_UP;
link->aspm_enabled |= PCIE_LINK_STATE_L0S_UP;
if (parent_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S)
link->aspm_enabled |= ASPM_STATE_L0S_DW;
link->aspm_enabled |= PCIE_LINK_STATE_L0S_DW;

/* Setup L1 state */
if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L1)
link->aspm_support |= ASPM_STATE_L1;
link->aspm_support |= PCIE_LINK_STATE_L1;

if (parent_lnkctl & child_lnkctl & PCI_EXP_LNKCTL_ASPM_L1)
link->aspm_enabled |= ASPM_STATE_L1;
link->aspm_enabled |= PCIE_LINK_STATE_L1;

aspm_l1ss_init(link);

Expand Down Expand Up @@ -876,21 +875,21 @@ static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state)
* If needed, disable L1, and it gets enabled later
* in pcie_config_aspm_link().
*/
if (enable_req & (ASPM_STATE_L1_1 | ASPM_STATE_L1_2)) {
if (enable_req & (PCIE_LINK_STATE_L1_1 | PCIE_LINK_STATE_L1_2)) {
pcie_capability_clear_word(child, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_ASPM_L1);
pcie_capability_clear_word(parent, PCI_EXP_LNKCTL,
PCI_EXP_LNKCTL_ASPM_L1);
}

val = 0;
if (state & ASPM_STATE_L1_1)
if (state & PCIE_LINK_STATE_L1_1)
val |= PCI_L1SS_CTL1_ASPM_L1_1;
if (state & ASPM_STATE_L1_2)
if (state & PCIE_LINK_STATE_L1_2)
val |= PCI_L1SS_CTL1_ASPM_L1_2;
if (state & ASPM_STATE_L1_1_PCIPM)
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
val |= PCI_L1SS_CTL1_PCIPM_L1_1;
if (state & ASPM_STATE_L1_2_PCIPM)
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
val |= PCI_L1SS_CTL1_PCIPM_L1_2;

/* Enable what we need to enable */
Expand All @@ -916,29 +915,29 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
state &= (link->aspm_capable & ~link->aspm_disable);

/* Can't enable any substates if L1 is not enabled */
if (!(state & ASPM_STATE_L1))
state &= ~ASPM_STATE_L1SS;
if (!(state & PCIE_LINK_STATE_L1))
state &= ~PCIE_LINK_STATE_L1SS;

/* 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 &= ~PCIE_LINK_STATE_L1_SS_PCIPM;
state |= (link->aspm_enabled & PCIE_LINK_STATE_L1_SS_PCIPM);
}

/* Nothing to do if the link is already in the requested state */
if (link->aspm_enabled == state)
return;
/* Convert ASPM state to upstream/downstream ASPM register state */
if (state & ASPM_STATE_L0S_UP)
if (state & PCIE_LINK_STATE_L0S_UP)
dwstream |= PCI_EXP_LNKCTL_ASPM_L0S;
if (state & ASPM_STATE_L0S_DW)
if (state & PCIE_LINK_STATE_L0S_DW)
upstream |= PCI_EXP_LNKCTL_ASPM_L0S;
if (state & ASPM_STATE_L1) {
if (state & PCIE_LINK_STATE_L1) {
upstream |= PCI_EXP_LNKCTL_ASPM_L1;
dwstream |= PCI_EXP_LNKCTL_ASPM_L1;
}

if (link->aspm_capable & ASPM_STATE_L1SS)
if (link->aspm_capable & PCIE_LINK_STATE_L1SS)
pcie_config_aspm_l1ss(link, state);

/*
Expand All @@ -947,11 +946,11 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
* upstream component first and then downstream, and vice
* versa for disabling ASPM L1. Spec doesn't mention L0S.
*/
if (state & ASPM_STATE_L1)
if (state & PCIE_LINK_STATE_L1)
pcie_config_aspm_dev(parent, upstream);
list_for_each_entry(child, &linkbus->devices, bus_list)
pcie_config_aspm_dev(child, dwstream);
if (!(state & ASPM_STATE_L1))
if (!(state & PCIE_LINK_STATE_L1))
pcie_config_aspm_dev(parent, upstream);

link->aspm_enabled = state;
Expand Down Expand Up @@ -1347,18 +1346,18 @@ static int __pci_disable_link_state(struct pci_dev *pdev, int state, bool locked
down_read(&pci_bus_sem);
mutex_lock(&aspm_lock);
if (state & PCIE_LINK_STATE_L0S)
link->aspm_disable |= ASPM_STATE_L0S;
link->aspm_disable |= PCIE_LINK_STATE_L0S;
if (state & PCIE_LINK_STATE_L1)
/* L1 PM substates require L1 */
link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
link->aspm_disable |= PCIE_LINK_STATE_L1 | PCIE_LINK_STATE_L1SS;
if (state & PCIE_LINK_STATE_L1_1)
link->aspm_disable |= ASPM_STATE_L1_1;
link->aspm_disable |= PCIE_LINK_STATE_L1_1;
if (state & PCIE_LINK_STATE_L1_2)
link->aspm_disable |= ASPM_STATE_L1_2;
link->aspm_disable |= PCIE_LINK_STATE_L1_2;
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
link->aspm_disable |= ASPM_STATE_L1_1_PCIPM;
link->aspm_disable |= PCIE_LINK_STATE_L1_1_PCIPM;
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
link->aspm_disable |= ASPM_STATE_L1_2_PCIPM;
link->aspm_disable |= PCIE_LINK_STATE_L1_2_PCIPM;
pcie_config_aspm_link(link, policy_to_aspm_state(link));

if (state & PCIE_LINK_STATE_CLKPM)
Expand Down Expand Up @@ -1416,18 +1415,18 @@ static int __pci_enable_link_state(struct pci_dev *pdev, int state, bool locked)
mutex_lock(&aspm_lock);
link->aspm_default = 0;
if (state & PCIE_LINK_STATE_L0S)
link->aspm_default |= ASPM_STATE_L0S;
link->aspm_default |= PCIE_LINK_STATE_L0S;
if (state & PCIE_LINK_STATE_L1)
link->aspm_default |= ASPM_STATE_L1;
link->aspm_default |= PCIE_LINK_STATE_L1;
/* L1 PM substates require L1 */
if (state & PCIE_LINK_STATE_L1_1)
link->aspm_default |= ASPM_STATE_L1_1 | ASPM_STATE_L1;
link->aspm_default |= PCIE_LINK_STATE_L1_1 | PCIE_LINK_STATE_L1;
if (state & PCIE_LINK_STATE_L1_2)
link->aspm_default |= ASPM_STATE_L1_2 | ASPM_STATE_L1;
link->aspm_default |= PCIE_LINK_STATE_L1_2 | PCIE_LINK_STATE_L1;
if (state & PCIE_LINK_STATE_L1_1_PCIPM)
link->aspm_default |= ASPM_STATE_L1_1_PCIPM | ASPM_STATE_L1;
link->aspm_default |= PCIE_LINK_STATE_L1_1_PCIPM | PCIE_LINK_STATE_L1;
if (state & PCIE_LINK_STATE_L1_2_PCIPM)
link->aspm_default |= ASPM_STATE_L1_2_PCIPM | ASPM_STATE_L1;
link->aspm_default |= PCIE_LINK_STATE_L1_2_PCIPM | PCIE_LINK_STATE_L1;
pcie_config_aspm_link(link, policy_to_aspm_state(link));

link->clkpm_default = (state & PCIE_LINK_STATE_CLKPM) ? 1 : 0;
Expand Down Expand Up @@ -1563,12 +1562,12 @@ static ssize_t aspm_attr_store_common(struct device *dev,
if (state_enable) {
link->aspm_disable &= ~state;
/* need to enable L1 for substates */
if (state & ASPM_STATE_L1SS)
link->aspm_disable &= ~ASPM_STATE_L1;
if (state & PCIE_LINK_STATE_L1SS)
link->aspm_disable &= ~PCIE_LINK_STATE_L1;
} else {
link->aspm_disable |= state;
if (state & ASPM_STATE_L1)
link->aspm_disable |= ASPM_STATE_L1SS;
if (state & PCIE_LINK_STATE_L1)
link->aspm_disable |= PCIE_LINK_STATE_L1SS;
}

pcie_config_aspm_link(link, policy_to_aspm_state(link));
Expand All @@ -1582,12 +1581,12 @@ static ssize_t aspm_attr_store_common(struct device *dev,
#define ASPM_ATTR(_f, _s) \
static ssize_t _f##_show(struct device *dev, \
struct device_attribute *attr, char *buf) \
{ return aspm_attr_show_common(dev, attr, buf, ASPM_STATE_##_s); } \
{ return aspm_attr_show_common(dev, attr, buf, PCIE_LINK_STATE_##_s); } \
\
static ssize_t _f##_store(struct device *dev, \
struct device_attribute *attr, \
const char *buf, size_t len) \
{ return aspm_attr_store_common(dev, attr, buf, len, ASPM_STATE_##_s); }
{ return aspm_attr_store_common(dev, attr, buf, len, PCIE_LINK_STATE_##_s); }

ASPM_ATTR(l0s_aspm, L0S)
ASPM_ATTR(l1_aspm, L1)
Expand Down Expand Up @@ -1654,12 +1653,12 @@ static umode_t aspm_ctrl_attrs_are_visible(struct kobject *kobj,
struct pci_dev *pdev = to_pci_dev(dev);
struct pcie_link_state *link = pcie_aspm_get_link(pdev);
static const u8 aspm_state_map[] = {
ASPM_STATE_L0S,
ASPM_STATE_L1,
ASPM_STATE_L1_1,
ASPM_STATE_L1_2,
ASPM_STATE_L1_1_PCIPM,
ASPM_STATE_L1_2_PCIPM,
PCIE_LINK_STATE_L0S,
PCIE_LINK_STATE_L1,
PCIE_LINK_STATE_L1_1,
PCIE_LINK_STATE_L1_2,
PCIE_LINK_STATE_L1_1_PCIPM,
PCIE_LINK_STATE_L1_2_PCIPM,
};

if (aspm_disabled || !link)
Expand Down
24 changes: 14 additions & 10 deletions include/linux/pci.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1821,17 +1821,21 @@ extern bool pcie_ports_native;
#define pcie_ports_native false
#endif

#define PCIE_LINK_STATE_L0S BIT(0)
#define PCIE_LINK_STATE_L1 BIT(1)
#define PCIE_LINK_STATE_CLKPM BIT(2)
#define PCIE_LINK_STATE_L1_1 BIT(3)
#define PCIE_LINK_STATE_L1_2 BIT(4)
#define PCIE_LINK_STATE_L1_1_PCIPM BIT(5)
#define PCIE_LINK_STATE_L1_2_PCIPM BIT(6)
#define PCIE_LINK_STATE_ALL (PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1 |\
PCIE_LINK_STATE_CLKPM | PCIE_LINK_STATE_L1_1 |\
PCIE_LINK_STATE_L1_2 | PCIE_LINK_STATE_L1_1_PCIPM |\
#define PCIE_LINK_STATE_L0S (BIT(0) | BIT(1)) /* Upstr/dwnstr L0s */
#define PCIE_LINK_STATE_L1 BIT(2) /* L1 state */
#define PCIE_LINK_STATE_L1_1 BIT(3) /* ASPM L1.1 state */
#define PCIE_LINK_STATE_L1_2 BIT(4) /* ASPM L1.2 state */
#define PCIE_LINK_STATE_L1_1_PCIPM BIT(5) /* PCI-PM L1.1 state */
#define PCIE_LINK_STATE_L1_2_PCIPM BIT(6) /* PCI-PM L1.2 state */
#define PCIE_LINK_STATE_ASPM_ALL (PCIE_LINK_STATE_L0S |\
PCIE_LINK_STATE_L1 |\
PCIE_LINK_STATE_L1_1 |\
PCIE_LINK_STATE_L1_2 |\
PCIE_LINK_STATE_L1_1_PCIPM |\
PCIE_LINK_STATE_L1_2_PCIPM)
#define PCIE_LINK_STATE_CLKPM BIT(7)
#define PCIE_LINK_STATE_ALL (PCIE_LINK_STATE_ASPM_ALL |\
PCIE_LINK_STATE_CLKPM)

#ifdef CONFIG_PCIEASPM
int pci_disable_link_state(struct pci_dev *pdev, int state);
Expand Down

0 comments on commit b478e16

Please sign in to comment.