[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aPsTeJ7/JefLfERa@yuanjiey.ap.qualcomm.com>
Date: Fri, 24 Oct 2025 13:49:44 +0800
From: yuanjiey <yuanjie.yang@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: robin.clark@....qualcomm.com, lumag@...nel.org, abhinav.kumar@...ux.dev,
sean@...rly.run, marijn.suijten@...ainline.org, airlied@...il.com,
simona@...ll.ch, maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
tzimmermann@...e.de, robh@...nel.org, krzk+dt@...nel.org,
conor+dt@...nel.org, quic_mkrishn@...cinc.com, jonathan@...ek.ca,
quic_khsieh@...cinc.com, neil.armstrong@...aro.org,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, tingwei.zhang@....qualcomm.com,
aiqun.yu@....qualcomm.com, yongxing.mou@....qualcomm.com
Subject: Re: [PATCH 06/12] drm/msm/dpu: Add Kaanapali SSPP sub-block support
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@....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@....qualcomm.com>
> > Signed-off-by: Yuanjie Yang <yuanjie.yang@....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
Powered by blists - more mailing lists