Re: [PATCH 2/2] PCI/portdrv: Place PCIe port hierarchy into D3cold at shutdown

From: Mario Limonciello
Date: Wed Dec 13 2023 - 13:42:21 EST


On 12/13/2023 12:38, Rafael J. Wysocki wrote:
On Wed, Dec 13, 2023 at 7:27 PM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:

When a system is being powered off it's important that PCIe ports
have been put into D3cold as there is no other software to turn
off the devices at S5.

If PCIe ports are left in D0 then any GPIOs toggled by the ACPI
power resources may be left enabled and devices may consume excess
power.

Isn't that a platform firmware issue?

It is the responsibility of the platform firmware to properly put the
platform into S5, including power removal from devices that are not
armed for power-on.

The specific issues that triggered this series were tied to the PCIe ports for dGPUs. There is a GPIO that is toggled by _ON or _OFF.

Windows calls _OFF as part of S5..


Cc: mpearson-lenovo@xxxxxxxxx
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
drivers/pci/pcie/portdrv.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
index 14a4b89a3b83..08238680c481 100644
--- a/drivers/pci/pcie/portdrv.c
+++ b/drivers/pci/pcie/portdrv.c
@@ -734,9 +734,14 @@ static void pcie_portdrv_remove(struct pci_dev *dev)
static void pcie_portdrv_shutdown(struct pci_dev *dev)
{
if (pci_bridge_d3_possible(dev)) {
- pm_runtime_forbid(&dev->dev);
- pm_runtime_get_noresume(&dev->dev);
- pm_runtime_dont_use_autosuspend(&dev->dev);
+ /* whole hierarchy goes into a low power state for S5 */
+ if (system_state == SYSTEM_POWER_OFF) {
+ pci_set_power_state(dev, PCI_D3cold);
+ } else {
+ pm_runtime_forbid(&dev->dev);
+ pm_runtime_get_noresume(&dev->dev);
+ pm_runtime_dont_use_autosuspend(&dev->dev);
+ }
}

Wouldn't it be better to remove power from the port after running the
code below?


Yes; I think you're right. I'll do some more testing with this.

pcie_port_device_remove(dev);
--