mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation
@ 2022-02-25 14:47 Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 1/4] ARM: imx: Add imx8 support for 18 bit SDRAM row size handle Joacim Zetterling
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Joacim Zetterling @ 2022-02-25 14:47 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

There is some issues with the imx8 ddrc sdram size calculation.

If we compare the imx8mn DDR4 evk against the LPDDR4 variant in
code and in the datasheets, we see the following:

             DDR4   LPDDR4
  ========================
  Bus width   16      16
  Rank        1       1
  Ranks       1       1
  Banks       4       8
  Banks grps  2       1
  Rows        17      15
  Col         10      10

This gives us the following problems:

1. Bus width problem.
   Does not support 16 bit SDRAM bus mode, only 32 bit supported

2. Row size problem.
   Only up to 16 bit row size support.

3. Bank groups support.
   Only support of 1 bank group.

4. Bit count problem.
   The imx_ddrc_count_bits function does not do a correct count.
   Found out during test of the 1-3 part fixes.

The fact that the code only handled a 32 bit bus width, compensated
the problems with rows, banks and rank.

This have been tested on the NXP IMX8MN-EVK with 2GB DDR4 SDRAM as
well as on a custom board with 512MB LPDDR4 SDRAM.


Joacim Zetterling (4):
  ARM: imx: Add imx8 support for 18 bit SDRAM row size handle
  ARM: imx: Add imx8 support for SDRAM with two or more bank groups
  ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width
  ARM: imx: Correct bit count function

 arch/arm/mach-imx/esdctl.c | 61 +++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 20 deletions(-)

-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* [PATCH 1/4] ARM: imx: Add imx8 support for 18 bit SDRAM row size handle
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
@ 2022-02-25 14:47 ` Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 2/4] ARM: imx: Add imx8 support for SDRAM with two or more bank groups Joacim Zetterling
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Joacim Zetterling @ 2022-02-25 14:47 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

The current row calculation in imx_ddrc_sdram_size only handle a
row size up to 16 bit, the row size of the imx8mn DDR4 needs 17 bits.

We need to add DDRC_ADDRMAP7_ROW_B16 and DDRC_ADDRMAP7_ROW_B17 in
the row check table to support a 18 bit row size.

Consulting the reference manual for imx8mm, imx8mn and imx8mq derivates
for 18 bit row size support and it is fin by them.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/esdctl.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index e56da3cb76d4..8e2d9e0c115c 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -427,7 +427,8 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc)
 		readl(ddrc + DDRC_ADDRMAP(4)),
 		readl(ddrc + DDRC_ADDRMAP(5)),
 		readl(ddrc + DDRC_ADDRMAP(6)),
-		readl(ddrc + DDRC_ADDRMAP(7))
+		readl(ddrc + DDRC_ADDRMAP(7)),
+		readl(ddrc + DDRC_ADDRMAP(8))
 	};
 	const u8 col_b[] = {
 		/*
@@ -445,15 +446,8 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc)
 		FIELD_GET(DDRC_ADDRMAP2_COL_B4,  addrmap[2]),
 	};
 	const u8 row_b[] = {
-		/*
-		 * FIXME: RM mentions the following fields as being
-		 * present, but looking at the code generated by DDR
-		 * tool it doesn't look like those registers are
-		 * really implemented/used.
-		 *
-		 * FIELD_GET(DDRC_ADDRMAP7_ROW_B17, addrmap[7]),
-		 * FIELD_GET(DDRC_ADDRMAP7_ROW_B16, addrmap[7]),
-		 */
+		FIELD_GET(DDRC_ADDRMAP7_ROW_B17, addrmap[7]),
+		FIELD_GET(DDRC_ADDRMAP7_ROW_B16, addrmap[7]),
 		FIELD_GET(DDRC_ADDRMAP6_ROW_B15, addrmap[6]),
 		FIELD_GET(DDRC_ADDRMAP6_ROW_B14, addrmap[6]),
 		FIELD_GET(DDRC_ADDRMAP6_ROW_B13, addrmap[6]),
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* [PATCH 2/4] ARM: imx: Add imx8 support for SDRAM with two or more bank groups
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 1/4] ARM: imx: Add imx8 support for 18 bit SDRAM row size handle Joacim Zetterling
@ 2022-02-25 14:47 ` Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 3/4] ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width Joacim Zetterling
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Joacim Zetterling @ 2022-02-25 14:47 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

Add bank group size to handle SDRAM with two or more bank groups
in one chip. The imx8mn DDR4 has one mem chip with ranks set to 1 and
the number of banks is 4 in 2 groups, total of 8 banks.

We need two add the DDRC_ADDRMAP8 and do a check DDRC_ADDRMAP8_BG_B0
and DDRC_ADDRMAP8_BG_B1 to get the number of bank groups in the chip.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/esdctl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index 8e2d9e0c115c..0e11033b81ae 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -346,6 +346,9 @@ static int vf610_ddrmc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
 #define DDRC_ADDRMAP7_ROW_B17			GENMASK(11,  8)
 #define DDRC_ADDRMAP7_ROW_B16			GENMASK( 3,  0)
 
+#define DDRC_ADDRMAP8_BG_B1			GENMASK(13,  8)
+#define DDRC_ADDRMAP8_BG_B0			GENMASK(4,  0)
+
 static unsigned int
 imx_ddrc_count_bits(unsigned int bits, const u8 config[],
 		     unsigned int config_num)
@@ -409,6 +412,13 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
 	if (FIELD_GET(DDRC_ADDRMAP1_BANK_B2, addrmap[1]) != 0b11111)
 		banks++;
 
+	if (addrmap[8]) {
+		if (FIELD_GET(DDRC_ADDRMAP8_BG_B0, addrmap[8]) != 0b11111)
+			banks++;
+		if (FIELD_GET(DDRC_ADDRMAP8_BG_B1, addrmap[8]) != 0b111111)
+			banks++;
+	}
+
 	columns = imx_ddrc_count_bits(col_max, col_b, col_b_num);
 	rows    = imx_ddrc_count_bits(row_max, row_b, row_b_num);
 
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* [PATCH 3/4] ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 1/4] ARM: imx: Add imx8 support for 18 bit SDRAM row size handle Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 2/4] ARM: imx: Add imx8 support for SDRAM with two or more bank groups Joacim Zetterling
@ 2022-02-25 14:47 ` Joacim Zetterling
  2022-02-25 14:47 ` [PATCH 4/4] ARM: imx: Correct bit count function Joacim Zetterling
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Joacim Zetterling @ 2022-02-25 14:47 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

The imx8mn has a 16-bit SDRAM bus width access but the calculation
of the memory size treat it as a 32-bit width bus which makes the
memory calculation to be wrong (meminfo wrong and memtest fails).

There is a difference between the imx7 and the imx8 familys.
The imx8 family has a device config field in the master register of
the DDRC controller which the imx7 family doesn't have (the bus width
is 32-bit as default).

The device config field together with the DQ configuration tells us
the actual bus width of the device for a correct mem size calculaton.

>From the imx8mn reference manual:
+----------------------------------------------------+
|    Field      |     Function                       |
|----------------------------------------------------|
|    31-30      | Indicates the configuration of the |
|               | device used in the system.         |
| device_config |     00b - x4 device                |
|               |     01b - x8 device                |
|               |     10b - x16 device               |
|               |     11b - x32 device               |
+----------------------------------------------------+
	...
	...

The imx8 supports a bus width of 4 bits or x4 (device_config b00).
This is a problem for the calculation of the mem size when it only
handle the bus width in bytes. Therefore we must treat the mem size
calculation for the half bus width (width = 0) in a special way.
Do the calculation with one byte width and then divide the mem size
by 2 later on.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/esdctl.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index 0e11033b81ae..29333340318b 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -320,6 +320,7 @@ static int vf610_ddrmc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
 #define DDRC_MSTR_LPDDR4			BIT(5)
 #define DDRC_MSTR_DATA_BUS_WIDTH		GENMASK(13, 12)
 #define DDRC_MSTR_ACTIVE_RANKS			GENMASK(27, 24)
+#define DDRC_MSTR_DEVICE_CONFIG		GENMASK(31, 30)
 
 #define DDRC_ADDRMAP0_CS_BIT1			GENMASK(12,  8)
 
@@ -364,7 +365,7 @@ static resource_size_t
 imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
 		    u8 col_max, const u8 col_b[], unsigned int col_b_num,
 		    u8 row_max, const u8 row_b[], unsigned int row_b_num,
-		    bool reduced_adress_space)
+		    bool reduced_adress_space, bool is_imx8)
 {
 	const u32 mstr = readl(ddrc + DDRC_MSTR);
 	unsigned int banks, ranks, columns, rows, active_ranks, width;
@@ -387,15 +388,20 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
 		BUG();
 	}
 
+	/* Bus width in bytes, 0 means half byte or 4-bit mode */
+	if (is_imx8)
+		width = (1 << FIELD_GET(DDRC_MSTR_DEVICE_CONFIG, mstr)) >> 1;
+	else
+		width = 4;
+
 	switch (FIELD_GET(DDRC_MSTR_DATA_BUS_WIDTH, mstr)) {
 	case 0b00:	/* Full DQ bus  */
-		width = 4;
 		break;
-	case 0b01:      /* Half DQ bus  */
-		width = 2;
+	case 0b01:	/* Half DQ bus  */
+		width >>= 1;
 		break;
 	case 0b10:	/* Quarter DQ bus  */
-		width = 1;
+		width >>= 2;
 		break;
 	default:
 		BUG();
@@ -422,7 +428,15 @@ imx_ddrc_sdram_size(void __iomem *ddrc, const u32 addrmap[],
 	columns = imx_ddrc_count_bits(col_max, col_b, col_b_num);
 	rows    = imx_ddrc_count_bits(row_max, row_b, row_b_num);
 
-	size = memory_sdram_size(columns, rows, 1 << banks, width) << ranks;
+	/*
+	 * Special case when bus width is 0 or x4 mode,
+	 * calculate the mem size and then divide the size by 2.
+	 */
+	if (width)
+		size = memory_sdram_size(columns, rows, 1 << banks, width);
+	else
+		size = memory_sdram_size(columns, rows, 1 << banks, 1) >> 1;
+	size <<= ranks;
 
 	return reduced_adress_space ? size * 3 / 4 : size;
 }
@@ -470,7 +484,7 @@ static resource_size_t imx8m_ddrc_sdram_size(void __iomem *ddrc)
 	return imx_ddrc_sdram_size(ddrc, addrmap,
 				   12, ARRAY_AND_SIZE(col_b),
 				   16, ARRAY_AND_SIZE(row_b),
-				   reduced_adress_space);
+				   reduced_adress_space, true);
 }
 
 static int imx8m_ddrc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
@@ -512,7 +526,7 @@ static resource_size_t imx7d_ddrc_sdram_size(void __iomem *ddrc)
 	return imx_ddrc_sdram_size(ddrc, addrmap,
 				   11, ARRAY_AND_SIZE(col_b),
 				   15, ARRAY_AND_SIZE(row_b),
-				   reduced_adress_space);
+				   reduced_adress_space, false);
 }
 
 static int imx7d_ddrc_add_mem(void *mmdcbase, struct imx_esdctl_data *data)
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* [PATCH 4/4] ARM: imx: Correct bit count function
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
                   ` (2 preceding siblings ...)
  2022-02-25 14:47 ` [PATCH 3/4] ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width Joacim Zetterling
@ 2022-02-25 14:47 ` Joacim Zetterling
  2022-02-28  9:46 ` [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Sascha Hauer
  2022-02-28 10:06 ` Sascha Hauer
  5 siblings, 0 replies; 7+ messages in thread
From: Joacim Zetterling @ 2022-02-25 14:47 UTC (permalink / raw)
  To: barebox; +Cc: a.fatoum

Correct the imx_ddrc_count_bits which currently did a wrong
bit check (stopped after the first check).

Tested on a IMX8MN Evk with 2GB DDR4 and on a IMX8MN custom board
with 512MB LPDDR4, checked size and made memory test.

Signed-off-by: Joacim Zetterling <joacim.zetterling@westermo.com>
---
 arch/arm/mach-imx/esdctl.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/esdctl.c b/arch/arm/mach-imx/esdctl.c
index 29333340318b..d3dbfff423da 100644
--- a/arch/arm/mach-imx/esdctl.c
+++ b/arch/arm/mach-imx/esdctl.c
@@ -355,8 +355,11 @@ imx_ddrc_count_bits(unsigned int bits, const u8 config[],
 		     unsigned int config_num)
 {
 	unsigned int i;
-	for (i = 0; i < config_num && config[i] == 0b1111; i++)
-		bits--;
+
+	for (i = 0; i < config_num; i++) {
+		if (config[i] == 0b1111)
+			bits--;
+	}
 
 	return bits;
 }
-- 
2.25.1


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* Re: [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
                   ` (3 preceding siblings ...)
  2022-02-25 14:47 ` [PATCH 4/4] ARM: imx: Correct bit count function Joacim Zetterling
@ 2022-02-28  9:46 ` Sascha Hauer
  2022-02-28 10:06 ` Sascha Hauer
  5 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2022-02-28  9:46 UTC (permalink / raw)
  To: Joacim Zetterling; +Cc: barebox, a.fatoum

On Fri, Feb 25, 2022 at 03:47:47PM +0100, Joacim Zetterling wrote:
> There is some issues with the imx8 ddrc sdram size calculation.
> 
> If we compare the imx8mn DDR4 evk against the LPDDR4 variant in
> code and in the datasheets, we see the following:
> 
>              DDR4   LPDDR4
>   ========================
>   Bus width   16      16
>   Rank        1       1
>   Ranks       1       1
>   Banks       4       8
>   Banks grps  2       1
>   Rows        17      15
>   Col         10      10
> 
> This gives us the following problems:
> 
> 1. Bus width problem.
>    Does not support 16 bit SDRAM bus mode, only 32 bit supported
> 
> 2. Row size problem.
>    Only up to 16 bit row size support.
> 
> 3. Bank groups support.
>    Only support of 1 bank group.
> 
> 4. Bit count problem.
>    The imx_ddrc_count_bits function does not do a correct count.
>    Found out during test of the 1-3 part fixes.
> 
> The fact that the code only handled a 32 bit bus width, compensated
> the problems with rows, banks and rank.

So this means this series causes a regression in between which is fixed
at the end of the series, right?

I think this is ok in this case and is better than putting everything in
a single patch. I just want to make sure everyone who bisects barebox
and ends up in one of these patches and likely reads this thread knows
that this is expected.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

* Re: [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation
  2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
                   ` (4 preceding siblings ...)
  2022-02-28  9:46 ` [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Sascha Hauer
@ 2022-02-28 10:06 ` Sascha Hauer
  5 siblings, 0 replies; 7+ messages in thread
From: Sascha Hauer @ 2022-02-28 10:06 UTC (permalink / raw)
  To: Joacim Zetterling; +Cc: barebox, a.fatoum

On Fri, Feb 25, 2022 at 03:47:47PM +0100, Joacim Zetterling wrote:
> There is some issues with the imx8 ddrc sdram size calculation.
> 
> If we compare the imx8mn DDR4 evk against the LPDDR4 variant in
> code and in the datasheets, we see the following:
> 
>              DDR4   LPDDR4
>   ========================
>   Bus width   16      16
>   Rank        1       1
>   Ranks       1       1
>   Banks       4       8
>   Banks grps  2       1
>   Rows        17      15
>   Col         10      10
> 
> This gives us the following problems:
> 
> 1. Bus width problem.
>    Does not support 16 bit SDRAM bus mode, only 32 bit supported
> 
> 2. Row size problem.
>    Only up to 16 bit row size support.
> 
> 3. Bank groups support.
>    Only support of 1 bank group.
> 
> 4. Bit count problem.
>    The imx_ddrc_count_bits function does not do a correct count.
>    Found out during test of the 1-3 part fixes.
> 
> The fact that the code only handled a 32 bit bus width, compensated
> the problems with rows, banks and rank.
> 
> This have been tested on the NXP IMX8MN-EVK with 2GB DDR4 SDRAM as
> well as on a custom board with 512MB LPDDR4 SDRAM.
> 
> 
> Joacim Zetterling (4):
>   ARM: imx: Add imx8 support for 18 bit SDRAM row size handle
>   ARM: imx: Add imx8 support for SDRAM with two or more bank groups
>   ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width
>   ARM: imx: Correct bit count function
> 
>  arch/arm/mach-imx/esdctl.c | 61 +++++++++++++++++++++++++-------------
>  1 file changed, 41 insertions(+), 20 deletions(-)

Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


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

end of thread, other threads:[~2022-02-28 10:07 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-25 14:47 [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Joacim Zetterling
2022-02-25 14:47 ` [PATCH 1/4] ARM: imx: Add imx8 support for 18 bit SDRAM row size handle Joacim Zetterling
2022-02-25 14:47 ` [PATCH 2/4] ARM: imx: Add imx8 support for SDRAM with two or more bank groups Joacim Zetterling
2022-02-25 14:47 ` [PATCH 3/4] ARM: imx: Correct mem size calculation for 4/8/16/32 bit bus width Joacim Zetterling
2022-02-25 14:47 ` [PATCH 4/4] ARM: imx: Correct bit count function Joacim Zetterling
2022-02-28  9:46 ` [PATCH 0/4] ARM: imx: Fix problem with imx8 SDRAM size calculation Sascha Hauer
2022-02-28 10:06 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox