From 43262f001b318a0c0531e252b47bc6b07794f356 Mon Sep 17 00:00:00 2001 From: "Bolarinwa O. Saheed" Date: Fri, 19 Nov 2021 20:37:29 +0100 Subject: [PATCH 1/4] PCI/ASPM: Move pci_function_0() upward Move pci_function_0() earlier so we can use it from other functions. Link: https://lore.kernel.org/r/20211119193732.12343-2-refactormyself@gmail.com Signed-off-by: Bolarinwa O. Saheed Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/aspm.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 52c74682601a..6f128b654730 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -105,6 +105,20 @@ static const char *policy_str[] = { #define LINK_RETRAIN_TIMEOUT HZ +/* + * The L1 PM substate capability is only implemented in function 0 in a + * multi function device. + */ +static struct pci_dev *pci_function_0(struct pci_bus *linkbus) +{ + struct pci_dev *child; + + list_for_each_entry(child, &linkbus->devices, bus_list) + if (PCI_FUNC(child->devfn) == 0) + return child; + return NULL; +} + static int policy_to_aspm_state(struct pcie_link_state *link) { switch (aspm_policy) { @@ -423,20 +437,6 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) } } -/* - * The L1 PM substate capability is only implemented in function 0 in a - * multi function device. - */ -static struct pci_dev *pci_function_0(struct pci_bus *linkbus) -{ - struct pci_dev *child; - - list_for_each_entry(child, &linkbus->devices, bus_list) - if (PCI_FUNC(child->devfn) == 0) - return child; - return NULL; -} - static void pci_clear_and_set_dword(struct pci_dev *pdev, int pos, u32 clear, u32 set) { From 222578dad4731cb8932471f42a0a606116ec5398 Mon Sep 17 00:00:00 2001 From: "Saheed O. Bolarinwa" Date: Fri, 19 Nov 2021 20:37:30 +0100 Subject: [PATCH 2/4] PCI/ASPM: Stop caching link L0s, L1 exit latencies Previously we calculated the upstream and downstream L0s and L1 exit latencies of the link in pcie_aspm_cap_init() and cached them in struct pcie_link_state.latency_*. These values are only used in pcie_aspm_check_latency() where they are compared with the acceptable latencies on the link. This path is used when removing or changing the D state of the device, so it's relatively low frequency. To reduce the amount of per-link data we store, remove the latency_* entries from struct pcie_link_state and calculate the latencies directly where they are needed. Link: https://lore.kernel.org/r/20211119193732.12343-3-refactormyself@gmail.com Signed-off-by: Saheed O. Bolarinwa Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aspm.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 6f128b654730..a7a1d59c338f 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -66,9 +66,6 @@ struct pcie_link_state { u32 clkpm_default:1; /* Default Clock PM state by BIOS */ u32 clkpm_disable:1; /* Clock PM disabled */ - /* Exit latencies */ - struct aspm_latency latency_up; /* Upstream direction exit latency */ - struct aspm_latency latency_dw; /* Downstream direction exit latency */ /* * Endpoint acceptable latencies. A pcie downstream port only * has one slot under it, so at most there are 8 functions. @@ -392,7 +389,8 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value) static void pcie_aspm_check_latency(struct pci_dev *endpoint) { - u32 latency, l1_switch_latency = 0; + u32 latency, lnkcap_up, lnkcap_dw, l1_switch_latency = 0; + struct aspm_latency latency_up, latency_dw; struct aspm_latency *acceptable; struct pcie_link_state *link; @@ -405,14 +403,26 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) acceptable = &link->acceptable[PCI_FUNC(endpoint->devfn)]; while (link) { + struct pci_dev *dev = pci_function_0(link->pdev->subordinate); + + /* Read direction exit latencies */ + pcie_capability_read_dword(link->pdev, PCI_EXP_LNKCAP, + &lnkcap_up); + pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, + &lnkcap_dw); + latency_up.l0s = calc_l0s_latency(lnkcap_up); + latency_up.l1 = calc_l1_latency(lnkcap_up); + latency_dw.l0s = calc_l0s_latency(lnkcap_dw); + latency_dw.l1 = calc_l1_latency(lnkcap_dw); + /* Check upstream direction L0s latency */ if ((link->aspm_capable & ASPM_STATE_L0S_UP) && - (link->latency_up.l0s > acceptable->l0s)) + (latency_up.l0s > acceptable->l0s)) link->aspm_capable &= ~ASPM_STATE_L0S_UP; /* Check downstream direction L0s latency */ if ((link->aspm_capable & ASPM_STATE_L0S_DW) && - (link->latency_dw.l0s > acceptable->l0s)) + (latency_dw.l0s > acceptable->l0s)) link->aspm_capable &= ~ASPM_STATE_L0S_DW; /* * Check L1 latency. @@ -427,7 +437,7 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) * L1 exit latencies advertised by a device include L1 * substate latencies (and hence do not do any check). */ - latency = max_t(u32, link->latency_up.l1, link->latency_dw.l1); + latency = max_t(u32, latency_up.l1, latency_dw.l1); if ((link->aspm_capable & ASPM_STATE_L1) && (latency + l1_switch_latency > acceptable->l1)) link->aspm_capable &= ~ASPM_STATE_L1; @@ -593,8 +603,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) link->aspm_enabled |= ASPM_STATE_L0S_UP; if (parent_lnkctl & PCI_EXP_LNKCTL_ASPM_L0S) link->aspm_enabled |= ASPM_STATE_L0S_DW; - link->latency_up.l0s = calc_l0s_latency(parent_lnkcap); - link->latency_dw.l0s = calc_l0s_latency(child_lnkcap); /* Setup L1 state */ if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L1) @@ -602,8 +610,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) if (parent_lnkctl & child_lnkctl & PCI_EXP_LNKCTL_ASPM_L1) link->aspm_enabled |= ASPM_STATE_L1; - link->latency_up.l1 = calc_l1_latency(parent_lnkcap); - link->latency_dw.l1 = calc_l1_latency(child_lnkcap); /* Setup L1 substate */ pci_read_config_dword(parent, parent->l1ss + PCI_L1SS_CAP, From 6e332df7c380a7bc936275cba1ed356d9eb36b39 Mon Sep 17 00:00:00 2001 From: "Saheed O. Bolarinwa" Date: Fri, 19 Nov 2021 20:37:31 +0100 Subject: [PATCH 3/4] PCI/ASPM: Stop caching device L0s, L1 acceptable exit latencies Previously we calculated the device's acceptable L0s and L1 exit latencies in pcie_aspm_cap_init() and cached them in struct pcie_link_state. These values are only used in pcie_aspm_check_latency() where they are compared with the actual exit latencies of the link. This path is used when removing or changing the D state of the device, so it's relatively low frequency. To reduce the amount of per-link data we store, remove the acceptable[] arrays from struct pcie_link_state and calculate them directly from the already-cached Device Capabilities register when needed. [bhelgaas: use endpoint->devcap instead of reading it again] Link: https://lore.kernel.org/r/20211119193732.12343-4-refactormyself@gmail.com Signed-off-by: Saheed O. Bolarinwa Signed-off-by: Bjorn Helgaas --- drivers/pci/pcie/aspm.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index a7a1d59c338f..9faefb4d3378 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -65,12 +65,6 @@ struct pcie_link_state { u32 clkpm_enabled:1; /* Current Clock PM state */ u32 clkpm_default:1; /* Default Clock PM state by BIOS */ u32 clkpm_disable:1; /* Clock PM disabled */ - - /* - * Endpoint acceptable latencies. A pcie downstream port only - * has one slot under it, so at most there are 8 functions. - */ - struct aspm_latency acceptable[8]; }; static int aspm_disabled, aspm_force; @@ -389,7 +383,8 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value) static void pcie_aspm_check_latency(struct pci_dev *endpoint) { - u32 latency, lnkcap_up, lnkcap_dw, l1_switch_latency = 0; + u32 latency, encoding, lnkcap_up, lnkcap_dw; + u32 l1_switch_latency = 0; struct aspm_latency latency_up, latency_dw; struct aspm_latency *acceptable; struct pcie_link_state *link; @@ -400,7 +395,14 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) return; link = endpoint->bus->self->link_state; - acceptable = &link->acceptable[PCI_FUNC(endpoint->devfn)]; + + /* Calculate endpoint L0s acceptable latency */ + encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L0S) >> 6; + acceptable->l0s = calc_l0s_acceptable(encoding); + + /* Calculate endpoint L1 acceptable latency */ + encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L1) >> 9; + acceptable->l1 = calc_l1_acceptable(encoding); while (link) { struct pci_dev *dev = pci_function_0(link->pdev->subordinate); @@ -666,22 +668,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) /* Get and check endpoint acceptable latencies */ list_for_each_entry(child, &linkbus->devices, bus_list) { - u32 reg32, encoding; - struct aspm_latency *acceptable = - &link->acceptable[PCI_FUNC(child->devfn)]; if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT && pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END) continue; - pcie_capability_read_dword(child, PCI_EXP_DEVCAP, ®32); - /* Calculate endpoint L0s acceptable latency */ - encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6; - acceptable->l0s = calc_l0s_acceptable(encoding); - /* Calculate endpoint L1 acceptable latency */ - encoding = (reg32 & PCI_EXP_DEVCAP_L1) >> 9; - acceptable->l1 = calc_l1_acceptable(encoding); - pcie_aspm_check_latency(child); } } From fa285baf844303d119d657d4f97e0777acd51b8e Mon Sep 17 00:00:00 2001 From: "Saheed O. Bolarinwa" Date: Fri, 19 Nov 2021 20:37:32 +0100 Subject: [PATCH 4/4] PCI/ASPM: Remove struct aspm_latency The struct aspm_latency is now used only inside pcie_aspm_check_latency(). Replace struct aspm_latency variables with u32 variables and remove struct aspm_latency. Link: https://lore.kernel.org/r/20211119193732.12343-5-refactormyself@gmail.com Signed-off-by: Saheed O. Bolarinwa Signed-off-by: Bjorn Helgaas Reviewed-by: Christoph Hellwig --- drivers/pci/pcie/aspm.c | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 9faefb4d3378..c6d2e76e0502 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -41,11 +41,6 @@ #define ASPM_STATE_ALL (ASPM_STATE_L0S | ASPM_STATE_L1 | \ ASPM_STATE_L1SS) -struct aspm_latency { - u32 l0s; /* L0s latency (nsec) */ - u32 l1; /* L1 latency (nsec) */ -}; - struct pcie_link_state { struct pci_dev *pdev; /* Upstream component of the Link */ struct pci_dev *downstream; /* Downstream component, function 0 */ @@ -384,9 +379,9 @@ static void encode_l12_threshold(u32 threshold_us, u32 *scale, u32 *value) static void pcie_aspm_check_latency(struct pci_dev *endpoint) { u32 latency, encoding, lnkcap_up, lnkcap_dw; - u32 l1_switch_latency = 0; - struct aspm_latency latency_up, latency_dw; - struct aspm_latency *acceptable; + u32 l1_switch_latency = 0, latency_up_l0s; + u32 latency_up_l1, latency_dw_l0s, latency_dw_l1; + u32 acceptable_l0s, acceptable_l1; struct pcie_link_state *link; /* Device not in D0 doesn't need latency check */ @@ -398,11 +393,11 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) /* Calculate endpoint L0s acceptable latency */ encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L0S) >> 6; - acceptable->l0s = calc_l0s_acceptable(encoding); + acceptable_l0s = calc_l0s_acceptable(encoding); /* Calculate endpoint L1 acceptable latency */ encoding = (endpoint->devcap & PCI_EXP_DEVCAP_L1) >> 9; - acceptable->l1 = calc_l1_acceptable(encoding); + acceptable_l1 = calc_l1_acceptable(encoding); while (link) { struct pci_dev *dev = pci_function_0(link->pdev->subordinate); @@ -412,19 +407,19 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) &lnkcap_up); pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap_dw); - latency_up.l0s = calc_l0s_latency(lnkcap_up); - latency_up.l1 = calc_l1_latency(lnkcap_up); - latency_dw.l0s = calc_l0s_latency(lnkcap_dw); - latency_dw.l1 = calc_l1_latency(lnkcap_dw); + latency_up_l0s = calc_l0s_latency(lnkcap_up); + latency_up_l1 = calc_l1_latency(lnkcap_up); + latency_dw_l0s = calc_l0s_latency(lnkcap_dw); + latency_dw_l1 = calc_l1_latency(lnkcap_dw); /* Check upstream direction L0s latency */ if ((link->aspm_capable & ASPM_STATE_L0S_UP) && - (latency_up.l0s > acceptable->l0s)) + (latency_up_l0s > acceptable_l0s)) link->aspm_capable &= ~ASPM_STATE_L0S_UP; /* Check downstream direction L0s latency */ if ((link->aspm_capable & ASPM_STATE_L0S_DW) && - (latency_dw.l0s > acceptable->l0s)) + (latency_dw_l0s > acceptable_l0s)) link->aspm_capable &= ~ASPM_STATE_L0S_DW; /* * Check L1 latency. @@ -439,9 +434,9 @@ static void pcie_aspm_check_latency(struct pci_dev *endpoint) * L1 exit latencies advertised by a device include L1 * substate latencies (and hence do not do any check). */ - latency = max_t(u32, latency_up.l1, latency_dw.l1); + latency = max_t(u32, latency_up_l1, latency_dw_l1); if ((link->aspm_capable & ASPM_STATE_L1) && - (latency + l1_switch_latency > acceptable->l1)) + (latency + l1_switch_latency > acceptable_l1)) link->aspm_capable &= ~ASPM_STATE_L1; l1_switch_latency += 1000; @@ -668,7 +663,6 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist) /* Get and check endpoint acceptable latencies */ list_for_each_entry(child, &linkbus->devices, bus_list) { - if (pci_pcie_type(child) != PCI_EXP_TYPE_ENDPOINT && pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END) continue;