diff mbox series

[v3,4/4] x86: fsp: Support a warning message when DRAM init is slow

Message ID 20200710004317.3017137-3-sjg@chromium.org
State Superseded
Headers show
Series [v3,1/4] timer: Allow delays with a 32-bit microsecond timer | expand

Commit Message

Simon Glass July 10, 2020, 12:43 a.m. UTC 
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>
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
Tested-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
---

(no changes since v2)

Changes in v2:
- Use the three-argument CONFIG_IS_ENABLED() instead of IF_ENABLED_INT()
- Update binding to mention timing for coral and a 1GB APL board
- Drop patch 'kconfig: Add support for conditional values'

 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             |  4 ++++
 5 files changed, 36 insertions(+), 8 deletions(-)

Comments

Bin Meng July 13, 2020, 2:17 a.m. UTC | #1 
On Fri, Jul 10, 2020 at 8:43 AM Simon Glass <sjg at chromium.org> wrote:
>
> 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>
> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> Tested-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Use the three-argument CONFIG_IS_ENABLED() instead of IF_ENABLED_INT()
> - Update binding to mention timing for coral and a 1GB APL board
> - Drop patch 'kconfig: Add support for conditional values'
>
>  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             |  4 ++++
>  5 files changed, 36 insertions(+), 8 deletions(-)
>

applied to u-boot-x86, thanks!
diff mbox series

Patch

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 965d9f387d..a17a9c2800 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..5311938f43 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,10 @@  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 Chromebook
+    Coral this is typically 21 seconds. For an APL board with 1GB of RAM, it may
+    be only 6 seconds.
 - fspm,serial-debug-port-address: Debug Serial Port Base address
 - fspm,serial-debug-port-type: Debug Serial Port Type
   0: NONE