All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15  9:40 ` Ulrich Hecht
  0 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15  9:40 UTC (permalink / raw)
  To: linux-renesas-soc, u-boot
  Cc: geert, marek.vasut, yoshihiro.shimoda.uh, magnus.damm,
	takuya.sakata.wz, Ulrich Hecht

Uses an SMC call to the ATF to determine the memory configuration, then
creates appropriate DT memory nodes.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
This is an attempt to avoid having to have a ton of different device trees
for Renesas R8A7795 H3 SoCs from revision 3.0, which come in a number of
different memory configurations.

The specific configuration is statically baked into the ARM Trusted Firmware
binary, so this solution adds an SMC "SiP Service Call" to the ATF returning
the memory configuration, and then has u-boot build appropriate memory nodes.

This has not been tested in the flesh successfully, as I only have a revision
1.0 board. I'd like to get some feedback on whether this approach is sound,
though. Thanks.

Depends on "[RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call".

CU
Uli



 board/renesas/salvator-x/salvator-x.c | 51 +++++++++++++++++++++++++++++++++++
 configs/r8a7795_salvator-x_defconfig  |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/board/renesas/salvator-x/salvator-x.c b/board/renesas/salvator-x/salvator-x.c
index 651877c..307cb64 100644
--- a/board/renesas/salvator-x/salvator-x.c
+++ b/board/renesas/salvator-x/salvator-x.c
@@ -24,6 +24,7 @@
 #include <asm/arch/sh_sdhi.h>
 #include <i2c.h>
 #include <mmc.h>
+#include <linux/arm-smccc.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -136,3 +137,53 @@ void reset_cpu(ulong addr)
 	writel(RST_CODE, RST_CA57RESCNT);
 #endif
 }
+
+#define ARM_SMCCC_RENESAS_MEMCONF	0x82000000UL
+int ft_board_setup(void *blob, bd_t *bd)
+{
+	if (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A7795 &&
+	    rmobile_get_cpu_rev_integer() >= 3) {
+		u64 base[CONFIG_NR_DRAM_BANKS];
+		u64 size[CONFIG_NR_DRAM_BANKS];
+		struct arm_smccc_res res;
+
+		arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
+			      0, 0, 0, 0, 0, 0, 0, &res);
+
+		switch (res.a0) {
+		case 1:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x038000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x040000000ULL;
+			base[2] = 0x600000000ULL;
+			size[2] = 0x040000000ULL;
+			base[3] = 0x700000000ULL;
+			size[3] = 0x040000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 4);
+			break;
+		case 2:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x078000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x080000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 2);
+			break;
+		case 3:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x078000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x080000000ULL;
+			base[2] = 0x600000000ULL;
+			size[2] = 0x080000000ULL;
+			base[3] = 0x700000000ULL;
+			size[3] = 0x080000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 4);
+			break;
+		default:
+			break;
+		}
+	}
+
+	return 0;
+}
diff --git a/configs/r8a7795_salvator-x_defconfig b/configs/r8a7795_salvator-x_defconfig
index fdfa41c..cd2bb59 100644
--- a/configs/r8a7795_salvator-x_defconfig
+++ b/configs/r8a7795_salvator-x_defconfig
@@ -61,3 +61,5 @@ CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_STORAGE=y
 CONFIG_OF_LIBFDT_OVERLAY=y
 CONFIG_SMBIOS_MANUFACTURER=""
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_ARM_SMCCC=y
-- 
2.7.4

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15  9:40 ` Ulrich Hecht
  0 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15  9:40 UTC (permalink / raw)
  To: u-boot

Uses an SMC call to the ATF to determine the memory configuration, then
creates appropriate DT memory nodes.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
This is an attempt to avoid having to have a ton of different device trees
for Renesas R8A7795 H3 SoCs from revision 3.0, which come in a number of
different memory configurations.

The specific configuration is statically baked into the ARM Trusted Firmware
binary, so this solution adds an SMC "SiP Service Call" to the ATF returning
the memory configuration, and then has u-boot build appropriate memory nodes.

This has not been tested in the flesh successfully, as I only have a revision
1.0 board. I'd like to get some feedback on whether this approach is sound,
though. Thanks.

Depends on "[RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call".

CU
Uli



 board/renesas/salvator-x/salvator-x.c | 51 +++++++++++++++++++++++++++++++++++
 configs/r8a7795_salvator-x_defconfig  |  2 ++
 2 files changed, 53 insertions(+)

diff --git a/board/renesas/salvator-x/salvator-x.c b/board/renesas/salvator-x/salvator-x.c
index 651877c..307cb64 100644
--- a/board/renesas/salvator-x/salvator-x.c
+++ b/board/renesas/salvator-x/salvator-x.c
@@ -24,6 +24,7 @@
 #include <asm/arch/sh_sdhi.h>
 #include <i2c.h>
 #include <mmc.h>
+#include <linux/arm-smccc.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -136,3 +137,53 @@ void reset_cpu(ulong addr)
 	writel(RST_CODE, RST_CA57RESCNT);
 #endif
 }
+
+#define ARM_SMCCC_RENESAS_MEMCONF	0x82000000UL
+int ft_board_setup(void *blob, bd_t *bd)
+{
+	if (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A7795 &&
+	    rmobile_get_cpu_rev_integer() >= 3) {
+		u64 base[CONFIG_NR_DRAM_BANKS];
+		u64 size[CONFIG_NR_DRAM_BANKS];
+		struct arm_smccc_res res;
+
+		arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
+			      0, 0, 0, 0, 0, 0, 0, &res);
+
+		switch (res.a0) {
+		case 1:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x038000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x040000000ULL;
+			base[2] = 0x600000000ULL;
+			size[2] = 0x040000000ULL;
+			base[3] = 0x700000000ULL;
+			size[3] = 0x040000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 4);
+			break;
+		case 2:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x078000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x080000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 2);
+			break;
+		case 3:
+			base[0] = 0x048000000ULL;
+			size[0] = 0x078000000ULL;
+			base[1] = 0x500000000ULL;
+			size[1] = 0x080000000ULL;
+			base[2] = 0x600000000ULL;
+			size[2] = 0x080000000ULL;
+			base[3] = 0x700000000ULL;
+			size[3] = 0x080000000ULL;
+			fdt_fixup_memory_banks(blob, base, size, 4);
+			break;
+		default:
+			break;
+		}
+	}
+
+	return 0;
+}
diff --git a/configs/r8a7795_salvator-x_defconfig b/configs/r8a7795_salvator-x_defconfig
index fdfa41c..cd2bb59 100644
--- a/configs/r8a7795_salvator-x_defconfig
+++ b/configs/r8a7795_salvator-x_defconfig
@@ -61,3 +61,5 @@ CONFIG_USB_EHCI_GENERIC=y
 CONFIG_USB_STORAGE=y
 CONFIG_OF_LIBFDT_OVERLAY=y
 CONFIG_SMBIOS_MANUFACTURER=""
+CONFIG_OF_BOARD_SETUP=y
+CONFIG_ARM_SMCCC=y
-- 
2.7.4

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

* [RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call
  2018-06-15  9:40 ` [U-Boot] " Ulrich Hecht
@ 2018-06-15  9:40   ` Ulrich Hecht
  -1 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15  9:40 UTC (permalink / raw)
  To: linux-renesas-soc, u-boot
  Cc: geert, marek.vasut, yoshihiro.shimoda.uh, magnus.damm,
	takuya.sakata.wz, Ulrich Hecht

Returns the memory configuration for Renesas R8A7795 (R-Car H3) SoCs,
revision 3.0 and up.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
See "[RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer"
for an explanation of this.

CU
Uli


 include/services/arm_arch_svc.h            |  1 +
 services/arm_arch_svc/arm_arch_svc_setup.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/services/arm_arch_svc.h b/include/services/arm_arch_svc.h
index 2961601..9132336 100644
--- a/include/services/arm_arch_svc.h
+++ b/include/services/arm_arch_svc.h
@@ -10,5 +10,6 @@
 #define SMCCC_VERSION			U(0x80000000)
 #define SMCCC_ARCH_FEATURES		U(0x80000001)
 #define SMCCC_ARCH_WORKAROUND_1		U(0x80008000)
+#define SMCCC_RENESAS_MEMCONF		U(0x82000000)
 
 #endif /* __ARM_ARCH_SVC_H__ */
diff --git a/services/arm_arch_svc/arm_arch_svc_setup.c b/services/arm_arch_svc/arm_arch_svc_setup.c
index eedac86..f836ae8 100644
--- a/services/arm_arch_svc/arm_arch_svc_setup.c
+++ b/services/arm_arch_svc/arm_arch_svc_setup.c
@@ -56,6 +56,20 @@ uintptr_t arm_arch_svc_smc_handler(uint32_t smc_fid,
 		 */
 		SMC_RET0(handle);
 #endif
+	case SMCCC_RENESAS_MEMCONF:
+#if (RCAR_DRAM_LPDDR4_MEMCONF == 0)
+		/* 4GB(1GBx4) */
+		SMC_RET1(handle, 1);
+#elif (RCAR_DRAM_LPDDR4_MEMCONF == 1) && (RCAR_DRAM_CHANNEL == 5) && \
+	(RCAR_DRAM_SPLIT == 2)
+		/* 4GB(2GBx2 2ch split) */
+		SMC_RET1(handle, 2);
+#elif (RCAR_DRAM_LPDDR4_MEMCONF == 1) && (RCAR_DRAM_CHANNEL == 15)
+		/* 8GB(2GBx4: default) */
+		SMC_RET1(handle, 3);
+#else
+		SMC_RET1(handle, 0);
+#endif /* RCAR_DRAM_LPDDR4_MEMCONF == 0 */
 	default:
 		WARN("Unimplemented Arm Architecture Service Call: 0x%x \n",
 			smc_fid);
-- 
2.7.4

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

* [U-Boot] [RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call
@ 2018-06-15  9:40   ` Ulrich Hecht
  0 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15  9:40 UTC (permalink / raw)
  To: u-boot

Returns the memory configuration for Renesas R8A7795 (R-Car H3) SoCs,
revision 3.0 and up.

Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
---
See "[RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer"
for an explanation of this.

CU
Uli


 include/services/arm_arch_svc.h            |  1 +
 services/arm_arch_svc/arm_arch_svc_setup.c | 14 ++++++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/services/arm_arch_svc.h b/include/services/arm_arch_svc.h
index 2961601..9132336 100644
--- a/include/services/arm_arch_svc.h
+++ b/include/services/arm_arch_svc.h
@@ -10,5 +10,6 @@
 #define SMCCC_VERSION			U(0x80000000)
 #define SMCCC_ARCH_FEATURES		U(0x80000001)
 #define SMCCC_ARCH_WORKAROUND_1		U(0x80008000)
+#define SMCCC_RENESAS_MEMCONF		U(0x82000000)
 
 #endif /* __ARM_ARCH_SVC_H__ */
diff --git a/services/arm_arch_svc/arm_arch_svc_setup.c b/services/arm_arch_svc/arm_arch_svc_setup.c
index eedac86..f836ae8 100644
--- a/services/arm_arch_svc/arm_arch_svc_setup.c
+++ b/services/arm_arch_svc/arm_arch_svc_setup.c
@@ -56,6 +56,20 @@ uintptr_t arm_arch_svc_smc_handler(uint32_t smc_fid,
 		 */
 		SMC_RET0(handle);
 #endif
+	case SMCCC_RENESAS_MEMCONF:
+#if (RCAR_DRAM_LPDDR4_MEMCONF == 0)
+		/* 4GB(1GBx4) */
+		SMC_RET1(handle, 1);
+#elif (RCAR_DRAM_LPDDR4_MEMCONF == 1) && (RCAR_DRAM_CHANNEL == 5) && \
+	(RCAR_DRAM_SPLIT == 2)
+		/* 4GB(2GBx2 2ch split) */
+		SMC_RET1(handle, 2);
+#elif (RCAR_DRAM_LPDDR4_MEMCONF == 1) && (RCAR_DRAM_CHANNEL == 15)
+		/* 8GB(2GBx4: default) */
+		SMC_RET1(handle, 3);
+#else
+		SMC_RET1(handle, 0);
+#endif /* RCAR_DRAM_LPDDR4_MEMCONF == 0 */
 	default:
 		WARN("Unimplemented Arm Architecture Service Call: 0x%x \n",
 			smc_fid);
-- 
2.7.4

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15  9:40 ` [U-Boot] " Ulrich Hecht
@ 2018-06-15 10:09   ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 10:09 UTC (permalink / raw)
  To: Ulrich Hecht, linux-renesas-soc, u-boot
  Cc: takuya.sakata.wz, magnus.damm, geert

On 06/15/2018 11:40 AM, Ulrich Hecht wrote:
> Uses an SMC call to the ATF to determine the memory configuration, then
> creates appropriate DT memory nodes.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> This is an attempt to avoid having to have a ton of different device trees
> for Renesas R8A7795 H3 SoCs from revision 3.0, which come in a number of
> different memory configurations.
> 
> The specific configuration is statically baked into the ARM Trusted Firmware
> binary, so this solution adds an SMC "SiP Service Call" to the ATF returning
> the memory configuration, and then has u-boot build appropriate memory nodes.
> 
> This has not been tested in the flesh successfully, as I only have a revision
> 1.0 board. I'd like to get some feedback on whether this approach is sound,
> though. Thanks.
> 
> Depends on "[RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call".
> 
> CU
> Uli
> 
> 
> 
>  board/renesas/salvator-x/salvator-x.c | 51 +++++++++++++++++++++++++++++++++++
>  configs/r8a7795_salvator-x_defconfig  |  2 ++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/board/renesas/salvator-x/salvator-x.c b/board/renesas/salvator-x/salvator-x.c
> index 651877c..307cb64 100644
> --- a/board/renesas/salvator-x/salvator-x.c
> +++ b/board/renesas/salvator-x/salvator-x.c
> @@ -24,6 +24,7 @@
>  #include <asm/arch/sh_sdhi.h>
>  #include <i2c.h>
>  #include <mmc.h>
> +#include <linux/arm-smccc.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -136,3 +137,53 @@ void reset_cpu(ulong addr)
>  	writel(RST_CODE, RST_CA57RESCNT);
>  #endif
>  }
> +
> +#define ARM_SMCCC_RENESAS_MEMCONF	0x82000000UL
> +int ft_board_setup(void *blob, bd_t *bd)
> +{
> +	if (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A7795 &&
> +	    rmobile_get_cpu_rev_integer() >= 3) {
> +		u64 base[CONFIG_NR_DRAM_BANKS];
> +		u64 size[CONFIG_NR_DRAM_BANKS];
> +		struct arm_smccc_res res;
> +
> +		arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> +			      0, 0, 0, 0, 0, 0, 0, &res);

Will this call work on platforms without patched ATF ?
(I think not, don't you need to handle return value?)

> +		switch (res.a0) {
> +		case 1:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x038000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x040000000ULL;
> +			base[2] = 0x600000000ULL;
> +			size[2] = 0x040000000ULL;
> +			base[3] = 0x700000000ULL;
> +			size[3] = 0x040000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 4);
> +			break;
> +		case 2:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x078000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x080000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 2);
> +			break;
> +		case 3:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x078000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x080000000ULL;
> +			base[2] = 0x600000000ULL;
> +			size[2] = 0x080000000ULL;
> +			base[3] = 0x700000000ULL;
> +			size[3] = 0x080000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 4);
> +			break;

Obvious design question is -- since you're adding new SMC call anyway,
can't the call just return the memory layout table itself, so that it
won't be duplicated both in U-Boot and ATF ?

> +		default:
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/configs/r8a7795_salvator-x_defconfig b/configs/r8a7795_salvator-x_defconfig
> index fdfa41c..cd2bb59 100644
> --- a/configs/r8a7795_salvator-x_defconfig
> +++ b/configs/r8a7795_salvator-x_defconfig
> @@ -61,3 +61,5 @@ CONFIG_USB_EHCI_GENERIC=y
>  CONFIG_USB_STORAGE=y
>  CONFIG_OF_LIBFDT_OVERLAY=y
>  CONFIG_SMBIOS_MANUFACTURER=""
> +CONFIG_OF_BOARD_SETUP=y
> +CONFIG_ARM_SMCCC=y

Update the defconfig with make savedefconfig .

-- 
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 10:09   ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 10:09 UTC (permalink / raw)
  To: u-boot

On 06/15/2018 11:40 AM, Ulrich Hecht wrote:
> Uses an SMC call to the ATF to determine the memory configuration, then
> creates appropriate DT memory nodes.
> 
> Signed-off-by: Ulrich Hecht <ulrich.hecht+renesas@gmail.com>
> ---
> This is an attempt to avoid having to have a ton of different device trees
> for Renesas R8A7795 H3 SoCs from revision 3.0, which come in a number of
> different memory configurations.
> 
> The specific configuration is statically baked into the ARM Trusted Firmware
> binary, so this solution adds an SMC "SiP Service Call" to the ATF returning
> the memory configuration, and then has u-boot build appropriate memory nodes.
> 
> This has not been tested in the flesh successfully, as I only have a revision
> 1.0 board. I'd like to get some feedback on whether this approach is sound,
> though. Thanks.
> 
> Depends on "[RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call".
> 
> CU
> Uli
> 
> 
> 
>  board/renesas/salvator-x/salvator-x.c | 51 +++++++++++++++++++++++++++++++++++
>  configs/r8a7795_salvator-x_defconfig  |  2 ++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/board/renesas/salvator-x/salvator-x.c b/board/renesas/salvator-x/salvator-x.c
> index 651877c..307cb64 100644
> --- a/board/renesas/salvator-x/salvator-x.c
> +++ b/board/renesas/salvator-x/salvator-x.c
> @@ -24,6 +24,7 @@
>  #include <asm/arch/sh_sdhi.h>
>  #include <i2c.h>
>  #include <mmc.h>
> +#include <linux/arm-smccc.h>
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> @@ -136,3 +137,53 @@ void reset_cpu(ulong addr)
>  	writel(RST_CODE, RST_CA57RESCNT);
>  #endif
>  }
> +
> +#define ARM_SMCCC_RENESAS_MEMCONF	0x82000000UL
> +int ft_board_setup(void *blob, bd_t *bd)
> +{
> +	if (rmobile_get_cpu_type() == RMOBILE_CPU_TYPE_R8A7795 &&
> +	    rmobile_get_cpu_rev_integer() >= 3) {
> +		u64 base[CONFIG_NR_DRAM_BANKS];
> +		u64 size[CONFIG_NR_DRAM_BANKS];
> +		struct arm_smccc_res res;
> +
> +		arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> +			      0, 0, 0, 0, 0, 0, 0, &res);

Will this call work on platforms without patched ATF ?
(I think not, don't you need to handle return value?)

> +		switch (res.a0) {
> +		case 1:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x038000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x040000000ULL;
> +			base[2] = 0x600000000ULL;
> +			size[2] = 0x040000000ULL;
> +			base[3] = 0x700000000ULL;
> +			size[3] = 0x040000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 4);
> +			break;
> +		case 2:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x078000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x080000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 2);
> +			break;
> +		case 3:
> +			base[0] = 0x048000000ULL;
> +			size[0] = 0x078000000ULL;
> +			base[1] = 0x500000000ULL;
> +			size[1] = 0x080000000ULL;
> +			base[2] = 0x600000000ULL;
> +			size[2] = 0x080000000ULL;
> +			base[3] = 0x700000000ULL;
> +			size[3] = 0x080000000ULL;
> +			fdt_fixup_memory_banks(blob, base, size, 4);
> +			break;

Obvious design question is -- since you're adding new SMC call anyway,
can't the call just return the memory layout table itself, so that it
won't be duplicated both in U-Boot and ATF ?

> +		default:
> +			break;
> +		}
> +	}
> +
> +	return 0;
> +}
> diff --git a/configs/r8a7795_salvator-x_defconfig b/configs/r8a7795_salvator-x_defconfig
> index fdfa41c..cd2bb59 100644
> --- a/configs/r8a7795_salvator-x_defconfig
> +++ b/configs/r8a7795_salvator-x_defconfig
> @@ -61,3 +61,5 @@ CONFIG_USB_EHCI_GENERIC=y
>  CONFIG_USB_STORAGE=y
>  CONFIG_OF_LIBFDT_OVERLAY=y
>  CONFIG_SMBIOS_MANUFACTURER=""
> +CONFIG_OF_BOARD_SETUP=y
> +CONFIG_ARM_SMCCC=y

Update the defconfig with make savedefconfig .

-- 
Best regards,
Marek Vasut

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 10:09   ` Marek Vasut
@ 2018-06-15 10:37     ` Ulrich Hecht
  -1 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15 10:37 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Linux-Renesas, u-boot, Geert Uytterhoeven, shimoda, Magnus Damm,
	takuya.sakata.wz

On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>
> Will this call work on platforms without patched ATF ?
> (I think not, don't you need to handle return value?)

I have not actually tested that, but if I understand the ATF code
correctly, unimplemented calls return
SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.

>
>> +             switch (res.a0) {
>> +             case 1:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x038000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x040000000ULL;
>> +                     base[2] = 0x600000000ULL;
>> +                     size[2] = 0x040000000ULL;
>> +                     base[3] = 0x700000000ULL;
>> +                     size[3] = 0x040000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> +                     break;
>> +             case 2:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x078000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x080000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>> +                     break;
>> +             case 3:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x078000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x080000000ULL;
>> +                     base[2] = 0x600000000ULL;
>> +                     size[2] = 0x080000000ULL;
>> +                     base[3] = 0x700000000ULL;
>> +                     size[3] = 0x080000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> +                     break;
>
> Obvious design question is -- since you're adding new SMC call anyway,
> can't the call just return the memory layout table itself, so that it
> won't be duplicated both in U-Boot and ATF ?

My gut feeling was to go with the smallest interface possible.

CU
Uli

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 10:37     ` Ulrich Hecht
  0 siblings, 0 replies; 37+ messages in thread
From: Ulrich Hecht @ 2018-06-15 10:37 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>
> Will this call work on platforms without patched ATF ?
> (I think not, don't you need to handle return value?)

I have not actually tested that, but if I understand the ATF code
correctly, unimplemented calls return
SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.

>
>> +             switch (res.a0) {
>> +             case 1:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x038000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x040000000ULL;
>> +                     base[2] = 0x600000000ULL;
>> +                     size[2] = 0x040000000ULL;
>> +                     base[3] = 0x700000000ULL;
>> +                     size[3] = 0x040000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> +                     break;
>> +             case 2:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x078000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x080000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>> +                     break;
>> +             case 3:
>> +                     base[0] = 0x048000000ULL;
>> +                     size[0] = 0x078000000ULL;
>> +                     base[1] = 0x500000000ULL;
>> +                     size[1] = 0x080000000ULL;
>> +                     base[2] = 0x600000000ULL;
>> +                     size[2] = 0x080000000ULL;
>> +                     base[3] = 0x700000000ULL;
>> +                     size[3] = 0x080000000ULL;
>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> +                     break;
>
> Obvious design question is -- since you're adding new SMC call anyway,
> can't the call just return the memory layout table itself, so that it
> won't be duplicated both in U-Boot and ATF ?

My gut feeling was to go with the smallest interface possible.

CU
Uli

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 10:37     ` [U-Boot] " Ulrich Hecht
@ 2018-06-15 11:43       ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 11:43 UTC (permalink / raw)
  To: Ulrich Hecht
  Cc: takuya.sakata.wz, Magnus Damm, Linux-Renesas, u-boot, Geert Uytterhoeven

On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>
>> Will this call work on platforms without patched ATF ?
>> (I think not, don't you need to handle return value?)
> 
> I have not actually tested that, but if I understand the ATF code
> correctly, unimplemented calls return
> SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.

Which means the board has a memory size of 0 and fails to boot ?

>>> +             switch (res.a0) {
>>> +             case 1:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x038000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x040000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x040000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x040000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>> +             case 2:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>> +                     break;
>>> +             case 3:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x080000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>
>> Obvious design question is -- since you're adding new SMC call anyway,
>> can't the call just return the memory layout table itself, so that it
>> won't be duplicated both in U-Boot and ATF ?
> 
> My gut feeling was to go with the smallest interface possible.

But this doesn't scale. The API here uses some ad-hoc constants to
identify memory layout tables which have to be encoded both in ATF and
U-Boot, both of which must be kept in sync.

The ATF already has those memory layout tables, it's only a matter of
passing them to U-Boot. If you do just that, the ad-hoc constants and
encoding of tables into U-Boot goes away and in fact simplifies the design.

Yet, I have to wonder if ATF doesn't already contain some sort of
standard SMC call to get memory topology. It surprises me that it wouldn't.

-- 
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 11:43       ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 11:43 UTC (permalink / raw)
  To: u-boot

On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>
>> Will this call work on platforms without patched ATF ?
>> (I think not, don't you need to handle return value?)
> 
> I have not actually tested that, but if I understand the ATF code
> correctly, unimplemented calls return
> SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.

Which means the board has a memory size of 0 and fails to boot ?

>>> +             switch (res.a0) {
>>> +             case 1:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x038000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x040000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x040000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x040000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>> +             case 2:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>> +                     break;
>>> +             case 3:
>>> +                     base[0] = 0x048000000ULL;
>>> +                     size[0] = 0x078000000ULL;
>>> +                     base[1] = 0x500000000ULL;
>>> +                     size[1] = 0x080000000ULL;
>>> +                     base[2] = 0x600000000ULL;
>>> +                     size[2] = 0x080000000ULL;
>>> +                     base[3] = 0x700000000ULL;
>>> +                     size[3] = 0x080000000ULL;
>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>> +                     break;
>>
>> Obvious design question is -- since you're adding new SMC call anyway,
>> can't the call just return the memory layout table itself, so that it
>> won't be duplicated both in U-Boot and ATF ?
> 
> My gut feeling was to go with the smallest interface possible.

But this doesn't scale. The API here uses some ad-hoc constants to
identify memory layout tables which have to be encoded both in ATF and
U-Boot, both of which must be kept in sync.

The ATF already has those memory layout tables, it's only a matter of
passing them to U-Boot. If you do just that, the ad-hoc constants and
encoding of tables into U-Boot goes away and in fact simplifies the design.

Yet, I have to wonder if ATF doesn't already contain some sort of
standard SMC call to get memory topology. It surprises me that it wouldn't.

-- 
Best regards,
Marek Vasut

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 11:43       ` Marek Vasut
@ 2018-06-15 12:00         ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 12:00 UTC (permalink / raw)
  To: Ulrich Hecht
  Cc: takuya.sakata.wz, Magnus Damm, Linux-Renesas, u-boot,
	Geert Uytterhoeven, Laurent Pinchart

On 06/15/2018 01:43 PM, Marek Vasut wrote:
> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>
>>> Will this call work on platforms without patched ATF ?
>>> (I think not, don't you need to handle return value?)
>>
>> I have not actually tested that, but if I understand the ATF code
>> correctly, unimplemented calls return
>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.
> 
> Which means the board has a memory size of 0 and fails to boot ?
> 
>>>> +             switch (res.a0) {
>>>> +             case 1:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x038000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x040000000ULL;
>>>> +                     base[2] = 0x600000000ULL;
>>>> +                     size[2] = 0x040000000ULL;
>>>> +                     base[3] = 0x700000000ULL;
>>>> +                     size[3] = 0x040000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>> +                     break;
>>>> +             case 2:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x078000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x080000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>> +                     break;
>>>> +             case 3:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x078000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x080000000ULL;
>>>> +                     base[2] = 0x600000000ULL;
>>>> +                     size[2] = 0x080000000ULL;
>>>> +                     base[3] = 0x700000000ULL;
>>>> +                     size[3] = 0x080000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>> +                     break;
>>>
>>> Obvious design question is -- since you're adding new SMC call anyway,
>>> can't the call just return the memory layout table itself, so that it
>>> won't be duplicated both in U-Boot and ATF ?
>>
>> My gut feeling was to go with the smallest interface possible.
> 
> But this doesn't scale. The API here uses some ad-hoc constants to
> identify memory layout tables which have to be encoded both in ATF and
> U-Boot, both of which must be kept in sync.
> 
> The ATF already has those memory layout tables, it's only a matter of
> passing them to U-Boot. If you do just that, the ad-hoc constants and
> encoding of tables into U-Boot goes away and in fact simplifies the design.
> 
> Yet, I have to wonder if ATF doesn't already contain some sort of
> standard SMC call to get memory topology. It surprises me that it wouldn't.

In fact, Laurent (CCed) was solving some similar issue with lossy decomp
and I think this involved some passing of memory layout information from
ATF to U-Boot too, or am I mistaken ?

-- 
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 12:00         ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 12:00 UTC (permalink / raw)
  To: u-boot

On 06/15/2018 01:43 PM, Marek Vasut wrote:
> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>
>>> Will this call work on platforms without patched ATF ?
>>> (I think not, don't you need to handle return value?)
>>
>> I have not actually tested that, but if I understand the ATF code
>> correctly, unimplemented calls return
>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP) below.
> 
> Which means the board has a memory size of 0 and fails to boot ?
> 
>>>> +             switch (res.a0) {
>>>> +             case 1:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x038000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x040000000ULL;
>>>> +                     base[2] = 0x600000000ULL;
>>>> +                     size[2] = 0x040000000ULL;
>>>> +                     base[3] = 0x700000000ULL;
>>>> +                     size[3] = 0x040000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>> +                     break;
>>>> +             case 2:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x078000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x080000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>> +                     break;
>>>> +             case 3:
>>>> +                     base[0] = 0x048000000ULL;
>>>> +                     size[0] = 0x078000000ULL;
>>>> +                     base[1] = 0x500000000ULL;
>>>> +                     size[1] = 0x080000000ULL;
>>>> +                     base[2] = 0x600000000ULL;
>>>> +                     size[2] = 0x080000000ULL;
>>>> +                     base[3] = 0x700000000ULL;
>>>> +                     size[3] = 0x080000000ULL;
>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>> +                     break;
>>>
>>> Obvious design question is -- since you're adding new SMC call anyway,
>>> can't the call just return the memory layout table itself, so that it
>>> won't be duplicated both in U-Boot and ATF ?
>>
>> My gut feeling was to go with the smallest interface possible.
> 
> But this doesn't scale. The API here uses some ad-hoc constants to
> identify memory layout tables which have to be encoded both in ATF and
> U-Boot, both of which must be kept in sync.
> 
> The ATF already has those memory layout tables, it's only a matter of
> passing them to U-Boot. If you do just that, the ad-hoc constants and
> encoding of tables into U-Boot goes away and in fact simplifies the design.
> 
> Yet, I have to wonder if ATF doesn't already contain some sort of
> standard SMC call to get memory topology. It surprises me that it wouldn't.

In fact, Laurent (CCed) was solving some similar issue with lossy decomp
and I think this involved some passing of memory layout information from
ATF to U-Boot too, or am I mistaken ?

-- 
Best regards,
Marek Vasut

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 12:00         ` Marek Vasut
@ 2018-06-15 23:21           ` Laurent Pinchart
  -1 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-15 23:21 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Ulrich Hecht, Linux-Renesas, u-boot, Geert Uytterhoeven, shimoda,
	Magnus Damm, takuya.sakata.wz

Hi Marek,

On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> > On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> 
wrote:
> >>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>> 
> >>> Will this call work on platforms without patched ATF ?
> >>> (I think not, don't you need to handle return value?)
> >> 
> >> I have not actually tested that, but if I understand the ATF code
> >> correctly, unimplemented calls return
> >> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
> >> below.> 
> > Which means the board has a memory size of 0 and fails to boot ?
> > 
> >>>> +             switch (res.a0) {
> >>>> +             case 1:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x038000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x040000000ULL;
> >>>> +                     base[2] = 0x600000000ULL;
> >>>> +                     size[2] = 0x040000000ULL;
> >>>> +                     base[3] = 0x700000000ULL;
> >>>> +                     size[3] = 0x040000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>> +                     break;
> >>>> +             case 2:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x078000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x080000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>> +                     break;
> >>>> +             case 3:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x078000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x080000000ULL;
> >>>> +                     base[2] = 0x600000000ULL;
> >>>> +                     size[2] = 0x080000000ULL;
> >>>> +                     base[3] = 0x700000000ULL;
> >>>> +                     size[3] = 0x080000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>> +                     break;
> >>> 
> >>> Obvious design question is -- since you're adding new SMC call anyway,
> >>> can't the call just return the memory layout table itself, so that it
> >>> won't be duplicated both in U-Boot and ATF ?
> >> 
> >> My gut feeling was to go with the smallest interface possible.
> > 
> > But this doesn't scale. The API here uses some ad-hoc constants to
> > identify memory layout tables which have to be encoded both in ATF and
> > U-Boot, both of which must be kept in sync.
> > 
> > The ATF already has those memory layout tables, it's only a matter of
> > passing them to U-Boot. If you do just that, the ad-hoc constants and
> > encoding of tables into U-Boot goes away and in fact simplifies the
> > design.
> > 
> > Yet, I have to wonder if ATF doesn't already contain some sort of
> > standard SMC call to get memory topology. It surprises me that it
> > wouldn't.
> 
> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
> and I think this involved some passing of memory layout information from
> ATF to U-Boot too, or am I mistaken ?

That's correct, ATF stores information about the memory layout at a fixed 
address in system memory, and U-Boot can read it.

-- 
Regards,

Laurent Pinchart

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 23:21           ` Laurent Pinchart
  0 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-15 23:21 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> > On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> 
wrote:
> >>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>> 
> >>> Will this call work on platforms without patched ATF ?
> >>> (I think not, don't you need to handle return value?)
> >> 
> >> I have not actually tested that, but if I understand the ATF code
> >> correctly, unimplemented calls return
> >> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
> >> below.> 
> > Which means the board has a memory size of 0 and fails to boot ?
> > 
> >>>> +             switch (res.a0) {
> >>>> +             case 1:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x038000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x040000000ULL;
> >>>> +                     base[2] = 0x600000000ULL;
> >>>> +                     size[2] = 0x040000000ULL;
> >>>> +                     base[3] = 0x700000000ULL;
> >>>> +                     size[3] = 0x040000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>> +                     break;
> >>>> +             case 2:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x078000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x080000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>> +                     break;
> >>>> +             case 3:
> >>>> +                     base[0] = 0x048000000ULL;
> >>>> +                     size[0] = 0x078000000ULL;
> >>>> +                     base[1] = 0x500000000ULL;
> >>>> +                     size[1] = 0x080000000ULL;
> >>>> +                     base[2] = 0x600000000ULL;
> >>>> +                     size[2] = 0x080000000ULL;
> >>>> +                     base[3] = 0x700000000ULL;
> >>>> +                     size[3] = 0x080000000ULL;
> >>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>> +                     break;
> >>> 
> >>> Obvious design question is -- since you're adding new SMC call anyway,
> >>> can't the call just return the memory layout table itself, so that it
> >>> won't be duplicated both in U-Boot and ATF ?
> >> 
> >> My gut feeling was to go with the smallest interface possible.
> > 
> > But this doesn't scale. The API here uses some ad-hoc constants to
> > identify memory layout tables which have to be encoded both in ATF and
> > U-Boot, both of which must be kept in sync.
> > 
> > The ATF already has those memory layout tables, it's only a matter of
> > passing them to U-Boot. If you do just that, the ad-hoc constants and
> > encoding of tables into U-Boot goes away and in fact simplifies the
> > design.
> > 
> > Yet, I have to wonder if ATF doesn't already contain some sort of
> > standard SMC call to get memory topology. It surprises me that it
> > wouldn't.
> 
> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
> and I think this involved some passing of memory layout information from
> ATF to U-Boot too, or am I mistaken ?

That's correct, ATF stores information about the memory layout at a fixed 
address in system memory, and U-Boot can read it.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 23:21           ` [U-Boot] " Laurent Pinchart
@ 2018-06-15 23:42             ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 23:42 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Ulrich Hecht, Linux-Renesas, u-boot, Geert Uytterhoeven, shimoda,
	Magnus Damm, takuya.sakata.wz

On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> 
> wrote:
>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>>>
>>>>> Will this call work on platforms without patched ATF ?
>>>>> (I think not, don't you need to handle return value?)
>>>>
>>>> I have not actually tested that, but if I understand the ATF code
>>>> correctly, unimplemented calls return
>>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
>>>> below.> 
>>> Which means the board has a memory size of 0 and fails to boot ?
>>>
>>>>>> +             switch (res.a0) {
>>>>>> +             case 1:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x038000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x040000000ULL;
>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>> +                     size[2] = 0x040000000ULL;
>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>> +                     size[3] = 0x040000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>> +                     break;
>>>>>> +             case 2:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>>>> +                     break;
>>>>>> +             case 3:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>> +                     size[2] = 0x080000000ULL;
>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>> +                     size[3] = 0x080000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>> +                     break;
>>>>>
>>>>> Obvious design question is -- since you're adding new SMC call anyway,
>>>>> can't the call just return the memory layout table itself, so that it
>>>>> won't be duplicated both in U-Boot and ATF ?
>>>>
>>>> My gut feeling was to go with the smallest interface possible.
>>>
>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>> identify memory layout tables which have to be encoded both in ATF and
>>> U-Boot, both of which must be kept in sync.
>>>
>>> The ATF already has those memory layout tables, it's only a matter of
>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>>> encoding of tables into U-Boot goes away and in fact simplifies the
>>> design.
>>>
>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>> standard SMC call to get memory topology. It surprises me that it
>>> wouldn't.
>>
>> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
>> and I think this involved some passing of memory layout information from
>> ATF to U-Boot too, or am I mistaken ?
> 
> That's correct, ATF stores information about the memory layout at a fixed 
> address in system memory, and U-Boot can read it.

Well, that sounds good ! Maybe we can avoid adding SMC call altogether
then? :)

-- 
Best regards,
Marek Vasut

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-15 23:42             ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-15 23:42 UTC (permalink / raw)
  To: u-boot

On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut <marek.vasut@gmail.com> 
> wrote:
>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>>>
>>>>> Will this call work on platforms without patched ATF ?
>>>>> (I think not, don't you need to handle return value?)
>>>>
>>>> I have not actually tested that, but if I understand the ATF code
>>>> correctly, unimplemented calls return
>>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
>>>> below.> 
>>> Which means the board has a memory size of 0 and fails to boot ?
>>>
>>>>>> +             switch (res.a0) {
>>>>>> +             case 1:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x038000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x040000000ULL;
>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>> +                     size[2] = 0x040000000ULL;
>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>> +                     size[3] = 0x040000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>> +                     break;
>>>>>> +             case 2:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>>>> +                     break;
>>>>>> +             case 3:
>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>> +                     size[2] = 0x080000000ULL;
>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>> +                     size[3] = 0x080000000ULL;
>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>> +                     break;
>>>>>
>>>>> Obvious design question is -- since you're adding new SMC call anyway,
>>>>> can't the call just return the memory layout table itself, so that it
>>>>> won't be duplicated both in U-Boot and ATF ?
>>>>
>>>> My gut feeling was to go with the smallest interface possible.
>>>
>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>> identify memory layout tables which have to be encoded both in ATF and
>>> U-Boot, both of which must be kept in sync.
>>>
>>> The ATF already has those memory layout tables, it's only a matter of
>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>>> encoding of tables into U-Boot goes away and in fact simplifies the
>>> design.
>>>
>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>> standard SMC call to get memory topology. It surprises me that it
>>> wouldn't.
>>
>> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
>> and I think this involved some passing of memory layout information from
>> ATF to U-Boot too, or am I mistaken ?
> 
> That's correct, ATF stores information about the memory layout at a fixed 
> address in system memory, and U-Boot can read it.

Well, that sounds good ! Maybe we can avoid adding SMC call altogether
then? :)

-- 
Best regards,
Marek Vasut

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-15 23:42             ` [U-Boot] " Marek Vasut
@ 2018-06-16 15:44               ` Laurent Pinchart
  -1 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-16 15:44 UTC (permalink / raw)
  To: Marek Vasut
  Cc: Ulrich Hecht, Linux-Renesas, u-boot, Geert Uytterhoeven, shimoda,
	Magnus Damm, takuya.sakata.wz

Hi Marek,

On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> >>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>>>> 
> >>>>> Will this call work on platforms without patched ATF ?
> >>>>> (I think not, don't you need to handle return value?)
> >>>> 
> >>>> I have not actually tested that, but if I understand the ATF code
> >>>> correctly, unimplemented calls return
> >>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
> >>>> below.
> >>> 
> >>> Which means the board has a memory size of 0 and fails to boot ?
> >>> 
> >>>>>> +             switch (res.a0) {
> >>>>>> +             case 1:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x038000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x040000000ULL;
> >>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>> +                     size[2] = 0x040000000ULL;
> >>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>> +                     size[3] = 0x040000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>> +                     break;
> >>>>>> +             case 2:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>>>> +                     break;
> >>>>>> +             case 3:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>> +                     size[2] = 0x080000000ULL;
> >>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>> +                     size[3] = 0x080000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>> +                     break;
> >>>>> 
> >>>>> Obvious design question is -- since you're adding new SMC call anyway,
> >>>>> can't the call just return the memory layout table itself, so that it
> >>>>> won't be duplicated both in U-Boot and ATF ?
> >>>> 
> >>>> My gut feeling was to go with the smallest interface possible.
> >>> 
> >>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>> identify memory layout tables which have to be encoded both in ATF and
> >>> U-Boot, both of which must be kept in sync.
> >>> 
> >>> The ATF already has those memory layout tables, it's only a matter of
> >>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> >>> encoding of tables into U-Boot goes away and in fact simplifies the
> >>> design.
> >>> 
> >>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>> standard SMC call to get memory topology. It surprises me that it
> >>> wouldn't.
> >> 
> >> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
> >> and I think this involved some passing of memory layout information from
> >> ATF to U-Boot too, or am I mistaken ?
> > 
> > That's correct, ATF stores information about the memory layout at a fixed
> > address in system memory, and U-Boot can read it.
> 
> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> then? :)

I'd prefer that, yes.

Let's not duplicate the mechanism used to pass FCNL information from ATF to U-
Boot, but instead create a data table format that can store all the 
information we need, in an easily extensible way.

To see how the mechanism is implemented for FCNL, search for 47FD7000 in the 
Renesas ATF sources (git://github.com/renesas-rcar/arm-trusted-firmware.git).

-- 
Regards,

Laurent Pinchart

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-16 15:44               ` Laurent Pinchart
  0 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-16 15:44 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> >>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>>>> 
> >>>>> Will this call work on platforms without patched ATF ?
> >>>>> (I think not, don't you need to handle return value?)
> >>>> 
> >>>> I have not actually tested that, but if I understand the ATF code
> >>>> correctly, unimplemented calls return
> >>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
> >>>> below.
> >>> 
> >>> Which means the board has a memory size of 0 and fails to boot ?
> >>> 
> >>>>>> +             switch (res.a0) {
> >>>>>> +             case 1:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x038000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x040000000ULL;
> >>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>> +                     size[2] = 0x040000000ULL;
> >>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>> +                     size[3] = 0x040000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>> +                     break;
> >>>>>> +             case 2:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>>>> +                     break;
> >>>>>> +             case 3:
> >>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>> +                     size[2] = 0x080000000ULL;
> >>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>> +                     size[3] = 0x080000000ULL;
> >>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>> +                     break;
> >>>>> 
> >>>>> Obvious design question is -- since you're adding new SMC call anyway,
> >>>>> can't the call just return the memory layout table itself, so that it
> >>>>> won't be duplicated both in U-Boot and ATF ?
> >>>> 
> >>>> My gut feeling was to go with the smallest interface possible.
> >>> 
> >>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>> identify memory layout tables which have to be encoded both in ATF and
> >>> U-Boot, both of which must be kept in sync.
> >>> 
> >>> The ATF already has those memory layout tables, it's only a matter of
> >>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> >>> encoding of tables into U-Boot goes away and in fact simplifies the
> >>> design.
> >>> 
> >>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>> standard SMC call to get memory topology. It surprises me that it
> >>> wouldn't.
> >> 
> >> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
> >> and I think this involved some passing of memory layout information from
> >> ATF to U-Boot too, or am I mistaken ?
> > 
> > That's correct, ATF stores information about the memory layout at a fixed
> > address in system memory, and U-Boot can read it.
> 
> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> then? :)

I'd prefer that, yes.

Let's not duplicate the mechanism used to pass FCNL information from ATF to U-
Boot, but instead create a data table format that can store all the 
information we need, in an easily extensible way.

To see how the mechanism is implemented for FCNL, search for 47FD7000 in the 
Renesas ATF sources (git://github.com/renesas-rcar/arm-trusted-firmware.git).

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-16 15:44               ` [U-Boot] " Laurent Pinchart
@ 2018-06-17  0:08                 ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-17  0:08 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Ulrich Hecht, Linux-Renesas, u-boot, Geert Uytterhoeven, shimoda,
	Magnus Damm, takuya.sakata.wz

On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
>>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>>>>>
>>>>>>> Will this call work on platforms without patched ATF ?
>>>>>>> (I think not, don't you need to handle return value?)
>>>>>>
>>>>>> I have not actually tested that, but if I understand the ATF code
>>>>>> correctly, unimplemented calls return
>>>>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
>>>>>> below.
>>>>>
>>>>> Which means the board has a memory size of 0 and fails to boot ?
>>>>>
>>>>>>>> +             switch (res.a0) {
>>>>>>>> +             case 1:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x038000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x040000000ULL;
>>>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>>>> +                     size[2] = 0x040000000ULL;
>>>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>>>> +                     size[3] = 0x040000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>>>> +                     break;
>>>>>>>> +             case 2:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>>>>>> +                     break;
>>>>>>>> +             case 3:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>>>> +                     size[2] = 0x080000000ULL;
>>>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>>>> +                     size[3] = 0x080000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>>>> +                     break;
>>>>>>>
>>>>>>> Obvious design question is -- since you're adding new SMC call anyway,
>>>>>>> can't the call just return the memory layout table itself, so that it
>>>>>>> won't be duplicated both in U-Boot and ATF ?
>>>>>>
>>>>>> My gut feeling was to go with the smallest interface possible.
>>>>>
>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>>>> identify memory layout tables which have to be encoded both in ATF and
>>>>> U-Boot, both of which must be kept in sync.
>>>>>
>>>>> The ATF already has those memory layout tables, it's only a matter of
>>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>>>>> encoding of tables into U-Boot goes away and in fact simplifies the
>>>>> design.
>>>>>
>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>>>> standard SMC call to get memory topology. It surprises me that it
>>>>> wouldn't.
>>>>
>>>> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
>>>> and I think this involved some passing of memory layout information from
>>>> ATF to U-Boot too, or am I mistaken ?
>>>
>>> That's correct, ATF stores information about the memory layout at a fixed
>>> address in system memory, and U-Boot can read it.
>>
>> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
>> then? :)
> 
> I'd prefer that, yes.
> 
> Let's not duplicate the mechanism used to pass FCNL information from ATF to U-
> Boot, but instead create a data table format that can store all the 
> information we need, in an easily extensible way.
> 
> To see how the mechanism is implemented for FCNL, search for 47FD7000 in the 
> Renesas ATF sources (git://github.com/renesas-rcar/arm-trusted-firmware.git).

For everyone involved, can you explain what FCNL is ? ;-)

Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
about passing the memory configuration around and the result is
basically the same -- pass a table from ATF to U-Boot. If there's
already something, great.

-- 
Best regards,
Marek Vasut

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-17  0:08                 ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-17  0:08 UTC (permalink / raw)
  To: u-boot

On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> Hi Marek,
> 
> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
>>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>>>>>>>
>>>>>>> Will this call work on platforms without patched ATF ?
>>>>>>> (I think not, don't you need to handle return value?)
>>>>>>
>>>>>> I have not actually tested that, but if I understand the ATF code
>>>>>> correctly, unimplemented calls return
>>>>>> SMC_UNK (0xffffffff), which should be handled by the default case (NOP)
>>>>>> below.
>>>>>
>>>>> Which means the board has a memory size of 0 and fails to boot ?
>>>>>
>>>>>>>> +             switch (res.a0) {
>>>>>>>> +             case 1:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x038000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x040000000ULL;
>>>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>>>> +                     size[2] = 0x040000000ULL;
>>>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>>>> +                     size[3] = 0x040000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>>>> +                     break;
>>>>>>>> +             case 2:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>>>>>>>> +                     break;
>>>>>>>> +             case 3:
>>>>>>>> +                     base[0] = 0x048000000ULL;
>>>>>>>> +                     size[0] = 0x078000000ULL;
>>>>>>>> +                     base[1] = 0x500000000ULL;
>>>>>>>> +                     size[1] = 0x080000000ULL;
>>>>>>>> +                     base[2] = 0x600000000ULL;
>>>>>>>> +                     size[2] = 0x080000000ULL;
>>>>>>>> +                     base[3] = 0x700000000ULL;
>>>>>>>> +                     size[3] = 0x080000000ULL;
>>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>>>>>>>> +                     break;
>>>>>>>
>>>>>>> Obvious design question is -- since you're adding new SMC call anyway,
>>>>>>> can't the call just return the memory layout table itself, so that it
>>>>>>> won't be duplicated both in U-Boot and ATF ?
>>>>>>
>>>>>> My gut feeling was to go with the smallest interface possible.
>>>>>
>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>>>> identify memory layout tables which have to be encoded both in ATF and
>>>>> U-Boot, both of which must be kept in sync.
>>>>>
>>>>> The ATF already has those memory layout tables, it's only a matter of
>>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>>>>> encoding of tables into U-Boot goes away and in fact simplifies the
>>>>> design.
>>>>>
>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>>>> standard SMC call to get memory topology. It surprises me that it
>>>>> wouldn't.
>>>>
>>>> In fact, Laurent (CCed) was solving some similar issue with lossy decomp
>>>> and I think this involved some passing of memory layout information from
>>>> ATF to U-Boot too, or am I mistaken ?
>>>
>>> That's correct, ATF stores information about the memory layout at a fixed
>>> address in system memory, and U-Boot can read it.
>>
>> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
>> then? :)
> 
> I'd prefer that, yes.
> 
> Let's not duplicate the mechanism used to pass FCNL information from ATF to U-
> Boot, but instead create a data table format that can store all the 
> information we need, in an easily extensible way.
> 
> To see how the mechanism is implemented for FCNL, search for 47FD7000 in the 
> Renesas ATF sources (git://github.com/renesas-rcar/arm-trusted-firmware.git).

For everyone involved, can you explain what FCNL is ? ;-)

Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
about passing the memory configuration around and the result is
basically the same -- pass a table from ATF to U-Boot. If there's
already something, great.

-- 
Best regards,
Marek Vasut

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-17  0:08                 ` [U-Boot] " Marek Vasut
@ 2018-06-19  2:15                   ` Laurent Pinchart
  -1 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  2:15 UTC (permalink / raw)
  To: Marek Vasut
  Cc: takuya.sakata.wz, Magnus Damm, Linux-Renesas, u-boot,
	Geert Uytterhoeven, Ulrich Hecht

On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > Hi Marek,
> > 
> > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>>>>>> 
> >>>>>>> Will this call work on platforms without patched ATF ?
> >>>>>>> (I think not, don't you need to handle return value?)
> >>>>>> 
> >>>>>> I have not actually tested that, but if I understand the ATF code
> >>>>>> correctly, unimplemented calls return
> >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
> >>>>>> (NOP)
> >>>>>> below.
> >>>>> 
> >>>>> Which means the board has a memory size of 0 and fails to boot ?
> >>>>> 
> >>>>>>>> +             switch (res.a0) {
> >>>>>>>> +             case 1:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x038000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x040000000ULL;
> >>>>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>>>> +                     size[2] = 0x040000000ULL;
> >>>>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>>>> +                     size[3] = 0x040000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>>>> +                     break;
> >>>>>>>> +             case 2:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>>>>>> +                     break;
> >>>>>>>> +             case 3:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>>>> +                     size[2] = 0x080000000ULL;
> >>>>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>>>> +                     size[3] = 0x080000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>>>> +                     break;
> >>>>>>> 
> >>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>> anyway,
> >>>>>>> can't the call just return the memory layout table itself, so that
> >>>>>>> it
> >>>>>>> won't be duplicated both in U-Boot and ATF ?
> >>>>>> 
> >>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>> 
> >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>> identify memory layout tables which have to be encoded both in ATF and
> >>>>> U-Boot, both of which must be kept in sync.
> >>>>> 
> >>>>> The ATF already has those memory layout tables, it's only a matter of
> >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
> >>>>> design.
> >>>>> 
> >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>> wouldn't.
> >>>> 
> >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>> decomp
> >>>> and I think this involved some passing of memory layout information
> >>>> from
> >>>> ATF to U-Boot too, or am I mistaken ?
> >>> 
> >>> That's correct, ATF stores information about the memory layout at a
> >>> fixed
> >>> address in system memory, and U-Boot can read it.
> >> 
> >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> >> then? :)
> > 
> > I'd prefer that, yes.
> > 
> > Let's not duplicate the mechanism used to pass FCNL information from ATF
> > to U- Boot, but instead create a data table format that can store all the
> > information we need, in an easily extensible way.
> > 
> > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
> > the Renesas ATF sources
> > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> For everyone involved, can you explain what FCNL is ? ;-)

FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth 
by transparent compression and decompression of video frames. Compression is 
handled by an IP core called FCP, and decompression is handled by the DRAM 
controller. ATF programs the DRAM controller with ranges of memory addresses 
that will be dynamically decompressed. The registers containing those ranges 
are accessible in secure mode only, so neither U-Boot nor Linux can read them. 
That's why ATF has to pass the information to U-Boot, in order to add the 
ranges as reserved memory in DT.

> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> about passing the memory configuration around and the result is
> basically the same -- pass a table from ATF to U-Boot. If there's
> already something, great.

-- 
Regards,

Laurent Pinchart



_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  2:15                   ` Laurent Pinchart
  0 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  2:15 UTC (permalink / raw)
  To: u-boot

On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > Hi Marek,
> > 
> > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> >>>>>>> 
> >>>>>>> Will this call work on platforms without patched ATF ?
> >>>>>>> (I think not, don't you need to handle return value?)
> >>>>>> 
> >>>>>> I have not actually tested that, but if I understand the ATF code
> >>>>>> correctly, unimplemented calls return
> >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
> >>>>>> (NOP)
> >>>>>> below.
> >>>>> 
> >>>>> Which means the board has a memory size of 0 and fails to boot ?
> >>>>> 
> >>>>>>>> +             switch (res.a0) {
> >>>>>>>> +             case 1:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x038000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x040000000ULL;
> >>>>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>>>> +                     size[2] = 0x040000000ULL;
> >>>>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>>>> +                     size[3] = 0x040000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>>>> +                     break;
> >>>>>>>> +             case 2:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> >>>>>>>> +                     break;
> >>>>>>>> +             case 3:
> >>>>>>>> +                     base[0] = 0x048000000ULL;
> >>>>>>>> +                     size[0] = 0x078000000ULL;
> >>>>>>>> +                     base[1] = 0x500000000ULL;
> >>>>>>>> +                     size[1] = 0x080000000ULL;
> >>>>>>>> +                     base[2] = 0x600000000ULL;
> >>>>>>>> +                     size[2] = 0x080000000ULL;
> >>>>>>>> +                     base[3] = 0x700000000ULL;
> >>>>>>>> +                     size[3] = 0x080000000ULL;
> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> >>>>>>>> +                     break;
> >>>>>>> 
> >>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>> anyway,
> >>>>>>> can't the call just return the memory layout table itself, so that
> >>>>>>> it
> >>>>>>> won't be duplicated both in U-Boot and ATF ?
> >>>>>> 
> >>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>> 
> >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>> identify memory layout tables which have to be encoded both in ATF and
> >>>>> U-Boot, both of which must be kept in sync.
> >>>>> 
> >>>>> The ATF already has those memory layout tables, it's only a matter of
> >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
> >>>>> design.
> >>>>> 
> >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>> wouldn't.
> >>>> 
> >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>> decomp
> >>>> and I think this involved some passing of memory layout information
> >>>> from
> >>>> ATF to U-Boot too, or am I mistaken ?
> >>> 
> >>> That's correct, ATF stores information about the memory layout at a
> >>> fixed
> >>> address in system memory, and U-Boot can read it.
> >> 
> >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> >> then? :)
> > 
> > I'd prefer that, yes.
> > 
> > Let's not duplicate the mechanism used to pass FCNL information from ATF
> > to U- Boot, but instead create a data table format that can store all the
> > information we need, in an easily extensible way.
> > 
> > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
> > the Renesas ATF sources
> > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> For everyone involved, can you explain what FCNL is ? ;-)

FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth 
by transparent compression and decompression of video frames. Compression is 
handled by an IP core called FCP, and decompression is handled by the DRAM 
controller. ATF programs the DRAM controller with ranges of memory addresses 
that will be dynamically decompressed. The registers containing those ranges 
are accessible in secure mode only, so neither U-Boot nor Linux can read them. 
That's why ATF has to pass the information to U-Boot, in order to add the 
ranges as reserved memory in DT.

> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> about passing the memory configuration around and the result is
> basically the same -- pass a table from ATF to U-Boot. If there's
> already something, great.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  2:15                   ` Laurent Pinchart
@ 2018-06-19  5:43                     ` Magnus Damm
  -1 siblings, 0 replies; 37+ messages in thread
From: Magnus Damm @ 2018-06-19  5:43 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Ulrich Hecht, Linux-Renesas, u-boot,
	Geert Uytterhoeven, shimoda, takuya.sakata.wz

Hi Laurent,

On Tue, Jun 19, 2018 at 11:15 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
>> > Hi Marek,
>> >
>> > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>> >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>> >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>> >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>> >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>> >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
>> >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>> >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>> >>>>>>>
>> >>>>>>> Will this call work on platforms without patched ATF ?
>> >>>>>>> (I think not, don't you need to handle return value?)
>> >>>>>>
>> >>>>>> I have not actually tested that, but if I understand the ATF code
>> >>>>>> correctly, unimplemented calls return
>> >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
>> >>>>>> (NOP)
>> >>>>>> below.
>> >>>>>
>> >>>>> Which means the board has a memory size of 0 and fails to boot ?
>> >>>>>
>> >>>>>>>> +             switch (res.a0) {
>> >>>>>>>> +             case 1:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x038000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x040000000ULL;
>> >>>>>>>> +                     base[2] = 0x600000000ULL;
>> >>>>>>>> +                     size[2] = 0x040000000ULL;
>> >>>>>>>> +                     base[3] = 0x700000000ULL;
>> >>>>>>>> +                     size[3] = 0x040000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> >>>>>>>> +                     break;
>> >>>>>>>> +             case 2:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x078000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x080000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>> >>>>>>>> +                     break;
>> >>>>>>>> +             case 3:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x078000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x080000000ULL;
>> >>>>>>>> +                     base[2] = 0x600000000ULL;
>> >>>>>>>> +                     size[2] = 0x080000000ULL;
>> >>>>>>>> +                     base[3] = 0x700000000ULL;
>> >>>>>>>> +                     size[3] = 0x080000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> >>>>>>>> +                     break;
>> >>>>>>>
>> >>>>>>> Obvious design question is -- since you're adding new SMC call
>> >>>>>>> anyway,
>> >>>>>>> can't the call just return the memory layout table itself, so that
>> >>>>>>> it
>> >>>>>>> won't be duplicated both in U-Boot and ATF ?
>> >>>>>>
>> >>>>>> My gut feeling was to go with the smallest interface possible.
>> >>>>>
>> >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>> >>>>> identify memory layout tables which have to be encoded both in ATF and
>> >>>>> U-Boot, both of which must be kept in sync.
>> >>>>>
>> >>>>> The ATF already has those memory layout tables, it's only a matter of
>> >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>> >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
>> >>>>> design.
>> >>>>>
>> >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>> >>>>> standard SMC call to get memory topology. It surprises me that it
>> >>>>> wouldn't.
>> >>>>
>> >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
>> >>>> decomp
>> >>>> and I think this involved some passing of memory layout information
>> >>>> from
>> >>>> ATF to U-Boot too, or am I mistaken ?
>> >>>
>> >>> That's correct, ATF stores information about the memory layout at a
>> >>> fixed
>> >>> address in system memory, and U-Boot can read it.
>> >>
>> >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
>> >> then? :)
>> >
>> > I'd prefer that, yes.
>> >
>> > Let's not duplicate the mechanism used to pass FCNL information from ATF
>> > to U- Boot, but instead create a data table format that can store all the
>> > information we need, in an easily extensible way.
>> >
>> > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
>> > the Renesas ATF sources
>> > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
>> For everyone involved, can you explain what FCNL is ? ;-)
>
> FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth
> by transparent compression and decompression of video frames. Compression is
> handled by an IP core called FCP, and decompression is handled by the DRAM
> controller. ATF programs the DRAM controller with ranges of memory addresses
> that will be dynamically decompressed. The registers containing those ranges
> are accessible in secure mode only, so neither U-Boot nor Linux can read them.
> That's why ATF has to pass the information to U-Boot, in order to add the
> ranges as reserved memory in DT.

Thanks for the clarification. I wonder how much of the split between
ATF and "the rest" can be adjusted. It smells like just a software
architecture policy, my gut feeling is that LIFEC can be programmed to
adjust the assignment between secure side and "regular". At least it
can do so for a wide range of bus masters. However if this is the case
for FCNL remains to be seen.

As a side note, for FCNL I personally would prefer a more dynamic
solution where we manage physically contiguous ranges from Linux
during run time instead of relying of statically setup windows.

>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
>> about passing the memory configuration around and the result is
>> basically the same -- pass a table from ATF to U-Boot. If there's
>> already something, great.
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  5:43                     ` Magnus Damm
  0 siblings, 0 replies; 37+ messages in thread
From: Magnus Damm @ 2018-06-19  5:43 UTC (permalink / raw)
  To: u-boot

Hi Laurent,

On Tue, Jun 19, 2018 at 11:15 AM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
>> > Hi Marek,
>> >
>> > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>> >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>> >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>> >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>> >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
>> >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
>> >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
>> >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
>> >>>>>>>
>> >>>>>>> Will this call work on platforms without patched ATF ?
>> >>>>>>> (I think not, don't you need to handle return value?)
>> >>>>>>
>> >>>>>> I have not actually tested that, but if I understand the ATF code
>> >>>>>> correctly, unimplemented calls return
>> >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
>> >>>>>> (NOP)
>> >>>>>> below.
>> >>>>>
>> >>>>> Which means the board has a memory size of 0 and fails to boot ?
>> >>>>>
>> >>>>>>>> +             switch (res.a0) {
>> >>>>>>>> +             case 1:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x038000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x040000000ULL;
>> >>>>>>>> +                     base[2] = 0x600000000ULL;
>> >>>>>>>> +                     size[2] = 0x040000000ULL;
>> >>>>>>>> +                     base[3] = 0x700000000ULL;
>> >>>>>>>> +                     size[3] = 0x040000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> >>>>>>>> +                     break;
>> >>>>>>>> +             case 2:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x078000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x080000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
>> >>>>>>>> +                     break;
>> >>>>>>>> +             case 3:
>> >>>>>>>> +                     base[0] = 0x048000000ULL;
>> >>>>>>>> +                     size[0] = 0x078000000ULL;
>> >>>>>>>> +                     base[1] = 0x500000000ULL;
>> >>>>>>>> +                     size[1] = 0x080000000ULL;
>> >>>>>>>> +                     base[2] = 0x600000000ULL;
>> >>>>>>>> +                     size[2] = 0x080000000ULL;
>> >>>>>>>> +                     base[3] = 0x700000000ULL;
>> >>>>>>>> +                     size[3] = 0x080000000ULL;
>> >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
>> >>>>>>>> +                     break;
>> >>>>>>>
>> >>>>>>> Obvious design question is -- since you're adding new SMC call
>> >>>>>>> anyway,
>> >>>>>>> can't the call just return the memory layout table itself, so that
>> >>>>>>> it
>> >>>>>>> won't be duplicated both in U-Boot and ATF ?
>> >>>>>>
>> >>>>>> My gut feeling was to go with the smallest interface possible.
>> >>>>>
>> >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>> >>>>> identify memory layout tables which have to be encoded both in ATF and
>> >>>>> U-Boot, both of which must be kept in sync.
>> >>>>>
>> >>>>> The ATF already has those memory layout tables, it's only a matter of
>> >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
>> >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
>> >>>>> design.
>> >>>>>
>> >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>> >>>>> standard SMC call to get memory topology. It surprises me that it
>> >>>>> wouldn't.
>> >>>>
>> >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
>> >>>> decomp
>> >>>> and I think this involved some passing of memory layout information
>> >>>> from
>> >>>> ATF to U-Boot too, or am I mistaken ?
>> >>>
>> >>> That's correct, ATF stores information about the memory layout at a
>> >>> fixed
>> >>> address in system memory, and U-Boot can read it.
>> >>
>> >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
>> >> then? :)
>> >
>> > I'd prefer that, yes.
>> >
>> > Let's not duplicate the mechanism used to pass FCNL information from ATF
>> > to U- Boot, but instead create a data table format that can store all the
>> > information we need, in an easily extensible way.
>> >
>> > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
>> > the Renesas ATF sources
>> > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
>> For everyone involved, can you explain what FCNL is ? ;-)
>
> FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth
> by transparent compression and decompression of video frames. Compression is
> handled by an IP core called FCP, and decompression is handled by the DRAM
> controller. ATF programs the DRAM controller with ranges of memory addresses
> that will be dynamically decompressed. The registers containing those ranges
> are accessible in secure mode only, so neither U-Boot nor Linux can read them.
> That's why ATF has to pass the information to U-Boot, in order to add the
> ranges as reserved memory in DT.

Thanks for the clarification. I wonder how much of the split between
ATF and "the rest" can be adjusted. It smells like just a software
architecture policy, my gut feeling is that LIFEC can be programmed to
adjust the assignment between secure side and "regular". At least it
can do so for a wide range of bus masters. However if this is the case
for FCNL remains to be seen.

As a side note, for FCNL I personally would prefer a more dynamic
solution where we manage physically contiguous ranges from Linux
during run time instead of relying of statically setup windows.

>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
>> about passing the memory configuration around and the result is
>> basically the same -- pass a table from ATF to U-Boot. If there's
>> already something, great.
>
> --
> Regards,
>
> Laurent Pinchart
>
>
>

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  5:43                     ` [U-Boot] " Magnus Damm
@ 2018-06-19  5:56                       ` Laurent Pinchart
  -1 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  5:56 UTC (permalink / raw)
  To: Magnus Damm
  Cc: takuya.sakata.wz, u-boot, Linux-Renesas, Geert Uytterhoeven,
	Ulrich Hecht

Hi Magnus,

On Tuesday, 19 June 2018 08:43:31 EEST Magnus Damm wrote:
> On Tue, Jun 19, 2018 at 11:15 AM, Laurent Pinchart wrote:
> > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:

[snip]

> >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>> wouldn't.
> >>>>>> 
> >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>> decomp and I think this involved some passing of memory layout
> >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>> 
> >>>>> That's correct, ATF stores information about the memory layout at a
> >>>>> fixed address in system memory, and U-Boot can read it.
> >>>> 
> >>>> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> >>>> then? :)
> >>> 
> >>> I'd prefer that, yes.
> >>> 
> >>> Let's not duplicate the mechanism used to pass FCNL information from
> >>> ATF to U- Boot, but instead create a data table format that can store
> >>> all the information we need, in an easily extensible way.
> >>> 
> >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>> in the Renesas ATF sources
> >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >> 
> >> For everyone involved, can you explain what FCNL is ? ;-)
> > 
> > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > bandwidth by transparent compression and decompression of video frames.
> > Compression is handled by an IP core called FCP, and decompression is
> > handled by the DRAM controller. ATF programs the DRAM controller with
> > ranges of memory addresses that will be dynamically decompressed. The
> > registers containing those ranges are accessible in secure mode only, so
> > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > information to U-Boot, in order to add the ranges as reserved memory in
> > DT.
> 
> Thanks for the clarification. I wonder how much of the split between
> ATF and "the rest" can be adjusted. It smells like just a software
> architecture policy, my gut feeling is that LIFEC can be programmed to
> adjust the assignment between secure side and "regular". At least it
> can do so for a wide range of bus masters. However if this is the case
> for FCNL remains to be seen.
> 
> As a side note, for FCNL I personally would prefer a more dynamic
> solution where we manage physically contiguous ranges from Linux
> during run time instead of relying of statically setup windows.

So would I, but whether we can be allowed to access those registers from Linux 
isn't my decision :-/

> >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >> about passing the memory configuration around and the result is
> >> basically the same -- pass a table from ATF to U-Boot. If there's
> >> already something, great.

-- 
Regards,

Laurent Pinchart



_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  5:56                       ` Laurent Pinchart
  0 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  5:56 UTC (permalink / raw)
  To: u-boot

Hi Magnus,

On Tuesday, 19 June 2018 08:43:31 EEST Magnus Damm wrote:
> On Tue, Jun 19, 2018 at 11:15 AM, Laurent Pinchart wrote:
> > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:

[snip]

> >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>> wouldn't.
> >>>>>> 
> >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>> decomp and I think this involved some passing of memory layout
> >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>> 
> >>>>> That's correct, ATF stores information about the memory layout at a
> >>>>> fixed address in system memory, and U-Boot can read it.
> >>>> 
> >>>> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> >>>> then? :)
> >>> 
> >>> I'd prefer that, yes.
> >>> 
> >>> Let's not duplicate the mechanism used to pass FCNL information from
> >>> ATF to U- Boot, but instead create a data table format that can store
> >>> all the information we need, in an easily extensible way.
> >>> 
> >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>> in the Renesas ATF sources
> >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >> 
> >> For everyone involved, can you explain what FCNL is ? ;-)
> > 
> > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > bandwidth by transparent compression and decompression of video frames.
> > Compression is handled by an IP core called FCP, and decompression is
> > handled by the DRAM controller. ATF programs the DRAM controller with
> > ranges of memory addresses that will be dynamically decompressed. The
> > registers containing those ranges are accessible in secure mode only, so
> > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > information to U-Boot, in order to add the ranges as reserved memory in
> > DT.
> 
> Thanks for the clarification. I wonder how much of the split between
> ATF and "the rest" can be adjusted. It smells like just a software
> architecture policy, my gut feeling is that LIFEC can be programmed to
> adjust the assignment between secure side and "regular". At least it
> can do so for a wide range of bus masters. However if this is the case
> for FCNL remains to be seen.
> 
> As a side note, for FCNL I personally would prefer a more dynamic
> solution where we manage physically contiguous ranges from Linux
> during run time instead of relying of statically setup windows.

So would I, but whether we can be allowed to access those registers from Linux 
isn't my decision :-/

> >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >> about passing the memory configuration around and the result is
> >> basically the same -- pass a table from ATF to U-Boot. If there's
> >> already something, great.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  5:56                       ` Laurent Pinchart
  (?)
@ 2018-06-19  6:44                       ` Magnus Damm
  -1 siblings, 0 replies; 37+ messages in thread
From: Magnus Damm @ 2018-06-19  6:44 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Ulrich Hecht, Linux-Renesas, Geert Uytterhoeven,
	shimoda, takuya.sakata.wz

Hi Laurent,

[dropped u-boot list]

On Tue, Jun 19, 2018 at 2:56 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Magnus,
>
> On Tuesday, 19 June 2018 08:43:31 EEST Magnus Damm wrote:
>> On Tue, Jun 19, 2018 at 11:15 AM, Laurent Pinchart wrote:
>> > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
>> >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
>> >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>> >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>> >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>> >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>
> [snip]
>
>> >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>> >>>>>>> standard SMC call to get memory topology. It surprises me that it
>> >>>>>>> wouldn't.
>> >>>>>>
>> >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
>> >>>>>> decomp and I think this involved some passing of memory layout
>> >>>>>> information from ATF to U-Boot too, or am I mistaken ?
>> >>>>>
>> >>>>> That's correct, ATF stores information about the memory layout at a
>> >>>>> fixed address in system memory, and U-Boot can read it.
>> >>>>
>> >>>> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
>> >>>> then? :)
>> >>>
>> >>> I'd prefer that, yes.
>> >>>
>> >>> Let's not duplicate the mechanism used to pass FCNL information from
>> >>> ATF to U- Boot, but instead create a data table format that can store
>> >>> all the information we need, in an easily extensible way.
>> >>>
>> >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
>> >>> in the Renesas ATF sources
>> >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
>> >>
>> >> For everyone involved, can you explain what FCNL is ? ;-)
>> >
>> > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
>> > bandwidth by transparent compression and decompression of video frames.
>> > Compression is handled by an IP core called FCP, and decompression is
>> > handled by the DRAM controller. ATF programs the DRAM controller with
>> > ranges of memory addresses that will be dynamically decompressed. The
>> > registers containing those ranges are accessible in secure mode only, so
>> > neither U-Boot nor Linux can read them. That's why ATF has to pass the
>> > information to U-Boot, in order to add the ranges as reserved memory in
>> > DT.
>>
>> Thanks for the clarification. I wonder how much of the split between
>> ATF and "the rest" can be adjusted. It smells like just a software
>> architecture policy, my gut feeling is that LIFEC can be programmed to
>> adjust the assignment between secure side and "regular". At least it
>> can do so for a wide range of bus masters. However if this is the case
>> for FCNL remains to be seen.
>>
>> As a side note, for FCNL I personally would prefer a more dynamic
>> solution where we manage physically contiguous ranges from Linux
>> during run time instead of relying of statically setup windows.
>
> So would I, but whether we can be allowed to access those registers from Linux
> isn't my decision :-/

I suspect the physical ranges programmed in the DRAM controller for
decompression purpose may be ranges in higher memory that are aliases
to actual physical memory. So if we would create statically aliased
ranges of the entire physical memory for decompression purpose then
inside Linux we can simply do a translation to these aliases instead
of actual physical memory at map/unmap time. This way you would get
both statically setup tables in ATF and dynamic operation inside
Linux.

Lets discuss this more when we meet tomorrow.

Cheers,

/ magnus

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  2:15                   ` Laurent Pinchart
@ 2018-06-19  6:58                     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2018-06-19  6:58 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Ulrich Hecht, Linux-Renesas, U-Boot Mailing List,
	Yoshihiro Shimoda, Magnus Damm, takuya.sakata.wz

Hi Laurent,

On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> > On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> > >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> > >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> > >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> > >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> > >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> > >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> > >>>>>>>
> > >>>>>>> Will this call work on platforms without patched ATF ?
> > >>>>>>> (I think not, don't you need to handle return value?)
> > >>>>>>
> > >>>>>> I have not actually tested that, but if I understand the ATF code
> > >>>>>> correctly, unimplemented calls return
> > >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
> > >>>>>> (NOP)
> > >>>>>> below.
> > >>>>>
> > >>>>> Which means the board has a memory size of 0 and fails to boot ?
> > >>>>>
> > >>>>>>>> +             switch (res.a0) {
> > >>>>>>>> +             case 1:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x038000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x040000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x040000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x040000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 2:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 3:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x080000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>
> > >>>>>>> Obvious design question is -- since you're adding new SMC call
> > >>>>>>> anyway,
> > >>>>>>> can't the call just return the memory layout table itself, so that
> > >>>>>>> it
> > >>>>>>> won't be duplicated both in U-Boot and ATF ?
> > >>>>>>
> > >>>>>> My gut feeling was to go with the smallest interface possible.
> > >>>>>
> > >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> > >>>>> identify memory layout tables which have to be encoded both in ATF and
> > >>>>> U-Boot, both of which must be kept in sync.
> > >>>>>
> > >>>>> The ATF already has those memory layout tables, it's only a matter of
> > >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> > >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
> > >>>>> design.
> > >>>>>
> > >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> > >>>>> standard SMC call to get memory topology. It surprises me that it
> > >>>>> wouldn't.
> > >>>>
> > >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> > >>>> decomp
> > >>>> and I think this involved some passing of memory layout information
> > >>>> from
> > >>>> ATF to U-Boot too, or am I mistaken ?
> > >>>
> > >>> That's correct, ATF stores information about the memory layout at a
> > >>> fixed
> > >>> address in system memory, and U-Boot can read it.
> > >>
> > >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> > >> then? :)
> > >
> > > I'd prefer that, yes.
> > >
> > > Let's not duplicate the mechanism used to pass FCNL information from ATF
> > > to U- Boot, but instead create a data table format that can store all the
> > > information we need, in an easily extensible way.
> > >
> > > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
> > > the Renesas ATF sources
> > > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> > For everyone involved, can you explain what FCNL is ? ;-)
>
> FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth
> by transparent compression and decompression of video frames. Compression is
> handled by an IP core called FCP, and decompression is handled by the DRAM
> controller. ATF programs the DRAM controller with ranges of memory addresses
> that will be dynamically decompressed. The registers containing those ranges
> are accessible in secure mode only, so neither U-Boot nor Linux can read them.
> That's why ATF has to pass the information to U-Boot, in order to add the
> ranges as reserved memory in DT.
>
> > Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> > about passing the memory configuration around and the result is
> > basically the same -- pass a table from ATF to U-Boot. If there's
> > already something, great.

Pass a "table"? Or an FDT containing the /memory nodes?
The latter allows for easier future extension.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  6:58                     ` Geert Uytterhoeven
  0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2018-06-19  6:58 UTC (permalink / raw)
  To: u-boot

Hi Laurent,

On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> > On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > > On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> > >> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > >>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> > >>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> > >>>>> On 06/15/2018 12:37 PM, Ulrich Hecht wrote:
> > >>>>>> On Fri, Jun 15, 2018 at 12:09 PM, Marek Vasut  wrote:
> > >>>>>>>> +             arm_smccc_smc(ARM_SMCCC_RENESAS_MEMCONF,
> > >>>>>>>> +                           0, 0, 0, 0, 0, 0, 0, &res);
> > >>>>>>>
> > >>>>>>> Will this call work on platforms without patched ATF ?
> > >>>>>>> (I think not, don't you need to handle return value?)
> > >>>>>>
> > >>>>>> I have not actually tested that, but if I understand the ATF code
> > >>>>>> correctly, unimplemented calls return
> > >>>>>> SMC_UNK (0xffffffff), which should be handled by the default case
> > >>>>>> (NOP)
> > >>>>>> below.
> > >>>>>
> > >>>>> Which means the board has a memory size of 0 and fails to boot ?
> > >>>>>
> > >>>>>>>> +             switch (res.a0) {
> > >>>>>>>> +             case 1:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x038000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x040000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x040000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x040000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 2:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 2);
> > >>>>>>>> +                     break;
> > >>>>>>>> +             case 3:
> > >>>>>>>> +                     base[0] = 0x048000000ULL;
> > >>>>>>>> +                     size[0] = 0x078000000ULL;
> > >>>>>>>> +                     base[1] = 0x500000000ULL;
> > >>>>>>>> +                     size[1] = 0x080000000ULL;
> > >>>>>>>> +                     base[2] = 0x600000000ULL;
> > >>>>>>>> +                     size[2] = 0x080000000ULL;
> > >>>>>>>> +                     base[3] = 0x700000000ULL;
> > >>>>>>>> +                     size[3] = 0x080000000ULL;
> > >>>>>>>> +                     fdt_fixup_memory_banks(blob, base, size, 4);
> > >>>>>>>> +                     break;
> > >>>>>>>
> > >>>>>>> Obvious design question is -- since you're adding new SMC call
> > >>>>>>> anyway,
> > >>>>>>> can't the call just return the memory layout table itself, so that
> > >>>>>>> it
> > >>>>>>> won't be duplicated both in U-Boot and ATF ?
> > >>>>>>
> > >>>>>> My gut feeling was to go with the smallest interface possible.
> > >>>>>
> > >>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> > >>>>> identify memory layout tables which have to be encoded both in ATF and
> > >>>>> U-Boot, both of which must be kept in sync.
> > >>>>>
> > >>>>> The ATF already has those memory layout tables, it's only a matter of
> > >>>>> passing them to U-Boot. If you do just that, the ad-hoc constants and
> > >>>>> encoding of tables into U-Boot goes away and in fact simplifies the
> > >>>>> design.
> > >>>>>
> > >>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> > >>>>> standard SMC call to get memory topology. It surprises me that it
> > >>>>> wouldn't.
> > >>>>
> > >>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> > >>>> decomp
> > >>>> and I think this involved some passing of memory layout information
> > >>>> from
> > >>>> ATF to U-Boot too, or am I mistaken ?
> > >>>
> > >>> That's correct, ATF stores information about the memory layout at a
> > >>> fixed
> > >>> address in system memory, and U-Boot can read it.
> > >>
> > >> Well, that sounds good ! Maybe we can avoid adding SMC call altogether
> > >> then? :)
> > >
> > > I'd prefer that, yes.
> > >
> > > Let's not duplicate the mechanism used to pass FCNL information from ATF
> > > to U- Boot, but instead create a data table format that can store all the
> > > information we need, in an easily extensible way.
> > >
> > > To see how the mechanism is implemented for FCNL, search for 47FD7000 in
> > > the Renesas ATF sources
> > > (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> > For everyone involved, can you explain what FCNL is ? ;-)
>
> FCNL is Frame Compression Near Lossless. It's a way to reduce memory bandwidth
> by transparent compression and decompression of video frames. Compression is
> handled by an IP core called FCP, and decompression is handled by the DRAM
> controller. ATF programs the DRAM controller with ranges of memory addresses
> that will be dynamically decompressed. The registers containing those ranges
> are accessible in secure mode only, so neither U-Boot nor Linux can read them.
> That's why ATF has to pass the information to U-Boot, in order to add the
> ranges as reserved memory in DT.
>
> > Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> > about passing the memory configuration around and the result is
> > basically the same -- pass a table from ATF to U-Boot. If there's
> > already something, great.

Pass a "table"? Or an FDT containing the /memory nodes?
The latter allows for easier future extension.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  6:58                     ` [U-Boot] " Geert Uytterhoeven
@ 2018-06-19  7:11                       ` Laurent Pinchart
  -1 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  7:11 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: takuya.sakata.wz, U-Boot Mailing List, Magnus Damm,
	Linux-Renesas, Ulrich Hecht

Hi Geert,

On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:

[snip]

> >>>>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>>>> anyway, can't the call just return the memory layout table
> >>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> >>>>>>>> 
> >>>>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>>>> 
> >>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>>>> identify memory layout tables which have to be encoded both in ATF
> >>>>>>> and U-Boot, both of which must be kept in sync.
> >>>>>>> 
> >>>>>>> The ATF already has those memory layout tables, it's only a matter
> >>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> >>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> >>>>>>> simplifies the design.
> >>>>>>> 
> >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>> wouldn't.
> >>>>>> 
> >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>> decomp and I think this involved some passing of memory layout
> >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>> 
> >>>>> That's correct, ATF stores information about the memory layout at a
> >>>>> fixed address in system memory, and U-Boot can read it.
> >>>> 
> >>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> >>>> altogether then? :)
> >>> 
> >>> I'd prefer that, yes.
> >>> 
> >>> Let's not duplicate the mechanism used to pass FCNL information from
> >>> ATF to U- Boot, but instead create a data table format that can store
> >>> all the information we need, in an easily extensible way.
> >>> 
> >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>> in the Renesas ATF sources
> >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >> 
> >> For everyone involved, can you explain what FCNL is ? ;-)
> > 
> > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > bandwidth by transparent compression and decompression of video frames.
> > Compression is handled by an IP core called FCP, and decompression is
> > handled by the DRAM controller. ATF programs the DRAM controller with
> > ranges of memory addresses that will be dynamically decompressed. The
> > registers containing those ranges are accessible in secure mode only, so
> > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > information to U-Boot, in order to add the ranges as reserved memory in
> > DT.
> > 
> >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >> about passing the memory configuration around and the result is
> >> basically the same -- pass a table from ATF to U-Boot. If there's
> >> already something, great.
> 
> Pass a "table"? Or an FDT containing the /memory nodes?
> The latter allows for easier future extension.

ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly 
before starting Linux.

-- 
Regards,

Laurent Pinchart



_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  7:11                       ` Laurent Pinchart
  0 siblings, 0 replies; 37+ messages in thread
From: Laurent Pinchart @ 2018-06-19  7:11 UTC (permalink / raw)
  To: u-boot

Hi Geert,

On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:

[snip]

> >>>>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>>>> anyway, can't the call just return the memory layout table
> >>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> >>>>>>>> 
> >>>>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>>>> 
> >>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>>>> identify memory layout tables which have to be encoded both in ATF
> >>>>>>> and U-Boot, both of which must be kept in sync.
> >>>>>>> 
> >>>>>>> The ATF already has those memory layout tables, it's only a matter
> >>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> >>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> >>>>>>> simplifies the design.
> >>>>>>> 
> >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>> wouldn't.
> >>>>>> 
> >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>> decomp and I think this involved some passing of memory layout
> >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>> 
> >>>>> That's correct, ATF stores information about the memory layout at a
> >>>>> fixed address in system memory, and U-Boot can read it.
> >>>> 
> >>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> >>>> altogether then? :)
> >>> 
> >>> I'd prefer that, yes.
> >>> 
> >>> Let's not duplicate the mechanism used to pass FCNL information from
> >>> ATF to U- Boot, but instead create a data table format that can store
> >>> all the information we need, in an easily extensible way.
> >>> 
> >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>> in the Renesas ATF sources
> >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >> 
> >> For everyone involved, can you explain what FCNL is ? ;-)
> > 
> > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > bandwidth by transparent compression and decompression of video frames.
> > Compression is handled by an IP core called FCP, and decompression is
> > handled by the DRAM controller. ATF programs the DRAM controller with
> > ranges of memory addresses that will be dynamically decompressed. The
> > registers containing those ranges are accessible in secure mode only, so
> > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > information to U-Boot, in order to add the ranges as reserved memory in
> > DT.
> > 
> >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >> about passing the memory configuration around and the result is
> >> basically the same -- pass a table from ATF to U-Boot. If there's
> >> already something, great.
> 
> Pass a "table"? Or an FDT containing the /memory nodes?
> The latter allows for easier future extension.

ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly 
before starting Linux.

-- 
Regards,

Laurent Pinchart

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  7:11                       ` Laurent Pinchart
@ 2018-06-19  7:17                         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2018-06-19  7:17 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Marek Vasut, Ulrich Hecht, Linux-Renesas, U-Boot Mailing List,
	Yoshihiro Shimoda, Magnus Damm, takuya.sakata.wz

Hi Laurent,

On Tue, Jun 19, 2018 at 9:10 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> > On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> > > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> > >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> > >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> > >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>
> [snip]
>
> > >>>>>>>>> Obvious design question is -- since you're adding new SMC call
> > >>>>>>>>> anyway, can't the call just return the memory layout table
> > >>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> > >>>>>>>>
> > >>>>>>>> My gut feeling was to go with the smallest interface possible.
> > >>>>>>>
> > >>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> > >>>>>>> identify memory layout tables which have to be encoded both in ATF
> > >>>>>>> and U-Boot, both of which must be kept in sync.
> > >>>>>>>
> > >>>>>>> The ATF already has those memory layout tables, it's only a matter
> > >>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> > >>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> > >>>>>>> simplifies the design.
> > >>>>>>>
> > >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> > >>>>>>> standard SMC call to get memory topology. It surprises me that it
> > >>>>>>> wouldn't.
> > >>>>>>
> > >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> > >>>>>> decomp and I think this involved some passing of memory layout
> > >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> > >>>>>
> > >>>>> That's correct, ATF stores information about the memory layout at a
> > >>>>> fixed address in system memory, and U-Boot can read it.
> > >>>>
> > >>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> > >>>> altogether then? :)
> > >>>
> > >>> I'd prefer that, yes.
> > >>>
> > >>> Let's not duplicate the mechanism used to pass FCNL information from
> > >>> ATF to U- Boot, but instead create a data table format that can store
> > >>> all the information we need, in an easily extensible way.
> > >>>
> > >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> > >>> in the Renesas ATF sources
> > >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> > >>
> > >> For everyone involved, can you explain what FCNL is ? ;-)
> > >
> > > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > > bandwidth by transparent compression and decompression of video frames.
> > > Compression is handled by an IP core called FCP, and decompression is
> > > handled by the DRAM controller. ATF programs the DRAM controller with
> > > ranges of memory addresses that will be dynamically decompressed. The
> > > registers containing those ranges are accessible in secure mode only, so
> > > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > > information to U-Boot, in order to add the ranges as reserved memory in
> > > DT.
> > >
> > >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> > >> about passing the memory configuration around and the result is
> > >> basically the same -- pass a table from ATF to U-Boot. If there's
> > >> already something, great.
> >
> > Pass a "table"? Or an FDT containing the /memory nodes?
> > The latter allows for easier future extension.
>
> ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly
> before starting Linux.

I know it passes a table. A table is very easy to parse, but complicates future
extension.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-19  7:17                         ` Geert Uytterhoeven
  0 siblings, 0 replies; 37+ messages in thread
From: Geert Uytterhoeven @ 2018-06-19  7:17 UTC (permalink / raw)
  To: u-boot

Hi Laurent,

On Tue, Jun 19, 2018 at 9:10 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> > On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> > > On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> > >> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> > >>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> > >>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> > >>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> > >>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
>
> [snip]
>
> > >>>>>>>>> Obvious design question is -- since you're adding new SMC call
> > >>>>>>>>> anyway, can't the call just return the memory layout table
> > >>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> > >>>>>>>>
> > >>>>>>>> My gut feeling was to go with the smallest interface possible.
> > >>>>>>>
> > >>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> > >>>>>>> identify memory layout tables which have to be encoded both in ATF
> > >>>>>>> and U-Boot, both of which must be kept in sync.
> > >>>>>>>
> > >>>>>>> The ATF already has those memory layout tables, it's only a matter
> > >>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> > >>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> > >>>>>>> simplifies the design.
> > >>>>>>>
> > >>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> > >>>>>>> standard SMC call to get memory topology. It surprises me that it
> > >>>>>>> wouldn't.
> > >>>>>>
> > >>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> > >>>>>> decomp and I think this involved some passing of memory layout
> > >>>>>> information from ATF to U-Boot too, or am I mistaken ?
> > >>>>>
> > >>>>> That's correct, ATF stores information about the memory layout at a
> > >>>>> fixed address in system memory, and U-Boot can read it.
> > >>>>
> > >>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> > >>>> altogether then? :)
> > >>>
> > >>> I'd prefer that, yes.
> > >>>
> > >>> Let's not duplicate the mechanism used to pass FCNL information from
> > >>> ATF to U- Boot, but instead create a data table format that can store
> > >>> all the information we need, in an easily extensible way.
> > >>>
> > >>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> > >>> in the Renesas ATF sources
> > >>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> > >>
> > >> For everyone involved, can you explain what FCNL is ? ;-)
> > >
> > > FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> > > bandwidth by transparent compression and decompression of video frames.
> > > Compression is handled by an IP core called FCP, and decompression is
> > > handled by the DRAM controller. ATF programs the DRAM controller with
> > > ranges of memory addresses that will be dynamically decompressed. The
> > > registers containing those ranges are accessible in secure mode only, so
> > > neither U-Boot nor Linux can read them. That's why ATF has to pass the
> > > information to U-Boot, in order to add the ranges as reserved memory in
> > > DT.
> > >
> > >> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> > >> about passing the memory configuration around and the result is
> > >> basically the same -- pass a table from ATF to U-Boot. If there's
> > >> already something, great.
> >
> > Pass a "table"? Or an FDT containing the /memory nodes?
> > The latter allows for easier future extension.
>
> ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly
> before starting Linux.

I know it passes a table. A table is very easy to parse, but complicates future
extension.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-19  7:11                       ` Laurent Pinchart
@ 2018-06-20  4:55                         ` Marek Vasut
  -1 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-20  4:55 UTC (permalink / raw)
  To: Laurent Pinchart, Geert Uytterhoeven
  Cc: Ulrich Hecht, Linux-Renesas, U-Boot Mailing List,
	Yoshihiro Shimoda, Magnus Damm, takuya.sakata.wz

On 06/19/2018 09:11 AM, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
>> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
>>> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
>>>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
>>>>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>>>>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>>>>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>>>>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> 
> [snip]
> 
>>>>>>>>>>> Obvious design question is -- since you're adding new SMC call
>>>>>>>>>>> anyway, can't the call just return the memory layout table
>>>>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
>>>>>>>>>>
>>>>>>>>>> My gut feeling was to go with the smallest interface possible.
>>>>>>>>>
>>>>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>>>>>>>> identify memory layout tables which have to be encoded both in ATF
>>>>>>>>> and U-Boot, both of which must be kept in sync.
>>>>>>>>>
>>>>>>>>> The ATF already has those memory layout tables, it's only a matter
>>>>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
>>>>>>>>> constants and encoding of tables into U-Boot goes away and in fact
>>>>>>>>> simplifies the design.
>>>>>>>>>
>>>>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>>>>>>>> standard SMC call to get memory topology. It surprises me that it
>>>>>>>>> wouldn't.
>>>>>>>>
>>>>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
>>>>>>>> decomp and I think this involved some passing of memory layout
>>>>>>>> information from ATF to U-Boot too, or am I mistaken ?
>>>>>>>
>>>>>>> That's correct, ATF stores information about the memory layout at a
>>>>>>> fixed address in system memory, and U-Boot can read it.
>>>>>>
>>>>>> Well, that sounds good ! Maybe we can avoid adding SMC call
>>>>>> altogether then? :)
>>>>>
>>>>> I'd prefer that, yes.
>>>>>
>>>>> Let's not duplicate the mechanism used to pass FCNL information from
>>>>> ATF to U- Boot, but instead create a data table format that can store
>>>>> all the information we need, in an easily extensible way.
>>>>>
>>>>> To see how the mechanism is implemented for FCNL, search for 47FD7000
>>>>> in the Renesas ATF sources
>>>>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
>>>>
>>>> For everyone involved, can you explain what FCNL is ? ;-)
>>>
>>> FCNL is Frame Compression Near Lossless. It's a way to reduce memory
>>> bandwidth by transparent compression and decompression of video frames.
>>> Compression is handled by an IP core called FCP, and decompression is
>>> handled by the DRAM controller. ATF programs the DRAM controller with
>>> ranges of memory addresses that will be dynamically decompressed. The
>>> registers containing those ranges are accessible in secure mode only, so
>>> neither U-Boot nor Linux can read them. That's why ATF has to pass the
>>> information to U-Boot, in order to add the ranges as reserved memory in
>>> DT.
>>>
>>>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
>>>> about passing the memory configuration around and the result is
>>>> basically the same -- pass a table from ATF to U-Boot. If there's
>>>> already something, great.
>>
>> Pass a "table"? Or an FDT containing the /memory nodes?
>> The latter allows for easier future extension.
> 
> ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly 
> before starting Linux.

At this point, U-Boot does not parse any such table. It could be some
fork does that, but mainline does not. But that code could (and probably
should) be added.

I don't think ATF has FDT support, but I might be wrong. And if ATF can
pass DT fragment or some simple DT, that'd be neat.

-- 
Best regards,
Marek Vasut

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-20  4:55                         ` Marek Vasut
  0 siblings, 0 replies; 37+ messages in thread
From: Marek Vasut @ 2018-06-20  4:55 UTC (permalink / raw)
  To: u-boot

On 06/19/2018 09:11 AM, Laurent Pinchart wrote:
> Hi Geert,
> 
> On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
>> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
>>> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
>>>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
>>>>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
>>>>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
>>>>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
>>>>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> 
> [snip]
> 
>>>>>>>>>>> Obvious design question is -- since you're adding new SMC call
>>>>>>>>>>> anyway, can't the call just return the memory layout table
>>>>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
>>>>>>>>>>
>>>>>>>>>> My gut feeling was to go with the smallest interface possible.
>>>>>>>>>
>>>>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
>>>>>>>>> identify memory layout tables which have to be encoded both in ATF
>>>>>>>>> and U-Boot, both of which must be kept in sync.
>>>>>>>>>
>>>>>>>>> The ATF already has those memory layout tables, it's only a matter
>>>>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
>>>>>>>>> constants and encoding of tables into U-Boot goes away and in fact
>>>>>>>>> simplifies the design.
>>>>>>>>>
>>>>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
>>>>>>>>> standard SMC call to get memory topology. It surprises me that it
>>>>>>>>> wouldn't.
>>>>>>>>
>>>>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
>>>>>>>> decomp and I think this involved some passing of memory layout
>>>>>>>> information from ATF to U-Boot too, or am I mistaken ?
>>>>>>>
>>>>>>> That's correct, ATF stores information about the memory layout at a
>>>>>>> fixed address in system memory, and U-Boot can read it.
>>>>>>
>>>>>> Well, that sounds good ! Maybe we can avoid adding SMC call
>>>>>> altogether then? :)
>>>>>
>>>>> I'd prefer that, yes.
>>>>>
>>>>> Let's not duplicate the mechanism used to pass FCNL information from
>>>>> ATF to U- Boot, but instead create a data table format that can store
>>>>> all the information we need, in an easily extensible way.
>>>>>
>>>>> To see how the mechanism is implemented for FCNL, search for 47FD7000
>>>>> in the Renesas ATF sources
>>>>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
>>>>
>>>> For everyone involved, can you explain what FCNL is ? ;-)
>>>
>>> FCNL is Frame Compression Near Lossless. It's a way to reduce memory
>>> bandwidth by transparent compression and decompression of video frames.
>>> Compression is handled by an IP core called FCP, and decompression is
>>> handled by the DRAM controller. ATF programs the DRAM controller with
>>> ranges of memory addresses that will be dynamically decompressed. The
>>> registers containing those ranges are accessible in secure mode only, so
>>> neither U-Boot nor Linux can read them. That's why ATF has to pass the
>>> information to U-Boot, in order to add the ranges as reserved memory in
>>> DT.
>>>
>>>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
>>>> about passing the memory configuration around and the result is
>>>> basically the same -- pass a table from ATF to U-Boot. If there's
>>>> already something, great.
>>
>> Pass a "table"? Or an FDT containing the /memory nodes?
>> The latter allows for easier future extension.
> 
> ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly 
> before starting Linux.

At this point, U-Boot does not parse any such table. It could be some
fork does that, but mainline does not. But that code could (and probably
should) be added.

I don't think ATF has FDT support, but I might be wrong. And if ATF can
pass DT fragment or some simple DT, that'd be neat.

-- 
Best regards,
Marek Vasut

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

* Re: [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
  2018-06-20  4:55                         ` [U-Boot] " Marek Vasut
@ 2018-06-28 17:24                           ` Eugeniu Rosca
  -1 siblings, 0 replies; 37+ messages in thread
From: Eugeniu Rosca @ 2018-06-28 17:24 UTC (permalink / raw)
  To: marek.vasut
  Cc: takuya.sakata.wz, magnus.damm, Linux-Renesas,
	U-Boot Mailing List, Geert Uytterhoeven, Laurent Pinchart,
	ulrich.hecht+renesas, Eugeniu Rosca

Hi Marek, All,

I am looking forward for the solution/approach you are going to reach
in this thread.
Currently, integrating Renesas Yocto v3.7.0 release [1], I can see
that the increase of SoC/SiP versions supported in this release
increases the number of ATF/U-boot build variants dramatically.

Renesas U-boot starts to rely not only on defconfig, but also on
build-time flags passed via Yocto recipes (see usage of
get_uboot_config_opt in [2,3]). This doesn't look right (I don't see
U-Boot KCFLAGS being adjusted too often in mainline). Latest Renesas
ATF also adds a bunch of new make flags, all being related to SiP DRAM
configuration, which means it's very easy to pick the wrong setting at
build-time and then deal with obscure issues afterwards (hopefully the
target just doesn't boot) :(

FWIW, with regard to FDT support in ATF, it seems to be there starting
with commit [4] and there are already users/callers of libfdt APIs in
the tree [5].
Something I am wondering about is why not getting the memory
configuration in ATF at runtime? Haven't DRAM vendors thought this
information (number/size of DDR banks) would be useful and should be
exposed by HW and accessible by SW? Currently Renesas tells their
customers that the correct SiP-specific DDR configuration has to be
hardcoded during build process. If anybody has an explanation why
runtime DDR detection is not possible in ATF, could you please share?

Best regards,
Eugeniu.

[1] https://github.com/renesas-rcar/meta-renesas/releases/tag/Renesas-Yocto-v3.7.0
[2] https://github.com/renesas-rcar/meta-renesas/blob/Renesas-Yocto-v3.7.0/meta-rcar-gen3/include/uboot-control.inc
[3] https://github.com/renesas-rcar/meta-renesas/blob/Renesas-Yocto-v3.7.0/meta-rcar-gen3/recipes-bsp/u-boot/u-boot_2015.04.bb
[4] https://github.com/ARM-software/arm-trusted-firmware/commit/91176bc6363d
("Import libfdt v1.4.1")
[5] git grep -El "fdt_|fdtw_" -- ":\!*lib/libfdt/*" "*.c"
common/fdt_wrappers.c
plat/arm/common/arm_dyn_cfg_helpers.c
plat/arm/css/sgi/sgi_image_load.c
plat/qemu/dt.c
plat/qemu/qemu_bl2_setup.c
On Wed, Jun 20, 2018 at 6:58 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>
> On 06/19/2018 09:11 AM, Laurent Pinchart wrote:
> > Hi Geert,
> >
> > On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> >> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> >>> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>>>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >
> > [snip]
> >
> >>>>>>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>>>>>> anyway, can't the call just return the memory layout table
> >>>>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> >>>>>>>>>>
> >>>>>>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>>>>>>
> >>>>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>>>>>> identify memory layout tables which have to be encoded both in ATF
> >>>>>>>>> and U-Boot, both of which must be kept in sync.
> >>>>>>>>>
> >>>>>>>>> The ATF already has those memory layout tables, it's only a matter
> >>>>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> >>>>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> >>>>>>>>> simplifies the design.
> >>>>>>>>>
> >>>>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>>>> wouldn't.
> >>>>>>>>
> >>>>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>>>> decomp and I think this involved some passing of memory layout
> >>>>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>>>>
> >>>>>>> That's correct, ATF stores information about the memory layout at a
> >>>>>>> fixed address in system memory, and U-Boot can read it.
> >>>>>>
> >>>>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> >>>>>> altogether then? :)
> >>>>>
> >>>>> I'd prefer that, yes.
> >>>>>
> >>>>> Let's not duplicate the mechanism used to pass FCNL information from
> >>>>> ATF to U- Boot, but instead create a data table format that can store
> >>>>> all the information we need, in an easily extensible way.
> >>>>>
> >>>>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>>>> in the Renesas ATF sources
> >>>>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >>>>
> >>>> For everyone involved, can you explain what FCNL is ? ;-)
> >>>
> >>> FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> >>> bandwidth by transparent compression and decompression of video frames.
> >>> Compression is handled by an IP core called FCP, and decompression is
> >>> handled by the DRAM controller. ATF programs the DRAM controller with
> >>> ranges of memory addresses that will be dynamically decompressed. The
> >>> registers containing those ranges are accessible in secure mode only, so
> >>> neither U-Boot nor Linux can read them. That's why ATF has to pass the
> >>> information to U-Boot, in order to add the ranges as reserved memory in
> >>> DT.
> >>>
> >>>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >>>> about passing the memory configuration around and the result is
> >>>> basically the same -- pass a table from ATF to U-Boot. If there's
> >>>> already something, great.
> >>
> >> Pass a "table"? Or an FDT containing the /memory nodes?
> >> The latter allows for easier future extension.
> >
> > ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly
> > before starting Linux.
>
> At this point, U-Boot does not parse any such table. It could be some
> fork does that, but mainline does not. But that code could (and probably
> should) be added.
>
> I don't think ATF has FDT support, but I might be wrong. And if ATF can
> pass DT fragment or some simple DT, that'd be neat.
>
> --
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

* [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer
@ 2018-06-28 17:24                           ` Eugeniu Rosca
  0 siblings, 0 replies; 37+ messages in thread
From: Eugeniu Rosca @ 2018-06-28 17:24 UTC (permalink / raw)
  To: u-boot

Hi Marek, All,

I am looking forward for the solution/approach you are going to reach
in this thread.
Currently, integrating Renesas Yocto v3.7.0 release [1], I can see
that the increase of SoC/SiP versions supported in this release
increases the number of ATF/U-boot build variants dramatically.

Renesas U-boot starts to rely not only on defconfig, but also on
build-time flags passed via Yocto recipes (see usage of
get_uboot_config_opt in [2,3]). This doesn't look right (I don't see
U-Boot KCFLAGS being adjusted too often in mainline). Latest Renesas
ATF also adds a bunch of new make flags, all being related to SiP DRAM
configuration, which means it's very easy to pick the wrong setting at
build-time and then deal with obscure issues afterwards (hopefully the
target just doesn't boot) :(

FWIW, with regard to FDT support in ATF, it seems to be there starting
with commit [4] and there are already users/callers of libfdt APIs in
the tree [5].
Something I am wondering about is why not getting the memory
configuration in ATF at runtime? Haven't DRAM vendors thought this
information (number/size of DDR banks) would be useful and should be
exposed by HW and accessible by SW? Currently Renesas tells their
customers that the correct SiP-specific DDR configuration has to be
hardcoded during build process. If anybody has an explanation why
runtime DDR detection is not possible in ATF, could you please share?

Best regards,
Eugeniu.

[1] https://github.com/renesas-rcar/meta-renesas/releases/tag/Renesas-Yocto-v3.7.0
[2] https://github.com/renesas-rcar/meta-renesas/blob/Renesas-Yocto-v3.7.0/meta-rcar-gen3/include/uboot-control.inc
[3] https://github.com/renesas-rcar/meta-renesas/blob/Renesas-Yocto-v3.7.0/meta-rcar-gen3/recipes-bsp/u-boot/u-boot_2015.04.bb
[4] https://github.com/ARM-software/arm-trusted-firmware/commit/91176bc6363d
("Import libfdt v1.4.1")
[5] git grep -El "fdt_|fdtw_" -- ":\!*lib/libfdt/*" "*.c"
common/fdt_wrappers.c
plat/arm/common/arm_dyn_cfg_helpers.c
plat/arm/css/sgi/sgi_image_load.c
plat/qemu/dt.c
plat/qemu/qemu_bl2_setup.c
On Wed, Jun 20, 2018 at 6:58 AM Marek Vasut <marek.vasut@gmail.com> wrote:
>
> On 06/19/2018 09:11 AM, Laurent Pinchart wrote:
> > Hi Geert,
> >
> > On Tuesday, 19 June 2018 09:58:59 EEST Geert Uytterhoeven wrote:
> >> On Tue, Jun 19, 2018 at 4:15 AM Laurent Pinchart wrote:
> >>> On Sunday, 17 June 2018 03:08:02 EEST Marek Vasut wrote:
> >>>> On 06/16/2018 05:44 PM, Laurent Pinchart wrote:
> >>>>> On Saturday, 16 June 2018 02:42:30 EEST Marek Vasut wrote:
> >>>>>> On 06/16/2018 01:21 AM, Laurent Pinchart wrote:
> >>>>>>> On Friday, 15 June 2018 15:00:31 EEST Marek Vasut wrote:
> >>>>>>>> On 06/15/2018 01:43 PM, Marek Vasut wrote:
> >
> > [snip]
> >
> >>>>>>>>>>> Obvious design question is -- since you're adding new SMC call
> >>>>>>>>>>> anyway, can't the call just return the memory layout table
> >>>>>>>>>>> itself, so that it won't be duplicated both in U-Boot and ATF ?
> >>>>>>>>>>
> >>>>>>>>>> My gut feeling was to go with the smallest interface possible.
> >>>>>>>>>
> >>>>>>>>> But this doesn't scale. The API here uses some ad-hoc constants to
> >>>>>>>>> identify memory layout tables which have to be encoded both in ATF
> >>>>>>>>> and U-Boot, both of which must be kept in sync.
> >>>>>>>>>
> >>>>>>>>> The ATF already has those memory layout tables, it's only a matter
> >>>>>>>>> of passing them to U-Boot. If you do just that, the ad-hoc
> >>>>>>>>> constants and encoding of tables into U-Boot goes away and in fact
> >>>>>>>>> simplifies the design.
> >>>>>>>>>
> >>>>>>>>> Yet, I have to wonder if ATF doesn't already contain some sort of
> >>>>>>>>> standard SMC call to get memory topology. It surprises me that it
> >>>>>>>>> wouldn't.
> >>>>>>>>
> >>>>>>>> In fact, Laurent (CCed) was solving some similar issue with lossy
> >>>>>>>> decomp and I think this involved some passing of memory layout
> >>>>>>>> information from ATF to U-Boot too, or am I mistaken ?
> >>>>>>>
> >>>>>>> That's correct, ATF stores information about the memory layout at a
> >>>>>>> fixed address in system memory, and U-Boot can read it.
> >>>>>>
> >>>>>> Well, that sounds good ! Maybe we can avoid adding SMC call
> >>>>>> altogether then? :)
> >>>>>
> >>>>> I'd prefer that, yes.
> >>>>>
> >>>>> Let's not duplicate the mechanism used to pass FCNL information from
> >>>>> ATF to U- Boot, but instead create a data table format that can store
> >>>>> all the information we need, in an easily extensible way.
> >>>>>
> >>>>> To see how the mechanism is implemented for FCNL, search for 47FD7000
> >>>>> in the Renesas ATF sources
> >>>>> (git://github.com/renesas-rcar/arm-trusted-firmware.git).
> >>>>
> >>>> For everyone involved, can you explain what FCNL is ? ;-)
> >>>
> >>> FCNL is Frame Compression Near Lossless. It's a way to reduce memory
> >>> bandwidth by transparent compression and decompression of video frames.
> >>> Compression is handled by an IP core called FCP, and decompression is
> >>> handled by the DRAM controller. ATF programs the DRAM controller with
> >>> ranges of memory addresses that will be dynamically decompressed. The
> >>> registers containing those ranges are accessible in secure mode only, so
> >>> neither U-Boot nor Linux can read them. That's why ATF has to pass the
> >>> information to U-Boot, in order to add the ranges as reserved memory in
> >>> DT.
> >>>
> >>>> Any yes, I agree this sounds good. I had a discussion on the U-Boot IRC
> >>>> about passing the memory configuration around and the result is
> >>>> basically the same -- pass a table from ATF to U-Boot. If there's
> >>>> already something, great.
> >>
> >> Pass a "table"? Or an FDT containing the /memory nodes?
> >> The latter allows for easier future extension.
> >
> > ATF passes a table to U-Boot, and U-Boot then updates the FDT accordingly
> > before starting Linux.
>
> At this point, U-Boot does not parse any such table. It could be some
> fork does that, but mainline does not. But that code could (and probably
> should) be added.
>
> I don't think ATF has FDT support, but I might be wrong. And if ATF can
> pass DT fragment or some simple DT, that'd be neat.
>
> --
> Best regards,
> Marek Vasut
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

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

end of thread, other threads:[~2018-06-28 17:24 UTC | newest]

Thread overview: 37+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-15  9:40 [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer Ulrich Hecht
2018-06-15  9:40 ` [U-Boot] " Ulrich Hecht
2018-06-15  9:40 ` [RFC ATF] Add SMCCC_RENESAS_MEMCONF SMC call Ulrich Hecht
2018-06-15  9:40   ` [U-Boot] " Ulrich Hecht
2018-06-15 10:09 ` [U-Boot] [RFC] ARM: rmobile: create DT memory nodes for R8A7795 3.0 and newer Marek Vasut
2018-06-15 10:09   ` Marek Vasut
2018-06-15 10:37   ` Ulrich Hecht
2018-06-15 10:37     ` [U-Boot] " Ulrich Hecht
2018-06-15 11:43     ` Marek Vasut
2018-06-15 11:43       ` Marek Vasut
2018-06-15 12:00       ` Marek Vasut
2018-06-15 12:00         ` Marek Vasut
2018-06-15 23:21         ` Laurent Pinchart
2018-06-15 23:21           ` [U-Boot] " Laurent Pinchart
2018-06-15 23:42           ` Marek Vasut
2018-06-15 23:42             ` [U-Boot] " Marek Vasut
2018-06-16 15:44             ` Laurent Pinchart
2018-06-16 15:44               ` [U-Boot] " Laurent Pinchart
2018-06-17  0:08               ` Marek Vasut
2018-06-17  0:08                 ` [U-Boot] " Marek Vasut
2018-06-19  2:15                 ` Laurent Pinchart
2018-06-19  2:15                   ` Laurent Pinchart
2018-06-19  5:43                   ` Magnus Damm
2018-06-19  5:43                     ` [U-Boot] " Magnus Damm
2018-06-19  5:56                     ` Laurent Pinchart
2018-06-19  5:56                       ` Laurent Pinchart
2018-06-19  6:44                       ` Magnus Damm
2018-06-19  6:58                   ` Geert Uytterhoeven
2018-06-19  6:58                     ` [U-Boot] " Geert Uytterhoeven
2018-06-19  7:11                     ` Laurent Pinchart
2018-06-19  7:11                       ` Laurent Pinchart
2018-06-19  7:17                       ` Geert Uytterhoeven
2018-06-19  7:17                         ` [U-Boot] " Geert Uytterhoeven
2018-06-20  4:55                       ` Marek Vasut
2018-06-20  4:55                         ` [U-Boot] " Marek Vasut
2018-06-28 17:24                         ` Eugeniu Rosca
2018-06-28 17:24                           ` Eugeniu Rosca

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.