Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
mb/google/hatch: Enable LPIT inclusion in DSDT
Include the lpit.asl file in Hatch's DSDT definition.
BUG=b:130764684 BRANCH=none TEST=compiles
Change-Id: If8ebff3db091257e8452869636c0e024f3123e8b Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/dsdt.asl 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/34175/1
diff --git a/src/mainboard/google/hatch/dsdt.asl b/src/mainboard/google/hatch/dsdt.asl index 243c627..87e98ea 100644 --- a/src/mainboard/google/hatch/dsdt.asl +++ b/src/mainboard/google/hatch/dsdt.asl @@ -51,6 +51,9 @@ /* Chipset specific sleep states */ #include <soc/intel/cannonlake/acpi/sleepstates.asl>
+ /* Low power idle table */ + #include <soc/intel/cannonlake/acpi/lpit.asl> + /* Chrome OS Embedded Controller */ Scope (_SB.PCI0.LPCB) {
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
Does the kernel use this table? Have you tried booting it?
Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
Patch Set 2:
Does the kernel use this table? Have you tried booting it?
Yes, I believe the kernel *will* try to use it to get S0ix increment counter register address. Tim, can you please run a few S0ix suspend_Stress_test iterations with this?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
Patch Set 2:
Does the kernel use this table? Have you tried booting it?
Yes it does and I have. For example, Wilco uses it.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Does the kernel use this table? Have you tried booting it?
Yes it does and I have. For example, Wilco uses it.
See drivers/acpi/acpi_lpit.c
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34175/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34175/2//COMMIT_MSG@13 PS2, Line 13: compiles Can you please test S3 and S0ix to ensure both work fine?
1. Using suspend_stress_test (at least 100 iterations) 2. Using echo {mem,freeze} > /sys/power/state. P.S. You need to set /sys/power/mem_sleep correctly too
Also, do you observe any issues with eventlog i.e. logging of S0ix enter/exit happening twice for a single iteration? Any other issues?
Evan Green has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Does the kernel use this table? Have you tried booting it?
Yes it does and I have. For example, Wilco uses it.
Thanks. Can you add what you did to the TEST= line.
Hello Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34175
to look at the new patch set (#3).
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
mb/google/hatch: Enable LPIT inclusion in DSDT
Include the lpit.asl file in Hatch's DSDT definition.
BUG=b:130764684 BRANCH=none TEST=S0ix and S3 suspend/resume work as intended. Ran > 100 iterations of suspend_stress_test.
Change-Id: If8ebff3db091257e8452869636c0e024f3123e8b Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/dsdt.asl 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/34175/3
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 3: Code-Review+1
Hello Paul Fagerburg, Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34175
to look at the new patch set (#4).
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
mb/google/hatch: Enable LPIT inclusion in DSDT
Include the lpit.asl file in Hatch's DSDT definition.
BUG=b:130764684 BRANCH=none TEST=compiles
Change-Id: If8ebff3db091257e8452869636c0e024f3123e8b Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/dsdt.asl 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/34175/4
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 4: Code-Review+1
Hello Paul Fagerburg, Subrata Banik, Duncan Laurie, Shelley Chen, build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34175
to look at the new patch set (#5).
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
mb/google/hatch: Enable LPIT inclusion in DSDT
Include the lpit.asl file in Hatch's DSDT definition.
BUG=b:130764684 BRANCH=none TEST=S3 suspend/resume and S0ix entry/exit work correctly. Ran > 200 iterations of suspend_stress_test and no issues found.
Change-Id: If8ebff3db091257e8452869636c0e024f3123e8b Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/mainboard/google/hatch/dsdt.asl 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/34175/5
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 5: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
Patch Set 5: Code-Review+2
Martin Roth has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34175 )
Change subject: mb/google/hatch: Enable LPIT inclusion in DSDT ......................................................................
mb/google/hatch: Enable LPIT inclusion in DSDT
Include the lpit.asl file in Hatch's DSDT definition.
BUG=b:130764684 BRANCH=none TEST=S3 suspend/resume and S0ix entry/exit work correctly. Ran > 200 iterations of suspend_stress_test and no issues found.
Change-Id: If8ebff3db091257e8452869636c0e024f3123e8b Signed-off-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/34175 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Paul Fagerburg pfagerburg@chromium.org --- M src/mainboard/google/hatch/dsdt.asl 1 file changed, 3 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Subrata Banik: Looks good to me, approved Paul Fagerburg: Looks good to me, but someone else must approve
diff --git a/src/mainboard/google/hatch/dsdt.asl b/src/mainboard/google/hatch/dsdt.asl index 243c627..87e98ea 100644 --- a/src/mainboard/google/hatch/dsdt.asl +++ b/src/mainboard/google/hatch/dsdt.asl @@ -51,6 +51,9 @@ /* Chipset specific sleep states */ #include <soc/intel/cannonlake/acpi/sleepstates.asl>
+ /* Low power idle table */ + #include <soc/intel/cannonlake/acpi/lpit.asl> + /* Chrome OS Embedded Controller */ Scope (_SB.PCI0.LPCB) {
-
Evan Green (Code Review) -
Furquan Shaikh (Code Review)
-
Martin Roth (Code Review)
-
Paul Fagerburg (Code Review)
-
Rajat Jain (Code Review)
-
Subrata Banik (Code Review)
-
Tim Wawrzynczak (Code Review)