EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40645 )
Change subject: src/lib: Enable SPD_READ_BY_WORD by default ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 1:
Patch Set 1:
Read by word takes 68,613 and Read by byte takes 116,784.
Ratio looks about right, the amount of clocks required for 256 bytes of SPD is approximately:
256 x 4 x 9 bits = 9216 for byte-read 128 x 5 x 9 bits = 5760 for word-read 16 * (3 x 9 + 16 * 9) = 2736 16-byte block read
Looks like implementation of i2c_eeprom_read() does not have a strict block size restriction, although longer than 32 bytes might violate something in the SMBus timing specs. There is potential to half the time still from SPD_READ_BY_WORD.
Some older raminit match the SPD serial number and CRC against previous boots, to detect if DIMMs have been swapped. IMHO, the ability to skip SPD EEPROM reads when there are cached memory training results available would be a more valuable feature than being able to access SPD fast.
Intel posted some patches to add an SPD cache as well. I believe it reads the SPD serial number only and if it matches what's in the cache, then it skips the rest of the SPD read (see patch train CB:40414). Good to see you back Kyosti 😊
We can add into TGL/CML later. But this still can help some, right?