PCI: Enable ASPM state clearing regardless of policy

PATCH Applied Alex Williamson 3 participants 4 messages Mar 10, 2011 – Apr 8, 2011 PCI

Share & tools

Alex Williamson

author

2011-03-10 18:55 UTC #0 lore raw

PATCH 1/1 PCI: Enable ASPM state clearing regardless of policy

Commit 2f671e2d allowed us to clear ASPM state when the FADT tells us it isn't supported, but we don't put this into effect if the aspm_policy is set to POLICY_POWERSAVE. Enable the state to be cleared regardless of policy.

Signed-off-by Alex Williamson <[email protected]>

---

 drivers/pci/pcie/aspm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

drivers/pci/pcie/aspm.c +1 -1

index 3188cd9..eb8ac5c 100644
@@ -607,7 +607,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	 * the BIOS's expectation, we'll do so once pci_enable_device() is
 	 * called.
 	 */
-	if (aspm_policy != POLICY_POWERSAVE) {
+	if (aspm_policy != POLICY_POWERSAVE || aspm_clear_state) {
 		pcie_config_aspm_path(link);
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
 	}

Andrew Morton

2011-04-07 22:23 UTC #1 lore raw

Quoted 16 lines · On Thu, 10 Mar 2011 11:54:16 -0700 Alex Williamson…

Commit 2f671e2d allowed us to clear ASPM state when the FADT tells us it isn't supported, but we don't put this into effect if the aspm_policy is set to POLICY_POWERSAVE. Enable the state to be cleared regardless of policy.
Signed-off-by Alex Williamson <[email protected]>
---
 drivers/pci/pcie/aspm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/pci/pcie/aspm.c +0 -0
index 3188cd9..eb8ac5c 100644

drivers/pci/pcie/aspm.c void pcie_aspm_init_link_state(struct pci_dev *pdev) · line 613

Quoted 8 lines

drivers/pci/pcie/aspm.c +1 -1

@@ -607,7 +607,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	 * the BIOS's expectation, we'll do so once pci_enable_device() is
 	 * called.
 	 */
-	if (aspm_policy != POLICY_POWERSAVE) {
+	if (aspm_policy != POLICY_POWERSAVE || aspm_clear_state) {
 		pcie_config_aspm_path(link);
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
 	}

hm. why. Presumably this change has some user-observeable effect.

What is that effect, and why is it desirable? ;)

Alex Williamson

author

2011-04-08 02:24 UTC #2 lore raw

Quoted 30 lines · Andrew Morton

Quoted 16 lines · On Thu, 10 Mar 2011 11:54:16 -0700 Alex Williamson…
Commit 2f671e2d allowed us to clear ASPM state when the FADT tells us it isn't supported, but we don't put this into effect if the aspm_policy is set to POLICY_POWERSAVE. Enable the state to be cleared regardless of policy.
Signed-off-by Alex Williamson <[email protected]>
---
 drivers/pci/pcie/aspm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/pci/pcie/aspm.c +0 -0
index 3188cd9..eb8ac5c 100644
drivers/pci/pcie/aspm.c void pcie_aspm_init_link_state(struct pci_dev *pdev) · line 613
Quoted 8 lines
drivers/pci/pcie/aspm.c +1 -1
@@ -607,7 +607,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	 * the BIOS's expectation, we'll do so once pci_enable_device() is
 	 * called.
 	 */
-	if (aspm_policy != POLICY_POWERSAVE) {
+	if (aspm_policy != POLICY_POWERSAVE || aspm_clear_state) {
 		pcie_config_aspm_path(link);
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
 	}
hm. why. Presumably this change has some user-observeable effect.

Quoted 1 line

What is that effect, and why is it desirable? ;)

Without this, the ASPM clearing from 2f671e2d doesn't actually do anything when aspm_policy is set to POLICY_POWERSAVE. A recent BIOS update on one of my systems left ASPM state on a device while reporting via the FADT that ASPM isn't supported. Exactly the situation 2f671e2d is meant to fix, but didn't because the downstream kernel used powersave as it's default policy. With this, the ASPM leftovers still get clear and the system works again. Thanks,

Alex

Jesse Barnes

Applied

2011-04-08 19:50 UTC #3 lore raw

In reply to Alex Williamson

Quoted 18 lines · On Thu, 10 Mar 2011 11:54:16 -0700 Alex Williamson…

Commit 2f671e2d allowed us to clear ASPM state when the FADT tells us it isn't supported, but we don't put this into effect if the aspm_policy is set to POLICY_POWERSAVE. Enable the state to be cleared regardless of policy.
Signed-off-by Alex Williamson <[email protected]>
---
 drivers/pci/pcie/aspm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/pci/pcie/aspm.c +0 -0
index 3188cd9..eb8ac5c 100644

drivers/pci/pcie/aspm.c void pcie_aspm_init_link_state(struct pci_dev *pdev) · line 613

Quoted 8 lines

drivers/pci/pcie/aspm.c +1 -1

@@ -607,7 +607,7 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	 * the BIOS's expectation, we'll do so once pci_enable_device() is
 	 * called.
 	 */
-	if (aspm_policy != POLICY_POWERSAVE) {
+	if (aspm_policy != POLICY_POWERSAVE || aspm_clear_state) {
 		pcie_config_aspm_path(link);
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
 	}

Applied to my linux-next branch, thanks.

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center