Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46480 )
Change subject: vc/intel/FSP2_0/CPX-SP: add FSPM DIMM watermark option ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46480/1/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/vpd.h:
https://review.coreboot.org/c/coreboot/+/46480/1/src/mainboard/ocp/deltalake... PS1, Line 37: #define FSPM_MEMREFRESHWATERMARK_DEFAULT 1 /* Default value when the VPD variable is not found */
I'd just drop the comment
makes sense
https://review.coreboot.org/c/coreboot/+/46480/1/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
PS1:
I'd say the changes to this UPD break the ABI when using the Intel CPX-SP FSP binary
Yes. When using the Intel (to-be-released) CPX-SP FSP binary, CONFIG_HAVE_FSPM_DIMM_WATERMARK_OPTION can not be set; on the other hand, it has to be set when using FB internal CPX-SP FSP binary.