[PATCH v2 0/4] drm/msm/dpu: cleanup plane state

[PATCH v2 0/4] drm/msm/dpu: cleanup plane state

[Dmitry Baryshkov]

This is a cleanup part of the DPU multirect patchset [1], split away to
ease review and merging per Abhinav's request.

Changes in v2:
- Drop patches that were accepted
- Removed pe argument from _dpu_hw_sspp_setup_scaler3
- Add CDP setup and DPU_SSPP features checks patches that were left from the previous series

[1] https://lore.kernel.org/linux-arm-msm/20210705012115.4179824-1-dmitry.baryshkov@linaro.org/

The following changes since commit fee32807633395e666f0951d6b7b6546e9b76c3d:

  mailmap: add and update email addresses (2021-11-29 16:19:58 -0800)

are available in the Git repository at:

  https://git.linaro.org/people/dmitry.baryshkov/kernel.git dpu-cleanup-more

for you to fetch changes up to 5f4622c2324756e942e6e40227997713cdd0a03a:

  drm/msm/dpu: fix CDP setup to account for multirect index (2021-12-02 01:29:18 +0300)

----------------------------------------------------------------
Dmitry Baryshkov (4):
      drm/msm/dpu: drop scaler config from plane state
      drm/msm/dpu: drop pe argument from _dpu_hw_sspp_setup_scaler3
      drm/msm/dpu: simplify DPU_SSPP features checks
      drm/msm/dpu: fix CDP setup to account for multirect index

 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 14 +++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 22 ++++++----
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c   | 64 +++++++++++++----------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h   |  6 ---
 4 files changed, 52 insertions(+), 54 deletions(-)

[PATCH v2 1/4] drm/msm/dpu: drop scaler config from plane state

[Dmitry Baryshkov]

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.

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;
 };
-- 
2.33.0

[PATCH v2 2/4] drm/msm/dpu: drop pe argument from _dpu_hw_sspp_setup_scaler3

[Dmitry Baryshkov]

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.

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);
 }
 
-- 
2.33.0

[PATCH v2 3/4] drm/msm/dpu: simplify DPU_SSPP features checks

[Dmitry Baryshkov]

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>
---
 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;
-- 
2.33.0

[PATCH v2 4/4] drm/msm/dpu: fix CDP setup to account for multirect index

[Dmitry Baryshkov]

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>
---
 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);
 		}
 	}
 
-- 
2.33.0

Re: [PATCH v2 1/4] drm/msm/dpu: drop scaler config from plane state

[Abhinav Kumar]

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;
>   };
> 

Re: [PATCH v2 2/4] drm/msm/dpu: drop pe argument from _dpu_hw_sspp_setup_scaler3

[Abhinav Kumar]

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

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);
>   }
>   
> 

Re: [Freedreno] [PATCH v2 3/4] drm/msm/dpu: simplify DPU_SSPP features checks

[Abhinav Kumar]

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;
> 

Re: [PATCH v2 4/4] drm/msm/dpu: fix CDP setup to account for multirect index

[Abhinav Kumar]

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);
>   		}
>   	}
>   
>