Hello Felix Singer, Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48412
to review the following change.
Change subject: nb/intel/sandybridge: Correct DMI setup function ......................................................................
nb/intel/sandybridge: Correct DMI setup function
Program DMI registers as per reference code.
Change-Id: I2c2254a83e0ea8f8ec9d7290d88a6df1b7372d88 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 186 insertions(+), 122 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/48412/1
diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index bcf3e4c..e41cc87 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -1,163 +1,227 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <arch/cpu.h> #include <console/console.h> +#include <cpu/intel/model_206ax/model_206ax.h> #include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h>
+static void dmi_update_field(const u32 offset, const u32 mask, const u32 value, const u8 shift) +{ + u32 reg32 = DMIBAR32(offset); + reg32 &= ~(mask << shift); + reg32 |= (value << shift); + DMIBAR32(offset) = reg32; +} + +static void set_load_bus(const int lane, u32 load_sel, u32 load_data, const u8 stepping) +{ + u32 lbcvalue = stepping == IVB_STEP_A0 ? 0x30000000 : 0x70000000; + + lbcvalue |= lane & 1 ? 0x100000 : 0x80000; /* lbclnsel */ + lbcvalue |= (load_sel & 0x3f) << 21; /* lbcldsel */ + lbcvalue |= (load_data << 1) & 0x7fff; /* lbc data */ + + DMIBAR32(0x91c + 0x20 * (lane >> 1)) = lbcvalue; +} + void early_init_dmi(void) { + const u32 cpuid = cpu_get_cpuid(); + + const u8 stepping = cpuid & 0xf; + + u32 value; int i;
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0914 + (i << 5)) |= (1 << 31); + /* E1 PWRFSM fix */ + if (IS_IVY_CPU(cpuid) && stepping == IVB_STEP_E1) { + for (i = 0; i < 2; i++) + dmi_update_field(0x0914 + (i << 5), 1, 1, 31); }
+ /* UneqMM steps */ for (i = 0; i < 4; i++) { - DMIBAR32(0x0a00 + (i << 4)) &= ~0x0c000000; - DMIBAR32(0x0a04 + (i << 4)) |= (1 << 11); + dmi_update_field(0x0a00 + (i << 4), 3, 0, 26); + dmi_update_field(0x0a04 + (i << 4), 1, 1, 11); } - DMIBAR32(0x0c30) = (DMIBAR32(0x0c30) & 0x0fffffff) | (1 << 30);
+ /* DFEFIX */ + dmi_update_field(0x0c30, 0xf, 4, 28); + + /* Disable the 2nd order loop */ for (i = 0; i < 2; i++) { - DMIBAR32(0x0904 + (i << 5)) &= ~0x01c00000; - DMIBAR32(0x090c + (i << 5)) &= ~0x000e0000; + dmi_update_field(0x0904 + (i << 5), 7, 0, 22); + dmi_update_field(0x090c + (i << 5), 7, 0, 17); }
+ if (IS_IVY_CPU(cpuid)) { + if (stepping <= IVB_STEP_B0) + value = 8; + else if (stepping <= IVB_STEP_D0) + value = 5; + else + value = 0; + } else { + value = 0; + } + + for (i = 0; i < 2; i++) + dmi_update_field(0x090c + (i << 5), 0xf, value, 21); + + if (IS_IVY_CPU(cpuid) && stepping <= IVB_STEP_B0) { + + /* setrxincmval */ + dmi_update_field(0x0c08, 0x3f, 8, 4); + + /* forcectleopamp */ + for (i = 0; i < 4; i++) { + set_load_bus(i, 0x2f, 0x40, stepping); + set_load_bus(i, 0x38, 0x40, stepping); + } + } + + /* setcdrpg (PGACQ and PGTRK) */ for (i = 0; i < 2; i++) { - DMIBAR32(0x090c + (i << 5)) &= ~0x01e00000; + dmi_update_field(0x0904 + (i << 5), 0x3f, 0x18, 16); + dmi_update_field(0x090c + (i << 5), 0x3f, 0x09, 5); }
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - DMIBAR32(0x0904 + (i << 5)) = 0x7a1842ec; - DMIBAR32(0x090c + (i << 5)); // !!! = 0x00000208 - DMIBAR32(0x090c + (i << 5)) = 0x00000128; + /* setl0sfixes */ + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_C0) { + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 15); }
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x46139008 - DMIBAR32(0x0700 + (i << 5)) = 0x46139008; + /* setrxctlectl */ + dmi_update_field(0x0c04, 0xff, 0x73, 21); + + /* setdfegainacq */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0904 + (i << 5), 3, 1, 29); + + /* setg2rxctlepeak */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0910 + (i << 5), 0xf, 0, 10); + + /* setVrefAdap */ + for (i = 0; i < 4; i++) + dmi_update_field(0x0a00 + (i << 4), 0x1f, 12, 1); + + /* setbypscmcode */ + value = IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_C0 ? 0 : 1; + dmi_update_field(0x0c00, 1, value, 4); + + /* setctleoc */ + value = IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_C0 ? 1 : 0; + for (i = 0; i < 4; i++) + dmi_update_field(0x0a04 + (i << 4), 1, value, 12); + + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_C0) { + /* setdfelsbsel */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0900 + (i << 5), 3, 0, 26); + + /* setagcgainacq */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0908 + (i << 5), 3, 2, 27); + + /* setagcacqlen */ + for (i = 0; i < 4; i++) + dmi_update_field(0x0a00 + (i << 4), 3, 2, 17); + + /* setsampleratspeedcal */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 3); + + /* setoffcorgain */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0904 + (i << 5), 3, 1, 10); + + /* setdfesumctl */ + for (i = 0; i < 4; i++) + dmi_update_field(0x0a00 + (i << 4), 3, 3, 24); }
- DMIBAR32(0x0c04); // !!! = 0x2e680008 - DMIBAR32(0x0c04) = 0x2e680008; + /* setrxvcmdssel */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0908 + (i << 5), 3, 1, 30);
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - DMIBAR32(0x0904 + (i << 5)) = 0x3a1842ec; + /* Swing Control: Reduced */ + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_C0) { + /* setpegvcm */ + dmi_update_field(0x0c00, 0x1f, 0x14, 25); + + /* setblegctl */ + dmi_update_field(0x0c0c, 0x1f, 0x00, 23); }
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0910 + (i << 5)); // !!! = 0x00006300 - DMIBAR32(0x0910 + (i << 5)) = 0x00004300; + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_K0) { + /* setK0ctocfix */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 21); + + /* setK0l0sfreezefix */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 3, 1, 24); + + /* setK0samplerfix */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 19); + + /* setK0resetalignfix */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 23); + + /* setK0cdcprotfix */ + for (i = 0; i < 2; i++) + dmi_update_field(0x0700 + (i << 5), 1, 1, 22); }
- for (i = 0; i < 4; i++) { - DMIBAR32(0x0a00 + (i << 4)); // !!! = 0x03042010 - DMIBAR32(0x0a00 + (i << 4)) = 0x03042018; + /* setctleocgen2 */ + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_D0) { + for (i = 0; i < 2; i++) + dmi_update_field(0x0914 + (i << 5), 1, 1, 9); }
- DMIBAR32(0x0c00); // !!! = 0x29700c08 - DMIBAR32(0x0c00) = 0x29700c08; - - for (i = 0; i < 4; i++) { - DMIBAR32(0x0a04 + (i << 4)); // !!! = 0x0c0708f0 - DMIBAR32(0x0a04 + (i << 4)) = 0x0c0718f0; + /* + * ECO(3621419): Aggresive Rx L0s freeze resulting in Rx + * errors since AGC is in open loop for long time (L0s Exit) + */ + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_E0) { + for (i = 0; i < 2; i++) + dmi_update_field(0x0914 + (i << 5), 1, 1, 27); }
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0900 + (i << 5)); // !!! = 0x50000000 - DMIBAR32(0x0900 + (i << 5)) = 0x50000000; + /* setD0disableL1exitfix */ + if (IS_IVY_CPU(cpuid) && stepping == IVB_STEP_D0) + dmi_update_field(0x0c38, 1, 1, 27); + + /* + * afeln0cfg0_d1f0.useerrd + * FIXME: PEG only + */ + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_D0) { + for (i = 0; i < 4; i++) + dmi_update_field(0x0a00 + (i << 4), 1, 1, 21); + }
- for (i = 0; i < 2; i++) { - DMIBAR32(0x0908 + (i << 5)); // !!! = 0x51ffffff - DMIBAR32(0x0908 + (i << 5)) = 0x51ffffff; + /* DMIBAR D0 */ + dmi_update_field(0x0258, 1, 1, 29); + + if (IS_IVY_CPU(cpuid) && stepping >= IVB_STEP_D0) { + for (i = 0; i < 2; i++) { + /* setD0dfeidacpdgen1 */ + dmi_update_field(0x0904 + (i << 5), 0xf, 5, 25); + + /* setD0dfeidacpdgen2 */ + dmi_update_field(0x0914 + (i << 5), 0xf, 0, 13); + } }
- for (i = 0; i < 4; i++) { - DMIBAR32(0x0a00 + (i << 4)); // !!! = 0x03042018 - DMIBAR32(0x0a00 + (i << 4)) = 0x03042018; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x46139008 - DMIBAR32(0x0700 + (i << 5)) = 0x46139008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0904 + (i << 5)); // !!! = 0x3a1842ec - DMIBAR32(0x0904 + (i << 5)) = 0x3a1846ec; - } - - for (i = 0; i < 4; i++) { - DMIBAR32(0x0a00 + (i << 4)); // !!! = 0x03042018 - DMIBAR32(0x0a00 + (i << 4)) = 0x03042018; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0908 + (i << 5)); // !!! = 0x51ffffff - DMIBAR32(0x0908 + (i << 5)) = 0x51ffffff; - } - - DMIBAR32(0x0c00); // !!! = 0x29700c08 - DMIBAR32(0x0c00) = 0x29700c08; - - DMIBAR32(0x0c0c); // !!! = 0x16063400 - DMIBAR32(0x0c0c) = 0x00063400; - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x46139008 - DMIBAR32(0x0700 + (i << 5)) = 0x46339008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x46339008 - DMIBAR32(0x0700 + (i << 5)) = 0x45339008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x45339008 - DMIBAR32(0x0700 + (i << 5)) = 0x453b9008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x453b9008 - DMIBAR32(0x0700 + (i << 5)) = 0x45bb9008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0700 + (i << 5)); // !!! = 0x45bb9008 - DMIBAR32(0x0700 + (i << 5)) = 0x45fb9008; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0914 + (i << 5)); // !!! = 0x9021a080 - DMIBAR32(0x0914 + (i << 5)) = 0x9021a280; - } - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0914 + (i << 5)); // !!! = 0x9021a080 - DMIBAR32(0x0914 + (i << 5)) = 0x9821a280; - } - - for (i = 0; i < 4; i++) { - DMIBAR32(0x0a00 + (i << 4)); // !!! = 0x03042018 - DMIBAR32(0x0a00 + (i << 4)) = 0x03242018; - } - - DMIBAR32(0x0258); // !!! = 0x40000600 - DMIBAR32(0x0258) = 0x60000600; - - for (i = 0; i < 2; i++) { - DMIBAR32(0x0904 + (i << 5)); // !!! = 0x3a1846ec - DMIBAR32(0x0904 + (i << 5)) = 0x2a1846ec; - DMIBAR32(0x0914 + (i << 5)); // !!! = 0x9821a280 - DMIBAR32(0x0914 + (i << 5)) = 0x98200280; - } - - DMIBAR32(DMIL0SLAT); // !!! = 0x00c26460 - DMIBAR32(DMIL0SLAT) = 0x00c2403c; + /* Update the N_FTS values */ + dmi_update_field(DMIL0SLAT, 0xffff, 0x403c, 0);
early_pch_init_native_dmi_pre();
Hello Felix Singer, build bot (Jenkins), Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48412
to look at the new patch set (#3).
Change subject: nb/intel/sandybridge: Correct DMI PHY setup ......................................................................
nb/intel/sandybridge: Correct DMI PHY setup
The DMI PHY init sequence was written using trace dumps as a reference. While the code works, there's still room for improvement. For starters, it does not account for stepping-specific steps. It is unknown whether initializing DMI PHYs using a sequence for another stepping negatively affects system stability, but it likely does: should the DMI data link go down, the PCH disallows link retraining, which causes a system hang.
Moreover, most register writes are preceded by a dummy read, which is illogical: dummy reads to ensure a posted register write has completed need to be done after the register has been written to, and not before. What reference code is actually doing is a read-modify-write operation.
Following what reference code does, rewrite the DMI PHY setup sequence.
Tested on Asus P8Z77-V LX2, still boots with Ivy Bridge E1 CPU.
Change-Id: I2c2254a83e0ea8f8ec9d7290d88a6df1b7372d88 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 181 insertions(+), 122 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/48412/3
Attention is currently required from: Angel Pons, Evgeny Zinoviev. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48412 )
Change subject: nb/intel/sandybridge: Correct DMI PHY setup ......................................................................
Patch Set 8: Code-Review+1
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48412 )
Change subject: nb/intel/sandybridge: Correct DMI PHY setup ......................................................................
Abandoned
Unreviewable
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)