[coreboot-gerrit] Change in coreboot[master]: nb/intel/x4x: Refactor setting default DQ DQS dll settings

Arthur Heymans (Code Review) gerrit at coreboot.org
Tue Jun 6 23:26:23 CEST 2017


Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/20057


Change subject: nb/intel/x4x: Refactor setting default DQ DQS dll settings
......................................................................

nb/intel/x4x: Refactor setting default DQ DQS dll settings

Makes clear that this sets safe defaults, improves readability,
makes it easy to restore previously trained values instead of the safe
default ones.

Change-Id: I6b7bf90085bf4ef14bbb6de852b96e04b429fca4
Signed-off-by: Arthur Heymans <arthur at aheymans.xyz>
---
M src/northbridge/intel/x4x/dq_dqs_dll.c
M src/northbridge/intel/x4x/raminit_ddr23.c
M src/northbridge/intel/x4x/x4x.h
3 files changed, 94 insertions(+), 75 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/20057/1

diff --git a/src/northbridge/intel/x4x/dq_dqs_dll.c b/src/northbridge/intel/x4x/dq_dqs_dll.c
index 7741b6f..043f144 100644
--- a/src/northbridge/intel/x4x/dq_dqs_dll.c
+++ b/src/northbridge/intel/x4x/dq_dqs_dll.c
@@ -649,23 +649,6 @@
 	0xdfdfdfdf, 0xbebebebe, 0x7f7f7f7f, 0xfefefefe
 };
 
-static void rt_set_dqs(u8 channel, u8 lane, u8 rank, struct rt_dqs_setting *dqs_setting)
-{
-	u8 saved_tap = MCHBAR16(0x540 + 0x400 * channel + lane * 4);
-	u8 saved_pi = MCHBAR16(0x542 + 0x400 * channel + lane * 4);
-	printk(RAM_SPEW, "RT DQS: ch%d, L%d, %d.%d\n", channel, lane,
-		dqs_setting->tap,
-		dqs_setting->pi);
-
-	saved_tap &= ~(0xf << (rank * 4));
-	saved_tap |= dqs_setting->tap << (rank * 4);
-	MCHBAR16(0x540 + 0x400 * channel + lane * 4) = saved_tap;
-
-	saved_pi &= ~(0x7 << (rank * 3));
-	saved_pi |= dqs_setting->pi << (rank * 4);
-	MCHBAR16(0x542 + 0x400 * channel + lane * 4) = saved_pi;
-}
-
 static int rt_increment_dqs(struct rt_dqs_setting *setting)
 {
 	if (setting->pi < 7) {
diff --git a/src/northbridge/intel/x4x/raminit_ddr23.c b/src/northbridge/intel/x4x/raminit_ddr23.c
index b5fcbd8..493867b 100644
--- a/src/northbridge/intel/x4x/raminit_ddr23.c
+++ b/src/northbridge/intel/x4x/raminit_ddr23.c
@@ -462,6 +462,24 @@
 		setting->tap;
 }
 
+void rt_set_dqs(u8 channel, u8 lane, u8 rank, struct rt_dqs_setting *dqs_setting)
+{
+	u8 saved_tap = MCHBAR16(0x540 + 0x400 * channel + lane * 4);
+	u8 saved_pi = MCHBAR16(0x542 + 0x400 * channel + lane * 4);
+	printk(RAM_SPEW, "RT DQS: ch%d, L%d, %d.%d\n", channel, lane,
+		dqs_setting->tap,
+		dqs_setting->pi);
+
+	saved_tap &= ~(0xf << (rank * 4));
+	saved_tap |= dqs_setting->tap << (rank * 4);
+	MCHBAR16(0x540 + 0x400 * channel + lane * 4) = saved_tap;
+
+	saved_pi &= ~(0x7 << (rank * 3));
+	saved_pi |= dqs_setting->pi << (rank * 4);
+	MCHBAR16(0x542 + 0x400 * channel + lane * 4) = saved_pi;
+}
+
+
 static void program_timings(struct sysinfo *s)
 {
 	u8 i;
@@ -762,20 +780,6 @@
 	cmdset(channel, &dll_settings[CMD]);
 }
 
-static void program_dq_dqs(struct sysinfo *s, u8 channel,
-			const struct dll_setting dll_setting[23])
-{
-	int lane;
-	for (lane = 0; lane < 8; lane++) {
-		s->dqs_settings[channel][lane] = dll_setting[DQS1 + lane];
-		dqsset(channel, lane, &dll_setting[DQS1 + lane]);
-	}
-	for (lane = 0; lane < 8; lane++) {
-		s->dq_settings[channel][lane] = dll_setting[DQ1 + lane];
-		dqset(channel, lane, &dll_setting[DQ1 + lane]);
-	}
-}
-
 void print_dll_setting(const struct dll_setting *dll_setting, u8 default_verbose)
 {
 	u8 debug_level;
@@ -795,7 +799,6 @@
 	u8 i, j, r, reg8, clk, async = 0;
 	u16 reg16 = 0;
 	u32 reg32 = 0;
-	u8 lane;
 
 	const u8 rank2clken[8] = { 0x04, 0x01, 0x20, 0x08, 0x01, 0x04,
 				   0x08, 0x10 };
@@ -1063,55 +1066,85 @@
 
 	if (s->selected_timings.mem_clk == MEM_CLOCK_1333MHz)
 		MCHBAR8(0x18c) = MCHBAR8(0x18c) | 1;
+}
 
-	// Program DQ/DQS dll settings
-	reg32 = 0;
-	FOR_EACH_POPULATED_CHANNEL(s->dimms, i) {
-		switch (s->selected_timings.mem_clk) {
-		default: /* Should not happen */
-			break;
-		case MEM_CLOCK_667MHz:
-			reg32 = 0x06db7777;
-			break;
-		case MEM_CLOCK_800MHz:
-			if (s->spd_type == DDR2)
-				reg32 = 0x00007777;
-			else /* DDR3 */
-				reg32 = 0x06db6666;
-			break;
-		case MEM_CLOCK_1066MHz:
-			reg32 = 0x06db5555;
-			break;
-		case MEM_CLOCK_1333MHz:
-			reg32 = 0x00007777;
-			break;
+static void select_default_dqdqs_dll_settings(struct sysinfo *s)
+{
+	int channel, lane;
+	u8 rt_pi, rt_tap;
+	const struct dll_setting *dll_settings;
+
+	switch (s->selected_timings.mem_clk) {
+	default: /* Should not happen */
+	case MEM_CLOCK_667MHz:
+		rt_pi = 3;
+		rt_tap = 7;
+		break;
+	case MEM_CLOCK_800MHz:
+		if (s->spd_type == DDR2) {
+			rt_tap = 7;
+			rt_pi = 0;
+		} else { /* DDR3 */
+			rt_tap = 6;
+			rt_pi = 3;
 		}
+		break;
+	case MEM_CLOCK_1066MHz:
+		rt_tap = 5;
+		rt_pi = 3;
+		break;
+	case MEM_CLOCK_1333MHz:
+		rt_tap = 7;
+		rt_pi = 0;
+		break;
+	}
+
+	/* RT DQS: set identical settings for each rank */
+	FOR_EACH_POPULATED_CHANNEL(s->dimms, channel) {
 		for (lane = 0; lane < 8; lane++) {
-			MCHBAR32(0x400*i + 0x540 + lane*4) =
-				(MCHBAR32(0x400*i + 0x540 + lane*4) & 0x0fffffff) |
-				reg32;
+			s->rt_dqs_setting[channel][lane].pi = rt_pi;
+			s->rt_dqs_setting[channel][lane].tap = rt_tap;
 		}
 	}
 
-	FOR_EACH_POPULATED_CHANNEL(s->dimms, i) {
-		switch (s->selected_timings.mem_clk) {
-		default: /* Should not happen */
-			break;
-		case MEM_CLOCK_667MHz:
-			program_dq_dqs(s, i, ddr2_dll_setting_667);
-			break;
-		case MEM_CLOCK_800MHz:
-			if (s->spd_type == DDR2)
-				program_dq_dqs(s, i, ddr2_dll_setting_800);
-			else /* DDR3 */
-				program_dq_dqs(s, i, ddr3_dll_setting_800[s->nmode - 1]);
-			break;
-		case MEM_CLOCK_1066MHz:
-			program_dq_dqs(s, i, ddr3_dll_setting_1066[s->nmode - 1]);
-			break;
-		case MEM_CLOCK_1333MHz:
-			program_dq_dqs(s, i, ddr3_dll_setting_1333[s->nmode - 1]);
-			break;
+	switch (s->selected_timings.mem_clk) {
+	default: /* Should not happen, but makes compiler happy */
+	case MEM_CLOCK_667MHz:
+		dll_settings = ddr2_dll_setting_667;
+		break;
+	case MEM_CLOCK_800MHz:
+		if (s->spd_type == DDR2)
+			dll_settings = ddr2_dll_setting_800;
+		else /* DDR3 */
+			dll_settings = ddr3_dll_setting_800[s->nmode - 1];
+		break;
+	case MEM_CLOCK_1066MHz:
+		dll_settings = ddr3_dll_setting_1066[s->nmode - 1];
+		break;
+	case MEM_CLOCK_1333MHz:
+		dll_settings = ddr3_dll_setting_1333[s->nmode - 1];
+		break;
+	}
+
+	FOR_EACH_POPULATED_CHANNEL(s->dimms, channel) {
+		for (lane = 0; lane < 8; lane++) {
+			s->dqs_settings[channel][lane] = dll_settings[DQS1 + lane];
+			s->dqs_settings[channel][lane] = dll_settings[DQ1 + lane];
+		}
+	}
+}
+
+static void program_dqdqs_settings(struct sysinfo *s)
+{
+	int channel, lane, rank;
+	/* RT DQS DLL settings */
+	FOR_EACH_POPULATED_CHANNEL(s->dimms, channel) {
+		for (lane = 0; lane < 8; lane++) {
+			dqsset(channel, lane, &(s->dqs_settings[channel][lane]));
+			dqset(channel, lane, &(s->dq_settings[channel][lane]));
+			for (rank = 0; rank < 4; rank++)
+				rt_set_dqs(channel, lane, rank,
+					&s->rt_dqs_setting[channel][lane]);
 		}
 	}
 }
@@ -2246,6 +2279,8 @@
 
 	// Program DLL
 	program_dll(s);
+	select_default_dqdqs_dll_settings(s);
+	program_dqdqs_settings(s);
 
 	// RCOMP
 	if (s->boot_path != BOOT_PATH_WARM_RESET) {
diff --git a/src/northbridge/intel/x4x/x4x.h b/src/northbridge/intel/x4x/x4x.h
index 07cc71b..d78a743 100644
--- a/src/northbridge/intel/x4x/x4x.h
+++ b/src/northbridge/intel/x4x/x4x.h
@@ -367,6 +367,7 @@
 void send_jedec_cmd(const struct sysinfo *s, u8 r, u8 ch, u8 cmd, u32 val);
 void dqsset(u8 ch, u8 lane, const struct dll_setting *setting);
 void dqset(u8 ch, u8 lane, const struct dll_setting *setting);
+void rt_set_dqs(u8 channel, u8 lane, u8 rank, struct rt_dqs_setting *dqs_setting);
 void print_dll_setting(const struct dll_setting *dll_setting, u8 default_verbose);
 void search_write_leveling(struct sysinfo *s);
 int do_write_training(struct sysinfo *s);

-- 
To view, visit https://review.coreboot.org/20057
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6b7bf90085bf4ef14bbb6de852b96e04b429fca4
Gerrit-Change-Number: 20057
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>



More information about the coreboot-gerrit mailing list