Hello build bot (Jenkins), Patrick Georgi, Duncan Laurie, Paul Menzel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/46734
to review the following change.
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Revert "broadwell: Switch to using common ACPI _SWS code"
This reverts commit 81a4c85acf664156bb68807f681cd40928bf8267.
Reason for revert: Blocks merging Haswell and Broadwell together.
Change-Id: I29c4ad9174ab84c7e9111daa0491ede9e1d639b4 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi/platform.asl M src/soc/intel/broadwell/ramstage.c 3 files changed, 64 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46734/1
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index 6561fe2..e01d559 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -42,7 +42,6 @@ select UDELAY_TSC select SOC_INTEL_COMMON select HAVE_INTEL_FIRMWARE - select SOC_INTEL_COMMON_ACPI_WAKE_SOURCE
config BOOTBLOCK_CPU_INIT string diff --git a/src/soc/intel/broadwell/acpi/platform.asl b/src/soc/intel/broadwell/acpi/platform.asl index c00edeb..d302720 100644 --- a/src/soc/intel/broadwell/acpi/platform.asl +++ b/src/soc/intel/broadwell/acpi/platform.asl @@ -18,9 +18,6 @@ * Foundation, Inc. */
-/* Enable ACPI _SWS methods */ -#include <soc/intel/common/acpi/acpi_wake_source.asl> - /* The APM port can be used for generating software SMIs */
OperationRegion (APMP, SystemIO, 0xb2, 2) @@ -74,3 +71,21 @@ { Return (Package (){ 0, 0 }) } + +Scope (_SB) +{ + Method (_SWS) + { + /* Index into PM1 for device that caused wake */ + Return (\PM1I) + } +} + +Scope (_GPE) +{ + Method (_SWS) + { + /* Index into GPE for device that caused wake */ + Return (\GPEI) + } +} diff --git a/src/soc/intel/broadwell/ramstage.c b/src/soc/intel/broadwell/ramstage.c index c8fb6ed..e699e02 100644 --- a/src/soc/intel/broadwell/ramstage.c +++ b/src/soc/intel/broadwell/ramstage.c @@ -27,23 +27,56 @@ #include <soc/pm.h> #include <soc/ramstage.h> #include <soc/intel/broadwell/chip.h> -#include <soc/intel/common/acpi.h>
-/* Save wake source information for calculating ACPI _SWS values */ -int soc_fill_acpi_wake(uint32_t *pm1, uint32_t **gpe0) +/* Save bit index for PM1_STS and GPE_STS for ACPI _SWS */ +static void save_acpi_wake_source(global_nvs_t *gnvs) { struct chipset_power_state *ps = cbmem_find(CBMEM_ID_POWER_STATE); - static uint32_t gpe0_sts[GPE0_REG_MAX]; - int i; + uint16_t pm1; + int gpe_reg;
- *pm1 = ps->pm1_sts & ps->pm1_en; + if (!ps) + return;
- /* Mask off GPE0 status bits that are not enabled */ - *gpe0 = &gpe0_sts[0]; - for (i = 0; i < GPE0_REG_MAX; i++) - gpe0_sts[i] = ps->gpe0_sts[i] & ps->gpe0_en[i]; + pm1 = ps->pm1_sts & ps->pm1_en;
- return GPE0_REG_MAX; + /* Scan for first set bit in PM1 */ + for (gnvs->pm1i = 0; gnvs->pm1i < 16; gnvs->pm1i++) { + if (pm1 & 1) + break; + pm1 >>= 1; + } + + /* If unable to determine then return -1 */ + if (gnvs->pm1i >= 16) + gnvs->pm1i = -1; + + /* Scan for first set bit in GPE registers */ + gnvs->gpei = -1; + for (gpe_reg = 0; gpe_reg < GPE0_REG_MAX; gpe_reg++) { + u32 gpe = ps->gpe0_sts[gpe_reg] & ps->gpe0_en[gpe_reg]; + int start = gpe_reg * GPE0_REG_SIZE; + int end = start + GPE0_REG_SIZE; + + if (gpe == 0) { + if (!gnvs->gpei) + gnvs->gpei = end; + continue; + } + + for (gnvs->gpei = start; gnvs->gpei < end; gnvs->gpei++) { + if (gpe & 1) + break; + gpe >>= 1; + } + } + + /* If unable to determine then return -1 */ + if (gnvs->gpei >= (GPE0_REG_MAX * GPE0_REG_SIZE)) + gnvs->gpei = -1; + + printk(BIOS_DEBUG, "ACPI _SWS is PM1 Index %lld GPE Index %lld\n", + gnvs->pm1i, gnvs->gpei); }
static void s3_resume_prepare(void) @@ -56,6 +89,8 @@
if (!acpi_is_wakeup_s3()) memset(gnvs, 0, sizeof(global_nvs_t)); + else + save_acpi_wake_source(gnvs); }
void broadwell_init_pre_device(void *chip_info)
Hello build bot (Jenkins), Patrick Georgi, Duncan Laurie, Paul Menzel, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46734
to look at the new patch set (#2).
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Revert "broadwell: Switch to using common ACPI _SWS code"
This reverts commit 81a4c85acf664156bb68807f681cd40928bf8267.
Reason for revert: Blocks merging Haswell and Broadwell together.
Tested on out-of-tree Acer Aspire E5-573, still boots.
Change-Id: I29c4ad9174ab84c7e9111daa0491ede9e1d639b4 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi/platform.asl M src/soc/intel/broadwell/ramstage.c 3 files changed, 64 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46734/2
Hello build bot (Jenkins), Patrick Georgi, Duncan Laurie, Paul Menzel, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46734
to look at the new patch set (#4).
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Revert "broadwell: Switch to using common ACPI _SWS code"
This reverts commit 81a4c85acf664156bb68807f681cd40928bf8267.
Reason for revert: Blocks merging Haswell and Broadwell together.
Tested on out-of-tree Acer Aspire E5-573, still boots.
Change-Id: I29c4ad9174ab84c7e9111daa0491ede9e1d639b4 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi/platform.asl M src/soc/intel/broadwell/ramstage.c 3 files changed, 64 insertions(+), 16 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/46734/4
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46734 )
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Patch Set 9:
What about making haswell use the common code instead of reverting this?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46734 )
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Patch Set 9:
Patch Set 9:
What about making haswell use the common code instead of reverting this?
Neither Haswell nor Broadwell are SoCs. I can switch to using common code again once there's only one copy of this code.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46734 )
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Patch Set 9: Code-Review+2
Patch Set 9:
Patch Set 9:
What about making haswell use the common code instead of reverting this?
Neither Haswell nor Broadwell are SoCs. I can switch to using common code again once there's only one copy of this code.
Who said they are? ;) Yeah, would be great. That whole wake source stuff could go to southbridge/common
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46734 )
Change subject: Revert "broadwell: Switch to using common ACPI _SWS code" ......................................................................
Revert "broadwell: Switch to using common ACPI _SWS code"
This reverts commit 81a4c85acf664156bb68807f681cd40928bf8267.
Reason for revert: Blocks merging Haswell and Broadwell together.
Tested on out-of-tree Acer Aspire E5-573, still boots.
Change-Id: I29c4ad9174ab84c7e9111daa0491ede9e1d639b4 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/46734 Reviewed-by: Michael Niewöhner foss@mniewoehner.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/broadwell/Kconfig M src/soc/intel/broadwell/acpi/platform.asl M src/soc/intel/broadwell/ramstage.c 3 files changed, 64 insertions(+), 16 deletions(-)
Approvals: build bot (Jenkins): Verified Michael Niewöhner: Looks good to me, approved
diff --git a/src/soc/intel/broadwell/Kconfig b/src/soc/intel/broadwell/Kconfig index 2430be6..6e57f0a 100644 --- a/src/soc/intel/broadwell/Kconfig +++ b/src/soc/intel/broadwell/Kconfig @@ -34,7 +34,6 @@ select TSC_MONOTONIC_TIMER select SOC_INTEL_COMMON select INTEL_DESCRIPTOR_MODE_CAPABLE - select SOC_INTEL_COMMON_ACPI_WAKE_SOURCE select HAVE_EM100PRO_SPI_CONSOLE_SUPPORT select INTEL_GMA_ACPI select HAVE_POWER_STATE_AFTER_FAILURE diff --git a/src/soc/intel/broadwell/acpi/platform.asl b/src/soc/intel/broadwell/acpi/platform.asl index f404352..880b206 100644 --- a/src/soc/intel/broadwell/acpi/platform.asl +++ b/src/soc/intel/broadwell/acpi/platform.asl @@ -1,7 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */
-/* Enable ACPI _SWS methods */ -#include <soc/intel/common/acpi/acpi_wake_source.asl> #include <southbridge/intel/common/acpi/platform.asl>
/* @@ -19,3 +17,21 @@ { Return (Package (){ 0, 0 }) } + +Scope (_SB) +{ + Method (_SWS) + { + /* Index into PM1 for device that caused wake */ + Return (\PM1I) + } +} + +Scope (_GPE) +{ + Method (_SWS) + { + /* Index into GPE for device that caused wake */ + Return (\GPEI) + } +} diff --git a/src/soc/intel/broadwell/ramstage.c b/src/soc/intel/broadwell/ramstage.c index c414d62..57abf95 100644 --- a/src/soc/intel/broadwell/ramstage.c +++ b/src/soc/intel/broadwell/ramstage.c @@ -2,32 +2,63 @@
#include <acpi/acpi.h> #include <cbmem.h> +#include <console/console.h> #include <device/device.h> #include <string.h> #include <soc/nvs.h> #include <soc/pm.h> #include <soc/ramstage.h> #include <soc/intel/broadwell/chip.h> -#include <soc/intel/common/acpi.h> -#include <assert.h>
-/* Save wake source information for calculating ACPI _SWS values */ -int soc_fill_acpi_wake(uint32_t *pm1, uint32_t **gpe0) +/* Save bit index for PM1_STS and GPE_STS for ACPI _SWS */ +static void save_acpi_wake_source(struct global_nvs *gnvs) { struct chipset_power_state *ps = cbmem_find(CBMEM_ID_POWER_STATE); - static uint32_t gpe0_sts[GPE0_REG_MAX]; - int i; + uint16_t pm1; + int gpe_reg;
- assert(ps != NULL); + if (!ps) + return;
- *pm1 = ps->pm1_sts & ps->pm1_en; + pm1 = ps->pm1_sts & ps->pm1_en;
- /* Mask off GPE0 status bits that are not enabled */ - *gpe0 = &gpe0_sts[0]; - for (i = 0; i < GPE0_REG_MAX; i++) - gpe0_sts[i] = ps->gpe0_sts[i] & ps->gpe0_en[i]; + /* Scan for first set bit in PM1 */ + for (gnvs->pm1i = 0; gnvs->pm1i < 16; gnvs->pm1i++) { + if (pm1 & 1) + break; + pm1 >>= 1; + }
- return GPE0_REG_MAX; + /* If unable to determine then return -1 */ + if (gnvs->pm1i >= 16) + gnvs->pm1i = -1; + + /* Scan for first set bit in GPE registers */ + gnvs->gpei = -1; + for (gpe_reg = 0; gpe_reg < GPE0_REG_MAX; gpe_reg++) { + u32 gpe = ps->gpe0_sts[gpe_reg] & ps->gpe0_en[gpe_reg]; + int start = gpe_reg * GPE0_REG_SIZE; + int end = start + GPE0_REG_SIZE; + + if (gpe == 0) { + if (!gnvs->gpei) + gnvs->gpei = end; + continue; + } + + for (gnvs->gpei = start; gnvs->gpei < end; gnvs->gpei++) { + if (gpe & 1) + break; + gpe >>= 1; + } + } + + /* If unable to determine then return -1 */ + if (gnvs->gpei >= (GPE0_REG_MAX * GPE0_REG_SIZE)) + gnvs->gpei = -1; + + printk(BIOS_DEBUG, "ACPI _SWS is PM1 Index %lld GPE Index %lld\n", + gnvs->pm1i, gnvs->gpei); }
static void s3_resume_prepare(void) @@ -40,6 +71,8 @@
if (!acpi_is_wakeup_s3()) memset(gnvs, 0, sizeof(struct global_nvs)); + else + save_acpi_wake_source(gnvs); }
void broadwell_init_pre_device(void *chip_info)
-
Angel Pons (Code Review) -
Michael Niewöhner (Code Review)