| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <24752023.ouqheUzb2q@diego>
Date: Sat, 06 Sep 2025 20:38:07 +0200
From: Heiko Stübner <heiko@...ech.de>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
Chanwoo Choi <chanwoo@...nel.org>
Cc: Chanwoo Choi <cw00.choi@...sung.com>,
MyungJoo Ham <myungjoo.ham@...sung.com>,
Kyungmin Park <kyungmin.park@...sung.com>,
Sascha Hauer <s.hauer@...gutronix.de>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Sebastian Reichel <sebastian.reichel@...labora.com>, kernel@...labora.com,
linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] PM / devfreq: rockchip-dfi: add support for LPDDR5
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@...labora.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@...labora.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, ®2);
> > regmap_read(regmap_pmu, RK3588_PMUGRF_OS_REG3, ®3);
> > @@ -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, ®6);
> > + 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
> >
> >
>
>
>
Powered by blists - more mailing lists