On Fri, Mar 11, 2022 at 9:25 PM Simon Glass sjg@chromium.org wrote:
Hi Peter,
On Mon, 21 Feb 2022 at 18:31, Peter Geis pgwipeout@gmail.com wrote:
This patch attempts to fix ram detection on rk3568. Prior to this, the rk3568 incorrectly detected 8gb ram as 2gb. On top of this, the board panics when u-boot accesses ram above 4gb.
Fix this by correcting ram detection in hopefully a backwards compatable way, and extend board_f.c to enforce an upper limit on the ram u-boot uses. This allows us to limit the ram u-boot accesses, while passing the correctly detected size to follow on software (eg linux).
This has been tested on rk3566 2gb, 4gb, and 8gb configurations, as well as rk3399 4gb configurations.
I do not have other configurations available, and I do not have the insights into rockchip ram handling to tell if this is the correct way to go about this.
Signed-off-by: Peter Geis pgwipeout@gmail.com
arch/arm/mach-rockchip/Kconfig | 1 + arch/arm/mach-rockchip/rk3568/rk3568.c | 29 ++++++++++++++++++++++++++ arch/arm/mach-rockchip/sdram.c | 19 +++++++++++------ common/board_f.c | 7 +++++++ include/configs/rk3568_common.h | 5 +++++ 5 files changed, 55 insertions(+), 6 deletions(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 92f35309e4a6..58393cc623f8 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -264,6 +264,7 @@ config ROCKCHIP_RK3568 select SYSCON select BOARD_LATE_INIT imply ROCKCHIP_COMMON_BOARD
imply OF_SYSTEM_SETUP help The Rockchip RK3568 is a ARM-based SoC with quad-core Cortex-A55, including NEON and GPU, 512K L3 cache, Mali-G52 based graphics,diff --git a/arch/arm/mach-rockchip/rk3568/rk3568.c b/arch/arm/mach-rockchip/rk3568/rk3568.c index ef6bc67a88b0..8d2a59bc649d 100644 --- a/arch/arm/mach-rockchip/rk3568/rk3568.c +++ b/arch/arm/mach-rockchip/rk3568/rk3568.c @@ -5,6 +5,7 @@
#include <common.h> #include <dm.h> +#include <fdt_support.h> #include <asm/armv8/mmu.h> #include <asm/io.h> #include <asm/arch-rockchip/bootrom.h> @@ -187,3 +188,31 @@ int board_usb_init(int index, enum usb_init_type init) #endif /* CONFIG_USB_DWC3_GADGET */
#endif /* CONFIG_USB_GADGET */
+#ifdef CONFIG_OF_SYSTEM_SETUP +int ft_system_setup(void *blob, struct bd_info *bd) +{
int ret;int areas = 1;u64 start[2], size[2];/* Reserve the io address space. */if (gd->ram_top > SDRAM_UPPER_ADDR_MIN) {Can you put SDRAM_UPPER_ADDR_MIN and SDRAM_LOWER_ADDR_MAX into the device tree, perhaps?
This is the part of the patch series that gets really cludgy and I'm not proud of it. Essentially this is a cleaner implementation of what downstream does. The issue is u-boot panics if it tries to access ram above 4G on this chip. Combined with the MMIO address space below 4G, the available memory map gets to be quite a mess. I did try to keep it backwards compatible with the previous chips however. The rk356x has 2G, 4G, and 8G boards in the wild.
I'll play a bit with a device tree method, to see if I can make it make sense though.
start[0] = gd->bd->bi_dram[0].start;size[0] = SDRAM_LOWER_ADDR_MAX - gd->bd->bi_dram[0].start;/* Add the upper 4GB address space */start[1] = SDRAM_UPPER_ADDR_MIN;size[1] = gd->ram_top - SDRAM_UPPER_ADDR_MIN;areas = 2;ret = fdt_set_usable_memory(blob, start, size, areas);if (ret) {printf("Cannot set usable memory\n");return ret;}}return 0;+}; +#endif diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c index 705ec7ba6450..52974e6dc333 100644 --- a/arch/arm/mach-rockchip/sdram.c +++ b/arch/arm/mach-rockchip/sdram.c @@ -3,6 +3,8 @@
- Copyright (C) 2017 Rockchip Electronics Co., Ltd.
*/
+#define DEBUG
#include <common.h> #include <dm.h> #include <init.h> @@ -98,8 +100,7 @@ size_t rockchip_sdram_size(phys_addr_t reg) SYS_REG_COL_MASK); cs1_col = cs0_col; bk = 3 - ((sys_reg2 >> SYS_REG_BK_SHIFT(ch)) & SYS_REG_BK_MASK);
if ((sys_reg3 >> SYS_REG_VERSION_SHIFT &SYS_REG_VERSION_MASK) == 0x2) {
if ((sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) >= 0x2) { cs1_col = 9 + (sys_reg3 >> SYS_REG_CS1_COL_SHIFT(ch) & SYS_REG_CS1_COL_MASK); if (((sys_reg3 >> SYS_REG_EXTEND_CS0_ROW_SHIFT(ch) &@@ -136,7 +137,7 @@ size_t rockchip_sdram_size(phys_addr_t reg) SYS_REG_BW_MASK)); row_3_4 = sys_reg2 >> SYS_REG_ROW_3_4_SHIFT(ch) & SYS_REG_ROW_3_4_MASK;
if (dram_type == DDR4) {
if ((dram_type == DDR4) && (sys_reg3 >> SYS_REG_VERSION_SHIFT & SYS_REG_VERSION_MASK) != 0x3){Space before {
Also can you please add a comment as to what this is doing?
Thanks! Indeed, I can. For reference here, it's retaining backwards compatibility with older versions of the ram controller, but the new ram controller ends up doubling the available ram if this is enabled.
dbw = (sys_reg2 >> SYS_REG_DBW_SHIFT(ch)) & SYS_REG_DBW_MASK; bg = (dbw == 2) ? 2 : 1;@@ -176,9 +177,11 @@ size_t rockchip_sdram_size(phys_addr_t reg) * 2. update board_get_usable_ram_top() and dram_init_banksize() * to reserve memory for peripheral space after previous update. */
+#ifndef __aarch64__
if (!IS_ENABLED(CONFIG_ARM64))
Thanks!
if (size_mb > (SDRAM_MAX_SIZE >> 20)) size_mb = (SDRAM_MAX_SIZE >> 20);
+#endif return (size_t)size_mb << 20; }
@@ -208,6 +211,10 @@ int dram_init(void) ulong board_get_usable_ram_top(ulong total_size) { unsigned long top = CONFIG_SYS_SDRAM_BASE + SDRAM_MAX_SIZE;
return (gd->ram_top > top) ? top : gd->ram_top;+#ifdef SDRAM_UPPER_ADDR_MIN
eek, what is going on here? Can you use C instead of the preprocessor?
I was trying to keep to current convention in u-boot (which I admit is uncomfortable coming from linux). This is clamping the max available u-boot ram to SDRAM_UPPER_ADDR_MIN, if it is set. This is necessary because of the above panic issue, and is very much a hack. Essentially I attempted to make this affect nothing if it isn't set, thus if it isn't set boards that have 8GB will detect as 2GB.
The way rockchip currently clamps the ram to less than the MMIO space feels really slimy too, but I didn't want to try and touch it too much and risk breaking things.
if (gd->ram_top > SDRAM_UPPER_ADDR_MIN)return gd->ram_top;else+#endif
return (gd->ram_top > top) ? top : gd->ram_top;} diff --git a/common/board_f.c b/common/board_f.c index a68760092ac1..933ba7aedac0 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -344,7 +344,14 @@ static int setup_dest_addr(void) #endif gd->ram_top = gd->ram_base + get_effective_memsize(); gd->ram_top = board_get_usable_ram_top(gd->mon_len); +#ifdef SDRAM_LOWER_ADDR_MAX
if (gd->ram_top > SDRAM_LOWER_ADDR_MAX)gd->relocaddr = SDRAM_LOWER_ADDR_MAX;elsegd->relocaddr = gd->ram_top;+#else gd->relocaddr = gd->ram_top; +#endif debug("Ram top: %08lX\n", (ulong)gd->ram_top); #if defined(CONFIG_MP) && (defined(CONFIG_MPC86xx) || defined(CONFIG_E500)) /* diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h index 25d7c5cc8fff..8dd1b033017b 100644 --- a/include/configs/rk3568_common.h +++ b/include/configs/rk3568_common.h @@ -27,6 +27,11 @@ #define CONFIG_SYS_SDRAM_BASE 0 #define SDRAM_MAX_SIZE 0xf0000000
+#ifdef CONFIG_OF_SYSTEM_SETUP +#define SDRAM_LOWER_ADDR_MAX 0xf0000000 +#define SDRAM_UPPER_ADDR_MIN 0x100000000 +#endif
These config files are going away, so either use Kconfig or device tree.
Noted, I'm not sure how this could fit in a sane way to a device tree, so Kconfig is probably the better option.
#ifndef CONFIG_SPL_BUILD #define ENV_MEM_LAYOUT_SETTINGS \ "scriptaddr=0x00c00000\0" \ -- 2.25.1
This series was meant to expose the various flaws I'm working around with the rk356x series right now. This patch in particular was to expose the current issues with rockchip ram detection. It extended the already hacky method of handling this, mostly because I don't have the internal knowledge of how Rockchip ram controllers work. On top of that I really don't like that u-boot can't touch above 4G without a panic, but I definitely don't have enough knowledge of u-boot's inner workings to fix it.
Regards, Simon
Thanks for the review! Peter