diff mbox series

[u-boot-marvell] PLEASE TEST ddr: marvell: a38x: fix SPLIT_OUT_MIX state decision

Message ID 20210208191225.14645-1-marek.behun@nic.cz
State Changes Requested
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series [u-boot-marvell] PLEASE TEST ddr: marvell: a38x: fix SPLIT_OUT_MIX state decision | expand

Commit Message

Marek Behún Feb. 8, 2021, 7:12 p.m. UTC 
From: motib <motib@marvell.com>

In each pattern cycle the bus state can be changed.
In order to avoid it, we need to switch back to the same bus state on
each pattern cycle.

Signed-off-by: motib <motib@marvell.com>

Fixed code style, removed commented code, switched to use DEBUG macros
instead of printf.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 .../a38x/ddr3_training_centralization.c       | 25 +++++++++++++++++++
 .../marvell/a38x/ddr3_training_ip_engine.c    |  8 ++++--
 2 files changed, 31 insertions(+), 2 deletions(-)

Comments

Marek Behún Feb. 8, 2021, 7:15 p.m. UTC | #1 
This patch is needed on some Turris Omnia boards with Samsung DDR chips,
otherwise DDR training fails in ~60% of cases.

Marvell send us this patch for testing, I have updated it a little.

Please test this on other A38x boards.

If it doesn't break anything on other boards, we can apply it and send
it to mv-ddr-marvell upstream.

Marek
Chris Packham Feb. 8, 2021, 8:14 p.m. UTC | #2 
On 9/02/21 8:15 am, Marek Behun wrote:
> This patch is needed on some Turris Omnia boards with Samsung DDR chips,
> otherwise DDR training fails in ~60% of cases.
>
> Marvell send us this patch for testing, I have updated it a little.
>
> Please test this on other A38x boards.
>
> If it doesn't break anything on other boards, we can apply it and send
> it to mv-ddr-marvell upstream.
They gave us the same patch. I was hoping their SoC team would get it 
into mv-ddr-marvell (or even their vendor USP) but obviously they're 
still sitting on it. I know it improved matters for some of our boards 
but it didn't totally fix them we still had yield problems when we 
ramped up production.
Marek Behún Feb. 8, 2021, 8:24 p.m. UTC | #3 
On Mon, 8 Feb 2021 20:14:26 +0000
Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:

> On 9/02/21 8:15 am, Marek Behun wrote:
> > This patch is needed on some Turris Omnia boards with Samsung DDR chips,
> > otherwise DDR training fails in ~60% of cases.
> >
> > Marvell send us this patch for testing, I have updated it a little.
> >
> > Please test this on other A38x boards.
> >
> > If it doesn't break anything on other boards, we can apply it and send
> > it to mv-ddr-marvell upstream.  
> They gave us the same patch. I was hoping their SoC team would get it 
> into mv-ddr-marvell (or even their vendor USP) but obviously they're 
> still sitting on it. I know it improved matters for some of our boards 
> but it didn't totally fix them we still had yield problems when we 
> ramped up production.

There is a bug in the version they sent us:

in file ddr3_training_ip_engine.c there is a line added:
   if ((bit_bit_mask[sybphy_id] & (1 << bit_id)) == 1)

try chaning it to
   if (bit_bit_mask[sybphy_id] & (1 << bit_id))

This is fixed in the version I sent to mailing list
Pali Rohár June 28, 2021, 7:20 a.m. UTC | #4 
On Monday 08 February 2021 21:24:09 Marek Behun wrote:
> On Mon, 8 Feb 2021 20:14:26 +0000
> Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> 
> > On 9/02/21 8:15 am, Marek Behun wrote:
> > > This patch is needed on some Turris Omnia boards with Samsung DDR chips,
> > > otherwise DDR training fails in ~60% of cases.
> > >
> > > Marvell send us this patch for testing, I have updated it a little.
> > >
> > > Please test this on other A38x boards.
> > >
> > > If it doesn't break anything on other boards, we can apply it and send
> > > it to mv-ddr-marvell upstream.  
> > They gave us the same patch. I was hoping their SoC team would get it 
> > into mv-ddr-marvell (or even their vendor USP) but obviously they're 
> > still sitting on it. I know it improved matters for some of our boards 
> > but it didn't totally fix them we still had yield problems when we 
> > ramped up production.
> 
> There is a bug in the version they sent us:
> 
> in file ddr3_training_ip_engine.c there is a line added:
>    if ((bit_bit_mask[sybphy_id] & (1 << bit_id)) == 1)
> 
> try chaning it to
>    if (bit_bit_mask[sybphy_id] & (1 << bit_id))
> 
> This is fixed in the version I sent to mailing list

Hello Chris! Have you tried above modification if it fixes your issue?
Chris Packham June 28, 2021, 9:21 a.m. UTC | #5 
On Mon, Jun 28, 2021 at 7:20 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 08 February 2021 21:24:09 Marek Behun wrote:
> > On Mon, 8 Feb 2021 20:14:26 +0000
> > Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> >
> > > On 9/02/21 8:15 am, Marek Behun wrote:
> > > > This patch is needed on some Turris Omnia boards with Samsung DDR chips,
> > > > otherwise DDR training fails in ~60% of cases.
> > > >
> > > > Marvell send us this patch for testing, I have updated it a little.
> > > >
> > > > Please test this on other A38x boards.
> > > >
> > > > If it doesn't break anything on other boards, we can apply it and send
> > > > it to mv-ddr-marvell upstream.
> > > They gave us the same patch. I was hoping their SoC team would get it
> > > into mv-ddr-marvell (or even their vendor USP) but obviously they're
> > > still sitting on it. I know it improved matters for some of our boards
> > > but it didn't totally fix them we still had yield problems when we
> > > ramped up production.
> >
> > There is a bug in the version they sent us:
> >
> > in file ddr3_training_ip_engine.c there is a line added:
> >    if ((bit_bit_mask[sybphy_id] & (1 << bit_id)) == 1)
> >
> > try chaning it to
> >    if (bit_bit_mask[sybphy_id] & (1 << bit_id))
> >
> > This is fixed in the version I sent to mailing list
>
> Hello Chris! Have you tried above modification if it fixes your issue?

I haven't tried Marek's patch specifically. What we saw seemed to be a
yield issue rather than something that could be tested on just one
board. I must have missed Marek's response to my comment. Given that
he's identified at least one problem with Marvell's SPLIT_OUT_MIX
patch it's probably worth me trying it out on an x530 board. If that's
OK then I think U-Boot can take the patch (or even better
mv-ddr-marvell apply it and U-Boot syncs).
Chris Packham June 28, 2021, 9:53 a.m. UTC | #6 
On Mon, Jun 28, 2021 at 9:21 PM Chris Packham <judge.packham@gmail.com> wrote:
>
> On Mon, Jun 28, 2021 at 7:20 PM Pali Rohár <pali@kernel.org> wrote:
> >
> > On Monday 08 February 2021 21:24:09 Marek Behun wrote:
> > > On Mon, 8 Feb 2021 20:14:26 +0000
> > > Chris Packham <Chris.Packham@alliedtelesis.co.nz> wrote:
> > >
> > > > On 9/02/21 8:15 am, Marek Behun wrote:
> > > > > This patch is needed on some Turris Omnia boards with Samsung DDR chips,
> > > > > otherwise DDR training fails in ~60% of cases.
> > > > >
> > > > > Marvell send us this patch for testing, I have updated it a little.
> > > > >
> > > > > Please test this on other A38x boards.
> > > > >
> > > > > If it doesn't break anything on other boards, we can apply it and send
> > > > > it to mv-ddr-marvell upstream.
> > > > They gave us the same patch. I was hoping their SoC team would get it
> > > > into mv-ddr-marvell (or even their vendor USP) but obviously they're
> > > > still sitting on it. I know it improved matters for some of our boards
> > > > but it didn't totally fix them we still had yield problems when we
> > > > ramped up production.
> > >
> > > There is a bug in the version they sent us:
> > >
> > > in file ddr3_training_ip_engine.c there is a line added:
> > >    if ((bit_bit_mask[sybphy_id] & (1 << bit_id)) == 1)
> > >
> > > try chaning it to
> > >    if (bit_bit_mask[sybphy_id] & (1 << bit_id))
> > >
> > > This is fixed in the version I sent to mailing list
> >
> > Hello Chris! Have you tried above modification if it fixes your issue?
>
> I haven't tried Marek's patch specifically. What we saw seemed to be a
> yield issue rather than something that could be tested on just one
> board. I must have missed Marek's response to my comment. Given that
> he's identified at least one problem with Marvell's SPLIT_OUT_MIX
> patch it's probably worth me trying it out on an x530 board. If that's
> OK then I think U-Boot can take the patch (or even better
> mv-ddr-marvell apply it and U-Boot syncs).

With http://patchwork.ozlabs.org/project/uboot/patch/20210208191225.14645-1-marek.behun@nic.cz/mbox/
applied on top of v2021.07-rc4 the x530 hangs after the SPL. I haven't
had time to look into why.
diff mbox series

Patch

diff --git a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
index 648b37ef6f..a92760681e 100644
--- a/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
+++ b/drivers/ddr/marvell/a38x/ddr3_training_centralization.c
@@ -24,6 +24,8 @@  u8 bus_start_window[NUM_OF_CENTRAL_TYPES][MAX_INTERFACE_NUM][MAX_BUS_NUM];
 u8 centralization_state[MAX_INTERFACE_NUM][MAX_BUS_NUM];
 static u8 ddr3_tip_special_rx_run_once_flag;
 
+extern u8 byte_status[MAX_INTERFACE_NUM][MAX_BUS_NUM];
+
 static int ddr3_tip_centralization(u32 dev_num, u32 mode);
 
 /*
@@ -110,6 +112,7 @@  static int ddr3_tip_centralization(u32 dev_num, u32 mode)
 				(max_win_size - 1) + cons_tap;
 			bus_start_window[mode][if_id][bus_id] = 0;
 			centralization_result[if_id][bus_id] = 0;
+			byte_status[if_id][bus_id] = BYTE_NOT_DEFINED;
 		}
 	}
 
@@ -166,6 +169,12 @@  static int ddr3_tip_centralization(u32 dev_num, u32 mode)
 						  result[search_dir_id][7]));
 				}
 
+				DEBUG_CENTRALIZATION_ENGINE
+					(DEBUG_LEVEL_TRACE,
+					 ("byte_status[%d][%d] = 0x%x\n",
+					 if_id,
+					 bus_id,
+					 byte_status[if_id][bus_id]));
 				for (bit_id = 0; bit_id < BUS_WIDTH_IN_BITS;
 				     bit_id++) {
 					/* check if this code is valid for 2 edge, probably not :( */
@@ -174,11 +183,27 @@  static int ddr3_tip_centralization(u32 dev_num, u32 mode)
 							       [HWS_LOW2HIGH]
 							       [bit_id],
 							       EDGE_1);
+					if ((byte_status[if_id][bus_id] & BYTE_SPLIT_OUT_MIX) ||
+					    (byte_status[if_id][bus_id] & BYTE_HOMOGENEOUS_SPLIT_OUT)) {
+						if (cur_start_win[bit_id] >= 64)
+							cur_start_win[bit_id] -= 64;
+						else
+							cur_start_win[bit_id] = 0;
+						DEBUG_CENTRALIZATION_ENGINE
+							(DEBUG_LEVEL_TRACE,
+							 ("--------------------------\n"));
+					}
 					cur_end_win[bit_id] =
 						GET_TAP_RESULT(result
 							       [HWS_HIGH2LOW]
 							       [bit_id],
 							       EDGE_1);
+					if (cur_end_win[bit_id] >= 64 && (byte_status[if_id][bus_id] & BYTE_SPLIT_OUT_MIX)) {
+						cur_end_win[bit_id] -= 64;
+						DEBUG_CENTRALIZATION_ENGINE
+							(DEBUG_LEVEL_TRACE,
+							 ("++++++++++++++++++++++++++\n"));
+					}
 					/* window length */
 					current_window[bit_id] =
 						cur_end_win[bit_id] -
diff --git a/drivers/ddr/marvell/a38x/ddr3_training_ip_engine.c b/drivers/ddr/marvell/a38x/ddr3_training_ip_engine.c
index 5fd9a052fa..3d1fa1e74e 100644
--- a/drivers/ddr/marvell/a38x/ddr3_training_ip_engine.c
+++ b/drivers/ddr/marvell/a38x/ddr3_training_ip_engine.c
@@ -1174,7 +1174,6 @@  int ddr3_tip_ip_training_wrapper(u32 dev_num, enum hws_access_type access_type,
 
 			/* zero the data base */
 			bit_bit_mask[sybphy_id] = 0;
-			byte_status[if_id][sybphy_id] = BYTE_NOT_DEFINED;
 			for (bit_id = 0; bit_id < bit_end; bit_id++) {
 				h2l_adll_value[sybphy_id][bit_id] = 64;
 				l2h_adll_value[sybphy_id][bit_id] = 0;
@@ -1276,6 +1275,7 @@  int ddr3_tip_ip_training_wrapper(u32 dev_num, enum hws_access_type access_type,
 			l2h_if_train_res = ddr3_tip_get_buf_ptr(dev_num, HWS_LOW2HIGH, result_type, if_id);
 			h2l_if_train_res = ddr3_tip_get_buf_ptr(dev_num, HWS_HIGH2LOW, result_type, if_id);
 			/* search from middle to end */
+
 			ddr3_tip_ip_training
 				(dev_num, ACCESS_TYPE_UNICAST,
 				 if_id, ACCESS_TYPE_MULTICAST,
@@ -1423,7 +1423,7 @@  int ddr3_tip_ip_training_wrapper(u32 dev_num, enum hws_access_type access_type,
 				* the byte can be aligned. in this case add 64 to the the low ui bits aligning it
 				* to the other ui bits
 				*/
-			if (center_subphy_adll_window[sybphy_id] >= 32) {
+			if (center_subphy_adll_window[sybphy_id] >= 32 || bit_bit_mask[sybphy_id] != 0x0) {
 				byte_status[if_id][sybphy_id] = BYTE_SPLIT_OUT_MIX;
 
 				DEBUG_TRAINING_IP_ENGINE
@@ -1432,8 +1432,12 @@  int ddr3_tip_ip_training_wrapper(u32 dev_num, enum hws_access_type access_type,
 					 if_id, sybphy_id, byte_status[if_id][sybphy_id]));
 				for (bit_id = 0; bit_id < bit_end; bit_id++) {
 					if (bit_state[sybphy_id * BUS_WIDTH_IN_BITS + bit_id] == BIT_LOW_UI) {
+						if (bit_bit_mask[sybphy_id] & (1 << bit_id))
+							continue;
 						l2h_if_train_res[sybphy_id * BUS_WIDTH_IN_BITS + bit_id] += 64;
+						l2h_adll_value[sybphy_id][bit_id] = l2h_if_train_res[sybphy_id * BUS_WIDTH_IN_BITS + bit_id];
 						h2l_if_train_res[sybphy_id * BUS_WIDTH_IN_BITS + bit_id] += 64;
+						h2l_adll_value[sybphy_id][bit_id] = h2l_if_train_res[sybphy_id * BUS_WIDTH_IN_BITS + bit_id];
 					}
 					DEBUG_TRAINING_IP_ENGINE
 						(DEBUG_LEVEL_TRACE,