Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45500 )
Change subject: nb/intel/sandybridge: Improve channel disable logic
......................................................................
nb/intel/sandybridge: Improve channel disable logic
Do not consider the failed channel's SPDs in emergency mode. Also, force
a full training if not resuming from S3 and the saved data indicates a
channel has failed. Special care needs to be taken as the selected
memory frequency could be raised after a channel has been disabled.
Tested on Asus P8Z77-V LX2, with a good DIMM on channel 0 and a bad DIMM
on channel 1. The failing channel causes errors when discovering timB,
and emergency mode raminit for the other channel is good enough to boot.
With this commit:
a. Regular boots always result in full training.
b. The board still boots, as emergency raminit trained the good DIMM.
c. S3 resume still works in all tested configurations.
d. If pretending the bad DIMM is slower:
1. The MPLL gets locked at the slow frequency of the bad DIMM.
2. The first raminit attempt still fails miserably on the bad DIMM.
3. Emergency mode uses the frequency the MPLL is locked at.
4. The board boots with the good DIMM running at the slow frequency.
With good DIMMs on both channels, everything works the same as before.
Change-Id: I3d34594561680906cb0b15a9c6a5fa7a773c0496
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M src/northbridge/intel/sandybridge/raminit.c
M src/northbridge/intel/sandybridge/raminit_common.h
M src/northbridge/intel/sandybridge/raminit_native.c
3 files changed, 38 insertions(+), 14 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/00/45500/1
diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c
index 319fea3..51c57c6 100644
--- a/src/northbridge/intel/sandybridge/raminit.c
+++ b/src/northbridge/intel/sandybridge/raminit.c
@@ -43,15 +43,7 @@
/* Disable a channel in ramctr_timing */
static void disable_channel(ramctr_timing *ctrl, int channel)
{
- ctrl->rankmap[channel] = 0;
-
- memset(&ctrl->rank_mirror[channel][0], 0, sizeof(ctrl->rank_mirror[0]));
-
- ctrl->channel_size_mb[channel] = 0;
- ctrl->cmd_stretch[channel] = 0;
- ctrl->mad_dimm[channel] = 0;
- memset(&ctrl->timings[channel][0], 0, sizeof(ctrl->timings[0]));
- memset(&ctrl->info.dimm[channel][0], 0, sizeof(ctrl->info.dimm[0]));
+ ctrl->failed_channels |= BIT(channel);
}
static bool nb_supports_ecc(const uint32_t capid0_a)
@@ -163,6 +155,10 @@
FOR_ALL_CHANNELS {
ctrl->channel_size_mb[channel] = 0;
+ /* Skip decoding the SPDs of failed channels */
+ if (ctrl->failed_channels & BIT(channel))
+ continue;
+
ch_dimms = 0;
/* Count dimms on channel */
for (slot = 0; slot < NUM_SLOTS; slot++) {
@@ -345,8 +341,21 @@
system_reset();
}
+ /*
+ * Discard the saved data if it is from raminit emergency mode.
+ * Ideally, we would not advertise support for ACPI S3 in such
+ * cases, because there is evidence that the RAM has stability
+ * problems. Until then, skip this sanity check on S3 resumes.
+ */
+ if (!s3resume && ctrl_cached && ctrl_cached->failed_channels) {
+ printk(BIOS_ERR, "Stored timings indicate channels have failed.\n");
+
+ ctrl_cached = NULL;
+ }
+
/* Verify MRC cache for fast boot */
if (!s3resume && ctrl_cached) {
+
/* Load SPD unique information data. */
memset(spds, 0, sizeof(spds));
mainboard_get_spd(spds, 1);
@@ -391,6 +400,13 @@
}
if (err) {
+ /*
+ * We can't reinitialize the DRAM MPLL at a different frequency.
+ * As we want to ignore the SPDs on the failed channel, we have
+ * to use the same tCK in emergency mode to avoid some troubles.
+ */
+ const u32 old_tCK = ctrl.tCK;
+
/* Fallback: disable failing channel */
printk(BIOS_ERR, "RAM training failed, trying fallback.\n");
printram("Disable failing channel.\n");
@@ -398,12 +414,15 @@
/* Reset internal state */
reinit_ctrl(&ctrl, cpuid);
- /* Reset DDR3 frequency */
- dram_find_spds_ddr3(spds, &ctrl);
-
/* Disable failing channel */
disable_channel(&ctrl, GET_ERR_CHANNEL(err));
+ /* Restore tCK */
+ ctrl.tCK = old_tCK;
+
+ /* Recalculate DDR3 frequency */
+ dram_find_spds_ddr3(spds, &ctrl);
+
err = try_init_dram_ddr3(&ctrl, fast_boot, s3resume, me_uma_size);
}
diff --git a/src/northbridge/intel/sandybridge/raminit_common.h b/src/northbridge/intel/sandybridge/raminit_common.h
index 32f2b44..cf3d6a9 100644
--- a/src/northbridge/intel/sandybridge/raminit_common.h
+++ b/src/northbridge/intel/sandybridge/raminit_common.h
@@ -105,7 +105,7 @@
/*
* WARNING: Do not forget to increase MRC_CACHE_VERSION when the saved data is changed!
*/
-#define MRC_CACHE_VERSION 5
+#define MRC_CACHE_VERSION 6
typedef struct odtmap_st {
u16 rttwr;
@@ -185,6 +185,9 @@
/* Bits [0..11] of PM_DLL_CONFIG: Master DLL wakeup delay timer */
u16 mdll_wake_delay;
+ /* Bitmap of channels that have failed */
+ u8 failed_channels;
+
u8 rankmap[NUM_CHANNELS];
int ref_card_offset[NUM_CHANNELS];
u32 mad_dimm[NUM_CHANNELS];
diff --git a/src/northbridge/intel/sandybridge/raminit_native.c b/src/northbridge/intel/sandybridge/raminit_native.c
index c23a5ac..a469910 100644
--- a/src/northbridge/intel/sandybridge/raminit_native.c
+++ b/src/northbridge/intel/sandybridge/raminit_native.c
@@ -265,7 +265,9 @@
printk(BIOS_DEBUG, "PLL_REF100_CFG value: 0x%x\n", ref_100mhz_support);
- ctrl->tCK = get_mem_min_tck();
+ /* In emergency mode, use provided tCK if non-zero. Otherwise, calculate it. */
+ if (!ctrl->failed_channels || !ctrl->tCK)
+ ctrl->tCK = get_mem_min_tck();
/* Find CAS latency */
while (1) {
--
To view, visit https://review.coreboot.org/c/coreboot/+/45500
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I3d34594561680906cb0b15a9c6a5fa7a773c0496
Gerrit-Change-Number: 45500
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-MessageType: newchange
Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/libgfxinit/+/44786 )
Change subject: gma registers: Allow to specify an offset for display registers
......................................................................
gma registers: Allow to specify an offset for display registers
Certain platforms, namely Bay Trail and Braswell, have the display
engine registers at an offset relative to GTTMMADR base. Apart from
that, the registers are rather similar to the ones on GMCH platforms.
Allow platforms to specify at which offset within GTTMM the display
registers are located. Use zero for all currently-supported platforms.
Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M common/hw-gfx-gma-config.ads.template
M common/hw-gfx-gma-registers.adb
2 files changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/libgfxinit refs/changes/86/44786/1
diff --git a/common/hw-gfx-gma-config.ads.template b/common/hw-gfx-gma-config.ads.template
index 241fe8e..ecbd908 100644
--- a/common/hw-gfx-gma-config.ads.template
+++ b/common/hw-gfx-gma-config.ads.template
@@ -55,6 +55,8 @@
Default_MMIO_Base : constant := <<DEFAULT_MMIO_BASE>>;
+ Display_Base_Offset : constant := 0;
+
LVDS_Dual_Threshold : constant := 95_000_000;
Ignore_Presence_Straps : constant Boolean := <<IGNORE_STRAPS>>;
diff --git a/common/hw-gfx-gma-registers.adb b/common/hw-gfx-gma-registers.adb
index 3f0d7ae..5ea134a 100644
--- a/common/hw-gfx-gma-registers.adb
+++ b/common/hw-gfx-gma-registers.adb
@@ -211,7 +211,7 @@
SPARK_Mode => Off
is
begin
- return Reg'Enum_Rep;
+ return Reg'Enum_Rep + Config.Display_Base_Offset;
end Index;
end Rep;
--
To view, visit https://review.coreboot.org/c/libgfxinit/+/44786
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: libgfxinit
Gerrit-Branch: master
Gerrit-Change-Id: Id5497a23776e9aa9e2fd5ca0479030cb8e55712f
Gerrit-Change-Number: 44786
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newchange