[PATCH v3] PCI/ACPI: PCI/ACPI: Validate devices with power resources support D3
From: Mario Limonciello
Date: Tue Oct 25 2022 - 18:11:26 EST
Firmware typically advertises that PCIe devices can support D3
by a combination of the value returned by _S0W as well as the
HotPlugSupportInD3 _DSD.
`acpi_pci_bridge_d3` looks for this combination but also contains
an assumption that if a device contains power resources it can support
D3. This was introduced from commit c6e331312ebf ("PCI/ACPI: Whitelist
hotplug ports for D3 if power managed by ACPI").
On some firmware configurations for "AMD Pink Sardine" D3 is not
supported for wake in _S0W for the PCIe root port for tunneling.
However the device will still be opted into runtime PM since
`acpi_pci_bridge_d3` returns since the ACPI device contains power
resources.
When the thunderbolt driver is loaded a device link between the USB4
router and the PCIe root port for tunneling is created where the PCIe
root port for tunneling is the consumer and the USB4 router is the
supplier. Here is a demonstration of this topology that occurs:
├─ 0000:00:03.1
| | ACPI Path: \_SB_.PCI0.GP11 (Supports "0" in _S0W)
| | Device Links: supplier:pci:0000:c4:00.5
| └─ D0 (Runtime PM enabled)
├─ 0000:00:04.1
| | ACPI Path: \_SB_.PCI0.GP12 (Supports "0" in _S0W)
| | Device Links: supplier:pci:0000:c4:00.6
| └─ D0 (Runtime PM enabled)
├─ 0000:00:08.3
| | ACPI Path: \_SB_.PCI0.GP19
| ├─ D0 (Runtime PM disabled)
| ├─ 0000:c4:00.3
| | | ACPI Path: \_SB_.PCI0.GP19.XHC3
| | | Device Links: supplier:pci:0000:c4:00.5
| | └─ D3cold (Runtime PM enabled)
| ├─ 0000:c4:00.4
| | | ACPI Path: \_SB_.PCI0.GP19.XHC4
| | | Device Links: supplier:pci:0000:c4:00.6
| | └─ D3cold (Runtime PM enabled)
| ├─ 0000:c4:00.5
| | | ACPI Path: \_SB_.PCI0.GP19.NHI0 (Supports "4" in _S0W)
| | | Device Links: consumer:pci:0000:00:03.1 consumer:pci:0000:c4:00.3
| | └─ D3cold (Runtime PM enabled)
| └─ 0000:c4:00.6
| | ACPI Path: \_SB_.PCI0.GP19.NHI1 (Supports "4" in _S0W)
| | Device Links: consumer:pci:0000:c4:00.4 consumer:pci:0000:00:04.1
| └─ D3cold (Runtime PM enabled)
Allowing the PCIe root port for tunneling to go into runtime PM (even if
it doesn't support D3) allows the USB4 router to also go into runtime PM.
The PCIe root port for tunneling stays in D0 but is in runtime PM. Due to
the device link the USB4 router transitions to D3cold when this happens.
The expectation is the USB4 router should have also remained in D0 since
the PCIe root port for tunneling remained in D0.
Instead of making this assertion from the power resources check
immediately, move the check to later on, which will have validated
that the device supports wake from D3hot or D3cold.
This fix prevents the USB4 router going into D3 when the firmware says that
the PCIe root port for tunneling can't handle it while still allowing
system that don't have the HotplugSupportInD3 _DSD to also enter D3 if they
have power resources that can wake from D3.
Fixes: dff6139015dc6 ("PCI/ACPI: Allow D3 only if Root Port can signal and wake from D3")
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v2->v3:
* Reword commit message
v1->v2:
* Just return value of acpi_pci_power_manageable (Rafael)
* Remove extra word in commit message
---
drivers/pci/pci-acpi.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index a46fec776ad77..8c6aec50dd471 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -984,10 +984,6 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
if (acpi_pci_disabled || !dev->is_hotplug_bridge)
return false;
- /* Assume D3 support if the bridge is power-manageable by ACPI. */
- if (acpi_pci_power_manageable(dev))
- return true;
-
rpdev = pcie_find_root_port(dev);
if (!rpdev)
return false;
@@ -1023,7 +1019,8 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev)
obj->integer.value == 1)
return true;
- return false;
+ /* Assume D3 support if the bridge is power-manageable by ACPI. */
+ return acpi_pci_power_manageable(dev);
}
int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
--
2.34.1