Philip Chen has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32513
Change subject: soc/intel/cannonlake: Clean up cannonlake_memcfg_init ......................................................................
soc/intel/cannonlake: Clean up cannonlake_memcfg_init
The major changes include: (1) Add enum 'mem_info_read_type' to spd_info. (2) Add spd_into into cnl_mb_cfg. (3) Squash meminit_memcfg_spd().
BUG=b:124990009 TEST=build
Change-Id: I686a85996858204c20fd05ef24787a0487817c34 --- M src/mainboard/google/hatch/romstage.c M src/mainboard/google/hatch/variants/baseboard/memory.c M src/mainboard/google/sarien/romstage.c M src/mainboard/intel/coffeelake_rvp/memory.c M src/mainboard/intel/coffeelake_rvp/romstage.c M src/soc/intel/cannonlake/cnl_memcfg_init.c M src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h 7 files changed, 90 insertions(+), 84 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/32513/1
diff --git a/src/mainboard/google/hatch/romstage.c b/src/mainboard/google/hatch/romstage.c index bdf951b..95db576 100644 --- a/src/mainboard/google/hatch/romstage.c +++ b/src/mainboard/google/hatch/romstage.c @@ -25,14 +25,8 @@ { struct cnl_mb_cfg memcfg;
- const struct spd_info spd = { - .spd_by_index = true, - .spd_spec.spd_index = variant_memory_sku(), - }; - variant_memory_params(&memcfg); - cannonlake_memcfg_init(&memupd->FspmConfig, - &memcfg, &spd); + cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); }
void mainboard_get_dram_part_num(const char **part_num, size_t *len) diff --git a/src/mainboard/google/hatch/variants/baseboard/memory.c b/src/mainboard/google/hatch/variants/baseboard/memory.c index b6a6615..8706332 100644 --- a/src/mainboard/google/hatch/variants/baseboard/memory.c +++ b/src/mainboard/google/hatch/variants/baseboard/memory.c @@ -20,6 +20,9 @@ #include <string.h>
static const struct cnl_mb_cfg baseboard_memcfg = { + .spd = { + .read_type = READ_SPD_CBFS}, + /* * The dqs_map arrays map the ddr4 pins to the SoC pins * for both channels. @@ -27,14 +30,14 @@ * the index = pin number on ddr4 part * the value = pin number on SoC */ - .dqs_map[DDR_CH0] = { 0, 1, 4, 5, 2, 3, 6, 7 }, - .dqs_map[DDR_CH1] = { 0, 1, 4, 5, 2, 3, 6, 7 }, + .dqs_map[DDR_CH0] = {0, 1, 4, 5, 2, 3, 6, 7}, + .dqs_map[DDR_CH1] = {0, 1, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */ - .rcomp_resistor = { 121, 81, 100 }, + .rcomp_resistor = {121, 81, 100},
/* Baseboard Rcomp target values */ - .rcomp_targets = { 100, 40, 20, 20, 26 }, + .rcomp_targets = {100, 40, 20, 20, 26},
/* Set CaVref config to 2 */ .vref_ca_config = 2, @@ -46,6 +49,7 @@ void __weak variant_memory_params(struct cnl_mb_cfg *bcfg) { memcpy(bcfg, &baseboard_memcfg, sizeof(baseboard_memcfg)); + bcfg->spd.spd_spec.spd_index = variant_memory_sku(); /* * GPP_F2 is the MEM_CH_SEL gpio, which is set to 1 for single * channel skus and 0 for dual channel skus. diff --git a/src/mainboard/google/sarien/romstage.c b/src/mainboard/google/sarien/romstage.c index e83cd4a..b66fd70 100644 --- a/src/mainboard/google/sarien/romstage.c +++ b/src/mainboard/google/sarien/romstage.c @@ -18,6 +18,14 @@ #include <soc/romstage.h>
static const struct cnl_mb_cfg memcfg = { + .spd = { + .read_type = READ_SMBUS, + .spd_spec = { + .spd_smbus_address[0] = 0xa0, + .spd_smbus_address[2] = 0xa4, + }, + }, + /* * The dqs_map arrays map the ddr4 pins to the SoC pins * for both channels. @@ -25,16 +33,16 @@ * the index = pin number on ddr4 part * the value = pin number on SoC */ - .dqs_map[DDR_CH0] = { 0, 1, 4, 5, 2, 3, 6, 7 }, - .dqs_map[DDR_CH1] = { 0, 1, 4, 5, 2, 3, 6, 7 }, + .dqs_map[DDR_CH0] = {0, 1, 4, 5, 2, 3, 6, 7}, + .dqs_map[DDR_CH1] = {0, 1, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */ - .rcomp_resistor = { 121, 81, 100 }, + .rcomp_resistor = {121, 81, 100},
/* * Baseboard Rcomp target values. */ - .rcomp_targets = { 100, 40, 20, 20, 26 }, + .rcomp_targets = {100, 40, 20, 20, 26},
/* Disable Early Command Training */ .ect = 0, @@ -45,12 +53,7 @@
void mainboard_memory_init_params(FSPM_UPD *memupd) { - const struct spd_info spd = { - .spd_smbus_address[0] = 0xa0, - .spd_smbus_address[2] = 0xa4 - }; - wilco_ec_romstage_init();
- cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg, &spd); + cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); } diff --git a/src/mainboard/intel/coffeelake_rvp/memory.c b/src/mainboard/intel/coffeelake_rvp/memory.c index a13000c..a9695e8 100644 --- a/src/mainboard/intel/coffeelake_rvp/memory.c +++ b/src/mainboard/intel/coffeelake_rvp/memory.c @@ -20,23 +20,32 @@ #include <soc/cnl_memcfg_init.h>
static const struct cnl_mb_cfg baseboard_memcfg_cfg = { - /* + .spd = { + .read_type = READ_SMBUS, + .spd_spec = { + .spd_smbus_address[0] = 0xA0, + .spd_smbus_address[1] = 0xA2, + .spd_smbus_address[2] = 0xA4, + .spd_smbus_address[3] = 0xA6, + }, + }; +/* * The dqs_map arrays map the ddr4 pins to the SoC pins * for both channels. * * the index = pin number on ddr4 part * the value = pin number on SoC */ - .dqs_map[DDR_CH0] = { 0, 1, 3, 2, 4, 5, 6, 7 }, - .dqs_map[DDR_CH1] = { 1, 0, 4, 5, 2, 3, 6, 7 }, +.dqs_map[DDR_CH0] = {0, 1, 3, 2, 4, 5, 6, 7}, + .dqs_map[DDR_CH1] = {1, 0, 4, 5, 2, 3, 6, 7},
/* Baseboard uses 121, 81 and 100 rcomp resistors */ - .rcomp_resistor = { 121, 81, 100 }, + .rcomp_resistor = {121, 81, 100},
/* * Baseboard Rcomp target values. */ - .rcomp_targets = { 100, 40, 20, 20, 26 }, + .rcomp_targets = {100, 40, 20, 20, 26},
/* Baseboard is an interleaved design */ .dq_pins_interleaved = 1, @@ -46,7 +55,8 @@
/* Disable Early Command Training */ .ect = 0, -}; +} +;
const struct cnl_mb_cfg *__weak variant_memcfg_config(void) { diff --git a/src/mainboard/intel/coffeelake_rvp/romstage.c b/src/mainboard/intel/coffeelake_rvp/romstage.c index 1ab2d78..09ef148 100644 --- a/src/mainboard/intel/coffeelake_rvp/romstage.c +++ b/src/mainboard/intel/coffeelake_rvp/romstage.c @@ -20,13 +20,5 @@
void mainboard_memory_init_params(FSPM_UPD *memupd) { - const struct spd_info spd = { - .spd_smbus_address[0] = 0xA0, - .spd_smbus_address[1] = 0xA2, - .spd_smbus_address[2] = 0xA4, - .spd_smbus_address[3] = 0xA6, - }; - - cannonlake_memcfg_init(&memupd->FspmConfig, - variant_memcfg_config(), &spd); + cannonlake_memcfg_init(&memupd->FspmConfig, variant_memcfg_config()); } diff --git a/src/soc/intel/cannonlake/cnl_memcfg_init.c b/src/soc/intel/cannonlake/cnl_memcfg_init.c index db001b8..ad7a71c 100644 --- a/src/soc/intel/cannonlake/cnl_memcfg_init.c +++ b/src/soc/intel/cannonlake/cnl_memcfg_init.c @@ -20,7 +20,7 @@ #include <string.h>
static void meminit_memcfg(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *board_cfg) + const struct cnl_mb_cfg *board_cfg) { /* * DqByteMapChx expects 12 bytes of data, but the last 6 bytes @@ -29,29 +29,34 @@ */ memset(&mem_cfg->DqByteMapCh0, 0, sizeof(mem_cfg->DqByteMapCh0)); memcpy(&mem_cfg->DqByteMapCh0, &board_cfg->dq_map[DDR_CH0], - sizeof(board_cfg->dq_map[DDR_CH0])); + sizeof(board_cfg->dq_map[DDR_CH0]));
memset(&mem_cfg->DqByteMapCh1, 0, sizeof(mem_cfg->DqByteMapCh1)); memcpy(&mem_cfg->DqByteMapCh1, &board_cfg->dq_map[DDR_CH1], - sizeof(board_cfg->dq_map[DDR_CH1])); + sizeof(board_cfg->dq_map[DDR_CH1]));
memcpy(&mem_cfg->DqsMapCpu2DramCh0, &board_cfg->dqs_map[DDR_CH0], - sizeof(board_cfg->dqs_map[DDR_CH0])); + sizeof(board_cfg->dqs_map[DDR_CH0])); memcpy(&mem_cfg->DqsMapCpu2DramCh1, &board_cfg->dqs_map[DDR_CH1], - sizeof(board_cfg->dqs_map[DDR_CH1])); + sizeof(board_cfg->dqs_map[DDR_CH1]));
memcpy(&mem_cfg->RcompResistor, &board_cfg->rcomp_resistor, - sizeof(mem_cfg->RcompResistor)); + sizeof(mem_cfg->RcompResistor));
/* Early cannonlake requires rcomp targets to be 0 */ memcpy(&mem_cfg->RcompTarget, &board_cfg->rcomp_targets, - sizeof(mem_cfg->RcompTarget)); + sizeof(mem_cfg->RcompTarget)); }
-static void meminit_memcfg_spd(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *cnl_cfg, - size_t spd_data_len, uintptr_t spd_data_ptr) +/* + * Initialize default memory settings using spd data contained in a buffer. + */ +static void meminit_spd_data(FSP_M_CONFIG *mem_cfg, + const struct cnl_mb_cfg *cnl_cfg, + size_t spd_data_len, uintptr_t spd_data_ptr) { + assert(spd_data_ptr && spd_data_len); + mem_cfg->MemorySpdDataLen = spd_data_len;
if (cnl_cfg->channel_empty[0] == 0) @@ -62,31 +67,20 @@ }
/* - * Initialize default memory settings using spd data contained in a buffer. - */ -static void meminit_spd_data(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *cnl_cfg, - size_t spd_data_len, uintptr_t spd_data_ptr) -{ - assert(spd_data_ptr && spd_data_len); - meminit_memcfg_spd(mem_cfg, cnl_cfg, spd_data_len, spd_data_ptr); -} - -/* * Initialize default memory settings using the spd file specified by * spd_index. The spd_index is an index into the SPD_SOURCES array defined * in spd/Makefile.inc. */ static void meminit_cbfs_spd_index(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *cnl_cfg, - int spd_index) + const struct cnl_mb_cfg *cnl_cfg) { size_t spd_data_len; uintptr_t spd_data_ptr; struct region_device spd_rdev;
- printk(BIOS_DEBUG, "SPD INDEX = %d\n", spd_index); - if (get_spd_cbfs_rdev(&spd_rdev, spd_index) < 0) + printk(BIOS_DEBUG, "SPD INDEX = %d\n", + cnl_cfg->spd.spd_spec.spd_index); + if (get_spd_cbfs_rdev(&spd_rdev, cnl_cfg->spd.spd_spec.spd_index) < 0) die("spd.bin not found or incorrect index\n"); spd_data_len = region_device_sz(&spd_rdev); /* Memory leak is ok since we have memory mapped boot media */ @@ -97,10 +91,8 @@
/* Initialize onboard memory configurations for CannonLake */ void cannonlake_memcfg_init(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *cnl_cfg, - const struct spd_info *spd) + const struct cnl_mb_cfg *cnl_cfg) { - bool OnModuleSpd = false; /* Early Command Training Enabled */ mem_cfg->ECT = cnl_cfg->ect; mem_cfg->DqPinsInterleaved = cnl_cfg->dq_pins_interleaved; @@ -108,24 +100,26 @@
/* Spd pointer will only be used if all smbus slave address of memory * sockets on the platform is empty */ - for (int i = 0; i < ARRAY_SIZE(mem_cfg->SpdAddressTable); i++) { - if (spd->spd_smbus_address[i] != 0) { - mem_cfg->SpdAddressTable[i] = spd->spd_smbus_address[i]; - OnModuleSpd = true; + switch (cnl_cfg->spd.read_type) { + case READ_SMBUS: + for (int i = 0; i < ARRAY_SIZE(mem_cfg->SpdAddressTable); i++) { + if (cnl_cfg->spd.spd_spec.spd_smbus_address[i]) { + mem_cfg->SpdAddressTable[i] = cnl_cfg->spd. + spd_spec.spd_smbus_address[i]; + } } - } - - if (!OnModuleSpd) { - if (spd->spd_by_index) { - meminit_cbfs_spd_index(mem_cfg, cnl_cfg, - spd->spd_spec.spd_index); - } else { - meminit_spd_data(mem_cfg, cnl_cfg, - spd->spd_spec.spd_data_ptr_info.spd_data_len, - spd->spd_spec.spd_data_ptr_info.spd_data_ptr); - } + break; + case READ_SPD_CBFS: + meminit_cbfs_spd_index(mem_cfg, cnl_cfg); + break; + case READ_SPD_MEMPTR: + meminit_spd_data(mem_cfg, cnl_cfg, + cnl_cfg->spd.spd_spec.spd_data_ptr_info.spd_data_len, + cnl_cfg->spd.spd_spec.spd_data_ptr_info.spd_data_ptr); + break; + default: + printk(BIOS_DEBUG, "No valid way to read mem info."); }
meminit_memcfg(mem_cfg, cnl_cfg); - } diff --git a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h index c602180..42d67a5 100644 --- a/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h +++ b/src/soc/intel/cannonlake/include/soc/cnl_memcfg_init.h @@ -40,17 +40,27 @@ uintptr_t spd_data_ptr; };
+enum mem_info_read_type { + NOT_EXISTING, + READ_SMBUS, + READ_SPD_CBFS, + READ_SPD_MEMPTR +}; + struct spd_info { - bool spd_by_index; + enum mem_info_read_type read_type; union spd_data_by { int spd_index; struct spd_by_pointer spd_data_ptr_info; + uint8_t spd_smbus_address[4]; } spd_spec; - uint8_t spd_smbus_address[4]; };
/* Board-specific memory dq mapping information */ struct cnl_mb_cfg { + /* Parameters required to access DRAM info. */ + struct spd_info spd; + /* * For each channel, there are 3 sets of DQ byte mappings, * where each set has a package 0 and a package 1 value (package 0 @@ -122,7 +132,6 @@ * Initialize default memory configurations for CannonLake. */ void cannonlake_memcfg_init(FSP_M_CONFIG *mem_cfg, - const struct cnl_mb_cfg *cnl_cfg, - const struct spd_info *spd); + const struct cnl_mb_cfg *cnl_cfg);
#endif /* _SOC_CANNONLAKE_MEMCFG_INIT_H_ */