Angel Pons has uploaded this change for review. ( 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 th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_common.h M src/northbridge/intel/sandybridge/raminit_native.c 2 files changed, 55 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/66/47766/1
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");
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47766 )
Change subject: nb/intel/sandybridge: Clean up COMPOFST1 logic ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47766/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_common.h:
https://review.coreboot.org/c/coreboot/+/47766/1/src/northbridge/intel/sandy... PS1, Line 155: u32 : 2; space prohibited before that ':' (ctx:WxW)
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47766 )
Change subject: nb/intel/sandybridge: Clean up COMPOFST1 logic ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/47766/4/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_common.h:
https://review.coreboot.org/c/coreboot/+/47766/4/src/northbridge/intel/sandy... PS4, Line 155: u32 : 2; space prohibited before that ':' (ctx:WxW)
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47766 )
Change subject: nb/intel/sandybridge: Clean up COMPOFST1 logic ......................................................................
Patch Set 4: Code-Review+2
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 th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47766 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- 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");
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
Patrick Georgi (Code Review)