Rob Barnes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
soc/picasso: Set max_speed_mts and configured_speed_mts
ddr_frequency is deprecated. Set max_speed_mts and configured_speed_mts instead. This will result in SMBIOS type 17 displaying more accurate speed information.
BUG=b:167218112 TEST=Boot ezkinil and observe dmidecode -t17 dmidecode -t17 # dmidecode 3.2 Getting SMBIOS data from sysfs. SMBIOS 3.0 present.
Handle 0x000B, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 64 bits Data Width: 64 bits Size: 4096 MB Form Factor: SODIMM Set: None Locator: Channel-0-DIMM-0 Bank Locator: BANK 0 Type: DDR4 Type Detail: Synchronous Speed: 3200 MT/s Manufacturer: Unknown (0) Serial Number: 00000000 Asset Tag: Not Specified Part Number: MT40A512M16TB-062E:J Rank: 1 Configured Memory Speed: 2400 MT/s Minimum Voltage: Unknown Maximum Voltage: Unknown Configured Voltage: Unknown
Signed-off-by: Rob Barnes robbarnes@google.com Change-Id: I1879676ea9436b6d19c768f1b78487a4e179f8d6 --- M src/soc/amd/picasso/dmi.c 1 file changed, 5 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/44984/1
diff --git a/src/soc/amd/picasso/dmi.c b/src/soc/amd/picasso/dmi.c index 3988616..5189cd6 100644 --- a/src/soc/amd/picasso/dmi.c +++ b/src/soc/amd/picasso/dmi.c @@ -28,14 +28,12 @@ dimm->ddr_type = dmi17->MemoryType;
/** - * Based on the name, ddr_frequency should hold the memory clock - * frequency in MHz. However it is interpreted as MT/s in SMBIOS - * downstream. So multiply by 2 to translate to memory speed in MT/s. - * ddr_frequency is used for setting both config speed and max - * speed. Using config speed so we don't get the false impression - * that the RAM is running faster than it actually is. + * TYPE17_DMI_INFO holds speed in MHz. + * Change to MT/s by multiplying by 2. */ - dimm->ddr_frequency = 2 * dmi17->ConfigSpeed; + dimm->configured_speed_mts = 2 * dmi17->ConfigSpeed; + + dimm->max_speed_mts = 2 * dmi17->Speed;
dimm->rank_per_dimm = dmi17->Attributes;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44984/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44984/1//COMMIT_MSG@7 PS1, Line 7: soc/picasso nit: soc/amd/picasso:
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
Patch Set 1: Code-Review+2
Hello Raul Rangel, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44984
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
soc/amd/picasso: Set max_speed_mts and configured_speed_mts
ddr_frequency is deprecated. Set max_speed_mts and configured_speed_mts instead. This will result in SMBIOS type 17 displaying more accurate speed information.
BUG=b:167218112 TEST=Boot ezkinil and observe dmidecode -t17 dmidecode -t17 # dmidecode 3.2 Getting SMBIOS data from sysfs. SMBIOS 3.0 present.
Handle 0x000B, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 64 bits Data Width: 64 bits Size: 4096 MB Form Factor: SODIMM Set: None Locator: Channel-0-DIMM-0 Bank Locator: BANK 0 Type: DDR4 Type Detail: Synchronous Speed: 3200 MT/s Manufacturer: Unknown (0) Serial Number: 00000000 Asset Tag: Not Specified Part Number: MT40A512M16TB-062E:J Rank: 1 Configured Memory Speed: 2400 MT/s Minimum Voltage: Unknown Maximum Voltage: Unknown Configured Voltage: Unknown
Signed-off-by: Rob Barnes robbarnes@google.com Change-Id: I1879676ea9436b6d19c768f1b78487a4e179f8d6 --- M src/soc/amd/picasso/dmi.c 1 file changed, 5 insertions(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/44984/2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/amd/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
Patch Set 2: Code-Review+2
Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/amd/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44984/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44984/1//COMMIT_MSG@7 PS1, Line 7: soc/picasso
nit: soc/amd/picasso:
Done
Aaron Durbin has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/amd/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
soc/amd/picasso: Set max_speed_mts and configured_speed_mts
ddr_frequency is deprecated. Set max_speed_mts and configured_speed_mts instead. This will result in SMBIOS type 17 displaying more accurate speed information.
BUG=b:167218112 TEST=Boot ezkinil and observe dmidecode -t17 dmidecode -t17 # dmidecode 3.2 Getting SMBIOS data from sysfs. SMBIOS 3.0 present.
Handle 0x000B, DMI type 17, 40 bytes Memory Device Array Handle: 0x000A Error Information Handle: Not Provided Total Width: 64 bits Data Width: 64 bits Size: 4096 MB Form Factor: SODIMM Set: None Locator: Channel-0-DIMM-0 Bank Locator: BANK 0 Type: DDR4 Type Detail: Synchronous Speed: 3200 MT/s Manufacturer: Unknown (0) Serial Number: 00000000 Asset Tag: Not Specified Part Number: MT40A512M16TB-062E:J Rank: 1 Configured Memory Speed: 2400 MT/s Minimum Voltage: Unknown Maximum Voltage: Unknown Configured Voltage: Unknown
Signed-off-by: Rob Barnes robbarnes@google.com Change-Id: I1879676ea9436b6d19c768f1b78487a4e179f8d6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44984 Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Raul Rangel rrangel@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/amd/picasso/dmi.c 1 file changed, 5 insertions(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Raul Rangel: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/soc/amd/picasso/dmi.c b/src/soc/amd/picasso/dmi.c index 3988616..5189cd6 100644 --- a/src/soc/amd/picasso/dmi.c +++ b/src/soc/amd/picasso/dmi.c @@ -28,14 +28,12 @@ dimm->ddr_type = dmi17->MemoryType;
/** - * Based on the name, ddr_frequency should hold the memory clock - * frequency in MHz. However it is interpreted as MT/s in SMBIOS - * downstream. So multiply by 2 to translate to memory speed in MT/s. - * ddr_frequency is used for setting both config speed and max - * speed. Using config speed so we don't get the false impression - * that the RAM is running faster than it actually is. + * TYPE17_DMI_INFO holds speed in MHz. + * Change to MT/s by multiplying by 2. */ - dimm->ddr_frequency = 2 * dmi17->ConfigSpeed; + dimm->configured_speed_mts = 2 * dmi17->ConfigSpeed; + + dimm->max_speed_mts = 2 * dmi17->Speed;
dimm->rank_per_dimm = dmi17->Attributes;
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44984 )
Change subject: soc/amd/picasso: Set max_speed_mts and configured_speed_mts ......................................................................
Patch Set 3:
Automatic boot test returned (PASS/FAIL/TOTAL): 7/1/8 "QEMU x86 q35/ich9" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17769 "QEMU x86 q35/ich9" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17768 "QEMU x86 i440fx/piix4" (x86_64) using payload SeaBIOS : FAIL : https://lava.9esec.io/r/17767 "QEMU x86 i440fx/piix4" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17766 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/17765 "HP Z220 SFF Workstation" (x86_32) using payload LinuxBoot_BusyBox_kexec : SUCCESS : https://lava.9esec.io/r/17772 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload TianoCore : SUCCESS : https://lava.9esec.io/r/17771 "HP Compaq 8200 Elite SFF PC" (x86_32) using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/17770
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Aaron Durbin (Code Review)
-
Furquan Shaikh (Code Review)
-
Raul Rangel (Code Review)
-
Rob Barnes (Code Review)
-
Tim Wawrzynczak (Code Review)