Change in coreboot[master]: mb/ocp/deltalake: Override DDR frequency limit via VPD variable

[Tim Chu (Code Review)]

Tim Chu has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/1

diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c
index f0cdd3d..58ee968 100644
--- a/src/mainboard/ocp/deltalake/romstage.c
+++ b/src/mainboard/ocp/deltalake/romstage.c
@@ -75,6 +75,28 @@
    	printk(BIOS_DEBUG, "Setting MemRefreshWatermark %d from VPD\n", val);
    	mupd->FspmConfig.UnusedUpdSpace0[0] = val;
    }
+
+	/*
+	 * Select DDR Frequency Limit
+	 * 0x0:Auto, 0x5:DDR_1333, 0x7:DDR_1600, 0x9:DDR_1866, 0xb:DDR_2133, 0xd:DDR_2400,
+	 * 0xf:DDR_2666, 0x11:DDR_2933, 0x13:DDR_3200
+	 */
+	static const unsigned int limit[] = { 0x0, 0x5, 0x7, 0x9, 0xb, 0xd, 0xf, 0x11, 0x13};
+	if (vpd_gets(FSP_DIMM_FREQ, val_str, VPD_LEN, VPD_RW_THEN_RO)) {
+		val = (uint8_t)atol(val_str);
+		if (val > 8) {
+			printk(BIOS_DEBUG, "Invalid DdrFreqLimit value from VPD: "
+				"%d\n", val);
+			val = FSP_DIMM_FREQ_DEFAULT;
+		}
+		printk(BIOS_DEBUG, "Setting DdrFreqLimit %d from VPD\n", val);
+		mupd->FspmConfig.DdrFreqLimit = limit[val];
+	} else {
+		printk(BIOS_INFO, "Not able to get VPD %s, default set "
+			"DebugPrintLevel to %d\n", FSP_DIMM_FREQ,
+			FSP_DIMM_FREQ_DEFAULT);
+		mupd->FspmConfig.DdrFreqLimit = limit[FSP_DIMM_FREQ_DEFAULT];
+	}
 }

/* Update bifurcation settings according to different Configs */
diff --git a/src/mainboard/ocp/deltalake/vpd.h b/src/mainboard/ocp/deltalake/vpd.h
index 71a3b09..9246fb0 100644
--- a/src/mainboard/ocp/deltalake/vpd.h
+++ b/src/mainboard/ocp/deltalake/vpd.h
@@ -40,4 +40,8 @@
 #define FSPM_MEMREFRESHWATERMARK "fspm_mem_refresh_watermark"
 #define FSPM_MEMREFRESHWATERMARK_DEFAULT 1

+/* FSP dimm frequency limit, 0:Auto, 1:DDR_1333, 2:DDR_1600, 3:DDR_1866, 4:DDR_2133, 5:DDR_2400, 6:DDR_2666, 7:DDR_2933, 8:DDR_3200 */
+#define FSP_DIMM_FREQ "fsp_dimm_freq"
+#define FSP_DIMM_FREQ_DEFAULT 0
+
 #endif

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/1/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/vpd.h:

https://review.coreboot.org/c/coreboot/+/48946/1/src/mainboard/ocp/deltalake... 
PS1, Line 43: /* FSP dimm frequency limit, 0:Auto, 1:DDR_1333, 2:DDR_1600, 3:DDR_1866, 4:DDR_2133, 5:DDR_2400, 6:DDR_2666, 7:DDR_2933, 8:DDR_3200 */
line over 96 characters

[Tim Chu (Code Review)]

Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Johnny Lin, Morgan Jang,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#2).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 27 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/2

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 2: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/2/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/2/src/mainboard/ocp/deltalake... 
PS2, Line 85: 	if (vpd_gets(FSP_DIMM_FREQ, val_str, VPD_LEN, VPD_RW_THEN_RO)) {
Please refer to [CB:48908] and use vpd_get_int().

[Tim Chu (Code Review)]

Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#3).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/3

[Johnny Lin (Code Review)]

Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/3/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/3/src/mainboard/ocp/deltalake... 
PS3, Line 82: unsigned int
DdrFreqLimit is UINT8, I think uint8_t instead of unsigned int is better. Also static shouldn't be necessary.

[Tim Chu (Code Review)]

Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#4).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/4

[Tim Chu (Code Review)]

Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/c/coreboot/+/48946/2/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/2/src/mainboard/ocp/deltalake... 
PS2, Line 85: 	if (vpd_gets(FSP_DIMM_FREQ, val_str, VPD_LEN, VPD_RW_THEN_RO)) {
> Please refer to [CB:48908] and use vpd_get_int().

Done

https://review.coreboot.org/c/coreboot/+/48946/3/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/3/src/mainboard/ocp/deltalake... 
PS3, Line 82: unsigned int
> DdrFreqLimit is UINT8, I think uint8_t instead of unsigned int is better. […]

Done

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 4: Code-Review+2

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 4: Code-Review+1

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/4/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/4/src/mainboard/ocp/deltalake... 
PS4, Line 84: 8
ARRAY_SIZE(limit)

[Tim Chu (Code Review)]

Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Arthur Heymans, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#5).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/5

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/5/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/5/src/mainboard/ocp/deltalake... 
PS5, Line 84: 		if (val_int > ARRAY_SIZE(limit) -1) {
need consistent spacing around '-' (ctx:WxV)

[Tim Chu (Code Review)]

Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48946/4/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/4/src/mainboard/ocp/deltalake... 
PS4, Line 84: 8
> ARRAY_SIZE(limit)

Done

[Tim Chu (Code Review)]

Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Arthur Heymans, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#6).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/6

[Jonathan Zhang (Code Review)]

Attention is currently required from: Tim Chu.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 6: Code-Review+1

(1 comment)

Patchset:

PS6: 
This PR itself looks good to me.

That being said, the FspmUpd.h file has wrong comments (auto generated during FSP build process). Tim, please open an IPS ticket to track it:
/** Offset 0x00C1 - Processor X2apic Function
  Enable(Default) or Disable Processor X2apic Function
  $EN_DIS
**/
  UINT8                       X2apic;

/** Offset 0x00C2 - DDR frequency limit
  Enable(Default) or Disable Processor X2apic Function
**/
  UINT8                       DdrFreqLimit;

/** Offset 0x00C3 - Memory Serial Debug Message Level
  Enable(Default) or Disable Processor X2apic Function
**/
  UINT8                       serialDebugMsgLvl;

[Tim Chu (Code Review)]

Attention is currently required from: Tim Chu.
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Arthur Heymans, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#7).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
2 files changed, 26 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/7

[Paul Menzel (Code Review)]

Attention is currently required from: Tim Chu.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 7:

(5 comments)

File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/comment/4ff498dd_cc5db42c 
PS7, Line 84: 	 * 0xf:DDR_2666, 0x11:DDR_2933, 0x13:DDR_3200
Please use macros or enums.

https://review.coreboot.org/c/coreboot/+/48946/comment/38f1cf3e_0cf1777f 
PS7, Line 88: > ARRAY_SIZE(limit) - 1
Use >= instead of - 1?

https://review.coreboot.org/c/coreboot/+/48946/comment/17e77788_cd3763fe 
PS7, Line 89: BIOS_DEBUG
This should be at least a warning, even an error?

https://review.coreboot.org/c/coreboot/+/48946/comment/a33874f2_59710864 
PS7, Line 93: BIOS_DEBUG
Maybe: BIOS_INFO

https://review.coreboot.org/c/coreboot/+/48946/comment/3dbefa31_c63eec3e 
PS7, Line 96: BIOS_INFO
Warning level?

[Paul Menzel (Code Review)]

Attention is currently required from: Jonathan Zhang, Tim Chu.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 7:

(1 comment)

Patchset:

PS6:
> This PR itself looks good to me. […]

(You can format code blocks with (extended) Markdown (indent with four spaces or enclose in ``` ```.)

[Tim Chu (Code Review)]

Attention is currently required from: Jonathan Zhang, Tim Chu.
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Arthur Heymans, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#8).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
M src/soc/intel/xeon_sp/cpx/chip.h
3 files changed, 60 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/8

[build bot (Jenkins) (Code Review)]

Attention is currently required from: Jonathan Zhang.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 8:

(4 comments)

File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/comment/52b63074_9ce11c6d 
PS8, Line 82: 	ddr_freq_limit DdrFreqLimit(int num)
open brace '{' following function definitions go on the next line

https://review.coreboot.org/c/coreboot/+/48946/comment/3af15351_5861302c 
PS8, Line 104: 			printk(BIOS_WARNING, "Invalid DdrFreqLimit value from VPD: "
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

https://review.coreboot.org/c/coreboot/+/48946/comment/f62034c5_7540b19c 
PS8, Line 112: 		printk(BIOS_INFO, "Setting DdrFreqLimit %d from VPD\n", val_int);
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

https://review.coreboot.org/c/coreboot/+/48946/comment/2022e602_40f28532 
PS8, Line 116: 			"DdrFreqLimit to %d\n", FSP_DIMM_FREQ,
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

[Tim Chu (Code Review)]

Attention is currently required from: Jonathan Zhang.
Hello build bot (Jenkins), Anjaneya "Reddy" Chagam, Jonathan Zhang, Ryback Hung, Johnny Lin, Angel Pons, Arthur Heymans, Morgan Jang, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/48946

to look at the new patch set (#9).

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
M src/soc/intel/xeon_sp/cpx/chip.h
3 files changed, 59 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/48946/9

[build bot (Jenkins) (Code Review)]

Attention is currently required from: Jonathan Zhang.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 9:

(4 comments)

File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/comment/d0cfaf8f_44d40b02 
PS9, Line 82: 	ddr_freq_limit DdrFreqLimit(int num) {
open brace '{' following function definitions go on the next line

https://review.coreboot.org/c/coreboot/+/48946/comment/80dd059c_2c590db0 
PS9, Line 103: 			printk(BIOS_WARNING, "Invalid DdrFreqLimit value from VPD: "
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

https://review.coreboot.org/c/coreboot/+/48946/comment/17de4c40_6983db1e 
PS9, Line 111: 		printk(BIOS_INFO, "Setting DdrFreqLimit %d from VPD\n", val_int);
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

https://review.coreboot.org/c/coreboot/+/48946/comment/b63c448b_0cd3f20b 
PS9, Line 115: 			"DdrFreqLimit to %d\n", FSP_DIMM_FREQ,
Prefer using '"%s...", __func__' to using 'DdrFreqLimit', this function's name, in a string

[Tim Chu (Code Review)]

Attention is currently required from: Paul Menzel.
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 10:

(5 comments)

This change is ready for review.

File src/mainboard/ocp/deltalake/romstage.c:

https://review.coreboot.org/c/coreboot/+/48946/comment/9b8f477d_1bf76713 
PS7, Line 84: 	 * 0xf:DDR_2666, 0x11:DDR_2933, 0x13:DDR_3200
> Please use macros or enums.

Done

https://review.coreboot.org/c/coreboot/+/48946/comment/4a4eae23_07e5ce9d 
PS7, Line 88: > ARRAY_SIZE(limit) - 1
> Use >= instead of - 1?

Use switch-case to replace.

https://review.coreboot.org/c/coreboot/+/48946/comment/aca148d3_2fafc66b 
PS7, Line 89: BIOS_DEBUG
> This should be at least a warning, even an error?

Done

https://review.coreboot.org/c/coreboot/+/48946/comment/d9ed975c_60af6367 
PS7, Line 93: BIOS_DEBUG
> Maybe: BIOS_INFO

Done

https://review.coreboot.org/c/coreboot/+/48946/comment/56d4a927_4ad04afb 
PS7, Line 96: BIOS_INFO
> Warning level?

Done

[Jonathan Zhang (Code Review)]

Attention is currently required from: Paul Menzel, Tim Chu.
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

Patch Set 10: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48946 )

Change subject: mb/ocp/deltalake: Override DDR frequency limit via VPD variable
......................................................................

mb/ocp/deltalake: Override DDR frequency limit via VPD variable

Use VPD variable "fsp_dimm_freq" to select DDR frequency limit.

Tested=On OCP Delta Lake, DDR frequency limit can be changed via VPD.

Signed-off-by: Tim Chu 
Change-Id: I1232feae5090420d8fa42596b46f2d4dcaf9d635
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48946
Tested-by: build bot (Jenkins) 
Reviewed-by: Jonathan Zhang 
---
M src/mainboard/ocp/deltalake/romstage.c
M src/mainboard/ocp/deltalake/vpd.h
M src/soc/intel/xeon_sp/cpx/chip.h
3 files changed, 60 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Jonathan Zhang: Looks good to me, approved

diff --git a/src/mainboard/ocp/deltalake/romstage.c b/src/mainboard/ocp/deltalake/romstage.c
index 4272e3a..9227508 100644
--- a/src/mainboard/ocp/deltalake/romstage.c
+++ b/src/mainboard/ocp/deltalake/romstage.c
@@ -13,6 +13,35 @@
 #include "ipmi.h"
 #include "vpd.h"

+/* Convert the vpd integer to the DDR frenquency limit enum */
+static enum ddr_freq_limit ddr_freq_limit(int num)
+{
+	switch (num) {
+	case 0:
+		return DDR_AUTO;
+	case 1:
+		return DDR_1333;
+	case 2:
+		return DDR_1600;
+	case 3:
+		return DDR_1866;
+	case 4:
+		return DDR_2133;
+	case 5:
+		return DDR_2400;
+	case 6:
+		return DDR_2666;
+	case 7:
+		return DDR_2933;
+	case 8:
+		return DDR_3200;
+	default:
+		printk(BIOS_WARNING, "Invalid DdrFreqLimit value from VPD: "
+			"%d\n", num);
+		return DDR_AUTO;
+	};
+}
+
 /*
  * Search from VPD_RW first then VPD_RO for UPD config variables,
  * overwrites them from VPD if it's found.
@@ -77,6 +106,17 @@
    	printk(BIOS_DEBUG, "Setting MemRefreshWatermark %d from VPD\n", val_int);
    	mupd->FspmConfig.UnusedUpdSpace0[0] = (uint8_t)val_int;
    }
+
+	/* Select DDR Frequency Limit */
+	if (vpd_get_int(FSP_DIMM_FREQ, VPD_RW_THEN_RO, (int *const) &val_int)) {
+		printk(BIOS_INFO, "Setting DdrFreqLimit %d from VPD\n", val_int);
+		mupd->FspmConfig.DdrFreqLimit = ddr_freq_limit(val_int);
+	} else {
+		printk(BIOS_WARNING, "Not able to get VPD %s, default set "
+			"DdrFreqLimit to %d\n", FSP_DIMM_FREQ,
+			FSP_DIMM_FREQ_DEFAULT);
+		mupd->FspmConfig.DdrFreqLimit = ddr_freq_limit(FSP_DIMM_FREQ_DEFAULT);
+	}
 }

/* Update bifurcation settings according to different Configs */
diff --git a/src/mainboard/ocp/deltalake/vpd.h b/src/mainboard/ocp/deltalake/vpd.h
index 55cbc5e..0b5a3e1 100644
--- a/src/mainboard/ocp/deltalake/vpd.h
+++ b/src/mainboard/ocp/deltalake/vpd.h
@@ -41,4 +41,9 @@
 #define COREBOOT_UART_IO "coreboot_uart_io"
 #define COREBOOT_UART_IO_DEFAULT 1

+/* FSP dimm frequency limit, 0:Auto, 1:DDR_1333, 2:DDR_1600, 3:DDR_1866, 4:DDR_2133,
+ * 5:DDR_2400, 6:DDR_2666, 7:DDR_2933, 8:DDR_3200 */
+#define FSP_DIMM_FREQ "fsp_dimm_freq"
+#define FSP_DIMM_FREQ_DEFAULT 0
+
 #endif
diff --git a/src/soc/intel/xeon_sp/cpx/chip.h b/src/soc/intel/xeon_sp/cpx/chip.h
index 1311678..abd358e 100644
--- a/src/soc/intel/xeon_sp/cpx/chip.h
+++ b/src/soc/intel/xeon_sp/cpx/chip.h
@@ -31,6 +31,21 @@
    PcieGen3
 } pcie_link_speed;

+/**
+ enum for DDR Frequency Limit
+ **/
+enum ddr_freq_limit {
+	DDR_AUTO = 0x0,
+	DDR_1333 = 0x5,
+	DDR_1600 = 0x7,
+	DDR_1866 = 0x9,
+	DDR_2133 = 0xb,
+	DDR_2400 = 0xd,
+	DDR_2666 = 0xf,
+	DDR_2933 = 0x11,
+	DDR_3200 = 0x13
+};
+
 struct soc_intel_xeon_sp_cpx_config {
    /* Common struct containing soc config data required by common code */
    struct soc_intel_common_config common_soc_config;