Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47899 )
Change subject: mb/google/hatch: Drop use of SPD cache for puff-based variants ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@11 PS1, Line 11: it's actually slower than simply letting FSP (vs coreboot) : read the SPD data via smbus, so drop it.
I'll have to retest swapping SODIMMs here on my Librem CML board and make sure MRC is invalidated/RA […]
It would also be good to test just removing a DIMM or adding a DIMM (i.e. not swapping with a different SODIMM) to ensure memory training is triggered. The referenced bug on the original change talks about adding/removing not triggering a retrain.
Jamie/Edward - can you please comment on this too since you have debugged/tested this?
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@19 PS1, Line 19: Test: build/boot WYVERN variant, check boot times via cbmem: : w/SPD caching: ~722 ms : w/FSP reading: ~627 ms
a good chunk is FSP-M: […]
Actually, it looks like the difference is before calling FSP-M? So, the ~120ms is spent doing something with the SPD cache in coreboot rather than in FSP-M?