diff mbox series

[v13,11/12] PCI: ACPI: Use device constraints to opt devices into D3 support

Message ID 20230818051319.551-12-mario.limonciello@amd.com
State New
Headers show
Series Fix wakeup problems on some AMD platforms | expand

Commit Message

Mario Limonciello Aug. 18, 2023, 5:13 a.m. UTC 
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].

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.

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@amd.com>
---
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(+)

Comments

Rafael J. Wysocki Aug. 18, 2023, 4:06 p.m. UTC | #1 
On Fri, Aug 18, 2023 at 7:15 AM Mario Limonciello
<mario.limonciello@amd.com> 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@amd.com>
> ---
> 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.
diff mbox series

Patch

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);
+}
+
 /**
  * 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.
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a4c3974340576..ac06c781a9b7c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -707,6 +707,7 @@  void pci_set_acpi_fwnode(struct pci_dev *dev);
 int pci_dev_acpi_reset(struct pci_dev *dev, bool probe);
 bool acpi_pci_power_manageable(struct pci_dev *dev);
 bool acpi_pci_bridge_d3(struct pci_dev *dev);
+bool acpi_pci_check_d3_constraint(struct pci_dev *dev);
 int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state);
 pci_power_t acpi_pci_get_power_state(struct pci_dev *dev);
 void acpi_pci_refresh_power_state(struct pci_dev *dev);
@@ -731,6 +732,10 @@  static inline bool acpi_pci_bridge_d3(struct pci_dev *dev)
 {
 	return false;
 }
+static inline bool acpi_pci_check_d3_constraint(struct pci_dev *dev)
+{
+	return false;
+}
 static inline int acpi_pci_set_power_state(struct pci_dev *dev, pci_power_t state)
 {
 	return -ENODEV;