Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39135 )
Change subject: src/soc/tigerlake: Add memory configuration support for Jasper Lake ......................................................................
Patch Set 18:
(2 comments)
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... File src/soc/intel/tigerlake/include/soc/meminit_jsl.h:
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 132: )
What if only 2 of the 4 LPDDR4x memory components/slots are populated. […]
Hi Karthik, I understood what you are referring. let me try to explain my understanding. Typically on chrome design we have seen LPDDRx DIMMs are connected at CHO, Slot_0 and CH1, Slot_0. Hence Meera has updated SPD here accordingly
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/me...
So far i haven't seen any alternative to this assumptions.
but your point is valid, what is some variant design of "dedede" baseboard has DIMM connected at CH0, Slot 1 and CH1, Slot 1 then above logic will fail and system will stuck.
In my opinion, we should have below logic to break all assumption and there is no harm of assigning SPD pointer to all FSP UPD. MRC logic is that intelligent to first understand the DIMM present then does the training.
/* Channel 0 */ mem_cfg->MemorySpdPtr00 = spd_data_ptr; mem_cfg->MemorySpdPtr01 = spd_data_ptr;
/* Channel 1 */ mem_cfg->MemorySpdPtr10 = spd_data_ptr; mem_cfg->MemorySpdPtr11 = spd_data_ptr;
Do you agree?
https://review.coreboot.org/c/coreboot/+/39135/14/src/soc/intel/tigerlake/in... PS14, Line 132: spd can you please change this to "spd_info" for better readability ?