[linux-sunxi] Re: [PATCH 1/2] sunxi: support asymmetric dual rank DRAM on A64/R40
On Thu, Feb 25, 2021 at 4:14 PM Icenowy Zheng wrote: > > Previously we have known that R40 has a configuration register for its > rank 1, which allows different configuration than rank 0. Reverse > engineering of newest libdram of A64 from Allwinner shows that A64 has > this register too. It's bit 0 (which enables dual rank in rank 0 > configuration register) means a dedicated rank size setup is used for > rank 1. > > Now, Pine64 scheduled to use a 3GiB LPDDR3 DRAM chip (which has 2GiB > rank 0 and 1GiB rank 1) on PinePhone, that makes asymmetric dual rank > DRAM support necessary. > > Add this support. The code could support both A64 and R40, but because > dual rank detection is broken on R40 now, we cannot really use it on R40 > currently. > > Signed-off-by: Icenowy Zheng Tested-by: Peter Robinson Tested on Pinephone Braveheart and 3Gb variants plus a Pine64 and all work as expected. > --- > .../include/asm/arch-sunxi/dram_sunxi_dw.h| 11 ++- > arch/arm/mach-sunxi/dram_sunxi_dw.c | 94 +++ > 2 files changed, 82 insertions(+), 23 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > b/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > index a5a7ebde44..e843c14202 100644 > --- a/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > +++ b/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > @@ -215,12 +215,17 @@ struct sunxi_mctl_ctl_reg { > #define NR_OF_BYTE_LANES (32 / BITS_PER_BYTE) > /* The eight data lines (DQn) plus DM, DQS and DQSN */ > #define LINES_PER_BYTE_LANE(BITS_PER_BYTE + 3) > -struct dram_para { > + > +struct rank_para { > u16 page_size; > - u8 bus_full_width; > - u8 dual_rank; > u8 row_bits; > u8 bank_bits; > +}; > + > +struct dram_para { > + u8 dual_rank; > + u8 bus_full_width; > + struct rank_para ranks[2]; > const u8 dx_read_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; > const u8 dx_write_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; > const u8 ac_delays[31]; > diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c > b/arch/arm/mach-sunxi/dram_sunxi_dw.c > index d0600011ff..2b9d631d49 100644 > --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c > +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c > @@ -399,11 +399,19 @@ static void mctl_set_cr(uint16_t socid, struct > dram_para *para) > #else > #error Unsupported DRAM type! > #endif > - (para->bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | > + (para->ranks[0].bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | >MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | >(para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | > - MCTL_CR_PAGE_SIZE(para->page_size) | > - MCTL_CR_ROW_BITS(para->row_bits), _com->cr); > + MCTL_CR_PAGE_SIZE(para->ranks[0].page_size) | > + MCTL_CR_ROW_BITS(para->ranks[0].row_bits), _com->cr); > + > + if (para->dual_rank && (socid == SOCID_A64 || socid == SOCID_R40)) { > + writel((para->ranks[1].bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | > + MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | > + MCTL_CR_DUAL_RANK | > + MCTL_CR_PAGE_SIZE(para->ranks[1].page_size) | > + MCTL_CR_ROW_BITS(para->ranks[1].row_bits), > _com->cr_r1); > + } > > if (socid == SOCID_R40) { > if (para->dual_rank) > @@ -646,35 +654,63 @@ static int mctl_channel_init(uint16_t socid, struct > dram_para *para) > return 0; > } > > -static void mctl_auto_detect_dram_size(uint16_t socid, struct dram_para > *para) > +/* > + * Test if memory at offset offset matches memory at a certain base > + */ > +static bool mctl_mem_matches_base(u32 offset, ulong base) > +{ > + /* Try to write different values to RAM at two addresses */ > + writel(0, base); > + writel(0xaa55aa55, base + offset); > + dsb(); > + /* Check if the same value is actually observed when reading back */ > + return readl(base) == > + readl(base + offset); > +} > + > +static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para > *para, ulong base, struct rank_para *rank) > { > /* detect row address bits */ > - para->page_size = 512; > - para->row_bits = 16; > - para->bank_bits = 2; > + rank->page_size = 512; > + rank->row_bits = 16; > + rank->bank_bits = 2; > mctl_set_cr(socid, para); > > - for (para->row_bits = 11; para->row_bits < 16; para->row_bits++) > - if (mctl_mem_matches((1 << (para->row_bits + > para->bank_bits)) * para->page_size)) > + for (rank->row_bits = 11; rank->row_bits < 16; rank->row_bits++) > + if (mctl_mem_matches_base((1 << (rank->row_bits + > rank->bank_bits)) * rank->page_size, base)) >
[linux-sunxi] Re: [PATCH 1/2] sunxi: support asymmetric dual rank DRAM on A64/R40
On Fri, 26 Feb 2021 00:13:24 +0800 Icenowy Zheng wrote: > Previously we have known that R40 has a configuration register for its > rank 1, which allows different configuration than rank 0. Reverse > engineering of newest libdram of A64 from Allwinner shows that A64 has > this register too. It's bit 0 (which enables dual rank in rank 0 > configuration register) means a dedicated rank size setup is used for > rank 1. > > Now, Pine64 scheduled to use a 3GiB LPDDR3 DRAM chip (which has 2GiB > rank 0 and 1GiB rank 1) on PinePhone, that makes asymmetric dual rank > DRAM support necessary. > > Add this support. The code could support both A64 and R40, but because > dual rank detection is broken on R40 now, we cannot really use it on R40 > currently. > > Signed-off-by: Icenowy Zheng So this looks alright to me. I tried to re-use the existing mctl_mem_matches() implementation for the _base() version you introduce, but interestingly this increases the code size - I guess we reach the limits of the toolchain garbage collection here. So it's some code duplication, but for the sake of keeping the SPL small, I am OK with that. I couldn't test this, but reportedly this makes quite some Pinephone people happy, so: Reviewed-by: Andre Przywara Cheers, Andre > --- > .../include/asm/arch-sunxi/dram_sunxi_dw.h| 11 ++- > arch/arm/mach-sunxi/dram_sunxi_dw.c | 94 +++ > 2 files changed, 82 insertions(+), 23 deletions(-) > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > b/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > index a5a7ebde44..e843c14202 100644 > --- a/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > +++ b/arch/arm/include/asm/arch-sunxi/dram_sunxi_dw.h > @@ -215,12 +215,17 @@ struct sunxi_mctl_ctl_reg { > #define NR_OF_BYTE_LANES (32 / BITS_PER_BYTE) > /* The eight data lines (DQn) plus DM, DQS and DQSN */ > #define LINES_PER_BYTE_LANE (BITS_PER_BYTE + 3) > -struct dram_para { > + > +struct rank_para { > u16 page_size; > - u8 bus_full_width; > - u8 dual_rank; > u8 row_bits; > u8 bank_bits; > +}; > + > +struct dram_para { > + u8 dual_rank; > + u8 bus_full_width; > + struct rank_para ranks[2]; > const u8 dx_read_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; > const u8 dx_write_delays[NR_OF_BYTE_LANES][LINES_PER_BYTE_LANE]; > const u8 ac_delays[31]; > diff --git a/arch/arm/mach-sunxi/dram_sunxi_dw.c > b/arch/arm/mach-sunxi/dram_sunxi_dw.c > index d0600011ff..2b9d631d49 100644 > --- a/arch/arm/mach-sunxi/dram_sunxi_dw.c > +++ b/arch/arm/mach-sunxi/dram_sunxi_dw.c > @@ -399,11 +399,19 @@ static void mctl_set_cr(uint16_t socid, struct > dram_para *para) > #else > #error Unsupported DRAM type! > #endif > -(para->bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | > +(para->ranks[0].bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | > MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | > (para->dual_rank ? MCTL_CR_DUAL_RANK : MCTL_CR_SINGLE_RANK) | > -MCTL_CR_PAGE_SIZE(para->page_size) | > -MCTL_CR_ROW_BITS(para->row_bits), _com->cr); > +MCTL_CR_PAGE_SIZE(para->ranks[0].page_size) | > +MCTL_CR_ROW_BITS(para->ranks[0].row_bits), _com->cr); > + > + if (para->dual_rank && (socid == SOCID_A64 || socid == SOCID_R40)) { > + writel((para->ranks[1].bank_bits == 3 ? MCTL_CR_EIGHT_BANKS : > MCTL_CR_FOUR_BANKS) | > +MCTL_CR_BUS_FULL_WIDTH(para->bus_full_width) | > +MCTL_CR_DUAL_RANK | > +MCTL_CR_PAGE_SIZE(para->ranks[1].page_size) | > +MCTL_CR_ROW_BITS(para->ranks[1].row_bits), > _com->cr_r1); > + } > > if (socid == SOCID_R40) { > if (para->dual_rank) > @@ -646,35 +654,63 @@ static int mctl_channel_init(uint16_t socid, struct > dram_para *para) > return 0; > } > > -static void mctl_auto_detect_dram_size(uint16_t socid, struct dram_para > *para) > +/* > + * Test if memory at offset offset matches memory at a certain base > + */ > +static bool mctl_mem_matches_base(u32 offset, ulong base) > +{ > + /* Try to write different values to RAM at two addresses */ > + writel(0, base); > + writel(0xaa55aa55, base + offset); > + dsb(); > + /* Check if the same value is actually observed when reading back */ > + return readl(base) == > +readl(base + offset); > +} > + > +static void mctl_auto_detect_dram_size_rank(uint16_t socid, struct dram_para > *para, ulong base, struct rank_para *rank) > { > /* detect row address bits */ > - para->page_size = 512; > - para->row_bits = 16; > - para->bank_bits = 2; > + rank->page_size = 512; > + rank->row_bits = 16; > + rank->bank_bits = 2; > mctl_set_cr(socid, para); > > - for (para->row_bits = 11; para->row_bits < 16; para->row_bits++) > -