Varun Joshi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39847 )
Change subject: soc/intel/tigerlake: Support to initialize Memory ......................................................................
Patch Set 10:
(7 comments)
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/meminit_tgl.h:
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/inc... PS5, Line 45: *
nit: space before comment end
Done
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/inc... PS5, Line 64: */
same here
Done
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/inc... PS5, Line 64: */
same here
Done
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... File src/soc/intel/tigerlake/meminit_tgl.c:
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... PS5, Line 22: #define MEM_INIT_CH_DQ_DQS_MAP(_mem_cfg, _b_cfg, _ch) \ : do { \ : memcpy(&_mem_cfg->DqMapCpu2DramCh ## _ch, \ : &_b_cfg->dq_map[_ch], \ : sizeof(_b_cfg->dq_map[_ch])); \ : memcpy(&_mem_cfg->DqsMapCpu2DramCh ## _ch, \ : &_b_cfg->dqs_map[_ch], \ : sizeof(_b_cfg->dqs_map[_ch])); \ : } while (0)
Using #define, Because we have fspm named "DqMapCpu2DramCh0[16], DqMapCpu2DramCh1[16] ... […]
Done
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... PS5, Line 89: MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 0); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 1); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 2); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 3); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 4); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 5); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 6); : MEM_INIT_CH_DQ_DQS_MAP(mem_cfg, board_cfg, 7
I don't think this is completely correct. DDR4 only supports 2 channels.
Ack
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... PS5, Line 179: meminit_ddr4x_sodimm
Deltan use CH0 and CH1. No MD in this project. But just one dimm for each channel.
Ack
https://review.coreboot.org/c/coreboot/+/39847/5/src/soc/intel/tigerlake/mem... PS5, Line 193: SpdAddressTable
You can make use of get_spd_smbus() [Reference: https://review.coreboot.org/cgit/coreboot. […]
Done