From 2bb361f0f5151f0cbf0bfcde085c05ca38c42de9 Mon Sep 17 00:00:00 2001
From: Nico Huber <nico.h@gmx.de>
Date: Sun, 28 Mar 2021 18:07:00 +0200
Subject: [PATCH] nb/intel/x4x: Refactor sync DLL programming (part 2)

Instead of counting consecutive matches (in `j`), check for a second
match directly in the control flow. Also, add some dedicated variables:

* `tap`: Keeps track of the tap value that resulted in a match and
         is eventually programmed into the hardware.

* `tap2`: Is just temporarily used to search for another edge.

Keeping `tap` sync'ed with the hardware has the benefit that we don't
need to read the programmed value back for later fixups.

Change-Id: I3ae541c39efdc695f5ca74bc757b2f009239ec93
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51903
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
---
 src/northbridge/intel/x4x/raminit_ddr23.c | 80 +++++++++++------------
 1 file changed, 39 insertions(+), 41 deletions(-)

diff --git a/src/northbridge/intel/x4x/raminit_ddr23.c b/src/northbridge/intel/x4x/raminit_ddr23.c
index 99c07f5970..2d70b96d5d 100644
--- a/src/northbridge/intel/x4x/raminit_ddr23.c
+++ b/src/northbridge/intel/x4x/raminit_ddr23.c
@@ -680,7 +680,7 @@ static bool sync_dll_test_tap(unsigned int tap, uint32_t val)
 	return mchbar_read32(0x184) == val;
 }
 
-static void sync_dll_search_tap(uint8_t *tap, uint32_t val)
+static void sync_dll_search_tap(unsigned int *tap, uint32_t val)
 {
 	for (; *tap < sync_dll_max_taps; ++*tap)
 		if (sync_dll_test_tap(*tap, val))
@@ -689,7 +689,7 @@ static void sync_dll_search_tap(uint8_t *tap, uint32_t val)
 
 static void program_dll(struct sysinfo *s)
 {
-	u8 i, j, r, reg8, clk, async = 0;
+	u8 i, r, reg8, clk, async = 0;
 	u16 reg16 = 0;
 	u32 reg32 = 0;
 
@@ -842,61 +842,59 @@ static void program_dll(struct sysinfo *s)
 	}
 
 	/* XXX if not async mode */
+	unsigned int tap;
 	mchbar_clrbits16(0x180, 1 << 15 | 1 << 9);
 	mchbar_setbits8(0x180, 1 << 2);
-	j = 0;
-	for (i = 0; i < sync_dll_max_taps; i++) {
-		if (sync_dll_test_tap(i, 0xffffffff)) {
-			j++;
-			if (j >= 2)
-				break;
+	for (tap = 0; tap < sync_dll_max_taps; ++tap) {
+		sync_dll_search_tap(&tap, 0xffffffff);
 
-			if (s->selected_timings.mem_clk == MEM_CLOCK_667MHz) {
-				j = 2;
+		if (s->selected_timings.mem_clk == MEM_CLOCK_667MHz)
+			break;
+
+		++tap; /* other clock speeds need a second match */
+		if (sync_dll_test_tap(tap, 0xffffffff))
+			break;
+	}
+
+	/* look for a real edge if we started with a match */
+	if (tap <= 1) {
+		unsigned int tap2 = tap + 1;
+		sync_dll_search_tap(&tap2, 0);
+
+		for (++tap2; tap2 < sync_dll_max_taps; ++tap2) {
+			sync_dll_search_tap(&tap2, 0xffffffff);
+
+			++tap2; /* we need a second match */
+			if (sync_dll_test_tap(tap2, 0xffffffff))
 				break;
-			}
+		}
+
+		if (tap2 < sync_dll_max_taps) {
+			tap = tap2;
 		} else {
-			j = 0;
-		}
-	}
-	if (i == 1 || ((i == 0) && s->selected_timings.mem_clk == MEM_CLOCK_667MHz)) {
-		j = 0;
-		i++;
-		sync_dll_search_tap(&i, 0);
-		i++;
-		for (; i < sync_dll_max_taps; i++) {
-			if (sync_dll_test_tap(i, 0xffffffff)) {
-				j++;
-				if (j >= 2)
-					break;
-			} else {
-				j = 0;
-			}
-		}
-		if (j < 2) {
+			/* Using 0 instead of the original `tap` seems
+			   inconsistent, but is what the code always did. */
 			sync_dll_load_tap(0);
-			j = 2;
+			tap = 0;
 		}
 	}
 
-	if (j < 2) {
+	if (tap >= sync_dll_max_taps) {
 		mchbar_clrsetbits8(0x1c8, 0x1f, 0);
+		tap = 0;
 		async = 1;
 		printk(BIOS_NOTICE, "HMC failed, using async mode\n");
 	}
 
 	mchbar_clrbits8(0x180, 1 << 7);
 
-	if ((s->spd_type == DDR3 && s->selected_timings.mem_clk == MEM_CLOCK_1066MHz)
-		|| (s->spd_type == DDR2 && s->selected_timings.fsb_clk == FSB_CLOCK_800MHz
-			&& s->selected_timings.mem_clk == MEM_CLOCK_667MHz)) {
-		i = mchbar_read8(0x1c8) & 0xf;
-		if (s->spd_type == DDR2)
-			i = (i + 10) % 14;
-		else /* DDR3 */
-			i = (i + 3) % 12;
-		sync_dll_load_tap(i);
-	}
+	if (s->spd_type == DDR3 && s->selected_timings.mem_clk == MEM_CLOCK_1066MHz)
+		sync_dll_load_tap((tap + 3) % 12);
+
+	if (s->spd_type == DDR2 &&
+	    s->selected_timings.fsb_clk == FSB_CLOCK_800MHz &&
+	    s->selected_timings.mem_clk == MEM_CLOCK_667MHz)
+		sync_dll_load_tap((tap + 10) % 14);
 
 	switch (s->selected_timings.mem_clk) {
 	case MEM_CLOCK_667MHz: