diff mbox

[U-Boot,v3,09/13] sunxi: DRAM: add Allwinner H5 support

Message ID CAD6G_RSdzWtYZcrN856kf6Dh+EcvN8AH02skWBqhN55BwJXrGw@mail.gmail.com
State Not Applicable
Delegated to: Jagannadha Sutradharudu Teki
Headers show

Commit Message

Jagan Teki Feb. 3, 2017, 3:26 p.m. UTC
On Feb 1, 2017 2:37 AM, "Andre Przywara" <andre.przywara@arm.com> wrote:

The DRAM controller in the Allwinner H5 SoC is again very similar to
the one in the H3 and A64.
Based on the existing socid parameter, add support for this controller
by reusing the bulk of the code and only deviating where needed.
These new bits set or cleared here and there have been mostly found by
looking at DRAM register dumps after using the H5 boot0 and comparing
them to what we set in the code. So for now it's mostly unclear what
those bits actually mean - hence the missing names and comments.
Also add the delay line parameters taken from the boot0 and libdram
disassembly.


Can you split this patch with delay line params as separate patch.


Register setup differences between H5 and H3 are courtesy of Jens Kuske.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/include/asm/arch-sunxi/cpu.h |  1 +
 arch/arm/mach-sunxi/dram_sun8i_h3.c   | 97 +++++++++++++++++++++++++++++-
-----
 2 files changed, 82 insertions(+), 16 deletions(-)

+       MBUS_CONF(DE_CFD, true, HIGHEST, 0,  600,  400,  200);
+}
+
 static void mctl_set_master_priority(uint16_t socid)
 {
        switch (socid) {
@@ -186,6 +214,9 @@ static void mctl_set_master_priority(uint16_t socid)
        case SOCID_A64:
                mctl_set_master_priority_a64();
                return;
+       case SOCID_H5:
+               mctl_set_master_priority_h5();
+               return;
        }
 }

@@ -256,7 +287,7 @@ static void mctl_set_timing_params(uint16_t socid,
struct dram_para *para)

        /* set two rank timing */
        clrsetbits_le32(&mctl_ctl->dramtmg[8], (0xff << 8) | (0xff << 0),
-                       (0x66 << 8) | (0x10 << 0));
+                       ((socid == SOCID_H5 ? 0x33 : 0x66) << 8) | (0x10 <<
0));

        /* set PHY interface timing, write latency and read latency
configure */
        writel((0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8) |
@@ -391,7 +422,7 @@ static void mctl_sys_init(uint16_t socid, struct
dram_para *para)
                                CCM_DRAMCLK_CFG_DIV(1) |
                                CCM_DRAMCLK_CFG_SRC_PLL11 |
                                CCM_DRAMCLK_CFG_UPD);
-       } else if (socid == SOCID_H3) {
+       } else if (socid == SOCID_H3 || socid == SOCID_H5) {
                clock_set_pll5(CONFIG_DRAM_CLK * 2 * 1000000, false);
                clrsetbits_le32(&ccm->dram_clk_cfg,
                                CCM_DRAMCLK_CFG_DIV_MASK |
@@ -410,7 +441,7 @@ static void mctl_sys_init(uint16_t socid, struct
dram_para *para)
        setbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_RST);
        udelay(10);

-       writel(0xc00e, &mctl_ctl->clken);
+       writel(socid == SOCID_H5 ? 0x8000 : 0xc00e, &mctl_ctl->clken);
        udelay(500);
 }

@@ -434,7 +465,10 @@ static int mctl_channel_init(uint16_t socid, struct
dram_para *para)

        /* setting VTC, default disable all VT */
        clrbits_le32(&mctl_ctl->pgcr[0], (1 << 30) | 0x3f);
-       clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);
+       if (socid == SOCID_H5)
+               setbits_le32(&mctl_ctl->pgcr[1], (1 << 24) | (1 << 26));
+       else
+               clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);

        /* increase DFI_PHY_UPD clock */
        writel(PROTECT_MAGIC, &mctl_com->protect);
@@ -444,15 +478,22 @@ static int mctl_channel_init(uint16_t socid, struct
dram_para *para)
        udelay(100);

        /* set dramc odt */
-       for (i = 0; i < 4; i++)
-               clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) |
-                               (0x1 << 1) | (0x3 << 2) | (0x3 << 12) |
-                               (0x3 << 14),
-                               IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
-                                       DX_GCR_ODT_DYNAMIC :
DX_GCR_ODT_OFF);
+       for (i = 0; i < 4; i++) {
+               u32 clearmask = (0x3 << 4) | (0x1 << 1) | (0x3 << 2) |
+                               (0x3 << 12) | (0x3 << 14);
+               u32 setmask = IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
+                               DX_GCR_ODT_DYNAMIC : DX_GCR_ODT_OFF;
+
+               if (socid == SOCID_H5) {
+                       clearmask |= 0x2 << 8;
+                       setmask |= 0x4 << 8;
+               }
+               clrsetbits_le32(&mctl_ctl->dx[i].gcr, clearmask, setmask);
+       }

        /* AC PDR should always ON */
-       setbits_le32(&mctl_ctl->aciocr, 0x1 << 1);
+       clrsetbits_le32(&mctl_ctl->aciocr, socid == SOCID_H5 ? (0x1 << 11)
: 0,
+                       0x1 << 1);

        /* set DQS auto gating PD mode */
        setbits_le32(&mctl_ctl->pgcr[2], 0x3 << 6);
@@ -464,7 +505,7 @@ static int mctl_channel_init(uint16_t socid, struct
dram_para *para)
                /* dphy & aphy phase select 270 degree */
                clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
8),
                                (0x1 << 10) | (0x2 << 8));
-       } else if (socid == SOCID_A64) {
+       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
                /* dphy & aphy phase select ? */
                clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
8),
                                (0x0 << 10) | (0x3 << 8));
@@ -488,11 +529,12 @@ static int mctl_channel_init(uint16_t socid, struct
dram_para *para)

                mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST |
                              PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
-       } else if (socid == SOCID_A64) {
+       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
                clrsetbits_le32(&mctl_ctl->zqcr, 0xffffff, CONFIG_DRAM_ZQ);

                mctl_phy_init(PIR_ZCAL | PIR_PLLINIT | PIR_DCAL |
PIR_PHYRST |
                              PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
+               /* no PIR_QSGATE for H5 ???? */
        }

        /* detect ranks and bus width */
@@ -533,7 +575,7 @@ static int mctl_channel_init(uint16_t socid, struct
dram_para *para)
        /* set PGCR3, CKE polarity */
        if (socid == SOCID_H3)
                writel(0x00aa0060, &mctl_ctl->pgcr[3]);
-       else if (socid == SOCID_A64)
+       else if (socid == SOCID_A64 || socid == SOCID_H5)
                writel(0xc0aa0060, &mctl_ctl->pgcr[3]);

        /* power down zq calibration module for power save */
@@ -604,6 +646,22 @@ static void mctl_auto_detect_dram_size(struct
dram_para *para)
           3,  4,  0,  3,  4,  1,  4,  0,                       \
           1,  1,  0,  1, 13,  5,  4      }

+#define SUN8I_H5_DX_READ_DELAYS                                        \
+       {{ 14, 15, 17, 17, 17, 17, 17, 18, 17,  3,  3 },        \
+        { 21, 21, 12, 22, 21, 21, 21, 21, 21,  3,  3 },        \
+        { 16, 19, 19, 17, 22, 22, 21, 22, 19,  3,  3 },        \
+        { 21, 21, 22, 22, 20, 21, 19, 19, 19,  3,  3 } }
+#define SUN8I_H5_DX_WRITE_DELAYS                               \
+       {{  1,  2,  3,  4,  3,  4,  4,  4,  6,  6,  6 },        \
+        {  6,  6,  6,  5,  5,  5,  5,  5,  6,  6,  6 },        \
+        {  0,  2,  4,  2,  6,  5,  5,  5,  6,  6,  6 },        \
+        {  3,  3,  3,  2,  2,  1,  1,  1,  4,  4,  4 } }
+#define SUN8I_H5_AC_DELAYS                                     \
+       {  0,  0,  5,  5,  0,  0,  0,  0,                       \
+          0,  0,  0,  0,  3,  3,  3,  3,                       \
+          3,  3,  3,  3,  3,  3,  3,  3,                       \
+          3,  3,  3,  3,  2,  0,  0      }
+
 unsigned long sunxi_dram_init(void)
 {
        struct sunxi_mctl_com_reg * const mctl_com =
@@ -625,6 +683,10 @@ unsigned long sunxi_dram_init(void)
                .dx_read_delays  = SUN50I_A64_DX_READ_DELAYS,
                .dx_write_delays = SUN50I_A64_DX_WRITE_DELAYS,
                .ac_delays       = SUN50I_A64_AC_DELAYS,
+#elif defined(CONFIG_MACH_SUN50I_H5)
+               .dx_read_delays  = SUN8I_H5_DX_READ_DELAYS,
+               .dx_write_delays = SUN8I_H5_DX_WRITE_DELAYS,
+               .ac_delays       = SUN8I_H5_AC_DELAYS,
 #endif
        };
 /*
@@ -636,6 +698,8 @@ unsigned long sunxi_dram_init(void)
        uint16_t socid = SOCID_H3;
 #elif defined(CONFIG_MACH_SUN50I)
        uint16_t socid = SOCID_A64;
+#elif defined(CONFIG_MACH_SUN50I_H5)
+       uint16_t socid = SOCID_H5;
 #endif

        mctl_sys_init(socid, &para);
@@ -652,8 +716,9 @@ unsigned long sunxi_dram_init(void)
        if (socid ==

Comments

Chen-Yu Tsai Feb. 3, 2017, 4:36 p.m. UTC | #1
Hi,

On Fri, Feb 3, 2017 at 11:26 PM, Jagan Teki <jagan@openedev.com> wrote:
> On Feb 1, 2017 2:37 AM, "Andre Przywara" <andre.przywara@arm.com> wrote:
>
> The DRAM controller in the Allwinner H5 SoC is again very similar to
> the one in the H3 and A64.
> Based on the existing socid parameter, add support for this controller
> by reusing the bulk of the code and only deviating where needed.
> These new bits set or cleared here and there have been mostly found by
> looking at DRAM register dumps after using the H5 boot0 and comparing
> them to what we set in the code. So for now it's mostly unclear what
> those bits actually mean - hence the missing names and comments.
> Also add the delay line parameters taken from the boot0 and libdram
> disassembly.
>
>
> Can you split this patch with delay line params as separate patch.

It looks like the delay lines are for the H5, merely taken from
different sources. They are and should be part of the same patch
adding support for DRAM on the H5.

>
> Register setup differences between H5 and H3 are courtesy of Jens Kuske.
>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  arch/arm/include/asm/arch-sunxi/cpu.h |  1 +
>  arch/arm/mach-sunxi/dram_sun8i_h3.c   | 97
> +++++++++++++++++++++++++++++------
>  2 files changed, 82 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm/include/asm/arch-sunxi/cpu.h
> b/arch/arm/include/asm/arch-sunxi/cpu.h
> index 6f96a97..e8e670e 100644
> --- a/arch/arm/include/asm/arch-sunxi/cpu.h
> +++ b/arch/arm/include/asm/arch-sunxi/cpu.h
> @@ -15,5 +15,6 @@
>
>  #define SOCID_A64      0x1689
>  #define SOCID_H3       0x1680
> +#define SOCID_H5       0x1718
>
>  #endif /* _SUNXI_CPU_H */
> diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c
> b/arch/arm/mach-sunxi/dram_sun8i_h3.c
> index 9f7cc7f..d681a9d 100644
> --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
> +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
> @@ -177,6 +177,34 @@ static void mctl_set_master_priority_a64(void)
>         writel(0x81000004, &mctl_com->mdfs_bwlr[2]);
>  }
>
> +static void mctl_set_master_priority_h5(void)
> +{
> +       struct sunxi_mctl_com_reg * const mctl_com =
> +                       (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
> +
> +       /* enable bandwidth limit windows and set windows size 1us */
> +       writel(399, &mctl_com->tmr);
> +       writel((1 << 16), &mctl_com->bwcr);
>
>
> I'm not fond of using direct numerical values make code unhealthy please use
> macros with bitops. Note that this comment will apply rest of the code where
> it applies.

I think you are being unreasonable. The commit message clearly states that
the added code either comes from register dumps, disassembled blobs, or
comparison of released code:

"""
These new bits set or cleared here and there have been mostly found by
looking at DRAM register dumps after using the H5 boot0 and comparing
them to what we set in the code. So for now it's mostly unclear what
those bits actually mean - hence the missing names and comments.
"""

For this particular instance, see

https://github.com/BPI-SINOVOIP/BPI-M2U-bsp/blob/master/u-boot-sunxi/arch/arm/cpu/armv7/sun8iw11p1/dram/lib-dram/mctl_hal.c#L197

which gives the exact same comment, and no named macros. Adding a macro
for it and calling it H5_DRAM_BW_UNKNOWN_MAGIC is not an improvement.
Same goes for the next few lines.

Allwinner has _never_ released documents for the DRAM controllers or DRAM PHYs,
and only sometimes releases code for DRAM init for some SoCs, sometimes with
questionable licenses (or lack of), of which some don't match what is actually
seen in provided blobs. Considering what the community has access to. This
patch seems to be quite good.

Regards
ChenYu

>
> +
> +       /* set cpu high priority */
> +       writel(0x00000001, &mctl_com->mapr);
> +
> +       /* Port 2 is reserved per Allwinner's linux-3.10 source, yet
> +        * they initialise it */
> +       MBUS_CONF(   CPU, true, HIGHEST, 0,  300,  260,  150);
> +       MBUS_CONF(   GPU, true, HIGHEST, 0,  600,  400,  200);
> +       MBUS_CONF(UNUSED, true, HIGHEST, 0,  512,  256,   96);
> +       MBUS_CONF(   DMA, true, HIGHEST, 0,  256,  128,   32);
> +       MBUS_CONF(    VE, true, HIGHEST, 0, 1900, 1500, 1000);
> +       MBUS_CONF(   CSI, true, HIGHEST, 0,  150,  120,  100);
> +       MBUS_CONF(  NAND, true,    HIGH, 0,  256,  128,   64);
> +       MBUS_CONF(    SS, true, HIGHEST, 0,  256,  128,   64);
> +       MBUS_CONF(    TS, true, HIGHEST, 0,  256,  128,   64);
> +       MBUS_CONF(    DI, true,    HIGH, 0, 1024,  256,   64);
> +       MBUS_CONF(    DE, true, HIGHEST, 3, 3400, 2400, 1024);
> +       MBUS_CONF(DE_CFD, true, HIGHEST, 0,  600,  400,  200);
> +}
> +
>  static void mctl_set_master_priority(uint16_t socid)
>  {
>         switch (socid) {
> @@ -186,6 +214,9 @@ static void mctl_set_master_priority(uint16_t socid)
>         case SOCID_A64:
>                 mctl_set_master_priority_a64();
>                 return;
> +       case SOCID_H5:
> +               mctl_set_master_priority_h5();
> +               return;
>         }
>  }
>
> @@ -256,7 +287,7 @@ static void mctl_set_timing_params(uint16_t socid,
> struct dram_para *para)
>
>         /* set two rank timing */
>         clrsetbits_le32(&mctl_ctl->dramtmg[8], (0xff << 8) | (0xff << 0),
> -                       (0x66 << 8) | (0x10 << 0));
> +                       ((socid == SOCID_H5 ? 0x33 : 0x66) << 8) | (0x10 <<
> 0));
>
>         /* set PHY interface timing, write latency and read latency
> configure */
>         writel((0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8) |
> @@ -391,7 +422,7 @@ static void mctl_sys_init(uint16_t socid, struct
> dram_para *para)
>                                 CCM_DRAMCLK_CFG_DIV(1) |
>                                 CCM_DRAMCLK_CFG_SRC_PLL11 |
>                                 CCM_DRAMCLK_CFG_UPD);
> -       } else if (socid == SOCID_H3) {
> +       } else if (socid == SOCID_H3 || socid == SOCID_H5) {
>                 clock_set_pll5(CONFIG_DRAM_CLK * 2 * 1000000, false);
>                 clrsetbits_le32(&ccm->dram_clk_cfg,
>                                 CCM_DRAMCLK_CFG_DIV_MASK |
> @@ -410,7 +441,7 @@ static void mctl_sys_init(uint16_t socid, struct
> dram_para *para)
>         setbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_RST);
>         udelay(10);
>
> -       writel(0xc00e, &mctl_ctl->clken);
> +       writel(socid == SOCID_H5 ? 0x8000 : 0xc00e, &mctl_ctl->clken);
>         udelay(500);
>  }
>
> @@ -434,7 +465,10 @@ static int mctl_channel_init(uint16_t socid, struct
> dram_para *para)
>
>         /* setting VTC, default disable all VT */
>         clrbits_le32(&mctl_ctl->pgcr[0], (1 << 30) | 0x3f);
> -       clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);
> +       if (socid == SOCID_H5)
> +               setbits_le32(&mctl_ctl->pgcr[1], (1 << 24) | (1 << 26));
> +       else
> +               clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);
>
>         /* increase DFI_PHY_UPD clock */
>         writel(PROTECT_MAGIC, &mctl_com->protect);
> @@ -444,15 +478,22 @@ static int mctl_channel_init(uint16_t socid, struct
> dram_para *para)
>         udelay(100);
>
>         /* set dramc odt */
> -       for (i = 0; i < 4; i++)
> -               clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) |
> -                               (0x1 << 1) | (0x3 << 2) | (0x3 << 12) |
> -                               (0x3 << 14),
> -                               IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
> -                                       DX_GCR_ODT_DYNAMIC :
> DX_GCR_ODT_OFF);
> +       for (i = 0; i < 4; i++) {
> +               u32 clearmask = (0x3 << 4) | (0x1 << 1) | (0x3 << 2) |
> +                               (0x3 << 12) | (0x3 << 14);
> +               u32 setmask = IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
> +                               DX_GCR_ODT_DYNAMIC : DX_GCR_ODT_OFF;
> +
> +               if (socid == SOCID_H5) {
> +                       clearmask |= 0x2 << 8;
> +                       setmask |= 0x4 << 8;
> +               }
> +               clrsetbits_le32(&mctl_ctl->dx[i].gcr, clearmask, setmask);
> +       }
>
>         /* AC PDR should always ON */
> -       setbits_le32(&mctl_ctl->aciocr, 0x1 << 1);
> +       clrsetbits_le32(&mctl_ctl->aciocr, socid == SOCID_H5 ? (0x1 << 11) :
> 0,
> +                       0x1 << 1);
>
>         /* set DQS auto gating PD mode */
>         setbits_le32(&mctl_ctl->pgcr[2], 0x3 << 6);
> @@ -464,7 +505,7 @@ static int mctl_channel_init(uint16_t socid, struct
> dram_para *para)
>                 /* dphy & aphy phase select 270 degree */
>                 clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
> 8),
>                                 (0x1 << 10) | (0x2 << 8));
> -       } else if (socid == SOCID_A64) {
> +       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
>                 /* dphy & aphy phase select ? */
>                 clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
> 8),
>                                 (0x0 << 10) | (0x3 << 8));
> @@ -488,11 +529,12 @@ static int mctl_channel_init(uint16_t socid, struct
> dram_para *para)
>
>                 mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST |
>                               PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
> -       } else if (socid == SOCID_A64) {
> +       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
>                 clrsetbits_le32(&mctl_ctl->zqcr, 0xffffff, CONFIG_DRAM_ZQ);
>
>                 mctl_phy_init(PIR_ZCAL | PIR_PLLINIT | PIR_DCAL | PIR_PHYRST
> |
>                               PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
> +               /* no PIR_QSGATE for H5 ???? */
>         }
>
>         /* detect ranks and bus width */
> @@ -533,7 +575,7 @@ static int mctl_channel_init(uint16_t socid, struct
> dram_para *para)
>         /* set PGCR3, CKE polarity */
>         if (socid == SOCID_H3)
>                 writel(0x00aa0060, &mctl_ctl->pgcr[3]);
> -       else if (socid == SOCID_A64)
> +       else if (socid == SOCID_A64 || socid == SOCID_H5)
>                 writel(0xc0aa0060, &mctl_ctl->pgcr[3]);
>
>         /* power down zq calibration module for power save */
> @@ -604,6 +646,22 @@ static void mctl_auto_detect_dram_size(struct dram_para
> *para)
>            3,  4,  0,  3,  4,  1,  4,  0,                       \
>            1,  1,  0,  1, 13,  5,  4      }
>
> +#define SUN8I_H5_DX_READ_DELAYS                                        \
> +       {{ 14, 15, 17, 17, 17, 17, 17, 18, 17,  3,  3 },        \
> +        { 21, 21, 12, 22, 21, 21, 21, 21, 21,  3,  3 },        \
> +        { 16, 19, 19, 17, 22, 22, 21, 22, 19,  3,  3 },        \
> +        { 21, 21, 22, 22, 20, 21, 19, 19, 19,  3,  3 } }
> +#define SUN8I_H5_DX_WRITE_DELAYS                               \
> +       {{  1,  2,  3,  4,  3,  4,  4,  4,  6,  6,  6 },        \
> +        {  6,  6,  6,  5,  5,  5,  5,  5,  6,  6,  6 },        \
> +        {  0,  2,  4,  2,  6,  5,  5,  5,  6,  6,  6 },        \
> +        {  3,  3,  3,  2,  2,  1,  1,  1,  4,  4,  4 } }
> +#define SUN8I_H5_AC_DELAYS                                     \
> +       {  0,  0,  5,  5,  0,  0,  0,  0,                       \
> +          0,  0,  0,  0,  3,  3,  3,  3,                       \
> +          3,  3,  3,  3,  3,  3,  3,  3,                       \
> +          3,  3,  3,  3,  2,  0,  0      }
> +
>  unsigned long sunxi_dram_init(void)
>  {
>         struct sunxi_mctl_com_reg * const mctl_com =
> @@ -625,6 +683,10 @@ unsigned long sunxi_dram_init(void)
>                 .dx_read_delays  = SUN50I_A64_DX_READ_DELAYS,
>                 .dx_write_delays = SUN50I_A64_DX_WRITE_DELAYS,
>                 .ac_delays       = SUN50I_A64_AC_DELAYS,
> +#elif defined(CONFIG_MACH_SUN50I_H5)
> +               .dx_read_delays  = SUN8I_H5_DX_READ_DELAYS,
> +               .dx_write_delays = SUN8I_H5_DX_WRITE_DELAYS,
> +               .ac_delays       = SUN8I_H5_AC_DELAYS,
>  #endif
>         };
>  /*
> @@ -636,6 +698,8 @@ unsigned long sunxi_dram_init(void)
>         uint16_t socid = SOCID_H3;
>  #elif defined(CONFIG_MACH_SUN50I)
>         uint16_t socid = SOCID_A64;
> +#elif defined(CONFIG_MACH_SUN50I_H5)
> +       uint16_t socid = SOCID_H5;
>  #endif
>
>         mctl_sys_init(socid, &para);
> @@ -652,8 +716,9 @@ unsigned long sunxi_dram_init(void)
>         if (socid ==
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Andre Przywara Feb. 6, 2017, 10:12 a.m. UTC | #2
Hi,

Chen-Yu, thanks for your comments.

On 03/02/17 16:36, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, Feb 3, 2017 at 11:26 PM, Jagan Teki <jagan@openedev.com> wrote:
>> On Feb 1, 2017 2:37 AM, "Andre Przywara" <andre.przywara@arm.com> wrote:
>>
>> The DRAM controller in the Allwinner H5 SoC is again very similar to
>> the one in the H3 and A64.
>> Based on the existing socid parameter, add support for this controller
>> by reusing the bulk of the code and only deviating where needed.
>> These new bits set or cleared here and there have been mostly found by
>> looking at DRAM register dumps after using the H5 boot0 and comparing
>> them to what we set in the code. So for now it's mostly unclear what
>> those bits actually mean - hence the missing names and comments.
>> Also add the delay line parameters taken from the boot0 and libdram
>> disassembly.
>>
>>
>> Can you split this patch with delay line params as separate patch.
> 
> It looks like the delay lines are for the H5, merely taken from
> different sources. They are and should be part of the same patch
> adding support for DRAM on the H5.

Yeah, I think they really belong into this patch. I don't see how
separating them would help, short of creating bisectability problems.

>>
>> Register setup differences between H5 and H3 are courtesy of Jens Kuske.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>> Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> ---
>>  arch/arm/include/asm/arch-sunxi/cpu.h |  1 +
>>  arch/arm/mach-sunxi/dram_sun8i_h3.c   | 97
>> +++++++++++++++++++++++++++++------
>>  2 files changed, 82 insertions(+), 16 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/arch-sunxi/cpu.h
>> b/arch/arm/include/asm/arch-sunxi/cpu.h
>> index 6f96a97..e8e670e 100644
>> --- a/arch/arm/include/asm/arch-sunxi/cpu.h
>> +++ b/arch/arm/include/asm/arch-sunxi/cpu.h
>> @@ -15,5 +15,6 @@
>>
>>  #define SOCID_A64      0x1689
>>  #define SOCID_H3       0x1680
>> +#define SOCID_H5       0x1718
>>
>>  #endif /* _SUNXI_CPU_H */
>> diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> index 9f7cc7f..d681a9d 100644
>> --- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> +++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
>> @@ -177,6 +177,34 @@ static void mctl_set_master_priority_a64(void)
>>         writel(0x81000004, &mctl_com->mdfs_bwlr[2]);
>>  }
>>
>> +static void mctl_set_master_priority_h5(void)
>> +{
>> +       struct sunxi_mctl_com_reg * const mctl_com =
>> +                       (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
>> +
>> +       /* enable bandwidth limit windows and set windows size 1us */
>> +       writel(399, &mctl_com->tmr);
>> +       writel((1 << 16), &mctl_com->bwcr);
>>
>>
>> I'm not fond of using direct numerical values make code unhealthy please use
>> macros with bitops. Note that this comment will apply rest of the code where
>> it applies.
> 
> I think you are being unreasonable. The commit message clearly states that
> the added code either comes from register dumps, disassembled blobs, or
> comparison of released code:
> 
> """
> These new bits set or cleared here and there have been mostly found by
> looking at DRAM register dumps after using the H5 boot0 and comparing
> them to what we set in the code. So for now it's mostly unclear what
> those bits actually mean - hence the missing names and comments.
> """
> 
> For this particular instance, see
> 
> https://github.com/BPI-SINOVOIP/BPI-M2U-bsp/blob/master/u-boot-sunxi/arch/arm/cpu/armv7/sun8iw11p1/dram/lib-dram/mctl_hal.c#L197
> 
> which gives the exact same comment, and no named macros. Adding a macro
> for it and calling it H5_DRAM_BW_UNKNOWN_MAGIC is not an improvement.
> Same goes for the next few lines.
> 
> Allwinner has _never_ released documents for the DRAM controllers or DRAM PHYs,
> and only sometimes releases code for DRAM init for some SoCs, sometimes with
> questionable licenses (or lack of), of which some don't match what is actually
> seen in provided blobs. Considering what the community has access to. This
> patch seems to be quite good.

I think we have some sort of rewrite of this code ahead of us anyway,
hopefully we can address some of these points then in a reasonable manner.
But until then I'd like to keep it at "399" instead of using
WINDOW_SIZE_1US or _guessing_ how this is computed from some frequency.

Cheers,
Andre.

>>
>> +
>> +       /* set cpu high priority */
>> +       writel(0x00000001, &mctl_com->mapr);
>> +
>> +       /* Port 2 is reserved per Allwinner's linux-3.10 source, yet
>> +        * they initialise it */
>> +       MBUS_CONF(   CPU, true, HIGHEST, 0,  300,  260,  150);
>> +       MBUS_CONF(   GPU, true, HIGHEST, 0,  600,  400,  200);
>> +       MBUS_CONF(UNUSED, true, HIGHEST, 0,  512,  256,   96);
>> +       MBUS_CONF(   DMA, true, HIGHEST, 0,  256,  128,   32);
>> +       MBUS_CONF(    VE, true, HIGHEST, 0, 1900, 1500, 1000);
>> +       MBUS_CONF(   CSI, true, HIGHEST, 0,  150,  120,  100);
>> +       MBUS_CONF(  NAND, true,    HIGH, 0,  256,  128,   64);
>> +       MBUS_CONF(    SS, true, HIGHEST, 0,  256,  128,   64);
>> +       MBUS_CONF(    TS, true, HIGHEST, 0,  256,  128,   64);
>> +       MBUS_CONF(    DI, true,    HIGH, 0, 1024,  256,   64);
>> +       MBUS_CONF(    DE, true, HIGHEST, 3, 3400, 2400, 1024);
>> +       MBUS_CONF(DE_CFD, true, HIGHEST, 0,  600,  400,  200);
>> +}
>> +
>>  static void mctl_set_master_priority(uint16_t socid)
>>  {
>>         switch (socid) {
>> @@ -186,6 +214,9 @@ static void mctl_set_master_priority(uint16_t socid)
>>         case SOCID_A64:
>>                 mctl_set_master_priority_a64();
>>                 return;
>> +       case SOCID_H5:
>> +               mctl_set_master_priority_h5();
>> +               return;
>>         }
>>  }
>>
>> @@ -256,7 +287,7 @@ static void mctl_set_timing_params(uint16_t socid,
>> struct dram_para *para)
>>
>>         /* set two rank timing */
>>         clrsetbits_le32(&mctl_ctl->dramtmg[8], (0xff << 8) | (0xff << 0),
>> -                       (0x66 << 8) | (0x10 << 0));
>> +                       ((socid == SOCID_H5 ? 0x33 : 0x66) << 8) | (0x10 <<
>> 0));
>>
>>         /* set PHY interface timing, write latency and read latency
>> configure */
>>         writel((0x2 << 24) | (t_rdata_en << 16) | (0x1 << 8) |
>> @@ -391,7 +422,7 @@ static void mctl_sys_init(uint16_t socid, struct
>> dram_para *para)
>>                                 CCM_DRAMCLK_CFG_DIV(1) |
>>                                 CCM_DRAMCLK_CFG_SRC_PLL11 |
>>                                 CCM_DRAMCLK_CFG_UPD);
>> -       } else if (socid == SOCID_H3) {
>> +       } else if (socid == SOCID_H3 || socid == SOCID_H5) {
>>                 clock_set_pll5(CONFIG_DRAM_CLK * 2 * 1000000, false);
>>                 clrsetbits_le32(&ccm->dram_clk_cfg,
>>                                 CCM_DRAMCLK_CFG_DIV_MASK |
>> @@ -410,7 +441,7 @@ static void mctl_sys_init(uint16_t socid, struct
>> dram_para *para)
>>         setbits_le32(&ccm->dram_clk_cfg, CCM_DRAMCLK_CFG_RST);
>>         udelay(10);
>>
>> -       writel(0xc00e, &mctl_ctl->clken);
>> +       writel(socid == SOCID_H5 ? 0x8000 : 0xc00e, &mctl_ctl->clken);
>>         udelay(500);
>>  }
>>
>> @@ -434,7 +465,10 @@ static int mctl_channel_init(uint16_t socid, struct
>> dram_para *para)
>>
>>         /* setting VTC, default disable all VT */
>>         clrbits_le32(&mctl_ctl->pgcr[0], (1 << 30) | 0x3f);
>> -       clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);
>> +       if (socid == SOCID_H5)
>> +               setbits_le32(&mctl_ctl->pgcr[1], (1 << 24) | (1 << 26));
>> +       else
>> +               clrsetbits_le32(&mctl_ctl->pgcr[1], 1 << 24, 1 << 26);
>>
>>         /* increase DFI_PHY_UPD clock */
>>         writel(PROTECT_MAGIC, &mctl_com->protect);
>> @@ -444,15 +478,22 @@ static int mctl_channel_init(uint16_t socid, struct
>> dram_para *para)
>>         udelay(100);
>>
>>         /* set dramc odt */
>> -       for (i = 0; i < 4; i++)
>> -               clrsetbits_le32(&mctl_ctl->dx[i].gcr, (0x3 << 4) |
>> -                               (0x1 << 1) | (0x3 << 2) | (0x3 << 12) |
>> -                               (0x3 << 14),
>> -                               IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
>> -                                       DX_GCR_ODT_DYNAMIC :
>> DX_GCR_ODT_OFF);
>> +       for (i = 0; i < 4; i++) {
>> +               u32 clearmask = (0x3 << 4) | (0x1 << 1) | (0x3 << 2) |
>> +                               (0x3 << 12) | (0x3 << 14);
>> +               u32 setmask = IS_ENABLED(CONFIG_DRAM_ODT_EN) ?
>> +                               DX_GCR_ODT_DYNAMIC : DX_GCR_ODT_OFF;
>> +
>> +               if (socid == SOCID_H5) {
>> +                       clearmask |= 0x2 << 8;
>> +                       setmask |= 0x4 << 8;
>> +               }
>> +               clrsetbits_le32(&mctl_ctl->dx[i].gcr, clearmask, setmask);
>> +       }
>>
>>         /* AC PDR should always ON */
>> -       setbits_le32(&mctl_ctl->aciocr, 0x1 << 1);
>> +       clrsetbits_le32(&mctl_ctl->aciocr, socid == SOCID_H5 ? (0x1 << 11) :
>> 0,
>> +                       0x1 << 1);
>>
>>         /* set DQS auto gating PD mode */
>>         setbits_le32(&mctl_ctl->pgcr[2], 0x3 << 6);
>> @@ -464,7 +505,7 @@ static int mctl_channel_init(uint16_t socid, struct
>> dram_para *para)
>>                 /* dphy & aphy phase select 270 degree */
>>                 clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
>> 8),
>>                                 (0x1 << 10) | (0x2 << 8));
>> -       } else if (socid == SOCID_A64) {
>> +       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
>>                 /* dphy & aphy phase select ? */
>>                 clrsetbits_le32(&mctl_ctl->pgcr[2], (0x3 << 10) | (0x3 <<
>> 8),
>>                                 (0x0 << 10) | (0x3 << 8));
>> @@ -488,11 +529,12 @@ static int mctl_channel_init(uint16_t socid, struct
>> dram_para *para)
>>
>>                 mctl_phy_init(PIR_PLLINIT | PIR_DCAL | PIR_PHYRST |
>>                               PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
>> -       } else if (socid == SOCID_A64) {
>> +       } else if (socid == SOCID_A64 || socid == SOCID_H5) {
>>                 clrsetbits_le32(&mctl_ctl->zqcr, 0xffffff, CONFIG_DRAM_ZQ);
>>
>>                 mctl_phy_init(PIR_ZCAL | PIR_PLLINIT | PIR_DCAL | PIR_PHYRST
>> |
>>                               PIR_DRAMRST | PIR_DRAMINIT | PIR_QSGATE);
>> +               /* no PIR_QSGATE for H5 ???? */
>>         }
>>
>>         /* detect ranks and bus width */
>> @@ -533,7 +575,7 @@ static int mctl_channel_init(uint16_t socid, struct
>> dram_para *para)
>>         /* set PGCR3, CKE polarity */
>>         if (socid == SOCID_H3)
>>                 writel(0x00aa0060, &mctl_ctl->pgcr[3]);
>> -       else if (socid == SOCID_A64)
>> +       else if (socid == SOCID_A64 || socid == SOCID_H5)
>>                 writel(0xc0aa0060, &mctl_ctl->pgcr[3]);
>>
>>         /* power down zq calibration module for power save */
>> @@ -604,6 +646,22 @@ static void mctl_auto_detect_dram_size(struct dram_para
>> *para)
>>            3,  4,  0,  3,  4,  1,  4,  0,                       \
>>            1,  1,  0,  1, 13,  5,  4      }
>>
>> +#define SUN8I_H5_DX_READ_DELAYS                                        \
>> +       {{ 14, 15, 17, 17, 17, 17, 17, 18, 17,  3,  3 },        \
>> +        { 21, 21, 12, 22, 21, 21, 21, 21, 21,  3,  3 },        \
>> +        { 16, 19, 19, 17, 22, 22, 21, 22, 19,  3,  3 },        \
>> +        { 21, 21, 22, 22, 20, 21, 19, 19, 19,  3,  3 } }
>> +#define SUN8I_H5_DX_WRITE_DELAYS                               \
>> +       {{  1,  2,  3,  4,  3,  4,  4,  4,  6,  6,  6 },        \
>> +        {  6,  6,  6,  5,  5,  5,  5,  5,  6,  6,  6 },        \
>> +        {  0,  2,  4,  2,  6,  5,  5,  5,  6,  6,  6 },        \
>> +        {  3,  3,  3,  2,  2,  1,  1,  1,  4,  4,  4 } }
>> +#define SUN8I_H5_AC_DELAYS                                     \
>> +       {  0,  0,  5,  5,  0,  0,  0,  0,                       \
>> +          0,  0,  0,  0,  3,  3,  3,  3,                       \
>> +          3,  3,  3,  3,  3,  3,  3,  3,                       \
>> +          3,  3,  3,  3,  2,  0,  0      }
>> +
>>  unsigned long sunxi_dram_init(void)
>>  {
>>         struct sunxi_mctl_com_reg * const mctl_com =
>> @@ -625,6 +683,10 @@ unsigned long sunxi_dram_init(void)
>>                 .dx_read_delays  = SUN50I_A64_DX_READ_DELAYS,
>>                 .dx_write_delays = SUN50I_A64_DX_WRITE_DELAYS,
>>                 .ac_delays       = SUN50I_A64_AC_DELAYS,
>> +#elif defined(CONFIG_MACH_SUN50I_H5)
>> +               .dx_read_delays  = SUN8I_H5_DX_READ_DELAYS,
>> +               .dx_write_delays = SUN8I_H5_DX_WRITE_DELAYS,
>> +               .ac_delays       = SUN8I_H5_AC_DELAYS,
>>  #endif
>>         };
>>  /*
>> @@ -636,6 +698,8 @@ unsigned long sunxi_dram_init(void)
>>         uint16_t socid = SOCID_H3;
>>  #elif defined(CONFIG_MACH_SUN50I)
>>         uint16_t socid = SOCID_A64;
>> +#elif defined(CONFIG_MACH_SUN50I_H5)
>> +       uint16_t socid = SOCID_H5;
>>  #endif
>>
>>         mctl_sys_init(socid, &para);
>> @@ -652,8 +716,9 @@ unsigned long sunxi_dram_init(void)
>>         if (socid ==
>>
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "linux-sunxi" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to linux-sunxi+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-sunxi/cpu.h
b/arch/arm/include/asm/arch-sunxi/cpu.h
index 6f96a97..e8e670e 100644
--- a/arch/arm/include/asm/arch-sunxi/cpu.h
+++ b/arch/arm/include/asm/arch-sunxi/cpu.h
@@ -15,5 +15,6 @@ 

 #define SOCID_A64      0x1689
 #define SOCID_H3       0x1680
+#define SOCID_H5       0x1718

 #endif /* _SUNXI_CPU_H */
diff --git a/arch/arm/mach-sunxi/dram_sun8i_h3.c b/arch/arm/mach-sunxi/dram_
sun8i_h3.c
index 9f7cc7f..d681a9d 100644
--- a/arch/arm/mach-sunxi/dram_sun8i_h3.c
+++ b/arch/arm/mach-sunxi/dram_sun8i_h3.c
@@ -177,6 +177,34 @@  static void mctl_set_master_priority_a64(void)
        writel(0x81000004, &mctl_com->mdfs_bwlr[2]);
 }

+static void mctl_set_master_priority_h5(void)
+{
+       struct sunxi_mctl_com_reg * const mctl_com =
+                       (struct sunxi_mctl_com_reg *)SUNXI_DRAM_COM_BASE;
+
+       /* enable bandwidth limit windows and set windows size 1us */
+       writel(399, &mctl_com->tmr);
+       writel((1 << 16), &mctl_com->bwcr);


I'm not fond of using direct numerical values make code unhealthy please
use macros with bitops. Note that this comment will apply rest of the code
where it applies.

+
+       /* set cpu high priority */
+       writel(0x00000001, &mctl_com->mapr);
+
+       /* Port 2 is reserved per Allwinner's linux-3.10 source, yet
+        * they initialise it */
+       MBUS_CONF(   CPU, true, HIGHEST, 0,  300,  260,  150);
+       MBUS_CONF(   GPU, true, HIGHEST, 0,  600,  400,  200);
+       MBUS_CONF(UNUSED, true, HIGHEST, 0,  512,  256,   96);
+       MBUS_CONF(   DMA, true, HIGHEST, 0,  256,  128,   32);
+       MBUS_CONF(    VE, true, HIGHEST, 0, 1900, 1500, 1000);
+       MBUS_CONF(   CSI, true, HIGHEST, 0,  150,  120,  100);
+       MBUS_CONF(  NAND, true,    HIGH, 0,  256,  128,   64);
+       MBUS_CONF(    SS, true, HIGHEST, 0,  256,  128,   64);
+       MBUS_CONF(    TS, true, HIGHEST, 0,  256,  128,   64);
+       MBUS_CONF(    DI, true,    HIGH, 0, 1024,  256,   64);
+       MBUS_CONF(    DE, true, HIGHEST, 3, 3400, 2400, 1024);