EricR Lai has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
mb/google/drallion: memory spd data debug
Add debug function for printing SPD data. This can be enable by unmark the SPD_DEBUG flag.
BUG=b:139397313 BRANCH=N/A TEST=Tested the same code on Hatch and checked cbmem log.
Signed-off-by: Eric Lai ericr_lai@compal.corp-partner.google.com Change-Id: Ie8e448f6f53db4ea0fd2ffdc58699064716c1d57 --- M src/mainboard/google/drallion/romstage.c 1 file changed, 114 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/35157/1
diff --git a/src/mainboard/google/drallion/romstage.c b/src/mainboard/google/drallion/romstage.c index 20eee7f..12cc3b3 100644 --- a/src/mainboard/google/drallion/romstage.c +++ b/src/mainboard/google/drallion/romstage.c @@ -17,6 +17,117 @@ #include <soc/cnl_memcfg_init.h> #include <soc/romstage.h>
+//#define SPD_DEBUG +#ifdef SPD_DEBUG +#include <variant/gpio.h> +#include <console/console.h> +#include <cbfs.h> + +/* Offset to identify DRAM type. */ +#define SPD_DRAM_TYPE_OFF 2 +#define SPD_DRAM_DDR4 0x0c + +/* Length of SPD data. */ +#define SPD_LEN_DDR4 512 + +/* Fields that are common across different memory types. */ +#define SPD_DENSITY_BANKS_OFF 4 +#define SPD_ADDRESSING_OFF 5 +#define SPD_PART_LEN 18 + +/* Fields that are different depending upon memory type. */ +#define SPD_ORG_OFF_DDR4 12 +#define SPD_BUSW_OFF_DDR4 13 +#define SPD_PART_OFF_DDR4 329 + +static const struct dram_info { + const char *str; + uint16_t type_code; + uint16_t len; + uint16_t org_off; + uint16_t busw_off; + uint16_t part_off; +} spd_info = { + .str = "DDR4", + .type_code = SPD_DRAM_DDR4, + .len = SPD_LEN_DDR4, + .org_off = SPD_ORG_OFF_DDR4, + .busw_off = SPD_BUSW_OFF_DDR4, + .part_off = SPD_PART_OFF_DDR4, +}; + +static void mainboard_print_spd_info(const uint8_t *spd) +{ + const int spd_banks[10] = { 4, 8, -1, -1, 8, 16, -1, -1, 16, 32 }; + const int spd_capmb[10] = { 1, 2, 4, 8, 16, 32, 64, 128, 48, 96 }; + const int spd_rows[8] = { 12, 13, 14, 15, 16, 17, 18, -1 }; + const int spd_cols[8] = { 9, 10, 11, 12, -1, -1, -1, -1 }; + const int spd_ranks[8] = { 1, 2, 3, 4, 5, 6, 7, 8 }; + const int spd_devw[8] = { 4, 8, 16, 32, -1, -1, -1, -1 }; + const int spd_busw[8] = { 8, 16, 32, 64, -1, -1, -1, -1 }; + char spd_name[SPD_PART_LEN+1] = { 0 }; + const struct dram_info *info = &spd_info; + + /* Module type */ + printk(BIOS_INFO, "SPD: module type is %s\n", info->str); + + int banks = spd_banks[(spd[SPD_DENSITY_BANKS_OFF] >> 4) & 7]; + int capmb = spd_capmb[spd[SPD_DENSITY_BANKS_OFF] & 7] * 256; + int rows = spd_rows[(spd[SPD_ADDRESSING_OFF] >> 3) & 7]; + int cols = spd_cols[spd[SPD_ADDRESSING_OFF] & 7]; + int ranks = spd_ranks[(spd[info->org_off] >> 3) & 7]; + int devw = spd_devw[spd[info->org_off] & 7]; + int busw = spd_busw[spd[info->busw_off] & 7]; + + /* Module Part Number */ + memcpy(spd_name, &spd[info->part_off], SPD_PART_LEN); + spd_name[SPD_PART_LEN] = 0; + printk(BIOS_INFO, "SPD: module part is %s\n", spd_name); + + printk(BIOS_INFO, + "SPD: banks %d, ranks %d, rows %d, columns %d, density %d Mb\n", + banks, ranks, rows, cols, capmb); + printk(BIOS_INFO, "SPD: device width %d bits, bus width %d bits\n", + devw, busw); + + if (capmb > 0 && busw > 0 && devw > 0 && ranks > 0) { + /* SIZE = DENSITY / 8 * BUS_WIDTH / SDRAM_WIDTH * RANKS */ + printk(BIOS_INFO, "SPD: module size is %u MB (per channel)\n", + capmb / 8 * busw / devw * ranks); + } +} + +static void mainboard_debug_spd_data(void) +{ + char *spd_file; + size_t spd_file_len; + int spd_index; + const size_t spd_len = spd_info.len; + const char *spd_bin = "spd.bin"; + + spd_index = variant_memory_sku(); + + printk(BIOS_INFO, "SPD: index %d\n", spd_index); + + /* Load SPD data from CBFS */ + spd_file = cbfs_boot_map_with_leak(spd_bin, CBFS_TYPE_SPD, + &spd_file_len); + if (!spd_file) + die("SPD data not found."); + + /* make sure we have at least one SPD in the file. */ + if (spd_file_len < spd_len) + die("Missing SPD data."); + + /* Make sure we did not overrun the buffer */ + if (spd_file_len < ((spd_index + 1) * spd_len)) + die("Invalid SPD index."); + + spd_index *= spd_len; + mainboard_print_spd_info((uint8_t *)(spd_file + spd_index)); +} +#endif + static const struct cnl_mb_cfg memcfg = { /* Access memory info through SMBUS. */ .spd[0] = { @@ -58,6 +169,8 @@ void mainboard_memory_init_params(FSPM_UPD *memupd) { wilco_ec_romstage_init(); - +#ifdef SPD_DEBUG + mainboard_debug_spd_data(); +#endif cannonlake_memcfg_init(&memupd->FspmConfig, &memcfg); }
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35157/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
PS3: Most of this debug info is similar to what is in src/lib/spd_bin.c. Can that be used instead of duplicating the code?
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 3:
Patch Set 3:
(1 comment)
I can check this. I think it's fine because I just modified from poppy romstage.c.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 3:
Patch Set 3:
(1 comment)
I have question about DDR4 banks. I think it should be bank groups * banks, right? const int spd_banks[8] = { 8, 16, 32, 64, -1, -1, -1, -1 }; this is for DDR3.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35157/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
PS3:
Most of this debug info is similar to what is in src/lib/spd_bin.c. […]
I think I can hook this into cnl_memcfg_init. Thanks.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/35157/3/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
PS3:
I think I can hook this into cnl_memcfg_init. Thanks.
I keep in this file. Only debug with drallion board.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
Patch Set 3:
(1 comment)
@Furquan, how about now?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35157/4/src/mainboard/google/dralli... File src/mainboard/google/drallion/romstage.c:
https://review.coreboot.org/c/coreboot/+/35157/4/src/mainboard/google/dralli... PS4, Line 54: print_spd_info If you add call to print_spd_info() here: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/cannonlake/..., then you wouldn't need all the logic to read SPD file again here in mainboard. Also, is there really a need to guard this with debug? I think its okay to print SPD info in SoC since it can be helpful.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
Patch Set 4:
(1 comment)
I encounter some difficulty for the pointer casting. If you think it's better to add for all project.Maybe we can raise a issue for this.
EricR Lai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
Patch Set 4:
(1 comment)
I encounter some difficulty for the pointer casting. If you think it's better to add for all project.Maybe we can raise a issue for this.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Patch Set 4:
Patch Set 4:
Patch Set 4:
(1 comment)
I encounter some difficulty for the pointer casting. If you think it's better to add for all project.Maybe we can raise a issue for this.
Sure. Please feel free to raise a bug and add details on what issues you are running into.
EricR Lai has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35157 )
Change subject: mb/google/drallion: memory spd data debug ......................................................................
Abandoned
move to this CL https://review.coreboot.org/c/coreboot/+/35235
-
EricR Lai (Code Review) -
Furquan Shaikh (Code Review)