Page MenuHomeFreeBSD
Differential D55118

acpi: Parse _S3D s2idle instead of _S255D
ClosedPublic
Actions

Authored by obiwac on Feb 5 2026, 12:13 AM.
Tags
None
Referenced Files
F151999599: D55118.id173102.diff
Sun, Apr 12, 12:39 AM
F151997808: D55118.id172077.diff
Sun, Apr 12, 12:24 AM
Unknown Object (File)
Wed, Apr 8, 7:01 AM
Unknown Object (File)
Tue, Apr 7, 11:29 PM
Unknown Object (File)
Sun, Apr 5, 1:06 PM
Unknown Object (File)
Fri, Apr 3, 9:18 PM
Unknown Object (File)
Thu, Apr 2, 7:46 PM
Unknown Object (File)
Thu, Apr 2, 3:10 AM
View All 43 Files
Subscribers
emaste
imp

Details

Reviewers
olce
jkim
Commits
rG19ab46cc6604: acpi: Parse _S3D in s2idle instead of _S255D
Summary

Currently, when entering s2idle, we are parsing the _S255D object to get the shallowest D-state supported by device, as acpi_stype_to_sstate() returns -1 for s2idle.

Instead, we should be parsing _S3D (not sure if this is the ideal solution, but it seems reasonable). Asking the SPMC for the minimum D-state constraints and honouring that does not work at least on my machine, and I'm not sure either way if "minimum" means "proper". So I think parsing _S3D is the right approach here.

Relevant document: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/07_Power_and_Performance_Mgmt/device-power-management-objects.html#s3d-s3-device-state

Sponsored by: The FreeBSD Foundation

Test Plan

I need someone with working S3 on their laptop to make sure I didn't break this (again)!

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 70732
Build 67615: arc lint + arc unit

Event Timeline

obiwac created this revision.Feb 5 2026, 12:13 AM
Herald added a subscriber: imp. · View Herald TranscriptFeb 5 2026, 12:13 AM
obiwac requested review of this revision.Feb 5 2026, 12:13 AM
Harbormaster completed remote builds in B70422: Diff 171184.Feb 5 2026, 12:13 AM
obiwac edited the summary of this revision. (Show Details)Feb 5 2026, 12:15 AM
obiwac added reviewers: olce, jkim.
obiwac retitled this revision from acpi: _S225D fix to acpi: Evaluate _S3D instead of _S225D when entering s2idle.
obiwac edited the summary of this revision. (Show Details)
obiwac updated this revision to Diff 171353.Feb 6 2026, 5:28 PM

Ask SPMC for D-state in s2idle instead of _S255D

Harbormaster completed remote builds in B70473: Diff 171353.Feb 6 2026, 5:28 PM
obiwac retitled this revision from acpi: Evaluate _S3D instead of _S225D when entering s2idle to acpi: Ask SPMC for D-state in s2idle instead of _S255D.Feb 6 2026, 5:32 PM
obiwac edited the summary of this revision. (Show Details)
obiwac edited the test plan for this revision. (Show Details)
emaste added a subscriber: emaste.Feb 10 2026, 3:04 PM

I need someone with working S3 on their laptop to make sure I didn't break this (again)!

I can give this a try on my WIP branch in a little while, on my Framework 13" 11th gen Intel. Currently running wipbsd-n283918-87f5b3153f15 which is a bunch of WIP from main at c59a47dc6c016dff74466cecb160459980a5d782, and S3 is working.

emaste added a comment.Feb 11 2026, 1:55 PM

Now running wipbsd-n284325-ae4a668b7590 which has been rebased on main as of 89aa8a94053fdd22ed716fdf424a2d10e70b3188 and includes this patch. CPU is 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz (2803.20-MHz K8-class CPU. S3 works as before.

obiwac updated this revision to Diff 172077.Feb 17 2026, 4:56 PM

Fix passing dev instead of handle to acpi_device_pwr_for_sleep_sxd

Harbormaster completed remote builds in B70732: Diff 172077.Feb 17 2026, 4:56 PM
obiwac updated this revision to Diff 173102.Mar 4 2026, 1:10 PM

rebase

Harbormaster completed remote builds in B71174: Diff 173102.Mar 4 2026, 1:10 PM
obiwac updated this revision to Diff 173103.Mar 4 2026, 1:13 PM

Remove comment about overriding next D-state, as it was confusing and was anyway explained elsewhere.

Harbormaster completed remote builds in B71175: Diff 173103.Mar 4 2026, 1:13 PM
obiwac planned changes to this revision.Sat, Mar 21, 7:23 AM

I have not looked into this yet, but I think this revision causes S0i3 entry to fail for some reason (even though power consumers should still be switched to D3). So changing status to planned changes.

obiwac added a comment.Tue, Mar 24, 5:18 PM

even though power consumers should still be switched to D3

erm no, I totally forgot what this patch did. It is switching to whatever the minimum D-state the SPMC is asking for is. For some reason, even though we honour this, the platform doesn't suspend (maybe platform bug), but either way we should aim to enter D3. So when doing s2idle, let's just evaluate _S3D.

obiwac updated this revision to Diff 174241.Tue, Mar 24, 5:19 PM

Don't honour SPMC min D-state. Just read _S3D as if we were entering S3.

Previous testing of the ACPI S3 path should still be valid. Fixes entering S0i3 on some platforms.

Harbormaster completed remote builds in B71693: Diff 174241.Tue, Mar 24, 5:19 PM
obiwac edited the summary of this revision. (Show Details)Tue, Mar 24, 5:24 PM
obiwac retitled this revision from acpi: Ask SPMC for D-state in s2idle instead of _S255D to acpi: Parse _S3D s2idle instead of _S255D.
emaste added a comment.Wed, Apr 1, 12:51 PM

I've applied the recent update to my testing tree

olce added a comment.Fri, Apr 3, 9:27 PM
In D55118#1285935, @emaste wrote:

I've applied the recent update to my testing tree

Same here.

olce accepted this revision.Tue, Apr 7, 10:21 AM
olce added inline comments.
sys/dev/acpica/acpi.c
2214

Now we know that's fine, but just for the sake of completeness I'd just check that acpi_stype_to_sstate() returns ACPI_STATE_UNKNOWN and if yes forcibly set state to ACPI_STATE_S3, so we don't try to invoke a non-existent method if something at higher levels is wrong (and you could as well add a KASSERT()/MPASS()).

This revision is now accepted and ready to land.Tue, Apr 7, 10:21 AM
obiwac marked an inline comment as done.Wed, Apr 8, 10:20 AM
obiwac added inline comments.
sys/dev/acpica/acpi.c
2214

maybe we should rather just not do anything. I'm gonna make it return ENOENT or something.

obiwac updated this revision to Diff 175083.Wed, Apr 8, 10:21 AM
obiwac marked an inline comment as done.

Return error if we get unknown state from acpi_stype_to_sstate

This revision now requires review to proceed.Wed, Apr 8, 10:21 AM
Harbormaster completed remote builds in B72038: Diff 175083.Wed, Apr 8, 10:21 AM
olce accepted this revision.Wed, Apr 8, 12:00 PM
This revision is now accepted and ready to land.Wed, Apr 8, 12:00 PM
Closed by commit rG19ab46cc6604: acpi: Parse _S3D in s2idle instead of _S255D (authored by obiwac). · Explain WhyWed, Apr 8, 12:12 PM
This revision was automatically updated to reflect the committed changes.
obiwac added a commit: rG19ab46cc6604: acpi: Parse _S3D in s2idle instead of _S255D.

Revision Contents
Changeset List

PathSize
sys/
conf/
1 line
dev/
acpica/
115 lines
20 lines
39 lines

Diff 172077

View Options

sys/conf/files

Loading...
View Options

sys/dev/acpica/acpi.c

Loading...
View Options

sys/dev/acpica/acpi_spmc.c

Loading...
View Options

sys/dev/acpica/acpi_spmc_if.m

Loading...