[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251023180645.1304701-1-helgaas@kernel.org>
Date: Thu, 23 Oct 2025 13:06:26 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: linux-pci@...r.kernel.org
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>,
Christian Zigotzky <chzigotzky@...osoft.de>,
FUKAUMI Naoki <naoki@...xa.com>,
Herve Codina <herve.codina@...tlin.com>,
Diederik de Haas <diederik@...ow-tech.com>,
Dragan Simic <dsimic@...jaro.org>,
linuxppc-dev@...ts.ozlabs.org,
linux-rockchip@...ts.infradead.org,
linux-kernel@...r.kernel.org,
Bjorn Helgaas <bhelgaas@...gle.com>
Subject: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms
From: Bjorn Helgaas <bhelgaas@...gle.com>
f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
platforms") enabled Clock Power Management and L1 PM Substates, but those
features depend on CLKREQ# and possibly other device-specific
configuration. We don't know whether CLKREQ# is supported, so we shouldn't
blindly enable Clock PM and L1 PM Substates.
Enable only ASPM L0s and L1, and only when both ends of the link advertise
support for them.
Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
Reported-by: Christian Zigotzky <chzigotzky@...osoft.de>
Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
Reported-by: FUKAUMI Naoki <naoki@...xa.com>
Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
Reported-by: Herve Codina <herve.codina@...tlin.com>
Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
Reported-by: Diederik de Haas <diederik@...ow-tech.com>
Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
Tested-by: FUKAUMI Naoki <naoki@...xa.com>
---
I intend this for v6.18-rc3.
I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
on Rockchip). I hope it will fix Christian's report on powerpc, but don't
have confirmation. I think the performance regression Herve reported is
related, but this patch doesn't seem to fix it.
FUKAUMI Naoki's successful testing report:
https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/
---
drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
1 file changed, 9 insertions(+), 25 deletions(-)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 7cc8281e7011..79b965158473 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -243,8 +243,7 @@ struct pcie_link_state {
/* Clock PM state */
u32 clkpm_capable:1; /* Clock PM capable? */
u32 clkpm_enabled:1; /* Current Clock PM state */
- u32 clkpm_default:1; /* Default Clock PM state by BIOS or
- override */
+ u32 clkpm_default:1; /* Default Clock PM state by BIOS */
u32 clkpm_disable:1; /* Clock PM disabled */
};
@@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
pcie_set_clkpm_nocheck(link, enable);
}
-static void pcie_clkpm_override_default_link_state(struct pcie_link_state *link,
- int enabled)
-{
- struct pci_dev *pdev = link->downstream;
-
- /* For devicetree platforms, enable ClockPM by default */
- if (of_have_populated_dt() && !enabled) {
- link->clkpm_default = 1;
- pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
- }
-}
-
static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
{
int capable = 1, enabled = 1;
@@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
}
link->clkpm_enabled = enabled;
link->clkpm_default = enabled;
- pcie_clkpm_override_default_link_state(link, enabled);
link->clkpm_capable = capable;
link->clkpm_disable = blacklist ? 1 : 0;
}
@@ -811,19 +797,17 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
struct pci_dev *pdev = link->downstream;
u32 override;
- /* For devicetree platforms, enable all ASPM states by default */
+ /* For devicetree platforms, enable L0s and L1 by default */
if (of_have_populated_dt()) {
- link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
+ if (link->aspm_support & PCIE_LINK_STATE_L0S)
+ link->aspm_default |= PCIE_LINK_STATE_L0S;
+ if (link->aspm_support & PCIE_LINK_STATE_L1)
+ link->aspm_default |= PCIE_LINK_STATE_L1;
override = link->aspm_default & ~link->aspm_enabled;
if (override)
- pci_info(pdev, "ASPM: DT platform, enabling%s%s%s%s%s%s%s\n",
- FLAG(override, L0S_UP, " L0s-up"),
- FLAG(override, L0S_DW, " L0s-dw"),
- FLAG(override, L1, " L1"),
- FLAG(override, L1_1, " ASPM-L1.1"),
- FLAG(override, L1_2, " ASPM-L1.2"),
- FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
- FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
+ pci_info(pdev, "ASPM: default states%s%s\n",
+ FLAG(override, L0S, " L0s"),
+ FLAG(override, L1, " L1"));
}
}
--
2.43.0
Powered by blists - more mailing lists