linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/7] Fix wakeup problems on some AMD platforms
@ 2023-08-04 21:01 Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 1/7] ACPI: Add comments to clarify some #ifdef statements Mario Limonciello
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

Problems have been reported on AMD laptops with suspend/resume
where particular root ports are put into D3 and then the system is unable
to resume properly.

The issue boils down to the currently selected kernel policy for root port
behavior at suspend time:
0) If the machine is from 2015 or later
1) If a PCIe root port is power manageable by the platform then platform
   will be used to determine the power state of the root port at suspend.
2) If the PCIe root is not power manageable by the platform then the kernel
   will check if it was configured to wakeup.
3) If it was, then it will be put into the deepest state that supports
   wakeup from PME.
4) If it wasn't, then it will be put into D3hot.

This patch adjusts it so that device constraints for low power idle are
considered if the device is not power manageable by the platform.

Mario Limonciello (7):
  ACPI: Add comments to clarify some #ifdef statements
  ACPI: Adjust #ifdef for *_lps0_dev use
  ACPI: x86: s2idle: Fix a logic error parsing AMD constraints table
  ACPI: x86: s2idle: Add more debugging for AMD constraints parsing
  ACPI: x86: s2idle: Store if constraint is enabled
  ACPI: x86: s2idle: Add a function to get constraints for a device
  PCI: Use device constraints to decide PCI target state fallback policy

 drivers/acpi/x86/s2idle.c | 75 ++++++++++++++++++++++++++++-----------
 drivers/pci/pci.c         | 15 ++++++++
 include/linux/acpi.h      | 14 +++++---
 3 files changed, 79 insertions(+), 25 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v10 1/7] ACPI: Add comments to clarify some #ifdef statements
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 2/7] ACPI: Adjust #ifdef for *_lps0_dev use Mario Limonciello
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

With nested #ifdef statements it's sometimes difficult to tell
which code goes with which statement.  One comment was wrong,
so fix it and add another comment to clarify another.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * no changes
---
 include/linux/acpi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 641dc48439873..0d5277b7c6323 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1117,10 +1117,10 @@ static inline void arch_reserve_mem_area(acpi_physical_address addr,
 					  size_t size)
 {
 }
-#endif /* CONFIG_X86 */
+#endif /* CONFIG_IA64 */
 #else
 #define acpi_os_set_prepare_sleep(func, pm1a_ctrl, pm1b_ctrl) do { } while (0)
-#endif
+#endif /* CONFIG_ACPI */
 
 #if defined(CONFIG_ACPI) && defined(CONFIG_PM)
 int acpi_dev_suspend(struct device *dev, bool wakeup);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 2/7] ACPI: Adjust #ifdef for *_lps0_dev use
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 1/7] ACPI: Add comments to clarify some #ifdef statements Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 3/7] ACPI: x86: s2idle: Fix a logic error parsing AMD constraints table Mario Limonciello
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

The #ifdef currently is guarded against CONFIG_X86, but these are
actually sleep related functions so they should be tied to
CONFIG_ACPI_SLEEP.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * split from other patches
---
 include/linux/acpi.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0d5277b7c6323..13a0fca3539f0 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1100,7 +1100,7 @@ void acpi_os_set_prepare_extended_sleep(int (*func)(u8 sleep_state,
 
 acpi_status acpi_os_prepare_extended_sleep(u8 sleep_state,
 					   u32 val_a, u32 val_b);
-#ifdef CONFIG_X86
+#if defined(CONFIG_ACPI_SLEEP) && defined(CONFIG_X86)
 struct acpi_s2idle_dev_ops {
 	struct list_head list_node;
 	void (*prepare)(void);
@@ -1109,7 +1109,7 @@ struct acpi_s2idle_dev_ops {
 };
 int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg);
 void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg);
-#endif /* CONFIG_X86 */
+#endif /* CONFIG_ACPI_SLEEP && CONFIG_X86 */
 #ifndef CONFIG_IA64
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
 #else
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 3/7] ACPI: x86: s2idle: Fix a logic error parsing AMD constraints table
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 1/7] ACPI: Add comments to clarify some #ifdef statements Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 2/7] ACPI: Adjust #ifdef for *_lps0_dev use Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 4/7] ACPI: x86: s2idle: Add more debugging for AMD constraints parsing Mario Limonciello
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

The constraints table should be resetting the `list` object
after running through all of `info_obj` iterations.

This adjusts whitespace as well as less code will now be included
with each loop.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * split from other patches
---
 drivers/acpi/x86/s2idle.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index ce62e61a9605e..b566b3aa09388 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -129,12 +129,12 @@ static void lpi_device_get_constraints_amd(void)
 				struct lpi_constraints *list;
 				acpi_status status;
 
+				list = &lpi_constraints_table[lpi_constraints_table_size];
+				list->min_dstate = -EINVAL;
+
 				for (k = 0; k < info_obj->package.count; ++k) {
 					union acpi_object *obj = &info_obj->package.elements[k];
 
-					list = &lpi_constraints_table[lpi_constraints_table_size];
-					list->min_dstate = -1;
-
 					switch (k) {
 					case 0:
 						dev_info.enabled = obj->integer.value;
@@ -149,26 +149,25 @@ static void lpi_device_get_constraints_amd(void)
 						dev_info.min_dstate = obj->integer.value;
 						break;
 					}
+				}
 
-					if (!dev_info.enabled || !dev_info.name ||
-					    !dev_info.min_dstate)
-						continue;
+				if (!dev_info.enabled || !dev_info.name ||
+				    !dev_info.min_dstate)
+					continue;
 
-					status = acpi_get_handle(NULL, dev_info.name,
-								 &list->handle);
-					if (ACPI_FAILURE(status))
-						continue;
+				status = acpi_get_handle(NULL, dev_info.name, &list->handle);
+				if (ACPI_FAILURE(status))
+					continue;
 
-					acpi_handle_debug(lps0_device_handle,
-							  "Name:%s\n", dev_info.name);
+				acpi_handle_debug(lps0_device_handle,
+						  "Name:%s\n", dev_info.name);
 
-					list->min_dstate = dev_info.min_dstate;
+				list->min_dstate = dev_info.min_dstate;
 
-					if (list->min_dstate < 0) {
-						acpi_handle_debug(lps0_device_handle,
-								  "Incomplete constraint defined\n");
-						continue;
-					}
+				if (list->min_dstate < 0) {
+					acpi_handle_debug(lps0_device_handle,
+							  "Incomplete constraint defined\n");
+					continue;
 				}
 				lpi_constraints_table_size++;
 			}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 4/7] ACPI: x86: s2idle: Add more debugging for AMD constraints parsing
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
                   ` (2 preceding siblings ...)
  2023-08-04 21:01 ` [PATCH v10 3/7] ACPI: x86: s2idle: Fix a logic error parsing AMD constraints table Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 5/7] ACPI: x86: s2idle: Store if constraint is enabled Mario Limonciello
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

While parsing the constraints show all the entries for the table
to aid with debugging other problems later.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * split from other patches
---
 drivers/acpi/x86/s2idle.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index b566b3aa09388..91cd6f8b8ade0 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -160,7 +160,11 @@ static void lpi_device_get_constraints_amd(void)
 					continue;
 
 				acpi_handle_debug(lps0_device_handle,
-						  "Name:%s\n", dev_info.name);
+						  "Name:%s, Enabled: %d, States: %d, MinDstate: %d\n",
+						  dev_info.name,
+						  dev_info.enabled,
+						  dev_info.function_states,
+						  dev_info.min_dstate);
 
 				list->min_dstate = dev_info.min_dstate;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 5/7] ACPI: x86: s2idle: Store if constraint is enabled
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
                   ` (3 preceding siblings ...)
  2023-08-04 21:01 ` [PATCH v10 4/7] ACPI: x86: s2idle: Add more debugging for AMD constraints parsing Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 6/7] ACPI: x86: s2idle: Add a function to get constraints for a device Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

Constraints are currently only stored when enabled.  To enable
the ability to check if constraints are present they need to be
stored even if disabled.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * split from other patches
---
 drivers/acpi/x86/s2idle.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 91cd6f8b8ade0..0c8101acc92ef 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -78,6 +78,7 @@ struct lpi_device_constraint {
 struct lpi_constraints {
 	acpi_handle handle;
 	int min_dstate;
+	bool enabled;
 };
 
 /* AMD Constraint package structure */
@@ -151,8 +152,7 @@ static void lpi_device_get_constraints_amd(void)
 					}
 				}
 
-				if (!dev_info.enabled || !dev_info.name ||
-				    !dev_info.min_dstate)
+				if (!dev_info.name)
 					continue;
 
 				status = acpi_get_handle(NULL, dev_info.name, &list->handle);
@@ -173,6 +173,7 @@ static void lpi_device_get_constraints_amd(void)
 							  "Incomplete constraint defined\n");
 					continue;
 				}
+				list->enabled = dev_info.enabled;
 				lpi_constraints_table_size++;
 			}
 		}
@@ -235,7 +236,7 @@ static void lpi_device_get_constraints(void)
 			}
 		}
 
-		if (!info.enabled || !info.package || !info.name)
+		if (!info.package || !info.name)
 			continue;
 
 		constraint = &lpi_constraints_table[lpi_constraints_table_size];
@@ -247,7 +248,7 @@ static void lpi_device_get_constraints(void)
 		acpi_handle_debug(lps0_device_handle,
 				  "index:%d Name:%s\n", i, info.name);
 
-		constraint->min_dstate = -1;
+		constraint->min_dstate = -EINVAL;
 
 		for (j = 0; j < package_count; ++j) {
 			union acpi_object *info_obj = &info.package[j];
@@ -284,7 +285,7 @@ static void lpi_device_get_constraints(void)
 					  "Incomplete constraint defined\n");
 			continue;
 		}
-
+		constraint->enabled = info.enabled;
 		lpi_constraints_table_size++;
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 6/7] ACPI: x86: s2idle: Add a function to get constraints for a device
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
                   ` (4 preceding siblings ...)
  2023-08-04 21:01 ` [PATCH v10 5/7] ACPI: x86: s2idle: Store if constraint is enabled Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
  6 siblings, 0 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

Other parts of the kernel may use constraints information to make
decisions on what power state to put a device into.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * split from other patches
 * kerneldoc fixes
 * move debug statement to this function
---
 drivers/acpi/x86/s2idle.c | 29 +++++++++++++++++++++++++++++
 include/linux/acpi.h      |  6 ++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 0c8101acc92ef..2a1a482f4803a 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -295,6 +295,35 @@ static void lpi_device_get_constraints(void)
 	ACPI_FREE(out_obj);
 }
 
+/**
+ * acpi_get_lps0_constraint - get any LPS0 constraint for a device
+ * @dev: device to get constraints for
+ *
+ * Returns:
+ *  - If the constraint is enabled, the value for constraint.
+ *  - If the constraint is disabled, 0.
+ *  - Otherwise, -ENODEV.
+ */
+int acpi_get_lps0_constraint(struct device *dev)
+{
+	int i;
+
+	for (i = 0; i < lpi_constraints_table_size; ++i) {
+		static struct lpi_constraints *entry;
+		int val;
+
+		entry = &lpi_constraints_table[i];
+		if (!device_match_acpi_handle(dev, entry->handle))
+			continue;
+		val = entry->enabled ? entry->min_dstate : 0;
+		acpi_handle_debug(entry->handle,
+				  "ACPI device constraint: %d\n", val);
+		return val;
+	}
+
+	return -ENODEV;
+}
+
 static void lpi_check_constraints(void)
 {
 	int i;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 13a0fca3539f0..99458502a7510 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1109,6 +1109,12 @@ struct acpi_s2idle_dev_ops {
 };
 int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg);
 void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg);
+int acpi_get_lps0_constraint(struct device *dev);
+#else /* CONFIG_ACPI_SLEEP && CONFIG_X86 */
+static inline int acpi_get_lps0_constraint(struct device *dev)
+{
+	return -ENODEV;
+}
 #endif /* CONFIG_ACPI_SLEEP && CONFIG_X86 */
 #ifndef CONFIG_IA64
 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
  2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
                   ` (5 preceding siblings ...)
  2023-08-04 21:01 ` [PATCH v10 6/7] ACPI: x86: s2idle: Add a function to get constraints for a device Mario Limonciello
@ 2023-08-04 21:01 ` Mario Limonciello
  2023-08-05  5:22   ` kernel test robot
                     ` (2 more replies)
  6 siblings, 3 replies; 11+ messages in thread
From: Mario Limonciello @ 2023-08-04 21:01 UTC (permalink / raw)
  To: Rafael J . Wysocki, Mika Westerberg, Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Andy Shevchenko, linux-acpi,
	Kuppuswamy Sathyanarayanan, Iain Lane, Shyam-sundar S-k,
	Mario Limonciello

Since commit 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
PCIe ports from modern machines (>=2015) are allowed to be put into D3 by
storing a value to the `bridge_d3` variable in the `struct pci_dev`
structure.

pci_power_manageable() uses this variable to indicate a PCIe port can
enter D3.
pci_pm_suspend_noirq() uses the return from pci_power_manageable() to
decide whether to try to put a device into its target state for a sleep
cycle via pci_prepare_to_sleep().

For devices that support D3, the target state is selected by this policy:
1. If platform_pci_power_manageable():
   Use platform_pci_choose_state()
2. If the device is armed for wakeup:
   Select the deepest D-state that supports a PME.
3. Else:
   Use D3hot.

Devices are considered power manageable by the platform when they have
one or more objects described in the table in section 7.3 of the ACPI 6.5
specification.

When devices are not considered power manageable; specs are ambiguous as
to what should happen.  In this situation Windows 11 leaves PCIe
ports in D0 while Linux puts them into D3 due to the above mentioned
commit.

In Windows systems that support Modern Standby specify hardware
pre-conditions for the SoC to achieve the lowest power state by device
constraints in a SOC specific "Power Engine Plugin" (PEP) [2] [3].
They can be marked as disabled or enabled and when enabled can specify
the minimum power state required for an ACPI device.

When it is ambiguous what should happen, adjust the logic for
pci_target_state() to check whether a device constraint is present
and enabled.
* If power manageable by ACPI use this to get to select target state
* If a device constraint is present but disabled then choose D0
* If a device constraint is present and enabled then use it
* If a device constraint is not present, then continue to existing
logic (if marked for wakeup use deepest state that PME works)
* If not marked for wakeup choose D3hot

Link: https://uefi.org/specs/ACPI/6.5/07_Power_and_Performance_Mgmt.html#device-power-management-objects [1]
Link: https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/platform-design-for-modern-standby#low-power-core-silicon-cpu-soc-dram [2]
Link: https://uefi.org/sites/default/files/resources/Intel_ACPI_Low_Power_S0_Idle.pdf [3]
Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend")
Reported-by: Iain Lane <iain@orangesquash.org.uk>
Closes: https://forums.lenovo.com/t5/Ubuntu/Z13-can-t-resume-from-suspend-with-external-USB-keyboard/m-p/5217121
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v9->v10:
 * kerneldoc fixes
 * split into more patches
 * adjust return variable handling
 * Adjust call-site to avoid problems for devices already in d3cold
---
 drivers/pci/pci.c | 47 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 60230da957e0c..108eacc4f8dd9 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 int platform_get_constraint(struct pci_dev *dev)
+{
+	if (pci_use_mid_pm())
+		return -ENODEV;
+
+	return acpi_get_lps0_constraint(&dev->dev);
+}
+
 /**
  * pci_update_current_state - Read power state of given device and cache it
  * @dev: PCI device to handle.
@@ -2660,6 +2668,20 @@ int pci_wake_from_d3(struct pci_dev *dev, bool enable)
 }
 EXPORT_SYMBOL(pci_wake_from_d3);
 
+/*
+ * Find the deepest state from which the device can generate
+ * PME#.
+ */
+static pci_power_t pci_get_wake_pme_state(struct pci_dev *dev)
+{
+	pci_power_t state = PCI_D3hot;
+
+	while (state && !(dev->pme_support & (1 << state)))
+		state--;
+
+	return state;
+}
+
 /**
  * pci_target_state - find an appropriate low power state for a given PCI dev
  * @dev: PCI device
@@ -2671,6 +2693,8 @@ EXPORT_SYMBOL(pci_wake_from_d3);
  */
 static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
 {
+	pci_power_t constraint;
+
 	if (platform_pci_power_manageable(dev)) {
 		/*
 		 * Call the platform to find the target state for the device.
@@ -2701,23 +2725,20 @@ static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
 	else if (!dev->pm_cap)
 		return PCI_D0;
 
-	if (wakeup && dev->pme_support) {
-		pci_power_t state = PCI_D3hot;
+	/* if platform indicates preferred state device constraint, use it */
+	constraint = platform_get_constraint(dev);
+	if (constraint < 0)
+		constraint = PCI_D3hot;
 
-		/*
-		 * Find the deepest state from which the device can generate
-		 * PME#.
-		 */
-		while (state && !(dev->pme_support & (1 << state)))
-			state--;
+	if (wakeup && dev->pme_support) {
+		pci_power_t pme_state = pci_get_wake_pme_state(dev);
 
-		if (state)
-			return state;
-		else if (dev->pme_support & 1)
-			return PCI_D0;
+		/* pick the lesser of any specified constraints */
+		if (pme_state < constraint)
+			constraint = pme_state;
 	}
 
-	return PCI_D3hot;
+	return constraint;
 }
 
 /**
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
  2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
@ 2023-08-05  5:22   ` kernel test robot
  2023-08-05 10:27   ` kernel test robot
  2023-08-05 10:27   ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-08-05  5:22 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Mika Westerberg,
	Bjorn Helgaas
  Cc: llvm, oe-kbuild-all, linux-pci, linux-kernel, Andy Shevchenko,
	linux-acpi, Kuppuswamy Sathyanarayanan, Iain Lane,
	Shyam-sundar S-k, Mario Limonciello

Hi Mario,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on pci/next pci/for-linus westeri-thunderbolt/next linus/master v6.5-rc4 next-20230804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-Add-comments-to-clarify-some-ifdef-statements/20230805-050559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20230804210129.5356-8-mario.limonciello%40amd.com
patch subject: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
config: mips-randconfig-r032-20230731 (https://download.01.org/0day-ci/archive/20230805/202308051340.k7mCXXL3-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230805/202308051340.k7mCXXL3-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308051340.k7mCXXL3-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/pci/pci.c:1090:9: error: call to undeclared function 'acpi_get_lps0_constraint'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    1090 |         return acpi_get_lps0_constraint(&dev->dev);
         |                ^
   1 error generated.


vim +/acpi_get_lps0_constraint +1090 drivers/pci/pci.c

  1084	
  1085	static inline int platform_get_constraint(struct pci_dev *dev)
  1086	{
  1087		if (pci_use_mid_pm())
  1088			return -ENODEV;
  1089	
> 1090		return acpi_get_lps0_constraint(&dev->dev);
  1091	}
  1092	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
  2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
  2023-08-05  5:22   ` kernel test robot
@ 2023-08-05 10:27   ` kernel test robot
  2023-08-05 10:27   ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-08-05 10:27 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Mika Westerberg,
	Bjorn Helgaas
  Cc: oe-kbuild-all, linux-pci, linux-kernel, Andy Shevchenko,
	linux-acpi, Kuppuswamy Sathyanarayanan, Iain Lane,
	Shyam-sundar S-k, Mario Limonciello

Hi Mario,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on pci/next pci/for-linus westeri-thunderbolt/next linus/master v6.5-rc4 next-20230804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-Add-comments-to-clarify-some-ifdef-statements/20230805-050559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20230804210129.5356-8-mario.limonciello%40amd.com
patch subject: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
config: x86_64-randconfig-r072-20230730 (https://download.01.org/0day-ci/archive/20230805/202308051831.tHlat46E-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230805/202308051831.tHlat46E-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308051831.tHlat46E-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   drivers/pci/pci.c:1117:36: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted pci_power_t [usertype] current_state @@     got int @@
   drivers/pci/pci.c:1117:36: sparse:     expected restricted pci_power_t [usertype] current_state
   drivers/pci/pci.c:1117:36: sparse:     got int
   drivers/pci/pci.c:1267:15: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted pci_power_t [assigned] [usertype] state @@     got int @@
   drivers/pci/pci.c:1267:15: sparse:     expected restricted pci_power_t [assigned] [usertype] state
   drivers/pci/pci.c:1267:15: sparse:     got int
   drivers/pci/pci.c:1269:50: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1269:69: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1317:28: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted pci_power_t [usertype] current_state @@     got int @@
   drivers/pci/pci.c:1317:28: sparse:     expected restricted pci_power_t [usertype] current_state
   drivers/pci/pci.c:1317:28: sparse:     got int
   drivers/pci/pci.c:1392:16: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1392:35: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1392:52: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1392:70: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1414:15: sparse: sparse: invalid assignment: |=
   drivers/pci/pci.c:1414:15: sparse:    left side has type unsigned short
   drivers/pci/pci.c:1414:15: sparse:    right side has type restricted pci_power_t
   drivers/pci/pci.c:1426:28: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted pci_power_t [usertype] current_state @@     got int @@
   drivers/pci/pci.c:1426:28: sparse:     expected restricted pci_power_t [usertype] current_state
   drivers/pci/pci.c:1426:28: sparse:     got int
   drivers/pci/pci.c:1457:13: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1457:21: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1459:18: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1459:26: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1482:13: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1482:22: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:1840:38: sparse: sparse: array of flexible structures
   drivers/pci/pci.c:2416:44: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2679:52: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2680:22: sparse: sparse: restricted pci_power_t degrades to integer
>> drivers/pci/pci.c:2729:20: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted pci_power_t [usertype] constraint @@     got int @@
   drivers/pci/pci.c:2729:20: sparse:     expected restricted pci_power_t [usertype] constraint
   drivers/pci/pci.c:2729:20: sparse:     got int
   drivers/pci/pci.c:2730:13: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2737:21: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2737:33: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2904:20: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2904:38: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2927:49: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:2927:67: sparse: sparse: restricted pci_power_t degrades to integer
   drivers/pci/pci.c:4894:13: sparse: sparse: invalid assignment: |=
   drivers/pci/pci.c:4894:13: sparse:    left side has type unsigned short
   drivers/pci/pci.c:4894:13: sparse:    right side has type restricted pci_power_t
   drivers/pci/pci.c:4899:13: sparse: sparse: invalid assignment: |=
   drivers/pci/pci.c:4899:13: sparse:    left side has type unsigned short
   drivers/pci/pci.c:4899:13: sparse:    right side has type restricted pci_power_t
   drivers/pci/pci.c:1064:24: sparse: sparse: incorrect type in return expression (different base types) @@     expected int @@     got restricted pci_power_t [usertype] @@
   drivers/pci/pci.c:1064:24: sparse:     expected int
   drivers/pci/pci.c:1064:24: sparse:     got restricted pci_power_t [usertype]
   drivers/pci/pci.c:1064:24: sparse: sparse: incorrect type in return expression (different base types) @@     expected int @@     got restricted pci_power_t [usertype] @@
   drivers/pci/pci.c:1064:24: sparse:     expected int
   drivers/pci/pci.c:1064:24: sparse:     got restricted pci_power_t [usertype]

vim +2729 drivers/pci/pci.c

  2684	
  2685	/**
  2686	 * pci_target_state - find an appropriate low power state for a given PCI dev
  2687	 * @dev: PCI device
  2688	 * @wakeup: Whether or not wakeup functionality will be enabled for the device.
  2689	 *
  2690	 * Use underlying platform code to find a supported low power state for @dev.
  2691	 * If the platform can't manage @dev, return the deepest state from which it
  2692	 * can generate wake events, based on any available PME info.
  2693	 */
  2694	static pci_power_t pci_target_state(struct pci_dev *dev, bool wakeup)
  2695	{
  2696		pci_power_t constraint;
  2697	
  2698		if (platform_pci_power_manageable(dev)) {
  2699			/*
  2700			 * Call the platform to find the target state for the device.
  2701			 */
  2702			pci_power_t state = platform_pci_choose_state(dev);
  2703	
  2704			switch (state) {
  2705			case PCI_POWER_ERROR:
  2706			case PCI_UNKNOWN:
  2707				return PCI_D3hot;
  2708	
  2709			case PCI_D1:
  2710			case PCI_D2:
  2711				if (pci_no_d1d2(dev))
  2712					return PCI_D3hot;
  2713			}
  2714	
  2715			return state;
  2716		}
  2717	
  2718		/*
  2719		 * If the device is in D3cold even though it's not power-manageable by
  2720		 * the platform, it may have been powered down by non-standard means.
  2721		 * Best to let it slumber.
  2722		 */
  2723		if (dev->current_state == PCI_D3cold)
  2724			return PCI_D3cold;
  2725		else if (!dev->pm_cap)
  2726			return PCI_D0;
  2727	
  2728		/* if platform indicates preferred state device constraint, use it */
> 2729		constraint = platform_get_constraint(dev);
  2730		if (constraint < 0)
  2731			constraint = PCI_D3hot;
  2732	
  2733		if (wakeup && dev->pme_support) {
  2734			pci_power_t pme_state = pci_get_wake_pme_state(dev);
  2735	
  2736			/* pick the lesser of any specified constraints */
  2737			if (pme_state < constraint)
  2738				constraint = pme_state;
  2739		}
  2740	
  2741		return constraint;
  2742	}
  2743	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
  2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
  2023-08-05  5:22   ` kernel test robot
  2023-08-05 10:27   ` kernel test robot
@ 2023-08-05 10:27   ` kernel test robot
  2 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2023-08-05 10:27 UTC (permalink / raw)
  To: Mario Limonciello, Rafael J . Wysocki, Mika Westerberg,
	Bjorn Helgaas
  Cc: llvm, oe-kbuild-all, linux-pci, linux-kernel, Andy Shevchenko,
	linux-acpi, Kuppuswamy Sathyanarayanan, Iain Lane,
	Shyam-sundar S-k, Mario Limonciello

Hi Mario,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on pci/next pci/for-linus westeri-thunderbolt/next linus/master v6.5-rc4 next-20230804]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mario-Limonciello/ACPI-Add-comments-to-clarify-some-ifdef-statements/20230805-050559
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20230804210129.5356-8-mario.limonciello%40amd.com
patch subject: [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy
config: x86_64-randconfig-r013-20230731 (https://download.01.org/0day-ci/archive/20230805/202308051803.x00umDzn-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230805/202308051803.x00umDzn-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202308051803.x00umDzn-lkp@intel.com/

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: acpi_get_lps0_constraint
   >>> referenced by pci.c:1090 (drivers/pci/pci.c:1090)
   >>>               vmlinux.o:(pci_target_state)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-08-05 10:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-04 21:01 [PATCH v10 0/7] Fix wakeup problems on some AMD platforms Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 1/7] ACPI: Add comments to clarify some #ifdef statements Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 2/7] ACPI: Adjust #ifdef for *_lps0_dev use Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 3/7] ACPI: x86: s2idle: Fix a logic error parsing AMD constraints table Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 4/7] ACPI: x86: s2idle: Add more debugging for AMD constraints parsing Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 5/7] ACPI: x86: s2idle: Store if constraint is enabled Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 6/7] ACPI: x86: s2idle: Add a function to get constraints for a device Mario Limonciello
2023-08-04 21:01 ` [PATCH v10 7/7] PCI: Use device constraints to decide PCI target state fallback policy Mario Limonciello
2023-08-05  5:22   ` kernel test robot
2023-08-05 10:27   ` kernel test robot
2023-08-05 10:27   ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).