Aaron Durbin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44481 )
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
soc/amd/common: add single function ACPI PM1 GPE helpers
The existing code in common/block/acpi is mixing multiple operations: saving things to cbmem in common code but then soc code uses that information, reliant upon soc-specific struct soc_power_reg object, and only saving/snapshotting ACPI registers very deep in ramsatage.
To unwind the above provide some functions that are more targeted: - Add struct acpi_pm_gpe_state object - Add acpi_fill_pm_gpe_state() - Add acpi_pm_gpe_add_events_print_events() - Add acpi_clear_pm_gpe_status()
BUG=b:159947207
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: Ia7afed2861343802b3c78728784f7cfaf6f53f62 --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h 2 files changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/44481/1
diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 2be9b9e..be331e3 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -91,6 +91,28 @@ elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); }
+void acpi_fill_pm_gpe_state(struct acpi_pm_gpe_state *state) +{ + state->pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); + state->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); + state->gpe0_sts = acpi_read32(MMIO_ACPI_GPE0_STS); + state->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); + state->previous_sx_state = acpi_get_sleep_type(); + state->aligning_field = 0; +} + +void acpi_pm_gpe_add_events_print_events(const struct acpi_pm_gpe_state *state) +{ + log_pm1_status(state->pm1_sts); + print_pm1_status(state->pm1_sts); +} + +void acpi_clear_pm_gpe_status(void) +{ + acpi_write16(MMIO_ACPI_PM1_STS, acpi_read16(MMIO_ACPI_PM1_STS)); + acpi_write32(MMIO_ACPI_GPE0_STS, acpi_read32(MMIO_ACPI_GPE0_STS)); +} + static void save_sws(uint16_t pm1_status) { struct soc_power_reg *sws; diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h index 4d22735..8f16054 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpi.h +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -16,6 +16,23 @@ #define MMIO_ACPI_GPE0_STS 0x14 #define MMIO_ACPI_GPE0_EN 0x18
+/* Structure to maintain standard ACPI register state for reporting purposes. */ +struct acpi_pm_gpe_state { + uint16_t pm1_sts; + uint16_t pm1_en; + uint32_t gpe0_sts; + uint32_t gpe0_en; + uint16_t previous_sx_state; + uint16_t aligning_field; +}; + +/* Fill object with the ACPI PM and GPE state. */ +void acpi_fill_pm_gpe_state(struct acpi_pm_gpe_state *state); +/* Save events to eventlog log and also print information on console. */ +void acpi_pm_gpe_add_events_print_events(const struct acpi_pm_gpe_state *state); +/* Clear PM and GPE status registers. */ +void acpi_clear_pm_gpe_status(void); + void acpi_clear_pm1_status(void);
/*
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44481 )
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/44481/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44481/1//COMMIT_MSG@12 PS1, Line 12: ramsatage nit: ramstage
https://review.coreboot.org/c/coreboot/+/44481/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpi.h:
https://review.coreboot.org/c/coreboot/+/44481/1/src/soc/amd/common/block/in... PS1, Line 26: uint16_t aligning_field; Is this for structure alignment?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44481 )
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/44481/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44481/1//COMMIT_MSG@12 PS1, Line 12: ramsatage
nit: ramstage
Done
https://review.coreboot.org/c/coreboot/+/44481/1/src/soc/amd/common/block/in... File src/soc/amd/common/block/include/amdblocks/acpi.h:
https://review.coreboot.org/c/coreboot/+/44481/1/src/soc/amd/common/block/in... PS1, Line 26: uint16_t aligning_field;
Is this for structure alignment?
Yes
Hello build bot (Jenkins), Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44481
to look at the new patch set (#2).
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
soc/amd/common: add single function ACPI PM1 GPE helpers
The existing code in common/block/acpi is mixing multiple operations: saving things to cbmem in common code but then soc code uses that information, reliant upon soc-specific struct soc_power_reg object, and only saving/snapshotting ACPI registers very deep in ramstage.
To unwind the above provide some functions that are more targeted: - Add struct acpi_pm_gpe_state object - Add acpi_fill_pm_gpe_state() - Add acpi_pm_gpe_add_events_print_events() - Add acpi_clear_pm_gpe_status()
BUG=b:159947207
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: Ia7afed2861343802b3c78728784f7cfaf6f53f62 --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h 2 files changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/81/44481/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44481 )
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
Patch Set 2: Code-Review+2
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44481 )
Change subject: soc/amd/common: add single function ACPI PM1 GPE helpers ......................................................................
soc/amd/common: add single function ACPI PM1 GPE helpers
The existing code in common/block/acpi is mixing multiple operations: saving things to cbmem in common code but then soc code uses that information, reliant upon soc-specific struct soc_power_reg object, and only saving/snapshotting ACPI registers very deep in ramstage.
To unwind the above provide some functions that are more targeted: - Add struct acpi_pm_gpe_state object - Add acpi_fill_pm_gpe_state() - Add acpi_pm_gpe_add_events_print_events() - Add acpi_clear_pm_gpe_status()
BUG=b:159947207
Signed-off-by: Aaron Durbin adurbin@chromium.org Change-Id: Ia7afed2861343802b3c78728784f7cfaf6f53f62 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44481 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/amd/common/block/acpi/acpi.c M src/soc/amd/common/block/include/amdblocks/acpi.h 2 files changed, 39 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/amd/common/block/acpi/acpi.c b/src/soc/amd/common/block/acpi/acpi.c index 2be9b9e..be331e3 100644 --- a/src/soc/amd/common/block/acpi/acpi.c +++ b/src/soc/amd/common/block/acpi/acpi.c @@ -91,6 +91,28 @@ elog_add_event_wake(ELOG_WAKE_SOURCE_PCIE, 0); }
+void acpi_fill_pm_gpe_state(struct acpi_pm_gpe_state *state) +{ + state->pm1_sts = acpi_read16(MMIO_ACPI_PM1_STS); + state->pm1_en = acpi_read16(MMIO_ACPI_PM1_EN); + state->gpe0_sts = acpi_read32(MMIO_ACPI_GPE0_STS); + state->gpe0_en = acpi_read32(MMIO_ACPI_GPE0_EN); + state->previous_sx_state = acpi_get_sleep_type(); + state->aligning_field = 0; +} + +void acpi_pm_gpe_add_events_print_events(const struct acpi_pm_gpe_state *state) +{ + log_pm1_status(state->pm1_sts); + print_pm1_status(state->pm1_sts); +} + +void acpi_clear_pm_gpe_status(void) +{ + acpi_write16(MMIO_ACPI_PM1_STS, acpi_read16(MMIO_ACPI_PM1_STS)); + acpi_write32(MMIO_ACPI_GPE0_STS, acpi_read32(MMIO_ACPI_GPE0_STS)); +} + static void save_sws(uint16_t pm1_status) { struct soc_power_reg *sws; diff --git a/src/soc/amd/common/block/include/amdblocks/acpi.h b/src/soc/amd/common/block/include/amdblocks/acpi.h index 4d22735..8f16054 100644 --- a/src/soc/amd/common/block/include/amdblocks/acpi.h +++ b/src/soc/amd/common/block/include/amdblocks/acpi.h @@ -16,6 +16,23 @@ #define MMIO_ACPI_GPE0_STS 0x14 #define MMIO_ACPI_GPE0_EN 0x18
+/* Structure to maintain standard ACPI register state for reporting purposes. */ +struct acpi_pm_gpe_state { + uint16_t pm1_sts; + uint16_t pm1_en; + uint32_t gpe0_sts; + uint32_t gpe0_en; + uint16_t previous_sx_state; + uint16_t aligning_field; +}; + +/* Fill object with the ACPI PM and GPE state. */ +void acpi_fill_pm_gpe_state(struct acpi_pm_gpe_state *state); +/* Save events to eventlog log and also print information on console. */ +void acpi_pm_gpe_add_events_print_events(const struct acpi_pm_gpe_state *state); +/* Clear PM and GPE status registers. */ +void acpi_clear_pm_gpe_status(void); + void acpi_clear_pm1_status(void);
/*
-
Aaron Durbin (Code Review) -
Furquan Shaikh (Code Review)