[PATCH] sunxi: dram: h6: Improve DDR3 config detection

["Jernej Škrabec" <jernej.skrabec@siol.net>]

Dne petek, 08. januar 2021 ob 03:01:42 CET je Andr? Przywara napisal(a):
> On 03/12/2020 17:46, Jernej Skrabec wrote:
> > It turns out that in rare cases, current analytical approach to detect
> > correct DRAM bus width and rank on H6 doesn't work. On some TV boxes
> > with DDR3, incorrect DRAM configuration triggers write leveling error
> > which immediately stops initialization process. Exact reason why this
> > error appears isn't known. However, if correct configuration is used,
> > initalization works without problem.
> > 
> > In order to fix this issue, simply try another configuration when any
> > kind of error appears during initialization, not just those related to
> > rank and bus width.
> 
> It's a bummer that this auto detection doesn't work, it looked to be the
> right thing.
> But I prefer functionality over pipe dreams ;-)
> 
> > 
> > Tested-by: Thomas Graichen <thomas.graichen@googlemail.com>
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@siol.net>
> > ---
> >  arch/arm/mach-sunxi/dram_sun50i_h6.c | 95 +++++++++++++++-------------
> >  1 file changed, 51 insertions(+), 44 deletions(-)
> > 
> > diff --git a/arch/arm/mach-sunxi/dram_sun50i_h6.c b/arch/arm/mach-sunxi/
dram_sun50i_h6.c
> > index 9e34da474798..1cde6132be2c 100644
> > --- a/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > +++ b/arch/arm/mach-sunxi/dram_sun50i_h6.c
> > @@ -37,9 +37,9 @@
> >  
> >  static void mctl_sys_init(struct dram_para *para);
> >  static void mctl_com_init(struct dram_para *para);
> > -static void mctl_channel_init(struct dram_para *para);
> > +static bool mctl_channel_init(struct dram_para *para);
> >  
> > -static void mctl_core_init(struct dram_para *para)
> > +static bool mctl_core_init(struct dram_para *para)
> >  {
> >  	mctl_sys_init(para);
> >  	mctl_com_init(para);
> > @@ -51,7 +51,7 @@ static void mctl_core_init(struct dram_para *para)
> >  	default:
> >  		panic("Unsupported DRAM type!");
> >  	};
> > -	mctl_channel_init(para);
> > +	return mctl_channel_init(para);
> >  }
> >  
> >  /* PHY initialisation */
> > @@ -411,7 +411,7 @@ static void mctl_bit_delay_set(struct dram_para *para)
> >  	}
> >  }
> >  
> > -static void mctl_channel_init(struct dram_para *para)
> > +static bool mctl_channel_init(struct dram_para *para)
> >  {
> >  	struct sunxi_mctl_com_reg * const mctl_com =
> >  			(struct sunxi_mctl_com_reg 
*)SUNXI_DRAM_COM_BASE;
> > @@ -528,46 +528,15 @@ static void mctl_channel_init(struct dram_para 
*para)
> >  		clrbits_le32(&mctl_phy->dx[i].gcr[3], ~0x3ffff);
> >  	udelay(10);
> >  
> > -	if (readl(&mctl_phy->pgsr[0]) & 0x400000)
> > -	{
> > -		/* 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) {
> > -			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;
> > -		}
> > -
> > -		/*
> > -		 * Check for dual rank and half DQ. NOTE: This 
combination
> > -		 * is highly unlikely and was not tested. Condition is 
the
> > -		 * same as in libdram, though.
> > -		 */
> > -		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) {
> >  		/* Oops! There's something wrong! */
> >  		debug("PLL = %x\n", readl(0x3001010));
> >  		debug("DRAM PHY PGSR0 = %x\n", readl(&mctl_phy-
>pgsr[0]));
> >  		for (i = 0; i < 4; i++)
> >  			debug("DRAM PHY DX%dRSR0 = %x\n", i, 
readl(&mctl_phy->dx[i].rsr[0]));
> > -		panic("Error while initializing DRAM PHY!\n");
> > +		debug("Error while initializing DRAM PHY!\n");
> > +
> > +		return false;
> >  	}
> >  
> >  	if (sunxi_dram_is_lpddr(para->type))
> > @@ -582,13 +551,54 @@ static void mctl_channel_init(struct dram_para 
*para)
> >  	writel(0xffffffff, &mctl_com->maer0);
> >  	writel(0x7ff, &mctl_com->maer1);
> >  	writel(0xffff, &mctl_com->maer2);
> > +
> > +	return true;
> > +}
> > +
> > +static void mctl_auto_detect_rank_width(struct dram_para *para)
> > +{
> > +	/* this is minimum size that it's supported */
> > +	para->cols = 8;
> > +	para->rows = 13;
> > +
> > +	/*
> 
> Can you add here that former versions of this code tried to autodetect
> rank and width, but this didn't work reliably? This would give people
> some breadcrumbs to follow with git log/git annotate.
> 
> Otherwise this is fine:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> Tested-by: Andre Przywara <andre.przywara@arm.com> (on Pine H64)
> 
> I can extend the commit while committing, if you like.

Please do. Thanks!

Best regards,
Jernej

> 
> Cheers,
> Andre.
> 
> 
> > +	 * Strategy here is to test most demanding combination first and 
least
> > +	 * demanding last, otherwise HW might not be fully utilized. For
> > +	 * example, half bus width and rank = 1 combination would also 
work
> > +	 * on HW with full bus width and rank = 2, but only 1/4 RAM would 
be
> > +	 * visible.
> > +	 */
> > +
> > +	debug("testing 32-bit width, rank = 2\n");
> > +	para->bus_full_width = 1;
> > +	para->ranks = 2;
> > +	if (mctl_core_init(para))
> > +		return;
> > +
> > +	debug("testing 32-bit width, rank = 1\n");
> > +	para->bus_full_width = 1;
> > +	para->ranks = 1;
> > +	if (mctl_core_init(para))
> > +		return;
> > +
> > +	debug("testing 16-bit width, rank = 2\n");
> > +	para->bus_full_width = 0;
> > +	para->ranks = 2;
> > +	if (mctl_core_init(para))
> > +		return;
> > +
> > +	debug("testing 16-bit width, rank = 1\n");
> > +	para->bus_full_width = 0;
> > +	para->ranks = 1;
> > +	if (mctl_core_init(para))
> > +		return;
> > +
> > +	panic("This DRAM setup is currently not supported.\n");
> >  }
> >  
> >  static void mctl_auto_detect_dram_size(struct dram_para *para)
> >  {
> >  	/* 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 */
> >  	para->cols = 8;
> > @@ -652,10 +662,6 @@ unsigned long sunxi_dram_init(void)
> >  			(struct sunxi_mctl_com_reg 
*)SUNXI_DRAM_COM_BASE;
> >  	struct dram_para para = {
> >  		.clk = CONFIG_DRAM_CLK,
> > -		.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,
> > @@ -673,6 +679,7 @@ unsigned long sunxi_dram_init(void)
> >  	setbits_le32(0x7010310, BIT(8));
> >  	clrbits_le32(0x7010318, 0x3f);
> >  
> > +	mctl_auto_detect_rank_width(&para);
> >  	mctl_auto_detect_dram_size(&para);
> >  
> >  	mctl_core_init(&para);
> > 
> 
>