All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] drm/msm/dpu: correctly implement SSPP & WB Clock Control Split
@ 2023-10-11 11:59 ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

Starting with the SM8550 platform, the SSPP & WB Clock Controls are
no more in the MDP TOP registers, but in the SSPP & WB register space.

Add the corresponding SSPP & WB ops and use them before/after calling the
QoS and OT limit setup functions.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v2:
- moved all force_clk_ctrl code out of vbif
- use major ver test to add force_clk_ctrl op
- do not add clk_ctrl reg into sspp/wb cap struct
- add WB2 on sm8550
- Link to v1: https://lore.kernel.org/r/20231009-topic-sm8550-graphics-sspp-split-clk-v1-0-806c0dee4e43@linaro.org

---
Neil Armstrong (5):
      drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper
      drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
      drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
      drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
      drm/msm/dpu: enable writeback on SM8550

 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 36 +++++++++----------
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        | 21 +++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        | 12 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c         | 23 +-----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c        | 21 +++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h        |  4 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c          | 20 +++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h          |  7 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
 13 files changed, 173 insertions(+), 88 deletions(-)
---
base-commit: 9119cf579b4432b36be9d33a92f4331922067d92
change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 0/5] drm/msm/dpu: correctly implement SSPP & WB Clock Control Split
@ 2023-10-11 11:59 ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

Starting with the SM8550 platform, the SSPP & WB Clock Controls are
no more in the MDP TOP registers, but in the SSPP & WB register space.

Add the corresponding SSPP & WB ops and use them before/after calling the
QoS and OT limit setup functions.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v2:
- moved all force_clk_ctrl code out of vbif
- use major ver test to add force_clk_ctrl op
- do not add clk_ctrl reg into sspp/wb cap struct
- add WB2 on sm8550
- Link to v1: https://lore.kernel.org/r/20231009-topic-sm8550-graphics-sspp-split-clk-v1-0-806c0dee4e43@linaro.org

---
Neil Armstrong (5):
      drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper
      drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
      drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
      drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
      drm/msm/dpu: enable writeback on SM8550

 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 36 +++++++++----------
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        | 21 +++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        | 12 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c         | 23 +-----------
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c        | 21 +++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h        |  4 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c          | 20 +++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h          |  7 +++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c             |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
 13 files changed, 173 insertions(+), 88 deletions(-)
---
base-commit: 9119cf579b4432b36be9d33a92f4331922067d92
change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/5] drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper
  2023-10-11 11:59 ` Neil Armstrong
@ 2023-10-11 11:59   ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

Add an helper to setup the force clock control as it will
be used in multiple HW files.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c  | 23 +----------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h |  4 ++++
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index cff48763ce25..24e734768a72 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -66,34 +66,13 @@ static void dpu_hw_setup_split_pipe(struct dpu_hw_mdp *mdp,
 static bool dpu_hw_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
 		enum dpu_clk_ctrl_type clk_ctrl, bool enable)
 {
-	struct dpu_hw_blk_reg_map *c;
-	u32 reg_off, bit_off;
-	u32 reg_val, new_val;
-	bool clk_forced_on;
-
 	if (!mdp)
 		return false;
 
-	c = &mdp->hw;
-
 	if (clk_ctrl <= DPU_CLK_CTRL_NONE || clk_ctrl >= DPU_CLK_CTRL_MAX)
 		return false;
 
-	reg_off = mdp->caps->clk_ctrls[clk_ctrl].reg_off;
-	bit_off = mdp->caps->clk_ctrls[clk_ctrl].bit_off;
-
-	reg_val = DPU_REG_READ(c, reg_off);
-
-	if (enable)
-		new_val = reg_val | BIT(bit_off);
-	else
-		new_val = reg_val & ~BIT(bit_off);
-
-	DPU_REG_WRITE(c, reg_off, new_val);
-
-	clk_forced_on = !(reg_val & BIT(bit_off));
-
-	return clk_forced_on;
+	return dpu_hw_clk_force_ctrl(&mdp->hw, &mdp->caps->clk_ctrls[clk_ctrl], enable);
 }
 
 
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 9d2273fd2fed..18b16b2d2bf5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
@@ -546,3 +546,24 @@ void dpu_setup_cdp(struct dpu_hw_blk_reg_map *c, u32 offset,
 
 	DPU_REG_WRITE(c, offset, cdp_cntl);
 }
+
+bool dpu_hw_clk_force_ctrl(struct dpu_hw_blk_reg_map *c,
+			   const struct dpu_clk_ctrl_reg *clk_ctrl_reg,
+			   bool enable)
+{
+	u32 reg_val, new_val;
+	bool clk_forced_on;
+
+	reg_val = DPU_REG_READ(c, clk_ctrl_reg->reg_off);
+
+	if (enable)
+		new_val = reg_val | BIT(clk_ctrl_reg->bit_off);
+	else
+		new_val = reg_val & ~BIT(clk_ctrl_reg->bit_off);
+
+	DPU_REG_WRITE(c, clk_ctrl_reg->reg_off, new_val);
+
+	clk_forced_on = !(reg_val & BIT(clk_ctrl_reg->bit_off));
+
+	return clk_forced_on;
+}
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 1f6079f47071..4bea139081bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -367,4 +367,8 @@ int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c,
 		u32 misr_signature_offset,
 		u32 *misr_value);
 
+bool dpu_hw_clk_force_ctrl(struct dpu_hw_blk_reg_map *c,
+			   const struct dpu_clk_ctrl_reg *clk_ctrl_reg,
+			   bool enable);
+
 #endif /* _DPU_HW_UTIL_H */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 1/5] drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper
@ 2023-10-11 11:59   ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

Add an helper to setup the force clock control as it will
be used in multiple HW files.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c  | 23 +----------------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c | 21 +++++++++++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h |  4 ++++
 3 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index cff48763ce25..24e734768a72 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -66,34 +66,13 @@ static void dpu_hw_setup_split_pipe(struct dpu_hw_mdp *mdp,
 static bool dpu_hw_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
 		enum dpu_clk_ctrl_type clk_ctrl, bool enable)
 {
-	struct dpu_hw_blk_reg_map *c;
-	u32 reg_off, bit_off;
-	u32 reg_val, new_val;
-	bool clk_forced_on;
-
 	if (!mdp)
 		return false;
 
-	c = &mdp->hw;
-
 	if (clk_ctrl <= DPU_CLK_CTRL_NONE || clk_ctrl >= DPU_CLK_CTRL_MAX)
 		return false;
 
-	reg_off = mdp->caps->clk_ctrls[clk_ctrl].reg_off;
-	bit_off = mdp->caps->clk_ctrls[clk_ctrl].bit_off;
-
-	reg_val = DPU_REG_READ(c, reg_off);
-
-	if (enable)
-		new_val = reg_val | BIT(bit_off);
-	else
-		new_val = reg_val & ~BIT(bit_off);
-
-	DPU_REG_WRITE(c, reg_off, new_val);
-
-	clk_forced_on = !(reg_val & BIT(bit_off));
-
-	return clk_forced_on;
+	return dpu_hw_clk_force_ctrl(&mdp->hw, &mdp->caps->clk_ctrls[clk_ctrl], enable);
 }
 
 
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 9d2273fd2fed..18b16b2d2bf5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.c
@@ -546,3 +546,24 @@ void dpu_setup_cdp(struct dpu_hw_blk_reg_map *c, u32 offset,
 
 	DPU_REG_WRITE(c, offset, cdp_cntl);
 }
+
+bool dpu_hw_clk_force_ctrl(struct dpu_hw_blk_reg_map *c,
+			   const struct dpu_clk_ctrl_reg *clk_ctrl_reg,
+			   bool enable)
+{
+	u32 reg_val, new_val;
+	bool clk_forced_on;
+
+	reg_val = DPU_REG_READ(c, clk_ctrl_reg->reg_off);
+
+	if (enable)
+		new_val = reg_val | BIT(clk_ctrl_reg->bit_off);
+	else
+		new_val = reg_val & ~BIT(clk_ctrl_reg->bit_off);
+
+	DPU_REG_WRITE(c, clk_ctrl_reg->reg_off, new_val);
+
+	clk_forced_on = !(reg_val & BIT(clk_ctrl_reg->bit_off));
+
+	return clk_forced_on;
+}
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 1f6079f47071..4bea139081bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h
@@ -367,4 +367,8 @@ int dpu_hw_collect_misr(struct dpu_hw_blk_reg_map *c,
 		u32 misr_signature_offset,
 		u32 *misr_value);
 
+bool dpu_hw_clk_force_ctrl(struct dpu_hw_blk_reg_map *c,
+			   const struct dpu_clk_ctrl_reg *clk_ctrl_reg,
+			   bool enable);
+
 #endif /* _DPU_HW_UTIL_H */

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
  2023-10-11 11:59 ` Neil Armstrong
@ 2023-10-11 11:59   ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

Starting from SM8550, the SSPP & WB clock controls are moved
the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
downstream.

Implement setup_clk_force_ctrl() only starting from major version 9
which corresponds to SM8550 MDSS.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
 5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -69,6 +69,7 @@
 #define SSPP_EXCL_REC_XY_REC1              0x188
 #define SSPP_EXCL_REC_SIZE                 0x1B4
 #define SSPP_EXCL_REC_XY                   0x1B8
+#define SSPP_CLK_CTRL                      0x330
 
 /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
 #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
@@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
 	dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
 }
 
+static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
+{
+	struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
+		.reg_off = SSPP_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)
+		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;
@@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
 
 	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
@@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
 #endif
 
 struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
-		void __iomem *addr, const struct msm_mdss_data *mdss_data)
+		void __iomem *addr, const struct msm_mdss_data *mdss_data,
+		const struct dpu_mdss_version *mdss_rev)
 {
 	struct dpu_hw_sspp *hw_pipe;
 
@@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
 	hw_pipe->ubwc = mdss_data;
 	hw_pipe->idx = cfg->id;
 	hw_pipe->cap = cfg;
-	_setup_layer_ops(hw_pipe, hw_pipe->cap->features);
+	_setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
 
 	return hw_pipe;
 }
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 cbf4f95ff0fd..f93969fddb22 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
 	void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
 			       bool danger_safe_en);
 
+	/**
+	 * setup_clk_force_ctrl - setup clock force control
+	 * @ctx: Pointer to pipe context
+	 * @enable: enable clock force if true
+	 */
+	bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
+				     bool enable);
+
 	/**
 	 * setup_histogram - setup histograms
 	 * @ctx: Pointer to pipe context
@@ -334,9 +342,11 @@ struct dpu_kms;
  * @cfg:  Pipe catalog entry for which driver object is required
  * @addr: Mapped register io address of MDP
  * @mdss_data: UBWC / MDSS configuration data
+ * @mdss_rev: dpu core's major and minor versions
  */
 struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
-		void __iomem *addr, const struct msm_mdss_data *mdss_data);
+		void __iomem *addr, const struct msm_mdss_data *mdss_data,
+		const struct dpu_mdss_version *mdss_rev);
 
 /**
  * dpu_hw_sspp_destroy(): Destroys SSPP driver context
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
index ebc416400382..374c2c64c9e4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -43,6 +43,7 @@
 #define WB_MUX                                0x150
 #define WB_CROP_CTRL                          0x154
 #define WB_CROP_OFFSET                        0x158
+#define WB_CLK_CTRL                           0x178
 #define WB_CSC_BASE                           0x260
 #define WB_DST_ADDR_SW_STATUS                 0x2B0
 #define WB_CDP_CNTL                           0x2B4
@@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
 	DPU_REG_WRITE(c, WB_MUX, mux_cfg);
 }
 
+static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
+{
+	struct dpu_clk_ctrl_reg wb_clk_ctrl = {
+		.reg_off = WB_CLK_CTRL,
+		.bit_off = 0
+	};
+
+	return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
+}
+
 static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
-		unsigned long features)
+		unsigned long features, const struct dpu_mdss_version *mdss_rev)
 {
 	ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
 	ops->setup_outformat = dpu_hw_wb_setup_format;
@@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
 
 	if (test_bit(DPU_WB_INPUT_CTRL, &features))
 		ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
+
+	if (mdss_rev->core_major_ver >= 9)
+		ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
 }
 
 struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
-		void __iomem *addr)
+		void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
 {
 	struct dpu_hw_wb *c;
 
@@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
 	/* Assign ops */
 	c->idx = cfg->id;
 	c->caps = cfg;
-	_setup_wb_ops(&c->ops, c->caps->features);
+	_setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
 
 	return c;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
index 2d7db2efa3d0..88792f450a92 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
@@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
  *  @setup_outformat: setup output format of writeback block from writeback job
  *  @setup_qos_lut:   setup qos LUT for writeback block based on input
  *  @setup_cdp:       setup chroma down prefetch block for writeback block
+ *  @setup_clk_force_ctrl: setup clock force control
  *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
  */
 struct dpu_hw_wb_ops {
@@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
 			  const struct dpu_format *fmt,
 			  bool enable);
 
+	bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
+				     bool enable);
+
 	void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
 				  const enum dpu_pingpong pp);
 };
@@ -74,10 +78,11 @@ struct dpu_hw_wb {
  * dpu_hw_wb_init() - Initializes the writeback hw driver object.
  * @cfg:  wb_path catalog entry for which driver object is required
  * @addr: mapped register io address of MDP
+ * @mdss_rev: dpu core's major and minor versions
  * Return: Error code or allocated dpu_hw_wb context
  */
 struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
-		void __iomem *addr);
+		void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
 
 /**
  * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..f363bcfdfd70 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
 		struct dpu_hw_wb *hw;
 		const struct dpu_wb_cfg *wb = &cat->wb[i];
 
-		hw = dpu_hw_wb_init(wb, mmio);
+		hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
 		if (IS_ERR(hw)) {
 			rc = PTR_ERR(hw);
 			DPU_ERROR("failed wb object creation: err %d\n", rc);
@@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
 		struct dpu_hw_sspp *hw;
 		const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
 
-		hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
+		hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
 		if (IS_ERR(hw)) {
 			rc = PTR_ERR(hw);
 			DPU_ERROR("failed sspp object creation: err %d\n", rc);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
@ 2023-10-11 11:59   ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

Starting from SM8550, the SSPP & WB clock controls are moved
the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
downstream.

Implement setup_clk_force_ctrl() only starting from major version 9
which corresponds to SM8550 MDSS.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
 drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
 5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -69,6 +69,7 @@
 #define SSPP_EXCL_REC_XY_REC1              0x188
 #define SSPP_EXCL_REC_SIZE                 0x1B4
 #define SSPP_EXCL_REC_XY                   0x1B8
+#define SSPP_CLK_CTRL                      0x330
 
 /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
 #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
@@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
 	dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
 }
 
+static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
+{
+	struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
+		.reg_off = SSPP_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)
+		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;
@@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
 
 	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
@@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
 #endif
 
 struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
-		void __iomem *addr, const struct msm_mdss_data *mdss_data)
+		void __iomem *addr, const struct msm_mdss_data *mdss_data,
+		const struct dpu_mdss_version *mdss_rev)
 {
 	struct dpu_hw_sspp *hw_pipe;
 
@@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
 	hw_pipe->ubwc = mdss_data;
 	hw_pipe->idx = cfg->id;
 	hw_pipe->cap = cfg;
-	_setup_layer_ops(hw_pipe, hw_pipe->cap->features);
+	_setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
 
 	return hw_pipe;
 }
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 cbf4f95ff0fd..f93969fddb22 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
 	void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
 			       bool danger_safe_en);
 
+	/**
+	 * setup_clk_force_ctrl - setup clock force control
+	 * @ctx: Pointer to pipe context
+	 * @enable: enable clock force if true
+	 */
+	bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
+				     bool enable);
+
 	/**
 	 * setup_histogram - setup histograms
 	 * @ctx: Pointer to pipe context
@@ -334,9 +342,11 @@ struct dpu_kms;
  * @cfg:  Pipe catalog entry for which driver object is required
  * @addr: Mapped register io address of MDP
  * @mdss_data: UBWC / MDSS configuration data
+ * @mdss_rev: dpu core's major and minor versions
  */
 struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
-		void __iomem *addr, const struct msm_mdss_data *mdss_data);
+		void __iomem *addr, const struct msm_mdss_data *mdss_data,
+		const struct dpu_mdss_version *mdss_rev);
 
 /**
  * dpu_hw_sspp_destroy(): Destroys SSPP driver context
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
index ebc416400382..374c2c64c9e4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -43,6 +43,7 @@
 #define WB_MUX                                0x150
 #define WB_CROP_CTRL                          0x154
 #define WB_CROP_OFFSET                        0x158
+#define WB_CLK_CTRL                           0x178
 #define WB_CSC_BASE                           0x260
 #define WB_DST_ADDR_SW_STATUS                 0x2B0
 #define WB_CDP_CNTL                           0x2B4
@@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
 	DPU_REG_WRITE(c, WB_MUX, mux_cfg);
 }
 
+static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
+{
+	struct dpu_clk_ctrl_reg wb_clk_ctrl = {
+		.reg_off = WB_CLK_CTRL,
+		.bit_off = 0
+	};
+
+	return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
+}
+
 static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
-		unsigned long features)
+		unsigned long features, const struct dpu_mdss_version *mdss_rev)
 {
 	ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
 	ops->setup_outformat = dpu_hw_wb_setup_format;
@@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
 
 	if (test_bit(DPU_WB_INPUT_CTRL, &features))
 		ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
+
+	if (mdss_rev->core_major_ver >= 9)
+		ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
 }
 
 struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
-		void __iomem *addr)
+		void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
 {
 	struct dpu_hw_wb *c;
 
@@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
 	/* Assign ops */
 	c->idx = cfg->id;
 	c->caps = cfg;
-	_setup_wb_ops(&c->ops, c->caps->features);
+	_setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
 
 	return c;
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
index 2d7db2efa3d0..88792f450a92 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
@@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
  *  @setup_outformat: setup output format of writeback block from writeback job
  *  @setup_qos_lut:   setup qos LUT for writeback block based on input
  *  @setup_cdp:       setup chroma down prefetch block for writeback block
+ *  @setup_clk_force_ctrl: setup clock force control
  *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
  */
 struct dpu_hw_wb_ops {
@@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
 			  const struct dpu_format *fmt,
 			  bool enable);
 
+	bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
+				     bool enable);
+
 	void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
 				  const enum dpu_pingpong pp);
 };
@@ -74,10 +78,11 @@ struct dpu_hw_wb {
  * dpu_hw_wb_init() - Initializes the writeback hw driver object.
  * @cfg:  wb_path catalog entry for which driver object is required
  * @addr: mapped register io address of MDP
+ * @mdss_rev: dpu core's major and minor versions
  * Return: Error code or allocated dpu_hw_wb context
  */
 struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
-		void __iomem *addr);
+		void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
 
 /**
  * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f9215643c71a..f363bcfdfd70 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
 		struct dpu_hw_wb *hw;
 		const struct dpu_wb_cfg *wb = &cat->wb[i];
 
-		hw = dpu_hw_wb_init(wb, mmio);
+		hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
 		if (IS_ERR(hw)) {
 			rc = PTR_ERR(hw);
 			DPU_ERROR("failed wb object creation: err %d\n", rc);
@@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
 		struct dpu_hw_sspp *hw;
 		const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
 
-		hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
+		hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
 		if (IS_ERR(hw)) {
 			rc = PTR_ERR(hw);
 			DPU_ERROR("failed sspp object creation: err %d\n", rc);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
  2023-10-11 11:59 ` Neil Armstrong
@ 2023-10-11 11:59   ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

Now SSPP and WB can have setup_force_clk_ctrl() ops, it's simpler to call
them from the plane and wb code and call into the mdp ops if not present.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
 4 files changed, 77 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 78037a697633..8802e007f8e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -34,6 +34,23 @@ static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys *phys_enc)
 	return true;
 }
 
+static bool _dpu_encoder_phys_wb_clk_force_ctrl(struct dpu_hw_wb *wb,
+						struct dpu_hw_mdp *mdp,
+						bool enable, bool *forced_on)
+{
+	if (wb->ops.setup_clk_force_ctrl) {
+		*forced_on = wb->ops.setup_clk_force_ctrl(wb, enable);
+		return true;
+	}
+
+	if (mdp->ops.setup_clk_force_ctrl) {
+		*forced_on = mdp->ops.setup_clk_force_ctrl(mdp, wb->caps->clk_ctrl, enable);
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback interface
  * @phys_enc:	Pointer to physical encoder
@@ -43,6 +60,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
 {
 	struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
 	struct dpu_vbif_set_ot_params ot_params;
+	bool forced_on = false;
 
 	memset(&ot_params, 0, sizeof(ot_params));
 	ot_params.xin_id = hw_wb->caps->xin_id;
@@ -52,10 +70,17 @@ static void dpu_encoder_phys_wb_set_ot_limit(
 	ot_params.is_wfd = true;
 	ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
 	ot_params.vbif_idx = hw_wb->caps->vbif_idx;
-	ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	ot_params.rd = false;
 
+	if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						 true, &forced_on))
+		return;
+
 	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
+
+	if (forced_on)
+		_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						    false, &forced_on);
 }
 
 /**
@@ -67,6 +92,7 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 {
 	struct dpu_hw_wb *hw_wb;
 	struct dpu_vbif_set_qos_params qos_params;
+	bool forced_on = false;
 
 	if (!phys_enc || !phys_enc->parent || !phys_enc->parent->crtc) {
 		DPU_ERROR("invalid arguments\n");
@@ -83,7 +109,6 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = hw_wb->caps->vbif_idx;
 	qos_params.xin_id = hw_wb->caps->xin_id;
-	qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	qos_params.num = hw_wb->idx - WB_0;
 	qos_params.is_rt = false;
 
@@ -92,7 +117,15 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 			qos_params.vbif_idx,
 			qos_params.xin_id, qos_params.is_rt);
 
+	if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						 true, &forced_on))
+		return;
+
 	dpu_vbif_set_qos_remap(phys_enc->dpu_kms, &qos_params);
+
+	if (forced_on)
+		_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						    false, &forced_on);
 }
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..c63cae8fb35c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -333,6 +333,23 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
 				       enable);
 }
 
+static bool _dpu_plane_sspp_clk_force_ctrl(struct dpu_hw_sspp *sspp,
+					   struct dpu_hw_mdp *mdp,
+					   bool enable, bool *forced_on)
+{
+	if (sspp->ops.setup_clk_force_ctrl) {
+		*forced_on = sspp->ops.setup_clk_force_ctrl(sspp, enable);
+		return true;
+	}
+
+	if (mdp->ops.setup_clk_force_ctrl) {
+		*forced_on = mdp->ops.setup_clk_force_ctrl(mdp, sspp->cap->clk_ctrl, enable);
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * _dpu_plane_set_ot_limit - set OT limit for the given plane
  * @plane:		Pointer to drm plane
@@ -348,6 +365,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_vbif_set_ot_params ot_params;
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	bool forced_on = false;
 
 	memset(&ot_params, 0, sizeof(ot_params));
 	ot_params.xin_id = pipe->sspp->cap->xin_id;
@@ -357,10 +375,17 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	ot_params.is_wfd = !pdpu->is_rt_pipe;
 	ot_params.frame_rate = frame_rate;
 	ot_params.vbif_idx = VBIF_RT;
-	ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
 	ot_params.rd = true;
 
+	if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					    true, &forced_on))
+		return;
+
 	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
+
+	if (forced_on)
+		_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					       false, &forced_on);
 }
 
 /**
@@ -374,21 +399,28 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_vbif_set_qos_params qos_params;
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	bool forced_on = false;
 
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = VBIF_RT;
-	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
 	qos_params.xin_id = pipe->sspp->cap->xin_id;
 	qos_params.num = pipe->sspp->idx - SSPP_VIG0;
 	qos_params.is_rt = pdpu->is_rt_pipe;
 
-	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
+	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
 			qos_params.num,
 			qos_params.vbif_idx,
-			qos_params.xin_id, qos_params.is_rt,
-			qos_params.clk_ctrl);
+			qos_params.xin_id, qos_params.is_rt);
+
+	if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					    true, &forced_on))
+		return;
 
 	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
+
+	if (forced_on)
+		_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					       false, &forced_on);
 }
 
 static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
index 1305e250b71e..47c02b98eac3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
@@ -169,23 +169,16 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		struct dpu_vbif_set_ot_params *params)
 {
 	struct dpu_hw_vbif *vbif;
-	struct dpu_hw_mdp *mdp;
-	bool forced_on = false;
 	u32 ot_lim;
 	int ret;
 
-	mdp = dpu_kms->hw_mdp;
-
 	vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
-	if (!vbif || !mdp) {
-		DRM_DEBUG_ATOMIC("invalid arguments vbif %d mdp %d\n",
-				vbif != NULL, mdp != NULL);
+	if (!vbif) {
+		DRM_DEBUG_ATOMIC("invalid arguments vbif %d\n", vbif != NULL);
 		return;
 	}
 
-	if (!mdp->ops.setup_clk_force_ctrl ||
-			!vbif->ops.set_limit_conf ||
-			!vbif->ops.set_halt_ctrl)
+	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
 		return;
 
 	/* set write_gather_en for all write clients */
@@ -200,8 +193,6 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
 		params->vbif_idx);
 
-	forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
-
 	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
 
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, true);
@@ -211,25 +202,19 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		trace_dpu_vbif_wait_xin_halt_fail(vbif->idx, params->xin_id);
 
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
-
-	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
 }
 
 void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		struct dpu_vbif_set_qos_params *params)
 {
 	struct dpu_hw_vbif *vbif;
-	struct dpu_hw_mdp *mdp;
-	bool forced_on = false;
 	const struct dpu_vbif_qos_tbl *qos_tbl;
 	int i;
 
-	if (!params || !dpu_kms->hw_mdp) {
+	if (!params) {
 		DPU_ERROR("invalid arguments\n");
 		return;
 	}
-	mdp = dpu_kms->hw_mdp;
 
 	vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
 
@@ -238,7 +223,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
+	if (!vbif->ops.set_qos_remap) {
 		DRM_DEBUG_ATOMIC("qos remap not supported\n");
 		return;
 	}
@@ -251,8 +236,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
-
 	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
 		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
 				dpu_vbif_name(params->vbif_idx), params->xin_id, i,
@@ -260,9 +243,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		vbif->ops.set_qos_remap(vbif, params->xin_id, i,
 				qos_tbl->priority_lvl[i]);
 	}
-
-	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
 }
 
 void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
index ab490177d886..e1b1f7f4e4be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
@@ -16,13 +16,11 @@ struct dpu_vbif_set_ot_params {
 	bool rd;
 	bool is_wfd;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 };
 
 struct dpu_vbif_set_memtype_params {
 	u32 xin_id;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 	bool is_cacheable;
 };
 
@@ -30,14 +28,12 @@ struct dpu_vbif_set_memtype_params {
  * struct dpu_vbif_set_qos_params - QoS remapper parameter
  * @vbif_idx: vbif identifier
  * @xin_id: client interface identifier
- * @clk_ctrl: clock control identifier of the xin
  * @num: pipe identifier (debug only)
  * @is_rt: true if pipe is used in real-time use case
  */
 struct dpu_vbif_set_qos_params {
 	u32 vbif_idx;
 	u32 xin_id;
-	u32 clk_ctrl;
 	u32 num;
 	bool is_rt;
 };

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
@ 2023-10-11 11:59   ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

Now SSPP and WB can have setup_force_clk_ctrl() ops, it's simpler to call
them from the plane and wb code and call into the mdp ops if not present.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
 4 files changed, 77 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
index 78037a697633..8802e007f8e2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
@@ -34,6 +34,23 @@ static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys *phys_enc)
 	return true;
 }
 
+static bool _dpu_encoder_phys_wb_clk_force_ctrl(struct dpu_hw_wb *wb,
+						struct dpu_hw_mdp *mdp,
+						bool enable, bool *forced_on)
+{
+	if (wb->ops.setup_clk_force_ctrl) {
+		*forced_on = wb->ops.setup_clk_force_ctrl(wb, enable);
+		return true;
+	}
+
+	if (mdp->ops.setup_clk_force_ctrl) {
+		*forced_on = mdp->ops.setup_clk_force_ctrl(mdp, wb->caps->clk_ctrl, enable);
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback interface
  * @phys_enc:	Pointer to physical encoder
@@ -43,6 +60,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
 {
 	struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
 	struct dpu_vbif_set_ot_params ot_params;
+	bool forced_on = false;
 
 	memset(&ot_params, 0, sizeof(ot_params));
 	ot_params.xin_id = hw_wb->caps->xin_id;
@@ -52,10 +70,17 @@ static void dpu_encoder_phys_wb_set_ot_limit(
 	ot_params.is_wfd = true;
 	ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
 	ot_params.vbif_idx = hw_wb->caps->vbif_idx;
-	ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	ot_params.rd = false;
 
+	if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						 true, &forced_on))
+		return;
+
 	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
+
+	if (forced_on)
+		_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						    false, &forced_on);
 }
 
 /**
@@ -67,6 +92,7 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 {
 	struct dpu_hw_wb *hw_wb;
 	struct dpu_vbif_set_qos_params qos_params;
+	bool forced_on = false;
 
 	if (!phys_enc || !phys_enc->parent || !phys_enc->parent->crtc) {
 		DPU_ERROR("invalid arguments\n");
@@ -83,7 +109,6 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = hw_wb->caps->vbif_idx;
 	qos_params.xin_id = hw_wb->caps->xin_id;
-	qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
 	qos_params.num = hw_wb->idx - WB_0;
 	qos_params.is_rt = false;
 
@@ -92,7 +117,15 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 			qos_params.vbif_idx,
 			qos_params.xin_id, qos_params.is_rt);
 
+	if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						 true, &forced_on))
+		return;
+
 	dpu_vbif_set_qos_remap(phys_enc->dpu_kms, &qos_params);
+
+	if (forced_on)
+		_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
+						    false, &forced_on);
 }
 
 /**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..c63cae8fb35c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -333,6 +333,23 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
 				       enable);
 }
 
+static bool _dpu_plane_sspp_clk_force_ctrl(struct dpu_hw_sspp *sspp,
+					   struct dpu_hw_mdp *mdp,
+					   bool enable, bool *forced_on)
+{
+	if (sspp->ops.setup_clk_force_ctrl) {
+		*forced_on = sspp->ops.setup_clk_force_ctrl(sspp, enable);
+		return true;
+	}
+
+	if (mdp->ops.setup_clk_force_ctrl) {
+		*forced_on = mdp->ops.setup_clk_force_ctrl(mdp, sspp->cap->clk_ctrl, enable);
+		return true;
+	}
+
+	return false;
+}
+
 /**
  * _dpu_plane_set_ot_limit - set OT limit for the given plane
  * @plane:		Pointer to drm plane
@@ -348,6 +365,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_vbif_set_ot_params ot_params;
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	bool forced_on = false;
 
 	memset(&ot_params, 0, sizeof(ot_params));
 	ot_params.xin_id = pipe->sspp->cap->xin_id;
@@ -357,10 +375,17 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	ot_params.is_wfd = !pdpu->is_rt_pipe;
 	ot_params.frame_rate = frame_rate;
 	ot_params.vbif_idx = VBIF_RT;
-	ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
 	ot_params.rd = true;
 
+	if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					    true, &forced_on))
+		return;
+
 	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
+
+	if (forced_on)
+		_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					       false, &forced_on);
 }
 
 /**
@@ -374,21 +399,28 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	struct dpu_vbif_set_qos_params qos_params;
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
+	bool forced_on = false;
 
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = VBIF_RT;
-	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
 	qos_params.xin_id = pipe->sspp->cap->xin_id;
 	qos_params.num = pipe->sspp->idx - SSPP_VIG0;
 	qos_params.is_rt = pdpu->is_rt_pipe;
 
-	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
+	DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
 			qos_params.num,
 			qos_params.vbif_idx,
-			qos_params.xin_id, qos_params.is_rt,
-			qos_params.clk_ctrl);
+			qos_params.xin_id, qos_params.is_rt);
+
+	if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					    true, &forced_on))
+		return;
 
 	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
+
+	if (forced_on)
+		_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
+					       false, &forced_on);
 }
 
 static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
index 1305e250b71e..47c02b98eac3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
@@ -169,23 +169,16 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		struct dpu_vbif_set_ot_params *params)
 {
 	struct dpu_hw_vbif *vbif;
-	struct dpu_hw_mdp *mdp;
-	bool forced_on = false;
 	u32 ot_lim;
 	int ret;
 
-	mdp = dpu_kms->hw_mdp;
-
 	vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
-	if (!vbif || !mdp) {
-		DRM_DEBUG_ATOMIC("invalid arguments vbif %d mdp %d\n",
-				vbif != NULL, mdp != NULL);
+	if (!vbif) {
+		DRM_DEBUG_ATOMIC("invalid arguments vbif %d\n", vbif != NULL);
 		return;
 	}
 
-	if (!mdp->ops.setup_clk_force_ctrl ||
-			!vbif->ops.set_limit_conf ||
-			!vbif->ops.set_halt_ctrl)
+	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
 		return;
 
 	/* set write_gather_en for all write clients */
@@ -200,8 +193,6 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
 		params->vbif_idx);
 
-	forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
-
 	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
 
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, true);
@@ -211,25 +202,19 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		trace_dpu_vbif_wait_xin_halt_fail(vbif->idx, params->xin_id);
 
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
-
-	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
 }
 
 void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		struct dpu_vbif_set_qos_params *params)
 {
 	struct dpu_hw_vbif *vbif;
-	struct dpu_hw_mdp *mdp;
-	bool forced_on = false;
 	const struct dpu_vbif_qos_tbl *qos_tbl;
 	int i;
 
-	if (!params || !dpu_kms->hw_mdp) {
+	if (!params) {
 		DPU_ERROR("invalid arguments\n");
 		return;
 	}
-	mdp = dpu_kms->hw_mdp;
 
 	vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
 
@@ -238,7 +223,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
+	if (!vbif->ops.set_qos_remap) {
 		DRM_DEBUG_ATOMIC("qos remap not supported\n");
 		return;
 	}
@@ -251,8 +236,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
-
 	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
 		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
 				dpu_vbif_name(params->vbif_idx), params->xin_id, i,
@@ -260,9 +243,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		vbif->ops.set_qos_remap(vbif, params->xin_id, i,
 				qos_tbl->priority_lvl[i]);
 	}
-
-	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
 }
 
 void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
index ab490177d886..e1b1f7f4e4be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
@@ -16,13 +16,11 @@ struct dpu_vbif_set_ot_params {
 	bool rd;
 	bool is_wfd;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 };
 
 struct dpu_vbif_set_memtype_params {
 	u32 xin_id;
 	u32 vbif_idx;
-	u32 clk_ctrl;
 	bool is_cacheable;
 };
 
@@ -30,14 +28,12 @@ struct dpu_vbif_set_memtype_params {
  * struct dpu_vbif_set_qos_params - QoS remapper parameter
  * @vbif_idx: vbif identifier
  * @xin_id: client interface identifier
- * @clk_ctrl: clock control identifier of the xin
  * @num: pipe identifier (debug only)
  * @is_rt: true if pipe is used in real-time use case
  */
 struct dpu_vbif_set_qos_params {
 	u32 vbif_idx;
 	u32 xin_id;
-	u32 clk_ctrl;
 	u32 num;
 	bool is_rt;
 };

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
  2023-10-11 11:59 ` Neil Armstrong
@ 2023-10-11 11:59   ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

The SM8550 has the SSPP clk_ctrl in the SSPP registers, remove the
duplicate clock controls from the MDP top.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 7bed819dfc39..4590a01c1252 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -24,16 +24,6 @@ static const struct dpu_mdp_cfg sm8550_mdp = {
 	.base = 0, .len = 0x494,
 	.features = BIT(DPU_MDP_PERIPH_0_REMOVED),
 	.clk_ctrls = {
-		[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x4330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG1] = { .reg_off = 0x6330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG2] = { .reg_off = 0x8330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG3] = { .reg_off = 0xa330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x24330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x26330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x28330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2a330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA4] = { .reg_off = 0x2c330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA5] = { .reg_off = 0x2e330, .bit_off = 0 },
 		[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 },
 	},
 };
@@ -81,7 +71,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_0,
 		.xin_id = 0,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG0,
 	}, {
 		.name = "sspp_1", .id = SSPP_VIG1,
 		.base = 0x6000, .len = 0x344,
@@ -89,7 +78,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_1,
 		.xin_id = 4,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG1,
 	}, {
 		.name = "sspp_2", .id = SSPP_VIG2,
 		.base = 0x8000, .len = 0x344,
@@ -97,7 +85,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_2,
 		.xin_id = 8,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG2,
 	}, {
 		.name = "sspp_3", .id = SSPP_VIG3,
 		.base = 0xa000, .len = 0x344,
@@ -105,7 +92,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_3,
 		.xin_id = 12,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG3,
 	}, {
 		.name = "sspp_8", .id = SSPP_DMA0,
 		.base = 0x24000, .len = 0x344,
@@ -113,7 +99,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_0,
 		.xin_id = 1,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA0,
 	}, {
 		.name = "sspp_9", .id = SSPP_DMA1,
 		.base = 0x26000, .len = 0x344,
@@ -121,7 +106,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_1,
 		.xin_id = 5,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA1,
 	}, {
 		.name = "sspp_10", .id = SSPP_DMA2,
 		.base = 0x28000, .len = 0x344,
@@ -129,7 +113,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_2,
 		.xin_id = 9,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA2,
 	}, {
 		.name = "sspp_11", .id = SSPP_DMA3,
 		.base = 0x2a000, .len = 0x344,
@@ -137,7 +120,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_3,
 		.xin_id = 13,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA3,
 	}, {
 		.name = "sspp_12", .id = SSPP_DMA4,
 		.base = 0x2c000, .len = 0x344,
@@ -145,7 +127,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_dma_sblk_4,
 		.xin_id = 14,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA4,
 	}, {
 		.name = "sspp_13", .id = SSPP_DMA5,
 		.base = 0x2e000, .len = 0x344,
@@ -153,7 +134,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_dma_sblk_5,
 		.xin_id = 15,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA5,
 	},
 };
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
@ 2023-10-11 11:59   ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

The SM8550 has the SSPP clk_ctrl in the SSPP registers, remove the
duplicate clock controls from the MDP top.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 7bed819dfc39..4590a01c1252 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -24,16 +24,6 @@ static const struct dpu_mdp_cfg sm8550_mdp = {
 	.base = 0, .len = 0x494,
 	.features = BIT(DPU_MDP_PERIPH_0_REMOVED),
 	.clk_ctrls = {
-		[DPU_CLK_CTRL_VIG0] = { .reg_off = 0x4330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG1] = { .reg_off = 0x6330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG2] = { .reg_off = 0x8330, .bit_off = 0 },
-		[DPU_CLK_CTRL_VIG3] = { .reg_off = 0xa330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA0] = { .reg_off = 0x24330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA1] = { .reg_off = 0x26330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA2] = { .reg_off = 0x28330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA3] = { .reg_off = 0x2a330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA4] = { .reg_off = 0x2c330, .bit_off = 0 },
-		[DPU_CLK_CTRL_DMA5] = { .reg_off = 0x2e330, .bit_off = 0 },
 		[DPU_CLK_CTRL_REG_DMA] = { .reg_off = 0x2bc, .bit_off = 20 },
 	},
 };
@@ -81,7 +71,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_0,
 		.xin_id = 0,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG0,
 	}, {
 		.name = "sspp_1", .id = SSPP_VIG1,
 		.base = 0x6000, .len = 0x344,
@@ -89,7 +78,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_1,
 		.xin_id = 4,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG1,
 	}, {
 		.name = "sspp_2", .id = SSPP_VIG2,
 		.base = 0x8000, .len = 0x344,
@@ -97,7 +85,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_2,
 		.xin_id = 8,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG2,
 	}, {
 		.name = "sspp_3", .id = SSPP_VIG3,
 		.base = 0xa000, .len = 0x344,
@@ -105,7 +92,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_vig_sblk_3,
 		.xin_id = 12,
 		.type = SSPP_TYPE_VIG,
-		.clk_ctrl = DPU_CLK_CTRL_VIG3,
 	}, {
 		.name = "sspp_8", .id = SSPP_DMA0,
 		.base = 0x24000, .len = 0x344,
@@ -113,7 +99,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_0,
 		.xin_id = 1,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA0,
 	}, {
 		.name = "sspp_9", .id = SSPP_DMA1,
 		.base = 0x26000, .len = 0x344,
@@ -121,7 +106,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_1,
 		.xin_id = 5,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA1,
 	}, {
 		.name = "sspp_10", .id = SSPP_DMA2,
 		.base = 0x28000, .len = 0x344,
@@ -129,7 +113,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_2,
 		.xin_id = 9,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA2,
 	}, {
 		.name = "sspp_11", .id = SSPP_DMA3,
 		.base = 0x2a000, .len = 0x344,
@@ -137,7 +120,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sdm845_dma_sblk_3,
 		.xin_id = 13,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA3,
 	}, {
 		.name = "sspp_12", .id = SSPP_DMA4,
 		.base = 0x2c000, .len = 0x344,
@@ -145,7 +127,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_dma_sblk_4,
 		.xin_id = 14,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA4,
 	}, {
 		.name = "sspp_13", .id = SSPP_DMA5,
 		.base = 0x2e000, .len = 0x344,
@@ -153,7 +134,6 @@ static const struct dpu_sspp_cfg sm8550_sspp[] = {
 		.sblk = &sm8550_dma_sblk_5,
 		.xin_id = 15,
 		.type = SSPP_TYPE_DMA,
-		.clk_ctrl = DPU_CLK_CTRL_DMA5,
 	},
 };
 

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550
  2023-10-11 11:59 ` Neil Armstrong
@ 2023-10-11 11:59   ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Neil Armstrong

Enable WB2 hardware block, enabling writeback support on this platform.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 4590a01c1252..d83a68a2cc0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -321,6 +321,20 @@ static const struct dpu_dsc_cfg sm8550_dsc[] = {
 	},
 };
 
+static const struct dpu_wb_cfg sm8550_wb[] = {
+	{
+		.name = "wb_2", .id = WB_2,
+		.base = 0x65000, .len = 0x2c8,
+		.features = WB_SM8250_MASK,
+		.format_list = wb2_formats,
+		.num_formats = ARRAY_SIZE(wb2_formats),
+		.xin_id = 6,
+		.vbif_idx = VBIF_RT,
+		.maxlinewidth = 4096,
+		.intr_wb_done = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 4),
+	},
+};
+
 static const struct dpu_intf_cfg sm8550_intf[] = {
 	{
 		.name = "intf_0", .id = INTF_0,
@@ -418,6 +432,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
 	.dsc = sm8550_dsc,
 	.merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
 	.merge_3d = sm8550_merge_3d,
+	.wb_count = ARRAY_SIZE(sm8550_wb),
+	.wb = sm8550_wb,
 	.intf_count = ARRAY_SIZE(sm8550_intf),
 	.intf = sm8550_intf,
 	.vbif_count = ARRAY_SIZE(sm8550_vbif),

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550
@ 2023-10-11 11:59   ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 11:59 UTC (permalink / raw)
  To: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
	Marijn Suijten, David Airlie, Daniel Vetter
  Cc: linux-arm-msm, freedreno, linux-kernel, dri-devel, Neil Armstrong

Enable WB2 hardware block, enabling writeback support on this platform.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 4590a01c1252..d83a68a2cc0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -321,6 +321,20 @@ static const struct dpu_dsc_cfg sm8550_dsc[] = {
 	},
 };
 
+static const struct dpu_wb_cfg sm8550_wb[] = {
+	{
+		.name = "wb_2", .id = WB_2,
+		.base = 0x65000, .len = 0x2c8,
+		.features = WB_SM8250_MASK,
+		.format_list = wb2_formats,
+		.num_formats = ARRAY_SIZE(wb2_formats),
+		.xin_id = 6,
+		.vbif_idx = VBIF_RT,
+		.maxlinewidth = 4096,
+		.intr_wb_done = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 4),
+	},
+};
+
 static const struct dpu_intf_cfg sm8550_intf[] = {
 	{
 		.name = "intf_0", .id = INTF_0,
@@ -418,6 +432,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
 	.dsc = sm8550_dsc,
 	.merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
 	.merge_3d = sm8550_merge_3d,
+	.wb_count = ARRAY_SIZE(sm8550_wb),
+	.wb = sm8550_wb,
 	.intf_count = ARRAY_SIZE(sm8550_intf),
 	.intf = sm8550_intf,
 	.vbif_count = ARRAY_SIZE(sm8550_vbif),

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
  2023-10-11 11:59   ` Neil Armstrong
@ 2023-10-11 12:45     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:45 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
	linux-kernel

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Starting from SM8550, the SSPP & WB clock controls are moved
> the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
> downstream.
>
> Implement setup_clk_force_ctrl() only starting from major version 9
> which corresponds to SM8550 MDSS.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

With two minor issues below fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
>  5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -69,6 +69,7 @@
>  #define SSPP_EXCL_REC_XY_REC1              0x188
>  #define SSPP_EXCL_REC_SIZE                 0x1B4
>  #define SSPP_EXCL_REC_XY                   0x1B8
> +#define SSPP_CLK_CTRL                      0x330
>
>  /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
>  #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
> @@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
>         dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
>  }
>
> +static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
> +{
> +       struct dpu_clk_ctrl_reg sspp_clk_ctrl = {

Nit: static const?

> +               .reg_off = SSPP_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)
> +               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;
> @@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>
>         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
> @@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>  #endif
>
>  struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
> -               void __iomem *addr, const struct msm_mdss_data *mdss_data)
> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
> +               const struct dpu_mdss_version *mdss_rev)
>  {
>         struct dpu_hw_sspp *hw_pipe;
>
> @@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>         hw_pipe->ubwc = mdss_data;
>         hw_pipe->idx = cfg->id;
>         hw_pipe->cap = cfg;
> -       _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
> +       _setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
>
>         return hw_pipe;
>  }
> 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 cbf4f95ff0fd..f93969fddb22 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> @@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
>         void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
>                                bool danger_safe_en);
>
> +       /**
> +        * setup_clk_force_ctrl - setup clock force control
> +        * @ctx: Pointer to pipe context
> +        * @enable: enable clock force if true
> +        */
> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
> +                                    bool enable);
> +
>         /**
>          * setup_histogram - setup histograms
>          * @ctx: Pointer to pipe context
> @@ -334,9 +342,11 @@ struct dpu_kms;
>   * @cfg:  Pipe catalog entry for which driver object is required
>   * @addr: Mapped register io address of MDP
>   * @mdss_data: UBWC / MDSS configuration data
> + * @mdss_rev: dpu core's major and minor versions
>   */
>  struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
> -               void __iomem *addr, const struct msm_mdss_data *mdss_data);
> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
> +               const struct dpu_mdss_version *mdss_rev);
>
>  /**
>   * dpu_hw_sspp_destroy(): Destroys SSPP driver context
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> index ebc416400382..374c2c64c9e4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> @@ -43,6 +43,7 @@
>  #define WB_MUX                                0x150
>  #define WB_CROP_CTRL                          0x154
>  #define WB_CROP_OFFSET                        0x158
> +#define WB_CLK_CTRL                           0x178
>  #define WB_CSC_BASE                           0x260
>  #define WB_DST_ADDR_SW_STATUS                 0x2B0
>  #define WB_CDP_CNTL                           0x2B4
> @@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
>         DPU_REG_WRITE(c, WB_MUX, mux_cfg);
>  }
>
> +static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
> +{
> +       struct dpu_clk_ctrl_reg wb_clk_ctrl = {

And here too, static const. We can even move them away from the function.

> +               .reg_off = WB_CLK_CTRL,
> +               .bit_off = 0
> +       };
> +
> +       return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
> +}
> +
>  static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
> -               unsigned long features)
> +               unsigned long features, const struct dpu_mdss_version *mdss_rev)
>  {
>         ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
>         ops->setup_outformat = dpu_hw_wb_setup_format;
> @@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>
>         if (test_bit(DPU_WB_INPUT_CTRL, &features))
>                 ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
> +
> +       if (mdss_rev->core_major_ver >= 9)
> +               ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
>  }
>
>  struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
> -               void __iomem *addr)
> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>  {
>         struct dpu_hw_wb *c;
>
> @@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>         /* Assign ops */
>         c->idx = cfg->id;
>         c->caps = cfg;
> -       _setup_wb_ops(&c->ops, c->caps->features);
> +       _setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
>
>         return c;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> index 2d7db2efa3d0..88792f450a92 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> @@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
>   *  @setup_outformat: setup output format of writeback block from writeback job
>   *  @setup_qos_lut:   setup qos LUT for writeback block based on input
>   *  @setup_cdp:       setup chroma down prefetch block for writeback block
> + *  @setup_clk_force_ctrl: setup clock force control
>   *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
>   */
>  struct dpu_hw_wb_ops {
> @@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
>                           const struct dpu_format *fmt,
>                           bool enable);
>
> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
> +                                    bool enable);
> +
>         void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
>                                   const enum dpu_pingpong pp);
>  };
> @@ -74,10 +78,11 @@ struct dpu_hw_wb {
>   * dpu_hw_wb_init() - Initializes the writeback hw driver object.
>   * @cfg:  wb_path catalog entry for which driver object is required
>   * @addr: mapped register io address of MDP
> + * @mdss_rev: dpu core's major and minor versions
>   * Return: Error code or allocated dpu_hw_wb context
>   */
>  struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
> -               void __iomem *addr);
> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
>
>  /**
>   * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f9215643c71a..f363bcfdfd70 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>                 struct dpu_hw_wb *hw;
>                 const struct dpu_wb_cfg *wb = &cat->wb[i];
>
> -               hw = dpu_hw_wb_init(wb, mmio);
> +               hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
>                 if (IS_ERR(hw)) {
>                         rc = PTR_ERR(hw);
>                         DPU_ERROR("failed wb object creation: err %d\n", rc);
> @@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>                 struct dpu_hw_sspp *hw;
>                 const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>
> -               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
> +               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
>                 if (IS_ERR(hw)) {
>                         rc = PTR_ERR(hw);
>                         DPU_ERROR("failed sspp object creation: err %d\n", rc);
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
@ 2023-10-11 12:45     ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:45 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: freedreno, Abhinav Kumar, dri-devel, linux-kernel, linux-arm-msm,
	Marijn Suijten, Sean Paul

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Starting from SM8550, the SSPP & WB clock controls are moved
> the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
> downstream.
>
> Implement setup_clk_force_ctrl() only starting from major version 9
> which corresponds to SM8550 MDSS.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

With two minor issues below fixed:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
>  drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
>  5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
> @@ -69,6 +69,7 @@
>  #define SSPP_EXCL_REC_XY_REC1              0x188
>  #define SSPP_EXCL_REC_SIZE                 0x1B4
>  #define SSPP_EXCL_REC_XY                   0x1B8
> +#define SSPP_CLK_CTRL                      0x330
>
>  /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
>  #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
> @@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
>         dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
>  }
>
> +static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
> +{
> +       struct dpu_clk_ctrl_reg sspp_clk_ctrl = {

Nit: static const?

> +               .reg_off = SSPP_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)
> +               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;
> @@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>
>         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
> @@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>  #endif
>
>  struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
> -               void __iomem *addr, const struct msm_mdss_data *mdss_data)
> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
> +               const struct dpu_mdss_version *mdss_rev)
>  {
>         struct dpu_hw_sspp *hw_pipe;
>
> @@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>         hw_pipe->ubwc = mdss_data;
>         hw_pipe->idx = cfg->id;
>         hw_pipe->cap = cfg;
> -       _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
> +       _setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
>
>         return hw_pipe;
>  }
> 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 cbf4f95ff0fd..f93969fddb22 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
> @@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
>         void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
>                                bool danger_safe_en);
>
> +       /**
> +        * setup_clk_force_ctrl - setup clock force control
> +        * @ctx: Pointer to pipe context
> +        * @enable: enable clock force if true
> +        */
> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
> +                                    bool enable);
> +
>         /**
>          * setup_histogram - setup histograms
>          * @ctx: Pointer to pipe context
> @@ -334,9 +342,11 @@ struct dpu_kms;
>   * @cfg:  Pipe catalog entry for which driver object is required
>   * @addr: Mapped register io address of MDP
>   * @mdss_data: UBWC / MDSS configuration data
> + * @mdss_rev: dpu core's major and minor versions
>   */
>  struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
> -               void __iomem *addr, const struct msm_mdss_data *mdss_data);
> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
> +               const struct dpu_mdss_version *mdss_rev);
>
>  /**
>   * dpu_hw_sspp_destroy(): Destroys SSPP driver context
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> index ebc416400382..374c2c64c9e4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
> @@ -43,6 +43,7 @@
>  #define WB_MUX                                0x150
>  #define WB_CROP_CTRL                          0x154
>  #define WB_CROP_OFFSET                        0x158
> +#define WB_CLK_CTRL                           0x178
>  #define WB_CSC_BASE                           0x260
>  #define WB_DST_ADDR_SW_STATUS                 0x2B0
>  #define WB_CDP_CNTL                           0x2B4
> @@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
>         DPU_REG_WRITE(c, WB_MUX, mux_cfg);
>  }
>
> +static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
> +{
> +       struct dpu_clk_ctrl_reg wb_clk_ctrl = {

And here too, static const. We can even move them away from the function.

> +               .reg_off = WB_CLK_CTRL,
> +               .bit_off = 0
> +       };
> +
> +       return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
> +}
> +
>  static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
> -               unsigned long features)
> +               unsigned long features, const struct dpu_mdss_version *mdss_rev)
>  {
>         ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
>         ops->setup_outformat = dpu_hw_wb_setup_format;
> @@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>
>         if (test_bit(DPU_WB_INPUT_CTRL, &features))
>                 ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
> +
> +       if (mdss_rev->core_major_ver >= 9)
> +               ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
>  }
>
>  struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
> -               void __iomem *addr)
> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>  {
>         struct dpu_hw_wb *c;
>
> @@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>         /* Assign ops */
>         c->idx = cfg->id;
>         c->caps = cfg;
> -       _setup_wb_ops(&c->ops, c->caps->features);
> +       _setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
>
>         return c;
>  }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> index 2d7db2efa3d0..88792f450a92 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
> @@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
>   *  @setup_outformat: setup output format of writeback block from writeback job
>   *  @setup_qos_lut:   setup qos LUT for writeback block based on input
>   *  @setup_cdp:       setup chroma down prefetch block for writeback block
> + *  @setup_clk_force_ctrl: setup clock force control
>   *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
>   */
>  struct dpu_hw_wb_ops {
> @@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
>                           const struct dpu_format *fmt,
>                           bool enable);
>
> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
> +                                    bool enable);
> +
>         void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
>                                   const enum dpu_pingpong pp);
>  };
> @@ -74,10 +78,11 @@ struct dpu_hw_wb {
>   * dpu_hw_wb_init() - Initializes the writeback hw driver object.
>   * @cfg:  wb_path catalog entry for which driver object is required
>   * @addr: mapped register io address of MDP
> + * @mdss_rev: dpu core's major and minor versions
>   * Return: Error code or allocated dpu_hw_wb context
>   */
>  struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
> -               void __iomem *addr);
> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
>
>  /**
>   * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f9215643c71a..f363bcfdfd70 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>                 struct dpu_hw_wb *hw;
>                 const struct dpu_wb_cfg *wb = &cat->wb[i];
>
> -               hw = dpu_hw_wb_init(wb, mmio);
> +               hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
>                 if (IS_ERR(hw)) {
>                         rc = PTR_ERR(hw);
>                         DPU_ERROR("failed wb object creation: err %d\n", rc);
> @@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>                 struct dpu_hw_sspp *hw;
>                 const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>
> -               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
> +               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
>                 if (IS_ERR(hw)) {
>                         rc = PTR_ERR(hw);
>                         DPU_ERROR("failed sspp object creation: err %d\n", rc);
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550
  2023-10-11 11:59   ` Neil Armstrong
@ 2023-10-11 12:46     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:46 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: freedreno, Abhinav Kumar, dri-devel, linux-kernel, linux-arm-msm,
	Marijn Suijten, Sean Paul

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Enable WB2 hardware block, enabling writeback support on this platform.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> index 4590a01c1252..d83a68a2cc0a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> @@ -321,6 +321,20 @@ static const struct dpu_dsc_cfg sm8550_dsc[] = {
>         },
>  };
>
> +static const struct dpu_wb_cfg sm8550_wb[] = {
> +       {
> +               .name = "wb_2", .id = WB_2,
> +               .base = 0x65000, .len = 0x2c8,
> +               .features = WB_SM8250_MASK,
> +               .format_list = wb2_formats,
> +               .num_formats = ARRAY_SIZE(wb2_formats),
> +               .xin_id = 6,
> +               .vbif_idx = VBIF_RT,
> +               .maxlinewidth = 4096,
> +               .intr_wb_done = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 4),
> +       },
> +};
> +
>  static const struct dpu_intf_cfg sm8550_intf[] = {
>         {
>                 .name = "intf_0", .id = INTF_0,
> @@ -418,6 +432,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
>         .dsc = sm8550_dsc,
>         .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
>         .merge_3d = sm8550_merge_3d,
> +       .wb_count = ARRAY_SIZE(sm8550_wb),
> +       .wb = sm8550_wb,
>         .intf_count = ARRAY_SIZE(sm8550_intf),
>         .intf = sm8550_intf,
>         .vbif_count = ARRAY_SIZE(sm8550_vbif),
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550
@ 2023-10-11 12:46     ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:46 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
	linux-kernel

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Enable WB2 hardware block, enabling writeback support on this platform.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

> ---
>  drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> index 4590a01c1252..d83a68a2cc0a 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> @@ -321,6 +321,20 @@ static const struct dpu_dsc_cfg sm8550_dsc[] = {
>         },
>  };
>
> +static const struct dpu_wb_cfg sm8550_wb[] = {
> +       {
> +               .name = "wb_2", .id = WB_2,
> +               .base = 0x65000, .len = 0x2c8,
> +               .features = WB_SM8250_MASK,
> +               .format_list = wb2_formats,
> +               .num_formats = ARRAY_SIZE(wb2_formats),
> +               .xin_id = 6,
> +               .vbif_idx = VBIF_RT,
> +               .maxlinewidth = 4096,
> +               .intr_wb_done = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 4),
> +       },
> +};
> +
>  static const struct dpu_intf_cfg sm8550_intf[] = {
>         {
>                 .name = "intf_0", .id = INTF_0,
> @@ -418,6 +432,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
>         .dsc = sm8550_dsc,
>         .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
>         .merge_3d = sm8550_merge_3d,
> +       .wb_count = ARRAY_SIZE(sm8550_wb),
> +       .wb = sm8550_wb,
>         .intf_count = ARRAY_SIZE(sm8550_intf),
>         .intf = sm8550_intf,
>         .vbif_count = ARRAY_SIZE(sm8550_vbif),
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
  2023-10-11 11:59   ` Neil Armstrong
@ 2023-10-11 12:55     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:55 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
	linux-kernel

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Now SSPP and WB can have setup_force_clk_ctrl() ops, it's simpler to call
> them from the plane and wb code and call into the mdp ops if not present.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
>  4 files changed, 77 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index 78037a697633..8802e007f8e2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -34,6 +34,23 @@ static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys *phys_enc)
>         return true;
>  }
>
> +static bool _dpu_encoder_phys_wb_clk_force_ctrl(struct dpu_hw_wb *wb,
> +                                               struct dpu_hw_mdp *mdp,
> +                                               bool enable, bool *forced_on)
> +{
> +       if (wb->ops.setup_clk_force_ctrl) {
> +               *forced_on = wb->ops.setup_clk_force_ctrl(wb, enable);
> +               return true;
> +       }
> +
> +       if (mdp->ops.setup_clk_force_ctrl) {
> +               *forced_on = mdp->ops.setup_clk_force_ctrl(mdp, wb->caps->clk_ctrl, enable);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
>  /**
>   * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback interface
>   * @phys_enc:  Pointer to physical encoder
> @@ -43,6 +60,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>  {
>         struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
>         struct dpu_vbif_set_ot_params ot_params;
> +       bool forced_on = false;
>
>         memset(&ot_params, 0, sizeof(ot_params));
>         ot_params.xin_id = hw_wb->caps->xin_id;
> @@ -52,10 +70,17 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>         ot_params.is_wfd = true;
>         ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
>         ot_params.vbif_idx = hw_wb->caps->vbif_idx;
> -       ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>         ot_params.rd = false;
>
> +       if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
> +
> +       if (forced_on)
> +               _dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                   false, &forced_on);
>  }
>
>  /**
> @@ -67,6 +92,7 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>  {
>         struct dpu_hw_wb *hw_wb;
>         struct dpu_vbif_set_qos_params qos_params;
> +       bool forced_on = false;
>
>         if (!phys_enc || !phys_enc->parent || !phys_enc->parent->crtc) {
>                 DPU_ERROR("invalid arguments\n");
> @@ -83,7 +109,6 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>         memset(&qos_params, 0, sizeof(qos_params));
>         qos_params.vbif_idx = hw_wb->caps->vbif_idx;
>         qos_params.xin_id = hw_wb->caps->xin_id;
> -       qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>         qos_params.num = hw_wb->idx - WB_0;
>         qos_params.is_rt = false;
>
> @@ -92,7 +117,15 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>                         qos_params.vbif_idx,
>                         qos_params.xin_id, qos_params.is_rt);
>
> +       if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_qos_remap(phys_enc->dpu_kms, &qos_params);
> +
> +       if (forced_on)
> +               _dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                   false, &forced_on);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c2aaaded07ed..c63cae8fb35c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -333,6 +333,23 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
>                                        enable);
>  }
>
> +static bool _dpu_plane_sspp_clk_force_ctrl(struct dpu_hw_sspp *sspp,
> +                                          struct dpu_hw_mdp *mdp,
> +                                          bool enable, bool *forced_on)
> +{
> +       if (sspp->ops.setup_clk_force_ctrl) {
> +               *forced_on = sspp->ops.setup_clk_force_ctrl(sspp, enable);
> +               return true;
> +       }
> +
> +       if (mdp->ops.setup_clk_force_ctrl) {
> +               *forced_on = mdp->ops.setup_clk_force_ctrl(mdp, sspp->cap->clk_ctrl, enable);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
>  /**
>   * _dpu_plane_set_ot_limit - set OT limit for the given plane
>   * @plane:             Pointer to drm plane
> @@ -348,6 +365,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
>         struct dpu_vbif_set_ot_params ot_params;
>         struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> +       bool forced_on = false;
>
>         memset(&ot_params, 0, sizeof(ot_params));
>         ot_params.xin_id = pipe->sspp->cap->xin_id;
> @@ -357,10 +375,17 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>         ot_params.is_wfd = !pdpu->is_rt_pipe;
>         ot_params.frame_rate = frame_rate;
>         ot_params.vbif_idx = VBIF_RT;
> -       ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>         ot_params.rd = true;
>
> +       if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                           true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
> +
> +       if (forced_on)
> +               _dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                              false, &forced_on);
>  }
>
>  /**
> @@ -374,21 +399,28 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
>         struct dpu_vbif_set_qos_params qos_params;
>         struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> +       bool forced_on = false;
>
>         memset(&qos_params, 0, sizeof(qos_params));
>         qos_params.vbif_idx = VBIF_RT;
> -       qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>         qos_params.xin_id = pipe->sspp->cap->xin_id;
>         qos_params.num = pipe->sspp->idx - SSPP_VIG0;
>         qos_params.is_rt = pdpu->is_rt_pipe;
>
> -       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
> +       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
>                         qos_params.num,
>                         qos_params.vbif_idx,
> -                       qos_params.xin_id, qos_params.is_rt,
> -                       qos_params.clk_ctrl);
> +                       qos_params.xin_id, qos_params.is_rt);
> +
> +       if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                           true, &forced_on))
> +               return;
>
>         dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
> +
> +       if (forced_on)
> +               _dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                              false, &forced_on);
>  }
>
>  static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> index 1305e250b71e..47c02b98eac3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> @@ -169,23 +169,16 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>                 struct dpu_vbif_set_ot_params *params)
>  {
>         struct dpu_hw_vbif *vbif;
> -       struct dpu_hw_mdp *mdp;
> -       bool forced_on = false;
>         u32 ot_lim;
>         int ret;
>
> -       mdp = dpu_kms->hw_mdp;
> -
>         vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
> -       if (!vbif || !mdp) {
> -               DRM_DEBUG_ATOMIC("invalid arguments vbif %d mdp %d\n",
> -                               vbif != NULL, mdp != NULL);
> +       if (!vbif) {
> +               DRM_DEBUG_ATOMIC("invalid arguments vbif %d\n", vbif != NULL);
>                 return;
>         }
>
> -       if (!mdp->ops.setup_clk_force_ctrl ||
> -                       !vbif->ops.set_limit_conf ||
> -                       !vbif->ops.set_halt_ctrl)
> +       if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>                 return;
>
>         /* set write_gather_en for all write clients */
> @@ -200,8 +193,6 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>         trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
>                 params->vbif_idx);
>
> -       forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> -
>         vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>
>         vbif->ops.set_halt_ctrl(vbif, params->xin_id, true);
> @@ -211,25 +202,19 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>                 trace_dpu_vbif_wait_xin_halt_fail(vbif->idx, params->xin_id);
>
>         vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
> -
> -       if (forced_on)
> -               mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>  }
>
>  void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 struct dpu_vbif_set_qos_params *params)
>  {
>         struct dpu_hw_vbif *vbif;
> -       struct dpu_hw_mdp *mdp;
> -       bool forced_on = false;
>         const struct dpu_vbif_qos_tbl *qos_tbl;
>         int i;
>
> -       if (!params || !dpu_kms->hw_mdp) {
> +       if (!params) {
>                 DPU_ERROR("invalid arguments\n");
>                 return;
>         }
> -       mdp = dpu_kms->hw_mdp;
>
>         vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
>
> @@ -238,7 +223,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 return;
>         }
>
> -       if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
> +       if (!vbif->ops.set_qos_remap) {
>                 DRM_DEBUG_ATOMIC("qos remap not supported\n");
>                 return;
>         }
> @@ -251,8 +236,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 return;
>         }
>
> -       forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> -
>         for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>                 DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
>                                 dpu_vbif_name(params->vbif_idx), params->xin_id, i,
> @@ -260,9 +243,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 vbif->ops.set_qos_remap(vbif, params->xin_id, i,
>                                 qos_tbl->priority_lvl[i]);
>         }
> -
> -       if (forced_on)
> -               mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>  }
>
>  void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> index ab490177d886..e1b1f7f4e4be 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> @@ -16,13 +16,11 @@ struct dpu_vbif_set_ot_params {
>         bool rd;
>         bool is_wfd;
>         u32 vbif_idx;
> -       u32 clk_ctrl;
>  };
>
>  struct dpu_vbif_set_memtype_params {
>         u32 xin_id;
>         u32 vbif_idx;
> -       u32 clk_ctrl;
>         bool is_cacheable;
>  };
>
> @@ -30,14 +28,12 @@ struct dpu_vbif_set_memtype_params {
>   * struct dpu_vbif_set_qos_params - QoS remapper parameter
>   * @vbif_idx: vbif identifier
>   * @xin_id: client interface identifier
> - * @clk_ctrl: clock control identifier of the xin
>   * @num: pipe identifier (debug only)
>   * @is_rt: true if pipe is used in real-time use case
>   */
>  struct dpu_vbif_set_qos_params {
>         u32 vbif_idx;
>         u32 xin_id;
> -       u32 clk_ctrl;
>         u32 num;
>         bool is_rt;
>  };
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb
@ 2023-10-11 12:55     ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:55 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: freedreno, Abhinav Kumar, dri-devel, linux-kernel, linux-arm-msm,
	Marijn Suijten, Sean Paul

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> Now SSPP and WB can have setup_force_clk_ctrl() ops, it's simpler to call
> them from the plane and wb code and call into the mdp ops if not present.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    | 37 +++++++++++++++++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          | 42 +++++++++++++++++++---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 30 +++-------------
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           |  4 ---
>  4 files changed, 77 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index 78037a697633..8802e007f8e2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -34,6 +34,23 @@ static bool dpu_encoder_phys_wb_is_master(struct dpu_encoder_phys *phys_enc)
>         return true;
>  }
>
> +static bool _dpu_encoder_phys_wb_clk_force_ctrl(struct dpu_hw_wb *wb,
> +                                               struct dpu_hw_mdp *mdp,
> +                                               bool enable, bool *forced_on)
> +{
> +       if (wb->ops.setup_clk_force_ctrl) {
> +               *forced_on = wb->ops.setup_clk_force_ctrl(wb, enable);
> +               return true;
> +       }
> +
> +       if (mdp->ops.setup_clk_force_ctrl) {
> +               *forced_on = mdp->ops.setup_clk_force_ctrl(mdp, wb->caps->clk_ctrl, enable);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
>  /**
>   * dpu_encoder_phys_wb_set_ot_limit - set OT limit for writeback interface
>   * @phys_enc:  Pointer to physical encoder
> @@ -43,6 +60,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>  {
>         struct dpu_hw_wb *hw_wb = phys_enc->hw_wb;
>         struct dpu_vbif_set_ot_params ot_params;
> +       bool forced_on = false;
>
>         memset(&ot_params, 0, sizeof(ot_params));
>         ot_params.xin_id = hw_wb->caps->xin_id;
> @@ -52,10 +70,17 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>         ot_params.is_wfd = true;
>         ot_params.frame_rate = drm_mode_vrefresh(&phys_enc->cached_mode);
>         ot_params.vbif_idx = hw_wb->caps->vbif_idx;
> -       ot_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>         ot_params.rd = false;
>
> +       if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
> +
> +       if (forced_on)
> +               _dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                   false, &forced_on);
>  }
>
>  /**
> @@ -67,6 +92,7 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>  {
>         struct dpu_hw_wb *hw_wb;
>         struct dpu_vbif_set_qos_params qos_params;
> +       bool forced_on = false;
>
>         if (!phys_enc || !phys_enc->parent || !phys_enc->parent->crtc) {
>                 DPU_ERROR("invalid arguments\n");
> @@ -83,7 +109,6 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>         memset(&qos_params, 0, sizeof(qos_params));
>         qos_params.vbif_idx = hw_wb->caps->vbif_idx;
>         qos_params.xin_id = hw_wb->caps->xin_id;
> -       qos_params.clk_ctrl = hw_wb->caps->clk_ctrl;
>         qos_params.num = hw_wb->idx - WB_0;
>         qos_params.is_rt = false;
>
> @@ -92,7 +117,15 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>                         qos_params.vbif_idx,
>                         qos_params.xin_id, qos_params.is_rt);
>
> +       if (!_dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_qos_remap(phys_enc->dpu_kms, &qos_params);
> +
> +       if (forced_on)
> +               _dpu_encoder_phys_wb_clk_force_ctrl(hw_wb, phys_enc->dpu_kms->hw_mdp,
> +                                                   false, &forced_on);
>  }
>
>  /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c2aaaded07ed..c63cae8fb35c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -333,6 +333,23 @@ static void _dpu_plane_set_qos_ctrl(struct drm_plane *plane,
>                                        enable);
>  }
>
> +static bool _dpu_plane_sspp_clk_force_ctrl(struct dpu_hw_sspp *sspp,
> +                                          struct dpu_hw_mdp *mdp,
> +                                          bool enable, bool *forced_on)
> +{
> +       if (sspp->ops.setup_clk_force_ctrl) {
> +               *forced_on = sspp->ops.setup_clk_force_ctrl(sspp, enable);
> +               return true;
> +       }
> +
> +       if (mdp->ops.setup_clk_force_ctrl) {
> +               *forced_on = mdp->ops.setup_clk_force_ctrl(mdp, sspp->cap->clk_ctrl, enable);
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
>  /**
>   * _dpu_plane_set_ot_limit - set OT limit for the given plane
>   * @plane:             Pointer to drm plane
> @@ -348,6 +365,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
>         struct dpu_vbif_set_ot_params ot_params;
>         struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> +       bool forced_on = false;
>
>         memset(&ot_params, 0, sizeof(ot_params));
>         ot_params.xin_id = pipe->sspp->cap->xin_id;
> @@ -357,10 +375,17 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>         ot_params.is_wfd = !pdpu->is_rt_pipe;
>         ot_params.frame_rate = frame_rate;
>         ot_params.vbif_idx = VBIF_RT;
> -       ot_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>         ot_params.rd = true;
>
> +       if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                           true, &forced_on))
> +               return;
> +
>         dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
> +
> +       if (forced_on)
> +               _dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                              false, &forced_on);
>  }
>
>  /**
> @@ -374,21 +399,28 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>         struct dpu_plane *pdpu = to_dpu_plane(plane);
>         struct dpu_vbif_set_qos_params qos_params;
>         struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> +       bool forced_on = false;
>
>         memset(&qos_params, 0, sizeof(qos_params));
>         qos_params.vbif_idx = VBIF_RT;
> -       qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>         qos_params.xin_id = pipe->sspp->cap->xin_id;
>         qos_params.num = pipe->sspp->idx - SSPP_VIG0;
>         qos_params.is_rt = pdpu->is_rt_pipe;
>
> -       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d, clk_ctrl:%d\n",
> +       DPU_DEBUG_PLANE(pdpu, "pipe:%d vbif:%d xin:%d rt:%d\n",
>                         qos_params.num,
>                         qos_params.vbif_idx,
> -                       qos_params.xin_id, qos_params.is_rt,
> -                       qos_params.clk_ctrl);
> +                       qos_params.xin_id, qos_params.is_rt);
> +
> +       if (!_dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                           true, &forced_on))
> +               return;
>
>         dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
> +
> +       if (forced_on)
> +               _dpu_plane_sspp_clk_force_ctrl(pipe->sspp, dpu_kms->hw_mdp,
> +                                              false, &forced_on);
>  }
>
>  static void _dpu_plane_setup_scaler3(struct dpu_hw_sspp *pipe_hw,
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> index 1305e250b71e..47c02b98eac3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> @@ -169,23 +169,16 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>                 struct dpu_vbif_set_ot_params *params)
>  {
>         struct dpu_hw_vbif *vbif;
> -       struct dpu_hw_mdp *mdp;
> -       bool forced_on = false;
>         u32 ot_lim;
>         int ret;
>
> -       mdp = dpu_kms->hw_mdp;
> -
>         vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
> -       if (!vbif || !mdp) {
> -               DRM_DEBUG_ATOMIC("invalid arguments vbif %d mdp %d\n",
> -                               vbif != NULL, mdp != NULL);
> +       if (!vbif) {
> +               DRM_DEBUG_ATOMIC("invalid arguments vbif %d\n", vbif != NULL);
>                 return;
>         }
>
> -       if (!mdp->ops.setup_clk_force_ctrl ||
> -                       !vbif->ops.set_limit_conf ||
> -                       !vbif->ops.set_halt_ctrl)
> +       if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>                 return;
>
>         /* set write_gather_en for all write clients */
> @@ -200,8 +193,6 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>         trace_dpu_perf_set_ot(params->num, params->xin_id, ot_lim,
>                 params->vbif_idx);
>
> -       forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> -
>         vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>
>         vbif->ops.set_halt_ctrl(vbif, params->xin_id, true);
> @@ -211,25 +202,19 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>                 trace_dpu_vbif_wait_xin_halt_fail(vbif->idx, params->xin_id);
>
>         vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
> -
> -       if (forced_on)
> -               mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>  }
>
>  void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 struct dpu_vbif_set_qos_params *params)
>  {
>         struct dpu_hw_vbif *vbif;
> -       struct dpu_hw_mdp *mdp;
> -       bool forced_on = false;
>         const struct dpu_vbif_qos_tbl *qos_tbl;
>         int i;
>
> -       if (!params || !dpu_kms->hw_mdp) {
> +       if (!params) {
>                 DPU_ERROR("invalid arguments\n");
>                 return;
>         }
> -       mdp = dpu_kms->hw_mdp;
>
>         vbif = dpu_get_vbif(dpu_kms, params->vbif_idx);
>
> @@ -238,7 +223,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 return;
>         }
>
> -       if (!vbif->ops.set_qos_remap || !mdp->ops.setup_clk_force_ctrl) {
> +       if (!vbif->ops.set_qos_remap) {
>                 DRM_DEBUG_ATOMIC("qos remap not supported\n");
>                 return;
>         }
> @@ -251,8 +236,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 return;
>         }
>
> -       forced_on = mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> -
>         for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>                 DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
>                                 dpu_vbif_name(params->vbif_idx), params->xin_id, i,
> @@ -260,9 +243,6 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>                 vbif->ops.set_qos_remap(vbif, params->xin_id, i,
>                                 qos_tbl->priority_lvl[i]);
>         }
> -
> -       if (forced_on)
> -               mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>  }
>
>  void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> index ab490177d886..e1b1f7f4e4be 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> @@ -16,13 +16,11 @@ struct dpu_vbif_set_ot_params {
>         bool rd;
>         bool is_wfd;
>         u32 vbif_idx;
> -       u32 clk_ctrl;
>  };
>
>  struct dpu_vbif_set_memtype_params {
>         u32 xin_id;
>         u32 vbif_idx;
> -       u32 clk_ctrl;
>         bool is_cacheable;
>  };
>
> @@ -30,14 +28,12 @@ struct dpu_vbif_set_memtype_params {
>   * struct dpu_vbif_set_qos_params - QoS remapper parameter
>   * @vbif_idx: vbif identifier
>   * @xin_id: client interface identifier
> - * @clk_ctrl: clock control identifier of the xin
>   * @num: pipe identifier (debug only)
>   * @is_rt: true if pipe is used in real-time use case
>   */
>  struct dpu_vbif_set_qos_params {
>         u32 vbif_idx;
>         u32 xin_id;
> -       u32 clk_ctrl;
>         u32 num;
>         bool is_rt;
>  };
>
> --
> 2.34.1
>


-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
  2023-10-11 11:59   ` Neil Armstrong
@ 2023-10-11 12:55     ` Dmitry Baryshkov
  -1 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:55 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
	linux-kernel

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> The SM8550 has the SSPP clk_ctrl in the SSPP registers, remove the
> duplicate clock controls from the MDP top.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 20 --------------------
>  1 file changed, 20 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries
@ 2023-10-11 12:55     ` Dmitry Baryshkov
  0 siblings, 0 replies; 22+ messages in thread
From: Dmitry Baryshkov @ 2023-10-11 12:55 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: freedreno, Abhinav Kumar, dri-devel, linux-kernel, linux-arm-msm,
	Marijn Suijten, Sean Paul

On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>
> The SM8550 has the SSPP clk_ctrl in the SSPP registers, remove the
> duplicate clock controls from the MDP top.
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h   | 20 --------------------
>  1 file changed, 20 deletions(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>



-- 
With best wishes
Dmitry

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
  2023-10-11 12:45     ` Dmitry Baryshkov
@ 2023-10-11 13:15       ` Neil Armstrong
  -1 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 13:15 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: Rob Clark, Abhinav Kumar, Sean Paul, Marijn Suijten,
	David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
	linux-kernel

On 11/10/2023 14:45, Dmitry Baryshkov wrote:
> On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> Starting from SM8550, the SSPP & WB clock controls are moved
>> the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
>> downstream.
>>
>> Implement setup_clk_force_ctrl() only starting from major version 9
>> which corresponds to SM8550 MDSS.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> With two minor issues below fixed:
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
>>   5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> @@ -69,6 +69,7 @@
>>   #define SSPP_EXCL_REC_XY_REC1              0x188
>>   #define SSPP_EXCL_REC_SIZE                 0x1B4
>>   #define SSPP_EXCL_REC_XY                   0x1B8
>> +#define SSPP_CLK_CTRL                      0x330
>>
>>   /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
>>   #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
>> @@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
>>          dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
>>   }
>>
>> +static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
>> +{
>> +       struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
> 
> Nit: static const?

Yep will add

> 
>> +               .reg_off = SSPP_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)
>> +               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;
>> @@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>>
>>          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
>> @@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>>   #endif
>>
>>   struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> -               void __iomem *addr, const struct msm_mdss_data *mdss_data)
>> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> +               const struct dpu_mdss_version *mdss_rev)
>>   {
>>          struct dpu_hw_sspp *hw_pipe;
>>
>> @@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>>          hw_pipe->ubwc = mdss_data;
>>          hw_pipe->idx = cfg->id;
>>          hw_pipe->cap = cfg;
>> -       _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
>> +       _setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
>>
>>          return hw_pipe;
>>   }
>> 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 cbf4f95ff0fd..f93969fddb22 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> @@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
>>          void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
>>                                 bool danger_safe_en);
>>
>> +       /**
>> +        * setup_clk_force_ctrl - setup clock force control
>> +        * @ctx: Pointer to pipe context
>> +        * @enable: enable clock force if true
>> +        */
>> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
>> +                                    bool enable);
>> +
>>          /**
>>           * setup_histogram - setup histograms
>>           * @ctx: Pointer to pipe context
>> @@ -334,9 +342,11 @@ struct dpu_kms;
>>    * @cfg:  Pipe catalog entry for which driver object is required
>>    * @addr: Mapped register io address of MDP
>>    * @mdss_data: UBWC / MDSS configuration data
>> + * @mdss_rev: dpu core's major and minor versions
>>    */
>>   struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> -               void __iomem *addr, const struct msm_mdss_data *mdss_data);
>> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> +               const struct dpu_mdss_version *mdss_rev);
>>
>>   /**
>>    * dpu_hw_sspp_destroy(): Destroys SSPP driver context
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> index ebc416400382..374c2c64c9e4 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> @@ -43,6 +43,7 @@
>>   #define WB_MUX                                0x150
>>   #define WB_CROP_CTRL                          0x154
>>   #define WB_CROP_OFFSET                        0x158
>> +#define WB_CLK_CTRL                           0x178
>>   #define WB_CSC_BASE                           0x260
>>   #define WB_DST_ADDR_SW_STATUS                 0x2B0
>>   #define WB_CDP_CNTL                           0x2B4
>> @@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
>>          DPU_REG_WRITE(c, WB_MUX, mux_cfg);
>>   }
>>
>> +static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
>> +{
>> +       struct dpu_clk_ctrl_reg wb_clk_ctrl = {
> 
> And here too, static const. We can even move them away from the function.

Since it's only used on the function, let's let it here in static const

Neil

> 
>> +               .reg_off = WB_CLK_CTRL,
>> +               .bit_off = 0
>> +       };
>> +
>> +       return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
>> +}
>> +
>>   static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>> -               unsigned long features)
>> +               unsigned long features, const struct dpu_mdss_version *mdss_rev)
>>   {
>>          ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
>>          ops->setup_outformat = dpu_hw_wb_setup_format;
>> @@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>>
>>          if (test_bit(DPU_WB_INPUT_CTRL, &features))
>>                  ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
>> +
>> +       if (mdss_rev->core_major_ver >= 9)
>> +               ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
>>   }
>>
>>   struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> -               void __iomem *addr)
>> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>>   {
>>          struct dpu_hw_wb *c;
>>
>> @@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>>          /* Assign ops */
>>          c->idx = cfg->id;
>>          c->caps = cfg;
>> -       _setup_wb_ops(&c->ops, c->caps->features);
>> +       _setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
>>
>>          return c;
>>   }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> index 2d7db2efa3d0..88792f450a92 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> @@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
>>    *  @setup_outformat: setup output format of writeback block from writeback job
>>    *  @setup_qos_lut:   setup qos LUT for writeback block based on input
>>    *  @setup_cdp:       setup chroma down prefetch block for writeback block
>> + *  @setup_clk_force_ctrl: setup clock force control
>>    *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
>>    */
>>   struct dpu_hw_wb_ops {
>> @@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
>>                            const struct dpu_format *fmt,
>>                            bool enable);
>>
>> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
>> +                                    bool enable);
>> +
>>          void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
>>                                    const enum dpu_pingpong pp);
>>   };
>> @@ -74,10 +78,11 @@ struct dpu_hw_wb {
>>    * dpu_hw_wb_init() - Initializes the writeback hw driver object.
>>    * @cfg:  wb_path catalog entry for which driver object is required
>>    * @addr: mapped register io address of MDP
>> + * @mdss_rev: dpu core's major and minor versions
>>    * Return: Error code or allocated dpu_hw_wb context
>>    */
>>   struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> -               void __iomem *addr);
>> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
>>
>>   /**
>>    * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> index f9215643c71a..f363bcfdfd70 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> @@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>>                  struct dpu_hw_wb *hw;
>>                  const struct dpu_wb_cfg *wb = &cat->wb[i];
>>
>> -               hw = dpu_hw_wb_init(wb, mmio);
>> +               hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
>>                  if (IS_ERR(hw)) {
>>                          rc = PTR_ERR(hw);
>>                          DPU_ERROR("failed wb object creation: err %d\n", rc);
>> @@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>>                  struct dpu_hw_sspp *hw;
>>                  const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>>
>> -               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
>> +               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
>>                  if (IS_ERR(hw)) {
>>                          rc = PTR_ERR(hw);
>>                          DPU_ERROR("failed sspp object creation: err %d\n", rc);
>>
>> --
>> 2.34.1
>>
> 
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb
@ 2023-10-11 13:15       ` Neil Armstrong
  0 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2023-10-11 13:15 UTC (permalink / raw)
  To: Dmitry Baryshkov
  Cc: freedreno, Abhinav Kumar, dri-devel, linux-kernel, linux-arm-msm,
	Marijn Suijten, Sean Paul

On 11/10/2023 14:45, Dmitry Baryshkov wrote:
> On Wed, 11 Oct 2023 at 14:59, Neil Armstrong <neil.armstrong@linaro.org> wrote:
>>
>> Starting from SM8550, the SSPP & WB clock controls are moved
>> the SSPP and WB register range, as it's called "VBIF_CLK_SPLIT"
>> downstream.
>>
>> Implement setup_clk_force_ctrl() only starting from major version 9
>> which corresponds to SM8550 MDSS.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> 
> With two minor issues below fixed:
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 21 ++++++++++++++++++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h | 12 +++++++++++-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c   | 20 +++++++++++++++++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h   |  7 ++++++-
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c      |  4 ++--
>>   5 files changed, 54 insertions(+), 10 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 f2192de93713..5fd213ed6491 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
>> @@ -69,6 +69,7 @@
>>   #define SSPP_EXCL_REC_XY_REC1              0x188
>>   #define SSPP_EXCL_REC_SIZE                 0x1B4
>>   #define SSPP_EXCL_REC_XY                   0x1B8
>> +#define SSPP_CLK_CTRL                      0x330
>>
>>   /* SSPP_SRC_OP_MODE & OP_MODE_REC1 */
>>   #define MDSS_MDP_OP_DEINTERLACE            BIT(22)
>> @@ -581,8 +582,18 @@ static void dpu_hw_sspp_setup_cdp(struct dpu_sw_pipe *pipe,
>>          dpu_setup_cdp(&ctx->hw, cdp_cntl_offset, fmt, enable);
>>   }
>>
>> +static bool dpu_hw_sspp_setup_clk_force_ctrl(struct dpu_hw_sspp *ctx, bool enable)
>> +{
>> +       struct dpu_clk_ctrl_reg sspp_clk_ctrl = {
> 
> Nit: static const?

Yep will add

> 
>> +               .reg_off = SSPP_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)
>> +               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;
>> @@ -612,6 +623,9 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
>>
>>          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
>> @@ -672,7 +686,8 @@ int _dpu_hw_sspp_init_debugfs(struct dpu_hw_sspp *hw_pipe, struct dpu_kms *kms,
>>   #endif
>>
>>   struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> -               void __iomem *addr, const struct msm_mdss_data *mdss_data)
>> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> +               const struct dpu_mdss_version *mdss_rev)
>>   {
>>          struct dpu_hw_sspp *hw_pipe;
>>
>> @@ -690,7 +705,7 @@ struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>>          hw_pipe->ubwc = mdss_data;
>>          hw_pipe->idx = cfg->id;
>>          hw_pipe->cap = cfg;
>> -       _setup_layer_ops(hw_pipe, hw_pipe->cap->features);
>> +       _setup_layer_ops(hw_pipe, hw_pipe->cap->features, mdss_rev);
>>
>>          return hw_pipe;
>>   }
>> 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 cbf4f95ff0fd..f93969fddb22 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
>> @@ -271,6 +271,14 @@ struct dpu_hw_sspp_ops {
>>          void (*setup_qos_ctrl)(struct dpu_hw_sspp *ctx,
>>                                 bool danger_safe_en);
>>
>> +       /**
>> +        * setup_clk_force_ctrl - setup clock force control
>> +        * @ctx: Pointer to pipe context
>> +        * @enable: enable clock force if true
>> +        */
>> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_sspp *ctx,
>> +                                    bool enable);
>> +
>>          /**
>>           * setup_histogram - setup histograms
>>           * @ctx: Pointer to pipe context
>> @@ -334,9 +342,11 @@ struct dpu_kms;
>>    * @cfg:  Pipe catalog entry for which driver object is required
>>    * @addr: Mapped register io address of MDP
>>    * @mdss_data: UBWC / MDSS configuration data
>> + * @mdss_rev: dpu core's major and minor versions
>>    */
>>   struct dpu_hw_sspp *dpu_hw_sspp_init(const struct dpu_sspp_cfg *cfg,
>> -               void __iomem *addr, const struct msm_mdss_data *mdss_data);
>> +               void __iomem *addr, const struct msm_mdss_data *mdss_data,
>> +               const struct dpu_mdss_version *mdss_rev);
>>
>>   /**
>>    * dpu_hw_sspp_destroy(): Destroys SSPP driver context
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> index ebc416400382..374c2c64c9e4 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
>> @@ -43,6 +43,7 @@
>>   #define WB_MUX                                0x150
>>   #define WB_CROP_CTRL                          0x154
>>   #define WB_CROP_OFFSET                        0x158
>> +#define WB_CLK_CTRL                           0x178
>>   #define WB_CSC_BASE                           0x260
>>   #define WB_DST_ADDR_SW_STATUS                 0x2B0
>>   #define WB_CDP_CNTL                           0x2B4
>> @@ -175,8 +176,18 @@ static void dpu_hw_wb_bind_pingpong_blk(
>>          DPU_REG_WRITE(c, WB_MUX, mux_cfg);
>>   }
>>
>> +static bool dpu_hw_wb_setup_clk_force_ctrl(struct dpu_hw_wb *ctx, bool enable)
>> +{
>> +       struct dpu_clk_ctrl_reg wb_clk_ctrl = {
> 
> And here too, static const. We can even move them away from the function.

Since it's only used on the function, let's let it here in static const

Neil

> 
>> +               .reg_off = WB_CLK_CTRL,
>> +               .bit_off = 0
>> +       };
>> +
>> +       return dpu_hw_clk_force_ctrl(&ctx->hw, &wb_clk_ctrl, enable);
>> +}
>> +
>>   static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>> -               unsigned long features)
>> +               unsigned long features, const struct dpu_mdss_version *mdss_rev)
>>   {
>>          ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
>>          ops->setup_outformat = dpu_hw_wb_setup_format;
>> @@ -192,10 +203,13 @@ static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
>>
>>          if (test_bit(DPU_WB_INPUT_CTRL, &features))
>>                  ops->bind_pingpong_blk = dpu_hw_wb_bind_pingpong_blk;
>> +
>> +       if (mdss_rev->core_major_ver >= 9)
>> +               ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
>>   }
>>
>>   struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> -               void __iomem *addr)
>> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev)
>>   {
>>          struct dpu_hw_wb *c;
>>
>> @@ -212,7 +226,7 @@ struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>>          /* Assign ops */
>>          c->idx = cfg->id;
>>          c->caps = cfg;
>> -       _setup_wb_ops(&c->ops, c->caps->features);
>> +       _setup_wb_ops(&c->ops, c->caps->features, mdss_rev);
>>
>>          return c;
>>   }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> index 2d7db2efa3d0..88792f450a92 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h
>> @@ -29,6 +29,7 @@ struct dpu_hw_wb_cfg {
>>    *  @setup_outformat: setup output format of writeback block from writeback job
>>    *  @setup_qos_lut:   setup qos LUT for writeback block based on input
>>    *  @setup_cdp:       setup chroma down prefetch block for writeback block
>> + *  @setup_clk_force_ctrl: setup clock force control
>>    *  @bind_pingpong_blk: enable/disable the connection with ping-pong block
>>    */
>>   struct dpu_hw_wb_ops {
>> @@ -48,6 +49,9 @@ struct dpu_hw_wb_ops {
>>                            const struct dpu_format *fmt,
>>                            bool enable);
>>
>> +       bool (*setup_clk_force_ctrl)(struct dpu_hw_wb *ctx,
>> +                                    bool enable);
>> +
>>          void (*bind_pingpong_blk)(struct dpu_hw_wb *ctx,
>>                                    const enum dpu_pingpong pp);
>>   };
>> @@ -74,10 +78,11 @@ struct dpu_hw_wb {
>>    * dpu_hw_wb_init() - Initializes the writeback hw driver object.
>>    * @cfg:  wb_path catalog entry for which driver object is required
>>    * @addr: mapped register io address of MDP
>> + * @mdss_rev: dpu core's major and minor versions
>>    * Return: Error code or allocated dpu_hw_wb context
>>    */
>>   struct dpu_hw_wb *dpu_hw_wb_init(const struct dpu_wb_cfg *cfg,
>> -               void __iomem *addr);
>> +               void __iomem *addr, const struct dpu_mdss_version *mdss_rev);
>>
>>   /**
>>    * dpu_hw_wb_destroy(): Destroy writeback hw driver object.
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> index f9215643c71a..f363bcfdfd70 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> @@ -175,7 +175,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>>                  struct dpu_hw_wb *hw;
>>                  const struct dpu_wb_cfg *wb = &cat->wb[i];
>>
>> -               hw = dpu_hw_wb_init(wb, mmio);
>> +               hw = dpu_hw_wb_init(wb, mmio, cat->mdss_ver);
>>                  if (IS_ERR(hw)) {
>>                          rc = PTR_ERR(hw);
>>                          DPU_ERROR("failed wb object creation: err %d\n", rc);
>> @@ -231,7 +231,7 @@ int dpu_rm_init(struct dpu_rm *rm,
>>                  struct dpu_hw_sspp *hw;
>>                  const struct dpu_sspp_cfg *sspp = &cat->sspp[i];
>>
>> -               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data);
>> +               hw = dpu_hw_sspp_init(sspp, mmio, mdss_data, cat->mdss_ver);
>>                  if (IS_ERR(hw)) {
>>                          rc = PTR_ERR(hw);
>>                          DPU_ERROR("failed sspp object creation: err %d\n", rc);
>>
>> --
>> 2.34.1
>>
> 
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-10-11 13:15 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-11 11:59 [PATCH v2 0/5] drm/msm/dpu: correctly implement SSPP & WB Clock Control Split Neil Armstrong
2023-10-11 11:59 ` Neil Armstrong
2023-10-11 11:59 ` [PATCH v2 1/5] drm/msm/dpu: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
2023-10-11 11:59   ` Neil Armstrong
2023-10-11 11:59 ` [PATCH v2 2/5] drm/msm/dpu: add setup_clk_force_ctrl() op to sspp & wb Neil Armstrong
2023-10-11 11:59   ` Neil Armstrong
2023-10-11 12:45   ` Dmitry Baryshkov
2023-10-11 12:45     ` Dmitry Baryshkov
2023-10-11 13:15     ` Neil Armstrong
2023-10-11 13:15       ` Neil Armstrong
2023-10-11 11:59 ` [PATCH v2 3/5] drm/msm/dpu: move setup_force_clk_ctrl handling into plane and wb Neil Armstrong
2023-10-11 11:59   ` Neil Armstrong
2023-10-11 12:55   ` Dmitry Baryshkov
2023-10-11 12:55     ` Dmitry Baryshkov
2023-10-11 11:59 ` [PATCH v2 4/5] drm/msm/dpu: sm8550: remove unused VIG and DMA clock controls entries Neil Armstrong
2023-10-11 11:59   ` Neil Armstrong
2023-10-11 12:55   ` Dmitry Baryshkov
2023-10-11 12:55     ` Dmitry Baryshkov
2023-10-11 11:59 ` [PATCH v2 5/5] drm/msm/dpu: enable writeback on SM8550 Neil Armstrong
2023-10-11 11:59   ` Neil Armstrong
2023-10-11 12:46   ` Dmitry Baryshkov
2023-10-11 12:46     ` Dmitry Baryshkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.