From d9bc4740da9fbd945790fa618c0ae3ee6e6760b2 Mon Sep 17 00:00:00 2001 From: Angel Pons Date: Sun, 22 Feb 2026 16:46:12 +0100 Subject: [PATCH] nb/intel/haswell: Fix DDR frequency reporting DDR frequency (in MHz) was doubled for no reason, then doubled again to convert it to MT/s. Moreover, the calculations assume a reference clock of 133 MHz, but a 100 MHz reference clock also exists. Add two functions: `is_100_mhz_refclk()` to check whether the reference clock is 100 MHz, and `get_ddr_freq_mhz()` to get the DDR frequency, in MHz. Use both functions in `report_memory_config()` to show the correct DDR ref. clock and frequency, and use one in `setup_sdram_meminfo()` so that SMBIOS tables contain the correct memory speed. Tested on ASRock Z97 Extreme6 with four DDR3-1600 sticks, DDR frequency is correctly reported as 800 MHz with either reference clock frequency: Default 133 MHz reference clock: memcfg DDR3 ref clock 133 MHz memcfg DDR3 clock 800 MHz After forcing 100 MHz ref clock for 800 MHz (edit NRI's `init_mpll.c`): memcfg DDR3 ref clock 100 MHz memcfg DDR3 clock 800 MHz Also, SMBIOS type 17 correctly reports memory speeds of 1600 MT/s: $ sudo dmidecode --type 17 | grep -i speed Speed: 1600 MT/s Configured Memory Speed: 1600 MT/s Speed: 1600 MT/s Configured Memory Speed: 1600 MT/s Speed: 1600 MT/s Configured Memory Speed: 1600 MT/s Speed: 1600 MT/s Configured Memory Speed: 1600 MT/s It is expected that behaviour using either MRC binary is the same since the `MC_BIOS_REQ` and `MC_BIOS_DATA` registers have to be programmed in order for the DDR clock to start running. The decision to test with NRI is because one can easily change the chosen reference clock to 100 MHz. Resolves: https://ticket.coreboot.org/issues/624 Change-Id: Idead9cd55b453d3ff4695c977dee763ff50830f8 Signed-off-by: Angel Pons Reviewed-on: https://review.coreboot.org/c/coreboot/+/91375 Tested-by: build bot (Jenkins) Reviewed-by: Paul Menzel Reviewed-by: Benjamin Doron Reviewed-by: Matt DeVillier --- .../intel/haswell/raminit_shared.c | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/northbridge/intel/haswell/raminit_shared.c b/src/northbridge/intel/haswell/raminit_shared.c index 1e6394da77..5e3602c31e 100644 --- a/src/northbridge/intel/haswell/raminit_shared.c +++ b/src/northbridge/intel/haswell/raminit_shared.c @@ -28,13 +28,28 @@ static const char *const ecc_decoder[] = { "active", }; +static bool is_100_mhz_refclk(void) +{ + return mchbar_read32(MC_BIOS_REQ) & (1 << 4); +} + +static unsigned int get_ddr_freq_mhz(void) +{ + /* Use KHz to minimise rounding errors */ + const unsigned int refclk_khz = is_100_mhz_refclk() ? 100000 : 133333; + const unsigned int multiplier = mchbar_read32(MC_BIOS_DATA) & 0xf; + + return DIV_ROUND_CLOSEST(multiplier * refclk_khz, 1000); +} + /* Print out the memory controller configuration, as per the values in its registers. */ void report_memory_config(void) { const uint32_t addr_decoder_common = mchbar_read32(MAD_CHNL); - printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", - DIV_ROUND_CLOSEST(mchbar_read32(MC_BIOS_DATA) * 13333 * 2, 100)); + const unsigned int refclk_mhz = is_100_mhz_refclk() ? 100 : 133; + printk(BIOS_DEBUG, "memcfg DDR3 ref clock %u MHz\n", refclk_mhz); + printk(BIOS_DEBUG, "memcfg DDR3 clock %u MHz\n", get_ddr_freq_mhz()); printk(BIOS_DEBUG, "memcfg channel assignment: A: %u, B: %u, C: %u\n", (addr_decoder_common >> 0) & 3, @@ -113,8 +128,7 @@ void setup_sdram_meminfo(const uint8_t *spd_data[NUM_CHANNELS][NUM_SLOTS]) memset(mem_info, 0, sizeof(struct memory_info)); - /* TODO: This looks like an open-coded DIV_ROUND_CLOSEST() */ - const uint32_t ddr_freq_mhz = (mchbar_read32(MC_BIOS_DATA) * 13333 * 2 + 50) / 100; + const uint32_t ddr_freq_mhz = get_ddr_freq_mhz(); unsigned int dimm_cnt = 0; for (unsigned int channel = 0; channel < NUM_CHANNELS; channel++) {