Change in coreboot[master]: soc/intel/tigerlake: Disable MrcSafeConfig

[Srinidhi N Kaushik (Code Review)]

Srinidhi N Kaushik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

soc/intel/tigerlake: Disable MrcSafeConfig

This change disables MrcSafeConfig option during MRC training.
FSP 2527 and later versions support running MRC without this
option hence disabling it.

BUG=b:150357377
BRANCH=master
TEST=build and boot ripto/volteer

Signed-off-by: Srinidhi N Kaushik 
Change-Id: I4e4069d83754aaf1e4885d6912ab2a6d506c5269
---
M src/soc/intel/tigerlake/meminit.c
1 file changed, 0 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/40106/1

diff --git a/src/soc/intel/tigerlake/meminit.c b/src/soc/intel/tigerlake/meminit.c
index 864f079..ec7aed7 100644
--- a/src/soc/intel/tigerlake/meminit.c
+++ b/src/soc/intel/tigerlake/meminit.c
@@ -231,7 +231,6 @@
    /* LPDDR4x does not allow interleaved memory */
    mem_cfg->DqPinsInterleaved = 0;
    mem_cfg->ECT = board_cfg->ect;
-	mem_cfg->MrcSafeConfig = 0x1;

read_md_spd(info, &spd_data, &spd_len);
    mem_cfg->MemorySpdDataLen = spd_len;

[Srinidhi N Kaushik (Code Review)]

Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 1: Code-Review+1

[Nick Vaccaro (Code Review)]

Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 1: Code-Review-1

Sounds like we will need to wait to merge this until the FSP uprev (b:150357377) occurs.  Setting to -1 CR so that it doesn't merge prematurely.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG@11 
PS1, Line 11: option hence disabling it.
Please add the motivation for disabling this. Why should the config not be saved anymore? What is the effect? Faster boot time?

[Srinidhi N Kaushik (Code Review)]

Srinidhi N Kaushik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG@11 
PS1, Line 11: option hence disabling it.
> Please add the motivation for disabling this. […]

Running with MrcSafeConfig disabled is the end goal. This config was enabled part of the early testing. Now with FSP 2527 we can run with full config.

[Wonkyu Kim (Code Review)]

Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 1: Code-Review+1

[Dossym Nurmukhanov (Code Review)]

Dossym Nurmukhanov has uploaded a new patch set (#3) to the change originally created by Srinidhi N Kaushik. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

soc/intel/tigerlake: Disable MrcSafeConfig

This change disables MrcSafeConfig option during MRC training.
MrcSafeConfig was enabled as part of the early testing.
Now with FSP 2527, there is no need to set this config anymore.

BUG=b:150357377
BRANCH=master
TEST=build and boot ripto/volteer

Signed-off-by: Srinidhi N Kaushik 
Change-Id: I4e4069d83754aaf1e4885d6912ab2a6d506c5269
---
M src/soc/intel/tigerlake/meminit.c
1 file changed, 0 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/40106/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 3: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40106/1//COMMIT_MSG@11 
PS1, Line 11: option hence disabling it.
> Running with MrcSafeConfig disabled is the end goal. […]

Done

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 3:
> Patch Set 1: Code-Review-1
>
> Sounds like we will need to wait to merge this until the FSP uprev (b:150357377) occurs.  Setting to -1 CR so that it doesn't merge prematurely.

Change is ready to be merged. All other pieces are ready. So going ahead and submitting this.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

soc/intel/tigerlake: Disable MrcSafeConfig

This change disables MrcSafeConfig option during MRC training.
MrcSafeConfig was enabled as part of the early testing.
Now with FSP 2527, there is no need to set this config anymore.

BUG=b:150357377
BRANCH=master
TEST=build and boot ripto/volteer

Signed-off-by: Srinidhi N Kaushik 
Change-Id: I4e4069d83754aaf1e4885d6912ab2a6d506c5269
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40106
Reviewed-by: Furquan Shaikh 
Reviewed-by: Wonkyu Kim 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/tigerlake/meminit.c
1 file changed, 0 insertions(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Srinidhi N Kaushik: Looks good to me, but someone else must approve
  Wonkyu Kim: Looks good to me, but someone else must approve

Objections:
  Nick Vaccaro: I would prefer that you didn't submit this

diff --git a/src/soc/intel/tigerlake/meminit.c b/src/soc/intel/tigerlake/meminit.c
index 7823cfe..8664547 100644
--- a/src/soc/intel/tigerlake/meminit.c
+++ b/src/soc/intel/tigerlake/meminit.c
@@ -239,7 +239,6 @@
    /* LPDDR4x does not allow interleaved memory */
    mem_cfg->DqPinsInterleaved = 0;
    mem_cfg->ECT = board_cfg->ect;
-	mem_cfg->MrcSafeConfig = 0x1;

read_md_spd(info, &spd_data, &spd_len);
    mem_cfg->MemorySpdDataLen = spd_len;

[9elements QA (Code Review)]

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40106 )

Change subject: soc/intel/tigerlake: Disable MrcSafeConfig
......................................................................

Patch Set 4:

Automatic boot test returned (PASS/FAIL/TOTAL): 3/0/3
Emulation targets:
EMULATION_QEMU_X86_Q35 using payload TianoCore : SUCCESS : https://lava.9esec.io/r/2274
EMULATION_QEMU_X86_Q35 using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2273
EMULATION_QEMU_X86_I440FX using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/2272

Please note: This test is under development and might not be accurate at all!