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.skra...@gmail.com>
> > >
> > > Adjust H616 LPDDR4 DRAM settings to be in line with vendor driver.
> > >
> > > Signed-off-by: Jernej Skrabec <jernej.skra...@gmail.com>
> > > Tested-by: Chris Morgan <macromor...@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.
>
> > > 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.
>
> What would be correct here then?
I've tested removing and/or changing this to writel(BIT(2), &mctl_ctl->dbictl);
and it failed to work. Leaving this as it is things do seem to work though.
Let me know what you want me to test here and I will.
Thank you,
Chris
>
> >
> > > 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;
>
> Thank you,
> Chris
