[U-Boot] sunxi: H6: DRAM: Add support for half DQ
diff mbox series

Message ID 20190717221617.10433-1-jernej.skrabec@siol.net
State Superseded
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series
  • [U-Boot] sunxi: H6: DRAM: Add support for half DQ
Related show

Commit Message

Jernej Škrabec July 17, 2019, 10:16 p.m. UTC
Half DQ configuration seems to be very rare for H6 based boards/STBs,
but exists nevertheless. Currently the only known product which needs
this support is Tanix TX6 mini.

This commit adds support for half DQ configuration. Code was tested
for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
TX6 4 GiB/DDR3) and none were found.

Thanks to Icenowy Zheng for help with this code.

Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
---
 .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
 arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
 2 files changed, 50 insertions(+), 25 deletions(-)

Comments

Jernej Škrabec Aug. 19, 2019, 6:29 p.m. UTC | #1
+CC: Thomas

Dne četrtek, 18. julij 2019 ob 00:16:17 CEST je Jernej Skrabec napisal(a):
> Half DQ configuration seems to be very rare for H6 based boards/STBs,
> but exists nevertheless. Currently the only known product which needs
> this support is Tanix TX6 mini.
> 
> This commit adds support for half DQ configuration. Code was tested
> for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
> TX6 4 GiB/DDR3) and none were found.
> 
> Thanks to Icenowy Zheng for help with this code.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

Thomas, please test this.

Best regards,
Jernej

> ---
>  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
>  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
>  2 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h index
> 0a1da02376..49a8a66f7b 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> @@ -315,6 +315,7 @@ struct dram_para {
>  	u8 cols;
>  	u8 rows;
>  	u8 ranks;
> +	u8 bus_full_width;
>  	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
>  	const u8 dx_write_delays[NR_OF_BYTE_LANES]
[WR_LINES_PER_BYTE_LANE];
>  };
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> b/arch/arm/mach-sunxi/dram_sun50i_h6.c index 2a8275da3a..0d65327d35 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para *para)
>  	u8 rows = para->rows;
>  	u8 ranks = para->ranks;
> 
> +	if (!para->bus_full_width)
> +		cols -= 1;
> +
>  	/* Ranks */
>  	if (ranks == 2)
>  		mctl_ctl->addrmap[0] = rows + cols - 3;
> @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para *para)
>  	/* Columns */
>  	mctl_ctl->addrmap[2] = 0;
>  	switch (cols) {
> +	case 7:
> +		mctl_ctl->addrmap[3] = 0x1F1F1F00;
> +		mctl_ctl->addrmap[4] = 0x1F1F;
> +		break;
>  	case 8:
>  		mctl_ctl->addrmap[3] = 0x1F1F0000;
>  		mctl_ctl->addrmap[4] = 0x1F1F;
> @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para *para)
>  		reg_val = 0x3f00;
>  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
> 
> -	/* TODO: half DQ, DDR4 */
> -	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> -		  MSTR_ACTIVE_RANKS(para->ranks);
> +	/* TODO: DDR4 */
> +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
>  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
>  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
>  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
>  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> +	if (para->bus_full_width)
> +		reg_val |= MSTR_BUSWIDTH_FULL;
> +	else
> +		reg_val |= MSTR_BUSWIDTH_HALF;
>  	writel(reg_val | BIT(31), &mctl_ctl->mstr);
> 
>  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
>  	}
>  	writel(reg_val, &mctl_ctl->odtcfg);
> 
> -	/* TODO: half DQ */
> +	if (!para->bus_full_width) {
> +		writel(0x0, &mctl_phy->dx[2].gcr[0]);
> +		writel(0x0, &mctl_phy->dx[3].gcr[0]);
> +	}
>  }
> 
>  static void mctl_bit_delay_set(struct dram_para *para)
> @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para *para)
> 
>  	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
>  	{
> -		/*
> -		 * Detect single rank.
> -		 * TODO: also detect half DQ.
> -		 */
> +		/* Check for single rank and optionally half DQ. */
>  		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
>  			para->ranks = 1;
> +
> +			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) != 
2 ||
> +			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) != 
2)
> +				para->bus_full_width = 0;
> +
>  			/* Restart DRAM initialization from scratch. 
*/
>  			mctl_core_init(para);
>  			return;
>  		}
> -		else {
> -			panic("This DRAM setup is currently not 
supported.\n");
> +
> +		/* Check for dual rank and half DQ */
> +		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> +			para->bus_full_width = 0;
> +
> +			/* Restart DRAM initialization from scratch. 
*/
> +			mctl_core_init(para);
> +			return;
>  		}
> +
> +		panic("This DRAM setup is currently not supported.\n");
>  	}
> 
>  	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para *para)
> 
>  static void mctl_auto_detect_dram_size(struct dram_para *para)
>  {
> -	/* TODO: non-LPDDR3, half DQ */
> -	/*
> -	 * Detect rank number by the code in mctl_channel_init. Furtherly
> -	 * when DQ detection is available it will also be executed there.
> -	 */
> +	/* TODO: non-(LP)DDR3 */
> +	/* Detect rank number and half DQ by the code in mctl_channel_init. 
*/
>  	mctl_core_init(para);
> 
>  	/* detect row address bits */
> @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct dram_para
> *para) mctl_core_init(para);
> 
>  	for (para->rows = 13; para->rows < 18; para->rows++) {
> -		/* 8 banks, 8 bit per byte and 32 bit width */
> -		if (mctl_mem_matches((1 << (para->rows + para->cols + 
5))))
> +		/* 8 banks, 8 bit per byte and 16/32 bit width */
> +		if (mctl_mem_matches((1 << (para->rows + para->cols +
> +					    4 + para-
>bus_full_width))))
>  			break;
>  	}
> 
> @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct
> dram_para *para) mctl_core_init(para);
> 
>  	for (para->cols = 8; para->cols < 11; para->cols++) {
> -		/* 8 bits per byte and 32 bit width */
> -		if (mctl_mem_matches(1 << (para->cols + 2)))
> +		/* 8 bits per byte and 16/32 bit width */
> +		if (mctl_mem_matches(1 << (para->cols + 1 +
> +					   para-
>bus_full_width)))
>  			break;
>  	}
>  }
> 
>  unsigned long mctl_calc_size(struct dram_para *para)
>  {
> -	/* TODO: non-LPDDR3, half DQ */
> +	u8 width = para->bus_full_width ? 4 : 2;
> +
> +	/* TODO: non-(LP)DDR3 */
> 
> -	/* 8 banks, 32-bit (4 byte) data width */
> -	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> +	/* 8 banks */
> +	return (1ULL << (para->cols + para->rows + 3)) * width * para-
>ranks;
>  }
> 
>  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
>  		.ranks = 2,
>  		.cols = 11,
>  		.rows = 14,
> +		.bus_full_width = 1,
>  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
>  		.type = SUNXI_DRAM_TYPE_LPDDR3,
>  		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
Andre Przywara Aug. 21, 2019, 12:31 a.m. UTC | #2
On 17/07/2019 23:16, Jernej Skrabec wrote:
> Half DQ configuration seems to be very rare for H6 based boards/STBs,
> but exists nevertheless. Currently the only known product which needs
> this support is Tanix TX6 mini.
> 
> This commit adds support for half DQ configuration. Code was tested
> for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
> TX6 4 GiB/DDR3) and none were found.
> 
> Thanks to Icenowy Zheng for help with this code.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>

I don't have a board with 16-bit DRAM, but can confirm that this still
works on the PineH64 and the Eachlink H6 Mini.

Aside from the one question below, the code looks alright for me. I
verified the bits set or checked below against the Zynq register
documentation.
I didn't browse through all of the register documentation to find other
bits that possibly affect the bus width. But since the code seems to
work for Jernej, I assume that's fine.

> ---
>  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
>  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
>  2 files changed, 50 insertions(+), 25 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> index 0a1da02376..49a8a66f7b 100644
> --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> @@ -315,6 +315,7 @@ struct dram_para {
>  	u8 cols;
>  	u8 rows;
>  	u8 ranks;
> +	u8 bus_full_width;
>  	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
>  	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
>  };
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> index 2a8275da3a..0d65327d35 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para *para)
>  	u8 rows = para->rows;
>  	u8 ranks = para->ranks;
>  
> +	if (!para->bus_full_width)
> +		cols -= 1;
> +
>  	/* Ranks */
>  	if (ranks == 2)
>  		mctl_ctl->addrmap[0] = rows + cols - 3;
> @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para *para)
>  	/* Columns */
>  	mctl_ctl->addrmap[2] = 0;
>  	switch (cols) {
> +	case 7:
> +		mctl_ctl->addrmap[3] = 0x1F1F1F00;
> +		mctl_ctl->addrmap[4] = 0x1F1F;
> +		break;
>  	case 8:
>  		mctl_ctl->addrmap[3] = 0x1F1F0000;
>  		mctl_ctl->addrmap[4] = 0x1F1F;
> @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para *para)
>  		reg_val = 0x3f00;
>  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
>  
> -	/* TODO: half DQ, DDR4 */
> -	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> -		  MSTR_ACTIVE_RANKS(para->ranks);
> +	/* TODO: DDR4 */
> +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
>  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
>  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
>  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
>  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> +	if (para->bus_full_width)
> +		reg_val |= MSTR_BUSWIDTH_FULL;
> +	else
> +		reg_val |= MSTR_BUSWIDTH_HALF;
>  	writel(reg_val | BIT(31), &mctl_ctl->mstr);
>  
>  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
>  	}
>  	writel(reg_val, &mctl_ctl->odtcfg);
>  
> -	/* TODO: half DQ */
> +	if (!para->bus_full_width) {
> +		writel(0x0, &mctl_phy->dx[2].gcr[0]);
> +		writel(0x0, &mctl_phy->dx[3].gcr[0]);
> +	}
>  }
>  
>  static void mctl_bit_delay_set(struct dram_para *para)
> @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para *para)
>  
>  	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
>  	{
> -		/*
> -		 * Detect single rank.
> -		 * TODO: also detect half DQ.
> -		 */
> +		/* Check for single rank and optionally half DQ. */
>  		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> -		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
>  			para->ranks = 1;
> +
> +			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) != 2 ||
> +			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) != 2)
> +				para->bus_full_width = 0;
> +
>  			/* Restart DRAM initialization from scratch. */
>  			mctl_core_init(para);
>  			return;
>  		}
> -		else {
> -			panic("This DRAM setup is currently not supported.\n");
> +
> +		/* Check for dual rank and half DQ */
> +		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> +			para->bus_full_width = 0;

Mmh, aren't those bits always supposed to be 0 here, regardless of the
bus width? So we just confirm dual rank here? Can we actually say from
those error bits whether no errors in bits 16-31 mean them being masked
or whether that means the upper 16 bits are working fine?
Or is bus_fill_width always 1 at this point? But then we would need to
check dx[2].rsr[0] and dx[3].rsr[0], wouldn't we?

Cheers,
Andre.

> +
> +			/* Restart DRAM initialization from scratch. */
> +			mctl_core_init(para);
> +			return;
>  		}
> +
> +		panic("This DRAM setup is currently not supported.\n");
>  	}
>  
>  	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para *para)
>  
>  static void mctl_auto_detect_dram_size(struct dram_para *para)
>  {
> -	/* TODO: non-LPDDR3, half DQ */
> -	/*
> -	 * Detect rank number by the code in mctl_channel_init. Furtherly
> -	 * when DQ detection is available it will also be executed there.
> -	 */
> +	/* TODO: non-(LP)DDR3 */
> +	/* Detect rank number and half DQ by the code in mctl_channel_init. */
>  	mctl_core_init(para);
>  
>  	/* detect row address bits */
> @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct dram_para *para)
>  	mctl_core_init(para);
>  
>  	for (para->rows = 13; para->rows < 18; para->rows++) {
> -		/* 8 banks, 8 bit per byte and 32 bit width */
> -		if (mctl_mem_matches((1 << (para->rows + para->cols + 5))))
> +		/* 8 banks, 8 bit per byte and 16/32 bit width */
> +		if (mctl_mem_matches((1 << (para->rows + para->cols +
> +					    4 + para->bus_full_width))))
>  			break;
>  	}
>  
> @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct dram_para *para)
>  	mctl_core_init(para);
>  
>  	for (para->cols = 8; para->cols < 11; para->cols++) {
> -		/* 8 bits per byte and 32 bit width */
> -		if (mctl_mem_matches(1 << (para->cols + 2)))
> +		/* 8 bits per byte and 16/32 bit width */
> +		if (mctl_mem_matches(1 << (para->cols + 1 +
> +					   para->bus_full_width)))
>  			break;
>  	}
>  }
>  
>  unsigned long mctl_calc_size(struct dram_para *para)
>  {
> -	/* TODO: non-LPDDR3, half DQ */
> +	u8 width = para->bus_full_width ? 4 : 2;
> +
> +	/* TODO: non-(LP)DDR3 */
>  
> -	/* 8 banks, 32-bit (4 byte) data width */
> -	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> +	/* 8 banks */
> +	return (1ULL << (para->cols + para->rows + 3)) * width * para->ranks;
>  }
>  
>  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
>  		.ranks = 2,
>  		.cols = 11,
>  		.rows = 14,
> +		.bus_full_width = 1,
>  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
>  		.type = SUNXI_DRAM_TYPE_LPDDR3,
>  		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
>
Jernej Škrabec Aug. 21, 2019, 6:01 a.m. UTC | #3
Hi!

Dne sreda, 21. avgust 2019 ob 02:31:04 CEST je André Przywara napisal(a):
> On 17/07/2019 23:16, Jernej Skrabec wrote:
> > Half DQ configuration seems to be very rare for H6 based boards/STBs,
> > but exists nevertheless. Currently the only known product which needs
> > this support is Tanix TX6 mini.
> > 
> > This commit adds support for half DQ configuration. Code was tested
> > for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
> > TX6 4 GiB/DDR3) and none were found.
> > 
> > Thanks to Icenowy Zheng for help with this code.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> 
> I don't have a board with 16-bit DRAM, but can confirm that this still
> works on the PineH64 and the Eachlink H6 Mini.

Thanks for testing!

> 
> Aside from the one question below, the code looks alright for me. I
> verified the bits set or checked below against the Zynq register
> documentation.
> I didn't browse through all of the register documentation to find other
> bits that possibly affect the bus width. But since the code seems to
> work for Jernej, I assume that's fine.

Unfortunately, there is no RSR0 documentation in Zynq register manual, but I 
found this page:
https://git.axiom-project.eu/axiom-evi-qemu/raw/
47171dbf860af6d12c9b82997629460b77378496/hw/misc/xilinx-ddr_phy.c

All RSR0 registers are defined as having QSGERR field, but there is no 
explanation of values it can hold.

BTW, I don't have board with such memory combination, but few people confirmed 
working on Tanix TX6 mini, which it does.

> 
> > ---
> > 
> >  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
> >  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
> >  2 files changed, 50 insertions(+), 25 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h index
> > 0a1da02376..49a8a66f7b 100644
> > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > @@ -315,6 +315,7 @@ struct dram_para {
> > 
> >  	u8 cols;
> >  	u8 rows;
> >  	u8 ranks;
> > 
> > +	u8 bus_full_width;
> > 
> >  	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
> >  	const u8 dx_write_delays[NR_OF_BYTE_LANES]
[WR_LINES_PER_BYTE_LANE];
> >  
> >  };
> > 
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > b/arch/arm/mach-sunxi/dram_sun50i_h6.c index 2a8275da3a..0d65327d35
> > 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para *para)
> > 
> >  	u8 rows = para->rows;
> >  	u8 ranks = para->ranks;
> > 
> > +	if (!para->bus_full_width)
> > +		cols -= 1;
> > +
> > 
> >  	/* Ranks */
> >  	if (ranks == 2)
> >  	
> >  		mctl_ctl->addrmap[0] = rows + cols - 3;
> > 
> > @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para *para)
> > 
> >  	/* Columns */
> >  	mctl_ctl->addrmap[2] = 0;
> >  	switch (cols) {
> > 
> > +	case 7:
> > +		mctl_ctl->addrmap[3] = 0x1F1F1F00;
> > +		mctl_ctl->addrmap[4] = 0x1F1F;
> > +		break;
> > 
> >  	case 8:
> >  		mctl_ctl->addrmap[3] = 0x1F1F0000;
> >  		mctl_ctl->addrmap[4] = 0x1F1F;
> > 
> > @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para *para)
> > 
> >  		reg_val = 0x3f00;
> >  	
> >  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
> > 
> > -	/* TODO: half DQ, DDR4 */
> > -	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> > -		  MSTR_ACTIVE_RANKS(para->ranks);
> > +	/* TODO: DDR4 */
> > +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
> > 
> >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> >  	
> >  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
> >  	
> >  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
> >  	
> >  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> > 
> > +	if (para->bus_full_width)
> > +		reg_val |= MSTR_BUSWIDTH_FULL;
> > +	else
> > +		reg_val |= MSTR_BUSWIDTH_HALF;
> > 
> >  	writel(reg_val | BIT(31), &mctl_ctl->mstr);
> >  	
> >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > 
> > @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
> > 
> >  	}
> >  	writel(reg_val, &mctl_ctl->odtcfg);
> > 
> > -	/* TODO: half DQ */
> > +	if (!para->bus_full_width) {
> > +		writel(0x0, &mctl_phy->dx[2].gcr[0]);
> > +		writel(0x0, &mctl_phy->dx[3].gcr[0]);
> > +	}
> > 
> >  }
> >  
> >  static void mctl_bit_delay_set(struct dram_para *para)
> > 
> > @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para
> > *para)
> > 
> >  	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
> >  	{
> > 
> > -		/*
> > -		 * Detect single rank.
> > -		 * TODO: also detect half DQ.
> > -		 */
> > +		/* Check for single rank and optionally half DQ. */
> > 
> >  		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> > 
> > -		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> > -		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> > -		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
> > 
> >  			para->ranks = 1;
> > 
> > +
> > +			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) !
= 2 ||
> > +			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) !
= 2)
> > +				para->bus_full_width = 0;
> > +
> > 
> >  			/* Restart DRAM initialization from scratch. 
*/
> >  			mctl_core_init(para);
> >  			return;
> >  		
> >  		}
> > 
> > -		else {
> > -			panic("This DRAM setup is currently not 
supported.\n");
> > +
> > +		/* Check for dual rank and half DQ */
> > +		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> > +			para->bus_full_width = 0;
> 
> Mmh, aren't those bits always supposed to be 0 here, regardless of the
> bus width? So we just confirm dual rank here? Can we actually say from
> those error bits whether no errors in bits 16-31 mean them being masked
> or whether that means the upper 16 bits are working fine?
> Or is bus_fill_width always 1 at this point? But then we would need to
> check dx[2].rsr[0] and dx[3].rsr[0], wouldn't we?

As I already pointed out, I couldn't found useful documentation of RSR0, so I 
checked what libdram does and put similar check here too. They always check 
only two bits in all RSR0 registers. I suppose 1 and 3 can be read from 
registers or register values may not be same. But I really have no idea what 
they mean. I would be glad if anyone can provide more information.

That being said, I see what bothers you. I checked libdram logic again and 
unless I missed anything, this is exactly the check libdram has. However, 
because having dual rank and half DQ seems unlikely, we may skip this block 
altogether, but that would require another round of tests.

Best regards,
Jernej

> 
> Cheers,
> Andre.
> 
> > +
> > +			/* Restart DRAM initialization from scratch. 
*/
> > +			mctl_core_init(para);
> > +			return;
> > 
> >  		}
> > 
> > +
> > +		panic("This DRAM setup is currently not supported.\n");
> > 
> >  	}
> >  	
> >  	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> > 
> > @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para *para)
> > 
> >  static void mctl_auto_detect_dram_size(struct dram_para *para)
> >  {
> > 
> > -	/* TODO: non-LPDDR3, half DQ */
> > -	/*
> > -	 * Detect rank number by the code in mctl_channel_init. Furtherly
> > -	 * when DQ detection is available it will also be executed there.
> > -	 */
> > +	/* TODO: non-(LP)DDR3 */
> > +	/* Detect rank number and half DQ by the code in 
mctl_channel_init. */
> > 
> >  	mctl_core_init(para);
> >  	
> >  	/* detect row address bits */
> > 
> > @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct
> > dram_para *para)> 
> >  	mctl_core_init(para);
> >  	
> >  	for (para->rows = 13; para->rows < 18; para->rows++) {
> > 
> > -		/* 8 banks, 8 bit per byte and 32 bit width */
> > -		if (mctl_mem_matches((1 << (para->rows + para->cols + 
5))))
> > +		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > +		if (mctl_mem_matches((1 << (para->rows + para->cols +
> > +					    4 + para-
>bus_full_width))))
> > 
> >  			break;
> >  	
> >  	}
> > 
> > @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct
> > dram_para *para)> 
> >  	mctl_core_init(para);
> >  	
> >  	for (para->cols = 8; para->cols < 11; para->cols++) {
> > 
> > -		/* 8 bits per byte and 32 bit width */
> > -		if (mctl_mem_matches(1 << (para->cols + 2)))
> > +		/* 8 bits per byte and 16/32 bit width */
> > +		if (mctl_mem_matches(1 << (para->cols + 1 +
> > +					   para-
>bus_full_width)))
> > 
> >  			break;
> >  	
> >  	}
> >  
> >  }
> >  
> >  unsigned long mctl_calc_size(struct dram_para *para)
> >  {
> > 
> > -	/* TODO: non-LPDDR3, half DQ */
> > +	u8 width = para->bus_full_width ? 4 : 2;
> > +
> > +	/* TODO: non-(LP)DDR3 */
> > 
> > -	/* 8 banks, 32-bit (4 byte) data width */
> > -	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> > +	/* 8 banks */
> > +	return (1ULL << (para->cols + para->rows + 3)) * width * para-
>ranks;
> > 
> >  }
> >  
> >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> > 
> > @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
> > 
> >  		.ranks = 2,
> >  		.cols = 11,
> >  		.rows = 14,
> > 
> > +		.bus_full_width = 1,
> > 
> >  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
> >  
> >  		.type = SUNXI_DRAM_TYPE_LPDDR3,
> >  		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
Andre Przywara Aug. 21, 2019, 11:07 a.m. UTC | #4
On Wed, 21 Aug 2019 08:01:31 +0200
Jernej Škrabec <jernej.skrabec@siol.net> wrote:

Hi,

> Dne sreda, 21. avgust 2019 ob 02:31:04 CEST je André Przywara napisal(a):
> > On 17/07/2019 23:16, Jernej Skrabec wrote:  
> > > Half DQ configuration seems to be very rare for H6 based boards/STBs,
> > > but exists nevertheless. Currently the only known product which needs
> > > this support is Tanix TX6 mini.
> > > 
> > > This commit adds support for half DQ configuration. Code was tested
> > > for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
> > > TX6 4 GiB/DDR3) and none were found.
> > > 
> > > Thanks to Icenowy Zheng for help with this code.
> > > 
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>  
> > 
> > I don't have a board with 16-bit DRAM, but can confirm that this still
> > works on the PineH64 and the Eachlink H6 Mini.  
> 
> Thanks for testing!
> 
> > 
> > Aside from the one question below, the code looks alright for me. I
> > verified the bits set or checked below against the Zynq register
> > documentation.
> > I didn't browse through all of the register documentation to find other
> > bits that possibly affect the bus width. But since the code seems to
> > work for Jernej, I assume that's fine.  
> 
> Unfortunately, there is no RSR0 documentation in Zynq register manual, but I 

Ah, I actually mistook RSR1 for RSR0. But ...

> found this page:
> https://git.axiom-project.eu/axiom-evi-qemu/raw/
> 47171dbf860af6d12c9b82997629460b77378496/hw/misc/xilinx-ddr_phy.c
> 
> All RSR0 registers are defined as having QSGERR field, but there is no 
> explanation of values it can hold.

Yeah, it looks like all RSR registers hold some error bits for each 8-bit channel. And since RSR1 mentions to be about "one bit per rank", I would assume the same to be true for RSR0, just for some other error cause.

> BTW, I don't have board with such memory combination, but few people confirmed 
> working on Tanix TX6 mini, which it does.

Do you have an overview on which H6 board uses single rank and which dual rank? Do we actually have both varieties, for the full bus width?

> > > ---
> > > 
> > >  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
> > >  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
> > >  2 files changed, 50 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h index
> > > 0a1da02376..49a8a66f7b 100644
> > > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > @@ -315,6 +315,7 @@ struct dram_para {
> > > 
> > >  	u8 cols;
> > >  	u8 rows;
> > >  	u8 ranks;
> > > 
> > > +	u8 bus_full_width;
> > > 
> > >  	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
> > >  	const u8 dx_write_delays[NR_OF_BYTE_LANES]  
> [WR_LINES_PER_BYTE_LANE];
> > >  
> > >  };
> > > 
> > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > b/arch/arm/mach-sunxi/dram_sun50i_h6.c index 2a8275da3a..0d65327d35
> > > 100644
> > > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para *para)
> > > 
> > >  	u8 rows = para->rows;
> > >  	u8 ranks = para->ranks;
> > > 
> > > +	if (!para->bus_full_width)
> > > +		cols -= 1;
> > > +
> > > 
> > >  	/* Ranks */
> > >  	if (ranks == 2)
> > >  	
> > >  		mctl_ctl->addrmap[0] = rows + cols - 3;
> > > 
> > > @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para *para)
> > > 
> > >  	/* Columns */
> > >  	mctl_ctl->addrmap[2] = 0;
> > >  	switch (cols) {
> > > 
> > > +	case 7:
> > > +		mctl_ctl->addrmap[3] = 0x1F1F1F00;
> > > +		mctl_ctl->addrmap[4] = 0x1F1F;
> > > +		break;
> > > 
> > >  	case 8:
> > >  		mctl_ctl->addrmap[3] = 0x1F1F0000;
> > >  		mctl_ctl->addrmap[4] = 0x1F1F;
> > > 
> > > @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para *para)
> > > 
> > >  		reg_val = 0x3f00;
> > >  	
> > >  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
> > > 
> > > -	/* TODO: half DQ, DDR4 */
> > > -	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> > > -		  MSTR_ACTIVE_RANKS(para->ranks);
> > > +	/* TODO: DDR4 */
> > > +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
> > > 
> > >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > >  	
> > >  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
> > >  	
> > >  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
> > >  	
> > >  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> > > 
> > > +	if (para->bus_full_width)
> > > +		reg_val |= MSTR_BUSWIDTH_FULL;
> > > +	else
> > > +		reg_val |= MSTR_BUSWIDTH_HALF;
> > > 
> > >  	writel(reg_val | BIT(31), &mctl_ctl->mstr);
> > >  	
> > >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > > 
> > > @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
> > > 
> > >  	}
> > >  	writel(reg_val, &mctl_ctl->odtcfg);
> > > 
> > > -	/* TODO: half DQ */
> > > +	if (!para->bus_full_width) {
> > > +		writel(0x0, &mctl_phy->dx[2].gcr[0]);
> > > +		writel(0x0, &mctl_phy->dx[3].gcr[0]);
> > > +	}
> > > 
> > >  }
> > >  
> > >  static void mctl_bit_delay_set(struct dram_para *para)
> > > 
> > > @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para
> > > *para)
> > > 
> > >  	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
> > >  	{
> > > 
> > > -		/*
> > > -		 * Detect single rank.
> > > -		 * TODO: also detect half DQ.
> > > -		 */
> > > +		/* Check for single rank and optionally half DQ. */
> > > 
> > >  		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> > > 
> > > -		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> > > -		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> > > -		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> > > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
> > > 
> > >  			para->ranks = 1;
> > > 
> > > +
> > > +			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) !  
> = 2 ||
> > > +			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) !  
> = 2)
> > > +				para->bus_full_width = 0;
> > > +
> > > 
> > >  			/* Restart DRAM initialization from scratch.   
> */
> > >  			mctl_core_init(para);
> > >  			return;
> > >  		
> > >  		}
> > > 
> > > -		else {
> > > -			panic("This DRAM setup is currently not   
> supported.\n");
> > > +
> > > +		/* Check for dual rank and half DQ */
> > > +		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> > > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> > > +			para->bus_full_width = 0;  
> > 
> > Mmh, aren't those bits always supposed to be 0 here, regardless of the
> > bus width? So we just confirm dual rank here? Can we actually say from
> > those error bits whether no errors in bits 16-31 mean them being masked
> > or whether that means the upper 16 bits are working fine?
> > Or is bus_fill_width always 1 at this point? But then we would need to
> > check dx[2].rsr[0] and dx[3].rsr[0], wouldn't we?  
> 
> As I already pointed out, I couldn't found useful documentation of RSR0, so I 
> checked what libdram does and put similar check here too. They always check 
> only two bits in all RSR0 registers. I suppose 1 and 3 can be read from 
> registers or register values may not be same. But I really have no idea what 
> they mean. I would be glad if anyone can provide more information.

Looking at the other registers (particularly RSR1) I assumed that it's one error bit per rank. So if just bit 1 is set, it probably means we only have one rank, assuming bit 0 is clear. So I would guess the following:
 DX0RSR0  DX1RSR0  DX2RSR0  DX3RSR0
    00       00       00       00    dual rank, 32 bit
    10       10       10       10    single rank, 32 bit
    00       00       11       11    dual rank, 16 bit
    10       10       11       11    single rank, 16 bit
(the first line is probably filtered by the PGSR0 check above)
But maybe the zeroing of DX[23]GCR0 also masks errors in the higher
bytes? So they are always zero, and dual-rank, 32-bit is indistinguishable
from dual rank, 16 bit? And the last line is actually 10-10-00-00?

Does that make sense?

> That being said, I see what bothers you. I checked libdram logic again and 
> unless I missed anything, this is exactly the check libdram has.

While it seems wise to follow libdram, I wouldn't give too much on their code. IIRC I have seen stupid things in there, so it's probably the usual "enterprise-grade" code, where everybody assumes it's super correct and well tested, where it actually just followed some "works? ship it!" rule ;-)

> However, 
> because having dual rank and half DQ seems unlikely, we may skip this block 
> altogether, but that would require another round of tests.

Na, don't bother. You could just add a comment stating that. Given the level of documentation we should only claim to support setups we actually tested, and leave it up to newer boards to trigger fixes.

So you could just send a v2 with that comment, and add my:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre.


> Best regards,
> Jernej
> 
> > 
> > Cheers,
> > Andre.
> >   
> > > +
> > > +			/* Restart DRAM initialization from scratch.   
> */
> > > +			mctl_core_init(para);
> > > +			return;
> > > 
> > >  		}
> > > 
> > > +
> > > +		panic("This DRAM setup is currently not supported.\n");
> > > 
> > >  	}
> > >  	
> > >  	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> > > 
> > > @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para *para)
> > > 
> > >  static void mctl_auto_detect_dram_size(struct dram_para *para)
> > >  {
> > > 
> > > -	/* TODO: non-LPDDR3, half DQ */
> > > -	/*
> > > -	 * Detect rank number by the code in mctl_channel_init. Furtherly
> > > -	 * when DQ detection is available it will also be executed there.
> > > -	 */
> > > +	/* TODO: non-(LP)DDR3 */
> > > +	/* Detect rank number and half DQ by the code in   
> mctl_channel_init. */
> > > 
> > >  	mctl_core_init(para);
> > >  	
> > >  	/* detect row address bits */
> > > 
> > > @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct
> > > dram_para *para)> 
> > >  	mctl_core_init(para);
> > >  	
> > >  	for (para->rows = 13; para->rows < 18; para->rows++) {
> > > 
> > > -		/* 8 banks, 8 bit per byte and 32 bit width */
> > > -		if (mctl_mem_matches((1 << (para->rows + para->cols +   
> 5))))
> > > +		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > > +		if (mctl_mem_matches((1 << (para->rows + para->cols +
> > > +					    4 + para-  
> >bus_full_width))))  
> > > 
> > >  			break;
> > >  	
> > >  	}
> > > 
> > > @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct
> > > dram_para *para)> 
> > >  	mctl_core_init(para);
> > >  	
> > >  	for (para->cols = 8; para->cols < 11; para->cols++) {
> > > 
> > > -		/* 8 bits per byte and 32 bit width */
> > > -		if (mctl_mem_matches(1 << (para->cols + 2)))
> > > +		/* 8 bits per byte and 16/32 bit width */
> > > +		if (mctl_mem_matches(1 << (para->cols + 1 +
> > > +					   para-  
> >bus_full_width)))  
> > > 
> > >  			break;
> > >  	
> > >  	}
> > >  
> > >  }
> > >  
> > >  unsigned long mctl_calc_size(struct dram_para *para)
> > >  {
> > > 
> > > -	/* TODO: non-LPDDR3, half DQ */
> > > +	u8 width = para->bus_full_width ? 4 : 2;
> > > +
> > > +	/* TODO: non-(LP)DDR3 */
> > > 
> > > -	/* 8 banks, 32-bit (4 byte) data width */
> > > -	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> > > +	/* 8 banks */
> > > +	return (1ULL << (para->cols + para->rows + 3)) * width * para-  
> >ranks;  
> > > 
> > >  }
> > >  
> > >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
> > > 
> > > @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
> > > 
> > >  		.ranks = 2,
> > >  		.cols = 11,
> > >  		.rows = 14,
> > > 
> > > +		.bus_full_width = 1,
> > > 
> > >  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
> > >  
> > >  		.type = SUNXI_DRAM_TYPE_LPDDR3,
> > >  		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,  
> 
> 
> 
>
Jernej Škrabec Aug. 21, 2019, 9:54 p.m. UTC | #5
Dne sreda, 21. avgust 2019 ob 13:07:13 CEST je Andre Przywara napisal(a):
> On Wed, 21 Aug 2019 08:01:31 +0200
> Jernej Škrabec <jernej.skrabec@siol.net> wrote:
> 
> Hi,
> 
> > Dne sreda, 21. avgust 2019 ob 02:31:04 CEST je André Przywara napisal(a):
> > > On 17/07/2019 23:16, Jernej Skrabec wrote:
> > > > Half DQ configuration seems to be very rare for H6 based boards/STBs,
> > > > but exists nevertheless. Currently the only known product which needs
> > > > this support is Tanix TX6 mini.
> > > > 
> > > > This commit adds support for half DQ configuration. Code was tested
> > > > for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3,
> > > > Tanix
> > > > TX6 4 GiB/DDR3) and none were found.
> > > > 
> > > > Thanks to Icenowy Zheng for help with this code.
> > > > 
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > > 
> > > I don't have a board with 16-bit DRAM, but can confirm that this still
> > > works on the PineH64 and the Eachlink H6 Mini.
> > 
> > Thanks for testing!
> > 
> > > Aside from the one question below, the code looks alright for me. I
> > > verified the bits set or checked below against the Zynq register
> > > documentation.
> > > I didn't browse through all of the register documentation to find other
> > > bits that possibly affect the bus width. But since the code seems to
> > > work for Jernej, I assume that's fine.
> > 
> > Unfortunately, there is no RSR0 documentation in Zynq register manual, but
> > I
> Ah, I actually mistook RSR1 for RSR0. But ...
> 
> > found this page:
> > https://git.axiom-project.eu/axiom-evi-qemu/raw/
> > 47171dbf860af6d12c9b82997629460b77378496/hw/misc/xilinx-ddr_phy.c
> > 
> > All RSR0 registers are defined as having QSGERR field, but there is no
> > explanation of values it can hold.
> 
> Yeah, it looks like all RSR registers hold some error bits for each 8-bit
> channel. And since RSR1 mentions to be about "one bit per rank", I would
> assume the same to be true for RSR0, just for some other error cause.
> > BTW, I don't have board with such memory combination, but few people
> > confirmed working on Tanix TX6 mini, which it does.
> 
> Do you have an overview on which H6 board uses single rank and which dual
> rank? Do we actually have both varieties, for the full bus width?

I think Icenowy can help you here. But AFAIK we have both varieties for full 
bus width.

> > > > ---
> > > > 
> > > >  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
> > > >  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74
> > > >  ++++++++++++-------
> > > >  2 files changed, 50 insertions(+), 25 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h index
> > > > 0a1da02376..49a8a66f7b 100644
> > > > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > > > @@ -315,6 +315,7 @@ struct dram_para {
> > > > 
> > > >  	u8 cols;
> > > >  	u8 rows;
> > > >  	u8 ranks;
> > > > 
> > > > +	u8 bus_full_width;
> > > > 
> > > >  	const u8 dx_read_delays[NR_OF_BYTE_LANES]
[RD_LINES_PER_BYTE_LANE];
> > > >  	const u8 dx_write_delays[NR_OF_BYTE_LANES]
> > 
> > [WR_LINES_PER_BYTE_LANE];
> > 
> > > >  };
> > > > 
> > > > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > b/arch/arm/mach-sunxi/dram_sun50i_h6.c index 2a8275da3a..0d65327d35
> > > > 100644
> > > > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > > > @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para
> > > > *para)
> > > > 
> > > >  	u8 rows = para->rows;
> > > >  	u8 ranks = para->ranks;
> > > > 
> > > > +	if (!para->bus_full_width)
> > > > +		cols -= 1;
> > > > +
> > > > 
> > > >  	/* Ranks */
> > > >  	if (ranks == 2)
> > > >  	
> > > >  		mctl_ctl->addrmap[0] = rows + cols - 3;
> > > > 
> > > > @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para
> > > > *para)
> > > > 
> > > >  	/* Columns */
> > > >  	mctl_ctl->addrmap[2] = 0;
> > > >  	switch (cols) {
> > > > 
> > > > +	case 7:
> > > > +		mctl_ctl->addrmap[3] = 0x1F1F1F00;
> > > > +		mctl_ctl->addrmap[4] = 0x1F1F;
> > > > +		break;
> > > > 
> > > >  	case 8:
> > > >  		mctl_ctl->addrmap[3] = 0x1F1F0000;
> > > >  		mctl_ctl->addrmap[4] = 0x1F1F;
> > > > 
> > > > @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para
> > > > *para)
> > > > 
> > > >  		reg_val = 0x3f00;
> > > >  	
> > > >  	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
> > > > 
> > > > -	/* TODO: half DQ, DDR4 */
> > > > -	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> > > > -		  MSTR_ACTIVE_RANKS(para->ranks);
> > > > +	/* TODO: DDR4 */
> > > > +	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
> > > > 
> > > >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > > >  	
> > > >  		reg_val |= MSTR_DEVICETYPE_LPDDR3;
> > > >  	
> > > >  	if (para->type == SUNXI_DRAM_TYPE_DDR3)
> > > >  	
> > > >  		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> > > > 
> > > > +	if (para->bus_full_width)
> > > > +		reg_val |= MSTR_BUSWIDTH_FULL;
> > > > +	else
> > > > +		reg_val |= MSTR_BUSWIDTH_HALF;
> > > > 
> > > >  	writel(reg_val | BIT(31), &mctl_ctl->mstr);
> > > >  	
> > > >  	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > > > 
> > > > @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
> > > > 
> > > >  	}
> > > >  	writel(reg_val, &mctl_ctl->odtcfg);
> > > > 
> > > > -	/* TODO: half DQ */
> > > > +	if (!para->bus_full_width) {
> > > > +		writel(0x0, &mctl_phy->dx[2].gcr[0]);
> > > > +		writel(0x0, &mctl_phy->dx[3].gcr[0]);
> > > > +	}
> > > > 
> > > >  }
> > > >  
> > > >  static void mctl_bit_delay_set(struct dram_para *para)
> > > > 
> > > > @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para
> > > > *para)
> > > > 
> > > >  	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
> > > >  	{
> > > > 
> > > > -		/*
> > > > -		 * Detect single rank.
> > > > -		 * TODO: also detect half DQ.
> > > > -		 */
> > > > +		/* Check for single rank and optionally half DQ. */
> > > > 
> > > >  		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> > > > 
> > > > -		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> > > > -		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> > > > -		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> > > > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
> > > > 
> > > >  			para->ranks = 1;
> > > > 
> > > > +
> > > > +			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) 
!
> > 
> > = 2 ||
> > 
> > > > +			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) 
!
> > 
> > = 2)
> > 
> > > > +				para->bus_full_width = 0;
> > > > +
> > > > 
> > > >  			/* Restart DRAM initialization from 
scratch.
> > 
> > */
> > 
> > > >  			mctl_core_init(para);
> > > >  			return;
> > > >  		
> > > >  		}
> > > > 
> > > > -		else {
> > > > -			panic("This DRAM setup is currently not
> > 
> > supported.\n");
> > 
> > > > +
> > > > +		/* Check for dual rank and half DQ */
> > > > +		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> > > > +		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> > > > +			para->bus_full_width = 0;
> > > 
> > > Mmh, aren't those bits always supposed to be 0 here, regardless of the
> > > bus width? So we just confirm dual rank here? Can we actually say from
> > > those error bits whether no errors in bits 16-31 mean them being masked
> > > or whether that means the upper 16 bits are working fine?
> > > Or is bus_fill_width always 1 at this point? But then we would need to
> > > check dx[2].rsr[0] and dx[3].rsr[0], wouldn't we?
> > 
> > As I already pointed out, I couldn't found useful documentation of RSR0,
> > so I checked what libdram does and put similar check here too. They
> > always check only two bits in all RSR0 registers. I suppose 1 and 3 can
> > be read from registers or register values may not be same. But I really
> > have no idea what they mean. I would be glad if anyone can provide more
> > information.
> Looking at the other registers (particularly RSR1) I assumed that it's one
> error bit per rank. So if just bit 1 is set, it probably means we only have
> one rank, assuming bit 0 is clear. So I would guess the following: DX0RSR0 
> DX1RSR0  DX2RSR0  DX3RSR0
>     00       00       00       00    dual rank, 32 bit
>     10       10       10       10    single rank, 32 bit
>     00       00       11       11    dual rank, 16 bit
>     10       10       11       11    single rank, 16 bit
> (the first line is probably filtered by the PGSR0 check above)

That's actually pretty nice explanation!

> But maybe the zeroing of DX[23]GCR0 also masks errors in the higher
> bytes? So they are always zero, and dual-rank, 32-bit is indistinguishable
> from dual rank, 16 bit? And the last line is actually 10-10-00-00?

Probably, but that's why we start with dual rank, full bus width assumption, 
in order not to miss anything.

> 
> Does that make sense?

Very much.

Slightly off topic - it would be interesting to compare error codes from your 
Eachlink box to above table and check if it make sense from what we know from 
experimentation.

> 
> > That being said, I see what bothers you. I checked libdram logic again and
> > unless I missed anything, this is exactly the check libdram has.
> 
> While it seems wise to follow libdram, I wouldn't give too much on their
> code. IIRC I have seen stupid things in there, so it's probably the usual
> "enterprise-grade" code, where everybody assumes it's super correct and
> well tested, where it actually just followed some "works? ship it!" rule
> ;-)
> > However,
> > because having dual rank and half DQ seems unlikely, we may skip this
> > block
> > altogether, but that would require another round of tests.
> 
> Na, don't bother. You could just add a comment stating that. Given the level
> of documentation we should only claim to support setups we actually tested,
> and leave it up to newer boards to trigger fixes.
> 
> So you could just send a v2 with that comment, and add my:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Ok, thanks. I'll send v2 soon.

Best regards,
Jernej

> 
> Thanks,
> Andre.
> 
> > Best regards,
> > Jernej
> > 
> > > Cheers,
> > > Andre.
> > > 
> > > > +
> > > > +			/* Restart DRAM initialization from 
scratch.
> > 
> > */
> > 
> > > > +			mctl_core_init(para);
> > > > +			return;
> > > > 
> > > >  		}
> > > > 
> > > > +
> > > > +		panic("This DRAM setup is currently not supported.
\n");
> > > > 
> > > >  	}
> > > >  	
> > > >  	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> > > > 
> > > > @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para
> > > > *para)
> > > > 
> > > >  static void mctl_auto_detect_dram_size(struct dram_para *para)
> > > >  {
> > > > 
> > > > -	/* TODO: non-LPDDR3, half DQ */
> > > > -	/*
> > > > -	 * Detect rank number by the code in mctl_channel_init. Furtherly
> > > > -	 * when DQ detection is available it will also be executed there.
> > > > -	 */
> > > > +	/* TODO: non-(LP)DDR3 */
> > > > +	/* Detect rank number and half DQ by the code in
> > 
> > mctl_channel_init. */
> > 
> > > >  	mctl_core_init(para);
> > > >  	
> > > >  	/* detect row address bits */
> > > > 
> > > > @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct
> > > > dram_para *para)>
> > > > 
> > > >  	mctl_core_init(para);
> > > >  	
> > > >  	for (para->rows = 13; para->rows < 18; para->rows++) {
> > > > 
> > > > -		/* 8 banks, 8 bit per byte and 32 bit width */
> > > > -		if (mctl_mem_matches((1 << (para->rows + para->cols +
> > 
> > 5))))
> > 
> > > > +		/* 8 banks, 8 bit per byte and 16/32 bit width */
> > > > +		if (mctl_mem_matches((1 << (para->rows + para->cols +
> > > > +					    4 + para-
> > >
> > >bus_full_width))))
> > >
> > > >  			break;
> > > >  	
> > > >  	}
> > > > 
> > > > @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct
> > > > dram_para *para)>
> > > > 
> > > >  	mctl_core_init(para);
> > > >  	
> > > >  	for (para->cols = 8; para->cols < 11; para->cols++) {
> > > > 
> > > > -		/* 8 bits per byte and 32 bit width */
> > > > -		if (mctl_mem_matches(1 << (para->cols + 2)))
> > > > +		/* 8 bits per byte and 16/32 bit width */
> > > > +		if (mctl_mem_matches(1 << (para->cols + 1 +
> > > > +					   para-
> > >
> > >bus_full_width)))
> > >
> > > >  			break;
> > > >  	
> > > >  	}
> > > >  
> > > >  }
> > > >  
> > > >  unsigned long mctl_calc_size(struct dram_para *para)
> > > >  {
> > > > 
> > > > -	/* TODO: non-LPDDR3, half DQ */
> > > > +	u8 width = para->bus_full_width ? 4 : 2;
> > > > +
> > > > +	/* TODO: non-(LP)DDR3 */
> > > > 
> > > > -	/* 8 banks, 32-bit (4 byte) data width */
> > > > -	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> > > > +	/* 8 banks */
> > > > +	return (1ULL << (para->cols + para->rows + 3)) * width * para-
> > >
> > >ranks;
> > >
> > > >  }
> > > >  
> > > >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			
\
> > > > 
> > > > @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
> > > > 
> > > >  		.ranks = 2,
> > > >  		.cols = 11,
> > > >  		.rows = 14,
> > > > 
> > > > +		.bus_full_width = 1,
> > > > 
> > > >  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
> > > >  
> > > >  		.type = SUNXI_DRAM_TYPE_LPDDR3,
> > > >  		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
Thomas Graichen Aug. 23, 2019, 4:02 p.m. UTC | #6
hi jernej,

On Mon, Aug 19, 2019 at 8:29 PM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> +CC: Thomas
>
> Dne četrtek, 18. julij 2019 ob 00:16:17 CEST je Jernej Skrabec napisal(a):
> > Half DQ configuration seems to be very rare for H6 based boards/STBs,
> > but exists nevertheless. Currently the only known product which needs
> > this support is Tanix TX6 mini.
> >
> > This commit adds support for half DQ configuration. Code was tested
> > for regressions on other configurations (OrangePi 3 1 GiB/LPDDR3, Tanix
> > TX6 4 GiB/DDR3) and none were found.
> >
> > Thanks to Icenowy Zheng for help with this code.
> >
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
>
> Thomas, please test this.

Tested-by: thomas graichen <thomas.graichen@gmail.com>

BEFORE (no patches applied or other earlier h6 patches applied):
the tanix tx6 mini tv box did not boot

AFTER (this patch applied):
the tanix tx6 mini tv box is booting fine now. also cross tested to
boot fine on a non-half-dq eachlink h6 mini tv box as well.

best wishes - thomas

> Best regards,
> Jernej
>
> > ---
> >  .../include/asm/arch-sunxi/dram_sun50i_h6.h   |  1 +
> >  arch/arm/mach-sunxi/dram_sun50i_h6.c          | 74 ++++++++++++-------
> >  2 files changed, 50 insertions(+), 25 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h index
> > 0a1da02376..49a8a66f7b 100644
> > --- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > +++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
> > @@ -315,6 +315,7 @@ struct dram_para {
> >       u8 cols;
> >       u8 rows;
> >       u8 ranks;
> > +     u8 bus_full_width;
> >       const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
> >       const u8 dx_write_delays[NR_OF_BYTE_LANES]
> [WR_LINES_PER_BYTE_LANE];
> >  };
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > b/arch/arm/mach-sunxi/dram_sun50i_h6.c index 2a8275da3a..0d65327d35 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > @@ -201,6 +201,9 @@ static void mctl_set_addrmap(struct dram_para *para)
> >       u8 rows = para->rows;
> >       u8 ranks = para->ranks;
> >
> > +     if (!para->bus_full_width)
> > +             cols -= 1;
> > +
> >       /* Ranks */
> >       if (ranks == 2)
> >               mctl_ctl->addrmap[0] = rows + cols - 3;
> > @@ -213,6 +216,10 @@ static void mctl_set_addrmap(struct dram_para *para)
> >       /* Columns */
> >       mctl_ctl->addrmap[2] = 0;
> >       switch (cols) {
> > +     case 7:
> > +             mctl_ctl->addrmap[3] = 0x1F1F1F00;
> > +             mctl_ctl->addrmap[4] = 0x1F1F;
> > +             break;
> >       case 8:
> >               mctl_ctl->addrmap[3] = 0x1F1F0000;
> >               mctl_ctl->addrmap[4] = 0x1F1F;
> > @@ -300,13 +307,16 @@ static void mctl_com_init(struct dram_para *para)
> >               reg_val = 0x3f00;
> >       clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
> >
> > -     /* TODO: half DQ, DDR4 */
> > -     reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
> > -               MSTR_ACTIVE_RANKS(para->ranks);
> > +     /* TODO: DDR4 */
> > +     reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
> >       if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> >               reg_val |= MSTR_DEVICETYPE_LPDDR3;
> >       if (para->type == SUNXI_DRAM_TYPE_DDR3)
> >               reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
> > +     if (para->bus_full_width)
> > +             reg_val |= MSTR_BUSWIDTH_FULL;
> > +     else
> > +             reg_val |= MSTR_BUSWIDTH_HALF;
> >       writel(reg_val | BIT(31), &mctl_ctl->mstr);
> >
> >       if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
> > @@ -333,7 +343,10 @@ static void mctl_com_init(struct dram_para *para)
> >       }
> >       writel(reg_val, &mctl_ctl->odtcfg);
> >
> > -     /* TODO: half DQ */
> > +     if (!para->bus_full_width) {
> > +             writel(0x0, &mctl_phy->dx[2].gcr[0]);
> > +             writel(0x0, &mctl_phy->dx[3].gcr[0]);
> > +     }
> >  }
> >
> >  static void mctl_bit_delay_set(struct dram_para *para)
> > @@ -514,22 +527,31 @@ static void mctl_channel_init(struct dram_para *para)
> >
> >       if (readl(&mctl_phy->pgsr[0]) & 0x400000)
> >       {
> > -             /*
> > -              * Detect single rank.
> > -              * TODO: also detect half DQ.
> > -              */
> > +             /* Check for single rank and optionally half DQ. */
> >               if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
> > -                 (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
> > -                 (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
> > -                 (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
> > +                 (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
> >                       para->ranks = 1;
> > +
> > +                     if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) !=
> 2 ||
> > +                         (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) !=
> 2)
> > +                             para->bus_full_width = 0;
> > +
> >                       /* Restart DRAM initialization from scratch.
> */
> >                       mctl_core_init(para);
> >                       return;
> >               }
> > -             else {
> > -                     panic("This DRAM setup is currently not
> supported.\n");
> > +
> > +             /* Check for dual rank and half DQ */
> > +             if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
> > +                 (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
> > +                     para->bus_full_width = 0;
> > +
> > +                     /* Restart DRAM initialization from scratch.
> */
> > +                     mctl_core_init(para);
> > +                     return;
> >               }
> > +
> > +             panic("This DRAM setup is currently not supported.\n");
> >       }
> >
> >       if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
> > @@ -557,11 +579,8 @@ static void mctl_channel_init(struct dram_para *para)
> >
> >  static void mctl_auto_detect_dram_size(struct dram_para *para)
> >  {
> > -     /* TODO: non-LPDDR3, half DQ */
> > -     /*
> > -      * Detect rank number by the code in mctl_channel_init. Furtherly
> > -      * when DQ detection is available it will also be executed there.
> > -      */
> > +     /* TODO: non-(LP)DDR3 */
> > +     /* Detect rank number and half DQ by the code in mctl_channel_init.
> */
> >       mctl_core_init(para);
> >
> >       /* detect row address bits */
> > @@ -570,8 +589,9 @@ static void mctl_auto_detect_dram_size(struct dram_para
> > *para) mctl_core_init(para);
> >
> >       for (para->rows = 13; para->rows < 18; para->rows++) {
> > -             /* 8 banks, 8 bit per byte and 32 bit width */
> > -             if (mctl_mem_matches((1 << (para->rows + para->cols +
> 5))))
> > +             /* 8 banks, 8 bit per byte and 16/32 bit width */
> > +             if (mctl_mem_matches((1 << (para->rows + para->cols +
> > +                                         4 + para-
> >bus_full_width))))
> >                       break;
> >       }
> >
> > @@ -580,18 +600,21 @@ static void mctl_auto_detect_dram_size(struct
> > dram_para *para) mctl_core_init(para);
> >
> >       for (para->cols = 8; para->cols < 11; para->cols++) {
> > -             /* 8 bits per byte and 32 bit width */
> > -             if (mctl_mem_matches(1 << (para->cols + 2)))
> > +             /* 8 bits per byte and 16/32 bit width */
> > +             if (mctl_mem_matches(1 << (para->cols + 1 +
> > +                                        para-
> >bus_full_width)))
> >                       break;
> >       }
> >  }
> >
> >  unsigned long mctl_calc_size(struct dram_para *para)
> >  {
> > -     /* TODO: non-LPDDR3, half DQ */
> > +     u8 width = para->bus_full_width ? 4 : 2;
> > +
> > +     /* TODO: non-(LP)DDR3 */
> >
> > -     /* 8 banks, 32-bit (4 byte) data width */
> > -     return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
> > +     /* 8 banks */
> > +     return (1ULL << (para->cols + para->rows + 3)) * width * para-
> >ranks;
> >  }
> >
> >  #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS                     \
> > @@ -625,6 +648,7 @@ unsigned long sunxi_dram_init(void)
> >               .ranks = 2,
> >               .cols = 11,
> >               .rows = 14,
> > +             .bus_full_width = 1,
> >  #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
> >               .type = SUNXI_DRAM_TYPE_LPDDR3,
> >               .dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,
>
>
>
>

Patch
diff mbox series

diff --git a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
index 0a1da02376..49a8a66f7b 100644
--- a/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
+++ b/arch/arm/include/asm/arch-sunxi/dram_sun50i_h6.h
@@ -315,6 +315,7 @@  struct dram_para {
 	u8 cols;
 	u8 rows;
 	u8 ranks;
+	u8 bus_full_width;
 	const u8 dx_read_delays[NR_OF_BYTE_LANES][RD_LINES_PER_BYTE_LANE];
 	const u8 dx_write_delays[NR_OF_BYTE_LANES][WR_LINES_PER_BYTE_LANE];
 };
diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/dram_sun50i_h6.c
index 2a8275da3a..0d65327d35 100644
--- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
+++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
@@ -201,6 +201,9 @@  static void mctl_set_addrmap(struct dram_para *para)
 	u8 rows = para->rows;
 	u8 ranks = para->ranks;
 
+	if (!para->bus_full_width)
+		cols -= 1;
+
 	/* Ranks */
 	if (ranks == 2)
 		mctl_ctl->addrmap[0] = rows + cols - 3;
@@ -213,6 +216,10 @@  static void mctl_set_addrmap(struct dram_para *para)
 	/* Columns */
 	mctl_ctl->addrmap[2] = 0;
 	switch (cols) {
+	case 7:
+		mctl_ctl->addrmap[3] = 0x1F1F1F00;
+		mctl_ctl->addrmap[4] = 0x1F1F;
+		break;
 	case 8:
 		mctl_ctl->addrmap[3] = 0x1F1F0000;
 		mctl_ctl->addrmap[4] = 0x1F1F;
@@ -300,13 +307,16 @@  static void mctl_com_init(struct dram_para *para)
 		reg_val = 0x3f00;
 	clrsetbits_le32(&mctl_com->unk_0x008, 0x3f00, reg_val);
 
-	/* TODO: half DQ, DDR4 */
-	reg_val = MSTR_BUSWIDTH_FULL | MSTR_BURST_LENGTH(8) |
-		  MSTR_ACTIVE_RANKS(para->ranks);
+	/* TODO: DDR4 */
+	reg_val = MSTR_BURST_LENGTH(8) | MSTR_ACTIVE_RANKS(para->ranks);
 	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
 		reg_val |= MSTR_DEVICETYPE_LPDDR3;
 	if (para->type == SUNXI_DRAM_TYPE_DDR3)
 		reg_val |= MSTR_DEVICETYPE_DDR3 | MSTR_2TMODE;
+	if (para->bus_full_width)
+		reg_val |= MSTR_BUSWIDTH_FULL;
+	else
+		reg_val |= MSTR_BUSWIDTH_HALF;
 	writel(reg_val | BIT(31), &mctl_ctl->mstr);
 
 	if (para->type == SUNXI_DRAM_TYPE_LPDDR3)
@@ -333,7 +343,10 @@  static void mctl_com_init(struct dram_para *para)
 	}
 	writel(reg_val, &mctl_ctl->odtcfg);
 
-	/* TODO: half DQ */
+	if (!para->bus_full_width) {
+		writel(0x0, &mctl_phy->dx[2].gcr[0]);
+		writel(0x0, &mctl_phy->dx[3].gcr[0]);
+	}
 }
 
 static void mctl_bit_delay_set(struct dram_para *para)
@@ -514,22 +527,31 @@  static void mctl_channel_init(struct dram_para *para)
 
 	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
 	{
-		/*
-		 * Detect single rank.
-		 * TODO: also detect half DQ.
-		 */
+		/* Check for single rank and optionally half DQ. */
 		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 2 &&
-		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2 &&
-		    (readl(&mctl_phy->dx[2].rsr[0]) & 0x3) == 2 &&
-		    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) == 2) {
+		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 2) {
 			para->ranks = 1;
+
+			if ((readl(&mctl_phy->dx[2].rsr[0]) & 0x3) != 2 ||
+			    (readl(&mctl_phy->dx[3].rsr[0]) & 0x3) != 2)
+				para->bus_full_width = 0;
+
 			/* Restart DRAM initialization from scratch. */
 			mctl_core_init(para);
 			return;
 		}
-		else {
-			panic("This DRAM setup is currently not supported.\n");
+
+		/* Check for dual rank and half DQ */
+		if ((readl(&mctl_phy->dx[0].rsr[0]) & 0x3) == 0 &&
+		    (readl(&mctl_phy->dx[1].rsr[0]) & 0x3) == 0) {
+			para->bus_full_width = 0;
+
+			/* Restart DRAM initialization from scratch. */
+			mctl_core_init(para);
+			return;
 		}
+
+		panic("This DRAM setup is currently not supported.\n");
 	}
 
 	if (readl(&mctl_phy->pgsr[0]) & 0xff00000) {
@@ -557,11 +579,8 @@  static void mctl_channel_init(struct dram_para *para)
 
 static void mctl_auto_detect_dram_size(struct dram_para *para)
 {
-	/* TODO: non-LPDDR3, half DQ */
-	/*
-	 * Detect rank number by the code in mctl_channel_init. Furtherly
-	 * when DQ detection is available it will also be executed there.
-	 */
+	/* TODO: non-(LP)DDR3 */
+	/* Detect rank number and half DQ by the code in mctl_channel_init. */
 	mctl_core_init(para);
 
 	/* detect row address bits */
@@ -570,8 +589,9 @@  static void mctl_auto_detect_dram_size(struct dram_para *para)
 	mctl_core_init(para);
 
 	for (para->rows = 13; para->rows < 18; para->rows++) {
-		/* 8 banks, 8 bit per byte and 32 bit width */
-		if (mctl_mem_matches((1 << (para->rows + para->cols + 5))))
+		/* 8 banks, 8 bit per byte and 16/32 bit width */
+		if (mctl_mem_matches((1 << (para->rows + para->cols +
+					    4 + para->bus_full_width))))
 			break;
 	}
 
@@ -580,18 +600,21 @@  static void mctl_auto_detect_dram_size(struct dram_para *para)
 	mctl_core_init(para);
 
 	for (para->cols = 8; para->cols < 11; para->cols++) {
-		/* 8 bits per byte and 32 bit width */
-		if (mctl_mem_matches(1 << (para->cols + 2)))
+		/* 8 bits per byte and 16/32 bit width */
+		if (mctl_mem_matches(1 << (para->cols + 1 +
+					   para->bus_full_width)))
 			break;
 	}
 }
 
 unsigned long mctl_calc_size(struct dram_para *para)
 {
-	/* TODO: non-LPDDR3, half DQ */
+	u8 width = para->bus_full_width ? 4 : 2;
+
+	/* TODO: non-(LP)DDR3 */
 
-	/* 8 banks, 32-bit (4 byte) data width */
-	return (1ULL << (para->cols + para->rows + 3)) * 4 * para->ranks;
+	/* 8 banks */
+	return (1ULL << (para->cols + para->rows + 3)) * width * para->ranks;
 }
 
 #define SUN50I_H6_LPDDR3_DX_WRITE_DELAYS			\
@@ -625,6 +648,7 @@  unsigned long sunxi_dram_init(void)
 		.ranks = 2,
 		.cols = 11,
 		.rows = 14,
+		.bus_full_width = 1,
 #ifdef CONFIG_SUNXI_DRAM_H6_LPDDR3
 		.type = SUNXI_DRAM_TYPE_LPDDR3,
 		.dx_read_delays  = SUN50I_H6_LPDDR3_DX_READ_DELAYS,