On 12/1/2021 2:51 PM, Dmitry Baryshkov wrote:

Scaler and pixel_ext configuration does not contain a long living state, it is used only during plane update, so remove these two fiels from dpu_plane_state and allocate them on stack.

s/fiels/fields

apart from that, Reviewed-by: Abhinav Kumar quic_abhinavk@quicinc.com

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 59 ++++++++++------------- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 6 --- 2 files changed, 26 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index ca190d92f0d5..4c373abbe89c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -536,14 +536,12 @@ static void _dpu_plane_setup_scaler3(struct dpu_plane *pdpu, struct dpu_plane_state *pstate, uint32_t src_w, uint32_t src_h, uint32_t dst_w, uint32_t dst_h, struct dpu_hw_scaler3_cfg *scale_cfg,

• struct dpu_hw_pixel_ext *pixel_ext,
  

  const struct dpu_format *fmt, uint32_t chroma_subsmpl_h, uint32_t chroma_subsmpl_v) { uint32_t i;

• memset(scale_cfg, 0, sizeof(*scale_cfg));
• memset(&pstate->pixel_ext, 0, sizeof(struct dpu_hw_pixel_ext));
• scale_cfg->phase_step_x[DPU_SSPP_COMP_0] = mult_frac((1 << PHASE_STEP_SHIFT), src_w, dst_w); scale_cfg->phase_step_y[DPU_SSPP_COMP_0] =

@@ -582,9 +580,9 @@ static void _dpu_plane_setup_scaler3(struct dpu_plane *pdpu, scale_cfg->preload_y[i] = DPU_QSEED3_DEFAULT_PRELOAD_V; }

• pstate->pixel_ext.num_ext_pxls_top[i] =
  

• pixel_ext->num_ext_pxls_top[i] =
  scale_cfg->src_height[i];
  

• pstate->pixel_ext.num_ext_pxls_left[i] =
  

• pixel_ext->num_ext_pxls_left[i] =
  scale_cfg->src_width[i];
  

  } if (!(DPU_FORMAT_IS_YUV(fmt)) && (src_h == dst_h)

@@ -662,6 +660,11 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu, struct dpu_hw_pipe_cfg *pipe_cfg) { const struct drm_format_info *info = drm_format_info(fmt->base.pixel_format);

• struct dpu_hw_scaler3_cfg scaler3_cfg;

• struct dpu_hw_pixel_ext pixel_ext;

• memset(&scaler3_cfg, 0, sizeof(scaler3_cfg));

• memset(&pixel_ext, 0, sizeof(pixel_ext));

  /* don't chroma subsample if decimating */ /* update scaler. calculate default config for QSEED3 */

@@ -670,8 +673,23 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu, drm_rect_height(&pipe_cfg->src_rect), drm_rect_width(&pipe_cfg->dst_rect), drm_rect_height(&pipe_cfg->dst_rect),

• 	&pstate->scaler3_cfg, fmt,
  

• 	&scaler3_cfg, &pixel_ext, fmt,
  info->hsub, info->vsub);
  

• if (pdpu->pipe_hw->ops.setup_pe)

• pdpu->pipe_hw->ops.setup_pe(pdpu->pipe_hw,
  

• 		&pixel_ext);
  

• /**

• * when programmed in multirect mode, scalar block will be
  

• * bypassed. Still we need to update alpha and bitwidth
  

• * ONLY for RECT0
  

• */
  

• if (pdpu->pipe_hw->ops.setup_scaler &&

• 	pstate->multirect_index != DPU_SSPP_RECT_1)
  

• pdpu->pipe_hw->ops.setup_scaler(pdpu->pipe_hw,
  

• 		pipe_cfg, &pixel_ext,
  

• 		&scaler3_cfg);
  

  }

  /**

@@ -712,7 +730,6 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, drm_rect_width(&pipe_cfg.dst_rect); pipe_cfg.src_rect.y2 = drm_rect_height(&pipe_cfg.dst_rect);

• _dpu_plane_setup_scaler(pdpu, pstate, fmt, true, &pipe_cfg);
  
  

  if (pdpu->pipe_hw->ops.setup_format) pdpu->pipe_hw->ops.setup_format(pdpu->pipe_hw,

@@ -724,15 +741,7 @@ static int _dpu_plane_color_fill(struct dpu_plane *pdpu, &pipe_cfg, pstate->multirect_index);

• if (pdpu->pipe_hw->ops.setup_pe)
  

• 	pdpu->pipe_hw->ops.setup_pe(pdpu->pipe_hw,
  

• 			&pstate->pixel_ext);
  

• if (pdpu->pipe_hw->ops.setup_scaler &&
  

• 		pstate->multirect_index != DPU_SSPP_RECT_1)
  

• 	pdpu->pipe_hw->ops.setup_scaler(pdpu->pipe_hw,
  

• 			&pipe_cfg, &pstate->pixel_ext,
  

• 			&pstate->scaler3_cfg);
  

• _dpu_plane_setup_scaler(pdpu, pstate, fmt, true, &pipe_cfg);
  

  }

  return 0;

@@ -1129,8 +1138,6 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane)

pipe_cfg.dst_rect = state->dst;

• _dpu_plane_setup_scaler(pdpu, pstate, fmt, false, &pipe_cfg);
• /* override for color fill */ if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) { /* skip remaining processing on color fill */

@@ -1143,21 +1150,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) pstate->multirect_index); }

• if (pdpu->pipe_hw->ops.setup_pe &&

• 	(pstate->multirect_index != DPU_SSPP_RECT_1))
  

• pdpu->pipe_hw->ops.setup_pe(pdpu->pipe_hw,
  

• 		&pstate->pixel_ext);
  

• /**

• * when programmed in multirect mode, scalar block will be
  

• * bypassed. Still we need to update alpha and bitwidth
  

• * ONLY for RECT0
  

• */
  

• if (pdpu->pipe_hw->ops.setup_scaler &&

• 	pstate->multirect_index != DPU_SSPP_RECT_1)
  

• pdpu->pipe_hw->ops.setup_scaler(pdpu->pipe_hw,
  

• 		&pipe_cfg, &pstate->pixel_ext,
  

• 		&pstate->scaler3_cfg);
  

• _dpu_plane_setup_scaler(pdpu, pstate, fmt, false, &pipe_cfg);

  if (pdpu->pipe_hw->ops.setup_multirect) pdpu->pipe_hw->ops.setup_multirect(

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h index 52792526e904..1ee5ca5fcdf7 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h @@ -23,8 +23,6 @@

• @multirect_index: index of the rectangle of SSPP
• @multirect_mode: parallel or time multiplex multirect mode
• @pending: whether the current update is still pending

• • @scaler3_cfg: configuration data for scaler3
• • @pixel_ext: configuration data for pixel extensions
  • @plane_fetch_bw: calculated BW per plane
  • @plane_clk: calculated clk per plane
  */

@@ -37,10 +35,6 @@ struct dpu_plane_state { uint32_t multirect_mode; bool pending;

• /* scaler configuration */
• struct dpu_hw_scaler3_cfg scaler3_cfg;
• struct dpu_hw_pixel_ext pixel_ext;
• u64 plane_fetch_bw; u64 plane_clk; };

+ Dan C for awareness as this is a follow up of our discussion on https://lore.kernel.org/linux-arm-msm/c1537b326b654f05be247ca61d21e9f0@codea...

On 12/1/2021 2:51 PM, Dmitry Baryshkov wrote:

The _dpu_hw_sspp_setup_scaler3 (hw_sspp->setup_scaler) does not use pe argument. Let's remove it while we are cleaning scaled configuration.

Thanks for pushing this.

This was originally reported by Dan Carpenter.

Hence, please add the Reported-by tag for Dan on this (Dan Carpenter dan.carpenter@oracle.com )

Apart from that, Reviewed-by: Abhinav Kumar quic_abhinavk@quicinc.com

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 2 -- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +- 3 files changed, 1 insertion(+), 5 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 d77eb7da5daf..7235605bfc9e 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -413,13 +413,11 @@ static void dpu_hw_sspp_setup_pe_config(struct dpu_hw_pipe *ctx,

static void _dpu_hw_sspp_setup_scaler3(struct dpu_hw_pipe *ctx, struct dpu_hw_pipe_cfg *sspp,

• struct dpu_hw_pixel_ext *pe,
  

  void *scaler_cfg) { u32 idx; struct dpu_hw_scaler3_cfg *scaler3_cfg = scaler_cfg;

• (void)pe; if (_sspp_subblk_offset(ctx, DPU_SSPP_SCALER_QSEED3, &idx) || !sspp || !scaler3_cfg) return;

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index e8939d7387cb..ad2002d75739 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -334,12 +334,10 @@ struct dpu_hw_sspp_ops { * setup_scaler - setup scaler * @ctx: Pointer to pipe context * @pipe_cfg: Pointer to pipe configuration

• * @pe_cfg: Pointer to pixel extension configuration
  

  • @scaler_cfg: Pointer to scaler configuration

  */ void (*setup_scaler)(struct dpu_hw_pipe *ctx, struct dpu_hw_pipe_cfg *pipe_cfg,

• struct dpu_hw_pixel_ext *pe_cfg,
  

  void *scaler_cfg);

  /**

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 4c373abbe89c..c7b065b14c5c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -688,7 +688,7 @@ static void _dpu_plane_setup_scaler(struct dpu_plane *pdpu, if (pdpu->pipe_hw->ops.setup_scaler && pstate->multirect_index != DPU_SSPP_RECT_1) pdpu->pipe_hw->ops.setup_scaler(pdpu->pipe_hw,

• 		pipe_cfg, &pixel_ext,
  

• 		pipe_cfg,
  	&scaler3_cfg);
  

  }

On 12/1/2021 2:51 PM, Dmitry Baryshkov wrote:

Add DPU_SSPP_CSC_ANY denoting any CSC block. As we are at it, rewrite DPU_SSPP_SCALER (any scaler) to use BIT(x) instead of hand-coded bitshifts.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar quic_abhinavk@quicinc.com

---

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 16 +++++++++++----- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 3 +-- 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index ad2002d75739..3c53bd03bdeb 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -25,11 +25,17 @@ struct dpu_hw_pipe; /**

• Define all scaler feature bits in catalog

*/ -#define DPU_SSPP_SCALER ((1UL << DPU_SSPP_SCALER_RGB) | \

• (1UL << DPU_SSPP_SCALER_QSEED2) | \

• (1UL << DPU_SSPP_SCALER_QSEED3) | \
  

• (1UL << DPU_SSPP_SCALER_QSEED3LITE) | \
  

•  (1UL << DPU_SSPP_SCALER_QSEED4))
  

+#define DPU_SSPP_SCALER (BIT(DPU_SSPP_SCALER_RGB) | \

• 	 BIT(DPU_SSPP_SCALER_QSEED2) | \
  

• 	 BIT(DPU_SSPP_SCALER_QSEED3) | \
  

• 	 BIT(DPU_SSPP_SCALER_QSEED3LITE) | \
  

• 	 BIT(DPU_SSPP_SCALER_QSEED4))
  

+/*

• • Define all CSC feature bits in catalog
• */

+#define DPU_SSPP_CSC_ANY (BIT(DPU_SSPP_CSC) | \

• 	  BIT(DPU_SSPP_CSC_10BIT))
  
  

  /**

  • Component indices

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index c7b065b14c5c..911f5f0b41d8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1010,8 +1010,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,

if (DPU_FORMAT_IS_YUV(fmt) && (!(pdpu->pipe_hw->cap->features & DPU_SSPP_SCALER) ||

•  !(pdpu->pipe_hw->cap->features & (BIT(DPU_SSPP_CSC)
  

•  | BIT(DPU_SSPP_CSC_10BIT))))) {
  

•  !(pdpu->pipe_hw->cap->features & DPU_SSPP_CSC_ANY))) {
  

  DPU_DEBUG_PLANE(pdpu, "plane doesn't have scaler/csc for yuv\n"); return -EINVAL;

On 12/1/2021 2:51 PM, Dmitry Baryshkov wrote:

Client driven prefetch (CDP) is properly setup only for SSPP REC0 currently. Enable client driven prefetch also for SSPP REC1.

Signed-off-by: Dmitry Baryshkov dmitry.baryshkov@linaro.org

Reviewed-by: Abhinav Kumar quic_abhinavk@quicinc.com

---

drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 12 ++++++++++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 4 +++- drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 2 +- 3 files changed, 14 insertions(+), 4 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 7235605bfc9e..75aa47835214 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c @@ -75,6 +75,7 @@ #define SSPP_TRAFFIC_SHAPER 0x130 #define SSPP_CDP_CNTL 0x134 #define SSPP_UBWC_ERROR_STATUS 0x138 +#define SSPP_CDP_CNTL_REC1 0x13c #define SSPP_TRAFFIC_SHAPER_PREFILL 0x150 #define SSPP_TRAFFIC_SHAPER_REC1_PREFILL 0x154 #define SSPP_TRAFFIC_SHAPER_REC1 0x158 @@ -624,10 +625,12 @@ static void dpu_hw_sspp_setup_qos_ctrl(struct dpu_hw_pipe *ctx, }

static void dpu_hw_sspp_setup_cdp(struct dpu_hw_pipe *ctx,

• struct dpu_hw_pipe_cdp_cfg *cfg)
  

• struct dpu_hw_pipe_cdp_cfg *cfg,
  

• enum dpu_sspp_multirect_index index)
  

  { u32 idx; u32 cdp_cntl = 0;

• u32 cdp_cntl_offset = 0;

  if (!ctx || !cfg) return;

@@ -635,6 +638,11 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_hw_pipe *ctx, if (_sspp_subblk_offset(ctx, DPU_SSPP_SRC, &idx)) return;

• if (index == DPU_SSPP_RECT_SOLO || index == DPU_SSPP_RECT_0)

• cdp_cntl_offset = SSPP_CDP_CNTL;
  

• else

• cdp_cntl_offset = SSPP_CDP_CNTL_REC1;
  

• if (cfg->enable) cdp_cntl |= BIT(0); if (cfg->ubwc_meta_enable)

@@ -644,7 +652,7 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_hw_pipe *ctx, if (cfg->preload_ahead == DPU_SSPP_CDP_PRELOAD_AHEAD_64) cdp_cntl |= BIT(3);

• DPU_REG_WRITE(&ctx->hw, SSPP_CDP_CNTL, cdp_cntl);

• DPU_REG_WRITE(&ctx->hw, cdp_cntl_offset, cdp_cntl); }

  static void _setup_layer_ops(struct dpu_hw_pipe *c,

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h index 3c53bd03bdeb..227b09fa4689 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h @@ -356,9 +356,11 @@ struct dpu_hw_sspp_ops { * setup_cdp - setup client driven prefetch * @ctx: Pointer to pipe context * @cfg: Pointer to cdp configuration

• * @index: rectangle index in multirect
  

  */ void (*setup_cdp)(struct dpu_hw_pipe *ctx,

• 	struct dpu_hw_pipe_cdp_cfg *cfg);
  

• 	struct dpu_hw_pipe_cdp_cfg *cfg,
  

• 	enum dpu_sspp_multirect_index index);
  

  };

  /**

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 911f5f0b41d8..1134171f4d1c 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1191,7 +1191,7 @@ static void dpu_plane_sspp_atomic_update(struct drm_plane *plane) DPU_FORMAT_IS_TILE(fmt); cdp_cfg.preload_ahead = DPU_SSPP_CDP_PRELOAD_AHEAD_64;

• 	pdpu->pipe_hw->ops.setup_cdp(pdpu->pipe_hw, &cdp_cfg);
  

• 	pdpu->pipe_hw->ops.setup_cdp(pdpu->pipe_hw, &cdp_cfg, pstate->multirect_index);
  

  } }