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

[Mikhail Kalashnikov]

On 16.08.2024 4:03 AM, Chris Morgan wrote:
> On Wed, Aug 14, 2024 at 04:06:23PM -0500, Chris Morgan wrote:
>> On Sat, Aug 03, 2024 at 03:29:17PM +0300, Mikhail Kalashnikov wrote:
>>> On 02.08.2024 01:55, Chris Morgan wrote:
>>>> From: Jernej Skrabec <jernej.skrabec at gmail.com>
>>>>
>>>> Adjust H616 LPDDR4 DRAM settings to be in line with vendor driver.
>>>>
>>>> Signed-off-by: Jernej Skrabec <jernej.skrabec at gmail.com>
>>>> Tested-by: Chris Morgan <macromorgan at 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.
>> Should I remove it? Sorry but I'm not super familiar with this specific
>> code.

Unfortunately, I don't know what exactly these changes should change. I 
think it would be right to add only the necessary changes for adding a 
new device. Some devices that use this driver have not been tested yet. 
I think the rest of the changes can be prepared by Jernej when he has time