Hello Kyösti Mälkki,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/34995
to review the following change.
Change subject: arch/x86: Cache the TSEG region at the top of ram
......................................................................
arch/x86: Cache the TSEG region at the top of ram
This patch helps to save additional ~6ms of booting time in
normal and s3 resume on CML-hatch.
Change-Id: I59432c02e04af1b931d77de3f6652b0327ca82bb
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Signed-off-by: Subrata Banik <subrata.banik(a)intel.com>
---
M src/arch/x86/postcar_loader.c
1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/95/34995/1
diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c
index bceed6c..408d155 100644
--- a/src/arch/x86/postcar_loader.c
+++ b/src/arch/x86/postcar_loader.c
@@ -20,6 +20,7 @@
#include <cpu/cpu.h>
#include <cpu/x86/msr.h>
#include <cpu/x86/mtrr.h>
+#include <cpu/x86/smm.h>
#include <program_loading.h>
#include <rmodule.h>
#include <romstage_handoff.h>
@@ -149,6 +150,23 @@
set_var_mtrr(mtrr, base, size, MTRR_TYPE_WRPROT);
}
+/*
+ * Cache the TSEG region at the top of ram. This region is
+ * not restricted to SMM mode until SMM has been relocated.
+ * By setting the region to cacheable it provides faster access
+ * when relocating the SMM handler as well as using the TSEG
+ * region for other purposes.
+ */
+static void enable_tseg_cache(struct postcar_frame *pcf)
+{
+ uintptr_t smm_base;
+ size_t smm_size;
+
+ smm_region(&smm_base, &smm_size);
+ postcar_frame_add_mtrr(pcf, smm_base, smm_size,
+ MTRR_TYPE_WRBACK);
+}
+
void postcar_frame_setup_top_of_dram_usage(struct postcar_frame *pcf,
uintptr_t addr, size_t size, int type)
{
@@ -159,6 +177,9 @@
*/
if (!acpi_is_wakeup_s3())
enable_top_of_dram_cache(addr, size);
+
+ enable_tseg_cache(pcf);
+
postcar_frame_add_mtrr(pcf, addr, size, type);
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/34995
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I59432c02e04af1b931d77de3f6652b0327ca82bb
Gerrit-Change-Number: 34995
Gerrit-PatchSet: 1
Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newchange
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35034 )
Change subject: arch/x86: Fix .bss section in CAR
......................................................................
arch/x86: Fix .bss section in CAR
Fix clearing CAR region, 'stosl' stores 4 bytes at a time.
Restrict the use of symbol names _car_global_[start|end]
to be used exclusively with CAR_GLOBAL_MIGRATION=y.
They just alias the start and end of .bss section in CAR.
Change-Id: I36c858a4f181516d4c61f9fd1d5005c7d2c06057
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
---
M src/arch/x86/assembly_entry.S
M src/arch/x86/car.ld
M src/arch/x86/include/arch/early_variables.h
M src/arch/x86/include/arch/symbols.h
M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
M src/soc/intel/quark/romstage/fsp2_0.c
6 files changed, 38 insertions(+), 22 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/35034/1
diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S
index 4ead9ea..b3871d6 100644
--- a/src/arch/x86/assembly_entry.S
+++ b/src/arch/x86/assembly_entry.S
@@ -35,12 +35,13 @@
/* reset stack pointer to CAR stack */
mov $_car_stack_end, %esp
- /* clear CAR_GLOBAL area as it is not shared */
+ /* clear .bss section as it is not shared */
cld
xor %eax, %eax
- movl $(_car_global_end), %ecx
- movl $(_car_global_start), %edi
+ movl $(_ebss), %ecx
+ movl $(_bss), %edi
sub %edi, %ecx
+ shrl $2, %ecx
rep stosl
#if ((ENV_VERSTAGE && CONFIG(VERSTAGE_DEBUG_SPINLOOP)) \
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index 822f7fd..bc0090b 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -68,11 +68,13 @@
. += 80;
_car_ehci_dbg_info_end = .;
- /* _car_global_start and _car_global_end provide symbols to per-stage
+ /* _bss and _ebss provide symbols to per-stage
* variables that are not shared like the timestamp and the pre-ram
* cbmem console. This is useful for clearing this area on a per-stage
* basis when more than one stage uses cache-as-ram for CAR_GLOBALs. */
- _car_global_start = .;
+
+ . = ALIGN(ARCH_POINTER_ALIGN_SIZE);
+ _bss = .;
#if ENV_STAGE_HAS_BSS_SECTION
/* Allow global uninitialized variables for stages without CAR teardown. */
*(.bss)
@@ -80,10 +82,12 @@
*(.sbss)
*(.sbss.*)
#else
+ _car_global_start = .;
*(.car.global_data);
+ _car_global_end = .;
#endif
. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
- _car_global_end = .;
+ _ebss = .;
#if CONFIG(CAR_GLOBAL_MIGRATION)
_car_stack_start = .;
diff --git a/src/arch/x86/include/arch/early_variables.h b/src/arch/x86/include/arch/early_variables.h
index b5db194..9e0441c 100644
--- a/src/arch/x86/include/arch/early_variables.h
+++ b/src/arch/x86/include/arch/early_variables.h
@@ -22,6 +22,15 @@
#if ENV_ROMSTAGE && CONFIG(CAR_GLOBAL_MIGRATION)
+/*
+ * The _car_global_[start|end]symbols cover CAR data which is relocatable
+ * once memory comes online. Variables with CAR_GLOBAL decoration
+ * reside within this region.
+ */
+extern char _car_global_start[];
+extern char _car_global_end[];
+#define _car_global_size (_car_global_end - _car_global_start)
+
asm(".section .car.global_data,\"w\",@nobits");
asm(".previous");
#ifdef __clang__
diff --git a/src/arch/x86/include/arch/symbols.h b/src/arch/x86/include/arch/symbols.h
index 5d65a7b..3f2e978 100644
--- a/src/arch/x86/include/arch/symbols.h
+++ b/src/arch/x86/include/arch/symbols.h
@@ -27,25 +27,26 @@
#define _car_region_size (_car_region_end - _car_region_start)
/*
- * This is the stack used under CONFIG_C_ENVIRONMENT_BOOTBLOCK for
- * all stages that execute when cache-as-ram is up.
+ * This is the stack area used for all stages that execute when cache-as-ram
+ * is up. Area is not cleared in between stages.
*/
extern char _car_stack_start[];
extern char _car_stack_end[];
#define _car_stack_size (_car_stack_end - _car_stack_start)
+/*
+ * This is the .bss section cleared between each stage. It is not
+ * available in romstage if CAR_GLOBAL_MIGRATION is enabled.
+ */
+extern char _bss[];
+extern char _ebss[];
+#define _bss_size (_ebss - _bss)
+
+extern char _car_unallocated_start;
+
extern char _car_ehci_dbg_info_start[];
extern char _car_ehci_dbg_info_end[];
#define _car_ehci_dbg_info_size \
(_car_ehci_dbg_info_end - _car_ehci_dbg_info_start)
-/*
- * The _car_global_[start|end]symbols cover CAR data which is relocatable
- * once memory comes online. Variables with CAR_GLOBAL decoration
- * reside within this region.
- */
-extern char _car_global_start[];
-extern char _car_global_end[];
-#define _car_global_size (_car_global_end - _car_global_start)
-
#endif
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
index 9a8ab5b..091fc4a 100644
--- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
+++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
@@ -89,11 +89,11 @@
/* Setup bootblock stack */
mov %edx, %esp
- /* clear CAR_GLOBAL area as it is not shared */
+ /* clear .bss section as it is not shared */
cld
xor %eax, %eax
- movl $(_car_global_end), %ecx
- movl $(_car_global_start), %edi
+ movl $(_ebss), %ecx
+ movl $(_bss), %edi
sub %edi, %ecx
shrl $2, %ecx
rep stosl
diff --git a/src/soc/intel/quark/romstage/fsp2_0.c b/src/soc/intel/quark/romstage/fsp2_0.c
index e6da5dd..0cf2d80 100644
--- a/src/soc/intel/quark/romstage/fsp2_0.c
+++ b/src/soc/intel/quark/romstage/fsp2_0.c
@@ -14,6 +14,7 @@
*/
#include <arch/cpu.h>
+#include <arch/early_variables.h>
#include <arch/romstage.h>
#include <arch/symbols.h>
#include <console/console.h>
@@ -142,10 +143,10 @@
aupd->StackBase);
printk(BIOS_SPEW, "| |\n");
printk(BIOS_SPEW, "+-------------------+ 0x%p\n",
- _car_global_end);
+ _bss);
printk(BIOS_SPEW, "| coreboot data |\n");
printk(BIOS_SPEW, "+-------------------+ 0x%p\n",
- _car_stack_end);
+ _car_stack_start);
printk(BIOS_SPEW, "| coreboot stack |\n");
printk(BIOS_SPEW,
"+-------------------+ 0x80000000 - ESRAM start\n\n");
--
To view, visit https://review.coreboot.org/c/coreboot/+/35034
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I36c858a4f181516d4c61f9fd1d5005c7d2c06057
Gerrit-Change-Number: 35034
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35032 )
Change subject: timestamps: Make ENV_ROMSTAGE_OR_BEFORE improvements
......................................................................
timestamps: Make ENV_ROMSTAGE_OR_BEFORE improvements
Keep track of the active timestamp table location using
a CAR_GLOBAL variable. Done this way, the entire table
can be located outside _car_relocatable_data and we just
switch the pointer to CBMEM and copy the data before
CAR gets torn down.
TBD: This probably breaks FSP1_0 which returns to coreboot
proper only after tearing down CAR. I have an idea how to
fix it though.
Change-Id: I87370f62db23318069b6fd56ba0d1171d619cb8a
Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
---
M src/arch/x86/car.ld
M src/include/timestamp.h
M src/lib/timestamp.c
3 files changed, 24 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/35032/1
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index d19818c..25b667c 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -60,17 +60,16 @@
. += 32;
_epdpt = .;
#endif
- _car_relocatable_data_start = .;
- /* The timestamp implementation relies on this storage to be around
- * after migration. One of the fields indicates not to use it as the
- * backing store once cbmem comes online. Therefore, this data needs
- * to reside in the migrated area (between _car_relocatable_data_start
- * and _car_relocatable_data_end). */
+
TIMESTAMP(., 0x200)
+
_car_ehci_dbg_info_start = .;
/* Reserve sizeof(struct ehci_dbg_info). */
. += 80;
_car_ehci_dbg_info_end = .;
+
+ _car_relocatable_data_start = .;
+
/* _car_global_start and _car_global_end provide symbols to per-stage
* variables that are not shared like the timestamp and the pre-ram
* cbmem console. This is useful for clearing this area on a per-stage
diff --git a/src/include/timestamp.h b/src/include/timestamp.h
index 04d5c12..1fa568c 100644
--- a/src/include/timestamp.h
+++ b/src/include/timestamp.h
@@ -22,11 +22,10 @@
#if CONFIG(COLLECT_TIMESTAMPS)
/*
* timestamp_init() needs to be called once for each of these cases:
- * 1. __PRE_RAM__ (bootblock, romstage, verstage, etc) and
- * 2. !__PRE_RAM__ (ramstage)
+ * 1. ENV_ROMSTAGE_OR_BEFORE (bootblock, romstage, verstage, etc) and
+ * 2. ENV_RAMSTAGE (ramstage)
* The latter is taken care of by the generic coreboot infrastructure so
- * it's up to the chipset/arch to call timestamp_init() in *one* of
- * the __PRE_RAM__ stages. If multiple calls are made timestamps will be lost.
+ * it's up to the chipset/arch to call timestamp_init() in the former stages.
*/
void timestamp_init(uint64_t base);
/*
diff --git a/src/lib/timestamp.c b/src/lib/timestamp.c
index 89152fd..3e9914e 100644
--- a/src/lib/timestamp.c
+++ b/src/lib/timestamp.c
@@ -41,7 +41,7 @@
DECLARE_OPTIONAL_REGION(timestamp);
-#if defined(__PRE_RAM__)
+#if ENV_ROMSTAGE_OR_BEFORE
#define USE_TIMESTAMP_REGION (REGION_SIZE(timestamp) > 0)
#else
#define USE_TIMESTAMP_REGION 0
@@ -58,6 +58,10 @@
*/
static struct timestamp_cache timestamp_cache;
+/* This points to the active timestamp_table and can change within a stage.
+ as CBMEM comes available. */
+static struct timestamp_table *glob_ts_table CAR_GLOBAL;
+
enum {
TIMESTAMP_CACHE_UNINITIALIZED = 0,
TIMESTAMP_CACHE_INITIALIZED,
@@ -87,7 +91,7 @@
} else if (USE_TIMESTAMP_REGION) {
if (REGION_SIZE(timestamp) < sizeof(*ts_cache))
BUG();
- ts_cache = car_get_var_ptr((void *)_timestamp);
+ ts_cache = (void *)_timestamp;
}
return ts_cache;
@@ -128,30 +132,21 @@
static struct timestamp_table *timestamp_table_get(void)
{
- MAYBE_STATIC_BSS struct timestamp_table *ts_table = NULL;
+ struct timestamp_table *ts_table;
struct timestamp_cache *ts_cache;
+ ts_table = car_get_var(glob_ts_table);
if (ts_table != NULL)
return ts_table;
ts_cache = timestamp_cache_get();
+ if (ts_cache)
+ ts_table = &ts_cache->table;
- if (ts_cache == NULL) {
- if (HAS_CBMEM)
- ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);
- return ts_table;
- }
+ if (ts_table == NULL && HAS_CBMEM)
+ ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);
- /* Cache is required. */
- if (ts_cache->cache_state != TIMESTAMP_CACHE_NOT_NEEDED)
- return &ts_cache->table;
-
- /* Cache shouldn't be used but there's no backing store. */
- if (!HAS_CBMEM)
- return NULL;
-
- ts_table = cbmem_find(CBMEM_ID_TIMESTAMP);
-
+ car_set_var(glob_ts_table, ts_table);
return ts_table;
}
@@ -237,7 +232,7 @@
uint32_t i;
struct timestamp_cache *ts_cache;
struct timestamp_table *ts_cache_table;
- struct timestamp_table *ts_cbmem_table = NULL;
+ struct timestamp_table *ts_cbmem_table;
if (!timestamp_should_run())
return;
@@ -270,6 +265,7 @@
if (ts_cbmem_table == NULL) {
printk(BIOS_ERR, "ERROR: No timestamp table allocated\n");
+ car_set_var(glob_ts_table, ts_cbmem_table);
return;
}
@@ -309,6 +305,7 @@
ts_cbmem_table->tick_freq_mhz = timestamp_tick_freq_mhz();
/* Cache no longer required. */
+ car_set_var(glob_ts_table, ts_cbmem_table);
ts_cache_table->num_entries = 0;
ts_cache->cache_state = TIMESTAMP_CACHE_NOT_NEEDED;
}
--
To view, visit https://review.coreboot.org/c/coreboot/+/35032
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I87370f62db23318069b6fd56ba0d1171d619cb8a
Gerrit-Change-Number: 35032
Gerrit-PatchSet: 1
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: newchange
huayang duan has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34989 )
Change subject: mediatek/mt8183: Add new config for EMCP DDR
......................................................................
mediatek/mt8183: Add new config for EMCP DDR
Devices using eMCP may run in a high DRAM frequency (e.g., 3600Mbs) while
discrete DRAM can only run at 3200Mbps.
A new Kconfig is introduced so mainboard can select right config if the
board support eMCP or not.
BUG=b:80501386
BRANCH=none
TEST=Boots correctly and stress test pass on Kukui.
Change-Id: I9b73c8b512db5104896ea0d330d56e63eb50a44b
Signed-off-by: Huayang Duan <huayang.duan(a)mediatek.com>
---
M src/soc/mediatek/mt8183/Kconfig
1 file changed, 6 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/89/34989/1
diff --git a/src/soc/mediatek/mt8183/Kconfig b/src/soc/mediatek/mt8183/Kconfig
index c60cdea..a2f7a04 100644
--- a/src/soc/mediatek/mt8183/Kconfig
+++ b/src/soc/mediatek/mt8183/Kconfig
@@ -22,6 +22,12 @@
help
This option enables additional DRAM related debug messages.
+config MT8183_DRAM_EMCP
+ bool
+ default n
+ help
+ The EMCP platform should select this config for using different DRAM frequency.
+
config MEMORY_TEST
bool
default y
--
To view, visit https://review.coreboot.org/c/coreboot/+/34989
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I9b73c8b512db5104896ea0d330d56e63eb50a44b
Gerrit-Change-Number: 34989
Gerrit-PatchSet: 1
Gerrit-Owner: huayang duan <huayangduan(a)gmail.com>
Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: huayang duan <huayangduan(a)gmail.com>
Gerrit-MessageType: newchange
Hello Raul Rangel,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/em100/+/34830
to review the following change.
Change subject: Automatically select the right FPGA firmware for 1.8/3.3V
......................................................................
Automatically select the right FPGA firmware for 1.8/3.3V
With the -G2 variant we can automatically select the right voltage. Do
this instead of just giving up. For the original em100 this will produce
an error, with a 2s delay.
Success (new unit):
will emulate 'W25Q128FW'
MCU version: 3.03
FPGA version: 2.10 (3.3V)
Serial number: DP142535
SPI flash database: 4.3.01
EM100Pro currently stopped
EM100Pro hold pin currently low
Stopped EM100Pro
Sending flash chip configuration
Voltage set to 1.8
Chip set to W25Q128FW
Hold pin state set to low
Sent 2097152 bytes of 16777216
...
Failure (old unit):
will emulate 'GD25Q32B'
MCU version: 2.27
FPGA version: 0.85 (1.8V)
Serial number: DP142073
SPI flash database: 4.3.01
EM100Pro currently running
EM100Pro hold pin currently low
Stopped EM100Pro
Sending flash chip configuration
Invalid voltage response: 0x12 (expected 0x21)
Error: The current FPGA firmware (1.8V) does not support GigaDevice GD25Q32B (3.3V)
Failed configuring chip type.
Change-Id: I55596b9857c1f340d532acf5d261800a654068c6
Signed-off-by: Simon Glass <sjg(a)chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1395949
Reviewed-by: Raul E Rangel <rrangel(a)chromium.org>
---
M em100.c
1 file changed, 12 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/em100 refs/changes/30/34830/1
diff --git a/em100.c b/em100.c
index 01fe761..fcd7bcd 100644
--- a/em100.c
+++ b/em100.c
@@ -510,7 +510,8 @@
*/
int result = 0;
int i;
- int fpga_voltage, chip_voltage = 0, wrong_voltage = 0;
+ int fpga_voltage, chip_voltage = 0;
+ int req_voltage = 0;
printf("Sending flash chip configuration\n");
@@ -528,22 +529,25 @@
case 1601: /* 1.65V-2V */
case 1800:
if (fpga_voltage == 3300)
- wrong_voltage = 1;
+ req_voltage = 18;
break;
case 2500: /* supported by both 1.8V and 3.3V FPGA */
break;
case 3300:
if (fpga_voltage == 1800)
- wrong_voltage = 1;
+ req_voltage = 33;
}
break;
}
- if (wrong_voltage) {
- printf("Error: The current FPGA firmware (%.1fV) does not "
- "support %s %s (%.1fV)\n", (float)fpga_voltage/1000,
- desc->vendor, desc->name, (float)chip_voltage/1000);
- return 0;
+ if (req_voltage) {
+ if (!set_fpga_voltage(em100, req_voltage)) {
+ printf("Error: The current FPGA firmware (%.1fV) does "
+ "not support %s %s (%.1fV)\n",
+ (float)fpga_voltage / 1000, desc->vendor,
+ desc->name, (float)chip_voltage / 1000);
+ return 0;
+ }
}
for (i = 0; i < desc->init_len; i++) {
--
To view, visit https://review.coreboot.org/c/em100/+/34830
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: em100
Gerrit-Branch: master
Gerrit-Change-Id: I55596b9857c1f340d532acf5d261800a654068c6
Gerrit-Change-Number: 34830
Gerrit-PatchSet: 1
Gerrit-Owner: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org>
Gerrit-MessageType: newchange