All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] rockchip: Fix eMMC performance regression
@ 2023-05-17 18:40 Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 1/5] mmc: rockchip_sdhci: Skip blocks read workaround on RK3399 Jonas Karlman
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich
  Cc: Eugen Hristev, u-boot, Jonas Karlman

The eMMC performance on RK3399 was reduced sigificant by the
commit 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks read
in a single command").

That workaround should only have been applied to RK3568 and RK3588.
This series fixes that and also help boost eMMC performance on two
RK3399 boards even more by enabling use of SDMA.

There is also an extra commit to help build a u-boot-rockchip-spi.bin
image that can be used for SPI flash boot on RockPro64.

Changes in v2:
- Rebase on top of defconfig and spi v2 series
- Collect r-b and t-b tags

Jonas Karlman (5):
  mmc: rockchip_sdhci: Skip blocks read workaround on RK3399
  mmc: rockchip_sdhci: Disable DMA mode using a device tree property
  rockchip: rockpro64: Use SDMA to boost eMMC performance
  rockchip: rock-pi-4: Use SDMA to boost eMMC performance
  rockchip: rockpro64: Build u-boot-rockchip-spi.bin

 arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi |  6 ++++++
 arch/arm/dts/rk3399-rockpro64-u-boot.dtsi |  5 +++++
 arch/arm/dts/rk3399-u-boot.dtsi           |  1 +
 arch/arm/dts/rk3588s-u-boot.dtsi          |  1 +
 configs/rock-pi-4-rk3399_defconfig        |  2 ++
 configs/rock5b-rk3588_defconfig           |  1 -
 configs/rockpro64-rk3399_defconfig        |  5 +++++
 drivers/mmc/rockchip_sdhci.c              | 12 +++++++++++-
 8 files changed, 31 insertions(+), 2 deletions(-)

-- 
2.40.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v2 1/5] mmc: rockchip_sdhci: Skip blocks read workaround on RK3399
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
@ 2023-05-17 18:40 ` Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 2/5] mmc: rockchip_sdhci: Disable DMA mode using a device tree property Jonas Karlman
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich, Peng Fan,
	Jaehoon Chung, Jonas Karlman
  Cc: Eugen Hristev, u-boot, Quentin Schulz

The workaround to limit number of blocks to read in a single command
should only be applied to RK3568 and RK3588. Change to be more strict
when to apply the workaround.

Fixes: 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks read in a single command")
Suggested-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Quentin Schulz <foss+uboot@0leil.net> # RK3399 Puma, RK3588 Tiger
---
v2:
- Collect r-b and t-b tags

 drivers/mmc/rockchip_sdhci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index 4f110976f4e8..8e4a158049a9 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -594,7 +594,9 @@ static int rockchip_sdhci_probe(struct udevice *dev)
 	 * triggers Data End Bit Error on RK3568 and RK3588. Limit to reading
 	 * max 4 blocks in one command when using PIO mode.
 	 */
-	if (!(host->flags & USE_DMA))
+	if (!(host->flags & USE_DMA) &&
+	    (device_is_compatible(dev, "rockchip,rk3568-dwcmshc") ||
+	     device_is_compatible(dev, "rockchip,rk3588-dwcmshc")))
 		cfg->b_max = 4;
 
 	return sdhci_probe(dev);
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 2/5] mmc: rockchip_sdhci: Disable DMA mode using a device tree property
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 1/5] mmc: rockchip_sdhci: Skip blocks read workaround on RK3399 Jonas Karlman
@ 2023-05-17 18:40 ` Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance Jonas Karlman
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, Peng Fan, Jaehoon Chung, Jonas Karlman
  Cc: u-boot, Quentin Schulz

Loading part of TF-A into SRAM from eMMC using DMA fails on RK3399
similar to other Rockchip SoCs. Checksum validation fails with:

  ## Checking hash(es) for Image atf-2 ... sha256 error!
  Bad hash value for 'hash' hash node in 'atf-2' image node
  spl_load_simple_fit: can't load image loadables index 1 (ret = -1)
  mmc_load_image_raw_sector: mmc block read error
  SPL: failed to boot from all boot devices
  ### ERROR ### Please RESET the board ###

Add a device tree property, u-boot,spl-fifo-mode, to control when the
rockchip_sdhci driver should disable the use of DMA and fallback on PIO
mode. Same device tree property is used by the rockchip_dw_mmc driver.

In commit 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks
read in a single command") the DMA mode was disabled using a CONFIG
option on RK3588. Revert that and instead disable DMA using the device
tree property for all RK3588 boards, also apply similar workaround for
all RK3399 boards.

Fixes: 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks read in a single command")
Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
Tested-by: Quentin Schulz <foss+uboot@0leil.net> # RK3399 Puma, RK3588 Tiger
---
v2:
- Rebase on top of defconfig and spi v2 series
- Collect r-b and t-b tags

 arch/arm/dts/rk3399-u-boot.dtsi  | 1 +
 arch/arm/dts/rk3588s-u-boot.dtsi | 1 +
 configs/rock5b-rk3588_defconfig  | 1 -
 drivers/mmc/rockchip_sdhci.c     | 8 ++++++++
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
index e677ae678dab..3423b882c437 100644
--- a/arch/arm/dts/rk3399-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -120,6 +120,7 @@
 &sdhci {
 	max-frequency = <200000000>;
 	bootph-all;
+	u-boot,spl-fifo-mode;
 };
 
 &sdmmc {
diff --git a/arch/arm/dts/rk3588s-u-boot.dtsi b/arch/arm/dts/rk3588s-u-boot.dtsi
index 64c309046587..c703e41802b6 100644
--- a/arch/arm/dts/rk3588s-u-boot.dtsi
+++ b/arch/arm/dts/rk3588s-u-boot.dtsi
@@ -239,6 +239,7 @@
 
 &sdhci {
 	bootph-pre-ram;
+	u-boot,spl-fifo-mode;
 };
 
 &uart2 {
diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index 9d0b55c01ac9..c1155c20efa8 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -66,7 +66,6 @@ CONFIG_MMC_DW=y
 CONFIG_MMC_DW_ROCKCHIP=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_SDMA=y
-# CONFIG_SPL_MMC_SDHCI_SDMA is not set
 CONFIG_MMC_SDHCI_ROCKCHIP=y
 CONFIG_SPI_FLASH_MACRONIX=y
 CONFIG_SPI_FLASH_XTX=y
diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
index 8e4a158049a9..285332d9f4fd 100644
--- a/drivers/mmc/rockchip_sdhci.c
+++ b/drivers/mmc/rockchip_sdhci.c
@@ -589,6 +589,14 @@ static int rockchip_sdhci_probe(struct udevice *dev)
 	if (ret)
 		return ret;
 
+	/*
+	 * Disable use of DMA and force use of PIO mode in SPL to fix an issue
+	 * where loading part of TF-A into SRAM using DMA silently fails.
+	 */
+	if (IS_ENABLED(CONFIG_SPL_BUILD) &&
+	    dev_read_bool(dev, "u-boot,spl-fifo-mode"))
+		host->flags &= ~USE_DMA;
+
 	/*
 	 * Reading more than 4 blocks with a single CMD18 command in PIO mode
 	 * triggers Data End Bit Error on RK3568 and RK3588. Limit to reading
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 1/5] mmc: rockchip_sdhci: Skip blocks read workaround on RK3399 Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 2/5] mmc: rockchip_sdhci: Disable DMA mode using a device tree property Jonas Karlman
@ 2023-05-17 18:40 ` Jonas Karlman
  2023-05-17 19:07   ` Peter Robinson
  2023-05-17 18:40 ` [PATCH v2 4/5] rockchip: rock-pi-4: " Jonas Karlman
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar, Jagan Teki
  Cc: Eugen Hristev, u-boot, Jonas Karlman

Enable the use of SDMA mode to boost eMMC performance on RockPro64.
Also add missing flags to indicate the supported MMC modes.

Using mmc read command to read 32 MiB data shows following improvement:

  => time mmc read 10000000 2000 10000

Before: time: 3.178 seconds
After: time: 0.402 seconds

This also enables CONFIG_SPL_FIT_SIGNATURE option to help discover
any possible future issue with loading TF-A into DRAM/SRAM.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Collect r-b tag

 arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 5 +++++
 configs/rockpro64-rk3399_defconfig        | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
index 32a83b2855ac..bd864d067018 100644
--- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
@@ -15,6 +15,11 @@
 	};
 };
 
+&sdhci {
+	cap-mmc-highspeed;
+	mmc-ddr-1_8v;
+};
+
 &spi1 {
 	spi_flash: flash@0 {
 		bootph-all;
diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
index 2b89b1baba51..0ca2cecade25 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -20,6 +20,7 @@ CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0x800800
 CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
+CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_USE_PREBOOT=y
@@ -63,6 +64,7 @@ CONFIG_ROCKCHIP_EFUSE=y
 CONFIG_MMC_DW=y
 CONFIG_MMC_DW_ROCKCHIP=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_ROCKCHIP=y
 CONFIG_SF_DEFAULT_BUS=1
 CONFIG_SPI_FLASH_GIGADEVICE=y
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 4/5] rockchip: rock-pi-4: Use SDMA to boost eMMC performance
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
                   ` (2 preceding siblings ...)
  2023-05-17 18:40 ` [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance Jonas Karlman
@ 2023-05-17 18:40 ` Jonas Karlman
  2023-05-17 18:40 ` [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin Jonas Karlman
  2023-05-17 19:05 ` [PATCH v2 0/5] rockchip: Fix eMMC performance regression Peter Robinson
  5 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar, Jagan Teki
  Cc: Eugen Hristev, u-boot, Jonas Karlman

Enable the use of SDMA mode to boost eMMC performance on ROCK Pi 4.
Also add missing flags to indicate the supported MMC modes.

Using mmc read command to read 32 MiB data shows following improvement:

  => time mmc read 10000000 2000 10000

Before: time: 3.178 seconds
After: time: 0.402 seconds

This also enables CONFIG_SPL_FIT_SIGNATURE option to help discover
any possible future issue with loading TF-A into DRAM/SRAM.

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Rebase to fix conflict with CONFIG_DEFAULT_FDT_FILE
- Collect r-b tag

 arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi | 6 ++++++
 configs/rock-pi-4-rk3399_defconfig        | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi b/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
index c17e769f649f..60122f3bcd6c 100644
--- a/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi
@@ -12,6 +12,12 @@
 	};
 };
 
+&sdhci {
+	cap-mmc-highspeed;
+	mmc-ddr-1_8v;
+	mmc-hs200-1_8v;
+};
+
 &vdd_log {
 	regulator-init-microvolt = <950000>;
 };
diff --git a/configs/rock-pi-4-rk3399_defconfig b/configs/rock-pi-4-rk3399_defconfig
index cd93093cd2d3..4b984adc6ef8 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -19,6 +19,7 @@ CONFIG_SYS_LOAD_ADDR=0x800800
 CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
 # CONFIG_ANDROID_BOOT_IMAGE is not set
+CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_MISC_INIT_R=y
@@ -56,6 +57,7 @@ CONFIG_ROCKCHIP_EFUSE=y
 CONFIG_MMC_DW=y
 CONFIG_MMC_DW_ROCKCHIP=y
 CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
 CONFIG_MMC_SDHCI_ROCKCHIP=y
 CONFIG_ETH_DESIGNWARE=y
 CONFIG_GMAC_ROCKCHIP=y
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
                   ` (3 preceding siblings ...)
  2023-05-17 18:40 ` [PATCH v2 4/5] rockchip: rock-pi-4: " Jonas Karlman
@ 2023-05-17 18:40 ` Jonas Karlman
  2023-05-17 19:14   ` Peter Robinson
  2023-06-11 20:22   ` Peter Robinson
  2023-05-17 19:05 ` [PATCH v2 0/5] rockchip: Fix eMMC performance regression Peter Robinson
  5 siblings, 2 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 18:40 UTC (permalink / raw)
  To: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar, Jagan Teki
  Cc: Eugen Hristev, u-boot, Jonas Karlman

Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.

  => sf probe
  SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
  => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
  1442304 bytes read in 27 ms (50.9 MiB/s)
  => sf update $fileaddr 0 $filesize
  device 0 offset 0x0, size 0x160200
  1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s

Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
---
v2:
- Collect r-b tag

 configs/rockpro64-rk3399_defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
index 0ca2cecade25..f41c03067903 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
 CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
 CONFIG_DM_RESET=y
 CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
 CONFIG_TARGET_ROCKPRO64_RK3399=y
 CONFIG_SPL_STACK=0x400000
 CONFIG_DEBUG_UART_BASE=0xFF1A0000
@@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0x800800
 CONFIG_PCI=y
 CONFIG_DEBUG_UART=y
+CONFIG_LTO=y
 CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
@@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
 CONFIG_SPL_SPI_LOAD=y
+CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
 CONFIG_TPL=y
 CONFIG_CMD_BOOTZ=y
 CONFIG_CMD_GPT=y
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/5] rockchip: Fix eMMC performance regression
  2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
                   ` (4 preceding siblings ...)
  2023-05-17 18:40 ` [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin Jonas Karlman
@ 2023-05-17 19:05 ` Peter Robinson
  2023-05-17 19:16   ` Jonas Karlman
  5 siblings, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-05-17 19:05 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Eugen Hristev, u-boot

On Wed, May 17, 2023 at 7:40 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> The eMMC performance on RK3399 was reduced sigificant by the
> commit 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks read
> in a single command").
>
> That workaround should only have been applied to RK3568 and RK3588.
> This series fixes that and also help boost eMMC performance on two
> RK3399 boards even more by enabling use of SDMA.

Is there a reason to do this on just two devices?

> There is also an extra commit to help build a u-boot-rockchip-spi.bin
> image that can be used for SPI flash boot on RockPro64.
>
> Changes in v2:
> - Rebase on top of defconfig and spi v2 series
> - Collect r-b and t-b tags
>
> Jonas Karlman (5):
>   mmc: rockchip_sdhci: Skip blocks read workaround on RK3399
>   mmc: rockchip_sdhci: Disable DMA mode using a device tree property
>   rockchip: rockpro64: Use SDMA to boost eMMC performance
>   rockchip: rock-pi-4: Use SDMA to boost eMMC performance
>   rockchip: rockpro64: Build u-boot-rockchip-spi.bin
>
>  arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi |  6 ++++++
>  arch/arm/dts/rk3399-rockpro64-u-boot.dtsi |  5 +++++
>  arch/arm/dts/rk3399-u-boot.dtsi           |  1 +
>  arch/arm/dts/rk3588s-u-boot.dtsi          |  1 +
>  configs/rock-pi-4-rk3399_defconfig        |  2 ++
>  configs/rock5b-rk3588_defconfig           |  1 -
>  configs/rockpro64-rk3399_defconfig        |  5 +++++
>  drivers/mmc/rockchip_sdhci.c              | 12 +++++++++++-
>  8 files changed, 31 insertions(+), 2 deletions(-)
>
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance
  2023-05-17 18:40 ` [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance Jonas Karlman
@ 2023-05-17 19:07   ` Peter Robinson
  2023-05-17 19:28     ` Jonas Karlman
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-05-17 19:07 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar,
	Jagan Teki, Eugen Hristev, u-boot

On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Enable the use of SDMA mode to boost eMMC performance on RockPro64.
> Also add missing flags to indicate the supported MMC modes.
>
> Using mmc read command to read 32 MiB data shows following improvement:
>
>   => time mmc read 10000000 2000 10000
>
> Before: time: 3.178 seconds
> After: time: 0.402 seconds
>
> This also enables CONFIG_SPL_FIT_SIGNATURE option to help discover
> any possible future issue with loading TF-A into DRAM/SRAM.
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> v2:
> - Collect r-b tag
>
>  arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 5 +++++
>  configs/rockpro64-rk3399_defconfig        | 2 ++
>  2 files changed, 7 insertions(+)
>
> diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> index 32a83b2855ac..bd864d067018 100644
> --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> @@ -15,6 +15,11 @@
>         };
>  };
>
> +&sdhci {
> +       cap-mmc-highspeed;
> +       mmc-ddr-1_8v;
> +};

Has this been submitted for the upstream Linux kernel DT? The
u-boot.dtsi isn't meant to be a general dumping ground for things that
should be going upstream. Does it work with the Linux kernel as well
as those people that boot using firmware provided DT will get this as
well.

Peter

>  &spi1 {
>         spi_flash: flash@0 {
>                 bootph-all;
> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> index 2b89b1baba51..0ca2cecade25 100644
> --- a/configs/rockpro64-rk3399_defconfig
> +++ b/configs/rockpro64-rk3399_defconfig
> @@ -20,6 +20,7 @@ CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0x800800
>  CONFIG_PCI=y
>  CONFIG_DEBUG_UART=y
> +CONFIG_SPL_FIT_SIGNATURE=y
>  CONFIG_BOOTSTAGE=y
>  CONFIG_BOOTSTAGE_REPORT=y
>  CONFIG_USE_PREBOOT=y
> @@ -63,6 +64,7 @@ CONFIG_ROCKCHIP_EFUSE=y
>  CONFIG_MMC_DW=y
>  CONFIG_MMC_DW_ROCKCHIP=y
>  CONFIG_MMC_SDHCI=y
> +CONFIG_MMC_SDHCI_SDMA=y
>  CONFIG_MMC_SDHCI_ROCKCHIP=y
>  CONFIG_SF_DEFAULT_BUS=1
>  CONFIG_SPI_FLASH_GIGADEVICE=y
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-05-17 18:40 ` [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin Jonas Karlman
@ 2023-05-17 19:14   ` Peter Robinson
  2023-05-17 19:52     ` Jonas Karlman
  2023-06-11 20:22   ` Peter Robinson
  1 sibling, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-05-17 19:14 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar,
	Jagan Teki, Eugen Hristev, u-boot

On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected

None of the other rk33* devices enable this offset yet my Pinebook Pro
works fine booting from SPI flash, what does this fix/enable/change
over the defaults?

> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.

The enabling of LTO seems like a separate change TBH, the changes seem
to be independent and there's no mention of it in the subject.

>   => sf probe
>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>   1442304 bytes read in 27 ms (50.9 MiB/s)
>   => sf update $fileaddr 0 $filesize
>   device 0 offset 0x0, size 0x160200
>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> v2:
> - Collect r-b tag
>
>  configs/rockpro64-rk3399_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> index 0ca2cecade25..f41c03067903 100644
> --- a/configs/rockpro64-rk3399_defconfig
> +++ b/configs/rockpro64-rk3399_defconfig
> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>  CONFIG_DM_RESET=y
>  CONFIG_ROCKCHIP_RK3399=y
> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>  CONFIG_SPL_STACK=0x400000
>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0x800800
>  CONFIG_PCI=y
>  CONFIG_DEBUG_UART=y
> +CONFIG_LTO=y
>  CONFIG_SPL_FIT_SIGNATURE=y
>  CONFIG_BOOTSTAGE=y
>  CONFIG_BOOTSTAGE_REPORT=y
> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>  CONFIG_SPL_SPI_LOAD=y
> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>  CONFIG_TPL=y
>  CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_GPT=y
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 0/5] rockchip: Fix eMMC performance regression
  2023-05-17 19:05 ` [PATCH v2 0/5] rockchip: Fix eMMC performance regression Peter Robinson
@ 2023-05-17 19:16   ` Jonas Karlman
  0 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 19:16 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Eugen Hristev, u-boot

Hi Peter,
On 2023-05-17 21:05, Peter Robinson wrote:
> On Wed, May 17, 2023 at 7:40 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> The eMMC performance on RK3399 was reduced sigificant by the
>> commit 2cc6cde647e2 ("mmc: rockchip_sdhci: Limit number of blocks read
>> in a single command").
>>
>> That workaround should only have been applied to RK3568 and RK3588.
>> This series fixes that and also help boost eMMC performance on two
>> RK3399 boards even more by enabling use of SDMA.
> 
> Is there a reason to do this on just two devices?

DMA use can and should probably be enabled on other RK3399 devices,
these two where the only ones I could runtime test on.

Regards,
Jonas

> 
>> There is also an extra commit to help build a u-boot-rockchip-spi.bin
>> image that can be used for SPI flash boot on RockPro64.
>>
>> Changes in v2:
>> - Rebase on top of defconfig and spi v2 series
>> - Collect r-b and t-b tags
>>
>> Jonas Karlman (5):
>>   mmc: rockchip_sdhci: Skip blocks read workaround on RK3399
>>   mmc: rockchip_sdhci: Disable DMA mode using a device tree property
>>   rockchip: rockpro64: Use SDMA to boost eMMC performance
>>   rockchip: rock-pi-4: Use SDMA to boost eMMC performance
>>   rockchip: rockpro64: Build u-boot-rockchip-spi.bin
>>
>>  arch/arm/dts/rk3399-rock-pi-4-u-boot.dtsi |  6 ++++++
>>  arch/arm/dts/rk3399-rockpro64-u-boot.dtsi |  5 +++++
>>  arch/arm/dts/rk3399-u-boot.dtsi           |  1 +
>>  arch/arm/dts/rk3588s-u-boot.dtsi          |  1 +
>>  configs/rock-pi-4-rk3399_defconfig        |  2 ++
>>  configs/rock5b-rk3588_defconfig           |  1 -
>>  configs/rockpro64-rk3399_defconfig        |  5 +++++
>>  drivers/mmc/rockchip_sdhci.c              | 12 +++++++++++-
>>  8 files changed, 31 insertions(+), 2 deletions(-)
>>
>> --
>> 2.40.1
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance
  2023-05-17 19:07   ` Peter Robinson
@ 2023-05-17 19:28     ` Jonas Karlman
  0 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 19:28 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar,
	Jagan Teki, Eugen Hristev, u-boot

Hi Peter,

On 2023-05-17 21:07, Peter Robinson wrote:
> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Enable the use of SDMA mode to boost eMMC performance on RockPro64.
>> Also add missing flags to indicate the supported MMC modes.
>>
>> Using mmc read command to read 32 MiB data shows following improvement:
>>
>>   => time mmc read 10000000 2000 10000
>>
>> Before: time: 3.178 seconds
>> After: time: 0.402 seconds
>>
>> This also enables CONFIG_SPL_FIT_SIGNATURE option to help discover
>> any possible future issue with loading TF-A into DRAM/SRAM.
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>> v2:
>> - Collect r-b tag
>>
>>  arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 5 +++++
>>  configs/rockpro64-rk3399_defconfig        | 2 ++
>>  2 files changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>> index 32a83b2855ac..bd864d067018 100644
>> --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>> +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
>> @@ -15,6 +15,11 @@
>>         };
>>  };
>>
>> +&sdhci {
>> +       cap-mmc-highspeed;
>> +       mmc-ddr-1_8v;
>> +};
> 
> Has this been submitted for the upstream Linux kernel DT? The
> u-boot.dtsi isn't meant to be a general dumping ground for things that
> should be going upstream. Does it work with the Linux kernel as well
> as those people that boot using firmware provided DT will get this as
> well.

It has not been submitted for upstream linux device tree yet. I have
patches pending for rk3399-rockpro64 and some rk35xx devices in my local
tree, hoping to have them submitted any day now.

This should work with linux, however linux should pick the hs200 mode
over ddr52 mode.

With hs200 mode Kconfig enabled in u-boot the above test cmd result
in ~0.2 seconds.

Regards,
Jonas

> 
> Peter
> 
>>  &spi1 {
>>         spi_flash: flash@0 {
>>                 bootph-all;
>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>> index 2b89b1baba51..0ca2cecade25 100644
>> --- a/configs/rockpro64-rk3399_defconfig
>> +++ b/configs/rockpro64-rk3399_defconfig
>> @@ -20,6 +20,7 @@ CONFIG_SPL_SPI=y
>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>  CONFIG_PCI=y
>>  CONFIG_DEBUG_UART=y
>> +CONFIG_SPL_FIT_SIGNATURE=y
>>  CONFIG_BOOTSTAGE=y
>>  CONFIG_BOOTSTAGE_REPORT=y
>>  CONFIG_USE_PREBOOT=y
>> @@ -63,6 +64,7 @@ CONFIG_ROCKCHIP_EFUSE=y
>>  CONFIG_MMC_DW=y
>>  CONFIG_MMC_DW_ROCKCHIP=y
>>  CONFIG_MMC_SDHCI=y
>> +CONFIG_MMC_SDHCI_SDMA=y
>>  CONFIG_MMC_SDHCI_ROCKCHIP=y
>>  CONFIG_SF_DEFAULT_BUS=1
>>  CONFIG_SPI_FLASH_GIGADEVICE=y
>> --
>> 2.40.1
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-05-17 19:14   ` Peter Robinson
@ 2023-05-17 19:52     ` Jonas Karlman
  0 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-05-17 19:52 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Akash Gajjar,
	Jagan Teki, Eugen Hristev, u-boot

Hi Peter,

On 2023-05-17 21:14, Peter Robinson wrote:
> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
> 
> None of the other rk33* devices enable this offset yet my Pinebook Pro
> works fine booting from SPI flash, what does this fix/enable/change
> over the defaults?

Most other RK3399 devices define the offset in the device tree, even for
the rockpro64. However, the CONFIG_SYS_SPI_U_BOOT_OFFS is used as the
fallback when device tree value is missing, and also as the offset used
when generating the u-boot-rockchip-spi.bin using binman.

puma-rk3399_defconfig is the only other RK3399 board that also generates
a u-boot-rockchip-spi.bin, has CONFIG_ROCKCHIP_SPI_IMAGE=y. That board
override the simple-bin-spi fit offset in rk3399-puma-haikou-u-boot.dtsi
instead of using the Kconfig option.

> 
>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
> 
> The enabling of LTO seems like a separate change TBH, the changes seem
> to be independent and there's no mention of it in the subject.

Without LTO enabled the idbloader.img grows too large that it does not
fit before the u-boot.itb payload at 0x60000 and bulding of
u-boot-rockchip-spi.bin fails the u-boot build.

In order to generate a valid u-boot-rockchip-spi.bin, LTO was required
to be enabled, there is also a short mention of it in the commit message.
The alternative would be to move the payload offset to e.g. 0x80000 but
that felt like a too big and risky change.

Regards,
Jonas

> 
>>   => sf probe
>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>>   1442304 bytes read in 27 ms (50.9 MiB/s)
>>   => sf update $fileaddr 0 $filesize
>>   device 0 offset 0x0, size 0x160200
>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>> v2:
>> - Collect r-b tag
>>
>>  configs/rockpro64-rk3399_defconfig | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>> index 0ca2cecade25..f41c03067903 100644
>> --- a/configs/rockpro64-rk3399_defconfig
>> +++ b/configs/rockpro64-rk3399_defconfig
>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>>  CONFIG_DM_RESET=y
>>  CONFIG_ROCKCHIP_RK3399=y
>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>>  CONFIG_SPL_STACK=0x400000
>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>  CONFIG_PCI=y
>>  CONFIG_DEBUG_UART=y
>> +CONFIG_LTO=y
>>  CONFIG_SPL_FIT_SIGNATURE=y
>>  CONFIG_BOOTSTAGE=y
>>  CONFIG_BOOTSTAGE_REPORT=y
>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>>  CONFIG_SPL_STACK_R=y
>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>>  CONFIG_SPL_SPI_LOAD=y
>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>>  CONFIG_TPL=y
>>  CONFIG_CMD_BOOTZ=y
>>  CONFIG_CMD_GPT=y
>> --
>> 2.40.1
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-05-17 18:40 ` [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin Jonas Karlman
  2023-05-17 19:14   ` Peter Robinson
@ 2023-06-11 20:22   ` Peter Robinson
  2023-06-12 22:20     ` Jonas Karlman
  1 sibling, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-06-11 20:22 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

Hi Jonas,

This regresses the Rockpro64 build for me when building with gcc 12/13
with the following error, if I remove CONFIG_LTO it builds, but
overlaps.

/usr/bin/aarch64-linux-gnu-ld:
/usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
`init_have_lse_atomics':
/builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
undefined reference to `__getauxval'
collect2: fatal error: ld terminated with signal 11 [Segmentation
fault], core dumped
compilation terminated.

Peter

On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
>
>   => sf probe
>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>   1442304 bytes read in 27 ms (50.9 MiB/s)
>   => sf update $fileaddr 0 $filesize
>   device 0 offset 0x0, size 0x160200
>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>
> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> ---
> v2:
> - Collect r-b tag
>
>  configs/rockpro64-rk3399_defconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> index 0ca2cecade25..f41c03067903 100644
> --- a/configs/rockpro64-rk3399_defconfig
> +++ b/configs/rockpro64-rk3399_defconfig
> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>  CONFIG_DM_RESET=y
>  CONFIG_ROCKCHIP_RK3399=y
> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>  CONFIG_SPL_STACK=0x400000
>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0x800800
>  CONFIG_PCI=y
>  CONFIG_DEBUG_UART=y
> +CONFIG_LTO=y
>  CONFIG_SPL_FIT_SIGNATURE=y
>  CONFIG_BOOTSTAGE=y
>  CONFIG_BOOTSTAGE_REPORT=y
> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>  CONFIG_SPL_STACK_R=y
>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>  CONFIG_SPL_SPI_LOAD=y
> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>  CONFIG_TPL=y
>  CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_GPT=y
> --
> 2.40.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-06-11 20:22   ` Peter Robinson
@ 2023-06-12 22:20     ` Jonas Karlman
  2023-06-13  7:58       ` Peter Robinson
  0 siblings, 1 reply; 18+ messages in thread
From: Jonas Karlman @ 2023-06-12 22:20 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

Hi Peter,

On 2023-06-11 22:22, Peter Robinson wrote:
> Hi Jonas,
> 
> This regresses the Rockpro64 build for me when building with gcc 12/13
> with the following error, if I remove CONFIG_LTO it builds, but
> overlaps.
> 
> /usr/bin/aarch64-linux-gnu-ld:
> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
> `init_have_lse_atomics':
> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
> undefined reference to `__getauxval'
> collect2: fatal error: ld terminated with signal 11 [Segmentation
> fault], core dumped
> compilation terminated.

Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
sure that is a good workaround.

An alternative could be to move the payload to @ 512KB instead of @ 384KB.

configs/rockpro64-rk3399_defconfig:
  CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000

arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
  u-boot,spl-payload-offset = <0x80000>;


[1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969

Regards,
Jonas

> 
> Peter
> 
> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
>>
>>   => sf probe
>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>>   1442304 bytes read in 27 ms (50.9 MiB/s)
>>   => sf update $fileaddr 0 $filesize
>>   device 0 offset 0x0, size 0x160200
>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>>
>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>> v2:
>> - Collect r-b tag
>>
>>  configs/rockpro64-rk3399_defconfig | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>> index 0ca2cecade25..f41c03067903 100644
>> --- a/configs/rockpro64-rk3399_defconfig
>> +++ b/configs/rockpro64-rk3399_defconfig
>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>>  CONFIG_DM_RESET=y
>>  CONFIG_ROCKCHIP_RK3399=y
>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>>  CONFIG_SPL_STACK=0x400000
>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>  CONFIG_PCI=y
>>  CONFIG_DEBUG_UART=y
>> +CONFIG_LTO=y
>>  CONFIG_SPL_FIT_SIGNATURE=y
>>  CONFIG_BOOTSTAGE=y
>>  CONFIG_BOOTSTAGE_REPORT=y
>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>>  CONFIG_SPL_STACK_R=y
>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>>  CONFIG_SPL_SPI_LOAD=y
>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>>  CONFIG_TPL=y
>>  CONFIG_CMD_BOOTZ=y
>>  CONFIG_CMD_GPT=y
>> --
>> 2.40.1
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-06-12 22:20     ` Jonas Karlman
@ 2023-06-13  7:58       ` Peter Robinson
  2023-06-13 18:48         ` Jonas Karlman
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-06-13  7:58 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Hi Peter,
>
> On 2023-06-11 22:22, Peter Robinson wrote:
> > Hi Jonas,
> >
> > This regresses the Rockpro64 build for me when building with gcc 12/13
> > with the following error, if I remove CONFIG_LTO it builds, but
> > overlaps.
> >
> > /usr/bin/aarch64-linux-gnu-ld:
> > /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
> > `init_have_lse_atomics':
> > /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
> > undefined reference to `__getauxval'
> > collect2: fatal error: ld terminated with signal 11 [Segmentation
> > fault], core dumped
> > compilation terminated.
>
> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
> sure that is a good workaround.

I would prefer a fix.

> An alternative could be to move the payload to @ 512KB instead of @ 384KB.

What impact will moving this around have on upgrades? I am not sure we
should be randomly moving stuff without knowing the impact TBH. Will
this break existing users vs what you feel is appropriate for your
usecase?

> configs/rockpro64-rk3399_defconfig:
>   CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000
>
> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
>   u-boot,spl-payload-offset = <0x80000>;
>
>
> [1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969
>
> Regards,
> Jonas
>
> >
> > Peter
> >
> > On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
> >>
> >> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
> >> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
> >> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
> >> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
> >>
> >>   => sf probe
> >>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
> >>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
> >>   1442304 bytes read in 27 ms (50.9 MiB/s)
> >>   => sf update $fileaddr 0 $filesize
> >>   device 0 offset 0x0, size 0x160200
> >>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
> >>
> >> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> >> ---
> >> v2:
> >> - Collect r-b tag
> >>
> >>  configs/rockpro64-rk3399_defconfig | 3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> >> index 0ca2cecade25..f41c03067903 100644
> >> --- a/configs/rockpro64-rk3399_defconfig
> >> +++ b/configs/rockpro64-rk3399_defconfig
> >> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
> >>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
> >>  CONFIG_DM_RESET=y
> >>  CONFIG_ROCKCHIP_RK3399=y
> >> +CONFIG_ROCKCHIP_SPI_IMAGE=y
> >>  CONFIG_TARGET_ROCKPRO64_RK3399=y
> >>  CONFIG_SPL_STACK=0x400000
> >>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
> >> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
> >>  CONFIG_SYS_LOAD_ADDR=0x800800
> >>  CONFIG_PCI=y
> >>  CONFIG_DEBUG_UART=y
> >> +CONFIG_LTO=y
> >>  CONFIG_SPL_FIT_SIGNATURE=y
> >>  CONFIG_BOOTSTAGE=y
> >>  CONFIG_BOOTSTAGE_REPORT=y
> >> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
> >>  CONFIG_SPL_STACK_R=y
> >>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
> >>  CONFIG_SPL_SPI_LOAD=y
> >> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
> >>  CONFIG_TPL=y
> >>  CONFIG_CMD_BOOTZ=y
> >>  CONFIG_CMD_GPT=y
> >> --
> >> 2.40.1
> >>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-06-13  7:58       ` Peter Robinson
@ 2023-06-13 18:48         ` Jonas Karlman
  2023-06-14  7:46           ` Peter Robinson
  0 siblings, 1 reply; 18+ messages in thread
From: Jonas Karlman @ 2023-06-13 18:48 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

Hi Peter,

On 2023-06-13 09:58, Peter Robinson wrote:
> On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Hi Peter,
>>
>> On 2023-06-11 22:22, Peter Robinson wrote:
>>> Hi Jonas,
>>>
>>> This regresses the Rockpro64 build for me when building with gcc 12/13
>>> with the following error, if I remove CONFIG_LTO it builds, but
>>> overlaps.
>>>
>>> /usr/bin/aarch64-linux-gnu-ld:
>>> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
>>> `init_have_lse_atomics':
>>> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
>>> undefined reference to `__getauxval'
>>> collect2: fatal error: ld terminated with signal 11 [Segmentation
>>> fault], core dumped
>>> compilation terminated.
>>
>> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
>> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
>> sure that is a good workaround.
> 
> I would prefer a fix.

Not sure what such fix could be, cross compiling in CI and under ubuntu
22.04 does not trigger this linking issue, not sure what is different in
your build environment to not allow build with CONFIG_LTO=y.

Searching for similar linking issue suggest it could be related to
outline-atomics, do you use any special C/LDFLAGS or similar?

> 
>> An alternative could be to move the payload to @ 512KB instead of @ 384KB.
> 
> What impact will moving this around have on upgrades? I am not sure we
> should be randomly moving stuff without knowing the impact TBH. Will
> this break existing users vs what you feel is appropriate for your
> usecase?

I did a re-calculation and using @ 512 KB offset may not be enough
for a worst-case scenario.

mkimage will enforce a 184 KB limit for the TPL on RK3399. With SPL
loaded at 0x0 and TF-A loaded to 0x40000, we have a 256 KB size limit of
SPL. BootRom that loads both TPL and SPL only read first 2 KB of every
4 KB page of SPI flash, so idbloader.img (TPL+SPL) could grow up to
~880 KB in a worst-case scenario. (184+256) x 2 = 880

Using a payload offset of @ 896 KB (0xE0000) is probably the safest
default option if we need to revert the CONFIG_LTO=y change.

As long as u-boot-rockchip-spi.bin is used to update SPI flash changing
offset should not have an impact. However, there are guides and scripts
that write idbloader.img and u-boot.itb separately, those could break.

Such guides and scripts can already lead to users overwriting part of
SPL when the produced idbloader.img is over 384 KB in size.
With CONFIG_ROCKCHIP_SPI_IMAGE=y the size of idbloader.img is instead
enforced to not cause an overlap of the u-boot.itb at build time.

Regards,
Jonas

> 
>> configs/rockpro64-rk3399_defconfig:
>>   CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000
>>
>> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
>>   u-boot,spl-payload-offset = <0x80000>;
>>
>>
>> [1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969
>>
>> Regards,
>> Jonas
>>
>>>
>>> Peter
>>>
>>> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>>>
>>>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
>>>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
>>>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
>>>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
>>>>
>>>>   => sf probe
>>>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>>>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>>>>   1442304 bytes read in 27 ms (50.9 MiB/s)
>>>>   => sf update $fileaddr 0 $filesize
>>>>   device 0 offset 0x0, size 0x160200
>>>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>>>>
>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>>>> ---
>>>> v2:
>>>> - Collect r-b tag
>>>>
>>>>  configs/rockpro64-rk3399_defconfig | 3 +++
>>>>  1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>>>> index 0ca2cecade25..f41c03067903 100644
>>>> --- a/configs/rockpro64-rk3399_defconfig
>>>> +++ b/configs/rockpro64-rk3399_defconfig
>>>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>>>>  CONFIG_DM_RESET=y
>>>>  CONFIG_ROCKCHIP_RK3399=y
>>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>>>>  CONFIG_SPL_STACK=0x400000
>>>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>>>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>>>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>>>  CONFIG_PCI=y
>>>>  CONFIG_DEBUG_UART=y
>>>> +CONFIG_LTO=y
>>>>  CONFIG_SPL_FIT_SIGNATURE=y
>>>>  CONFIG_BOOTSTAGE=y
>>>>  CONFIG_BOOTSTAGE_REPORT=y
>>>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>>>>  CONFIG_SPL_STACK_R=y
>>>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>>>>  CONFIG_SPL_SPI_LOAD=y
>>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>>>>  CONFIG_TPL=y
>>>>  CONFIG_CMD_BOOTZ=y
>>>>  CONFIG_CMD_GPT=y
>>>> --
>>>> 2.40.1
>>>>
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-06-13 18:48         ` Jonas Karlman
@ 2023-06-14  7:46           ` Peter Robinson
  2023-06-14 17:22             ` Jonas Karlman
  0 siblings, 1 reply; 18+ messages in thread
From: Peter Robinson @ 2023-06-14  7:46 UTC (permalink / raw)
  To: Jonas Karlman
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>
> Hi Peter,
>
> On 2023-06-13 09:58, Peter Robinson wrote:
> > On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@kwiboo.se> wrote:
> >>
> >> Hi Peter,
> >>
> >> On 2023-06-11 22:22, Peter Robinson wrote:
> >>> Hi Jonas,
> >>>
> >>> This regresses the Rockpro64 build for me when building with gcc 12/13
> >>> with the following error, if I remove CONFIG_LTO it builds, but
> >>> overlaps.
> >>>
> >>> /usr/bin/aarch64-linux-gnu-ld:
> >>> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
> >>> `init_have_lse_atomics':
> >>> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
> >>> undefined reference to `__getauxval'
> >>> collect2: fatal error: ld terminated with signal 11 [Segmentation
> >>> fault], core dumped
> >>> compilation terminated.
> >>
> >> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
> >> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
> >> sure that is a good workaround.
> >
> > I would prefer a fix.
>
> Not sure what such fix could be, cross compiling in CI and under ubuntu
> 22.04 does not trigger this linking issue, not sure what is different in
> your build environment to not allow build with CONFIG_LTO=y.
>
> Searching for similar linking issue suggest it could be related to
> outline-atomics, do you use any special C/LDFLAGS or similar?
>
> >
> >> An alternative could be to move the payload to @ 512KB instead of @ 384KB.
> >
> > What impact will moving this around have on upgrades? I am not sure we
> > should be randomly moving stuff without knowing the impact TBH. Will
> > this break existing users vs what you feel is appropriate for your
> > usecase?
>
> I did a re-calculation and using @ 512 KB offset may not be enough
> for a worst-case scenario.
>
> mkimage will enforce a 184 KB limit for the TPL on RK3399. With SPL
> loaded at 0x0 and TF-A loaded to 0x40000, we have a 256 KB size limit of
> SPL. BootRom that loads both TPL and SPL only read first 2 KB of every
> 4 KB page of SPI flash, so idbloader.img (TPL+SPL) could grow up to
> ~880 KB in a worst-case scenario. (184+256) x 2 = 880
>
> Using a payload offset of @ 896 KB (0xE0000) is probably the safest
> default option if we need to revert the CONFIG_LTO=y change.
>
> As long as u-boot-rockchip-spi.bin is used to update SPI flash changing
> offset should not have an impact. However, there are guides and scripts
> that write idbloader.img and u-boot.itb separately, those could break.

Well I believe your patch only enabled that option by default so most
users I suspect write them separately up until now, does the new
offsets affect existing env variables at all?

> Such guides and scripts can already lead to users overwriting part of
> SPL when the produced idbloader.img is over 384 KB in size.
> With CONFIG_ROCKCHIP_SPI_IMAGE=y the size of idbloader.img is instead
> enforced to not cause an overlap of the u-boot.itb at build time.

Ultimately I would prefer to not actively break existing users if
they're not aware of changes, and that is hard to communicate. People
with broken devices tend to scream a lot.

> Regards,
> Jonas
>
> >
> >> configs/rockpro64-rk3399_defconfig:
> >>   CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000
> >>
> >> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
> >>   u-boot,spl-payload-offset = <0x80000>;
> >>
> >>
> >> [1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969
> >>
> >> Regards,
> >> Jonas
> >>
> >>>
> >>> Peter
> >>>
> >>> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
> >>>>
> >>>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
> >>>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
> >>>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
> >>>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
> >>>>
> >>>>   => sf probe
> >>>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
> >>>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
> >>>>   1442304 bytes read in 27 ms (50.9 MiB/s)
> >>>>   => sf update $fileaddr 0 $filesize
> >>>>   device 0 offset 0x0, size 0x160200
> >>>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
> >>>>
> >>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
> >>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
> >>>> ---
> >>>> v2:
> >>>> - Collect r-b tag
> >>>>
> >>>>  configs/rockpro64-rk3399_defconfig | 3 +++
> >>>>  1 file changed, 3 insertions(+)
> >>>>
> >>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> >>>> index 0ca2cecade25..f41c03067903 100644
> >>>> --- a/configs/rockpro64-rk3399_defconfig
> >>>> +++ b/configs/rockpro64-rk3399_defconfig
> >>>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
> >>>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
> >>>>  CONFIG_DM_RESET=y
> >>>>  CONFIG_ROCKCHIP_RK3399=y
> >>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
> >>>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
> >>>>  CONFIG_SPL_STACK=0x400000
> >>>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
> >>>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
> >>>>  CONFIG_SYS_LOAD_ADDR=0x800800
> >>>>  CONFIG_PCI=y
> >>>>  CONFIG_DEBUG_UART=y
> >>>> +CONFIG_LTO=y
> >>>>  CONFIG_SPL_FIT_SIGNATURE=y
> >>>>  CONFIG_BOOTSTAGE=y
> >>>>  CONFIG_BOOTSTAGE_REPORT=y
> >>>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
> >>>>  CONFIG_SPL_STACK_R=y
> >>>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
> >>>>  CONFIG_SPL_SPI_LOAD=y
> >>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
> >>>>  CONFIG_TPL=y
> >>>>  CONFIG_CMD_BOOTZ=y
> >>>>  CONFIG_CMD_GPT=y
> >>>> --
> >>>> 2.40.1
> >>>>
> >>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin
  2023-06-14  7:46           ` Peter Robinson
@ 2023-06-14 17:22             ` Jonas Karlman
  0 siblings, 0 replies; 18+ messages in thread
From: Jonas Karlman @ 2023-06-14 17:22 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Kever Yang, Simon Glass, Philipp Tomsich, Jagan Teki,
	Eugen Hristev, u-boot

On 2023-06-14 09:46, Peter Robinson wrote:
> On Tue, Jun 13, 2023 at 7:48 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>
>> Hi Peter,
>>
>> On 2023-06-13 09:58, Peter Robinson wrote:
>>> On Mon, Jun 12, 2023 at 11:20 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>>>
>>>> Hi Peter,
>>>>
>>>> On 2023-06-11 22:22, Peter Robinson wrote:
>>>>> Hi Jonas,
>>>>>
>>>>> This regresses the Rockpro64 build for me when building with gcc 12/13
>>>>> with the following error, if I remove CONFIG_LTO it builds, but
>>>>> overlaps.
>>>>>
>>>>> /usr/bin/aarch64-linux-gnu-ld:
>>>>> /usr/lib/gcc/aarch64-linux-gnu/13/libgcc.a(lse-init.o): in function
>>>>> `init_have_lse_atomics':
>>>>> /builddir/build/BUILD/gcc-13.1.1-20230519/aarch64-linux-gnu/aarch64-linux-gnu/libgcc/../../../gcc-13.1.1-20230519/libgcc/config/aarch64/lse-init.c:46:
>>>>> undefined reference to `__getauxval'
>>>>> collect2: fatal error: ld terminated with signal 11 [Segmentation
>>>>> fault], core dumped
>>>>> compilation terminated.
>>>>
>>>> Interesting, my test build using ubuntu-22.04 and gcc-12 did not seem
>>>> to fail, see [1]. Compiling with -mno-outline-atomics flag may work, not
>>>> sure that is a good workaround.
>>>
>>> I would prefer a fix.
>>
>> Not sure what such fix could be, cross compiling in CI and under ubuntu
>> 22.04 does not trigger this linking issue, not sure what is different in
>> your build environment to not allow build with CONFIG_LTO=y.
>>
>> Searching for similar linking issue suggest it could be related to
>> outline-atomics, do you use any special C/LDFLAGS or similar?
>>
>>>
>>>> An alternative could be to move the payload to @ 512KB instead of @ 384KB.
>>>
>>> What impact will moving this around have on upgrades? I am not sure we
>>> should be randomly moving stuff without knowing the impact TBH. Will
>>> this break existing users vs what you feel is appropriate for your
>>> usecase?
>>
>> I did a re-calculation and using @ 512 KB offset may not be enough
>> for a worst-case scenario.
>>
>> mkimage will enforce a 184 KB limit for the TPL on RK3399. With SPL
>> loaded at 0x0 and TF-A loaded to 0x40000, we have a 256 KB size limit of
>> SPL. BootRom that loads both TPL and SPL only read first 2 KB of every
>> 4 KB page of SPI flash, so idbloader.img (TPL+SPL) could grow up to
>> ~880 KB in a worst-case scenario. (184+256) x 2 = 880
>>
>> Using a payload offset of @ 896 KB (0xE0000) is probably the safest
>> default option if we need to revert the CONFIG_LTO=y change.
>>
>> As long as u-boot-rockchip-spi.bin is used to update SPI flash changing
>> offset should not have an impact. However, there are guides and scripts
>> that write idbloader.img and u-boot.itb separately, those could break.
> 
> Well I believe your patch only enabled that option by default so most
> users I suspect write them separately up until now, does the new
> offsets affect existing env variables at all?

It does not look like it will overwrite env offset.

  CONFIG_ENV_SIZE=0x8000
  CONFIG_ENV_OFFSET=0x3F8000

There will be room for a 3168 KB u-boot.itb payload until it would
overlap env in SPI flash with a 0xE0000 offset for u-boot.itb.

Also look like many RK3399 defconfig have a SPL_MAX_SIZE=0x2e000,
something that could be increased to SPL_MAX_SIZE=0x40000 with an
updated offset for u-boot.itb.

I will prepare a small fix series that address your LTO issue and use
proper offsets and max size options.

> 
>> Such guides and scripts can already lead to users overwriting part of
>> SPL when the produced idbloader.img is over 384 KB in size.
>> With CONFIG_ROCKCHIP_SPI_IMAGE=y the size of idbloader.img is instead
>> enforced to not cause an overlap of the u-boot.itb at build time.
> 
> Ultimately I would prefer to not actively break existing users if
> they're not aware of changes, and that is hard to communicate. People
> with broken devices tend to scream a lot.

Such change will not actively break existing users, SPL look for a FIT
header and will fall back to next media in u-boot,spl-boot-order.

Will include an update to SPI flash section of rockchip.rst to mention
how to properly flash to correct offset using u-boot-rockchip-spi.bin
in the fix series.

Regards,
Jonas

> 
>> Regards,
>> Jonas
>>
>>>
>>>> configs/rockpro64-rk3399_defconfig:
>>>>   CONFIG_SYS_SPI_U_BOOT_OFFS=0x80000
>>>>
>>>> arch/arm/dts/rk3399-rockpro64-u-boot.dtsi:
>>>>   u-boot,spl-payload-offset = <0x80000>;
>>>>
>>>>
>>>> [1] https://github.com/Kwiboo/u-boot-build/actions/runs/5248398191/jobs/9479827969
>>>>
>>>> Regards,
>>>> Jonas
>>>>
>>>>>
>>>>> Peter
>>>>>
>>>>> On Wed, May 17, 2023 at 7:41 PM Jonas Karlman <jonas@kwiboo.se> wrote:
>>>>>>
>>>>>> Enable CONFIG_ROCKCHIP_SPI_IMAGE to build u-boot-rockchip-spi.bin.
>>>>>> Define CONFIG_SYS_SPI_U_BOOT_OFFS to write u-boot.itb at the expected
>>>>>> offset. Enable CONFIG_LTO to reduce size of SPL so that the mkimage
>>>>>> output fit before the 0x60000 offset in u-boot-rockchip-spi.bin.
>>>>>>
>>>>>>   => sf probe
>>>>>>   SF: Detected gd25q128 with page size 256 Bytes, erase size 4 KiB, total 16 MiB
>>>>>>   => load mmc 1:1 10000000 u-boot-rockchip-spi.bin
>>>>>>   1442304 bytes read in 27 ms (50.9 MiB/s)
>>>>>>   => sf update $fileaddr 0 $filesize
>>>>>>   device 0 offset 0x0, size 0x160200
>>>>>>   1421824 bytes written, 20480 bytes skipped in 9.501s, speed 155432 B/s
>>>>>>
>>>>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>>>>> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>
>>>>>> ---
>>>>>> v2:
>>>>>> - Collect r-b tag
>>>>>>
>>>>>>  configs/rockpro64-rk3399_defconfig | 3 +++
>>>>>>  1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
>>>>>> index 0ca2cecade25..f41c03067903 100644
>>>>>> --- a/configs/rockpro64-rk3399_defconfig
>>>>>> +++ b/configs/rockpro64-rk3399_defconfig
>>>>>> @@ -11,6 +11,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>>>>>  CONFIG_DEFAULT_DEVICE_TREE="rk3399-rockpro64"
>>>>>>  CONFIG_DM_RESET=y
>>>>>>  CONFIG_ROCKCHIP_RK3399=y
>>>>>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>>>>>  CONFIG_TARGET_ROCKPRO64_RK3399=y
>>>>>>  CONFIG_SPL_STACK=0x400000
>>>>>>  CONFIG_DEBUG_UART_BASE=0xFF1A0000
>>>>>> @@ -20,6 +21,7 @@ CONFIG_SPL_SPI=y
>>>>>>  CONFIG_SYS_LOAD_ADDR=0x800800
>>>>>>  CONFIG_PCI=y
>>>>>>  CONFIG_DEBUG_UART=y
>>>>>> +CONFIG_LTO=y
>>>>>>  CONFIG_SPL_FIT_SIGNATURE=y
>>>>>>  CONFIG_BOOTSTAGE=y
>>>>>>  CONFIG_BOOTSTAGE_REPORT=y
>>>>>> @@ -37,6 +39,7 @@ CONFIG_SPL_BSS_MAX_SIZE=0x2000
>>>>>>  CONFIG_SPL_STACK_R=y
>>>>>>  CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x10000
>>>>>>  CONFIG_SPL_SPI_LOAD=y
>>>>>> +CONFIG_SYS_SPI_U_BOOT_OFFS=0x60000
>>>>>>  CONFIG_TPL=y
>>>>>>  CONFIG_CMD_BOOTZ=y
>>>>>>  CONFIG_CMD_GPT=y
>>>>>> --
>>>>>> 2.40.1
>>>>>>
>>>>
>>


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-06-14 17:22 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-17 18:40 [PATCH v2 0/5] rockchip: Fix eMMC performance regression Jonas Karlman
2023-05-17 18:40 ` [PATCH v2 1/5] mmc: rockchip_sdhci: Skip blocks read workaround on RK3399 Jonas Karlman
2023-05-17 18:40 ` [PATCH v2 2/5] mmc: rockchip_sdhci: Disable DMA mode using a device tree property Jonas Karlman
2023-05-17 18:40 ` [PATCH v2 3/5] rockchip: rockpro64: Use SDMA to boost eMMC performance Jonas Karlman
2023-05-17 19:07   ` Peter Robinson
2023-05-17 19:28     ` Jonas Karlman
2023-05-17 18:40 ` [PATCH v2 4/5] rockchip: rock-pi-4: " Jonas Karlman
2023-05-17 18:40 ` [PATCH v2 5/5] rockchip: rockpro64: Build u-boot-rockchip-spi.bin Jonas Karlman
2023-05-17 19:14   ` Peter Robinson
2023-05-17 19:52     ` Jonas Karlman
2023-06-11 20:22   ` Peter Robinson
2023-06-12 22:20     ` Jonas Karlman
2023-06-13  7:58       ` Peter Robinson
2023-06-13 18:48         ` Jonas Karlman
2023-06-14  7:46           ` Peter Robinson
2023-06-14 17:22             ` Jonas Karlman
2023-05-17 19:05 ` [PATCH v2 0/5] rockchip: Fix eMMC performance regression Peter Robinson
2023-05-17 19:16   ` Jonas Karlman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.