Re: [PATCH v2 6/7] PM / devfreq: rockchip-dfi: prepare support for v4 DDRMON

[Choi Chanwoo <cwchoi00@gmail.com>]

On 9/22/2026 6:15 AM, Sebastian Reichel wrote:
> Add support for the new control register layout found for DDRMON
> controllers having version 0x40. This prepares the driver for
> adding RK3576 support.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  drivers/devfreq/event/rockchip-dfi.c | 85 ++++++++++++++++++++++++++----------
>  1 file changed, 62 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/devfreq/event/rockchip-dfi.c b/drivers/devfreq/event/rockchip-dfi.c
> index 8490219f61be..8ba97af78e3d 100644
> --- a/drivers/devfreq/event/rockchip-dfi.c
> +++ b/drivers/devfreq/event/rockchip-dfi.c
> @@ -42,6 +42,19 @@
>  #define DDRMON_CTRL_TIMER_CNT_EN	BIT(0)
>  #define DDRMON_CTRL_LP5_BANK_MODE_MASK	GENMASK(8, 7)
>  
> +#define DDRMON_CTRL0	0x04
> +#define DDRMON_CTRL0_EXT_TRIGGER_EN		BIT(6)
> +#define DDRMON_CTRL0_DDR4			BIT(5)
> +#define DDRMON_CTRL0_LPDDR4			BIT(4)
> +#define DDRMON_CTRL0_HARDWARE_EN		BIT(3)
> +#define DDRMON_CTRL0_LPDDR23			BIT(2)
> +#define DDRMON_CTRL0_SOFTWARE_EN		BIT(1)
> +#define DDRMON_CTRL0_TIMER_CNT_EN		BIT(0)
> +#define DDRMON_CTRL0_HOT_RANK			GENMASK(8, 7)
> +
> +#define DDRMON_CTRL1	0x08
> +#define DDRMON_CTRL1_LPDDR5			BIT(0)
> +#define DDRMON_CTRL1_LP5_BANK_MODE_MASK		GENMASK(2, 1)
>  
>  #define PERF_EVENT_CYCLES		0x0
>  #define PERF_EVENT_READ_BYTES		0x1
> @@ -143,8 +156,6 @@ struct rockchip_dfi_variant {
>  
>  static int rockchip_dfi_ddrtype_to_ctrl(struct rockchip_dfi *dfi, u32 *ctrl)
>  {
> -	u32 ddrmon_ver;
> -
>  	switch (dfi->ddr_type) {
>  	case ROCKCHIP_DDRTYPE_LPDDR2:
>  	case ROCKCHIP_DDRTYPE_LPDDR3:
> @@ -159,26 +170,43 @@ static int rockchip_dfi_ddrtype_to_ctrl(struct rockchip_dfi *dfi, u32 *ctrl)
>  			FIELD_PREP_WM16(DDRMON_CTRL_LPDDR5, 0);
>  		break;
>  	case ROCKCHIP_DDRTYPE_LPDDR5:
> -		ddrmon_ver = readl_relaxed(dfi->regs);
> -		if (ddrmon_ver < 0x40) {
> -			*ctrl = FIELD_PREP_WM16(DDRMON_CTRL_LPDDR23, 0) |
> -				FIELD_PREP_WM16(DDRMON_CTRL_LPDDR4, 0) |
> -				FIELD_PREP_WM16(DDRMON_CTRL_LPDDR5, 1) |
> -				FIELD_PREP_WM16(DDRMON_CTRL_LP5_BANK_MODE_MASK,
> -						dfi->lp5_bank_mode);
> -			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);
> +		*ctrl = FIELD_PREP_WM16(DDRMON_CTRL_LPDDR23, 0) |
> +			FIELD_PREP_WM16(DDRMON_CTRL_LPDDR4, 0) |
> +			FIELD_PREP_WM16(DDRMON_CTRL_LPDDR5, 1) |
> +			FIELD_PREP_WM16(DDRMON_CTRL_LP5_BANK_MODE_MASK,
> +					dfi->lp5_bank_mode);
> +		break;
> +	default:
> +		dev_err(&dfi->edev->dev, "unsupported memory type 0x%X\n",
> +			dfi->ddr_type);
>  		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int rockchip_dfi_v4_ddrtype_to_ctrl(struct rockchip_dfi *dfi, u32 *ctrl0, u32 *ctrl1)
> +{
> +	switch (dfi->ddr_type) {
> +	case ROCKCHIP_DDRTYPE_LPDDR2:
> +	case ROCKCHIP_DDRTYPE_LPDDR3:
> +		*ctrl0 = FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR23, 1) |
> +			 FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR4, 0);
> +		*ctrl1 = FIELD_PREP_WM16(DDRMON_CTRL1_LPDDR5, 0);
> +		break;
> +	case ROCKCHIP_DDRTYPE_LPDDR4:
> +	case ROCKCHIP_DDRTYPE_LPDDR4X:
> +		*ctrl0 = FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR23, 0) |
> +			 FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR4, 1);
> +		*ctrl1 = FIELD_PREP_WM16(DDRMON_CTRL1_LPDDR5, 0);
> +		break;
> +	case ROCKCHIP_DDRTYPE_LPDDR5:
> +		*ctrl0 = FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR23, 0) |
> +			 FIELD_PREP_WM16(DDRMON_CTRL0_LPDDR4, 0);
> +		*ctrl1 = FIELD_PREP_WM16(DDRMON_CTRL1_LPDDR5, 1) |
> +			 FIELD_PREP_WM16(DDRMON_CTRL_LP5_BANK_MODE_MASK,
> +					dfi->lp5_bank_mode);
> +		break;
>  	default:
>  		dev_err(&dfi->edev->dev, "unsupported memory type 0x%X\n",
>  			dfi->ddr_type);
> @@ -188,11 +216,12 @@ static int rockchip_dfi_ddrtype_to_ctrl(struct rockchip_dfi *dfi, u32 *ctrl)
>  	return 0;
>  }
>  
> +
>  static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
>  {
>  	void __iomem *dfi_regs = dfi->regs;
> +	u32 ctrl, ctrl1, ddrmon_ver;
>  	int i, ret = 0;
> -	u32 ctrl;
>  
>  	mutex_lock(&dfi->mutex);
>  
> @@ -207,10 +236,16 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
>  		goto out;
>  	}
>  
> -	ret = rockchip_dfi_ddrtype_to_ctrl(dfi, &ctrl);
> +	ddrmon_ver = readl_relaxed(dfi->regs);
> +	if (ddrmon_ver < 0x40)
> +		ret = rockchip_dfi_ddrtype_to_ctrl(dfi, &ctrl);
> +	else
> +		ret = rockchip_dfi_v4_ddrtype_to_ctrl(dfi, &ctrl, &ctrl1);
> +
>  	if (ret)
>  		goto out;
>  
> +
>  	for (i = 0; i < dfi->variant->max_channels; i++) {
>  
>  		if (!(dfi->channel_mask & BIT(i)))
> @@ -225,6 +260,10 @@ static int rockchip_dfi_enable(struct rockchip_dfi *dfi)
>  		writel_relaxed(ctrl, dfi_regs + i * dfi->variant->stride +
>  			       DDRMON_CTRL);
>  
> +		if (ddrmon_ver >= 0x40)
> +			writel_relaxed(ctrl1, dfi_regs + i * dfi->variant->stride +
> +				       DDRMON_CTRL1);
> +
>  		/* enable count, use software mode */
>  		writel_relaxed(FIELD_PREP_WM16(DDRMON_CTRL_SOFTWARE_EN, 1),
>  			       dfi_regs + i * dfi->variant->stride + DDRMON_CTRL);
> 

Acked-by: Chanwoo Choi <cw00.choi@samsung.com>

-- 
Best Regards,
Chanwoo Choi
Principal Engineer, Samsung Electronics