nb/x4x/raminit: Rewrite SPD decode and timing selection

This is mostly written from scratch and uses common spd ddr2 decode
functions.

This improves the following:
* This fixes incorrect CAS/Freq detection on DDR2;

* Fixes tRFC computation; tRFC == 78 is a valid timing which is
  excluded and 0 ends up being used; (TESTED)

* Timings selection does not use loops;

* Removes ddr3 spd decode and is re-added in follow-up patches using
  common ddr3 spd functions;

* Raminit would bail out if a dimm was unsupported, now in some cases it
  just marks the dimm slot as empty;

* It dramatically reduces stack usage since it does not allocate 4
  times 256 bytes to store full SPDs, amongs other unused things that
  were stored in sysinfo;

* Reports when no dimms are present;

* Uses i2c block read to read SPD which is about 5 times faster than
  bytewise read, with a fallback to smbus mode in case of failure,
  which does seem to happen when the system is forcefully powered
  off.

Change-Id: I760eeaa3bd4f2bc25a517ddb1b9533c971454071
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/19143
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>

3 files changed, 271 insertions, 362 deletions

diff --git a/src/northbridge/intel/x4x/raminit.c b/src/northbridge/intel/x4x/raminit.c
index dc64c51ffc..cd786bb266 100644
--- a/src/northbridge/intel/x4x/raminit.c
+++ b/src/northbridge/intel/x4x/raminit.c
@@ -32,150 +32,71 @@
 #include <pc80/mc146818rtc.h>
 #include <spd.h>
 #include <string.h>
+#include <device/dram/ddr2.h>
 
 static inline int spd_read_byte(unsigned int device, unsigned int address)
 {
 	return smbus_read_byte(device, address);
 }
 
-static void sdram_read_spds(struct sysinfo *s)
+struct abs_timings {
+	u32 min_tclk;
+	u32 min_tRAS;
+	u32 min_tRP;
+	u32 min_tRCD;
+	u32 min_tWR;
+	u32 min_tRFC;
+	u32 min_tWTR;
+	u32 min_tRRD;
+	u32 min_tRTP;
+	u32 min_tCLK_cas[8];
+	u32 cas_supported;
+};
+
+#define CTRL_MIN_TCLK_DDR2 TCK_400MHZ
+
+static void select_cas_dramfreq_ddr2(struct sysinfo *s,
+				const struct abs_timings *saved_timings)
 {
-	u8 i, j, chan;
-	int status = 0;
-	FOR_EACH_DIMM(i) {
-		if (s->spd_map[i] == 0) {
-			/* Non-existent SPD address */
-			s->dimms[i].card_type = RAW_CARD_UNPOPULATED;
-			continue;
-		}
-		for (j = 0; j < 64; j++) {
-			status = spd_read_byte(s->spd_map[i], j);
-			if (status < 0) {
-				/* No SPD here */
-				s->dimms[i].card_type = RAW_CARD_UNPOPULATED;
-				break;
-			}
-			s->dimms[i].spd_data[j] = (u8) status;
-			if (j == 62)
-				s->dimms[i].card_type = ((u8) status) & 0x1f;
-		}
-
-		if (status >= 0) {
-			if (IS_ENABLED(CONFIG_DEBUG_RAM_SETUP))
-				hexdump(s->dimms[i].spd_data, 64);
-		}
-	}
-
-	s->spd_type = 0;
-	int fail = 1;
-	FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-		switch ((enum ddrxspd) s->dimms[i].spd_data[2]) {
-		case DDR2SPD:
-			if (s->spd_type == 0)
-				s->spd_type = DDR2;
-			else if (s->spd_type == DDR3)
-				die("DIMM type mismatch\n");
-			break;
-		case DDR3SPD:
-		default:
-			if (s->spd_type == 0)
-				s->spd_type = DDR3;
-			else if (s->spd_type == DDR2)
-				die("DIMM type mismatch\n");
+	u8 try_cas;
+	/* Currently only these CAS are supported */
+	u8 cas_mask = SPD_CAS_LATENCY_DDR2_5 | SPD_CAS_LATENCY_DDR2_6;
+
+	cas_mask &= saved_timings->cas_supported;
+	try_cas = spd_get_msbs(cas_mask);
+
+	while (cas_mask & (1 << try_cas) && try_cas > 0) {
+		s->selected_timings.CAS = try_cas;
+		s->selected_timings.tclk = saved_timings->min_tCLK_cas[try_cas];
+		if (s->selected_timings.tclk >= CTRL_MIN_TCLK_DDR2 &&
+				saved_timings->min_tCLK_cas[try_cas] !=
+				saved_timings->min_tCLK_cas[try_cas - 1])
 			break;
-		}
+		try_cas--;
 	}
-	if (s->spd_type == DDR3) {
-		FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-			s->dimms[i].sides = (s->dimms[i].spd_data[5] & 0x0f) + 1;
-			s->dimms[i].ranks = ((s->dimms[i].spd_data[7] >> 3) & 0x7) + 1;
-			s->dimms[i].chip_capacity = (s->dimms[i].spd_data[4] & 0xf);
-			s->dimms[i].banks = 8;
-			s->dimms[i].rows = ((s->dimms[i].spd_data[5] >> 3) & 0x7) + 12;
-			s->dimms[i].cols = (s->dimms[i].spd_data[5] & 0x7) + 9;
-			s->dimms[i].cas_latencies = 0xfe;
-			s->dimms[i].cas_latencies &= (s->dimms[i].spd_data[14] << 1);
-			if (s->dimms[i].cas_latencies == 0)
-				s->dimms[i].cas_latencies = 0x40;
-			s->dimms[i].tAAmin = s->dimms[i].spd_data[16];
-			s->dimms[i].tCKmin = s->dimms[i].spd_data[12];
-			s->dimms[i].width = s->dimms[i].spd_data[7] & 0x7;
-			s->dimms[i].page_size = s->dimms[i].width * (1 << s->dimms[i].cols); // Bytes
-			s->dimms[i].tRAS = ((s->dimms[i].spd_data[21] & 0xf) << 8) |
-				s->dimms[i].spd_data[22];
-			s->dimms[i].tRP = s->dimms[i].spd_data[20];
-			s->dimms[i].tRCD = s->dimms[i].spd_data[18];
-			s->dimms[i].tWR = s->dimms[i].spd_data[17];
-			fail = 0;
-		}
-	} else if (s->spd_type == DDR2) {
-		FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-			s->dimms[i].sides = (s->dimms[i].spd_data[5] & 0x7) + 1;
-			s->dimms[i].banks = (s->dimms[i].spd_data[17] >> 2) - 1;
-			s->dimms[i].chip_capacity = s->dimms[i].banks;
-			s->dimms[i].rows = s->dimms[i].spd_data[3];// - 12;
-			s->dimms[i].cols = s->dimms[i].spd_data[4];// - 9;
-			s->dimms[i].cas_latencies = s->dimms[i].spd_data[18];
-			if (s->dimms[i].cas_latencies == 0)
-				s->dimms[i].cas_latencies = 0x60; // 6,5 CL
-			s->dimms[i].tAAmin = s->dimms[i].spd_data[26];
-			s->dimms[i].tCKmin = s->dimms[i].spd_data[25];
-			s->dimms[i].width = (s->dimms[i].spd_data[13] >> 3) - 1;
-			s->dimms[i].page_size = (s->dimms[i].width+1) * (1 << s->dimms[i].cols); // Bytes
-			s->dimms[i].tRAS = s->dimms[i].spd_data[30];
-			s->dimms[i].tRP = s->dimms[i].spd_data[27];
-			s->dimms[i].tRCD = s->dimms[i].spd_data[29];
-			s->dimms[i].tWR = s->dimms[i].spd_data[36];
-			s->dimms[i].ranks = s->dimms[i].sides; // XXX
-
-			printk(BIOS_DEBUG, "DIMM %d\n", i);
-			printk(BIOS_DEBUG, "  Sides     : %d\n", s->dimms[i].sides);
-			printk(BIOS_DEBUG, "  Banks     : %d\n", s->dimms[i].banks);
-			printk(BIOS_DEBUG, "  Ranks     : %d\n", s->dimms[i].ranks);
-			printk(BIOS_DEBUG, "  Rows      : %d\n", s->dimms[i].rows);
-			printk(BIOS_DEBUG, "  Cols      : %d\n", s->dimms[i].cols);
-			printk(BIOS_DEBUG, "  Page size : %d\n", s->dimms[i].page_size);
-			printk(BIOS_DEBUG, "  Width     : %d\n", (s->dimms[i].width+1)*8);
-			fail = 0;
-		}
-	}
-	if (fail)
-		die("No memory dimms, halt\n");
 
-	FOR_EACH_POPULATED_CHANNEL(s->dimms, chan) {
-		FOR_EACH_POPULATED_DIMM_IN_CHANNEL(s->dimms, chan, i) {
-			int dimm_config;
-			if (s->dimms[i].ranks == 1) {
-				if (s->dimms[i].width == 0)	/* x8 */
-					dimm_config = 1;
-				else				/* x16 */
-					dimm_config = 3;
-			} else {
-				if (s->dimms[i].width == 0)	/* x8 */
-					dimm_config = 2;
-				else
-					die("Dual-rank x16 not supported\n");
-			}
-			s->dimm_config[chan] |=
-				dimm_config << (i % DIMMS_PER_CHANNEL) * 2;
-		}
-		printk(BIOS_DEBUG, "  Config[CH%d] : %d\n", chan, s->dimm_config[chan]);
-	}
-}
 
-static u8 msbpos(u8 val) //Reverse
-{
-	u8 i;
-	for (i = 7; (i >= 0) && ((val & (1 << i)) == 0); i--)
-		;
-	return i;
+	if ((s->selected_timings.CAS < 3) || (s->selected_timings.tclk == 0))
+		die("Could not find common memory frequency and CAS\n");
+
+	switch (s->selected_timings.tclk) {
+	case TCK_200MHZ:
+	case TCK_266MHZ:
+		/* FIXME: this works on vendor BIOS */
+		die("Selected dram frequency not supported\n");
+	case TCK_333MHZ:
+		s->selected_timings.mem_clk = MEM_CLOCK_667MHz;
+		break;
+	case TCK_400MHZ:
+		s->selected_timings.mem_clk = MEM_CLOCK_800MHz;
+		break;
+	}
 }
 
 static void mchinfo_ddr2(struct sysinfo *s)
 {
 	const u32 eax = cpuid_ext(0x04, 0).eax;
-	s->cores = ((eax >> 26) & 0x3f) + 1;
-	printk(BIOS_WARNING, "%d CPU cores\n", s->cores);
+	printk(BIOS_WARNING, "%d CPU cores\n", ((eax >> 26) & 0x3f) + 1);
 
 	u32 capid = pci_read_config16(PCI_DEV(0, 0, 0), 0xe8);
 	if (!(capid & (1<<(79-64))))
@@ -195,21 +116,126 @@ static void mchinfo_ddr2(struct sysinfo *s)
 		printk(BIOS_WARNING, "VT-d enabled\n");
 }
 
-static void sdram_detect_ram_speed(struct sysinfo *s)
+static int ddr2_save_dimminfo(u8 dimm_idx, u8 *raw_spd,
+		struct abs_timings *saved_timings, struct sysinfo *s)
+{
+	struct dimm_attr_st decoded_dimm;
+	int i;
+
+	if (spd_decode_ddr2(&decoded_dimm, raw_spd) != SPD_STATUS_OK) {
+		printk(BIOS_DEBUG, "Problems decoding SPD\n");
+		return CB_ERR;
+	}
+
+	if (IS_ENABLED(CONFIG_DEBUG_RAM_SETUP))
+		dram_print_spd_ddr2(&decoded_dimm);
+
+	if (!(decoded_dimm.width & (0x08 | 0x10))) {
+
+		printk(BIOS_ERR,
+			"DIMM%d Unsupported width: x%d. Disabling dimm\n",
+			dimm_idx, s->dimms[dimm_idx].width);
+		return CB_ERR;
+	}
+	s->dimms[dimm_idx].width = (decoded_dimm.width >> 3) - 1;
+	/*
+	 * This boils down to:
+	 * "Except for the x16 configuration, all DDR2 devices have a
+	 * 1KB page size. For the x16 configuration, the page size is 2KB
+	 * for all densities except the 256Mb device, which has a 1KB page
+	 * size." Micron, 'TN-47-16 Designing for High-Density DDR2 Memory'
+	 */
+	s->dimms[dimm_idx].page_size = s->dimms[dimm_idx].width *
+		 (1 << decoded_dimm.col_bits);
+
+	switch (decoded_dimm.banks) {
+	case 4:
+		s->dimms[dimm_idx].n_banks = N_BANKS_4;
+		break;
+	case 8:
+		s->dimms[dimm_idx].n_banks = N_BANKS_8;
+		break;
+	default:
+		printk(BIOS_ERR,
+			"DIMM%d Unsupported #banks: x%d. Disabling dimm\n",
+			 dimm_idx, decoded_dimm.banks);
+		return CB_ERR;
+	}
+
+	s->dimms[dimm_idx].ranks = decoded_dimm.ranks;
+	s->dimms[dimm_idx].rows = decoded_dimm.row_bits;
+	s->dimms[dimm_idx].cols = decoded_dimm.col_bits;
+
+	saved_timings->cas_supported &= decoded_dimm.cas_supported;
+
+	saved_timings->min_tRAS =
+		MAX(saved_timings->min_tRAS, decoded_dimm.tRAS);
+	saved_timings->min_tRP =
+		MAX(saved_timings->min_tRP, decoded_dimm.tRP);
+	saved_timings->min_tRCD =
+		MAX(saved_timings->min_tRCD, decoded_dimm.tRCD);
+	saved_timings->min_tWR =
+		MAX(saved_timings->min_tWR, decoded_dimm.tWR);
+	saved_timings->min_tRFC =
+		MAX(saved_timings->min_tRFC, decoded_dimm.tRFC);
+	saved_timings->min_tWTR =
+		MAX(saved_timings->min_tWTR, decoded_dimm.tWTR);
+	saved_timings->min_tRRD =
+		MAX(saved_timings->min_tRRD, decoded_dimm.tRRD);
+	saved_timings->min_tRTP =
+		MAX(saved_timings->min_tRTP, decoded_dimm.tRTP);
+	for (i = 0; i < 8; i++) {
+		if (!(saved_timings->cas_supported & (1 << i)))
+			saved_timings->min_tCLK_cas[i] = 0;
+		else
+			saved_timings->min_tCLK_cas[i] =
+				MAX(saved_timings->min_tCLK_cas[i],
+					decoded_dimm.cycle_time[i]);
+	}
+	return CB_SUCCESS;
+}
+
+static void select_discrete_timings(struct sysinfo *s,
+				const struct abs_timings *timings)
+{
+	s->selected_timings.tRAS = DIV_ROUND_UP(timings->min_tRAS,
+						s->selected_timings.tclk);
+	s->selected_timings.tRP = DIV_ROUND_UP(timings->min_tRP,
+						s->selected_timings.tclk);
+	s->selected_timings.tRCD = DIV_ROUND_UP(timings->min_tRCD,
+						s->selected_timings.tclk);
+	s->selected_timings.tWR = DIV_ROUND_UP(timings->min_tWR,
+						s->selected_timings.tclk);
+	s->selected_timings.tRFC = DIV_ROUND_UP(timings->min_tRFC,
+						s->selected_timings.tclk);
+	s->selected_timings.tWTR = DIV_ROUND_UP(timings->min_tWTR,
+						s->selected_timings.tclk);
+	s->selected_timings.tRRD = DIV_ROUND_UP(timings->min_tRRD,
+						s->selected_timings.tclk);
+	s->selected_timings.tRTP = DIV_ROUND_UP(timings->min_tRTP,
+						s->selected_timings.tclk);
+}
+static void print_selected_timings(struct sysinfo *s)
+{
+	printk(BIOS_DEBUG, "Selected timings:\n");
+	printk(BIOS_DEBUG, "\tFSB:  %dMHz\n",
+		fsb2mhz(s->selected_timings.fsb_clk));
+	printk(BIOS_DEBUG, "\tDDR:  %dMHz\n",
+		ddr2mhz(s->selected_timings.mem_clk));
+
+	printk(BIOS_DEBUG, "\tCAS:  %d\n", s->selected_timings.CAS);
+	printk(BIOS_DEBUG, "\ttRAS: %d\n", s->selected_timings.tRAS);
+	printk(BIOS_DEBUG, "\ttRP:  %d\n", s->selected_timings.tRP);
+	printk(BIOS_DEBUG, "\ttRCD: %d\n", s->selected_timings.tRCD);
+	printk(BIOS_DEBUG, "\ttWR:  %d\n", s->selected_timings.tWR);
+	printk(BIOS_DEBUG, "\ttRFC: %d\n", s->selected_timings.tRFC);
+	printk(BIOS_DEBUG, "\ttWTR: %d\n", s->selected_timings.tWTR);
+	printk(BIOS_DEBUG, "\ttRRD: %d\n", s->selected_timings.tRRD);
+	printk(BIOS_DEBUG, "\ttRTP: %d\n", s->selected_timings.tRTP);
+}
+
+static void find_fsb_speed(struct sysinfo *s)
 {
-	u8 i;
-	u8 commoncas = 0;
-	u8 currcas;
-	u8 currfreq;
-	u8 maxfreq;
-	u8 freq = 0;
-
-	// spdidx,cycletime @CAS				 5	   6
-	u8 idx800[7][2] = {{0, 0}, {0, 0}, {0, 0}, {0, 0}, {0, 0}, {23, 0x30},
-			   {9, 0x25} };
-	int found = 0;
-
-	// Find max FSB speed
 	switch (MCHBAR32(0xc00) & 0x7) {
 	case 0x0:
 		s->max_fsb = FSB_CLOCK_1066MHz;
@@ -225,88 +251,98 @@ static void sdram_detect_ram_speed(struct sysinfo *s)
 		printk(BIOS_WARNING, "Can't detect FSB, setting 800MHz\n");
 		break;
 	}
+	s->selected_timings.fsb_clk = s->max_fsb;
+}
 
-	// Max RAM speed
-	if (s->spd_type == DDR2) {
-
-		maxfreq = MEM_CLOCK_800MHz;
-
-		// Choose common CAS latency from {6,5}, 4 does not work
-		commoncas = 0x60;
+static void decode_spd_select_timings(struct sysinfo *s)
+{
+	unsigned int device;
+	u8 dram_type_mask = (1 << DDR2) | (1 << DDR3);
+	u8 dimm_mask = 0;
+	u8 raw_spd[256];
+	int i, j;
+	struct abs_timings saved_timings;
+	memset(&saved_timings, 0, sizeof(saved_timings));
+	saved_timings.cas_supported = UINT32_MAX;
 
-		FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-			commoncas &= s->dimms[i].cas_latencies;
+	FOR_EACH_DIMM(i) {
+		s->dimms[i].card_type = RAW_CARD_POPULATED;
+		device = s->spd_map[i];
+		if (!device) {
+			s->dimms[i].card_type = RAW_CARD_UNPOPULATED;
+			continue;
 		}
-		if (commoncas == 0)
-			die("No common CAS among dimms\n");
-
-		// Working from fastest to slowest,
-		// fast->slow 5@800 6@800 5@667
-		found = 0;
-		for (currcas = 5; currcas <= msbpos(commoncas); currcas++) {
-			currfreq = maxfreq;
-			if (currfreq == MEM_CLOCK_800MHz) {
-				found = 1;
-				FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-					if (s->dimms[i].spd_data[idx800[currcas][0]] > idx800[currcas][1]) {
-						// this is too fast
-						found = 0;
-					}
-				}
-				if (found)
-					break;
-			}
+		switch (spd_read_byte(s->spd_map[i], SPD_MEMORY_TYPE)) {
+		case DDR2SPD:
+			dram_type_mask &= 1 << DDR2;
+			s->spd_type = DDR2;
+			break;
+		case DDR3SPD:
+			dram_type_mask &= 1 << DDR3;
+			s->spd_type = DDR3;
+			break;
+		default:
+			s->dimms[i].card_type = RAW_CARD_UNPOPULATED;
+			continue;
 		}
+		if (!dram_type_mask)
+			die("Mixing up dimm types is not supported!\n");
 
-		if (!found) {
-			currcas = 5;
-			currfreq = MEM_CLOCK_667MHz;
-			found = 1;
-			FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-				if (s->dimms[i].spd_data[9] > 0x30) {
-					// this is too fast
-					found = 0;
-				}
+		printk(BIOS_DEBUG, "Decoding dimm %d\n", i);
+		if (s->spd_type == DDR2){
+			printk(BIOS_DEBUG,
+				"Reading SPD using i2c block operation.\n");
+			if (i2c_block_read(device, 0, 64, raw_spd) != 64) {
+				printk(BIOS_DEBUG, "i2c block operation failed,"
+					" trying smbus byte operation.\n");
+				for (j = 0; j < 64; j++)
+					raw_spd[j] = spd_read_byte(device, j);
+			}
+			if (ddr2_save_dimminfo(i, raw_spd, &saved_timings, s)) {
+				printk(BIOS_WARNING,
+					"Encountered problems with SPD, "
+					"skipping this DIMM.\n");
+				s->dimms[i].card_type = RAW_CARD_UNPOPULATED;
+				continue;
 			}
+		} else { /* DDR3: not implemented so don't decode */
+			die("DDR3 support is not implemented\n");
 		}
+		dimm_mask |= (1 << i);
+	}
+	if (!dimm_mask)
+		die("No memory installed.\n");
 
-		if (!found)
-			die("No valid CAS/frequencies detected\n");
-
-		s->selected_timings.mem_clk = currfreq;
-		s->selected_timings.CAS = currcas;
-
-	} else { // DDR3
-		// Limit frequency for MCH
-		maxfreq = (s->max_ddr2_mhz == 800) ? MEM_CLOCK_800MHz : MEM_CLOCK_667MHz;
-		maxfreq >>= 3;
-		freq = MEM_CLOCK_1333MHz;
-		if (maxfreq)
-			freq = maxfreq + 2;
-		if (freq > MEM_CLOCK_1333MHz)
-			freq = MEM_CLOCK_1333MHz;
-
-		// Limit DDR speed to FSB speed
-		switch (s->max_fsb) {
-		case FSB_CLOCK_800MHz:
-			if (freq > MEM_CLOCK_800MHz)
-				freq = MEM_CLOCK_800MHz;
-			break;
-		case FSB_CLOCK_1066MHz:
-			if (freq > MEM_CLOCK_1066MHz)
-				freq = MEM_CLOCK_1066MHz;
-			break;
-		case FSB_CLOCK_1333MHz:
-			if (freq > MEM_CLOCK_1333MHz)
-				freq = MEM_CLOCK_1333MHz;
-			break;
-		default:
-			die("Invalid FSB\n");
-			break;
-		}
+	if (s->spd_type == DDR2)
+		select_cas_dramfreq_ddr2(s, &saved_timings);
+	select_discrete_timings(s, &saved_timings);
+}
+
+static void find_dimm_config(struct sysinfo *s)
+{
+	int chan, i;
 
-		// TODO: CAS detection for DDR3
+	FOR_EACH_POPULATED_CHANNEL(s->dimms, chan) {
+		FOR_EACH_POPULATED_DIMM_IN_CHANNEL(s->dimms, chan, i) {
+			int dimm_config;
+			if (s->dimms[i].ranks == 1) {
+				if (s->dimms[i].width == 0)	/* x8 */
+					dimm_config = 1;
+				else				/* x16 */
+					dimm_config = 3;
+			} else {
+				if (s->dimms[i].width == 0)	/* x8 */
+					dimm_config = 2;
+				else
+					die("Dual-rank x16 not supported\n");
+			}
+			s->dimm_config[chan] |=
+				dimm_config << (i % DIMMS_PER_CHANNEL) * 2;
+		}
+		printk(BIOS_DEBUG, "  Config[CH%d] : %d\n", chan,
+			s->dimm_config[chan]);
 	}
+
 }
 
 static void checkreset_ddr2(int boot_path)
@@ -367,19 +403,15 @@ void sdram_initialize(int boot_path, const u8 *spd_map)
 	checkreset_ddr2(s.boot_path);
 
 	/* Detect dimms per channel */
-	s.dimms_per_ch = 2;
 	reg8 = pci_read_config8(PCI_DEV(0, 0, 0), 0xe9);
-	if (reg8 & 0x10)
-		s.dimms_per_ch = 1;
-
-	printk(BIOS_DEBUG, "Dimms per channel: %d\n", s.dimms_per_ch);
+	printk(BIOS_DEBUG, "Dimms per channel: %d\n", (reg8 & 0x10) ? 1 : 2);
 
 	mchinfo_ddr2(&s);
 
-	sdram_read_spds(&s);
-
-	/* Choose Common Frequency */
-	sdram_detect_ram_speed(&s);
+	find_fsb_speed(&s);
+	decode_spd_select_timings(&s);
+	print_selected_timings(&s);
+	find_dimm_config(&s);
 
 	switch (s.spd_type) {
 	case DDR2:
diff --git a/src/northbridge/intel/x4x/raminit_ddr2.c b/src/northbridge/intel/x4x/raminit_ddr2.c
index 02c7fee475..e6ba2dcef8 100644
--- a/src/northbridge/intel/x4x/raminit_ddr2.c
+++ b/src/northbridge/intel/x4x/raminit_ddr2.c
@@ -31,12 +31,12 @@
 
 #define ME_UMA_SIZEMB 0
 
-static u32 fsb2mhz(u32 speed)
+u32 fsb2mhz(u32 speed)
 {
 	return (speed * 267) + 800;
 }
 
-static u32 ddr2mhz(u32 speed)
+u32 ddr2mhz(u32 speed)
 {
 	static const u16 mhz[] = { 0, 0, 667, 800, 1067, 1333 };
 
@@ -64,113 +64,6 @@ static u8 msbpos(u32 val)
 	return (u8)(pos & 0xff);
 }
 
-static void sdram_detect_smallest_params2(struct sysinfo *s)
-{
-	u16 mult[6] = {
-		5000, // 400
-		3750, // 533
-		3000, // 667
-		2500, // 800
-		1875, // 1066
-		1500, // 1333
-	};
-
-	u8 i;
-	u32 tmp;
-	u32 maxtras = 0;
-	u32 maxtrp = 0;
-	u32 maxtrcd = 0;
-	u32 maxtwr = 0;
-	u32 maxtrfc = 0;
-	u32 maxtwtr = 0;
-	u32 maxtrrd = 0;
-	u32 maxtrtp = 0;
-
-	FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-		maxtras = MAX(maxtras, s->dimms[i].spd_data[30] * 1000);
-		maxtrp = MAX(maxtrp, (s->dimms[i].spd_data[27] * 1000) >> 2);
-		maxtrcd = MAX(maxtrcd, (s->dimms[i].spd_data[29] * 1000) >> 2);
-		maxtwr = MAX(maxtwr, (s->dimms[i].spd_data[36] * 1000) >> 2);
-		maxtrfc = MAX(maxtrfc, s->dimms[i].spd_data[42] * 1000 +
-				(s->dimms[i].spd_data[40] & 0xf));
-		maxtwtr = MAX(maxtwtr, (s->dimms[i].spd_data[37] * 1000) >> 2);
-		maxtrrd = MAX(maxtrrd, (s->dimms[i].spd_data[28] * 1000) >> 2);
-		maxtrtp = MAX(maxtrtp, (s->dimms[i].spd_data[38] * 1000) >> 2);
-	}
-	for (i = 9; i < 24; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtras) {
-			s->selected_timings.tRAS = i;
-			break;
-		}
-	}
-	for (i = 3; i < 10; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtrp) {
-			s->selected_timings.tRP = i;
-			break;
-		}
-	}
-	for (i = 3; i < 10; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtrcd) {
-			s->selected_timings.tRCD = i;
-			break;
-		}
-	}
-	for (i = 3; i < 15; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtwr) {
-			s->selected_timings.tWR = i;
-			break;
-		}
-	}
-	for (i = 15; i < 78; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtrfc) {
-			s->selected_timings.tRFC = ((i + 16) & 0xfe) - 15;
-			break;
-		}
-	}
-	for (i = 4; i < 15; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtwtr) {
-			s->selected_timings.tWTR = i;
-			break;
-		}
-	}
-	for (i = 2; i < 15; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtrrd) {
-			s->selected_timings.tRRD = i;
-			break;
-		}
-	}
-	for (i = 4; i < 15; i++) {
-		tmp = mult[s->selected_timings.mem_clk] * i;
-		if (tmp >= maxtrtp) {
-			s->selected_timings.tRTP = i;
-			break;
-		}
-	}
-
-	s->selected_timings.fsb_clk = s->max_fsb;
-
-	printk(BIOS_DEBUG, "Selected timings:\n");
-	printk(BIOS_DEBUG, "\tFSB:  %dMHz\n", fsb2mhz(s->selected_timings.fsb_clk));
-	printk(BIOS_DEBUG, "\tDDR:  %dMHz\n", ddr2mhz(s->selected_timings.mem_clk));
-
-	printk(BIOS_DEBUG, "\tCAS:  %d\n", s->selected_timings.CAS);
-	printk(BIOS_DEBUG, "\ttRAS: %d\n", s->selected_timings.tRAS);
-	printk(BIOS_DEBUG, "\ttRP:  %d\n", s->selected_timings.tRP);
-	printk(BIOS_DEBUG, "\ttRCD: %d\n", s->selected_timings.tRCD);
-	printk(BIOS_DEBUG, "\ttWR:  %d\n", s->selected_timings.tWR);
-	printk(BIOS_DEBUG, "\ttRFC: %d\n", s->selected_timings.tRFC);
-	printk(BIOS_DEBUG, "\ttWTR: %d\n", s->selected_timings.tWTR);
-	printk(BIOS_DEBUG, "\ttRRD: %d\n", s->selected_timings.tRRD);
-	printk(BIOS_DEBUG, "\ttRTP: %d\n", s->selected_timings.tRTP);
-}
-
 static void clkcross_ddr2(struct sysinfo *s)
 {
 	u8 i, j;
@@ -500,8 +393,7 @@ static void timings_ddr2(struct sysinfo *s)
 	twl = s->selected_timings.CAS - 1;
 
 	FOR_EACH_POPULATED_DIMM(s->dimms, i) {
-		if (s->dimms[i].banks == 1) {
-			/* 8 banks */
+		if (s->dimms[i].n_banks == N_BANKS_8) {
 			trpmod = 1;
 			bankmod = 0;
 		}
@@ -1276,11 +1168,12 @@ static void dradrb_ddr2(struct sysinfo *s)
 			i = ch << 1;
 		else
 			i = (ch << 1) + 1;
-		dra = dratab[s->dimms[i].banks]
+
+		dra = dratab[s->dimms[i].n_banks]
 			[s->dimms[i].width]
 			[s->dimms[i].cols-9]
 			[s->dimms[i].rows-12];
-		if (s->dimms[i].banks == 1)
+		if (s->dimms[i].n_banks == N_BANKS_8)
 			dra |= 0x80;
 		if (ch == 0) {
 			c0dra |= dra << (r*8);
@@ -1313,12 +1206,10 @@ static void dradrb_ddr2(struct sysinfo *s)
 		if (ch == 0) {
 			dra0 = (c0dra >> (8*r)) & 0x7f;
 			c0drb = (u16)(c0drb + drbtab[dra0]);
-			s->dimms[i].rank_capacity_mb = drbtab[dra0] << 6;
 			MCHBAR16(0x200 + 2*r) = c0drb;
 		} else {
 			dra1 = (c1dra >> (8*r)) & 0x7f;
 			c1drb = (u16)(c1drb + drbtab[dra1]);
-			s->dimms[i].rank_capacity_mb = drbtab[dra1] << 6;
 			MCHBAR16(0x600 + 2*r) = c1drb;
 		}
 	}
@@ -1588,9 +1479,6 @@ void raminit_ddr2(struct sysinfo *s)
 	u8 r, bank;
 	u32 reg32;
 
-	// Select timings based on SPD info
-	sdram_detect_smallest_params2(s);
-
 	if (s->boot_path != BOOT_PATH_WARM_RESET) {
 		// Clear self refresh
 		MCHBAR32(PMSTS_MCHBAR) = MCHBAR32(PMSTS_MCHBAR)
diff --git a/src/northbridge/intel/x4x/x4x.h b/src/northbridge/intel/x4x/x4x.h
index 9db6c12122..cbb1853fab 100644
--- a/src/northbridge/intel/x4x/x4x.h
+++ b/src/northbridge/intel/x4x/x4x.h
@@ -144,6 +144,7 @@
 #define TOTAL_DIMMS 4
 #define DIMMS_PER_CHANNEL (TOTAL_DIMMS / TOTAL_CHANNELS)
 #define RAW_CARD_UNPOPULATED 0xff
+#define RAW_CARD_POPULATED 0
 
 #define DIMM_IS_POPULATED(dimms, idx) (dimms[idx].card_type != RAW_CARD_UNPOPULATED)
 #define IF_DIMM_POPULATED(dimms, idx) if (dimms[idx].card_type != RAW_CARD_UNPOPULATED)
@@ -249,8 +250,14 @@ struct dll_setting {
 	u8 coarse;
 };
 
+enum n_banks {
+	N_BANKS_4 = 0,
+	N_BANKS_8 = 1,
+};
+
 struct timings {
 	unsigned int	CAS;
+	unsigned int    tclk;
 	enum fsb_clock	fsb_clk;
 	enum mem_clock	mem_clk;
 	unsigned int	tRAS;
@@ -267,40 +274,20 @@ struct dimminfo {
 	unsigned int	card_type; /* 0xff: unpopulated,
 				      0xa - 0xf: raw card type A - F */
 	enum chip_width	width;
-	enum chip_cap	chip_capacity;
 	unsigned int	page_size; /* of whole DIMM in Bytes (4096 or 8192) */
-	unsigned int	sides;
-	unsigned int	banks;
+	enum n_banks	n_banks;
 	unsigned int	ranks;
 	unsigned int	rows;
 	unsigned int	cols;
-	unsigned int	cas_latencies;
-	unsigned int	tAAmin;
-	unsigned int	tCKmin;
-	unsigned int	tWR;
-	unsigned int	tRP;
-	unsigned int	tRCD;
-	unsigned int	tRAS;
-	unsigned int	rank_capacity_mb; /* per rank in Megabytes */
-	u8		spd_data[256];
 };
 
 /* The setup is up to two DIMMs per channel */
 struct sysinfo {
-	int		txt_enabled;
-	int		cores;
 	int		boot_path;
 	int		max_ddr2_mhz;
-	int		max_ddr3_mt;
 	enum fsb_clock	max_fsb;
-	int		max_fsb_mhz;
-	int		max_render_mhz;
-	int		enable_igd;
-	int		enable_peg;
-	u16		ggc;
 
 	int		dimm_config[2];
-	int		dimms_per_ch;
 	int		spd_type;
 	int		channel_capacity[2];
 	struct timings	selected_timings;
@@ -346,6 +333,8 @@ u8 decode_pciebar(u32 *const base, u32 *const len);
 void sdram_initialize(int boot_path, const u8 *spd_map);
 void raminit_ddr2(struct sysinfo *);
 void rcven(const struct sysinfo *);
+u32 fsb2mhz(u32 speed);
+u32 ddr2mhz(u32 speed);
 
 struct acpi_rsdp;
 #ifndef __SIMPLE_DEVICE__