linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler
@ 2022-03-10 19:17 Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback Mario Limonciello
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-03-10 19:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket, Mario Limonciello

Currenty the latest thing run during a suspend to idle attempt is
the LPS0 `prepare_late` callback and the earliest thing is the
`resume_early` callback.

There is a desire for the `amd-pmc` driver to suspend later in the
suspend process (ideally the very last thing), so create a callback
that it or any other driver can hook into to do this.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v1->v2:
 * Change register/unregister arguments to be struct

 drivers/acpi/x86/s2idle.c | 65 ++++++++++++++++++++++++++++++++++++++-
 include/linux/acpi.h      | 11 ++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index abc06e7f89d8..7418443580d4 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -86,6 +86,9 @@ struct lpi_device_constraint_amd {
 	int min_dstate;
 };
 
+static LIST_HEAD(lps0_callback_handler_head);
+static DEFINE_MUTEX(lps0_callback_handler_mutex);
+
 static struct lpi_constraints *lpi_constraints_table;
 static int lpi_constraints_table_size;
 static int rev_id;
@@ -444,6 +447,9 @@ static struct acpi_scan_handler lps0_handler = {
 
 int acpi_s2idle_prepare_late(void)
 {
+	struct lps0_callback_handler *handler;
+	int rc = 0;
+
 	if (!lps0_device_handle || sleep_no_lps0)
 		return 0;
 
@@ -474,14 +480,31 @@ int acpi_s2idle_prepare_late(void)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
 				lps0_dsm_func_mask_microsoft, lps0_dsm_guid_microsoft);
 	}
-	return 0;
+
+	mutex_lock(&lps0_callback_handler_mutex);
+	list_for_each_entry(handler, &lps0_callback_handler_head, list_node) {
+		rc = handler->prepare_late_callback(handler->context);
+		if (rc)
+			goto out;
+	}
+out:
+	mutex_unlock(&lps0_callback_handler_mutex);
+
+	return rc;
 }
 
 void acpi_s2idle_restore_early(void)
 {
+	struct lps0_callback_handler *handler;
+
 	if (!lps0_device_handle || sleep_no_lps0)
 		return;
 
+	mutex_lock(&lps0_callback_handler_mutex);
+	list_for_each_entry(handler, &lps0_callback_handler_head, list_node)
+		handler->restore_early_callback(handler->context);
+	mutex_unlock(&lps0_callback_handler_mutex);
+
 	/* Modern standby exit */
 	if (lps0_dsm_func_mask_microsoft > 0)
 		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
@@ -524,4 +547,44 @@ void acpi_s2idle_setup(void)
 	s2idle_set_ops(&acpi_s2idle_ops_lps0);
 }
 
+int acpi_register_lps0_callbacks(struct lps0_callback_handler *arg)
+{
+	struct lps0_callback_handler *handler;
+
+	if (!lps0_device_handle || sleep_no_lps0)
+		return -ENODEV;
+
+	handler = kmalloc(sizeof(*handler), GFP_KERNEL);
+	if (!handler)
+		return -ENOMEM;
+	handler->prepare_late_callback = arg->prepare_late_callback;
+	handler->restore_early_callback = arg->restore_early_callback;
+	handler->context = arg->context;
+
+	mutex_lock(&lps0_callback_handler_mutex);
+	list_add(&handler->list_node, &lps0_callback_handler_head);
+	mutex_unlock(&lps0_callback_handler_mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(acpi_register_lps0_callbacks);
+
+void acpi_unregister_lps0_callbacks(struct lps0_callback_handler *arg)
+{
+	struct lps0_callback_handler *handler;
+
+	mutex_lock(&lps0_callback_handler_mutex);
+	list_for_each_entry(handler, &lps0_callback_handler_head, list_node) {
+		if (handler->prepare_late_callback == arg->prepare_late_callback &&
+		    handler->restore_early_callback == arg->restore_early_callback &&
+		    handler->context == arg->context) {
+			list_del(&handler->list_node);
+			kfree(handler);
+			break;
+		}
+	}
+	mutex_unlock(&lps0_callback_handler_mutex);
+}
+EXPORT_SYMBOL_GPL(acpi_unregister_lps0_callbacks);
+
 #endif /* CONFIG_SUSPEND */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 6274758648e3..df105f5e03e5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1023,7 +1023,16 @@ 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
+struct lps0_callback_handler {
+	struct list_head list_node;
+	int (*prepare_late_callback)(void *context);
+	void (*restore_early_callback)(void *context);
+	void *context;
+};
+int acpi_register_lps0_callbacks(struct lps0_callback_handler *arg);
+void acpi_unregister_lps0_callbacks(struct lps0_callback_handler *arg);
+#endif /* 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] 6+ messages in thread

* [PATCH v2 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback
  2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
@ 2022-03-10 19:17 ` Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 3/5] ACPI / x86: Check LPI constraints by default Mario Limonciello
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-03-10 19:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket, Mario Limonciello

If constraints checking has been enabled by the LPS0 code, it may
also be useful for drivers using the callback to make a decision what
to do.

For example this may in the future allow a failing constraints check
preventing another driver from notifying firmware that all required
devices have entered the deepest state.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
Changes from v1->v2:
 * rebase on top of changes in first patch

 drivers/acpi/x86/s2idle.c | 11 +++++++----
 include/linux/acpi.h      |  2 +-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 7418443580d4..808018bbab90 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -290,7 +290,7 @@ static void lpi_device_get_constraints(void)
 	ACPI_FREE(out_obj);
 }
 
-static void lpi_check_constraints(void)
+static void lpi_check_constraints(bool *met)
 {
 	int i;
 
@@ -312,11 +312,13 @@ static void lpi_check_constraints(void)
 			continue;
 		}
 
-		if (adev->power.state < lpi_constraints_table[i].min_dstate)
+		if (adev->power.state < lpi_constraints_table[i].min_dstate) {
 			acpi_handle_info(handle,
 				"LPI: Constraint not met; min power state:%s current power state:%s\n",
 				acpi_power_state_string(lpi_constraints_table[i].min_dstate),
 				acpi_power_state_string(adev->power.state));
+			*met = false;
+		}
 	}
 }
 
@@ -448,13 +450,14 @@ static struct acpi_scan_handler lps0_handler = {
 int acpi_s2idle_prepare_late(void)
 {
 	struct lps0_callback_handler *handler;
+	bool constraints_met = true;
 	int rc = 0;
 
 	if (!lps0_device_handle || sleep_no_lps0)
 		return 0;
 
 	if (pm_debug_messages_on)
-		lpi_check_constraints();
+		lpi_check_constraints(&constraints_met);
 
 	/* Screen off */
 	if (lps0_dsm_func_mask > 0)
@@ -483,7 +486,7 @@ int acpi_s2idle_prepare_late(void)
 
 	mutex_lock(&lps0_callback_handler_mutex);
 	list_for_each_entry(handler, &lps0_callback_handler_head, list_node) {
-		rc = handler->prepare_late_callback(handler->context);
+		rc = handler->prepare_late_callback(handler->context, constraints_met);
 		if (rc)
 			goto out;
 	}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index df105f5e03e5..4906db854554 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1026,7 +1026,7 @@ acpi_status acpi_os_prepare_extended_sleep(u8 sleep_state,
 #ifdef CONFIG_X86
 struct lps0_callback_handler {
 	struct list_head list_node;
-	int (*prepare_late_callback)(void *context);
+	int (*prepare_late_callback)(void *context, bool constraints);
 	void (*restore_early_callback)(void *context);
 	void *context;
 };
-- 
2.34.1


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

* [PATCH v2 3/5] ACPI / x86: Check LPI constraints by default
  2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback Mario Limonciello
@ 2022-03-10 19:17 ` Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 4/5] platform/x86: amd-pmc: Move to later in the suspend process Mario Limonciello
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-03-10 19:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket, Mario Limonciello, David E . Box

Currently LPI constraints are only checked when `pm_debug_messages` has
been set. They are mostly used as a debugging tactic.

As the results of constraint checking will be passed to consumers of
the LPS0 callback, ensure that constraints are checked by default so this
value can be trusted.

Suggested-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v1->v2:
 * New patch

 drivers/acpi/x86/s2idle.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index 808018bbab90..3236bd66f499 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -307,16 +307,19 @@ static void lpi_check_constraints(bool *met)
 			acpi_power_state_string(adev->power.state));
 
 		if (!adev->flags.power_manageable) {
-			acpi_handle_info(handle, "LPI: Device not power manageable\n");
+			if (pm_debug_messages_on)
+				acpi_handle_info(handle, "LPI: Device not power manageable\n");
 			lpi_constraints_table[i].handle = NULL;
 			continue;
 		}
 
 		if (adev->power.state < lpi_constraints_table[i].min_dstate) {
-			acpi_handle_info(handle,
-				"LPI: Constraint not met; min power state:%s current power state:%s\n",
-				acpi_power_state_string(lpi_constraints_table[i].min_dstate),
-				acpi_power_state_string(adev->power.state));
+			if (pm_debug_messages_on) {
+				acpi_handle_info(handle,
+					"LPI: Constraint not met; min power state:%s current power state:%s\n",
+					acpi_power_state_string(lpi_constraints_table[i].min_dstate),
+					acpi_power_state_string(adev->power.state));
+			}
 			*met = false;
 		}
 	}
@@ -456,8 +459,7 @@ int acpi_s2idle_prepare_late(void)
 	if (!lps0_device_handle || sleep_no_lps0)
 		return 0;
 
-	if (pm_debug_messages_on)
-		lpi_check_constraints(&constraints_met);
+	lpi_check_constraints(&constraints_met);
 
 	/* Screen off */
 	if (lps0_dsm_func_mask > 0)
-- 
2.34.1


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

* [PATCH v2 4/5] platform/x86: amd-pmc: Move to later in the suspend process
  2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 3/5] ACPI / x86: Check LPI constraints by default Mario Limonciello
@ 2022-03-10 19:17 ` Mario Limonciello
  2022-03-10 19:17 ` [PATCH v2 5/5] platform/x86: amd-pmc: Drop CPU QoS workaround Mario Limonciello
  2022-03-10 21:43 ` [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler David E. Box
  4 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-03-10 19:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket, Mario Limonciello

The `OS_HINT` message is supposed to indicate that everything else
that is supposed to go into the deepest state has done so.

This assumption is invalid as:
1) The CPUs will still go in and out of the deepest state
2) Other devices may still run their `noirq` suspend routines
3) The LPS0 ACPI device will still run

To more closely mirror how this works on other operating systems,
move the `amd-pmc` suspend to the very last thing before the s2idle
loop via an lps0 callback.

Fixes: 8d89835b0467 ("PM: suspend: Do not pause cpuidle in the suspend-to-idle path")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v1->v2:
 * adjust for changes in previous patches
 * display a debugging message for constraints

 drivers/platform/x86/amd-pmc.c | 42 +++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index 971aaabaa9c8..752a99d759d2 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -639,13 +639,16 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
 	return rc;
 }
 
-static int __maybe_unused amd_pmc_suspend(struct device *dev)
+static int amd_pmc_suspend(void *context, bool constraints)
 {
-	struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
+	struct amd_pmc_dev *pdev = dev_get_drvdata((struct device *)context);
 	int rc;
 	u8 msg;
 	u32 arg = 1;
 
+	/* for enabling constraints checking in the future */
+	dev_dbg(pdev->dev, "LPI constraints were%smet.\n", constraints ? " " : " not ");
+
 	/* Reset and Start SMU logging - to monitor the s0i3 stats */
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_RESET, 0);
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_START, 0);
@@ -658,7 +661,7 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
 	}
 
 	/* Dump the IdleMask before we send hint to SMU */
-	amd_pmc_idlemask_read(pdev, dev, NULL);
+	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
 	msg = amd_pmc_get_os_hint(pdev);
 	rc = amd_pmc_send_cmd(pdev, arg, NULL, msg, 0);
 	if (rc) {
@@ -681,28 +684,28 @@ static int __maybe_unused amd_pmc_suspend(struct device *dev)
 	return rc;
 }
 
-static int __maybe_unused amd_pmc_resume(struct device *dev)
+static void amd_pmc_resume(void *context)
 {
-	struct amd_pmc_dev *pdev = dev_get_drvdata(dev);
+	struct amd_pmc_dev *pdev = dev_get_drvdata((struct device *)context);
 	int rc;
 	u8 msg;
 
 	msg = amd_pmc_get_os_hint(pdev);
 	rc = amd_pmc_send_cmd(pdev, 0, NULL, msg, 0);
 	if (rc)
-		dev_err(pdev->dev, "resume failed\n");
+		dev_err(pdev->dev, "resume failed: %d\n", rc);
 
 	/* Let SMU know that we are looking for stats */
 	amd_pmc_send_cmd(pdev, 0, NULL, SMU_MSG_LOG_DUMP_DATA, 0);
 
 	/* Dump the IdleMask to see the blockers */
-	amd_pmc_idlemask_read(pdev, dev, NULL);
+	amd_pmc_idlemask_read(pdev, pdev->dev, NULL);
 
 	/* Write data incremented by 1 to distinguish in stb_read */
 	if (enable_stb)
 		rc = amd_pmc_write_stb(pdev, AMD_PMC_STB_PREDEF + 1);
 	if (rc)
-		dev_err(pdev->dev, "error writing to STB\n");
+		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
 
 	/* Restore the QoS request back to defaults if it was set */
 	if (pdev->cpu_id == AMD_CPU_ID_CZN)
@@ -711,15 +714,8 @@ static int __maybe_unused amd_pmc_resume(struct device *dev)
 
 	/* Notify on failed entry */
 	amd_pmc_validate_deepest(pdev);
-
-	return rc;
 }
 
-static const struct dev_pm_ops amd_pmc_pm_ops = {
-	.suspend_noirq = amd_pmc_suspend,
-	.resume_noirq = amd_pmc_resume,
-};
-
 static const struct pci_device_id pmc_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_YC) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_AMD, AMD_CPU_ID_CZN) },
@@ -805,6 +801,11 @@ static int amd_pmc_read_stb(struct amd_pmc_dev *dev, u32 *buf)
 static int amd_pmc_probe(struct platform_device *pdev)
 {
 	struct amd_pmc_dev *dev = &pmc;
+	struct lps0_callback_handler lps0_handler = {
+		.prepare_late_callback = amd_pmc_suspend,
+		.restore_early_callback = amd_pmc_resume,
+		.context = &pdev->dev,
+	};
 	struct pci_dev *rdev;
 	u32 base_addr_lo, base_addr_hi;
 	u64 base_addr, fch_phys_addr;
@@ -884,6 +885,10 @@ static int amd_pmc_probe(struct platform_device *pdev)
 
 	amd_pmc_get_smu_version(dev);
 	platform_set_drvdata(pdev, dev);
+	err = acpi_register_lps0_callbacks(&lps0_handler);
+	if (err)
+		goto err_pci_dev_put;
+
 	amd_pmc_dbgfs_register(dev);
 	cpu_latency_qos_add_request(&dev->amd_pmc_pm_qos_req, PM_QOS_DEFAULT_VALUE);
 	return 0;
@@ -896,7 +901,13 @@ static int amd_pmc_probe(struct platform_device *pdev)
 static int amd_pmc_remove(struct platform_device *pdev)
 {
 	struct amd_pmc_dev *dev = platform_get_drvdata(pdev);
+	struct lps0_callback_handler lps0_handler = {
+		.prepare_late_callback = amd_pmc_suspend,
+		.restore_early_callback = amd_pmc_resume,
+		.context = &pdev->dev,
+	};
 
+	acpi_unregister_lps0_callbacks(&lps0_handler);
 	amd_pmc_dbgfs_unregister(dev);
 	pci_dev_put(dev->rdev);
 	mutex_destroy(&dev->lock);
@@ -917,7 +928,6 @@ static struct platform_driver amd_pmc_driver = {
 	.driver = {
 		.name = "amd_pmc",
 		.acpi_match_table = amd_pmc_acpi_ids,
-		.pm = &amd_pmc_pm_ops,
 	},
 	.probe = amd_pmc_probe,
 	.remove = amd_pmc_remove,
-- 
2.34.1


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

* [PATCH v2 5/5] platform/x86: amd-pmc: Drop CPU QoS workaround
  2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
                   ` (2 preceding siblings ...)
  2022-03-10 19:17 ` [PATCH v2 4/5] platform/x86: amd-pmc: Move to later in the suspend process Mario Limonciello
@ 2022-03-10 19:17 ` Mario Limonciello
  2022-03-10 21:43 ` [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler David E. Box
  4 siblings, 0 replies; 6+ messages in thread
From: Mario Limonciello @ 2022-03-10 19:17 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket, Mario Limonciello

A workaround was previously introduced as part of commit 646f429ec2de
("platform/x86: amd-pmc: Set QOS during suspend on CZN w/ timer wakeup")
to prevent CPUs from going into the deepest state during the execution
of the `noirq` stage of `amd_pmc`.  As `amd_pmc` has been pushed to the
last step for suspend on AMD platforms, this workaround is no longer
necessary.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
changes from v1->v2:
 * No changes

 drivers/platform/x86/amd-pmc.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/platform/x86/amd-pmc.c b/drivers/platform/x86/amd-pmc.c
index 752a99d759d2..b3c6b5cc83c1 100644
--- a/drivers/platform/x86/amd-pmc.c
+++ b/drivers/platform/x86/amd-pmc.c
@@ -21,7 +21,6 @@
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/platform_device.h>
-#include <linux/pm_qos.h>
 #include <linux/rtc.h>
 #include <linux/suspend.h>
 #include <linux/seq_file.h>
@@ -96,9 +95,6 @@
 #define PMC_MSG_DELAY_MIN_US		50
 #define RESPONSE_REGISTER_LOOP_MAX	20000
 
-/* QoS request for letting CPUs in idle states, but not the deepest */
-#define AMD_PMC_MAX_IDLE_STATE_LATENCY	3
-
 #define SOC_SUBSYSTEM_IP_MAX	12
 #define DELAY_MIN_US		2000
 #define DELAY_MAX_US		3000
@@ -153,7 +149,6 @@ struct amd_pmc_dev {
 	struct device *dev;
 	struct pci_dev *rdev;
 	struct mutex lock; /* generic mutex lock */
-	struct pm_qos_request amd_pmc_pm_qos_req;
 #if IS_ENABLED(CONFIG_DEBUG_FS)
 	struct dentry *dbgfs_dir;
 #endif /* CONFIG_DEBUG_FS */
@@ -628,14 +623,6 @@ static int amd_pmc_verify_czn_rtc(struct amd_pmc_dev *pdev, u32 *arg)
 	rc = rtc_alarm_irq_enable(rtc_device, 0);
 	dev_dbg(pdev->dev, "wakeup timer programmed for %lld seconds\n", duration);
 
-	/*
-	 * Prevent CPUs from getting into deep idle states while sending OS_HINT
-	 * which is otherwise generally safe to send when at least one of the CPUs
-	 * is not in deep idle states.
-	 */
-	cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req, AMD_PMC_MAX_IDLE_STATE_LATENCY);
-	wake_up_all_idle_cpus();
-
 	return rc;
 }
 
@@ -678,9 +665,6 @@ static int amd_pmc_suspend(void *context, bool constraints)
 
 	return 0;
 fail:
-	if (pdev->cpu_id == AMD_CPU_ID_CZN)
-		cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req,
-						PM_QOS_DEFAULT_VALUE);
 	return rc;
 }
 
@@ -707,11 +691,6 @@ static void amd_pmc_resume(void *context)
 	if (rc)
 		dev_err(pdev->dev, "error writing to STB: %d\n", rc);
 
-	/* Restore the QoS request back to defaults if it was set */
-	if (pdev->cpu_id == AMD_CPU_ID_CZN)
-		cpu_latency_qos_update_request(&pdev->amd_pmc_pm_qos_req,
-						PM_QOS_DEFAULT_VALUE);
-
 	/* Notify on failed entry */
 	amd_pmc_validate_deepest(pdev);
 }
@@ -890,7 +869,6 @@ static int amd_pmc_probe(struct platform_device *pdev)
 		goto err_pci_dev_put;
 
 	amd_pmc_dbgfs_register(dev);
-	cpu_latency_qos_add_request(&dev->amd_pmc_pm_qos_req, PM_QOS_DEFAULT_VALUE);
 	return 0;
 
 err_pci_dev_put:
-- 
2.34.1


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

* Re: [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler
  2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
                   ` (3 preceding siblings ...)
  2022-03-10 19:17 ` [PATCH v2 5/5] platform/x86: amd-pmc: Drop CPU QoS workaround Mario Limonciello
@ 2022-03-10 21:43 ` David E. Box
  4 siblings, 0 replies; 6+ messages in thread
From: David E. Box @ 2022-03-10 21:43 UTC (permalink / raw)
  To: Mario Limonciello, Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi, Shyam Sundar S K,
	Goswami Sanket

On Thu, 2022-03-10 at 13:17 -0600, Mario Limonciello wrote:
> Currenty the latest thing run during a suspend to idle attempt is
> the LPS0 `prepare_late` callback and the earliest thing is the
> `resume_early` callback.
> 
> There is a desire for the `amd-pmc` driver to suspend later in the
> suspend process (ideally the very last thing), so create a callback
> that it or any other driver can hook into to do this.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> changes from v1->v2:
>  * Change register/unregister arguments to be struct
> 
>  drivers/acpi/x86/s2idle.c | 65 ++++++++++++++++++++++++++++++++++++++-
>  include/linux/acpi.h      | 11 ++++++-
>  2 files changed, 74 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index abc06e7f89d8..7418443580d4 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -86,6 +86,9 @@ struct lpi_device_constraint_amd {
>  	int min_dstate;
>  };
>  
> +static LIST_HEAD(lps0_callback_handler_head);
> +static DEFINE_MUTEX(lps0_callback_handler_mutex);
> +
>  static struct lpi_constraints *lpi_constraints_table;
>  static int lpi_constraints_table_size;
>  static int rev_id;
> @@ -444,6 +447,9 @@ static struct acpi_scan_handler lps0_handler = {
>  
>  int acpi_s2idle_prepare_late(void)
>  {
> +	struct lps0_callback_handler *handler;
> +	int rc = 0;
> +
>  	if (!lps0_device_handle || sleep_no_lps0)
>  		return 0;
>  
> @@ -474,14 +480,31 @@ int acpi_s2idle_prepare_late(void)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_ENTRY,
>  				lps0_dsm_func_mask_microsoft,
> lps0_dsm_guid_microsoft);
>  	}
> -	return 0;
> +
> +	mutex_lock(&lps0_callback_handler_mutex);
> +	list_for_each_entry(handler, &lps0_callback_handler_head, list_node) {
> +		rc = handler->prepare_late_callback(handler->context);

Check callback exists first. Sorry I missed earlier.

> +		if (rc)
> +			goto out;
> +	}
> +out:
> +	mutex_unlock(&lps0_callback_handler_mutex);
> +
> +	return rc;
>  }
>  
>  void acpi_s2idle_restore_early(void)
>  {
> +	struct lps0_callback_handler *handler;
> +
>  	if (!lps0_device_handle || sleep_no_lps0)
>  		return;
>  
> +	mutex_lock(&lps0_callback_handler_mutex);
> +	list_for_each_entry(handler, &lps0_callback_handler_head, list_node)
> +		handler->restore_early_callback(handler->context);

Here too.

David

> +	mutex_unlock(&lps0_callback_handler_mutex);
> +
>  	/* Modern standby exit */
>  	if (lps0_dsm_func_mask_microsoft > 0)
>  		acpi_sleep_run_lps0_dsm(ACPI_LPS0_MS_EXIT,
> @@ -524,4 +547,44 @@ void acpi_s2idle_setup(void)
>  	s2idle_set_ops(&acpi_s2idle_ops_lps0);
>  }
>  
> +int acpi_register_lps0_callbacks(struct lps0_callback_handler *arg)
> +{
> +	struct lps0_callback_handler *handler;
> +
> +	if (!lps0_device_handle || sleep_no_lps0)
> +		return -ENODEV;
> +
> +	handler = kmalloc(sizeof(*handler), GFP_KERNEL);
> +	if (!handler)
> +		return -ENOMEM;
> +	handler->prepare_late_callback = arg->prepare_late_callback;
> +	handler->restore_early_callback = arg->restore_early_callback;
> +	handler->context = arg->context;
> +
> +	mutex_lock(&lps0_callback_handler_mutex);
> +	list_add(&handler->list_node, &lps0_callback_handler_head);
> +	mutex_unlock(&lps0_callback_handler_mutex);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(acpi_register_lps0_callbacks);
> +
> +void acpi_unregister_lps0_callbacks(struct lps0_callback_handler *arg)
> +{
> +	struct lps0_callback_handler *handler;
> +
> +	mutex_lock(&lps0_callback_handler_mutex);
> +	list_for_each_entry(handler, &lps0_callback_handler_head, list_node) {
> +		if (handler->prepare_late_callback == arg->prepare_late_callback 
> &&
> +		    handler->restore_early_callback == arg-
> >restore_early_callback &&
> +		    handler->context == arg->context) {
> +			list_del(&handler->list_node);
> +			kfree(handler);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&lps0_callback_handler_mutex);
> +}
> +EXPORT_SYMBOL_GPL(acpi_unregister_lps0_callbacks);
> +
>  #endif /* CONFIG_SUSPEND */
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 6274758648e3..df105f5e03e5 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1023,7 +1023,16 @@ 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
> +struct lps0_callback_handler {
> +	struct list_head list_node;
> +	int (*prepare_late_callback)(void *context);
> +	void (*restore_early_callback)(void *context);
> +	void *context;
> +};
> +int acpi_register_lps0_callbacks(struct lps0_callback_handler *arg);
> +void acpi_unregister_lps0_callbacks(struct lps0_callback_handler *arg);
> +#endif /* CONFIG_X86 */
>  #ifndef CONFIG_IA64
>  void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
>  #else


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

end of thread, other threads:[~2022-03-10 21:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10 19:17 [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler Mario Limonciello
2022-03-10 19:17 ` [PATCH v2 2/5] ACPI / x86: Pass the constraints checking result to LPS0 callback Mario Limonciello
2022-03-10 19:17 ` [PATCH v2 3/5] ACPI / x86: Check LPI constraints by default Mario Limonciello
2022-03-10 19:17 ` [PATCH v2 4/5] platform/x86: amd-pmc: Move to later in the suspend process Mario Limonciello
2022-03-10 19:17 ` [PATCH v2 5/5] platform/x86: amd-pmc: Drop CPU QoS workaround Mario Limonciello
2022-03-10 21:43 ` [PATCH v2 1/5] ACPI / x86: Add support for LPS0 callback handler David E. Box

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).