Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75698?usp=email )
Change subject: mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM ......................................................................
mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM
One specific Hynix LPDDR5x DRAM part requires an ABL workaround to eliminate DRAM-related failures during a FAFT test, but due to the use of generic/common SPDs, there is no way for the ABL to determine the DRAM part # itself.
Until the issue can be root-caused, we will have coreboot check the DRAM part #, and set/clear a CMOS bit as appropriate, which the ABL will check in order to apply (or not apply) the workaround.
The ABL uses already uses byte 0xD of the extended CMOS ports 72/73 for memory context related toggles, so we will use a spare bit there.
BUG=b:270499009, b:281614369, b:286338775 BRANCH=skyrim TEST=run FAFT bios tests on frostflow, markarth, and whiterun without any failures.
Change-Id: Ibb6e145f6cdba7270e0a322ef414bf1cb09c5eaa Signed-off-by: Matt DeVillier matt.devillier@amd.corp-partner.google.com --- M src/mainboard/google/skyrim/bootblock.c 1 file changed, 59 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/75698/1
diff --git a/src/mainboard/google/skyrim/bootblock.c b/src/mainboard/google/skyrim/bootblock.c index 4128090..16c9201 100644 --- a/src/mainboard/google/skyrim/bootblock.c +++ b/src/mainboard/google/skyrim/bootblock.c @@ -1,9 +1,66 @@ /* SPDX-License-Identifier: GPL-2.0-only */
#include <amdblocks/espi.h> +#include <amdblocks/reset.h> #include <bootblock_common.h> #include <baseboard/variants.h> +#include <console/console.h> +#include <ec/google/chromeec/ec.h> +#include <pc80/mc146818rtc.h> #include <soc/espi.h> +#include <string.h> + +#define CMOS_EXTENDED_ADDR(x) (128 + (x)) +#define CMOS_MEM_RESTORE_OFFSET 0x0D +#define CMOS_BITMAP_SKIP_RESET_TOGGLE 0x10 +#define HYNIX_PART_NAME "H9JCNNNCP3MLYR-N6E" +#define HYNIX_PART_LEN 18 + +/* Ensure SKIP_RESET_TOGGLE CMOS bit set for problematic Hynix part, cleared otherwise */ +static void hynix_dram_cmos_check(void) +{ + char cbi_part_number[DIMM_INFO_PART_NUMBER_SIZE]; + bool skip_reset_toggle, cmos_bit_set; + unsigned char byte_value; + + byte_value = cmos_read(CMOS_EXTENDED_ADDR(CMOS_MEM_RESTORE_OFFSET)); + cmos_bit_set = (byte_value & CMOS_BITMAP_SKIP_RESET_TOGGLE) != 0; + + printk(BIOS_SPEW, "Checking DRAM part #\n"); + + /* Get RAM module part # from CBI */ + if (google_chromeec_cbi_get_dram_part_num( + cbi_part_number, sizeof(cbi_part_number)) != 0) { + printk(BIOS_ERR, "Unable to read DRAM part # from CBI\n"); + return; + } + /* Ensure DRAM part # null terminated */ + cbi_part_number[DIMM_INFO_PART_NUMBER_SIZE - 1] = 0; + printk(BIOS_SPEW, "DRAM part # is %s\n", cbi_part_number); + + skip_reset_toggle = strncmp(cbi_part_number, HYNIX_PART_NAME, HYNIX_PART_LEN) == 0; + + /* If problematic Hynix module, ensure bit set */ + if (skip_reset_toggle) { + printk(BIOS_SPEW, "SKIP_RESET_TOGGLE needed, checking CMOS bit is set\n"); + /* if flag unset, set and reboot so memory re-trained */ + if (!cmos_bit_set) { + printk(BIOS_SPEW, "Bit is unset; setting and rebooting\n"); + cmos_write((byte_value | CMOS_BITMAP_SKIP_RESET_TOGGLE), + CMOS_EXTENDED_ADDR(CMOS_MEM_RESTORE_OFFSET)); + warm_reset(); + } + printk(BIOS_SPEW, "Bit already set; nothing to do.\n"); + } else if (cmos_bit_set) { + /* Otherwise, ensure SKIP_RESET_TOGGLE bit cleared */ + printk(BIOS_SPEW, "CMOS SKIP_RESET_TOGGLE bit is set; clearing and rebooting\n"); + cmos_write((byte_value & ~CMOS_BITMAP_SKIP_RESET_TOGGLE), + CMOS_EXTENDED_ADDR(CMOS_MEM_RESTORE_OFFSET)); + warm_reset(); + } else { + printk(BIOS_SPEW, "No change to CMOS SKIP_RESET_TOGGLE bit is needed\n"); + } +}
void mb_set_up_early_espi(void) { @@ -34,6 +91,8 @@ size_t num_gpios; const struct soc_amd_gpio *gpios;
+ hynix_dram_cmos_check(); + variant_bootblock_gpio_table(&gpios, &num_gpios); gpio_configure_pads(gpios, num_gpios); }