Jamie Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39140 )
Change subject: mb/google/puff: enable retraining memory support ......................................................................
mb/google/puff: enable retraining memory support
Add chromeos-puff.fmd for Puff and insert a region for spd cache. Implement mainboard_prepare_dimm_info and is_the_same_memory_dimm.
BUG=b:146457985 BRANCH=None TEST=1. build puff and boot up OS 2. power off and swap DDR4 memory dimm 3. power up and check log of coreboot to confirm that device re-train the DDR4 memory.
Change-Id: I4eff8bc546c2728ff2de875e0e05da446f73c704 Signed-off-by: Jamie Chen jamie.chen@intel.com --- M src/include/spd_bin.h M src/mainboard/google/hatch/Kconfig M src/mainboard/google/hatch/Kconfig.name A src/mainboard/google/hatch/chromeos-puff.fmd M src/mainboard/google/hatch/romstage_spd_smbus.c 5 files changed, 109 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/39140/1
diff --git a/src/include/spd_bin.h b/src/include/spd_bin.h index f144b14..cf6c723 100644 --- a/src/include/spd_bin.h +++ b/src/include/spd_bin.h @@ -40,6 +40,8 @@ #define LPDDR3_SPD_PART_LEN 18 #define DDR4_SPD_PART_OFF 329 #define DDR4_SPD_PART_LEN 20 +#define DDR4_SPD_SN_OFF 325 +#define DDR4_SPD_SN_LEN 4 #define LPDDR4_SPD_PART_OFF 329 #define LPDDR4_SPD_PART_LEN 20
diff --git a/src/mainboard/google/hatch/Kconfig b/src/mainboard/google/hatch/Kconfig index 783ec73..8cd4d8f 100644 --- a/src/mainboard/google/hatch/Kconfig +++ b/src/mainboard/google/hatch/Kconfig @@ -76,6 +76,7 @@
config FMDFILE string + default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/chromeos-puff.fmd" if BOARD_GOOGLE_PUFF default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/chromeos-16MiB.fmd" if BOARD_ROMSIZE_KB_16384 default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/chromeos.fmd" if BOARD_ROMSIZE_KB_32768
diff --git a/src/mainboard/google/hatch/Kconfig.name b/src/mainboard/google/hatch/Kconfig.name index 2427b12..6e973fa 100644 --- a/src/mainboard/google/hatch/Kconfig.name +++ b/src/mainboard/google/hatch/Kconfig.name @@ -49,6 +49,7 @@ select BOARD_ROMSIZE_KB_32768 select ROMSTAGE_SPD_SMBUS select SPD_READ_BY_WORD + select MRC_SETTINGS_SPD_DATA
config BOARD_GOOGLE_HELIOS_DISKSWAP bool "-> Helios_Diskswap" diff --git a/src/mainboard/google/hatch/chromeos-puff.fmd b/src/mainboard/google/hatch/chromeos-puff.fmd new file mode 100644 index 0000000..30ff68f --- /dev/null +++ b/src/mainboard/google/hatch/chromeos-puff.fmd @@ -0,0 +1,48 @@ +FLASH@0xfe000000 0x2000000 { + SI_ALL@0x0 0x400000 { + SI_DESC@0x0 0x1000 + SI_ME@0x1000 0x3ff000 + } + SI_BIOS@0x400000 0x1c00000 { + # Place RW_LEGACY at the start of BIOS region such that the rest + # of BIOS regions start at 16MiB boundary. Since this is a 32MiB + # SPI flash only the top 16MiB actually gets memory mapped. + RW_LEGACY(CBFS)@0x0 0x1000000 + RW_SECTION_A@0x1000000 0x3e0000 { + VBLOCK_A@0x0 0x10000 + FW_MAIN_A(CBFS)@0x10000 0x3cffc0 + RW_FWID_A@0x3dffc0 0x40 + } + RW_SECTION_B@0x13e0000 0x3e0000 { + VBLOCK_B@0x0 0x10000 + FW_MAIN_B(CBFS)@0x10000 0x3cffc0 + RW_FWID_B@0x3dffc0 0x40 + } + RW_MISC@0x17c0000 0x40000 { + UNIFIED_MRC_CACHE(PRESERVE)@0x0 0x30000 { + RECOVERY_MRC_CACHE@0x0 0x10000 + RW_MRC_CACHE@0x10000 0x1F800 + RW_SPD_MRC_CACHE @0x2F800 0x800 + } + RW_ELOG(PRESERVE)@0x30000 0x4000 + RW_SHARED@0x34000 0x4000 { + SHARED_DATA@0x0 0x2000 + VBLOCK_DEV@0x2000 0x2000 + } + RW_VPD(PRESERVE)@0x38000 0x2000 + RW_NVRAM(PRESERVE)@0x3a000 0x6000 + } + # Make WP_RO region align with SPI vendor + # memory protected range specification. + WP_RO@0x1800000 0x400000 { + RO_VPD(PRESERVE)@0x0 0x4000 + RO_SECTION@0x4000 0x3fc000 { + FMAP@0x0 0x800 + RO_FRID@0x800 0x40 + RO_FRID_PAD@0x840 0x7c0 + GBB@0x1000 0x3000 + COREBOOT(CBFS)@0x4000 0x3f8000 + } + } + } +} diff --git a/src/mainboard/google/hatch/romstage_spd_smbus.c b/src/mainboard/google/hatch/romstage_spd_smbus.c index 9073744..b76b450 100644 --- a/src/mainboard/google/hatch/romstage_spd_smbus.c +++ b/src/mainboard/google/hatch/romstage_spd_smbus.c @@ -14,6 +14,7 @@ */
#include <baseboard/variants.h> +#include <fsp/api.h> #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h> #include <spd_bin.h> @@ -49,3 +50,59 @@
cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); } + +struct dimm_info_str { + char part[DDR4_SPD_PART_LEN]; + uint8_t serial_no[DDR4_SPD_SN_LEN]; +}; + +void mainboard_prepare_dimm_info(FSPM_UPD *upd, void* data, size_t *data_sz) +{ + uint8_t *spd1_ptr, *spd2_ptr; + struct dimm_info_str *dimm_data = data; + + memset(dimm_data, 0, sizeof(struct dimm_info_str) * 2); + + spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; + spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; + + if (spd1_ptr != NULL) { + memcpy(&dimm_data[0].part, spd1_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN); + memcpy(&dimm_data[0].serial_no, spd1_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); + } + + if (spd2_ptr != NULL) { + memcpy(&dimm_data[1].part, spd2_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN); + memcpy(&dimm_data[1].serial_no, spd2_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); + } + + *data_sz = sizeof(struct dimm_info_str) * 2; +} + +bool is_the_same_memory_dimm(FSPM_UPD *upd, const void *data, size_t *data_sz) +{ + uint8_t *spd1_ptr, *spd2_ptr; + const struct dimm_info_str *dimm_data = data; + + spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; + spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; + + if (spd1_ptr == NULL) { + if (strlen(dimm_data[0].part) != 0) + return FALSE; + } else { + if ((memcmp(&dimm_data[0].part, spd1_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN) != 0) && + (memcmp(&dimm_data[0].serial_no, spd1_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN) != 0)) + return FALSE; + } + + if (spd2_ptr == NULL) { + if (strlen(dimm_data[1].part) != 0) + return FALSE; + } else { + if ((memcmp(&dimm_data[1].part, spd2_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN) != 0) && + (memcmp(&dimm_data[1].serial_no, spd2_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN) != 0)) + return FALSE; + } + return TRUE; +}
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39140 )
Change subject: mb/google/puff: enable retraining memory support ......................................................................
Patch Set 1:
(27 comments)
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... File src/mainboard/google/hatch/romstage_spd_smbus.c:
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 59: void mainboard_prepare_dimm_info(FSPM_UPD *upd, void* data, size_t *data_sz) "foo* bar" should be "foo *bar"
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 66: spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 66: spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 66: spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 67: spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 67: spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 67: spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 69: if (spd1_ptr != NULL) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 69: if (spd1_ptr != NULL) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 71: memcpy(&dimm_data[0].serial_no, spd1_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 71: memcpy(&dimm_data[0].serial_no, spd1_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 72: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 72: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 74: if (spd2_ptr != NULL) { code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 74: if (spd2_ptr != NULL) { please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 75: memcpy(&dimm_data[1].part, spd2_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 75: memcpy(&dimm_data[1].part, spd2_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 76: memcpy(&dimm_data[1].serial_no, spd2_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 76: memcpy(&dimm_data[1].serial_no, spd2_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN); please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 77: } code indent should use tabs where possible
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 77: } please, no spaces at the start of a line
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 87: spd1_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr00; "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 88: spd2_ptr = (uint8_t*) upd->FspmConfig.MemorySpdPtr10; "(foo*)" should be "(foo *)"
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 94: if ((memcmp(&dimm_data[0].part, spd1_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN) != 0) && line over 96 characters
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 95: (memcmp(&dimm_data[0].serial_no, spd1_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN) != 0)) line over 96 characters
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 103: if ((memcmp(&dimm_data[1].part, spd2_ptr + DDR4_SPD_PART_OFF, DDR4_SPD_PART_LEN) != 0) && line over 96 characters
https://review.coreboot.org/c/coreboot/+/39140/1/src/mainboard/google/hatch/... PS1, Line 104: (memcmp(&dimm_data[1].serial_no, spd2_ptr + DDR4_SPD_SN_OFF, DDR4_SPD_SN_LEN) != 0)) line over 96 characters
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39140 )
Change subject: mb/google/puff: enable retraining memory support ......................................................................
Patch Set 1:
Comparing SPD's is not mainboard specific at all. I suggest putting those in ddr4 specific and FSP (for UPD pointers) helper functions.
Jamie Chen has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39140 )
Change subject: mb/google/puff: enable retraining memory support ......................................................................
Abandoned
-
Arthur Heymans (Code Review) -
build bot (Jenkins) (Code Review)
-
Jamie Chen (Code Review)