Re: [PATCH 6/6] x86: fsp: Support a warning message when DRAM init is slow

Hi Simon,

-----"Simon Glass" sjg@chromium.org schrieb: -----

Betreff: [PATCH 6/6] x86: fsp: Support a warning message when DRAM init is slow

With DDR4, Intel SOCs take quite a long time to init their memory. During this time, if the user is watching, it looks like SPL has hung. Add a message in this case.

This works by adding a return code to fspm_update_config() that indicates whether MRC data was found and a new property to the device tree.

Also add one more debug message while starting.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/apollolake/fsp_m.c | 12 ++++++++-- arch/x86/dts/chromebook_coral.dts | 1 + arch/x86/include/asm/fsp2/fsp_internal.h | 3 ++- arch/x86/lib/fsp2/fsp_meminit.c | 24 +++++++++++++++---- .../fsp/fsp2/apollolake/fsp-m.txt | 3 +++ 5 files changed, 35 insertions(+), 8 deletions(-)

diff --git a/arch/x86/cpu/apollolake/fsp_m.c b/arch/x86/cpu/apollolake/fsp_m.c index 1301100cd5..65461d85b8 100644 --- a/arch/x86/cpu/apollolake/fsp_m.c +++ b/arch/x86/cpu/apollolake/fsp_m.c @@ -16,10 +16,14 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd) { struct fsp_m_config *cfg = &upd->config; struct fspm_arch_upd *arch = &upd->arch;

• int cache_ret = 0; ofnode node;

• int ret;

  arch->nvs_buffer_ptr = NULL;

• prepare_mrc_cache(upd);

• cache_ret = prepare_mrc_cache(upd);
• if (cache_ret && cache_ret != -ENOENT)

• return log_msg_ret("mrc", cache_ret);
  

  arch->stack_base = (void *)0xfef96000; arch->boot_loader_tolum_size = 0; arch->boot_mode = FSP_BOOT_WITH_FULL_CONFIGURATION;

@@ -28,7 +32,11 @@ int fspm_update_config(struct udevice *dev, struct fspm_upd *upd) if (!ofnode_valid(node)) return log_msg_ret("fsp-m settings", -ENOENT);

• return fsp_m_update_config_from_dtb(node, cfg);

• ret = fsp_m_update_config_from_dtb(node, cfg);
• if (ret)

• return log_msg_ret("dtb", cache_ret);
  

• return cache_ret;

}

/* diff --git a/arch/x86/dts/chromebook_coral.dts b/arch/x86/dts/chromebook_coral.dts index dea35b73a0..aad12f2c4d 100644 --- a/arch/x86/dts/chromebook_coral.dts +++ b/arch/x86/dts/chromebook_coral.dts @@ -117,6 +117,7 @@ reg = <0x00000000 0 0 0 0>; compatible = "intel,apl-hostbridge"; pciex-region-size = <0x10000000>;

• 	fspm,training-delay = <21>;
  /*
   * Parameters used by the FSP-S binary blob. This is
   * really unfortunate since these parameters mostly
  

diff --git a/arch/x86/include/asm/fsp2/fsp_internal.h b/arch/x86/include/asm/fsp2/fsp_internal.h index f751fbf961..b4a4fbbd84 100644 --- a/arch/x86/include/asm/fsp2/fsp_internal.h +++ b/arch/x86/include/asm/fsp2/fsp_internal.h @@ -57,7 +57,8 @@ int arch_fsps_preinit(void);

• @dev: Hostbridge device containing config
• @upd: Config data to fill in

• • @return 0 if OK, -ve on error

• • @return 0 if OK, -ENOENT if OK but no MRC-cache data was found, other -ve on
• • error
  */

int fspm_update_config(struct udevice *dev, struct fspm_upd *upd);

diff --git a/arch/x86/lib/fsp2/fsp_meminit.c b/arch/x86/lib/fsp2/fsp_meminit.c index faf9c29aef..ce0b0aff76 100644 --- a/arch/x86/lib/fsp2/fsp_meminit.c +++ b/arch/x86/lib/fsp2/fsp_meminit.c @@ -9,6 +9,7 @@ #include <common.h> #include <binman.h> #include <bootstage.h> +#include <dm.h> #include <log.h> #include <asm/mrccache.h> #include <asm/fsp/fsp_infoheader.h> @@ -63,8 +64,10 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash) struct fsp_header *hdr; struct hob_header *hob; struct udevice *dev;

• int delay; int ret;

• log_debug("Locating FSP\n"); ret = fsp_locate_fsp(FSP_M, &entry, use_spi_flash, &dev, &hdr, NULL); if (ret) return log_msg_ret("locate FSP", ret);

@@ -76,21 +79,32 @@ int fsp_memory_init(bool s3wake, bool use_spi_flash) return log_msg_ret("Bad UPD signature", -EPERM); memcpy(&upd, fsp_upd, sizeof(upd));

• delay = dev_read_u32_default(dev, "fspm,training-delay", 0); ret = fspm_update_config(dev, &upd);

• if (ret)

• return log_msg_ret("Could not setup config", ret);
  

• debug("SDRAM init...");

• if (ret) {

• if (ret != -ENOENT)
  

• 	return log_msg_ret("Could not setup config", ret);
  

• } else {

• delay = 0;
  

• }

• if (delay)

• printf("SDRAM training (%d seconds)...", delay);
  

• else

• log_debug("SDRAM init...");
  

  bootstage_start(BOOTSTAGE_ID_ACCUM_FSP_M, "fsp-m"); func = (fsp_memory_init_func)(hdr->img_base + hdr->fsp_mem_init); ret = func(&upd, &hob); bootstage_accum(BOOTSTAGE_ID_ACCUM_FSP_M); cpu_reinit_fpu();

• if (delay)

• printf("done\n");
  

• else

• log_debug("done\n");
  

  if (ret) return log_msg_ret("SDRAM init fail\n", ret);

  gd->arch.hob_list = hob;

• debug("done\n");

  ret = fspm_done(dev); if (ret)

diff --git a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt index 647a0862d4..67407fa935 100644 --- a/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt +++ b/doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt @@ -17,6 +17,9 @@ values of the FSP-M are used. [2] https://github.com/IntelFsp/FSP/tree/master/ApolloLakeFspBinPkg/Docs

Optional properties: +- fspm,training-delay: Time taken to train DDR memory if there is no cached MRC

• data, in seconds. This is used to show a message if possible. For Intel APL
• this is typicaly 21 seconds.

1) The required time is not APL specific, I think it depends on the memory configuration. I have tested it on an APL board with just 1 GB RAM, and there it only takes ~6 seconds.

How about rewording it as follows: "As an example, for Chromebook Coral this is typically 21 seconds."

2) Typo: typically

• fspm,serial-debug-port-address: Debug Serial Port Base address
• fspm,serial-debug-port-type: Debug Serial Port Type 0: NONE

-- 2.27.0.rc0.183.gde8f92d652-goog

Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com

Tested-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested on a custom Apollo Lake board