On Fri, Aug 18, 2023 at 7:15 AM Mario Limonciello <mario.limonciello@xxxxxxx> wrote: > > In Windows, systems that support Modern Standby specify hardware > pre-conditions for the SoC to be able to achieve the lowest power state > by using 'device constraints' in a SOC specific "Power Engine > Plugin" (PEP) [1] [2]. The connection between the device power states and the "PEP constraints" is still rather unclear after reading the above. I would add something like "For each device, the constraint is the minimum (shallowest) power state in which the device can be for the platform to be still able to achieve significant energy conservation in a system-wide low-power idle configuration." > Device constraints are specified in the return value for a _DSM of > a PNP0D80 device, and Linux enumerates the constraints during probing. > > In cases that the existing logic for pci_bridge_d3_possible() may not > enable D3 support query for any constraints specified by the device. > If the constraints specify D3 support, then use D3 for the PCI device. The above paragraph is not particularly clear IMO. I would say something like this instead: "For PCI bridges (including PCIe ports), the constraints may be regarded as an additional source of information regarding the power state to put the device into when it is suspended. In particular, if the constraint for a given PCI bridge is D3hot, the platform regards D3hot as a valid power state for the bridge and it is reasonable to expect that there won't be any side effects caused by putting the bridge into that power state. Accordingly, take the low-power S0 idle (LPS0) constraints into account when deciding whether or not to allow a given PCI bridge to be put into D3." > Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram [1] > Link: https://uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf [2] > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > v12->v13: > * Move back to PCI code > * Trim commit message > v11->v12: > * Adjust for dropped patch 8/9 from v11. > * Update comment > v10->v11: > * Fix kernel kernel build robot errors and various sparse warnings > related to new handling of pci_power_t. > * Use the new helpers introduced in previous patches > --- > drivers/pci/pci-acpi.c | 14 ++++++++++++++ > drivers/pci/pci.c | 12 ++++++++++++ > drivers/pci/pci.h | 5 +++++ > 3 files changed, 31 insertions(+) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index b5b65cdfa3b8b..bcc76e9d399c5 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -1081,6 +1081,20 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) > return false; > } > > +/** > + * acpi_pci_check_d3_constraint - Check if device specifies a D3 platform constraint > + * @dev: PCI device to check > + * > + * This function checks if the platform specifies that a given PCI device should > + * be put into D3 to satisfy a low power platform constraint > + * > + * Returns: TRUE if constraint for D3hot or deeper, FALSE otherwise. > + */ > +bool acpi_pci_check_d3_constraint(struct pci_dev *dev) > +{ > + return acpi_get_lps0_constraint(&dev->dev) >= ACPI_STATE_D3_HOT; > +} > + > static void acpi_pci_config_space_access(struct pci_dev *dev, bool enable) > { > int val = enable ? ACPI_REG_CONNECT : ACPI_REG_DISCONNECT; > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 051e88ee64c63..0fc8d35154f97 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -1082,6 +1082,14 @@ static inline bool platform_pci_bridge_d3(struct pci_dev *dev) > return acpi_pci_bridge_d3(dev); > } > > +static inline bool platform_check_d3_constraint(struct pci_dev *dev) > +{ > + if (pci_use_mid_pm()) > + return false; > + > + return acpi_pci_check_d3_constraint(dev); > +} As I said elsewhere, the above looks redundant to me. I would rather make acpi_pci_bridge_d3() use the PEP constraints as an additional source of information. > + > /** > * pci_update_current_state - Read power state of given device and cache it > * @dev: PCI device to handle. > @@ -3036,6 +3044,10 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > if (dmi_check_system(bridge_d3_blacklist)) > return false; > > + /* the platform specifies in LPS0 constraints to use D3 */ > + if (platform_check_d3_constraint(bridge)) > + return true; > + > /* > * It is safe to put Intel PCIe ports from 2015 or newer > * to D3.
