From: Mario Limonciello <mario.limonciello@amd.com>
To: "Rafael J . Wysocki" <rjw@rjwysocki.net>, <linux-acpi@vger.kernel.org>
Cc: Mario Limonciello <mario.limonciello@amd.com>
Subject: [RFC] acpi/x86: s2idle: Use constraints to enforce behavior
Date: Fri, 1 Oct 2021 11:18:54 -0500 [thread overview]
Message-ID: <20211001161854.19802-1-mario.limonciello@amd.com> (raw)
Currently s2idle constraints are only verified when pm_debug_messages
is set. Although useful for debugging, it does have a tendency to paper
over some underlying issues.
Of particular note, I found a case that a system that has two physical
SATA controllers that share the same ACPI Power Resource. When a drive is
connected to one of the controllers then it will bind with PCI devices with
the ahci driver and form a relationship with the firmware node and physical
node. During s2idle I see that the constraints are met for this device as
it's transitioned into the appropriate state. However the second ACPI node
doesn't have any relationship with a physical node and so it stays in "D0":
```
ACPI: \_SB_.PCI0.GP18.SATA: ACPI: PM: Power state change: D0 -> D3cold
ACPI: PM: Power resource [P0SA] still in use
acpi device:2a: Power state changed to D3cold
```
Due to the refcounting used on the shared power resource putting the
device with a physical node into D3 doesn't result in the _OFF method
being called.
To help this problem, make the following changes to the constraint
checking:
1) Make constraint checking compulsory but gate the output on
pm_debug_messages
2) As part of constraint checking verify if the ACPI device has a physical
node or not.
3) If a device doesn't have a physical node, but does have a requirement
set the power state for the device to allow shared resources to transition.
After making this change, here is what the flow looks like:
```
ACPI: \_SB_.PCI0.GP18.SATA: ACPI: PM: Power state change: D0 -> D3cold
ACPI: PM: Power resource [P0SA] still in use
acpi device:2a: Power state changed to Dcold
<snip>
ACPI: \_SB_.PCI0.GP18.SAT1: LPI: Device is not physically present - forcing transition from D0 to D3cold
ACPI: \_SB_.PCI0.GP18.SAT1: ACPI: PM: Power state change: D0 -> D3cold
ACPI: PM: Power resource [P0SA] turned off
acpi device:2c: Power state changed to D3cold
```
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214091
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
drivers/acpi/x86/s2idle.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index bd92b549fd5a..fbfac40733eb 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -304,14 +304,25 @@ static void lpi_check_constraints(void)
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",
+ if (adev->power.state >= lpi_constraints_table[i].min_dstate)
+ continue;
+ /* make sure that anything with shared resources isn't blocked */
+ if (!acpi_get_first_physical_node(adev)) {
+ if (pm_debug_messages_on)
+ acpi_handle_info(handle, "LPI: Device is not physically present - forcing transition from %s to %s\n",
+ acpi_power_state_string(adev->power.state),
+ acpi_power_state_string(ACPI_STATE_D3_COLD));
+ acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
+ continue;
+ }
+ 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));
}
@@ -446,8 +457,7 @@ int acpi_s2idle_prepare_late(void)
if (!lps0_device_handle || sleep_no_lps0)
return 0;
- if (pm_debug_messages_on)
- lpi_check_constraints();
+ lpi_check_constraints();
/* Screen off */
if (lps0_dsm_func_mask > 0)
--
2.25.1
next reply other threads:[~2021-10-01 16:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-01 16:18 Mario Limonciello [this message]
2021-10-01 16:41 ` [RFC] acpi/x86: s2idle: Use constraints to enforce behavior Rafael J. Wysocki
2021-10-01 17:27 ` Limonciello, Mario
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211001161854.19802-1-mario.limonciello@amd.com \
--to=mario.limonciello@amd.com \
--cc=linux-acpi@vger.kernel.org \
--cc=rjw@rjwysocki.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).