[v3,16/18] ram: rk3399: Add rank detection support

Message ID 20190715182856.21688-17-jagan@amarulasolutions.com
State New
Headers show
Series
  • ram: rk3399: Add rank detection
Related show

Commit Message

Jagan Teki July 15, 2019, 6:28 p.m. UTC
Right now the rk3399 sdram driver assume that the board
has configured with 2 channels, so any possibility to
enable single channel on the same driver will encounter
channel #1 data training failure.

Log:
U-Boot TPL board init
sdram_init: data training failed
rk3399_dmc_init DRAM init failed -5

So, add an algorithm that can capable to compute the active
or configured rank with associated channel like
a) do rank loop to compute the active rank, with associated
   channel numbers
b) then, succeed the data training only for configured channel
c) preserve the rank for given channel
d) do channel loop for setting the active channel
e) if given rank is zero or inactive on the specific channel,
   clear the timings for the associated channel
f) finally, return error if number of channels is zero

Tested in NanoPI-NEO4 since it support single channel sdram
configuration.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
Signed-off-by: YouMin Chen <cym@rock-chips.com>
---
 drivers/ram/rockchip/sdram_rk3399.c | 110 ++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 24 deletions(-)

Comments

Kever Yang July 16, 2019, 7:45 a.m. UTC | #1
On 2019/7/16 上午2:28, Jagan Teki wrote:
> Right now the rk3399 sdram driver assume that the board
> has configured with 2 channels, so any possibility to
> enable single channel on the same driver will encounter
> channel #1 data training failure.
>
> Log:
> U-Boot TPL board init
> sdram_init: data training failed
> rk3399_dmc_init DRAM init failed -5
>
> So, add an algorithm that can capable to compute the active
> or configured rank with associated channel like
> a) do rank loop to compute the active rank, with associated
>     channel numbers
> b) then, succeed the data training only for configured channel
> c) preserve the rank for given channel
> d) do channel loop for setting the active channel
> e) if given rank is zero or inactive on the specific channel,
>     clear the timings for the associated channel
> f) finally, return error if number of channels is zero
>
> Tested in NanoPI-NEO4 since it support single channel sdram
> configuration.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Signed-off-by: YouMin Chen <cym@rock-chips.com>

Reviewed-by: Kever Yang <Kever.yang@rock-chips.com>

Thanks,
  - Kever
> ---
>   drivers/ram/rockchip/sdram_rk3399.c | 110 ++++++++++++++++++++++------
>   1 file changed, 86 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 8bbacb5275..b83955f94e 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -1254,13 +1254,52 @@ static unsigned char calculate_stride(struct rk3399_sdram_params *params)
>   	return stride;
>   }
>   
> +static void clear_channel_params(struct rk3399_sdram_params *params, u8 channel)
> +{
> +	params->ch[channel].cap_info.rank = 0;
> +	params->ch[channel].cap_info.col = 0;
> +	params->ch[channel].cap_info.bk = 0;
> +	params->ch[channel].cap_info.bw = 32;
> +	params->ch[channel].cap_info.dbw = 32;
> +	params->ch[channel].cap_info.row_3_4 = 0;
> +	params->ch[channel].cap_info.cs0_row = 0;
> +	params->ch[channel].cap_info.cs1_row = 0;
> +	params->ch[channel].cap_info.ddrconfig = 0;
> +}
> +
> +static int pctl_init(struct dram_info *dram, struct rk3399_sdram_params *params)
> +{
> +	int channel;
> +	int ret;
> +
> +	for (channel = 0; channel < 2; channel++) {
> +		const struct chan_info *chan = &dram->chan[channel];
> +		struct rk3399_cru *cru = dram->cru;
> +		struct rk3399_ddr_publ_regs *publ = chan->publ;
> +
> +		phy_pctrl_reset(cru, channel);
> +		phy_dll_bypass_set(publ, params->base.ddr_freq);
> +
> +		ret = pctl_cfg(dram, chan, channel, params);
> +		if (ret < 0) {
> +			printf("%s: pctl config failed\n", __func__);
> +			return ret;
> +		}
> +
> +		/* start to trigger initialization */
> +		pctl_start(dram, channel);
> +	}
> +
> +	return 0;
> +}
> +
>   static int sdram_init(struct dram_info *dram,
>   		      struct rk3399_sdram_params *params)
>   {
>   	unsigned char dramtype = params->base.dramtype;
>   	unsigned int ddr_freq = params->base.ddr_freq;
> -	struct rk3399_cru *cru = dram->cru;
> -	int channel;
> +	u32 training_flag = PI_READ_GATE_TRAINING;
> +	int channel, ch, rank;
>   	int ret;
>   
>   	debug("Starting SDRAM initialization...\n");
> @@ -1272,36 +1311,59 @@ static int sdram_init(struct dram_info *dram,
>   		return -E2BIG;
>   	}
>   
> -	for (channel = 0; channel < 2; channel++) {
> -		const struct chan_info *chan = &dram->chan[channel];
> -		struct rk3399_ddr_publ_regs *publ = chan->publ;
> +	for (ch = 0; ch < 2; ch++) {
> +		params->ch[ch].cap_info.rank = 2;
> +		for (rank = 2; rank != 0; rank--) {
> +			ret = pctl_init(dram, params);
> +			if (ret < 0) {
> +				printf("%s: pctl init failed\n", __func__);
> +				return ret;
> +			}
>   
> -		phy_pctrl_reset(cru, channel);
> -		phy_dll_bypass_set(publ, ddr_freq);
> +			/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
> +			if (dramtype == LPDDR3)
> +				udelay(10);
>   
> -		if (channel >= params->base.num_channels)
> -			continue;
> +			params->ch[ch].cap_info.rank = rank;
>   
> -		ret = pctl_cfg(dram, chan, channel, params);
> -		if (ret < 0) {
> -			printf("%s: pctl config failed\n", __func__);
> -			return ret;
> -		}
> +			/*
> +			 * LPDDR3 CA training msut be trigger before
> +			 * other training.
> +			 * DDR3 is not have CA training.
> +			 */
> +			if (params->base.dramtype == LPDDR3)
> +				training_flag |= PI_CA_TRAINING;
>   
> -		/* start to trigger initialization */
> -		pctl_start(dram, channel);
> +			if (!(data_training(&dram->chan[ch], ch,
> +					    params, training_flag)))
> +				break;
> +		}
> +		/* Computed rank with associated channel number */
> +		params->ch[ch].cap_info.rank = rank;
> +	}
>   
> -		/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
> -		if (dramtype == LPDDR3)
> -			udelay(10);
> +	params->base.num_channels = 0;
> +	for (channel = 0; channel < 2; channel++) {
> +		const struct chan_info *chan = &dram->chan[channel];
> +		struct sdram_cap_info *cap_info = &params->ch[channel].cap_info;
>   
> -		if (data_training(chan, channel, params, PI_FULL_TRAINING)) {
> -			printf("%s: data training failed\n", __func__);
> -			return -EIO;
> +		if (cap_info->rank == 0) {
> +			clear_channel_params(params, channel);
> +			continue;
> +		} else {
> +			params->base.num_channels++;
>   		}
>   
> -		set_ddrconfig(chan, params, channel,
> -			      params->ch[channel].cap_info.ddrconfig);
> +		debug("Channel ");
> +		debug(channel ? "1: " : "0: ");
> +
> +		set_ddrconfig(chan, params, channel, cap_info->ddrconfig);
> +	}
> +
> +	if (params->base.num_channels == 0) {
> +		printf("%s: ", __func__);
> +		printf(" - %dMHz failed!\n", params->base.ddr_freq);
> +		return -EINVAL;
>   	}
>   
>   	params->base.stride = calculate_stride(params);
Kever Yang July 17, 2019, 12:53 p.m. UTC | #2
Hi Jagan,

     I'm sure this patch have some problem with evb-rk3399, which I 
think make the ddr not init correctly and error happen in ATF.

     Please help to check the detail of this patch, you can send new 
version of this patch only to mailing list and I will replace it to 
older version.

Thanks,
- Kever
On 2019/7/16 上午2:28, Jagan Teki wrote:
> Right now the rk3399 sdram driver assume that the board
> has configured with 2 channels, so any possibility to
> enable single channel on the same driver will encounter
> channel #1 data training failure.
>
> Log:
> U-Boot TPL board init
> sdram_init: data training failed
> rk3399_dmc_init DRAM init failed -5
>
> So, add an algorithm that can capable to compute the active
> or configured rank with associated channel like
> a) do rank loop to compute the active rank, with associated
>     channel numbers
> b) then, succeed the data training only for configured channel
> c) preserve the rank for given channel
> d) do channel loop for setting the active channel
> e) if given rank is zero or inactive on the specific channel,
>     clear the timings for the associated channel
> f) finally, return error if number of channels is zero
>
> Tested in NanoPI-NEO4 since it support single channel sdram
> configuration.
>
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> Signed-off-by: YouMin Chen <cym@rock-chips.com>
> ---
>   drivers/ram/rockchip/sdram_rk3399.c | 110 ++++++++++++++++++++++------
>   1 file changed, 86 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 8bbacb5275..b83955f94e 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -1254,13 +1254,52 @@ static unsigned char calculate_stride(struct rk3399_sdram_params *params)
>   	return stride;
>   }
>   
> +static void clear_channel_params(struct rk3399_sdram_params *params, u8 channel)
> +{
> +	params->ch[channel].cap_info.rank = 0;
> +	params->ch[channel].cap_info.col = 0;
> +	params->ch[channel].cap_info.bk = 0;
> +	params->ch[channel].cap_info.bw = 32;
> +	params->ch[channel].cap_info.dbw = 32;
> +	params->ch[channel].cap_info.row_3_4 = 0;
> +	params->ch[channel].cap_info.cs0_row = 0;
> +	params->ch[channel].cap_info.cs1_row = 0;
> +	params->ch[channel].cap_info.ddrconfig = 0;
> +}
> +
> +static int pctl_init(struct dram_info *dram, struct rk3399_sdram_params *params)
> +{
> +	int channel;
> +	int ret;
> +
> +	for (channel = 0; channel < 2; channel++) {
> +		const struct chan_info *chan = &dram->chan[channel];
> +		struct rk3399_cru *cru = dram->cru;
> +		struct rk3399_ddr_publ_regs *publ = chan->publ;
> +
> +		phy_pctrl_reset(cru, channel);
> +		phy_dll_bypass_set(publ, params->base.ddr_freq);
> +
> +		ret = pctl_cfg(dram, chan, channel, params);
> +		if (ret < 0) {
> +			printf("%s: pctl config failed\n", __func__);
> +			return ret;
> +		}
> +
> +		/* start to trigger initialization */
> +		pctl_start(dram, channel);
> +	}
> +
> +	return 0;
> +}
> +
>   static int sdram_init(struct dram_info *dram,
>   		      struct rk3399_sdram_params *params)
>   {
>   	unsigned char dramtype = params->base.dramtype;
>   	unsigned int ddr_freq = params->base.ddr_freq;
> -	struct rk3399_cru *cru = dram->cru;
> -	int channel;
> +	u32 training_flag = PI_READ_GATE_TRAINING;
> +	int channel, ch, rank;
>   	int ret;
>   
>   	debug("Starting SDRAM initialization...\n");
> @@ -1272,36 +1311,59 @@ static int sdram_init(struct dram_info *dram,
>   		return -E2BIG;
>   	}
>   
> -	for (channel = 0; channel < 2; channel++) {
> -		const struct chan_info *chan = &dram->chan[channel];
> -		struct rk3399_ddr_publ_regs *publ = chan->publ;
> +	for (ch = 0; ch < 2; ch++) {
> +		params->ch[ch].cap_info.rank = 2;
> +		for (rank = 2; rank != 0; rank--) {
> +			ret = pctl_init(dram, params);
> +			if (ret < 0) {
> +				printf("%s: pctl init failed\n", __func__);
> +				return ret;
> +			}
>   
> -		phy_pctrl_reset(cru, channel);
> -		phy_dll_bypass_set(publ, ddr_freq);
> +			/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
> +			if (dramtype == LPDDR3)
> +				udelay(10);
>   
> -		if (channel >= params->base.num_channels)
> -			continue;
> +			params->ch[ch].cap_info.rank = rank;
>   
> -		ret = pctl_cfg(dram, chan, channel, params);
> -		if (ret < 0) {
> -			printf("%s: pctl config failed\n", __func__);
> -			return ret;
> -		}
> +			/*
> +			 * LPDDR3 CA training msut be trigger before
> +			 * other training.
> +			 * DDR3 is not have CA training.
> +			 */
> +			if (params->base.dramtype == LPDDR3)
> +				training_flag |= PI_CA_TRAINING;
>   
> -		/* start to trigger initialization */
> -		pctl_start(dram, channel);
> +			if (!(data_training(&dram->chan[ch], ch,
> +					    params, training_flag)))
> +				break;
> +		}
> +		/* Computed rank with associated channel number */
> +		params->ch[ch].cap_info.rank = rank;
> +	}
>   
> -		/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
> -		if (dramtype == LPDDR3)
> -			udelay(10);
> +	params->base.num_channels = 0;
> +	for (channel = 0; channel < 2; channel++) {
> +		const struct chan_info *chan = &dram->chan[channel];
> +		struct sdram_cap_info *cap_info = &params->ch[channel].cap_info;
>   
> -		if (data_training(chan, channel, params, PI_FULL_TRAINING)) {
> -			printf("%s: data training failed\n", __func__);
> -			return -EIO;
> +		if (cap_info->rank == 0) {
> +			clear_channel_params(params, channel);
> +			continue;
> +		} else {
> +			params->base.num_channels++;
>   		}
>   
> -		set_ddrconfig(chan, params, channel,
> -			      params->ch[channel].cap_info.ddrconfig);
> +		debug("Channel ");
> +		debug(channel ? "1: " : "0: ");
> +
> +		set_ddrconfig(chan, params, channel, cap_info->ddrconfig);
> +	}
> +
> +	if (params->base.num_channels == 0) {
> +		printf("%s: ", __func__);
> +		printf(" - %dMHz failed!\n", params->base.ddr_freq);
> +		return -EINVAL;
>   	}
>   
>   	params->base.stride = calculate_stride(params);
Jagan Teki July 18, 2019, 9:30 a.m. UTC | #3
Hi Kever,

On Wed, Jul 17, 2019 at 6:23 PM Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Jagan,
>
>      I'm sure this patch have some problem with evb-rk3399, which I
> think make the ddr not init correctly and error happen in ATF.

Would you help to share the log for this. I made test on Nanopi-m4
which has same ddr type as evb, LPDDR3 and it booted fine with
mainline ATF (with PLAT=rk3399 bl31). Below is the log details.

U-Boot TPL 2019.07-00145-gcb5756d9e2-dirty (Jul 18 2019 - 14:55:23)
Channel 0: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=32 Size=2048MB
Channel 1: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=32 Size=2048MB
256B stride
Trying to boot from BOOTROM
Returning to boot ROM...

U-Boot SPL 2019.07-00145-gcb5756d9e2-dirty (Jul 18 2019 - 14:55:23 +0530)
Trying to boot from MMC1


U-Boot 2019.07-00145-gcb5756d9e2-dirty (Jul 18 2019 - 14:55:23 +0530)

Model: FriendlyElec NanoPi M4
DRAM:  3.9 GiB
MMC:   dwmmc@fe310000: 2, dwmmc@fe320000: 1, sdhci@fe330000: 0
Loading Environment from MMC... *** Warning - bad CRC, using default environment

In:    serial@ff1a0000
Out:   serial@ff1a0000
Err:   serial@ff1a0000
Model: FriendlyElec NanoPi M4

Let me know if have anything to add?

Jagan.

Patch

diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
index 8bbacb5275..b83955f94e 100644
--- a/drivers/ram/rockchip/sdram_rk3399.c
+++ b/drivers/ram/rockchip/sdram_rk3399.c
@@ -1254,13 +1254,52 @@  static unsigned char calculate_stride(struct rk3399_sdram_params *params)
 	return stride;
 }
 
+static void clear_channel_params(struct rk3399_sdram_params *params, u8 channel)
+{
+	params->ch[channel].cap_info.rank = 0;
+	params->ch[channel].cap_info.col = 0;
+	params->ch[channel].cap_info.bk = 0;
+	params->ch[channel].cap_info.bw = 32;
+	params->ch[channel].cap_info.dbw = 32;
+	params->ch[channel].cap_info.row_3_4 = 0;
+	params->ch[channel].cap_info.cs0_row = 0;
+	params->ch[channel].cap_info.cs1_row = 0;
+	params->ch[channel].cap_info.ddrconfig = 0;
+}
+
+static int pctl_init(struct dram_info *dram, struct rk3399_sdram_params *params)
+{
+	int channel;
+	int ret;
+
+	for (channel = 0; channel < 2; channel++) {
+		const struct chan_info *chan = &dram->chan[channel];
+		struct rk3399_cru *cru = dram->cru;
+		struct rk3399_ddr_publ_regs *publ = chan->publ;
+
+		phy_pctrl_reset(cru, channel);
+		phy_dll_bypass_set(publ, params->base.ddr_freq);
+
+		ret = pctl_cfg(dram, chan, channel, params);
+		if (ret < 0) {
+			printf("%s: pctl config failed\n", __func__);
+			return ret;
+		}
+
+		/* start to trigger initialization */
+		pctl_start(dram, channel);
+	}
+
+	return 0;
+}
+
 static int sdram_init(struct dram_info *dram,
 		      struct rk3399_sdram_params *params)
 {
 	unsigned char dramtype = params->base.dramtype;
 	unsigned int ddr_freq = params->base.ddr_freq;
-	struct rk3399_cru *cru = dram->cru;
-	int channel;
+	u32 training_flag = PI_READ_GATE_TRAINING;
+	int channel, ch, rank;
 	int ret;
 
 	debug("Starting SDRAM initialization...\n");
@@ -1272,36 +1311,59 @@  static int sdram_init(struct dram_info *dram,
 		return -E2BIG;
 	}
 
-	for (channel = 0; channel < 2; channel++) {
-		const struct chan_info *chan = &dram->chan[channel];
-		struct rk3399_ddr_publ_regs *publ = chan->publ;
+	for (ch = 0; ch < 2; ch++) {
+		params->ch[ch].cap_info.rank = 2;
+		for (rank = 2; rank != 0; rank--) {
+			ret = pctl_init(dram, params);
+			if (ret < 0) {
+				printf("%s: pctl init failed\n", __func__);
+				return ret;
+			}
 
-		phy_pctrl_reset(cru, channel);
-		phy_dll_bypass_set(publ, ddr_freq);
+			/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
+			if (dramtype == LPDDR3)
+				udelay(10);
 
-		if (channel >= params->base.num_channels)
-			continue;
+			params->ch[ch].cap_info.rank = rank;
 
-		ret = pctl_cfg(dram, chan, channel, params);
-		if (ret < 0) {
-			printf("%s: pctl config failed\n", __func__);
-			return ret;
-		}
+			/*
+			 * LPDDR3 CA training msut be trigger before
+			 * other training.
+			 * DDR3 is not have CA training.
+			 */
+			if (params->base.dramtype == LPDDR3)
+				training_flag |= PI_CA_TRAINING;
 
-		/* start to trigger initialization */
-		pctl_start(dram, channel);
+			if (!(data_training(&dram->chan[ch], ch,
+					    params, training_flag)))
+				break;
+		}
+		/* Computed rank with associated channel number */
+		params->ch[ch].cap_info.rank = rank;
+	}
 
-		/* LPDDR2/LPDDR3 need to wait DAI complete, max 10us */
-		if (dramtype == LPDDR3)
-			udelay(10);
+	params->base.num_channels = 0;
+	for (channel = 0; channel < 2; channel++) {
+		const struct chan_info *chan = &dram->chan[channel];
+		struct sdram_cap_info *cap_info = &params->ch[channel].cap_info;
 
-		if (data_training(chan, channel, params, PI_FULL_TRAINING)) {
-			printf("%s: data training failed\n", __func__);
-			return -EIO;
+		if (cap_info->rank == 0) {
+			clear_channel_params(params, channel);
+			continue;
+		} else {
+			params->base.num_channels++;
 		}
 
-		set_ddrconfig(chan, params, channel,
-			      params->ch[channel].cap_info.ddrconfig);
+		debug("Channel ");
+		debug(channel ? "1: " : "0: ");
+
+		set_ddrconfig(chan, params, channel, cap_info->ddrconfig);
+	}
+
+	if (params->base.num_channels == 0) {
+		printf("%s: ", __func__);
+		printf(" - %dMHz failed!\n", params->base.ddr_freq);
+		return -EINVAL;
 	}
 
 	params->base.stride = calculate_stride(params);