lib: Add new argument as `ddr_type` to smbios_bus_width_to_spd_width()
Add DDR5 and LPDDR5 memory type checks while calculating bus width extension (in bits). Additionally, update all caller functions of smbios_bus_width_to_spd_width() to pass `MemoryType` as argument. Update `test_smbios_bus_width_to_spd_width()` to accommodate different memory types. Create new macro to fix incorrect bus width reporting on platform with DDR5 and LPDDR5 memory. With this code changes, on DDR5 system with 2 Ch per DIMM, 32 bit primary bus width per Ch showed the Total width as: Handle 0x000F, DMI type 17, 40 bytes Memory Device Array Handle: 0x0009 Error Information Handle: Not Provided Total Width: 80 bits Data Width: 64 bits Size: 16 GB ... BUG=b:194659789 Tested=On Alder Lake DDR5 RVP, SMBIOS type 17 shows expected `Total Width`. Change-Id: I79ec64c9d522a34cb44b3f575725571823048380 Signed-off-by: Subrata Banik <subrata.banik@intel.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58601 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Rob Barnes <robbarnes@google.com>
This commit is contained in:
parent
35bcf5071c
commit
3306f37fd6
|
@ -12,7 +12,7 @@
|
||||||
* Use this when setting dimm_info.bus_width if the raw SPD values are not
|
* Use this when setting dimm_info.bus_width if the raw SPD values are not
|
||||||
* available.
|
* available.
|
||||||
*/
|
*/
|
||||||
uint8_t smbios_bus_width_to_spd_width(uint16_t total_width,
|
uint8_t smbios_bus_width_to_spd_width(uint8_t ddr_type, uint16_t total_width,
|
||||||
uint16_t data_width);
|
uint16_t data_width);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
|
@ -209,5 +209,6 @@ enum spd_memory_type {
|
||||||
#define SPD_MINI_UDIMM 0x20
|
#define SPD_MINI_UDIMM 0x20
|
||||||
|
|
||||||
#define SPD_ECC_8BIT (1<<3)
|
#define SPD_ECC_8BIT (1<<3)
|
||||||
|
#define SPD_ECC_8BIT_LP5_DDR5 (1<<4)
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
|
@ -5,7 +5,8 @@
|
||||||
#include <spd.h>
|
#include <spd.h>
|
||||||
#include <console/console.h>
|
#include <console/console.h>
|
||||||
|
|
||||||
uint8_t smbios_bus_width_to_spd_width(uint16_t total_width, uint16_t data_width)
|
uint8_t smbios_bus_width_to_spd_width(uint8_t ddr_type, uint16_t total_width,
|
||||||
|
uint16_t data_width)
|
||||||
{
|
{
|
||||||
uint8_t out;
|
uint8_t out;
|
||||||
|
|
||||||
|
@ -38,7 +39,10 @@ uint8_t smbios_bus_width_to_spd_width(uint16_t total_width, uint16_t data_width)
|
||||||
|
|
||||||
switch (extension_bits) {
|
switch (extension_bits) {
|
||||||
case 8:
|
case 8:
|
||||||
out |= SPD_ECC_8BIT;
|
if (ddr_type == MEMORY_TYPE_DDR5 || ddr_type == MEMORY_TYPE_LPDDR5)
|
||||||
|
out |= SPD_ECC_8BIT_LP5_DDR5;
|
||||||
|
else
|
||||||
|
out |= SPD_ECC_8BIT;
|
||||||
break;
|
break;
|
||||||
case 0:
|
case 0:
|
||||||
/* No extension bits */
|
/* No extension bits */
|
||||||
|
|
|
@ -54,8 +54,8 @@ static void transfer_memory_info(const TYPE17_DMI_INFO *dmi17,
|
||||||
|
|
||||||
dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
|
dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
|
||||||
|
|
||||||
dimm->bus_width =
|
dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth,
|
||||||
smbios_bus_width_to_spd_width(dmi17->TotalWidth, dmi17->DataWidth);
|
dmi17->DataWidth);
|
||||||
|
|
||||||
dimm->mod_id = dmi17->ManufacturerIdCode;
|
dimm->mod_id = dmi17->ManufacturerIdCode;
|
||||||
|
|
||||||
|
|
|
@ -38,8 +38,8 @@ static void transfer_memory_info(TYPE17_DMI_INFO *dmi17,
|
||||||
|
|
||||||
dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
|
dimm->mod_type = smbios_form_factor_to_spd_mod_type(dmi17->FormFactor);
|
||||||
|
|
||||||
dimm->bus_width =
|
dimm->bus_width = smbios_bus_width_to_spd_width(dmi17->MemoryType, dmi17->TotalWidth,
|
||||||
smbios_bus_width_to_spd_width(dmi17->TotalWidth, dmi17->DataWidth);
|
dmi17->DataWidth);
|
||||||
|
|
||||||
dimm->mod_id = dmi17->ManufacturerIdCode;
|
dimm->mod_id = dmi17->ManufacturerIdCode;
|
||||||
|
|
||||||
|
|
|
@ -65,5 +65,5 @@ void dimm_info_fill(struct dimm_info *dimm, u32 dimm_capacity, u8 ddr_type,
|
||||||
if (ecc_support)
|
if (ecc_support)
|
||||||
total_width += EXTENSION_BUS_WIDTH_8BITS;
|
total_width += EXTENSION_BUS_WIDTH_8BITS;
|
||||||
|
|
||||||
dimm->bus_width = smbios_bus_width_to_spd_width(total_width, data_width);
|
dimm->bus_width = smbios_bus_width_to_spd_width(ddr_type, total_width, data_width);
|
||||||
}
|
}
|
||||||
|
|
|
@ -4,28 +4,51 @@
|
||||||
#include <spd.h>
|
#include <spd.h>
|
||||||
#include <tests/test.h>
|
#include <tests/test.h>
|
||||||
|
|
||||||
static void test_smbios_bus_width_to_spd_width(void **state)
|
static void test_smbios_bus_width_to_spd_width_parametrized(smbios_memory_type ddr_type)
|
||||||
{
|
{
|
||||||
/* Non-ECC variants */
|
/* Non-ECC variants */
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_64, smbios_bus_width_to_spd_width(64, 64));
|
assert_int_equal(MEMORY_BUS_WIDTH_64, smbios_bus_width_to_spd_width(ddr_type, 64, 64));
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_32, smbios_bus_width_to_spd_width(32, 32));
|
assert_int_equal(MEMORY_BUS_WIDTH_32, smbios_bus_width_to_spd_width(ddr_type, 32, 32));
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_16, smbios_bus_width_to_spd_width(16, 16));
|
assert_int_equal(MEMORY_BUS_WIDTH_16, smbios_bus_width_to_spd_width(ddr_type, 16, 16));
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_8, smbios_bus_width_to_spd_width(8, 8));
|
assert_int_equal(MEMORY_BUS_WIDTH_8, smbios_bus_width_to_spd_width(ddr_type, 8, 8));
|
||||||
/* Incorrect data width. Fallback to 8-bit */
|
/* Incorrect data width. Fallback to 8-bit */
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_8, smbios_bus_width_to_spd_width(15, 15));
|
assert_int_equal(MEMORY_BUS_WIDTH_8, smbios_bus_width_to_spd_width(ddr_type, 15, 15));
|
||||||
|
|
||||||
/* ECC variants */
|
/* ECC variants */
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_64 | SPD_ECC_8BIT,
|
uint8_t extension_8bits = SPD_ECC_8BIT;
|
||||||
smbios_bus_width_to_spd_width(64 + 8, 64));
|
if (ddr_type == MEMORY_TYPE_DDR5 || ddr_type == MEMORY_TYPE_LPDDR5)
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_32 | SPD_ECC_8BIT,
|
extension_8bits = SPD_ECC_8BIT_LP5_DDR5;
|
||||||
smbios_bus_width_to_spd_width(32 + 8, 32));
|
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_16 | SPD_ECC_8BIT,
|
assert_int_equal(MEMORY_BUS_WIDTH_64 | extension_8bits,
|
||||||
smbios_bus_width_to_spd_width(16 + 8, 16));
|
smbios_bus_width_to_spd_width(ddr_type, 64 + 8, 64));
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_8 | SPD_ECC_8BIT,
|
assert_int_equal(MEMORY_BUS_WIDTH_32 | extension_8bits,
|
||||||
smbios_bus_width_to_spd_width(8 + 8, 8));
|
smbios_bus_width_to_spd_width(ddr_type, 32 + 8, 32));
|
||||||
|
assert_int_equal(MEMORY_BUS_WIDTH_16 | extension_8bits,
|
||||||
|
smbios_bus_width_to_spd_width(ddr_type, 16 + 8, 16));
|
||||||
|
assert_int_equal(MEMORY_BUS_WIDTH_8 | extension_8bits,
|
||||||
|
smbios_bus_width_to_spd_width(ddr_type, 8 + 8, 8));
|
||||||
/* Incorrect data width. Fallback to 8-bit */
|
/* Incorrect data width. Fallback to 8-bit */
|
||||||
assert_int_equal(MEMORY_BUS_WIDTH_8 | SPD_ECC_8BIT,
|
assert_int_equal(MEMORY_BUS_WIDTH_8 | extension_8bits,
|
||||||
smbios_bus_width_to_spd_width(15 + 8, 15));
|
smbios_bus_width_to_spd_width(ddr_type, 15 + 8, 15));
|
||||||
|
}
|
||||||
|
|
||||||
|
static void test_smbios_bus_width_to_spd_width(void **state)
|
||||||
|
{
|
||||||
|
smbios_memory_type memory_type[] = {
|
||||||
|
MEMORY_TYPE_DDR2,
|
||||||
|
MEMORY_TYPE_DDR3,
|
||||||
|
MEMORY_TYPE_DDR4,
|
||||||
|
MEMORY_TYPE_DDR5,
|
||||||
|
MEMORY_TYPE_LPDDR3,
|
||||||
|
MEMORY_TYPE_LPDDR4,
|
||||||
|
MEMORY_TYPE_LPDDR5,
|
||||||
|
};
|
||||||
|
|
||||||
|
for (int i = 0; i < ARRAY_SIZE(memory_type); i++) {
|
||||||
|
print_message("test_smbios_bus_width_to_spd_width_parametrized(%d)\n",
|
||||||
|
memory_type[i]);
|
||||||
|
test_smbios_bus_width_to_spd_width_parametrized(memory_type[i]);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static void test_smbios_memory_size_to_mib(void **state)
|
static void test_smbios_memory_size_to_mib(void **state)
|
||||||
|
|
Loading…
Reference in New Issue