Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62294 )
Change subject: mb, soc: Add the SPD_CACHE_ENABLE ......................................................................
mb, soc: Add the SPD_CACHE_ENABLE
In order to cache the spd data which reads from the memory module, we add SPD_CACHE_ENABLE option to enable the cache for the spd data. If this option is enabled, the RW_SPD_CACHE region needs to be added to the flash layout for caching the data. Since the user may remove the memory module after the bios caching the data, we need to add the invalidate flag to invalidate the mrc cache. Otherwise, the bios will use the mrc cache and can make the device malfunction.
BUG=b:200243989 BRANCH=firmware-brya-14505.B TEST=build pass and enable this feature to the brask the device could speed up around 150ms with this feature.
Change-Id: If7625a00c865dc268e2a22efd71b34b40c40877b Signed-off-by: Zhuohao Lee zhuohao@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/62294 Reviewed-by: EricR Lai ericr_lai@compal.corp-partner.google.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/google/brya/romstage.c M src/mainboard/intel/adlrvp/romstage_fsp_params.c M src/mainboard/intel/shadowmountain/romstage.c M src/mainboard/prodrive/atlas/romstage_fsp_params.c M src/soc/intel/alderlake/include/soc/meminit.h M src/soc/intel/alderlake/meminit.c M src/soc/intel/common/block/include/intelblocks/meminit.h M src/soc/intel/common/block/memory/Kconfig M src/soc/intel/common/block/memory/meminit.c M src/soc/intel/tigerlake/meminit.c 10 files changed, 80 insertions(+), 14 deletions(-)
Approvals: build bot (Jenkins): Verified EricR Lai: Looks good to me, approved
diff --git a/src/mainboard/google/brya/romstage.c b/src/mainboard/google/brya/romstage.c index ae47167..a00cf32 100644 --- a/src/mainboard/google/brya/romstage.c +++ b/src/mainboard/google/brya/romstage.c @@ -13,6 +13,7 @@ const struct mb_cfg *mem_config = variant_memory_params(); bool half_populated = variant_is_half_populated(); struct mem_spd spd_info; + bool dimms_changed = false;
memset(&spd_info, 0, sizeof(spd_info)); variant_get_spd_info(&spd_info); @@ -20,7 +21,11 @@ const struct pad_config *pads; size_t pads_num;
- memcfg_init(m_cfg, mem_config, &spd_info, half_populated); + memcfg_init(m_cfg, mem_config, &spd_info, half_populated, &dimms_changed); + if (dimms_changed) { + memupd->FspmArchUpd.NvsBufferPtr = 0; + memupd->FspmArchUpd.BootMode = FSP_BOOT_WITH_FULL_CONFIGURATION; + }
pads = variant_romstage_gpio_table(&pads_num); gpio_configure_pads(pads, pads_num); diff --git a/src/mainboard/intel/adlrvp/romstage_fsp_params.c b/src/mainboard/intel/adlrvp/romstage_fsp_params.c index a0453ef..5b5cc6b 100644 --- a/src/mainboard/intel/adlrvp/romstage_fsp_params.c +++ b/src/mainboard/intel/adlrvp/romstage_fsp_params.c @@ -48,6 +48,7 @@ const struct mb_cfg *mem_config = variant_memory_params(); int board_id = get_board_id(); const bool half_populated = false; + bool dimms_changed = false;
const struct mem_spd memory_down_spd_info = { .topo = MEM_TOPO_MEMORY_DOWN, @@ -72,7 +73,8 @@ case ADL_P_DDR4_1: case ADL_P_DDR4_2: case ADL_P_DDR5_1: - memcfg_init(m_cfg, mem_config, &dimm_module_spd_info, half_populated); + memcfg_init(m_cfg, mem_config, &dimm_module_spd_info, half_populated, + &dimms_changed); break; case ADL_P_DDR5_2: case ADL_P_LP4_1: @@ -82,7 +84,8 @@ case ADL_M_LP4: case ADL_M_LP5: case ADL_N_LP5: - memcfg_init(m_cfg, mem_config, &memory_down_spd_info, half_populated); + memcfg_init(m_cfg, mem_config, &memory_down_spd_info, half_populated, + &dimms_changed); break; default: die("Unknown board id = 0x%x\n", board_id); diff --git a/src/mainboard/intel/shadowmountain/romstage.c b/src/mainboard/intel/shadowmountain/romstage.c index 48c20db..0951936 100644 --- a/src/mainboard/intel/shadowmountain/romstage.c +++ b/src/mainboard/intel/shadowmountain/romstage.c @@ -12,11 +12,12 @@ FSP_M_CONFIG *m_cfg = &memupd->FspmConfig; const struct mb_cfg *mem_config = variant_memory_params(); const bool half_populated = false; + bool dimms_changed = false;
const struct mem_spd lp5_spd_info = { .topo = MEM_TOPO_MEMORY_DOWN, .cbfs_index = variant_memory_sku(), };
- memcfg_init(m_cfg, mem_config, &lp5_spd_info, half_populated); + memcfg_init(m_cfg, mem_config, &lp5_spd_info, half_populated, &dimms_changed); } diff --git a/src/mainboard/prodrive/atlas/romstage_fsp_params.c b/src/mainboard/prodrive/atlas/romstage_fsp_params.c index 837a528..9728a99 100644 --- a/src/mainboard/prodrive/atlas/romstage_fsp_params.c +++ b/src/mainboard/prodrive/atlas/romstage_fsp_params.c @@ -32,6 +32,7 @@ FSP_M_CONFIG *m_cfg = &memupd->FspmConfig; const struct mb_cfg *mem_config = &ddr5_mem_config; const bool half_populated = false; + bool dimms_changed = false;
const struct mem_spd dimm_module_spd_info = { .topo = MEM_TOPO_DIMM_MODULE, @@ -47,5 +48,5 @@ }, };
- memcfg_init(m_cfg, mem_config, &dimm_module_spd_info, half_populated); + memcfg_init(m_cfg, mem_config, &dimm_module_spd_info, half_populated, &dimms_changed); } diff --git a/src/soc/intel/alderlake/include/soc/meminit.h b/src/soc/intel/alderlake/include/soc/meminit.h index debdef2..98482e3 100644 --- a/src/soc/intel/alderlake/include/soc/meminit.h +++ b/src/soc/intel/alderlake/include/soc/meminit.h @@ -110,6 +110,6 @@ };
void memcfg_init(FSP_M_CONFIG *mem_cfg, const struct mb_cfg *mb_cfg, - const struct mem_spd *spd_info, bool half_populated); + const struct mem_spd *spd_info, bool half_populated, bool *dimms_changed);
#endif /* _SOC_ALDERLAKE_MEMINIT_H_ */ diff --git a/src/soc/intel/alderlake/meminit.c b/src/soc/intel/alderlake/meminit.c index 9c1f667..8b54cb1 100644 --- a/src/soc/intel/alderlake/meminit.c +++ b/src/soc/intel/alderlake/meminit.c @@ -236,7 +236,7 @@ }
void memcfg_init(FSP_M_CONFIG *mem_cfg, const struct mb_cfg *mb_cfg, - const struct mem_spd *spd_info, bool half_populated) + const struct mem_spd *spd_info, bool half_populated, bool *dimms_changed) { struct mem_channel_data data; bool dq_dqs_auto_detect = false; @@ -278,7 +278,8 @@ die("Unsupported memory type(%d)\n", mb_cfg->type); }
- mem_populate_channel_data(&soc_mem_cfg[mb_cfg->type], spd_info, half_populated, &data); + mem_populate_channel_data(&soc_mem_cfg[mb_cfg->type], spd_info, half_populated, &data, + dimms_changed); mem_init_spd_upds(mem_cfg, &data); mem_init_dq_upds(mem_cfg, &data, mb_cfg, dq_dqs_auto_detect); mem_init_dqs_upds(mem_cfg, &data, mb_cfg, dq_dqs_auto_detect); diff --git a/src/soc/intel/common/block/include/intelblocks/meminit.h b/src/soc/intel/common/block/include/intelblocks/meminit.h index 1fed02f..130249c 100644 --- a/src/soc/intel/common/block/include/intelblocks/meminit.h +++ b/src/soc/intel/common/block/include/intelblocks/meminit.h @@ -134,11 +134,13 @@ * the mainboard. * spd_info : Information about the memory topology. * half_populated: Hint from mainboard if channels are half populated. + * dimms_changed: True if the dimms is changed after caching the spd data. */ void mem_populate_channel_data(const struct soc_mem_cfg *soc_mem_cfg, const struct mem_spd *spd_info, bool half_populated, - struct mem_channel_data *data); + struct mem_channel_data *data, + bool *dimms_changed);
/* * Given a channel number and the maximum number of supported channels, this diff --git a/src/soc/intel/common/block/memory/Kconfig b/src/soc/intel/common/block/memory/Kconfig index fdcccbf..7156be7 100644 --- a/src/soc/intel/common/block/memory/Kconfig +++ b/src/soc/intel/common/block/memory/Kconfig @@ -28,4 +28,12 @@ support is expected to set MRC_CHANNEL_WIDTH as per the FSP MRC expectation.
+config SPD_CACHE_ENABLE + bool + default n + help + Enable to cache the spd data to the RW_SPD_CACHE region. If + this option is enabled, please make sure the RW_SPD_CACHE + region is added to the flash layout. + endif diff --git a/src/soc/intel/common/block/memory/meminit.c b/src/soc/intel/common/block/memory/meminit.c index 78dc2d4..89cd4e3 100644 --- a/src/soc/intel/common/block/memory/meminit.c +++ b/src/soc/intel/common/block/memory/meminit.c @@ -5,6 +5,7 @@ #include <intelblocks/meminit.h> #include <commonlib/region.h> #include <spd_bin.h> +#include <spd_cache.h> #include <string.h> #include <types.h>
@@ -96,7 +97,7 @@
static bool read_spd_dimm(const struct soc_mem_cfg *soc_mem_cfg, const struct mem_spd *info, bool half_populated, struct mem_channel_data *channel_data, - size_t *spd_len) + size_t *spd_len, bool *dimms_changed) { size_t ch, dimm; struct spd_block blk = { 0 }; @@ -108,6 +109,7 @@ * channel is marked as populated. */ uint32_t pop_mask = 0; + *dimms_changed = false;
if (!(info->topo & MEM_TOPO_DIMM_MODULE)) return false; @@ -119,7 +121,46 @@ } }
- get_spd_smbus(&blk); + if (CONFIG(SPD_CACHE_ENABLE)) { + uint8_t *spd_cache; + size_t spd_cache_sz; + bool need_update_cache = false; + bool dimm_changed = true; + + /* load spd cache from RW_SPD_CACHE */ + if (load_spd_cache(&spd_cache, &spd_cache_sz) == CB_SUCCESS) { + if (!spd_cache_is_valid(spd_cache, spd_cache_sz)) { + printk(BIOS_WARNING, "Invalid SPD cache\n"); + } else { + dimm_changed = check_if_dimm_changed(spd_cache, &blk); + if (dimm_changed) { + /* + * Set flag to indicate that the + * mrc_cache need to be invalidated + */ + printk(BIOS_INFO, + "DIMM change, invalidate cache.\n"); + *dimms_changed = true; + } + } + need_update_cache = true; + } + + if (!dimm_changed) { + printk(BIOS_INFO, "Use the SPD cache data\n"); + spd_fill_from_cache(spd_cache, &blk); + } else { + /* Access memory info through SMBUS. */ + get_spd_smbus(&blk); + + if (need_update_cache && + update_spd_cache(&blk) == CB_ERR) + printk(BIOS_ERR, "update SPD cache failed\n"); + } + } else { + get_spd_smbus(&blk); + } + *spd_len = blk.len;
for (ch = 0; ch < num_phys_ch; ch++) { @@ -145,7 +186,8 @@ void mem_populate_channel_data(const struct soc_mem_cfg *soc_mem_cfg, const struct mem_spd *spd_info, bool half_populated, - struct mem_channel_data *data) + struct mem_channel_data *data, + bool *dimms_changed) { size_t spd_md_len = 0, spd_dimm_len = 0; bool have_dimms; @@ -153,7 +195,8 @@ memset(data, 0, sizeof(*data));
read_spd_md(soc_mem_cfg, spd_info, half_populated, data, &spd_md_len); - have_dimms = read_spd_dimm(soc_mem_cfg, spd_info, half_populated, data, &spd_dimm_len); + have_dimms = read_spd_dimm(soc_mem_cfg, spd_info, half_populated, data, + &spd_dimm_len, dimms_changed);
if (data->ch_population_flags == NO_CHANNEL_POPULATED) die("No channels are populated. Incorrect memory configuration!\n"); diff --git a/src/soc/intel/tigerlake/meminit.c b/src/soc/intel/tigerlake/meminit.c index ce6316e..e8c7b68 100644 --- a/src/soc/intel/tigerlake/meminit.c +++ b/src/soc/intel/tigerlake/meminit.c @@ -151,11 +151,13 @@ const struct mem_spd *spd_info, bool half_populated) { struct mem_channel_data data; + bool dimms_changed = false;
if (mb_cfg->type >= ARRAY_SIZE(soc_mem_cfg)) die("Invalid memory type(%x)!\n", mb_cfg->type);
- mem_populate_channel_data(&soc_mem_cfg[mb_cfg->type], spd_info, half_populated, &data); + mem_populate_channel_data(&soc_mem_cfg[mb_cfg->type], spd_info, half_populated, &data, + &dimms_changed); mem_init_spd_upds(mem_cfg, &data); mem_init_dq_upds(mem_cfg, &data, mb_cfg); mem_init_dqs_upds(mem_cfg, &data, mb_cfg);
-
Felix Held (Code Review)