Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/50004 )
Change subject: ACPI: Add acpi_reset_gnvs_for_wake() ......................................................................
ACPI: Add acpi_reset_gnvs_for_wake()
With chipset_power_state filled in romstage CBMEM hooks and GNVS allocated early in ramstage, GNVS wake source is now also filled for normal boot path.
Change-Id: I2d44770392d14d2d6e22cc98df9d1751c8717ff3 Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/50004 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/acpi/gnvs.c M src/include/acpi/acpi_gnvs.h M src/soc/amd/common/block/acpi/pm_state.c M src/soc/amd/picasso/acpi.c M src/soc/amd/stoneyridge/acpi.c M src/soc/intel/alderlake/acpi.c M src/soc/intel/apollolake/acpi.c M src/soc/intel/baytrail/acpi.c M src/soc/intel/baytrail/ramstage.c M src/soc/intel/braswell/acpi.c M src/soc/intel/broadwell/pch/lpc.c M src/soc/intel/broadwell/pch/ramstage.c M src/soc/intel/cannonlake/acpi.c M src/soc/intel/common/block/acpi/acpi_wake_source.c M src/soc/intel/elkhartlake/acpi.c M src/soc/intel/icelake/acpi.c M src/soc/intel/jasperlake/acpi.c M src/soc/intel/skylake/acpi.c M src/soc/intel/tigerlake/acpi.c 19 files changed, 63 insertions(+), 131 deletions(-)
Approvals: build bot (Jenkins): Verified Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/acpi/gnvs.c b/src/acpi/gnvs.c index 63740d0..01aff62 100644 --- a/src/acpi/gnvs.c +++ b/src/acpi/gnvs.c @@ -92,3 +92,16 @@ acpigen_pop_len(); } } + +int acpi_reset_gnvs_for_wake(struct global_nvs **gnvs_) +{ + if (!gnvs) + return -1; + + /* Set unknown wake source */ + gnvs->pm1i = -1; + gnvs->gpei = -1; + + *gnvs_ = gnvs; + return 0; +} diff --git a/src/include/acpi/acpi_gnvs.h b/src/include/acpi/acpi_gnvs.h index 498743f..86d68a4 100644 --- a/src/include/acpi/acpi_gnvs.h +++ b/src/include/acpi/acpi_gnvs.h @@ -5,12 +5,16 @@
#include <types.h>
+struct global_nvs; + void acpi_create_gnvs(void); #if CONFIG(ACPI_SOC_NVS) void *acpi_get_gnvs(void); void *acpi_get_device_nvs(void); +int acpi_reset_gnvs_for_wake(struct global_nvs **gnvs); #else static inline void *acpi_get_gnvs(void) { return NULL; } +static inline int acpi_reset_gnvs_for_wake(struct global_nvs **gnvs) { return -1; } #endif
void gnvs_assign_chromeos(void *gnvs_section); @@ -21,7 +25,6 @@ * Defined as weak in common acpi as gnvs structure definition is * chipset specific. */ -struct global_nvs; void soc_fill_gnvs(struct global_nvs *gnvs); void mainboard_fill_gnvs(struct global_nvs *gnvs);
diff --git a/src/soc/amd/common/block/acpi/pm_state.c b/src/soc/amd/common/block/acpi/pm_state.c index 643cb02..787b732 100644 --- a/src/soc/amd/common/block/acpi/pm_state.c +++ b/src/soc/amd/common/block/acpi/pm_state.c @@ -35,29 +35,25 @@ int index;
index = get_index_bit(state->pm1_sts & state->pm1_en, PM1_LIMIT); - if (index < 0) - gnvs->pm1i = ~0ULL; - else + if (index >= 0) gnvs->pm1i = index;
index = get_index_bit(state->gpe0_sts & state->gpe0_en, GPE0_LIMIT); - if (index < 0) - gnvs->gpei = ~0ULL; - else + if (index >= 0) gnvs->gpei = index; }
static void acpi_save_wake_source(void *unused) { const struct chipset_power_state *ps; - struct global_nvs *gnvs = acpi_get_gnvs(); - if (!gnvs) - return; + struct global_nvs *gnvs;
+ if (acpi_reset_gnvs_for_wake(&gnvs) < 0) + return; if (acpi_pm_state_for_wake(&ps) < 0) return;
pm_fill_gnvs(gnvs, &ps->gpe_state); }
-BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, acpi_save_wake_source, NULL); +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, acpi_save_wake_source, NULL); diff --git a/src/soc/amd/picasso/acpi.c b/src/soc/amd/picasso/acpi.c index 568a5dd..d629d1c 100644 --- a/src/soc/amd/picasso/acpi.c +++ b/src/soc/amd/picasso/acpi.c @@ -7,7 +7,6 @@ #include <string.h> #include <console/console.h> #include <acpi/acpi.h> -#include <acpi/acpi_gnvs.h> #include <acpi/acpigen.h> #include <device/pci_ops.h> #include <arch/ioapic.h> @@ -26,7 +25,6 @@ #include <soc/pci_devs.h> #include <soc/msr.h> #include <soc/southbridge.h> -#include <soc/nvs.h> #include <soc/gpio.h> #include <version.h> #include "chip.h" @@ -366,13 +364,6 @@ acpigen_pop_len(); }
-void soc_fill_gnvs(struct global_nvs *gnvs) -{ - /* Set unknown wake source */ - gnvs->pm1i = ~0ULL; - gnvs->gpei = ~0ULL; -} - static int acpigen_soc_gpio_op(const char *op, unsigned int gpio_num) { if (gpio_num >= SOC_GPIO_TOTAL_PINS) { diff --git a/src/soc/amd/stoneyridge/acpi.c b/src/soc/amd/stoneyridge/acpi.c index 8007e67..8b97ace 100644 --- a/src/soc/amd/stoneyridge/acpi.c +++ b/src/soc/amd/stoneyridge/acpi.c @@ -7,7 +7,6 @@ #include <string.h> #include <console/console.h> #include <acpi/acpi.h> -#include <acpi/acpi_gnvs.h> #include <acpi/acpigen.h> #include <device/pci_ops.h> #include <arch/ioapic.h> @@ -21,7 +20,6 @@ #include <soc/pci_devs.h> #include <soc/southbridge.h> #include <soc/northbridge.h> -#include <soc/nvs.h> #include <soc/gpio.h> #include <version.h>
@@ -159,13 +157,6 @@ acpigen_pop_len(); }
-void soc_fill_gnvs(struct global_nvs *gnvs) -{ - /* Set unknown wake source */ - gnvs->pm1i = ~0ULL; - gnvs->gpei = ~0ULL; -} - static void acpigen_soc_get_gpio_in_local5(uintptr_t addr) { /* diff --git a/src/soc/intel/alderlake/acpi.c b/src/soc/intel/alderlake/acpi.c index 1536de1..2eb6101 100644 --- a/src/soc/intel/alderlake/acpi.c +++ b/src/soc/intel/alderlake/acpi.c @@ -271,9 +271,6 @@ { config_t *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/apollolake/acpi.c b/src/soc/intel/apollolake/acpi.c index 2c271d2..879aec4 100644 --- a/src/soc/intel/apollolake/acpi.c +++ b/src/soc/intel/apollolake/acpi.c @@ -76,9 +76,6 @@ struct soc_intel_apollolake_config *cfg; cfg = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = ~0ULL; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = cfg->dptf_enable;
diff --git a/src/soc/intel/baytrail/acpi.c b/src/soc/intel/baytrail/acpi.c index 682f9b4..445441e 100644 --- a/src/soc/intel/baytrail/acpi.c +++ b/src/soc/intel/baytrail/acpi.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <acpi/acpi.h> -#include <acpi/acpi_gnvs.h> #include <acpi/acpigen.h> #include <arch/ioapic.h> #include <device/mmio.h> @@ -16,7 +15,6 @@ #include <soc/iomap.h> #include <soc/irq.h> #include <soc/msr.h> -#include <soc/nvs.h> #include <soc/pattrs.h> #include <soc/pm.h>
@@ -55,12 +53,6 @@ } };
-void soc_fill_gnvs(struct global_nvs *gnvs) -{ - /* Set unknown wake source */ - gnvs->pm1i = -1; -} - int acpi_sci_irq(void) { u32 *actl = (u32 *)(ILB_BASE_ADDRESS + ACTL); diff --git a/src/soc/intel/baytrail/ramstage.c b/src/soc/intel/baytrail/ramstage.c index f1401e2..28aa588 100644 --- a/src/soc/intel/baytrail/ramstage.c +++ b/src/soc/intel/baytrail/ramstage.c @@ -121,37 +121,38 @@ static void pm_fill_gnvs(struct global_nvs *gnvs, const struct chipset_power_state *ps) { uint16_t pm1; + int index; + pm1 = ps->pm1_sts & ps->pm1_en;
/* Scan for first set bit in PM1 */ - for (gnvs->pm1i = 0; gnvs->pm1i < 16; gnvs->pm1i++) { + for (index = 0; index < 16; index++) { if (pm1 & 1) break; pm1 >>= 1; }
- /* If unable to determine then return -1 */ - if (gnvs->pm1i >= 16) - gnvs->pm1i = -1; - - printk(BIOS_DEBUG, "ACPI System Wake Source is PM1 Index %d\n", - gnvs->pm1i); + if (index < 16) + gnvs->pm1i = index; }
static void acpi_save_wake_source(void *unused) { const struct chipset_power_state *ps; - struct global_nvs *gnvs = acpi_get_gnvs(); - if (!gnvs) - return; + struct global_nvs *gnvs;
+ if (acpi_reset_gnvs_for_wake(&gnvs) < 0) + return; if (acpi_pm_state_for_wake(&ps) < 0) return;
pm_fill_gnvs(gnvs, ps); + + printk(BIOS_DEBUG, "ACPI System Wake Source is PM1 Index %d\n", + gnvs->pm1i); }
-BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, acpi_save_wake_source, NULL); +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, acpi_save_wake_source, NULL);
static void baytrail_enable_2x_refresh_rate(void) { diff --git a/src/soc/intel/braswell/acpi.c b/src/soc/intel/braswell/acpi.c index 819990a..c55632c 100644 --- a/src/soc/intel/braswell/acpi.c +++ b/src/soc/intel/braswell/acpi.c @@ -61,9 +61,6 @@
void soc_fill_gnvs(struct global_nvs *gnvs) { - /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Fill in the Wi-Fi Region ID */ if (CONFIG(HAVE_REGULATORY_DOMAIN)) gnvs->cid1 = wifi_regulatory_domain(); diff --git a/src/soc/intel/broadwell/pch/lpc.c b/src/soc/intel/broadwell/pch/lpc.c index 377a4da..e6900d6 100644 --- a/src/soc/intel/broadwell/pch/lpc.c +++ b/src/soc/intel/broadwell/pch/lpc.c @@ -10,13 +10,11 @@ #include <device/pci_ops.h> #include <arch/ioapic.h> #include <acpi/acpi.h> -#include <acpi/acpi_gnvs.h> #include <cpu/x86/smm.h> #include <string.h> #include <soc/iobp.h> #include <soc/iomap.h> #include <soc/lpc.h> -#include <soc/nvs.h> #include <soc/pch.h> #include <soc/pci_devs.h> #include <soc/pm.h> @@ -603,12 +601,6 @@ pch_lpc_add_io_resources(dev); }
-void soc_fill_gnvs(struct global_nvs *gnvs) -{ - /* Set unknown wake source */ - gnvs->pm1i = -1; -} - static unsigned long broadwell_write_acpi_tables(const struct device *device, unsigned long current, struct acpi_rsdp *rsdp) diff --git a/src/soc/intel/broadwell/pch/ramstage.c b/src/soc/intel/broadwell/pch/ramstage.c index a75bd36..54654e2 100644 --- a/src/soc/intel/broadwell/pch/ramstage.c +++ b/src/soc/intel/broadwell/pch/ramstage.c @@ -15,60 +15,51 @@ static void pm_fill_gnvs(struct global_nvs *gnvs, const struct chipset_power_state *ps) { uint16_t pm1; - int gpe_reg; + int gpe_reg, index;
pm1 = ps->pm1_sts & ps->pm1_en;
/* Scan for first set bit in PM1 */ - for (gnvs->pm1i = 0; gnvs->pm1i < 16; gnvs->pm1i++) { + for (index = 0; index < 16; index++) { if (pm1 & 1) break; pm1 >>= 1; }
- /* If unable to determine then return -1 */ - if (gnvs->pm1i >= 16) - gnvs->pm1i = -1; + if (index < 16) + gnvs->pm1i = index;
/* 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++) { + for (index = start; index < end; index++) { 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); + if (index < GPE0_REG_MAX * GPE0_REG_SIZE) + gnvs->gpei = index; }
static void acpi_save_wake_source(void *unused) { const struct chipset_power_state *ps; - struct global_nvs *gnvs = acpi_get_gnvs(); - if (!gnvs) - return; + struct global_nvs *gnvs;
+ if (acpi_reset_gnvs_for_wake(&gnvs) < 0) + return; if (acpi_pm_state_for_wake(&ps) < 0) return;
pm_fill_gnvs(gnvs, ps); + + printk(BIOS_DEBUG, "ACPI _SWS is PM1 Index %lld GPE Index %lld\n", + gnvs->pm1i, gnvs->gpei); }
-BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, acpi_save_wake_source, NULL); +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, acpi_save_wake_source, NULL); diff --git a/src/soc/intel/cannonlake/acpi.c b/src/soc/intel/cannonlake/acpi.c index 5b09b48..03bf3c8 100644 --- a/src/soc/intel/cannonlake/acpi.c +++ b/src/soc/intel/cannonlake/acpi.c @@ -175,9 +175,6 @@ const struct soc_intel_cannonlake_config *config; config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/common/block/acpi/acpi_wake_source.c b/src/soc/intel/common/block/acpi/acpi_wake_source.c index 3ffcac8..710ccf5 100644 --- a/src/soc/intel/common/block/acpi/acpi_wake_source.c +++ b/src/soc/intel/common/block/acpi/acpi_wake_source.c @@ -14,7 +14,7 @@ static void pm_fill_gnvs(struct global_nvs *gnvs, const struct chipset_power_state *ps) { uint32_t pm1, *gpe0; - int gpe_reg, gpe_reg_count; + int index, gpe_reg, gpe_reg_count; int reg_size = sizeof(uint32_t) * 8;
gpe_reg_count = soc_fill_acpi_wake(ps, &pm1, &gpe0); @@ -22,15 +22,14 @@ return;
/* Scan for first set bit in PM1 */ - for (gnvs->pm1i = 0; gnvs->pm1i < reg_size; gnvs->pm1i++) { + for (index = 0; index < reg_size; index++) { if (pm1 & 1) break; pm1 >>= 1; }
- /* If unable to determine then return -1 */ - if (gnvs->pm1i >= 16) - gnvs->pm1i = -1; + if (index < 16) + gnvs->pm1i = index;
/* Scan for first set bit in GPE registers */ for (gpe_reg = 0; gpe_reg < gpe_reg_count; gpe_reg++) { @@ -38,41 +37,31 @@ int start = gpe_reg * reg_size; int end = start + reg_size;
- if (gpe == 0) { - if (!gnvs->gpei) - gnvs->gpei = end; - continue; - } - - for (gnvs->gpei = start; gnvs->gpei < end; gnvs->gpei++) { + for (index = start; index < end; index++) { if (gpe & 1) break; gpe >>= 1; } }
- /* If unable to determine then return -1 */ - if (gnvs->gpei >= gpe_reg_count * reg_size) - gnvs->gpei = -1; - - printk(BIOS_DEBUG, "ACPI _SWS is PM1 Index %lld GPE Index %lld\n", - (long long)gnvs->pm1i, (long long)gnvs->gpei); + if (index < gpe_reg_count * reg_size) + gnvs->gpei = index; }
static void acpi_save_wake_source(void *unused) { const struct chipset_power_state *ps; - struct global_nvs *gnvs = acpi_get_gnvs(); - if (!gnvs) + struct global_nvs *gnvs; + + if (acpi_reset_gnvs_for_wake(&gnvs) < 0) return; - - gnvs->pm1i = -1; - gnvs->gpei = -1; - if (acpi_pm_state_for_wake(&ps) < 0) return;
pm_fill_gnvs(gnvs, ps); + + printk(BIOS_DEBUG, "ACPI _SWS is PM1 Index %lld GPE Index %lld\n", + (long long)gnvs->pm1i, (long long)gnvs->gpei); }
-BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, acpi_save_wake_source, NULL); +BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, acpi_save_wake_source, NULL); diff --git a/src/soc/intel/elkhartlake/acpi.c b/src/soc/intel/elkhartlake/acpi.c index 0c87cfd..12706c9 100644 --- a/src/soc/intel/elkhartlake/acpi.c +++ b/src/soc/intel/elkhartlake/acpi.c @@ -239,9 +239,6 @@ { config_t *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/icelake/acpi.c b/src/soc/intel/icelake/acpi.c index 6240f68..67c43e2 100644 --- a/src/soc/intel/icelake/acpi.c +++ b/src/soc/intel/icelake/acpi.c @@ -170,9 +170,6 @@ { config_t *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/jasperlake/acpi.c b/src/soc/intel/jasperlake/acpi.c index 1a9286a..f33bebd 100644 --- a/src/soc/intel/jasperlake/acpi.c +++ b/src/soc/intel/jasperlake/acpi.c @@ -266,9 +266,6 @@ { config_t *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c index 10ce77e..100f16f 100644 --- a/src/soc/intel/skylake/acpi.c +++ b/src/soc/intel/skylake/acpi.c @@ -159,9 +159,6 @@ { const struct soc_intel_skylake_config *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
diff --git a/src/soc/intel/tigerlake/acpi.c b/src/soc/intel/tigerlake/acpi.c index d81efb9..17d8a34 100644 --- a/src/soc/intel/tigerlake/acpi.c +++ b/src/soc/intel/tigerlake/acpi.c @@ -266,9 +266,6 @@ { config_t *config = config_of_soc();
- /* Set unknown wake source */ - gnvs->pm1i = -1; - /* Enable DPTF based on mainboard configuration */ gnvs->dpte = config->dptf_enable;
-
Kyösti Mälkki (Code Review)