Re: [PATCH 3/3] ram: rk3399: Conduct memory training at 400MHz

[Xavier Drudis Ferran <xdrudis@tinet.cat>]

El Tue, Jun 21, 2022 at 10:07:29AM +0000, Lee Jones deia:
> Currently the default initialisation frequency is 50MHz.  Although
> this does appear to be suitable for some LPDDR4 RAM chips, training at
> this low frequency has been seen to cause Column errors, leading to
> Capacity check errors on others.
> 
> Here we force RAM initialisation to happen at 400MHz before ramping up
> to the final value running value of 800MHz after everything has been
> successfully configured.
> 
> Link: https://lore.kernel.org/u-boot/Yo4v3jUeHXTovjOH@google.com/
> Suggested-by: YouMin Chen <cym@rock-chips.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

My board doesn't suffer with the issue resolved by this series,
however I did apply it and nothing regressed.

Tested-by: Xavier Drudis Ferran <xdrudis@tinet.cat>

> ---
>  drivers/ram/rockchip/sdram_rk3399.c | 36 +++++++++++++++++------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/ram/rockchip/sdram_rk3399.c b/drivers/ram/rockchip/sdram_rk3399.c
> index 34d6c93f95..b05c5925d5 100644
> --- a/drivers/ram/rockchip/sdram_rk3399.c
> +++ b/drivers/ram/rockchip/sdram_rk3399.c
> @@ -85,7 +85,7 @@ struct sdram_rk3399_ops {
>  	int (*data_training_first)(struct dram_info *dram, u32 channel, u8 rank,
>  				   struct rk3399_sdram_params *sdram);
>  	int (*set_rate_index)(struct dram_info *dram,
> -			      struct rk3399_sdram_params *params);
> +			      struct rk3399_sdram_params *params, u32 ctl_fn);
>  	void (*modify_param)(const struct chan_info *chan,
>  			     struct rk3399_sdram_params *params);
>  	struct rk3399_sdram_params *
> @@ -1644,7 +1644,8 @@ static int data_training_first(struct dram_info *dram, u32 channel, u8 rank,
>  }
>  
>  static int switch_to_phy_index1(struct dram_info *dram,
> -				struct rk3399_sdram_params *params)
> +				struct rk3399_sdram_params *params,
> +				u32 unused)
>  {
>  	u32 channel;
>  	u32 *denali_phy;
> @@ -2539,26 +2540,25 @@ static int lpddr4_set_ctl(struct dram_info *dram,
>  }
>  
>  static int lpddr4_set_rate(struct dram_info *dram,
> -			   struct rk3399_sdram_params *params)
> +			    struct rk3399_sdram_params *params,
> +			    u32 ctl_fn)
>  {
> -	u32 ctl_fn;
>  	u32 phy_fn;
>  
> -	for (ctl_fn = 0; ctl_fn < 2; ctl_fn++) {
> -		phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
> +	phy_fn = lpddr4_get_phy_fn(params, ctl_fn);
>  
> -		lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> -		lpddr4_set_ctl(dram, params, ctl_fn,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
> +	lpddr4_set_phy(dram, params, phy_fn, &dfs_cfgs_lpddr4[ctl_fn]);
> +	lpddr4_set_ctl(dram, params, ctl_fn,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq);
>  
> -		if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> -			printf("%s: change freq to %dMHz %d, %d\n", __func__,
> -			       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> -			       ctl_fn, phy_fn);
> -	}
> +	if (IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG))
> +		printf("%s: change freq to %dMHz %d, %d\n", __func__,
> +		       dfs_cfgs_lpddr4[ctl_fn].base.ddr_freq / MHz,
> +		       ctl_fn, phy_fn);
>  
>  	return 0;
>  }
> +
>  #endif /* CONFIG_RAM_RK3399_LPDDR4 */
>  
>  /* CS0,n=1
> @@ -2955,6 +2955,12 @@ static int sdram_init(struct dram_info *dram,
>  		params->ch[ch].cap_info.rank = rank;
>  	}
>  
> +#if defined(CONFIG_RAM_RK3399_LPDDR4)
> +	/* LPDDR4 needs to be trained at 400MHz */
> +	lpddr4_set_rate(dram, params, 0);
> +	params->base.ddr_freq = dfs_cfgs_lpddr4[0].base.ddr_freq / MHz;
> +#endif
> +
>  	params->base.num_channels = 0;
>  	for (channel = 0; channel < 2; channel++) {
>  		const struct chan_info *chan = &dram->chan[channel];
> @@ -3005,7 +3011,7 @@ static int sdram_init(struct dram_info *dram,
>  	params->base.stride = calculate_stride(params);
>  	dram_all_config(dram, params);
>  
> -	ret = dram->ops->set_rate_index(dram, params);
> +	ret = dram->ops->set_rate_index(dram, params, 1);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.37.0.rc0.104.g0611611a94-goog
>