[coreboot-gerrit] Change in coreboot[master]: nb/intel/sandybridge: Clean up COMPOFST1 logic

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47766 )

Change subject: nb/intel/sandybridge: Clean up COMPOFST1 logic
......................................................................

nb/intel/sandybridge: Clean up COMPOFST1 logic

This register needs to be updated differently depending on the CPU
generation and stepping. Handle this as per reference code. Further,
introduce a bitfield for the register to make the code easier to read.

Change-Id: I51649cb2fd06c5896f90559f59f25d49a8e6695e
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47766
Tested-by: build bot (Jenkins) 
Reviewed-by: Arthur Heymans 
---
M src/northbridge/intel/sandybridge/raminit_common.h
M src/northbridge/intel/sandybridge/raminit_native.c
2 files changed, 55 insertions(+), 7 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved

diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h
index f2d0fb5..debfaa2 100644
--- a/src/northbridge/intel/sandybridge/raminit_common.h
+++ b/src/northbridge/intel/sandybridge/raminit_common.h
@@ -140,6 +140,23 @@
    u32 raw;
 };

+union comp_ofst_1_reg {
+	struct {
+		u32 dq_odt_down  : 3; /* [ 2.. 0] */
+		u32 dq_odt_up    : 3; /* [ 5.. 3] */
+		u32 clk_odt_down : 3; /* [ 8.. 6] */
+		u32 clk_odt_up   : 3; /* [11.. 9] */
+		u32 dq_drv_down  : 3; /* [14..12] */
+		u32 dq_drv_up    : 3; /* [17..15] */
+		u32 clk_drv_down : 3; /* [20..18] */
+		u32 clk_drv_up   : 3; /* [23..21] */
+		u32 ctl_drv_down : 3; /* [26..24] */
+		u32 ctl_drv_up   : 3; /* [29..27] */
+		u32              : 2;
+	};
+	u32 raw;
+};
+
 union tc_dbp_reg {
    struct {
    	u32 tRCD : 4; /* [ 3.. 0] */
diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c
index e7a3352..3522e96 100644
--- a/src/northbridge/intel/sandybridge/raminit_native.c
+++ b/src/northbridge/intel/sandybridge/raminit_native.c
@@ -176,6 +176,43 @@
    	return is_ivybridge ? 0x0D6FF5E4 : 0x0D6BEDCC;
 }

+/* Get updated COMP1 based on CPU generation and stepping */
+static u32 get_COMP1(ramctr_timing *ctrl, const int channel)
+{
+	const union comp_ofst_1_reg orig_comp = {
+		.raw = MCHBAR32(CRCOMPOFST1_ch(channel)),
+	};
+
+	if (IS_SANDY_CPU(ctrl->cpu) && !IS_SANDY_CPU_D2(ctrl->cpu)) {
+		union comp_ofst_1_reg comp_ofst_1 = orig_comp;
+
+		comp_ofst_1.clk_odt_up = 1;
+		comp_ofst_1.clk_drv_up = 1;
+		comp_ofst_1.ctl_drv_up = 1;
+
+		return comp_ofst_1.raw;
+	}
+
+	/* Fix PCODE COMP offset bug: revert to default values */
+	union comp_ofst_1_reg comp_ofst_1 = {
+		.dq_odt_down  = 4,
+		.dq_odt_up    = 4,
+		.clk_odt_down = 4,
+		.clk_odt_up   = orig_comp.clk_odt_up,
+		.dq_drv_down  = 4,
+		.dq_drv_up    = orig_comp.dq_drv_up,
+		.clk_drv_down = 4,
+		.clk_drv_up   = orig_comp.clk_drv_up,
+		.ctl_drv_down = 4,
+		.ctl_drv_up   = orig_comp.ctl_drv_up,
+	};
+
+	if (IS_IVY_CPU(ctrl->cpu))
+		comp_ofst_1.dq_drv_up = 2;	/* 28p6 ohms */
+
+	return comp_ofst_1.raw;
+}
+
 static void normalize_tclk(ramctr_timing *ctrl, bool ref_100mhz_support)
 {
    if (ctrl->tCK <= TCK_1200MHZ) {
@@ -568,8 +605,6 @@

static void dram_ioregs(ramctr_timing *ctrl)
 {
-	u32 reg;
-
    int channel;

/* IO clock */
@@ -600,11 +635,7 @@

/* Set COMP1 */
    FOR_ALL_POPULATED_CHANNELS {
-		reg = MCHBAR32(CRCOMPOFST1_ch(channel));
-		reg = (reg & ~0x00000e00) | (1 <<  9);	/* ODT */
-		reg = (reg & ~0x00e00000) | (1 << 21);	/* clk drive up */
-		reg = (reg & ~0x38000000) | (1 << 27);	/* ctl drive up */
-		MCHBAR32(CRCOMPOFST1_ch(channel)) = reg;
+		MCHBAR32(CRCOMPOFST1_ch(channel)) = get_COMP1(ctrl, channel);
    }
    printram("COMP1 done\n");