Re: [PATCH 2/2] PM / devfreq: rockchip-dfi: add support for LPDDR5

["Heiko Stübner" <heiko@sntech.de>]

Hi,

Am Samstag, 6. September 2025, 18:09:53 Mitteleuropäische Sommerzeit schrieb Chanwoo Choi:
> I'm sorry for late reply.
> 
> Looks good to me. But, this patch contains the change of following header.
> If there are ACK about change in include/soc/rockchip, I'll merge this series.
> 
> include/soc/rockchip/rk3588_grf.h    |  8 +++-
> include/soc/rockchip/rockchip_grf.h  |  1 +

done :-)

Thanks
Heiko


> On Fri, May 30, 2025 at 10:39 PM Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> >
> > The Rockchip RK3588 SoC can also support LPDDR5 memory. This type of
> > memory needs some special case handling in the rockchip-dfi driver.
> >
> > Add support for it in rockchip-dfi, as well as the needed GRF register
> > definitions.
> >
> > This has been tested as returning both the right cycle count and
> > bandwidth on a LPDDR5 board where the CKR bit is 1. I couldn't test
> > whether the values are correct on a system where CKR is 0, as I'm not
> > savvy enough with the Rockchip tooling to know whether this can be set
> > in the DDR init blob.
> >
> > Downstream has some special case handling for a hardware version where
> > not just the control bits differ, but also the register. Since I don't
> > know whether that hardware version is in any production silicon, it's
> > left unimplemented for now, with an error message urging users to report
> > if they have such a system.
> >
> > There is a slight change of behaviour for non-LPDDR5 systems: instead of
> > writing 0 as the control flags to the control register and pretending
> > everything is alright if the memory type is unknown, we now explicitly
> > return an error.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/devfreq/event/rockchip-dfi.c | 84 ++++++++++++++++++++++++++++--------
> >  include/soc/rockchip/rk3588_grf.h    |  8 +++-
> >  include/soc/rockchip/rockchip_grf.h  |  1 +
> >  3 files changed, 73 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> > index 54effb63519653d20b40eed88681330983399a77..5a2c9badcc64c552303c2f55c52e5420dec5ffc1 100644
> > --- a/drivers/devfreq/event/rockchip-dfi.c
> > +++ b/drivers/devfreq/event/rockchip-dfi.c
> > @@ -34,15 +34,18 @@
> >
> >  /* DDRMON_CTRL */
> >  #define DDRMON_CTRL    0x04
> > +#define DDRMON_CTRL_LPDDR5             BIT(6)
> >  #define DDRMON_CTRL_DDR4               BIT(5)
> >  #define DDRMON_CTRL_LPDDR4             BIT(4)
> >  #define DDRMON_CTRL_HARDWARE_EN                BIT(3)
> >  #define DDRMON_CTRL_LPDDR23            BIT(2)
> >  #define DDRMON_CTRL_SOFTWARE_EN                BIT(1)
> >  #define DDRMON_CTRL_TIMER_CNT_EN       BIT(0)
> > -#define DDRMON_CTRL_DDR_TYPE_MASK      (DDRMON_CTRL_DDR4 | \
> > +#define DDRMON_CTRL_DDR_TYPE_MASK      (DDRMON_CTRL_LPDDR5 | \
> > +                                        DDRMON_CTRL_DDR4 | \
> >                                          DDRMON_CTRL_LPDDR4 | \
> >                                          DDRMON_CTRL_LPDDR23)
> > +#define DDRMON_CTRL_LP5_BANK_MODE_MASK GENMASK(8, 7)
> >
> >  #define DDRMON_CH0_WR_NUM              0x20
> >  #define DDRMON_CH0_RD_NUM              0x24
> > @@ -116,13 +119,60 @@ struct rockchip_dfi {
> >         int buswidth[DMC_MAX_CHANNELS];
> >         int ddrmon_stride;
> >         bool ddrmon_ctrl_single;
> > +       u32 lp5_bank_mode;
> > +       bool lp5_ckr;   /* true if in 4:1 command-to-data clock ratio mode */
> >         unsigned int count_multiplier;  /* number of data clocks per count */
> >  };
> >
> > +static int rockchip_dfi_ddrtype_to_ctrl(struct rockchip_dfi *dfi, u32 *ctrl,
> > +                                       u32 *mask)
> > +{
> > +       u32 ddrmon_ver;
> > +
> > +       *mask = DDRMON_CTRL_DDR_TYPE_MASK;
> > +
> > +       switch (dfi->ddr_type) {
> > +       case ROCKCHIP_DDRTYPE_LPDDR2:
> > +       case ROCKCHIP_DDRTYPE_LPDDR3:
> > +               *ctrl = DDRMON_CTRL_LPDDR23;
> > +               break;
> > +       case ROCKCHIP_DDRTYPE_LPDDR4:
> > +       case ROCKCHIP_DDRTYPE_LPDDR4X:
> > +               *ctrl = DDRMON_CTRL_LPDDR4;
> > +               break;
> > +       case ROCKCHIP_DDRTYPE_LPDDR5:
> > +               ddrmon_ver = readl_relaxed(dfi->regs);
> > +               if (ddrmon_ver < 0x40) {
> > +                       *ctrl = DDRMON_CTRL_LPDDR5 | dfi->lp5_bank_mode;
> > +                       *mask |= DDRMON_CTRL_LP5_BANK_MODE_MASK;
> > +                       break;
> > +               }
> > +
> > +               /*
> > +                * As it is unknown whether the unpleasant special case
> > +                * behaviour used by the vendor kernel is needed for any
> > +                * shipping hardware, ask users to report if they have
> > +                * some of that hardware.
> > +                */
> > +               dev_err(&dfi->edev->dev,
> > +                       "unsupported DDRMON version 0x%04X, please let linux-rockchip know!\n",
> > +                       ddrmon_ver);
> > +               return -EOPNOTSUPP;
> > +       default:
> > +               dev_err(&dfi->edev->dev, "unsupported memory type 0x%X\n",
> > +                       dfi->ddr_type);
> > +               return -EOPNOTSUPP;
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
> >  {
> >         void __iomem *dfi_regs = dfi->regs;
> >         int i, ret = 0;
> > +       u32 ctrl;
> > +       u32 ctrl_mask;
> >
> >         mutex_lock(&dfi->mutex);
> >
> > @@ -136,8 +186,11 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
> >                 goto out;
> >         }
> >
> > +       ret = rockchip_dfi_ddrtype_to_ctrl(dfi, &ctrl, &ctrl_mask);
> > +       if (ret)
> > +               goto out;
> > +
> >         for (i = 0; i < dfi->max_channels; i++) {
> > -               u32 ctrl = 0;
> >
> >                 if (!(dfi->channel_mask & BIT(i)))
> >                         continue;
> > @@ -147,21 +200,7 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
> >                                DDRMON_CTRL_SOFTWARE_EN | DDRMON_CTRL_HARDWARE_EN),
> >                                dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL);
> >
> > -               /* set ddr type to dfi */
> > -               switch (dfi->ddr_type) {
> > -               case ROCKCHIP_DDRTYPE_LPDDR2:
> > -               case ROCKCHIP_DDRTYPE_LPDDR3:
> > -                       ctrl = DDRMON_CTRL_LPDDR23;
> > -                       break;
> > -               case ROCKCHIP_DDRTYPE_LPDDR4:
> > -               case ROCKCHIP_DDRTYPE_LPDDR4X:
> > -                       ctrl = DDRMON_CTRL_LPDDR4;
> > -                       break;
> > -               default:
> > -                       break;
> > -               }
> > -
> > -               writel_relaxed(HIWORD_UPDATE(ctrl, DDRMON_CTRL_DDR_TYPE_MASK),
> > +               writel_relaxed(HIWORD_UPDATE(ctrl, ctrl_mask),
> >                                dfi_regs + i * dfi->ddrmon_stride + DDRMON_CTRL);
> >
> >                 /* enable count, use software mode */
> > @@ -652,6 +691,7 @@ static int rockchip_ddr_perf_init(struct rockchip_dfi *dfi)
> >                 break;
> >         case ROCKCHIP_DDRTYPE_LPDDR4:
> >         case ROCKCHIP_DDRTYPE_LPDDR4X:
> > +       case ROCKCHIP_DDRTYPE_LPDDR5:
> >                 dfi->burst_len = 16;
> >                 break;
> >         }
> > @@ -730,7 +770,7 @@ static int rk3568_dfi_init(struct rockchip_dfi *dfi)
> >  static int rk3588_dfi_init(struct rockchip_dfi *dfi)
> >  {
> >         struct regmap *regmap_pmu = dfi->regmap_pmu;
> > -       u32 reg2, reg3, reg4;
> > +       u32 reg2, reg3, reg4, reg6;
> >
> >         regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG2, &reg2);
> >         regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG3, &reg3);
> > @@ -757,6 +797,14 @@ static int rk3588_dfi_init(struct rockchip_dfi *dfi)
> >         dfi->ddrmon_stride = 0x4000;
> >         dfi->count_multiplier = 2;
> >
> > +       if (dfi->ddr_type == ROCKCHIP_DDRTYPE_LPDDR5) {
> > +               regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG6, &reg6);
> > +               dfi->lp5_bank_mode = FIELD_GET(RK3588_PMUGRF_OS_REG6_LP5_BANK_MODE, reg6) << 7;
> > +               dfi->lp5_ckr = FIELD_GET(RK3588_PMUGRF_OS_REG6_LP5_CKR, reg6);
> > +               if (dfi->lp5_ckr)
> > +                       dfi->count_multiplier *= 2;
> > +       }
> > +
> >         return 0;
> >  };
> >
> > diff --git a/include/soc/rockchip/rk3588_grf.h b/include/soc/rockchip/rk3588_grf.h
> > index 630b35a550640e57f1b5a50dfbe362653a7cbcc1..02a7b2432d9942e15a77424c44fefec189faaa33 100644
> > --- a/include/soc/rockchip/rk3588_grf.h
> > +++ b/include/soc/rockchip/rk3588_grf.h
> > @@ -12,7 +12,11 @@
> >  #define RK3588_PMUGRF_OS_REG3_DRAMTYPE_INFO_V3         GENMASK(13, 12)
> >  #define RK3588_PMUGRF_OS_REG3_SYSREG_VERSION           GENMASK(31, 28)
> >
> > -#define RK3588_PMUGRF_OS_REG4           0x210
> > -#define RK3588_PMUGRF_OS_REG5           0x214
> > +#define RK3588_PMUGRF_OS_REG4                          0x210
> > +#define RK3588_PMUGRF_OS_REG5                          0x214
> > +#define RK3588_PMUGRF_OS_REG6                          0x218
> > +#define RK3588_PMUGRF_OS_REG6_LP5_BANK_MODE            GENMASK(2, 1)
> > +/* Whether the LPDDR5 is in 2:1 (= 0) or 4:1 (= 1) CKR a.k.a. DQS mode */
> > +#define RK3588_PMUGRF_OS_REG6_LP5_CKR                  BIT(0)
> >
> >  #endif /* __SOC_RK3588_GRF_H */
> > diff --git a/include/soc/rockchip/rockchip_grf.h b/include/soc/rockchip/rockchip_grf.h
> > index e46fd72aea8d1f649768a3269b85176dacceef0e..41c7bb26fd5387df85e5b58186b67bf74706f360 100644
> > --- a/include/soc/rockchip/rockchip_grf.h
> > +++ b/include/soc/rockchip/rockchip_grf.h
> > @@ -13,6 +13,7 @@ enum {
> >         ROCKCHIP_DDRTYPE_LPDDR3 = 6,
> >         ROCKCHIP_DDRTYPE_LPDDR4 = 7,
> >         ROCKCHIP_DDRTYPE_LPDDR4X = 8,
> > +       ROCKCHIP_DDRTYPE_LPDDR5 = 9,
> >  };
> >
> >  #endif /* __SOC_ROCKCHIP_GRF_H */
> >
> > --
> > 2.49.0
> >
> >
> 
> 
>