Re: [PATCH 3/9] sunxi: H616: dram: LPDDR4: adjust settings

[Mikhail Kalashnikov <iuncuim@gmail.com>]

On 02.08.2024 01:55, Chris Morgan wrote:
> From: Jernej Skrabec <jernej.skrabec@gmail.com>
>
> Adjust H616 LPDDR4 DRAM settings to be in line with vendor driver.
>
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> Tested-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>   arch/arm/mach-sunxi/dram_sun50i_h616.c        | 35 +++++++++++++------
>   .../dram_timings/h616_lpddr4_2133.c           |  2 +-
>   2 files changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/dram_sun50i_h616.c b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> index a20264d8b4..b6638c519e 100644
> --- a/arch/arm/mach-sunxi/dram_sun50i_h616.c
> +++ b/arch/arm/mach-sunxi/dram_sun50i_h616.c
> @@ -293,14 +293,22 @@ static void mctl_phy_configure_odt(const struct dram_para *para)
>   	dmb();
>   }
>   
> -static bool mctl_phy_write_leveling(const struct dram_config *config)
> +static bool mctl_phy_write_leveling(const struct dram_para *para,
> +				    const struct dram_config *config)
>   {
>   	bool result = true;
>   	u32 val;
>   
>   	clrsetbits_le32(SUNXI_DRAM_PHY0_BASE + 8, 0xc0, 0x80);
> -	writel(4, SUNXI_DRAM_PHY0_BASE + 0xc);
> -	writel(0x40, SUNXI_DRAM_PHY0_BASE + 0x10);
> +
> +	if (para->type == SUNXI_DRAM_TYPE_LPDDR4) {
> +		/* MR2 value */
> +		writel(0x1b, SUNXI_DRAM_PHY0_BASE + 0xc);
> +		writel(0, SUNXI_DRAM_PHY0_BASE + 0x10);
> +	} else {
> +		writel(4, SUNXI_DRAM_PHY0_BASE + 0xc);
> +		writel(0x40, SUNXI_DRAM_PHY0_BASE + 0x10);
> +	}
>   
>   	setbits_le32(SUNXI_DRAM_PHY0_BASE + 8, 4);
>   
> @@ -859,9 +867,9 @@ static void mctl_phy_ca_bit_delay_compensation(const struct dram_para *para,
>   		}
>   		break;
>   	case SUNXI_DRAM_TYPE_LPDDR4:
> -		if (para->tpr2 & 1) {
> -			writel(val, SUNXI_DRAM_PHY0_BASE + 0x788);
> -		} else {
> +		writel(val, SUNXI_DRAM_PHY0_BASE + 0x788);
> +		if (config->ranks == 2) {
> +			val = (para->tpr10 >> 11) & 0x1e;
>   			writel(val, SUNXI_DRAM_PHY0_BASE + 0x794);
>   		};
>   		break;
> @@ -949,6 +957,8 @@ static bool mctl_phy_init(const struct dram_para *para,
>   		break;
>   	case SUNXI_DRAM_TYPE_LPDDR4:
>   		val = para->tpr6 >> 24 & 0xff;
> +		if (!val)
> +			val = 0x33;

We don't need this as the default value from 
arch/arm/mach-sunxi/Kconfigcovers
this case:

config DRAM_SUN50I_H616_TPR6
hex "H616 DRAM TPR6 parameter"
default 0x3300c080

>   		break;
>   	case SUNXI_DRAM_TYPE_DDR4:
>   	default:
> @@ -1080,19 +1090,27 @@ static bool mctl_phy_init(const struct dram_para *para,
>   		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
>   
>   		writel(0xb04, &mctl_ctl->mrctrl1);
> +		udelay(10);
>   		writel(0x80000030, &mctl_ctl->mrctrl0);
> +		udelay(10);
>   		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
>   
>   		writel(0xc72, &mctl_ctl->mrctrl1);
> +		udelay(10);
>   		writel(0x80000030, &mctl_ctl->mrctrl0);
> +		udelay(10);
>   		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
>   
>   		writel(0xe09, &mctl_ctl->mrctrl1);
> +		udelay(10);
>   		writel(0x80000030, &mctl_ctl->mrctrl0);
> +		udelay(10);
>   		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
>   
>   		writel(0x1624, &mctl_ctl->mrctrl1);
> +		udelay(10);
>   		writel(0x80000030, &mctl_ctl->mrctrl0);
> +		udelay(10);
Looks like something experimental.
>   		mctl_await_completion(&mctl_ctl->mrctrl0, BIT(31), 0);
>   		break;
>   	case SUNXI_DRAM_TYPE_DDR4:
> @@ -1108,7 +1126,7 @@ static bool mctl_phy_init(const struct dram_para *para,
>   
>   	if (para->tpr10 & TPR10_WRITE_LEVELING) {
>   		for (i = 0; i < 5; i++)
> -			if (mctl_phy_write_leveling(config))
> +			if (mctl_phy_write_leveling(para, config))
>   				break;
>   		if (i == 5) {
>   			debug("write leveling failed!\n");
> @@ -1234,9 +1252,6 @@ static bool mctl_ctrl_init(const struct dram_para *para,
>   	setbits_le32(&mctl_ctl->unk_0x3180, BIT(31) | BIT(30));
>   	setbits_le32(&mctl_ctl->unk_0x4180, BIT(31) | BIT(30));
>   
> -	if (para->type == SUNXI_DRAM_TYPE_LPDDR4)
> -		setbits_le32(&mctl_ctl->dbictl, 0x1);
> -

I have this piece of code in the opizero3 mtd dump. This condition depends
on tpr13. Judging by the dram_para parameters, it should be performed for
lpddr4 (and maybe ddr4) memory. According to the opizero3 dump it should
clear all values and set BIT(2). Don't know, why I only set BIT(0) here. 
This
may be a bug or another spl from t507 and the first version of the driver.

>   	setbits_le32(&mctl_ctl->rfshctl3, BIT(0));
>   	clrbits_le32(&mctl_ctl->dfimisc, BIT(0));
>   
> diff --git a/arch/arm/mach-sunxi/dram_timings/h616_lpddr4_2133.c b/arch/arm/mach-sunxi/dram_timings/h616_lpddr4_2133.c
> index e6446b9180..6f5c4acbd6 100644
> --- a/arch/arm/mach-sunxi/dram_timings/h616_lpddr4_2133.c
> +++ b/arch/arm/mach-sunxi/dram_timings/h616_lpddr4_2133.c
> @@ -23,7 +23,7 @@ void mctl_set_timing_params(const struct dram_para *para)
>   	u8 trcd		= max(ns_to_t(18), 2);
>   	u8 trc		= ns_to_t(65);
>   	u8 txp		= max(ns_to_t(8), 2);
> -	u8 trtp		= max(ns_to_t(8), 4);
> +	u8 trtp		= 4;
>   	u8 trp		= ns_to_t(21);
>   	u8 tras		= ns_to_t(42);
>   	u16 trefi	= ns_to_t(3904) / 32;