Change in coreboot[master]: mb/ocp/deltalake: Override SMBIOS type 4 max speed

[Tim Chu (Code Review)]

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

mb/ocp/deltalake: Override SMBIOS type 4 max speed

Override SMBIOS type 4 max speed. This field should be maximum speed
supported by the system. 4000MHz is expected for Delta Lake.

Tested=Execute "dmidecode -t 4" to check max speed is correct.

Signed-off-by: Tim Chu 
Change-Id: I67edf657a2fe66b38e08056d558e1b360c4b8adc
---
M src/mainboard/ocp/deltalake/ramstage.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/48640/1

diff --git a/src/mainboard/ocp/deltalake/ramstage.c b/src/mainboard/ocp/deltalake/ramstage.c
index 72a74da..64229fe 100644
--- a/src/mainboard/ocp/deltalake/ramstage.c
+++ b/src/mainboard/ocp/deltalake/ramstage.c
@@ -96,6 +96,12 @@
    return 0x10;
 }

+unsigned int smbios_cpu_get_max_speed_mhz(void)
+{
+	/* This will return 4000MHz */
+	return 0xfa0;
+}
+
 /* System Slot Socket, Stack, Type and Data bus width Information */
 typedef struct {
    u8 stack;

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/48640/1/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/ramstage.c:

https://review.coreboot.org/c/coreboot/+/48640/1/src/mainboard/ocp/deltalake... 
PS1, Line 101: /* This will return 4000MHz */
             : 	return 0xfa0;
return 4000;

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

Patch Set 1:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/48640/1//COMMIT_MSG@10 
PS1, Line 10: 4000MHz
Are there CooperLake CPUs with a faster speed? If not, I would move this into xeon_sp code

[Tim Chu (Code Review)]

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

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

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

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

mb/ocp/deltalake: Override SMBIOS type 4 max speed

Override SMBIOS type 4 max speed. This field should be maximum speed
supported by the system. 4000MHz is expected for Delta Lake.

Tested=Execute "dmidecode -t 4" to check max speed is correct.

Signed-off-by: Tim Chu 
Change-Id: I67edf657a2fe66b38e08056d558e1b360c4b8adc
---
M src/mainboard/ocp/deltalake/ramstage.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/48640/2

[Tim Chu (Code Review)]

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

Patch Set 2:

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/48640/1//COMMIT_MSG@10 
PS1, Line 10: 4000MHz
> Are there CooperLake CPUs with a faster speed? If not, I would move this into xeon_sp code

Yes, there're CooperLake CPUs with faster speed.

https://review.coreboot.org/c/coreboot/+/48640/1/src/mainboard/ocp/deltalake... 
File src/mainboard/ocp/deltalake/ramstage.c:

https://review.coreboot.org/c/coreboot/+/48640/1/src/mainboard/ocp/deltalake... 
PS1, Line 101: /* This will return 4000MHz */
             : 	return 0xfa0;
> return 4000;

Done

[Patrick Rudolph (Code Review)]

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

Patch Set 2: Code-Review+1

(2 comments)

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

https://review.coreboot.org/c/coreboot/+/48640/comment/7839c01a_a4226842 
PS2, Line 99: smbios_cpu_get_max_speed_mhz
why is this mainboard specific? Shouldn't this part of the SoC as all boards selecting the SoC code support the same sockets/CPUs and thus the max speed is also the same?

https://review.coreboot.org/c/coreboot/+/48640/comment/80cef5af_2cef0ed0 
PS2, Line 102: 	return 4000;
which datasheet is this value from?

[Angel Pons (Code Review)]

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

Patch Set 2:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/48640/comment/170faea8_095eba40 
PS2, Line 101: /* This will return 4000MHz */
Please drop this comment. The function name already says the `4000` is in MHz.

[Tim Chu (Code Review)]

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

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

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

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

Change subject: mb/ocp/deltalake: Override SMBIOS type 4 max speed
......................................................................

mb/ocp/deltalake: Override SMBIOS type 4 max speed

Override SMBIOS type 4 max speed. This field should be maximum speed
supported by the system. 3900MHz is expected for Cooper Lake.

Tested=Execute "dmidecode -t 4" to check max speed is correct.

Signed-off-by: Tim Chu 
Change-Id: I67edf657a2fe66b38e08056d558e1b360c4b8adc
---
M src/soc/intel/xeon_sp/cpx/ramstage.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/48640/3

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Tim Chu (Code Review)]

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

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

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

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

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

Override SMBIOS type 4 max speed. This field should be maximum speed
supported by the system. 3900MHz is expected for Cooper Lake.

Tested=Execute "dmidecode -t 4" to check max speed is correct.

Signed-off-by: Tim Chu 
Change-Id: I67edf657a2fe66b38e08056d558e1b360c4b8adc
---
M src/soc/intel/xeon_sp/cpx/ramstage.c
1 file changed, 6 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/40/48640/4

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Tim Chu (Code Review)]

Attention is currently required from: Patrick Rudolph, Angel Pons.
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

Patch Set 4:

(3 comments)

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

https://review.coreboot.org/c/coreboot/+/48640/comment/46c98ec0_d3111a24 
PS2, Line 99: smbios_cpu_get_max_speed_mhz
> why is this mainboard specific? Shouldn't this part of the SoC as all boards selecting the SoC code  […]

Yes, you are right. I move this function to soc/intel/xeon_sp/ramstage.c. Thanks.

https://review.coreboot.org/c/coreboot/+/48640/comment/ff8b1d03_8caa0892 
PS2, Line 101: /* This will return 4000MHz */
> Please drop this comment. The function name already says the `4000` is in MHz.

Done

https://review.coreboot.org/c/coreboot/+/48640/comment/c987d670_a7aa70aa 
PS2, Line 102: 	return 4000;
> which datasheet is this value from?

After check again, I update the correct value. This value should be the max non-turbo freq. Can refer to Intel website for products formerly list of Cooper Lake.

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Angel Pons (Code Review)]

Attention is currently required from: Patrick Rudolph, Tim Chu.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

Patch Set 4: Code-Review+1

(1 comment)

File src/soc/intel/xeon_sp/cpx/ramstage.c:

https://review.coreboot.org/c/coreboot/+/48640/comment/ea1b5726_949cd456 
PS4, Line 13: 3900
This is the max non-turbo ratio, right? Should we read it from a MSR, or is this the maximum non-turbo ratio that CPX processors can have?

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Tim Chu (Code Review)]

Attention is currently required from: Patrick Rudolph, Angel Pons.
Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

Patch Set 4:

(1 comment)

File src/soc/intel/xeon_sp/cpx/ramstage.c:

https://review.coreboot.org/c/coreboot/+/48640/comment/f48ec5ee_698c6df3 
PS4, Line 13: 3900
> This is the max non-turbo ratio, right? Should we read it from a MSR, or is this the maximum non-tur […]

Yes. This is the max non-turbo ratio CPX processors can have.

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Patrick Rudolph (Code Review)]

Attention is currently required from: Patrick Rudolph, Angel Pons, Tim Chu.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

Patch Set 4:

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/48640/comment/6508b5ae_824c1a56 
PS2, Line 102: 	return 4000;
> After check again, I update the correct value. This value should be the max non-turbo freq. […]

confirmed Intel® Xeon® Platinum 8356H Processor is the fastest

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Angel Pons (Code Review)]

Attention is currently required from: Patrick Rudolph, Tim Chu.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48640 )

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

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

https://review.coreboot.org/c/coreboot/+/48640/comment/de6e7a33_2ee8dbfb 
PS2, Line 102: 	return 4000;
> confirmed Intel® Xeon® Platinum 8356H Processor is the fastest

Ack

Change in coreboot[master]: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

[Patrick Georgi (Code Review)]

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

Change subject: soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed
......................................................................

soc/intel/xeon_sp/cpx: Override SMBIOS type 4 max speed

Override SMBIOS type 4 max speed. This field should be maximum speed
supported by the system. 3900MHz is expected for Cooper Lake.

Tested=Execute "dmidecode -t 4" to check max speed is correct.

Signed-off-by: Tim Chu 
Change-Id: I67edf657a2fe66b38e08056d558e1b360c4b8adc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48640
Tested-by: build bot (Jenkins) 
Reviewed-by: Angel Pons 
---
M src/soc/intel/xeon_sp/cpx/ramstage.c
1 file changed, 6 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/soc/intel/xeon_sp/cpx/ramstage.c b/src/soc/intel/xeon_sp/cpx/ramstage.c
index deb9030..1e0ba00 100644
--- a/src/soc/intel/xeon_sp/cpx/ramstage.c
+++ b/src/soc/intel/xeon_sp/cpx/ramstage.c
@@ -1,8 +1,14 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <fsp/api.h>
+#include <smbios.h>

int soc_fsp_multi_phase_init_is_enable(void)
 {
    return 0;
 }
+
+unsigned int smbios_cpu_get_max_speed_mhz(void)
+{
+	return 3900;
+}