Re: [PATCH 06/12] drm/msm/dpu: Add Kaanapali SSPP sub-block support

[yuanjiey <yuanjie.yang@oss.qualcomm.com>]

On Thu, Oct 23, 2025 at 03:14:00PM +0300, Dmitry Baryshkov wrote:
> On Thu, Oct 23, 2025 at 04:06:03PM +0800, yuanjie yang wrote:
> > From: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> > 
> > Add support for Kaanapali platform SSPP sub-blocks, which
> > introduce structural changes including register additions,
> > removals, and relocations. Add the new common and rectangle
> > blocks, and update register definitions and handling to
> > ensure compatibility with DPU v13.0.
> 
> Merge new structures into this patch.
OK will adjust patch.


> > 
> > Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
> > Signed-off-by: Yuanjie Yang <yuanjie.yang@oss.qualcomm.com>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 474 +++++++++++++++++++-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c |  23 +
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h |   4 +
> >  3 files changed, 483 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > index 6f1fc790ad6d..7217c952f01c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> > @@ -17,6 +17,7 @@
> >  
> >  #define DPU_FETCH_CONFIG_RESET_VALUE   0x00000087
> >  
> > +/* <= v12 DPU */
> >  /* SSPP registers */
> >  #define SSPP_SRC_SIZE                      0x00
> >  #define SSPP_SRC_XY                        0x08
> > @@ -74,12 +75,116 @@
> >  #define SSPP_EXCL_REC_XY                   0x1B8
> >  #define SSPP_CLK_CTRL                      0x330
> >  
> > +/* >= v13 DPU */
> > +/* CMN Registers -> Source Surface Processing Pipe Common SSPP registers */
> > +/*      Name                                        Offset */
> > +#define SSPP_CMN_CLK_CTRL                0x0
> > +#define SSPP_CMN_CLK_STATUS              0x4
> > +#define SSPP_CMN_MULTI_REC_OP_MODE       0x10
> > +#define SSPP_CMN_ADDR_CONFIG             0x14
> > +#define SSPP_CMN_CAC_CTRL                0x20
> > +#define SSPP_CMN_SYS_CACHE_MODE          0x24
> > +#define SSPP_CMN_QOS_CTRL                0x28
> > +#define SSPP_CMN_DANGER_LUT              0x2C
> > +#define SSPP_CMN_SAFE_LUT                0x30
> 
> lowercase hex.
OK will fix

> 
> > @@ -141,6 +246,12 @@
> >  /* traffic shaper clock in Hz */
> >  #define TS_CLK			19200000
> >  
> > +static inline u32 _sspp_calculate_rect_off(enum dpu_sspp_multirect_index rect_index,
> > +					   struct dpu_hw_sspp *ctx)
> > +{
> > +	return (rect_index == DPU_SSPP_RECT_SOLO || rect_index == DPU_SSPP_RECT_0) ?
> > +			ctx->cap->sblk->sspp_rec0_blk.base : ctx->cap->sblk->sspp_rec1_blk.base;
> > +}
> >  
> >  static void dpu_hw_sspp_setup_multirect(struct dpu_sw_pipe *pipe)
> >  {
> > @@ -169,6 +280,34 @@ static void dpu_hw_sspp_setup_multirect(struct dpu_sw_pipe *pipe)
> >  	DPU_REG_WRITE(&ctx->hw, SSPP_MULTIRECT_OPMODE, mode_mask);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_multirect_v13(struct dpu_sw_pipe *pipe)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	u32 offset = ctx->cap->sblk->cmn_blk.base;
> > +	u32 mode_mask;
> > +
> > +	if (!ctx)
> > +		return;
> > +
> > +	if (pipe->multirect_index == DPU_SSPP_RECT_SOLO) {
> > +		/**
> > +		 * if rect index is RECT_SOLO, we cannot expect a
> > +		 * virtual plane sharing the same SSPP id. So we go
> > +		 * and disable multirect
> > +		 */
> > +		mode_mask = 0;
> > +	} else {
> > +		mode_mask = DPU_REG_READ(&ctx->hw, offset + SSPP_CMN_MULTI_REC_OP_MODE);
> > +		mode_mask |= pipe->multirect_index;
> > +		if (pipe->multirect_mode == DPU_SSPP_MULTIRECT_TIME_MX)
> > +			mode_mask |= BIT(2);
> > +		else
> > +			mode_mask &= ~BIT(2);
> > +	}
> > +
> > +	DPU_REG_WRITE(&ctx->hw, offset + SSPP_CMN_MULTI_REC_OP_MODE, mode_mask);
> > +}
> > +
> >  static void _sspp_setup_opmode(struct dpu_hw_sspp *ctx,
> >  		u32 mask, u8 en)
> >  {
> > @@ -328,6 +467,119 @@ static void dpu_hw_sspp_setup_format(struct dpu_sw_pipe *pipe,
> >  	DPU_REG_WRITE(c, SSPP_UBWC_ERROR_STATUS, BIT(31));
> >  }
> >  
> > +static void dpu_hw_sspp_setup_format_v13(struct dpu_sw_pipe *pipe,
> > +		const struct msm_format *fmt, u32 flags)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	struct dpu_hw_blk_reg_map *c;
> > +	u32 chroma_samp, unpack, src_format;
> > +	u32 opmode = 0;
> > +	u32 fast_clear = 0;
> > +	u32 offset;
> > +
> > +	if (!ctx || !fmt)
> > +		return;
> > +
> > +	offset = _sspp_calculate_rect_off(pipe->multirect_index, ctx);
> > +
> > +	c = &ctx->hw;
> > +
> > +	opmode = DPU_REG_READ(c, offset + SSPP_REC_SRC_OP_MODE);
> > +	opmode &= ~(MDSS_MDP_OP_FLIP_LR | MDSS_MDP_OP_FLIP_UD |
> > +			MDSS_MDP_OP_BWC_EN | MDSS_MDP_OP_PE_OVERRIDE
> > +			| MDSS_MDP_OP_ROT_90);
> > +
> > +	if (flags & DPU_SSPP_FLIP_LR)
> > +		opmode |= MDSS_MDP_OP_FLIP_LR;
> > +	if (flags & DPU_SSPP_FLIP_UD)
> > +		opmode |= MDSS_MDP_OP_FLIP_UD;
> > +	if (flags & DPU_SSPP_ROT_90)
> > +		opmode |= MDSS_MDP_OP_ROT_90;
> > +
> > +	chroma_samp = fmt->chroma_sample;
> > +	if (flags & DPU_SSPP_SOURCE_ROTATED_90) {
> > +		if (chroma_samp == CHROMA_H2V1)
> > +			chroma_samp = CHROMA_H1V2;
> > +		else if (chroma_samp == CHROMA_H1V2)
> > +			chroma_samp = CHROMA_H2V1;
> > +	}
> > +
> > +	src_format = (chroma_samp << 23) | (fmt->fetch_type << 19) |
> > +		(fmt->bpc_a << 6) | (fmt->bpc_r_cr << 4) |
> > +		(fmt->bpc_b_cb << 2) | (fmt->bpc_g_y << 0);
> > +
> > +	if (flags & DPU_SSPP_ROT_90)
> > +		src_format |= BIT(11); /* ROT90 */
> > +
> > +	if (fmt->alpha_enable && fmt->fetch_type == MDP_PLANE_INTERLEAVED)
> > +		src_format |= BIT(8); /* SRCC3_EN */
> > +
> > +	if (flags & DPU_SSPP_SOLID_FILL)
> > +		src_format |= BIT(22);
> > +
> > +	unpack = (fmt->element[3] << 24) | (fmt->element[2] << 16) |
> > +		(fmt->element[1] << 8) | (fmt->element[0] << 0);
> > +	src_format |= ((fmt->unpack_count - 1) << 12) |
> > +		((fmt->flags & MSM_FORMAT_FLAG_UNPACK_TIGHT ? 1 : 0) << 17) |
> > +		((fmt->flags & MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB ? 1 : 0) << 18) |
> > +		((fmt->bpp - 1) << 9);
> > +
> > +	if (fmt->fetch_mode != MDP_FETCH_LINEAR) {
> > +		if (MSM_FORMAT_IS_UBWC(fmt))
> > +			opmode |= MDSS_MDP_OP_BWC_EN;
> > +		src_format |= (fmt->fetch_mode & 3) << 30; /*FRAME_FORMAT */
> > +		switch (ctx->ubwc->ubwc_enc_version) {
> > +		case UBWC_1_0:
> > +			fast_clear = fmt->alpha_enable ? BIT(31) : 0;
> > +			DPU_REG_WRITE(c, offset + SSPP_REC_UBWC_STATIC_CTRL,
> > +				      fast_clear | (ctx->ubwc->ubwc_swizzle & 0x1) |
> > +				      BIT(8) |
> > +				     (ctx->ubwc->highest_bank_bit << 4));
> > +			break;
> > +		case UBWC_2_0:
> > +			fast_clear = fmt->alpha_enable ? BIT(31) : 0;
> > +			DPU_REG_WRITE(c, offset + SSPP_REC_UBWC_STATIC_CTRL,
> > +				      fast_clear | (ctx->ubwc->ubwc_swizzle) |
> > +				     (ctx->ubwc->highest_bank_bit << 4));
> > +			break;
> > +		case UBWC_3_0:
> > +			DPU_REG_WRITE(c, offset + SSPP_REC_UBWC_STATIC_CTRL,
> > +				      BIT(30) | (ctx->ubwc->ubwc_swizzle) |
> > +				     (ctx->ubwc->highest_bank_bit << 4));
> > +			break;
> > +		case UBWC_4_0:
> > +			DPU_REG_WRITE(c, offset + SSPP_REC_UBWC_STATIC_CTRL,
> > +				      MSM_FORMAT_IS_YUV(fmt) ? 0 : BIT(30));
> > +			break;
> > +		}
> > +	}
> > +
> > +	opmode |= MDSS_MDP_OP_PE_OVERRIDE;
> > +
> > +	/* if this is YUV pixel format, enable CSC */
> > +	if (MSM_FORMAT_IS_YUV(fmt))
> > +		src_format |= BIT(15);
> > +
> > +	if (MSM_FORMAT_IS_DX(fmt))
> > +		src_format |= BIT(14);
> > +
> > +	/* update scaler opmode, if appropriate */
> > +	if (test_bit(DPU_SSPP_CSC, &ctx->cap->features))
> > +		_sspp_setup_opmode(ctx, VIG_OP_CSC_EN | VIG_OP_CSC_SRC_DATAFMT,
> > +				   MSM_FORMAT_IS_YUV(fmt));
> > +	else if (test_bit(DPU_SSPP_CSC_10BIT, &ctx->cap->features))
> > +		_sspp_setup_csc10_opmode(ctx,
> > +					 VIG_CSC_10_EN | VIG_CSC_10_SRC_DATAFMT,
> > +					 MSM_FORMAT_IS_YUV(fmt));
> 
> This function is a nice c&p of the old one. Can we abstract it at least
> a bit and let it be shared between the old code and the new one?
OK, I will try to abstract some common code into a function.


> > +
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_SRC_FORMAT, src_format);
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_SRC_UNPACK_PATTERN, unpack);
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_SRC_OP_MODE, opmode);
> > +
> > +	/* clear previous UBWC error */
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_UBWC_ERROR_STATUS, BIT(31));
> > +}
> > +
> >  static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_sspp *ctx,
> >  		struct dpu_hw_pixel_ext *pe_ext)
> >  {
> > @@ -385,6 +637,53 @@ static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_sspp *ctx,
> >  			tot_req_pixels[3]);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_pe_config_v13(struct dpu_hw_sspp *ctx,
> > +		struct dpu_hw_pixel_ext *pe_ext)
> > +{
> > +	struct dpu_hw_blk_reg_map *c;
> > +	u8 color;
> > +	u32 lr_pe[4], tb_pe[4], tot_req_pixels[4];
> > +	const u32 bytemask = 0xff;
> > +	const u32 shortmask = 0xffff;
> > +	u32 offset = ctx->cap->sblk->sspp_rec0_blk.base;
> > +
> > +	if (!ctx || !pe_ext)
> > +		return;
> > +
> > +	c = &ctx->hw;
> > +	/* program SW pixel extension override for all pipes*/
> > +	for (color = 0; color < DPU_MAX_PLANES; color++) {
> > +		/* color 2 has the same set of registers as color 1 */
> > +		if (color == 2)
> > +			continue;
> > +
> > +		lr_pe[color] = ((pe_ext->right_ftch[color] & bytemask) << 24) |
> > +			       ((pe_ext->right_rpt[color] & bytemask) << 16) |
> > +			       ((pe_ext->left_ftch[color] & bytemask) << 8) |
> > +			       (pe_ext->left_rpt[color] & bytemask);
> > +
> > +		tb_pe[color] = ((pe_ext->btm_ftch[color] & bytemask) << 24) |
> > +			       ((pe_ext->btm_rpt[color] & bytemask) << 16) |
> > +			       ((pe_ext->top_ftch[color] & bytemask) << 8) |
> > +			       (pe_ext->top_rpt[color] & bytemask);
> > +
> > +		tot_req_pixels[color] = (((pe_ext->roi_h[color] +
> > +			pe_ext->num_ext_pxls_top[color] +
> > +			pe_ext->num_ext_pxls_btm[color]) & shortmask) << 16) |
> > +			((pe_ext->roi_w[color] +
> > +			pe_ext->num_ext_pxls_left[color] +
> > +			pe_ext->num_ext_pxls_right[color]) & shortmask);
> > +	}
> 
> Same comment. No C&P, of a sensible code, please.
OK, will do. Thanks 

> > +
> > +	/* color 0 */
> > +	DPU_REG_WRITE(c, SSPP_REC_SW_PIX_EXT_LR + offset, lr_pe[0]);
> > +	DPU_REG_WRITE(c, SSPP_REC_SW_PIX_EXT_TB + offset, tb_pe[0]);
> > +
> > +	/* color 1 and color 2 */
> > +	DPU_REG_WRITE(c, SSPP_REC_SW_PIX_EXT_LR_ODX + offset, lr_pe[1]);
> > +	DPU_REG_WRITE(c, SSPP_REC_SW_PIX_EXT_TB_ODX + offset, tb_pe[1]);
> > +}
> > +
> >  static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_sspp *ctx,
> >  		struct dpu_hw_scaler3_cfg *scaler3_cfg,
> >  		const struct msm_format *format)
> > @@ -443,6 +742,36 @@ static void dpu_hw_sspp_setup_rects(struct dpu_sw_pipe *pipe,
> >  	DPU_REG_WRITE(c, out_xy_off, dst_xy);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_rects_v13(struct dpu_sw_pipe *pipe,
> > +					struct dpu_sw_pipe_cfg *cfg)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	struct dpu_hw_blk_reg_map *c;
> > +	u32 src_size, src_xy, dst_size, dst_xy;
> > +	u32 offset;
> > +
> > +	if (!ctx || !cfg)
> > +		return;
> > +
> > +	c = &ctx->hw;
> > +
> > +	offset = _sspp_calculate_rect_off(pipe->multirect_index, ctx);
> > +
> > +	/* src and dest rect programming */
> > +	src_xy = (cfg->src_rect.y1 << 16) | cfg->src_rect.x1;
> > +	src_size = (drm_rect_height(&cfg->src_rect) << 16) |
> > +		   drm_rect_width(&cfg->src_rect);
> > +	dst_xy = (cfg->dst_rect.y1 << 16) | cfg->dst_rect.x1;
> > +	dst_size = (drm_rect_height(&cfg->dst_rect) << 16) |
> > +		drm_rect_width(&cfg->dst_rect);
> > +
> > +	/* rectangle register programming */
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_SRC_SIZE, src_size);
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_SRC_XY, src_xy);
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_OUT_SIZE, dst_size);
> > +	DPU_REG_WRITE(c, offset + SSPP_REC_OUT_XY, dst_xy);
> > +}
> > +
> >  static void dpu_hw_sspp_setup_sourceaddress(struct dpu_sw_pipe *pipe,
> >  		struct dpu_hw_fmt_layout *layout)
> >  {
> > @@ -497,6 +826,29 @@ static void dpu_hw_sspp_setup_sourceaddress(struct dpu_sw_pipe *pipe,
> >  	DPU_REG_WRITE(&ctx->hw, SSPP_SRC_YSTRIDE1, ystride1);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_sourceaddress_v13(struct dpu_sw_pipe *pipe,
> > +						struct dpu_hw_fmt_layout *layout)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	int i;
> > +	u32 addr, ystride0, ystride1;
> > +
> > +	if (!ctx)
> > +		return;
> > +
> > +	addr = _sspp_calculate_rect_off(pipe->multirect_index, ctx);
> > +
> > +	for (i = 0; i < ARRAY_SIZE(layout->plane_addr); i++)
> > +		DPU_REG_WRITE(&ctx->hw, addr + SSPP_REC_SRC0_ADDR + i * 0x4,
> > +			      layout->plane_addr[i]);
> > +
> > +	ystride0 = (layout->plane_pitch[0]) | (layout->plane_pitch[2] << 16);
> > +	ystride1 = (layout->plane_pitch[1]) | (layout->plane_pitch[3] << 16);
> > +
> > +	DPU_REG_WRITE(&ctx->hw, addr + SSPP_REC_SRC_YSTRIDE0, ystride0);
> > +	DPU_REG_WRITE(&ctx->hw, addr + SSPP_REC_SRC_YSTRIDE1, ystride1);
> > +}
> > +
> >  static void dpu_hw_sspp_setup_csc(struct dpu_hw_sspp *ctx,
> >  		const struct dpu_csc_cfg *data)
> >  {
> > @@ -536,6 +888,24 @@ static void dpu_hw_sspp_setup_solidfill(struct dpu_sw_pipe *pipe, u32 color)
> >  				color);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_solidfill_v13(struct dpu_sw_pipe *pipe, u32 color)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	struct dpu_hw_fmt_layout cfg;
> > +	u32 offset;
> > +
> > +	if (!ctx)
> > +		return;
> > +
> > +	offset = _sspp_calculate_rect_off(pipe->multirect_index, ctx);
> > +
> > +	/* cleanup source addresses */
> > +	memset(&cfg, 0, sizeof(cfg));
> > +	ctx->ops.setup_sourceaddress(pipe, &cfg);
> > +
> > +	DPU_REG_WRITE(&ctx->hw, offset + SSPP_REC_SRC_CONSTANT_COLOR, color);
> > +}
> > +
> >  static void dpu_hw_sspp_setup_qos_lut(struct dpu_hw_sspp *ctx,
> >  				      struct dpu_hw_qos_cfg *cfg)
> >  {
> > @@ -547,6 +917,17 @@ static void dpu_hw_sspp_setup_qos_lut(struct dpu_hw_sspp *ctx,
> >  			      cfg);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_qos_lut_v13(struct dpu_hw_sspp *ctx,
> > +					  struct dpu_hw_qos_cfg *cfg)
> > +{
> > +	if (!ctx || !cfg)
> > +		return;
> > +
> > +	_dpu_hw_setup_qos_lut_v13(&ctx->hw, 0,
> > +				  ctx->mdss_ver->core_major_ver >= 4,
> 
> This is v13, of course it is >= 4.
OK will fix.


> > +				  cfg);
> > +}
> > +
> >  static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx,
> >  				       bool danger_safe_en)
> >  {
> > @@ -557,6 +938,16 @@ static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_sspp *ctx,
> >  		      danger_safe_en ? SSPP_QOS_CTRL_DANGER_SAFE_EN : 0);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_qos_ctrl_v13(struct dpu_hw_sspp *ctx,
> > +					   bool danger_safe_en)
> > +{
> > +	if (!ctx)
> > +		return;
> > +
> > +	DPU_REG_WRITE(&ctx->hw, SSPP_CMN_QOS_CTRL,
> > +		      danger_safe_en ? SSPP_QOS_CTRL_DANGER_SAFE_EN : 0);
> > +}
> > +
> >  static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
> >  				  const struct msm_format *fmt,
> >  				  bool enable)
> > @@ -576,6 +967,20 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
> >  	dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
> >  }
> >  
> > +static void dpu_hw_sspp_setup_cdp_v13(struct dpu_sw_pipe *pipe,
> > +				      const struct msm_format *fmt,
> > +				      bool enable)
> > +{
> > +	struct dpu_hw_sspp *ctx = pipe->sspp;
> > +	u32 offset = 0;
> > +
> > +	if (!ctx)
> > +		return;
> > +
> > +	offset = _sspp_calculate_rect_off(pipe->multirect_index, ctx);
> > +	dpu_setup_cdp(&ctx->hw, offset + SSPP_REC_CDP_CNTL, fmt, enable);
> > +}
> > +
> >  static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
> >  {
> >  	static const struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
> > @@ -586,36 +991,69 @@ static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enabl
> >  	return dpu_hw_clk_force_ctrl(&ctx->hw, &sspp_clk_ctrl, enable);
> >  }
> >  
> > +static bool dpu_hw_sspp_setup_clk_force_ctrl_v13(struct dpu_hw_sspp *ctx, bool enable)
> > +{
> > +	static const struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
> > +		.reg_off = SSPP_CMN_CLK_CTRL,
> > +		.bit_off = 0
> > +	};
> > +
> > +	return dpu_hw_clk_force_ctrl(&ctx->hw, &sspp_clk_ctrl, enable);
> > +}
> > +
> >  static void _setup_layer_ops(struct dpu_hw_sspp *c,
> >  		unsigned long features, const struct dpu_mdss_version *mdss_rev)
> >  {
> > -	c->ops.setup_format = dpu_hw_sspp_setup_format;
> > -	c->ops.setup_rects = dpu_hw_sspp_setup_rects;
> > -	c->ops.setup_sourceaddress = dpu_hw_sspp_setup_sourceaddress;
> > -	c->ops.setup_solidfill = dpu_hw_sspp_setup_solidfill;
> > -	c->ops.setup_pe = dpu_hw_sspp_setup_pe_config;
> > -
> > -	if (test_bit(DPU_SSPP_QOS, &features)) {
> > -		c->ops.setup_qos_lut = dpu_hw_sspp_setup_qos_lut;
> > -		c->ops.setup_qos_ctrl = dpu_hw_sspp_setup_qos_ctrl;
> > +	if (mdss_rev->core_major_ver >= 13) {
> > +		c->ops.setup_format = dpu_hw_sspp_setup_format_v13;
> > +		c->ops.setup_rects = dpu_hw_sspp_setup_rects_v13;
> > +		c->ops.setup_sourceaddress = dpu_hw_sspp_setup_sourceaddress_v13;
> > +		c->ops.setup_solidfill = dpu_hw_sspp_setup_solidfill_v13;
> > +		c->ops.setup_pe = dpu_hw_sspp_setup_pe_config_v13;
> > +
> > +		if (test_bit(DPU_SSPP_QOS, &features)) {
> > +			c->ops.setup_qos_lut = dpu_hw_sspp_setup_qos_lut_v13;
> > +			c->ops.setup_qos_ctrl = dpu_hw_sspp_setup_qos_ctrl_v13;
> > +		}
> > +
> > +		if (test_bit(DPU_SSPP_SMART_DMA_V1, &c->cap->features) ||
> > +		    test_bit(DPU_SSPP_SMART_DMA_V2, &c->cap->features))
> > +			c->ops.setup_multirect = dpu_hw_sspp_setup_multirect_v13;
> > +
> > +		if (test_bit(DPU_SSPP_CDP, &features))
> > +			c->ops.setup_cdp = dpu_hw_sspp_setup_cdp_v13;
> > +
> > +		c->ops.setup_clk_force_ctrl = dpu_hw_sspp_setup_clk_force_ctrl_v13;
> > +	} else {
> > +		c->ops.setup_format = dpu_hw_sspp_setup_format;
> > +		c->ops.setup_rects = dpu_hw_sspp_setup_rects;
> > +		c->ops.setup_sourceaddress = dpu_hw_sspp_setup_sourceaddress;
> > +		c->ops.setup_solidfill = dpu_hw_sspp_setup_solidfill;
> > +		c->ops.setup_pe = dpu_hw_sspp_setup_pe_config;
> > +
> > +		if (test_bit(DPU_SSPP_QOS, &features)) {
> > +			c->ops.setup_qos_lut = dpu_hw_sspp_setup_qos_lut;
> > +			c->ops.setup_qos_ctrl = dpu_hw_sspp_setup_qos_ctrl;
> > +		}
> > +
> > +		if (test_bit(DPU_SSPP_SMART_DMA_V1, &c->cap->features) ||
> > +		    test_bit(DPU_SSPP_SMART_DMA_V2, &c->cap->features))
> > +			c->ops.setup_multirect = dpu_hw_sspp_setup_multirect;
> > +
> > +		if (test_bit(DPU_SSPP_CDP, &features))
> > +			c->ops.setup_cdp = dpu_hw_sspp_setup_cdp;
> > +
> > +		if (mdss_rev->core_major_ver >= 9)
> > +			c->ops.setup_clk_force_ctrl = dpu_hw_sspp_setup_clk_force_ctrl;
> >  	}
> >  
> >  	if (test_bit(DPU_SSPP_CSC, &features) ||
> >  		test_bit(DPU_SSPP_CSC_10BIT, &features))
> >  		c->ops.setup_csc = dpu_hw_sspp_setup_csc;
> >  
> > -	if (test_bit(DPU_SSPP_SMART_DMA_V1, &c->cap->features) ||
> > -		test_bit(DPU_SSPP_SMART_DMA_V2, &c->cap->features))
> > -		c->ops.setup_multirect = dpu_hw_sspp_setup_multirect;
> > -
> >  	if (test_bit(DPU_SSPP_SCALER_QSEED3_COMPATIBLE, &features))
> >  		c->ops.setup_scaler = _dpu_hw_sspp_setup_scaler3;
> 
> If the only common items are CSC and scaler (and both are already pretty
> abstracted), please don't squash it in the same old file. Create new
> file for the new hardware block (like we did for DSC 1.1 vs 1.2).
Yes, I agree. My original idea is to make a file named dpu_hw_sspp_v13.c to include
all DPU version 13 SSPP change. Will do it in next patch.


> >  
> > -	if (test_bit(DPU_SSPP_CDP, &features))
> > -		c->ops.setup_cdp = dpu_hw_sspp_setup_cdp;
> > -
> > -	if (mdss_rev->core_major_ver >= 9)
> > -		c->ops.setup_clk_force_ctrl = dpu_hw_sspp_setup_clk_force_ctrl;
> >  }
> >  
> >  #ifdef CONFIG_DEBUG_FS
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> > index 486be346d40d..1acf456c425b 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
> > @@ -81,6 +81,13 @@ static u32 dpu_hw_util_log_mask = DPU_DBG_MASK_NONE;
> >  #define QOS_CREQ_LUT_0                    0x14
> >  #define QOS_CREQ_LUT_1                    0x18
> >  
> > +/* CMN_QOS_LUT */
> > +#define SSPP_CMN_DANGER_LUT                    0x2C
> > +#define SSPP_CMN_SAFE_LUT                      0x30
> > +#define SSPP_CMN_CREQ_LUT_0                    0x34
> > +#define SSPP_CMN_CREQ_LUT_1                    0x38
> > +#define SSPP_CMN_QOS_CTRL	                    0x28
> 
> Lowercase hex, sort by the address.
OK will fix

> > +
> >  /* QOS_QOS_CTRL */
> >  #define QOS_QOS_CTRL_DANGER_SAFE_EN       BIT(0)
> >  #define QOS_QOS_CTRL_DANGER_VBLANK_MASK   GENMASK(5, 4)
> > @@ -475,6 +482,22 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset,
> >  		      cfg->danger_safe_en ? QOS_QOS_CTRL_DANGER_SAFE_EN : 0);
> >  }
> >  
> > +void _dpu_hw_setup_qos_lut_v13(struct dpu_hw_blk_reg_map *c, u32 offset,
> > +			       bool qos_8lvl,
> > +			       const struct dpu_hw_qos_cfg *cfg)
> > +{
> > +	DPU_REG_WRITE(c, offset + SSPP_CMN_DANGER_LUT, cfg->danger_lut);
> > +	DPU_REG_WRITE(c, offset + SSPP_CMN_SAFE_LUT, cfg->safe_lut);
> > +
> > +	if (qos_8lvl) {
> > +		DPU_REG_WRITE(c, offset + SSPP_CMN_CREQ_LUT_0, cfg->creq_lut);
> > +		DPU_REG_WRITE(c, offset + SSPP_CMN_CREQ_LUT_1, cfg->creq_lut >> 32);
> > +	}
> > +
> > +	DPU_REG_WRITE(c, offset + SSPP_CMN_QOS_CTRL,
> > +		      cfg->danger_safe_en ? QOS_QOS_CTRL_DANGER_SAFE_EN : 0);
> > +}
> > +
> >  /*
> >   * note: Aside from encoders, input_sel should be set to 0x0 by default
> >   */
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> > index 67b08e99335d..9d442d6fc11c 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
> > @@ -360,6 +360,10 @@ void _dpu_hw_setup_qos_lut(struct dpu_hw_blk_reg_map *c, u32 offset,
> >  			   bool qos_8lvl,
> >  			   const struct dpu_hw_qos_cfg *cfg);
> >  
> > +void _dpu_hw_setup_qos_lut_v13(struct dpu_hw_blk_reg_map *c, u32 offset,
> > +			       bool qos_8lvl,
> > +			       const struct dpu_hw_qos_cfg *cfg);
> > +
> >  void dpu_hw_setup_misr(struct dpu_hw_blk_reg_map *c,
> >  		u32 misr_ctrl_offset, u8 input_sel);
> >  
> > -- 
> > 2.34.1
> > 
> 
> -- 
> With best wishes
> Dmitry

Thanks,
Yuanjie