[PATCH v1 0/3] ACPI: PM: s2idle: Tidy up the code and avoid invoking _DSM unnecessarily

[PATCH v1 0/3] ACPI: PM: s2idle: Tidy up the code and avoid invoking _DSM unnecessarily

[Rafael J. Wysocki]

Hi Everyone,

There is an unused function related to s2idle on x86, so drop it (patch [1/3]).

Also, some callback functions used in struct platform_s2idle_ops on x86 can
be made static, so do that (patch [2/3]).

Finally, the evaluation of LPS0 _DSM Function 1 is useless overhead unless
printing PM debug messages has been enabled, so rearrange the code to only
evaluate it when the data produced by it will be used (patch [3/3]).

Please see changelogs of the individual patches for details.

Thanks!

[PATCH v1 1/3] ACPI: PM: s2idle: Drop acpi_get_lps0_constraint()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Drop unused function acpi_get_lps0_constraint().

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/x86/s2idle.c |   24 ------------------------
 include/linux/acpi.h      |    5 -----
 2 files changed, 29 deletions(-)

--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -299,30 +299,6 @@ free_acpi_buffer:
 	ACPI_FREE(out_obj);
 }
 
-/**
- * acpi_get_lps0_constraint - Get the LPS0 constraint for a device.
- * @adev: Device to get the constraint for.
- *
- * The LPS0 constraint is the shallowest (minimum) power state in which the
- * device can be so as to allow the platform as a whole to achieve additional
- * energy conservation by utilizing a system-wide low-power state.
- *
- * Returns:
- *  - ACPI power state value of the constraint for @adev on success.
- *  - Otherwise, ACPI_STATE_UNKNOWN.
- */
-int acpi_get_lps0_constraint(struct acpi_device *adev)
-{
-	struct lpi_constraints *entry;
-
-	for_each_lpi_constraint(entry) {
-		if (adev->handle == entry->handle)
-			return entry->min_dstate;
-	}
-
-	return ACPI_STATE_UNKNOWN;
-}
-
 static void lpi_check_constraints(void)
 {
 	struct lpi_constraints *entry;
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1157,12 +1157,7 @@ struct acpi_s2idle_dev_ops {
 #if defined(CONFIG_SUSPEND) && defined(CONFIG_X86)
 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 acpi_device *adev);
 #else /* CONFIG_SUSPEND && CONFIG_X86 */
-static inline int acpi_get_lps0_constraint(struct device *dev)
-{
-	return ACPI_STATE_UNKNOWN;
-}
 static inline int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg)
 {
 	return -ENODEV;

[PATCH v1 2/3] ACPI: PM: s2idle: Staticise LPS0 callback functions

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The LPS0 callback functions in x86/s2idle.c can be made static, so do
that and remove their declarations from sleep.h.

While at it, add the _lps0 suffix to their names to indicate that
they are LPS0-specific.

No functional impact.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/sleep.h      |    3 ---
 drivers/acpi/x86/s2idle.c |   12 ++++++------
 2 files changed, 6 insertions(+), 9 deletions(-)

--- a/drivers/acpi/sleep.h
+++ b/drivers/acpi/sleep.h
@@ -17,10 +17,7 @@ static inline acpi_status acpi_set_wakin
 
 extern int acpi_s2idle_begin(void);
 extern int acpi_s2idle_prepare(void);
-extern int acpi_s2idle_prepare_late(void);
-extern void acpi_s2idle_check(void);
 extern bool acpi_s2idle_wake(void);
-extern void acpi_s2idle_restore_early(void);
 extern void acpi_s2idle_restore(void);
 extern void acpi_s2idle_end(void);
 
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -515,7 +515,7 @@ static struct acpi_scan_handler lps0_han
 	.attach = lps0_device_attach,
 };
 
-int acpi_s2idle_prepare_late(void)
+static int acpi_s2idle_prepare_late_lps0(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
 
@@ -561,7 +561,7 @@ int acpi_s2idle_prepare_late(void)
 	return 0;
 }
 
-void acpi_s2idle_check(void)
+static void acpi_s2idle_check_lps0(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
 
@@ -574,7 +574,7 @@ void acpi_s2idle_check(void)
 	}
 }
 
-void acpi_s2idle_restore_early(void)
+static void acpi_s2idle_restore_early_lps0(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
 
@@ -614,10 +614,10 @@ void acpi_s2idle_restore_early(void)
 static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
 	.begin = acpi_s2idle_begin,
 	.prepare = acpi_s2idle_prepare,
-	.prepare_late = acpi_s2idle_prepare_late,
-	.check = acpi_s2idle_check,
+	.prepare_late = acpi_s2idle_prepare_late_lps0,
+	.check = acpi_s2idle_check_lps0,
 	.wake = acpi_s2idle_wake,
-	.restore_early = acpi_s2idle_restore_early,
+	.restore_early = acpi_s2idle_restore_early_lps0,
 	.restore = acpi_s2idle_restore,
 	.end = acpi_s2idle_end,
 };

[PATCH v1 3/3] ACPI: PM: s2idle: Only retrieve constraints when needed

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The evaluation of LPS0 _DSM Function 1 in lps0_device_attach() may be
useless if pm_debug_messages_on is never set.

For this reason, instead of evaluating it in lps0_device_attach(), do
that in a new .begin() callback for s2idle, acpi_s2idle_begin_lps0(),
only when pm_debug_messages_on is set at that point.

However, never attempt to evaluate LPS0 _DSM Function 1 more than once
to avoid recurring failures.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/x86/s2idle.c |   29 +++++++++++++++++++++++------
 1 file changed, 23 insertions(+), 6 deletions(-)

--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -303,6 +303,9 @@ static void lpi_check_constraints(void)
 {
 	struct lpi_constraints *entry;
 
+	if (IS_ERR_OR_NULL(lpi_constraints_table))
+		return;
+
 	for_each_lpi_constraint(entry) {
 		struct acpi_device *adev = acpi_fetch_acpi_dev(entry->handle);
 
@@ -484,11 +487,6 @@ static int lps0_device_attach(struct acp
 
 	lps0_device_handle = adev->handle;
 
-	if (acpi_s2idle_vendor_amd())
-		lpi_device_get_constraints_amd();
-	else
-		lpi_device_get_constraints();
-
 	/*
 	 * Use suspend-to-idle by default if ACPI_FADT_LOW_POWER_S0 is set in
 	 * the FADT and the default suspend mode was not set from the command
@@ -515,6 +513,25 @@ static struct acpi_scan_handler lps0_han
 	.attach = lps0_device_attach,
 };
 
+static int acpi_s2idle_begin_lps0(void)
+{
+	if (pm_debug_messages_on && !lpi_constraints_table) {
+		if (acpi_s2idle_vendor_amd())
+			lpi_device_get_constraints_amd();
+		else
+			lpi_device_get_constraints();
+
+		/*
+		 * Try to retrieve the constraints only once because failures
+		 * to do so usually are sticky.
+		 */
+		if (!lpi_constraints_table)
+			lpi_constraints_table = ERR_PTR(-ENODATA);
+	}
+
+	return acpi_s2idle_begin();
+}
+
 static int acpi_s2idle_prepare_late_lps0(void)
 {
 	struct acpi_s2idle_dev_ops *handler;
@@ -612,7 +629,7 @@ static void acpi_s2idle_restore_early_lp
 }
 
 static const struct platform_s2idle_ops acpi_s2idle_ops_lps0 = {
-	.begin = acpi_s2idle_begin,
+	.begin = acpi_s2idle_begin_lps0,
 	.prepare = acpi_s2idle_prepare,
 	.prepare_late = acpi_s2idle_prepare_late_lps0,
 	.check = acpi_s2idle_check_lps0,

Re: [PATCH v1 0/3] ACPI: PM: s2idle: Tidy up the code and avoid invoking _DSM unnecessarily

[Mario Limonciello]

On 10/9/25 2:25 PM, Rafael J. Wysocki wrote:
> Hi Everyone,
> 
> There is an unused function related to s2idle on x86, so drop it (patch [1/3]).
> 
> Also, some callback functions used in struct platform_s2idle_ops on x86 can
> be made static, so do that (patch [2/3]).
> 
> Finally, the evaluation of LPS0 _DSM Function 1 is useless overhead unless
> printing PM debug messages has been enabled, so rearrange the code to only
> evaluate it when the data produced by it will be used (patch [3/3]).
> 
> Please see changelogs of the individual patches for details.
> 
> Thanks!
> 
> 
> 

Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>