Re: [PATCH 2/2] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when needed
On 6/29/22 05:01, Kever Yang wrote:
>
> On 2022/6/9 23:23, Jerome Forissier wrote:
>> Commit 5c606ca35c42 ("rockchip: rk3399: enable spl-fifo-mode for sdmmc")
>> mentions that the RK3399 SoC can't do DMA between SDMMC and SRAM.
>> According to the TRM "7.3.2 Embedded SRAM access path" [1], only the
>> 8KB SRAM at 0xff3b (INTMEM1) is in this situation. The 192KB SRAM
>> can be accessed by both DMA controllers.
>>
>> Assuming the only use case for writing from MMC to INTMEM1 is loading
>> a FIT image, and with the introduction of a temporary buffer for that
>> purpose (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE, which is required
>> anyways to ensure the destination boundaries are enforced), then
>> spl-fifo-mode is not needed anymore and DMA can be enabled safely.
>>
>> Link: [1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
>> CC: Deepak Das
>> Signed-off-by: Jerome Forissier
>
> Reviewed-by: Kever Yang
Thanks. I will post a v2 with the corrected conditional [#if (xxx == 0)].
--
Jerome
>
> Thanks,
>
> - Kever
>
>> ---
>> arch/arm/dts/rk3399-u-boot.dtsi | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi
>> b/arch/arm/dts/rk3399-u-boot.dtsi
>> index 716b9a433a..a1b6d6f007 100644
>> --- a/arch/arm/dts/rk3399-u-boot.dtsi
>> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
>> @@ -124,8 +124,10 @@
>> {
>> u-boot,dm-pre-reloc;
>> +#ifndef CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE
>> /* mmc to sram can't do dma, prevent aborts transferring TF-A parts */
>> u-boot,spl-fifo-mode;
>> +#endif
>> };
>> {
Re: [PATCH 2/2] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when needed
On 2022/6/9 23:23, Jerome Forissier wrote:
Commit 5c606ca35c42 ("rockchip: rk3399: enable spl-fifo-mode for sdmmc")
mentions that the RK3399 SoC can't do DMA between SDMMC and SRAM.
According to the TRM "7.3.2 Embedded SRAM access path" [1], only the
8KB SRAM at 0xff3b (INTMEM1) is in this situation. The 192KB SRAM
can be accessed by both DMA controllers.
Assuming the only use case for writing from MMC to INTMEM1 is loading
a FIT image, and with the introduction of a temporary buffer for that
purpose (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE, which is required
anyways to ensure the destination boundaries are enforced), then
spl-fifo-mode is not needed anymore and DMA can be enabled safely.
Link: [1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
CC: Deepak Das
Signed-off-by: Jerome Forissier
Reviewed-by: Kever Yang
Thanks,
- Kever
---
arch/arm/dts/rk3399-u-boot.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
index 716b9a433a..a1b6d6f007 100644
--- a/arch/arm/dts/rk3399-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -124,8 +124,10 @@
{
u-boot,dm-pre-reloc;
+#ifndef CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE
/* mmc to sram can't do dma, prevent aborts transferring TF-A parts */
u-boot,spl-fifo-mode;
+#endif
};
{
Re: [PATCH 2/2] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when needed
El Tue, Jun 14, 2022 at 11:16:42AM -0700, Jerome Forissier deia:
> Oops, that should rather be:
>
> +#if (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE == 0)
>
I tested with this change, not that my opinion counts much, but
anyway:
Reviewed-by: Xavier Drudis Ferran
Tested-by: Xavier Drudis Ferran
This changes reduce some 0.2 s the boot time of my Rock Pi 4B.
Before this series, just with u-boot/master from today
c18e5fb055 dtoc: Update test_src_scan.py for new tegra compatibles
plus the patches I sent to this list (I can't boot from MMC without
them)
https://lists.denx.de/pipermail/u-boot/2022-June/485497.html
and bootstage configured, I get :
Timer summary in microseconds (10 records):
MarkElapsed Stage
1,903,436 1,903,436 board_init_f
1,903,436 0 board_init_f
2,900,331996,895 board_init_r
4,091,657 1,191,326 id=64
4,930,650838,993 id=65
4,930,827177 main_loop
7,946,715 3,015,888 bootm_start
9,010,637 1,063,922 id=15
9,010,639 2 start_kernel
Accumulated time:
22,700 dm_r
479,397 dm_f
With that plus 1/2 in this series and
CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE=0x3000
I get something similar (slightly slower because of >=3 correct reads
instead of 1 overwriting read per image):
Timer summary in microseconds (10 records):
MarkElapsed Stage
1,979,414 1,979,414 board_init_f
1,979,414 0 board_init_f
2,976,429997,015 board_init_r
4,166,623 1,190,194 id=64
5,005,623839,000 id=65
5,005,800177 main_loop
8,020,791 3,014,991 bootm_start
9,084,116 1,063,325 id=15
9,084,118 2 start_kernel
Accumulated time:
22,699 dm_r
479,480 dm_f
With that plus this 2/2 it's faster (while safer) than initially.
Timer summary in microseconds (10 records):
MarkElapsed Stage
1,709,384 1,709,384 board_init_f
1,709,384 0 board_init_f
2,706,192996,808 board_init_r
3,895,269 1,189,077 id=64
4,733,786838,517 id=65
4,733,963177 main_loop
7,751,063 3,017,100 bootm_start
8,814,449 1,063,386 id=15
8,814,451 2 start_kernel
Accumulated time:
22,703 dm_r
479,520 dm_f
With this change your 2/2 patch becomes
---
From: Jerome Forissier
Date: Thu, 9 Jun 2022 17:23:22 +0200
Subject: [PATCH] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when
needed
Commit 5c606ca35c42 ("rockchip: rk3399: enable spl-fifo-mode for sdmmc")
mentions that the RK3399 SoC can't do DMA between SDMMC and SRAM.
According to the TRM "7.3.2 Embedded SRAM access path" [1], only the
8KB SRAM at 0xff3b (INTMEM1) is in this situation. The 192KB SRAM
can be accessed by both DMA controllers.
Assuming the only use case for writing from MMC to INTMEM1 is loading
a FIT image, and with the introduction of a temporary buffer for that
purpose (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE, which is required
anyways to ensure the destination boundaries are enforced), then
spl-fifo-mode is not needed anymore and DMA can be enabled safely.
Link: [1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
CC: Deepak Das
Signed-off-by: Jerome Forissier
---
arch/arm/dts/rk3399-u-boot.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
index 716b9a433a..e0bb230022 100644
--- a/arch/arm/dts/rk3399-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -124,9 +124,10 @@
{
u-boot,dm-pre-reloc;
+#if (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE == 0)
/* mmc to sram can't do dma, prevent aborts transferring TF-A parts */
u-boot,spl-fifo-mode;
+#endif
};
{
--
2.20.1
Re: [PATCH 2/2] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when needed
On 6/9/22 08:23, Jerome Forissier wrote:
> Commit 5c606ca35c42 ("rockchip: rk3399: enable spl-fifo-mode for sdmmc")
> mentions that the RK3399 SoC can't do DMA between SDMMC and SRAM.
> According to the TRM "7.3.2 Embedded SRAM access path" [1], only the
> 8KB SRAM at 0xff3b (INTMEM1) is in this situation. The 192KB SRAM
> can be accessed by both DMA controllers.
>
> Assuming the only use case for writing from MMC to INTMEM1 is loading
> a FIT image, and with the introduction of a temporary buffer for that
> purpose (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE, which is required
> anyways to ensure the destination boundaries are enforced), then
> spl-fifo-mode is not needed anymore and DMA can be enabled safely.
>
> Link: [1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
> CC: Deepak Das
> Signed-off-by: Jerome Forissier
> ---
> arch/arm/dts/rk3399-u-boot.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
> index 716b9a433a..a1b6d6f007 100644
> --- a/arch/arm/dts/rk3399-u-boot.dtsi
> +++ b/arch/arm/dts/rk3399-u-boot.dtsi
> @@ -124,8 +124,10 @@
> {
> u-boot,dm-pre-reloc;
>
> +#ifndef CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE
Oops, that should rather be:
+#if (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE == 0)
> /* mmc to sram can't do dma, prevent aborts transferring TF-A parts */
> u-boot,spl-fifo-mode;
> +#endif
> };
>
> {
[PATCH 2/2] rockchip: rk3399: enable spl-fifo-mode for sdmmc only when needed
Commit 5c606ca35c42 ("rockchip: rk3399: enable spl-fifo-mode for sdmmc")
mentions that the RK3399 SoC can't do DMA between SDMMC and SRAM.
According to the TRM "7.3.2 Embedded SRAM access path" [1], only the
8KB SRAM at 0xff3b (INTMEM1) is in this situation. The 192KB SRAM
can be accessed by both DMA controllers.
Assuming the only use case for writing from MMC to INTMEM1 is loading
a FIT image, and with the introduction of a temporary buffer for that
purpose (CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE, which is required
anyways to ensure the destination boundaries are enforced), then
spl-fifo-mode is not needed anymore and DMA can be enabled safely.
Link: [1] https://www.rockchip.fr/Rockchip%20RK3399%20TRM%20V1.4%20Part1.pdf
CC: Deepak Das
Signed-off-by: Jerome Forissier
---
arch/arm/dts/rk3399-u-boot.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/dts/rk3399-u-boot.dtsi b/arch/arm/dts/rk3399-u-boot.dtsi
index 716b9a433a..a1b6d6f007 100644
--- a/arch/arm/dts/rk3399-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-u-boot.dtsi
@@ -124,8 +124,10 @@
{
u-boot,dm-pre-reloc;
+#ifndef CONFIG_SPL_LOAD_FIT_IMAGE_BUFFER_SIZE
/* mmc to sram can't do dma, prevent aborts transferring TF-A parts */
u-boot,spl-fifo-mode;
+#endif
};
{
--
2.34.1
