On Thu, Aug 11, 2022 at 08:58:48AM +0100, 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/yo4v3juehxtov...@google.com/
> Suggested-by: YouMin Chen <c...@rock-chips.com>
> Signed-off-by: Lee Jones <l...@kernel.org>
> Tested-by: Xavier Drudis Ferran <xdru...@tinet.cat>
Also does not cause any regression on a Pinebook Pro
Tested-by: Michal Suchánek <msucha...@suse.de>
Thanks
Michal
> Reviewed-by: Kever Yang <kever.y...@rock-chips.com>
> ---
> 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.1.559.g78731f0fdb-goog
>
