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

[Kever Yang]

On 2022/6/21 18:07, Lee Jones wrote:
> 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 at rock-chips.com>
> Signed-off-by: Lee Jones <lee.jones at linaro.org>

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

Thanks,
- Kever
> ---
>   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;
>