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

Wolfgang Wallner wolfgang.wallner at br-automation.com
Thu Jun 18 14:54:41 CEST 2020


Hi Simon,

-----"Simon Glass" <sjg at 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 at 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 at br-automation.com>

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



More information about the U-Boot mailing list