Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
nb/intel/sandybridge: Reorder IOSAV writes
We only write to the IOSAV LFSR registers twice, but we do so between the writes to the other four IOSAV per-subsequence registers. Since we know that the IOSAV is sleeping when we program the subsequences, we might as well do the two oddball LFSR register writes after we have programmed the always-written-to group of four registers. That way, subsequent changes can reproducibly replace the four writes with a single macro.
Change-Id: If7bb14a9862a53a3eba565d17401347dcc9ffbe9 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/40973/1
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index d97632e..1c84237 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -2031,16 +2031,16 @@ MCHBAR32(IOSAV_n_SUBSEQ_CTRL_ch(channel, 1)) = 0x8001020 | ((ctrl->CWL + ctrl->tWTR + 8) << 16); MCHBAR32(IOSAV_n_SP_CMD_ADDR_ch(channel, 1)) = (slotrank << 24); - MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 1)) = 0x389abcd; MCHBAR32(IOSAV_n_ADDR_UPDATE_ch(channel, 1)) = 0x20e42; + MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 1)) = 0x389abcd;
/* DRAM command RD */ MCHBAR32(IOSAV_n_SP_CMD_CTRL_ch(channel, 2)) = IOSAV_RD; MCHBAR32(IOSAV_n_SUBSEQ_CTRL_ch(channel, 2)) = 0x4001020 | (MAX(ctrl->tRTP, 8) << 16); MCHBAR32(IOSAV_n_SP_CMD_ADDR_ch(channel, 2)) = (slotrank << 24); - MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 2)) = 0x389abcd; MCHBAR32(IOSAV_n_ADDR_UPDATE_ch(channel, 2)) = 0x20e42; + MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 2)) = 0x389abcd;
/* DRAM command PRE */ MCHBAR32(IOSAV_n_SP_CMD_CTRL_ch(channel, 3)) = IOSAV_PRE;
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
Patch Set 1: Code-Review-2
Untested, blocking just in case
Hello build bot (Jenkins), Patrick Rudolph, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/40973
to look at the new patch set (#4).
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
nb/intel/sandybridge: Reorder IOSAV writes
We only write to the IOSAV LFSR registers twice, but we do so between the writes to the other four IOSAV per-subsequence registers. Since we know that the IOSAV is sleeping when we program the subsequences, we might as well do the two oddball LFSR register writes after we have programmed the always-written-to group of four registers. That way, subsequent changes can reproducibly replace the four writes with a single macro.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: If7bb14a9862a53a3eba565d17401347dcc9ffbe9 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/40973/4
Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
Removed Code-Review-2 by Angel Pons th3fanbus@gmail.com
Patrick Rudolph has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
nb/intel/sandybridge: Reorder IOSAV writes
We only write to the IOSAV LFSR registers twice, but we do so between the writes to the other four IOSAV per-subsequence registers. Since we know that the IOSAV is sleeping when we program the subsequences, we might as well do the two oddball LFSR register writes after we have programmed the always-written-to group of four registers. That way, subsequent changes can reproducibly replace the four writes with a single macro.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: If7bb14a9862a53a3eba565d17401347dcc9ffbe9 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/40973 Reviewed-by: Felix Held felix-coreboot@felixheld.de Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 2 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 34d82fc..05cffd3 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -2030,16 +2030,16 @@ MCHBAR32(IOSAV_n_SUBSEQ_CTRL_ch(channel, 1)) = 0x8001020 | ((ctrl->CWL + ctrl->tWTR + 8) << 16); MCHBAR32(IOSAV_n_SP_CMD_ADDR_ch(channel, 1)) = (slotrank << 24); - MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 1)) = 0x389abcd; MCHBAR32(IOSAV_n_ADDR_UPDATE_ch(channel, 1)) = 0x20e42; + MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 1)) = 0x389abcd;
/* DRAM command RD */ MCHBAR32(IOSAV_n_SP_CMD_CTRL_ch(channel, 2)) = IOSAV_RD; MCHBAR32(IOSAV_n_SUBSEQ_CTRL_ch(channel, 2)) = 0x4001020 | (MAX(ctrl->tRTP, 8) << 16); MCHBAR32(IOSAV_n_SP_CMD_ADDR_ch(channel, 2)) = (slotrank << 24); - MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 2)) = 0x389abcd; MCHBAR32(IOSAV_n_ADDR_UPDATE_ch(channel, 2)) = 0x20e42; + MCHBAR32(IOSAV_n_ADDRESS_LFSR_ch(channel, 2)) = 0x389abcd;
/* DRAM command PRE */ MCHBAR32(IOSAV_n_SP_CMD_CTRL_ch(channel, 3)) = IOSAV_PRE;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40973 )
Change subject: nb/intel/sandybridge: Reorder IOSAV writes ......................................................................
Patch Set 5:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/3355 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3354 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/3353 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/3352
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Angel Pons (Code Review)
-
Felix Held (Code Review)
-
Patrick Rudolph (Code Review)