Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44429 )
Change subject: util: Add spd_tools to generate DDR4 SPDs for TGL boards ......................................................................
Patch Set 26: Code-Review+1
(5 comments)
https://review.coreboot.org/c/coreboot/+/44429/18/util/spd_tools/intel/ddr4/... File util/spd_tools/intel/ddr4/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/44429/18/util/spd_tools/intel/ddr4/... PS18, Line 513: 4284
tRRD_S(1K) = Max(4nCK,4.2ns), where CK=1.071, so max = 4 * 1.071 = 4. […]
Ah okay. But, what if Tck is provided in JSON file? Shouldnt 4 * tCK be calculated by the tool to decide whether to use that or the fixed value provided in the spec?
Basically something like:
speedToTRRDSMinPsOneKPageSize = map[int]int { 1600: 5000, 1866: 4200, 2133: 3700, 2400: 3300, 2666: 3000, 2933: 2700, 3200: 2500 }
speedToTRRDSMinPsTwoKPageSize = map[int]int { 1600: 6000, 1866: 5300, 2133: 5300, 2400: 5300, 2666: 5300, 2933: 5300, 3200: 5300 }
func getTRRDSMinPs(memAttribs *memAttributes) int { var tRRDFixed int var tRRDFromTck int
switch pageSizefromBusWidthEncoding[memAttribs.DeviceBusWidth] { case 1: tRRDFixed = speedToTRRDSMinPsOneKPageSize[memAttribs.SpeedMTps] case 2: tRRDFixed = speedToTRRDSMinPsTwoKPageSize[memAttribs.SpeedMTps] default: tRRDFixed = 0 }
tRRDFromTck = 4 * memAttribs.tckMinPs
if tRRDFixed < tRRDFromTck { return tRRDFromTck } else { return tRRDFixed } }
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... File util/spd_tools/intel/ddr4/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... PS26, Line 265: case 4: : width = 0 This is not really supported, right?
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... PS26, Line 530: nit: extra blank line not required.
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... PS26, Line 537: nit: extra blank line not required.
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... File util/spd_tools/intel/ddr4/global_ddr4_mem_parts.json.txt:
https://review.coreboot.org/c/coreboot/+/44429/26/util/spd_tools/intel/ddr4/... PS26, Line 12: 9 10 11 12 13 14 15 16 17 18 19 20 Please see comments on patchset 18 and 11.