diff mbox series

[U-Boot,03/10] ddr: altera: s10: Add multiple memory banks support

Message ID 1552379474-12867-4-git-send-email-ley.foon.tan@intel.com
State Superseded
Delegated to: Marek Vasut
Headers show
Series Update Stratix 10 SDRAM driver | expand

Commit Message

Ley Foon Tan March 12, 2019, 8:31 a.m. UTC
Setup bank start address and size based on total SDRAM
memory size calculated from hardware. Update sdram_size_check()
to support multiple banks.

Stratix10 supports up to 2 memory banks.

Bank 0: Address 0, size 2GB
Bank 1: Address 0x100000000, size 124GB

Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 .../arm/mach-socfpga/include/mach/sdram_s10.h |  1 +
 drivers/ddr/altera/sdram_s10.c                | 93 ++++++++++++++++++-
 2 files changed, 91 insertions(+), 3 deletions(-)

Comments

Marek Vasut March 12, 2019, 10:44 a.m. UTC | #1
On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> Setup bank start address and size based on total SDRAM
> memory size calculated from hardware. Update sdram_size_check()
> to support multiple banks.
> 
> Stratix10 supports up to 2 memory banks.
> 
> Bank 0: Address 0, size 2GB
> Bank 1: Address 0x100000000, size 124GB
> 
> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>

Shouldn't Quartus generate some sort of DT which gives you all the DRAM
layout information ? Then you won't need any of this
CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such info
from DT and apply the sanity check only on DRAM you know exists.
Westergreen, Dalon March 12, 2019, 2:05 p.m. UTC | #2
On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > Setup bank start address and size based on total SDRAM
> > memory size calculated from hardware. Update sdram_size_check()
> > to support multiple banks.
> > 
> > Stratix10 supports up to 2 memory banks.
> > 
> > Bank 0: Address 0, size 2GB
> > Bank 1: Address 0x100000000, size 124GB
> > 
> > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> 
> Shouldn't Quartus generate some sort of DT which gives you all the DRAM
> layout information ? Then you won't need any of this
> CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such info
> from DT and apply the sanity check only on DRAM you know exists.
> 
Yes, I think this can be much cleaner and the dts should provide the bank
information rather than creating the banks from the determined dram size.

The determined dram size can just be a check to validate the bank configuration
in the dts.

so

diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
b/arch/arm/dts/socfpga_stratix10_socdk.dts
index 6e8ddcd9f4..94c71bb0ed 100644
--- a/arch/arm/dts/socfpga_stratix10_socdk.dts
+++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
@@ -36,7 +36,9 @@

        memory {
                device_type = "memory";
-               reg = <0 0 0 0x80000000>; /* 2GB */
+               /* 4GB */
+               reg = < 0 0x00000000 0 0x80000000
+                       1 0x80000000 0 0x80000000>;
                u-boot,dm-pre-reloc;
        };
 };

--dalon
Ley Foon Tan March 13, 2019, 7:28 a.m. UTC | #3
On Tue, 2019-03-12 at 14:05 +0000, Westergreen, Dalon wrote:
> On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
> > 
> > On 3/12/19 9:31 AM, Ley Foon Tan wrote:
> > > 
> > > Setup bank start address and size based on total SDRAM
> > > memory size calculated from hardware. Update sdram_size_check()
> > > to support multiple banks.
> > > 
> > > Stratix10 supports up to 2 memory banks.
> > > 
> > > Bank 0: Address 0, size 2GB
> > > Bank 1: Address 0x100000000, size 124GB
> > > 
> > > Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
> > > Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> > Shouldn't Quartus generate some sort of DT which gives you all the
> > DRAM
> > layout information ? Then you won't need any of this
> > CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such
> > info
> > from DT and apply the sanity check only on DRAM you know exists.
> > 
> Yes, I think this can be much cleaner and the dts should provide the
> bank
> information rather than creating the banks from the determined dram
> size.
> 
> The determined dram size can just be a check to validate the bank
> configuration
> in the dts.

Okay.Then user needs update memory node in dts for different SDRAM
size.

Regards
Ley Foon
> 
> so
> 
> diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
> b/arch/arm/dts/socfpga_stratix10_socdk.dts
> index 6e8ddcd9f4..94c71bb0ed 100644
> --- a/arch/arm/dts/socfpga_stratix10_socdk.dts
> +++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
> @@ -36,7 +36,9 @@
> 
>         memory {
>                 device_type = "memory";
> -               reg = <0 0 0 0x80000000>; /* 2GB */
> +               /* 4GB */
> +               reg = < 0 0x00000000 0 0x80000000
> +                       1 0x80000000 0 0x80000000>;
>                 u-boot,dm-pre-reloc;
>         };
>  };
> 
> --dalon
> 
> 
>
Marek Vasut March 13, 2019, 9:14 a.m. UTC | #4
On 3/13/19 8:28 AM, Ley Foon Tan wrote:
> On Tue, 2019-03-12 at 14:05 +0000, Westergreen, Dalon wrote:
>> On Tue, 2019-03-12 at 11:44 +0100, Marek Vasut wrote:
>>>
>>> On 3/12/19 9:31 AM, Ley Foon Tan wrote:
>>>>
>>>> Setup bank start address and size based on total SDRAM
>>>> memory size calculated from hardware. Update sdram_size_check()
>>>> to support multiple banks.
>>>>
>>>> Stratix10 supports up to 2 memory banks.
>>>>
>>>> Bank 0: Address 0, size 2GB
>>>> Bank 1: Address 0x100000000, size 124GB
>>>>
>>>> Signed-off-by: Dalon Westergreen <dalon.westergreen@intel.com>
>>>> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
>>> Shouldn't Quartus generate some sort of DT which gives you all the
>>> DRAM
>>> layout information ? Then you won't need any of this
>>> CONFIG_NR_DRAM_BANKS stuff, but you would be able to extract such
>>> info
>>> from DT and apply the sanity check only on DRAM you know exists.
>>>
>> Yes, I think this can be much cleaner and the dts should provide the
>> bank
>> information rather than creating the banks from the determined dram
>> size.
>>
>> The determined dram size can just be a check to validate the bank
>> configuration
>> in the dts.
> 
> Okay.Then user needs update memory node in dts for different SDRAM
> size.

The DT should describe hardware correctly, so yes.

> Regards
> Ley Foon
>>
>> so
>>
>> diff --git a/arch/arm/dts/socfpga_stratix10_socdk.dts
>> b/arch/arm/dts/socfpga_stratix10_socdk.dts
>> index 6e8ddcd9f4..94c71bb0ed 100644
>> --- a/arch/arm/dts/socfpga_stratix10_socdk.dts
>> +++ b/arch/arm/dts/socfpga_stratix10_socdk.dts
>> @@ -36,7 +36,9 @@
>>
>>         memory {
>>                 device_type = "memory";
>> -               reg = <0 0 0 0x80000000>; /* 2GB */
>> +               /* 4GB */
>> +               reg = < 0 0x00000000 0 0x80000000
>> +                       1 0x80000000 0 0x80000000>;
>>                 u-boot,dm-pre-reloc;
>>         };
>>  };
>>
>> --dalon
>>
>>
>>
diff mbox series

Patch

diff --git a/arch/arm/mach-socfpga/include/mach/sdram_s10.h b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
index ca68594445..89e355010d 100644
--- a/arch/arm/mach-socfpga/include/mach/sdram_s10.h
+++ b/arch/arm/mach-socfpga/include/mach/sdram_s10.h
@@ -10,6 +10,7 @@ 
 phys_size_t sdram_calculate_size(void);
 int sdram_mmr_init_full(unsigned int sdr_phy_reg);
 int sdram_calibration_full(void);
+void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[]);
 
 #define DDR_TWR				15
 #define DDR_READ_LATENCY_DELAY		40
diff --git a/drivers/ddr/altera/sdram_s10.c b/drivers/ddr/altera/sdram_s10.c
index 8895813440..ae4e5ea2fd 100644
--- a/drivers/ddr/altera/sdram_s10.c
+++ b/drivers/ddr/altera/sdram_s10.c
@@ -13,6 +13,7 @@ 
 #include <asm/arch/sdram_s10.h>
 #include <asm/arch/system_manager.h>
 #include <asm/arch/reset_manager.h>
+#include <linux/sizes.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -134,17 +135,100 @@  static int poll_hmc_clock_status(void)
 				 SYSMGR_HMC_CLK_STATUS_MSK, true, 1000, false);
 }
 
-static void sdram_size_check(void)
+static void sdram_size_check(phys_addr_t bank_start[], phys_size_t bank_size[])
 {
+	phys_size_t total_ram_check = 0;
+	phys_size_t ram_check = 0;
+	int bank;
+
 	/* Sanity check ensure correct SDRAM size specified */
 	debug("DDR: Running SDRAM size sanity check\n");
-	if (get_ram_size(0, gd->ram_size) != gd->ram_size) {
+
+	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
+		while (ram_check < bank_size[bank]) {
+			ram_check += get_ram_size((void *)(bank_start[bank]
+							   + ram_check),
+							   (phys_size_t)SZ_1G);
+		}
+		total_ram_check += ram_check;
+		ram_check = 0;
+	}
+
+	if (total_ram_check != gd->ram_size) {
 		puts("DDR: SDRAM size check failed!\n");
 		hang();
 	}
+
 	debug("DDR: SDRAM size check passed!\n");
 }
 
+void setup_memory_banks(phys_addr_t bank_addr[], phys_size_t bank_size[])
+{
+	phys_size_t ram_alias;
+
+	bank_addr[0] = CONFIG_SYS_SDRAM_BASE;
+	bank_size[0] = min((phys_size_t)SZ_2G, (phys_size_t)gd->ram_size);
+
+	if (CONFIG_NR_DRAM_BANKS > 1) {
+		if (gd->ram_size > ((phys_size_t)SZ_2G * (phys_size_t)32)) {
+			/* With memories > 64GB it is not possible to
+			 * access the entire memory. You cannot avoid
+			 * the 2GB IO hole from 2GB to 4GB in the HPS
+			 * memory space. The dram banks must be configured
+			 * as :
+			 * dram[0].start = 0x0
+			 * dram[0].size = SZ_2GB
+			 * dram[1].start = SZ_2GB << 1 (or 4GB)
+			 * dram[1].size = gd->ram_size - SZ_2G
+			 */
+			bank_addr[1] = (phys_addr_t)SZ_2G +
+				(phys_addr_t)SZ_2G;
+			bank_size[1] = gd->ram_size - SZ_2G;
+
+			/* reduce ram size as useable space is smaller.
+			 * We cannot avoid the IO hole.
+			 */
+			gd->ram_size -= SZ_2G;
+
+		} else if (gd->ram_size > SZ_2G) {
+			/* We can use that the DRAM address space
+			 * is repeatedly aliased in the overall
+			 * 128GB of address space as the SDRAM NOC
+			 * ignores unused upper address bits.
+			 *
+			 * This means, a 4GB DDR is replicated every
+			 * 4GB in the address, and a 16GB DDR every
+			 * 16GB.
+			 *
+			 * So, we can access the entire memory, for
+			 * the 4GB case:
+			 * dram[0].start = 0x0
+			 * dram[0].size = SZ_2GB
+			 * dram[1].start = SZ_2GB << 1 + SZ_2GB (or 6GB)
+			 * dram[1].size = gd->ram_size - SZ_2G
+			 *
+			 * more interestingly, the 3GB case would be the same.
+			 */
+
+			/* Now we need to find the appropriate alias
+			 * location.
+			 */
+			ram_alias = SZ_2G;
+			while (ram_alias < gd->ram_size)
+				ram_alias = ram_alias << 1;
+
+			bank_addr[1] = ram_alias + SZ_2G;
+			bank_size[1] = gd->ram_size - SZ_2G;
+		} else {
+			/* Here the map is simple as all memory fits in the
+			 * lower 2GB window.
+			 */
+			bank_addr[1] = 0;
+			bank_size[1] = 0;
+		}
+	}
+}
+
 /**
  * sdram_mmr_init_full() - Function to initialize SDRAM MMR
  *
@@ -155,6 +239,8 @@  int sdram_mmr_init_full(unsigned int unused)
 	u32 update_value, io48_value, ddrioctl;
 	u32 i;
 	int ret;
+	phys_addr_t bank_start[CONFIG_NR_DRAM_BANKS];
+	phys_size_t bank_size[CONFIG_NR_DRAM_BANKS];
 
 	/* Enable access to DDR from CPU master */
 	clrbits_le32(CCU_REG_ADDR(CCU_CPU0_MPRT_ADBASE_DDRREG),
@@ -351,6 +437,7 @@  int sdram_mmr_init_full(unsigned int unused)
 		gd->ram_size = size;
 
 	printf("DDR: %d MiB\n", (int)(gd->ram_size >> 20));
+	setup_memory_banks(bank_start, bank_size);
 
 	/* Enable or disable the SDRAM ECC */
 	if (CTRLCFG1_CFG_CTRL_EN_ECC(ctrlcfg1)) {
@@ -374,7 +461,7 @@  int sdram_mmr_init_full(unsigned int unused)
 			      DDR_HMC_ECCCTL2_AWB_EN_SET_MSK));
 	}
 
-	sdram_size_check();
+	sdram_size_check(bank_start, bank_size);
 
 	debug("DDR: HMC init success\n");
 	return 0;