Furquan Shaikh 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 7:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/jsl_memcfg_init.h:
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 70: struct spd_info spd[NUM_DIMM_SLOT]; Does JSL support different types of memories for each slot? If not, mainboard can just pass in SPD information which can be used to populate all slots. Please see how this is done on TGL.
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 103: uint16_t rcomp_resistor[3]; On TGL, we were told that rcomp_resistor and rcomp_target need to be set to auto configurable by MRC rather than having to pass those in by mainboard unless the board deviates from design guidelines. Is this true for JSL as well? Some details captured here: b/142650263
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 116: dq_pins_interleaved Does JSL with LPDDR4 allow interleaved memory configuration? I know with TGL this was not a possibility as per PDG and so interleaved config was not exposed to mainboard.
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/inc... PS7, Line 136: void jasperlake_memcfg_init(FSP_M_CONFIG *mem_cfg, : const struct jsl_mb_cfg *jsl_cfg); Quick comment: Please take a look at TGL memcfg_init. We intentionally split mb_cfg to separate out mainboard configuration from SPD data and half populated info. It makes it easier to have static const structures in mainboard to pass into memcfg_init().
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/jsl... File src/soc/intel/tigerlake/jsl_memcfg_init.c:
https://review.coreboot.org/c/coreboot/+/39135/7/src/soc/intel/tigerlake/jsl... PS7, Line 136: for (int i = 0; i < NUM_DIMM_SLOT; i++) { I think some of these can be simplified based on what the memory controller really supports. Please see TGL as reference.