Thread (12 messages) 12 messages, 5 authors, 2025-09-06

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

From: Sascha Hauer <s.hauer@pengutronix.de>
Date: 2025-06-11 07:33:02
Also in: linux-pm, linux-rockchip, lkml

On Fri, May 30, 2025 at 03:38:09PM +0200, Nicolas Frattaroli wrote:
quoted hunk ↗ jump to hunk
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(-)
@@ -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);
You could move the HIWORD_UPDATE(ctrl, ctrl_mask) to
rockchip_dfi_ddrtype_to_ctrl() and by that you only have to pass one
u32* to that function.

That's just nitpicking though, so for the series:

Reviewed-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help