All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sunxi: dram: h6: Improve DDR3 config detection
@ 2020-12-03 17:46 Jernej Skrabec
  2021-01-08  2:01 ` André Przywara
  0 siblings, 1 reply; 4+ messages in thread
From: Jernej Skrabec @ 2020-12-03 17:46 UTC (permalink / raw
  To: u-boot

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.

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;
+
+	/*
+	 * 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);
-- 
2.29.2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] sunxi: dram: h6: Improve DDR3 config detection
  2020-12-03 17:46 [PATCH] sunxi: dram: h6: Improve DDR3 config detection Jernej Skrabec
@ 2021-01-08  2:01 ` André Przywara
  2021-01-10 18:43   ` Jernej Škrabec
  0 siblings, 1 reply; 4+ messages in thread
From: André Przywara @ 2021-01-08  2:01 UTC (permalink / raw
  To: u-boot

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.

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);
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] sunxi: dram: h6: Improve DDR3 config detection
  2021-01-08  2:01 ` André Przywara
@ 2021-01-10 18:43   ` Jernej Škrabec
  2021-01-10 23:05     ` André Przywara
  0 siblings, 1 reply; 4+ messages in thread
From: Jernej Škrabec @ 2021-01-10 18:43 UTC (permalink / raw
  To: u-boot

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);
> > 
> 
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] sunxi: dram: h6: Improve DDR3 config detection
  2021-01-10 18:43   ` Jernej Škrabec
@ 2021-01-10 23:05     ` André Przywara
  0 siblings, 0 replies; 4+ messages in thread
From: André Przywara @ 2021-01-10 23:05 UTC (permalink / raw
  To: u-boot

On 10/01/2021 18:43, Jernej ?krabec wrote:
> 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>

...

>>> +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!

Done and applied!

Thanks,
Andre

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-10 23:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-03 17:46 [PATCH] sunxi: dram: h6: Improve DDR3 config detection Jernej Skrabec
2021-01-08  2:01 ` André Przywara
2021-01-10 18:43   ` Jernej Škrabec
2021-01-10 23:05     ` André Przywara

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.