[U-Boot] FSL DDR3/4 wrlvl_override question

in drivers/ddr/fsl/options.c we have:

#if defined(CONFIG_SYS_FSL_DDR3) || defined(CONFIG_SYS_FSL_DDR4)
	/*
	 * due to ddr3 dimm is fly-by topology
	 * we suggest to enable write leveling to
	 * meet the tQDSS under different loading.
	 */
	popts->wrlvl_en = 1;
	popts->zq_en = 1;
	popts->wrlvl_override = 0;
#endif

Here one disable wrlvl_override no matter what board code wants.
However board code still sets wrlvl_override as it needs specify
a good start value for WRLVL_START DQS[0], just like one do for
the other DQS[X] in WRLVL_CNTL_2/3

How about remove the above popts->wrlvl_override = 0; line?



Furthermore drivers/ddr/fsl/ctrl_regs.c I think set_ddr_wrlvl_cntl()
should be adjusted to(or similar):

so that board not using auto WRLVL a chance to specify these parameters.

I am no expert on DDR so I might be totally off here ...

 Jocke

Jocke,

On 09/03/2015 09:55 AM, Joakim Tjernlund wrote:
> in drivers/ddr/fsl/options.c we have:
> 
> #if defined(CONFIG_SYS_FSL_DDR3) || defined(CONFIG_SYS_FSL_DDR4)
> 	/*
> 	 * due to ddr3 dimm is fly-by topology
> 	 * we suggest to enable write leveling to
> 	 * meet the tQDSS under different loading.
> 	 */
> 	popts->wrlvl_en = 1;
> 	popts->zq_en = 1;
> 	popts->wrlvl_override = 0;
> #endif
> 
> Here one disable wrlvl_override no matter what board code wants.
> However board code still sets wrlvl_override as it needs specify
> a good start value for WRLVL_START DQS[0], just like one do for
> the other DQS[X] in WRLVL_CNTL_2/3
> 
> How about remove the above popts->wrlvl_override = 0; line?

You don't need to. This is the default to start with. See the call at the end of
this function, fsl_ddr_board_options()? It calls for board function to overwrite
the default.

> 
> 
> 
> Furthermore drivers/ddr/fsl/ctrl_regs.c I think set_ddr_wrlvl_cntl()
> should be adjusted to(or similar):
> 
> --- a/drivers/ddr/fsl/ctrl_regs.c
> +++ b/drivers/ddr/fsl/ctrl_regs.c
> @@ -2098,7 +2098,7 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>         /* WRLVL_WLR: Write leveling repeition time */
>         unsigned int wrlvl_wlr = 0;
>         /* WRLVL_START: Write leveling start time */
> -       unsigned int wrlvl_start = 0;
> +       unsigned int wrlvl_start = 8;
>  
>         /* suggest enable write leveling for DDR3 due to fly-by topology */
>         if (wrlvl_en) {
> @@ -2131,10 +2131,11 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>                  * Override the write leveling sample and start time
>                  * according to specific board
>                  */
> -               if (popts->wrlvl_override) {
> -                       wrlvl_smpl = popts->wrlvl_sample;
> -                       wrlvl_start = popts->wrlvl_start;
> -               }
> +       }
> +
> +       if (popts->wrlvl_override) {
> +               wrlvl_smpl = popts->wrlvl_sample;
> +               wrlvl_start = popts->wrlvl_start;
>         }
> 
> 
> so that board not using auto WRLVL a chance to specify these parameters.
> 
> I am no expert on DDR so I might be totally off here ...
> 

NACK. Your change is wrong. wrlvl_smpl and wrlvl_start only have meaning if
wrlvl_en is set. You can make DDR work without write leveling by setting a lot
of timing registers. It is not easy and requires extensive experience and
exercise. It is not recommended for most users. If you have to use that way,
please work with an FAE to get all the correct settings.

On the other side, wrlvl_smpl and wrlvl_start are actually mostly overwritten by
board file. You can take any Freescale board as reference, a random example,
board/freescale/t102xrdb/ddr.c.

York

On Thu, 2015-09-03 at 10:44 -0500, York Sun wrote:
> Jocke,
> 
> On 09/03/2015 09:55 AM, Joakim Tjernlund wrote:
> > in drivers/ddr/fsl/options.c we have:
> > 
> > #if defined(CONFIG_SYS_FSL_DDR3) || defined(CONFIG_SYS_FSL_DDR4)
> > 	/*
> > 	 * due to ddr3 dimm is fly-by topology
> > 	 * we suggest to enable write leveling to
> > 	 * meet the tQDSS under different loading.
> > 	 */
> > 	popts->wrlvl_en = 1;
> > 	popts->zq_en = 1;
> > 	popts->wrlvl_override = 0;
> > #endif
> > 
> > Here one disable wrlvl_override no matter what board code wants.
> > However board code still sets wrlvl_override as it needs specify
> > a good start value for WRLVL_START DQS[0], just like one do for
> > the other DQS[X] in WRLVL_CNTL_2/3
> > 
> > How about remove the above popts->wrlvl_override = 0; line?
> 
> You don't need to. This is the default to start with. See the call at the end of
> this function, fsl_ddr_board_options()? It calls for board function to overwrite
> the default.

Gahh, missed that one :)

> 
> > 
> > 
> > 
> > Furthermore drivers/ddr/fsl/ctrl_regs.c I think set_ddr_wrlvl_cntl()
> > should be adjusted to(or similar):
> > 
> > --- a/drivers/ddr/fsl/ctrl_regs.c
> > +++ b/drivers/ddr/fsl/ctrl_regs.c
> > @@ -2098,7 +2098,7 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
> >         /* WRLVL_WLR: Write leveling repeition time */
> >         unsigned int wrlvl_wlr = 0;
> >         /* WRLVL_START: Write leveling start time */
> > -       unsigned int wrlvl_start = 0;
> > +       unsigned int wrlvl_start = 8;
> >  
> >         /* suggest enable write leveling for DDR3 due to fly-by topology */
> >         if (wrlvl_en) {
> > @@ -2131,10 +2131,11 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
> >                  * Override the write leveling sample and start time
> >                  * according to specific board
> >                  */
> > -               if (popts->wrlvl_override) {
> > -                       wrlvl_smpl = popts->wrlvl_sample;
> > -                       wrlvl_start = popts->wrlvl_start;
> > -               }
> > +       }
> > +
> > +       if (popts->wrlvl_override) {
> > +               wrlvl_smpl = popts->wrlvl_sample;
> > +               wrlvl_start = popts->wrlvl_start;
> >         }
> > 
> > 
> > so that board not using auto WRLVL a chance to specify these parameters.
> > 
> > I am no expert on DDR so I might be totally off here ...
> > 
> 
> NACK. Your change is wrong. wrlvl_smpl and wrlvl_start only have meaning if

OK, I was unsure about wrlvl_start as it was not spelled out in RM like it is for the
other bits in this register.

> wrlvl_en is set. You can make DDR work without write leveling by setting a lot
> of timing registers. It is not easy and requires extensive experience and
> exercise. It is not recommended for most users. If you have to use that way,
> please work with an FAE to get all the correct settings.

We have this coldstart problem that often DDR init fails, we get "Waiting for D_INIT timeout.
Memory may not work." and then the board is stuck.

Working with FAE trying to find the cause it was suggested we try specify the values
ourself (using CW and a probe to auto calculate these). So far this has not worked at
all so I think we will stop here with that track.

Any idea where to look? So far all kinds of HW measurements has been done, all looks ok.
Once one gets past this fail(new coldstart or warmstat) RAM works with anything we
can throw at it, even rebuilt @world (gentoo) on the board.
  
> 
> On the other side, wrlvl_smpl and wrlvl_start are actually mostly overwritten by
> board file. You can take any Freescale board as reference, a random example,
> board/freescale/t102xrdb/ddr.c.

Yes, I see that now

> 
> York

On 09/03/2015 11:09 AM, Joakim Tjernlund wrote:
> On Thu, 2015-09-03 at 10:44 -0500, York Sun wrote:
>> Jocke,
>>
>> On 09/03/2015 09:55 AM, Joakim Tjernlund wrote:
>>> in drivers/ddr/fsl/options.c we have:
>>>
>>> #if defined(CONFIG_SYS_FSL_DDR3) || defined(CONFIG_SYS_FSL_DDR4)
>>> 	/*
>>> 	 * due to ddr3 dimm is fly-by topology
>>> 	 * we suggest to enable write leveling to
>>> 	 * meet the tQDSS under different loading.
>>> 	 */
>>> 	popts->wrlvl_en = 1;
>>> 	popts->zq_en = 1;
>>> 	popts->wrlvl_override = 0;
>>> #endif
>>>
>>> Here one disable wrlvl_override no matter what board code wants.
>>> However board code still sets wrlvl_override as it needs specify
>>> a good start value for WRLVL_START DQS[0], just like one do for
>>> the other DQS[X] in WRLVL_CNTL_2/3
>>>
>>> How about remove the above popts->wrlvl_override = 0; line?
>>
>> You don't need to. This is the default to start with. See the call at the end of
>> this function, fsl_ddr_board_options()? It calls for board function to overwrite
>> the default.
> 
> Gahh, missed that one :)
> 
>>
>>>
>>>
>>>
>>> Furthermore drivers/ddr/fsl/ctrl_regs.c I think set_ddr_wrlvl_cntl()
>>> should be adjusted to(or similar):
>>>
>>> --- a/drivers/ddr/fsl/ctrl_regs.c
>>> +++ b/drivers/ddr/fsl/ctrl_regs.c
>>> @@ -2098,7 +2098,7 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>>>         /* WRLVL_WLR: Write leveling repeition time */
>>>         unsigned int wrlvl_wlr = 0;
>>>         /* WRLVL_START: Write leveling start time */
>>> -       unsigned int wrlvl_start = 0;
>>> +       unsigned int wrlvl_start = 8;
>>>  
>>>         /* suggest enable write leveling for DDR3 due to fly-by topology */
>>>         if (wrlvl_en) {
>>> @@ -2131,10 +2131,11 @@ static void set_ddr_wrlvl_cntl(fsl_ddr_cfg_regs_t *ddr, unsigned int wrlvl_en,
>>>                  * Override the write leveling sample and start time
>>>                  * according to specific board
>>>                  */
>>> -               if (popts->wrlvl_override) {
>>> -                       wrlvl_smpl = popts->wrlvl_sample;
>>> -                       wrlvl_start = popts->wrlvl_start;
>>> -               }
>>> +       }
>>> +
>>> +       if (popts->wrlvl_override) {
>>> +               wrlvl_smpl = popts->wrlvl_sample;
>>> +               wrlvl_start = popts->wrlvl_start;
>>>         }
>>>
>>>
>>> so that board not using auto WRLVL a chance to specify these parameters.
>>>
>>> I am no expert on DDR so I might be totally off here ...
>>>
>>
>> NACK. Your change is wrong. wrlvl_smpl and wrlvl_start only have meaning if
> 
> OK, I was unsure about wrlvl_start as it was not spelled out in RM like it is for the
> other bits in this register.
> 
>> wrlvl_en is set. You can make DDR work without write leveling by setting a lot
>> of timing registers. It is not easy and requires extensive experience and
>> exercise. It is not recommended for most users. If you have to use that way,
>> please work with an FAE to get all the correct settings.
> 
> We have this coldstart problem that often DDR init fails, we get "Waiting for D_INIT timeout.
> Memory may not work." and then the board is stuck.
> 
> Working with FAE trying to find the cause it was suggested we try specify the values
> ourself (using CW and a probe to auto calculate these). So far this has not worked at
> all so I think we will stop here with that track.
> 
> Any idea where to look? So far all kinds of HW measurements has been done, all looks ok.
> Once one gets past this fail(new coldstart or warmstat) RAM works with anything we
> can throw at it, even rebuilt @world (gentoo) on the board.
>   

First thing to check is the DDR reset signal, the one resets the DDR DIMM.
Second, you mentioned CW tool, does that pass?
Third, you can hard-code the registers you got from CW into u-boot, totally
bypass this driver.

York