Re: [PATCH v7 2/2] PCI: Don't put non-power manageable PCIe root ports into D3

From: Limonciello, Mario
Date: Fri Jul 14 2023 - 20:47:17 EST



On 7/14/2023 2:17 PM, Rafael J. Wysocki wrote:
Generally speaking, pci_bridge_d3_possible() is there to prevent
bridges (and PCIe ports in particular) from being put into D3hot/cold
if there are reasons to believe that it may not work.
acpi_pci_bridge_d3() is part of that.

Even if it returns 'true', the _SxD/_SxW check should still be applied
via pci_target_state() to determine whether or not the firmware allows
this particular bridge to go into D3hot/cold. So arguably, the _SxW
check in acpi_pci_bridge_d3() should not be necessary and if it makes
any functional difference, there is a bug somewhere else.
But only if it was power manageable would the _SxD/_SxW check be
applied. This issue is around the branch of pci_target_state() where
it's not power manageable and so it uses PME or it falls back to D3hot.
Well, this looks like a spec interpretation difference.

We thought that _SxD/_SxW would only be relevant for devices with ACPI
PM support, but the firmware people seem to think that those objects
are also relevant for PCI devices that don't have ACPI PM support
(because those devices are still power-manageable via PMCSR). If
Windows agrees with that viewpoint, we'll need to adjust, but not
through adding _SxW checks in random places.
I think that depends upon how you want to handle the lack of _S0W.

On these problematic devices there is no _S0W under the PCIe
root port.  As I said; Windows puts them into D0 in this case though.

So acpi_dev_power_state_for_wake should return ACPI_STATE_UNKNOWN.

Can you suggest where you think adding a acpi_dev_power_state_for_wake() does make sense?

Two areas that I think would work would be in: pci_pm_suspend_noirq() (to avoid calling pci_prepare_to_sleep)

or

directly in pci_prepare_to_sleep() to check that value in lieu of pci_target_state().