[PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split

[PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split

[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 from the vbif QoS
and OT limit setup functions.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Neil Armstrong (5):
      drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper
      drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb
      drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper
      drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
      drm/msm: dpu1: sm8550: move split clock controls to sspp entries

 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++-----------
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |  4 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        |  9 +++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        |  9 +++++
 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          |  9 +++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h          |  4 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 38 +++++++++++++++++-----
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
 13 files changed, 120 insertions(+), 61 deletions(-)
---
base-commit: 9119cf579b4432b36be9d33a92f4331922067d92
change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa

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

[PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper

[Neil Armstrong]

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

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

Re: [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper

[Dmitry Baryshkov]

On 09/10/2023 19:36, Neil Armstrong wrote:
> Add an helper to setup the force clock control as it will
> be used in multiple HW files.
> 
> 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(-)

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

-- 
With best wishes
Dmitry

[PATCH RFC 2/5] drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb

[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.

An optional clk_ctrl struct is added to the SSPP & WB caps,
which can be used by the setup_clk_force_ctrl() op.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 ++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c    | 9 +++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h    | 9 +++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c      | 9 +++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h      | 4 ++++
 5 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 6c9634209e9f..d9e8673e46f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -505,6 +505,7 @@ struct dpu_ctl_cfg {
  * @sblk:              SSPP sub-blocks information
  * @xin_id:            bus client identifier
  * @clk_ctrl           clock control identifier
+ * @clk_ctrl_reg       local sspp clock control register
  * @type               sspp type identifier
  */
 struct dpu_sspp_cfg {
@@ -512,6 +513,7 @@ struct dpu_sspp_cfg {
 	const struct dpu_sspp_sub_blks *sblk;
 	u32 xin_id;
 	enum dpu_clk_ctrl_type clk_ctrl;
+	const struct dpu_clk_ctrl_reg *clk_ctrl_reg;
 	u32 type;
 };
 
@@ -620,6 +622,7 @@ struct dpu_intf_cfg  {
  * @format_list:	    list of formats supported by this writeback block
  * @num_formats:	    number of formats supported by this writeback block
  * @clk_ctrl:	        clock control identifier
+ * @clk_ctrl_reg        local wb clock control register
  */
 struct dpu_wb_cfg {
 	DPU_HW_BLK_INFO;
@@ -630,6 +633,7 @@ struct dpu_wb_cfg {
 	const u32 *format_list;
 	u32 num_formats;
 	enum dpu_clk_ctrl_type clk_ctrl;
+	const struct dpu_clk_ctrl_reg *clk_ctrl_reg;
 };
 
 /**
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..cc4c7141791f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c
@@ -581,6 +581,14 @@ 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)
+{
+	if (!ctx->cap->clk_ctrl_reg)
+		return false;
+
+	return dpu_hw_clk_force_ctrl(&ctx->hw, ctx->cap->clk_ctrl_reg, enable);
+}
+
 static void _setup_layer_ops(struct dpu_hw_sspp *c,
 		unsigned long features)
 {
@@ -589,6 +597,7 @@ static void _setup_layer_ops(struct dpu_hw_sspp *c,
 	c->ops.setup_sourceaddress = dpu_hw_sspp_setup_sourceaddress;
 	c->ops.setup_solidfill = dpu_hw_sspp_setup_solidfill;
 	c->ops.setup_pe = dpu_hw_sspp_setup_pe_config;
+	c->ops.setup_clk_force_ctrl = dpu_hw_sspp_setup_clk_force_ctrl;
 
 	if (test_bit(DPU_SSPP_QOS, &features)) {
 		c->ops.setup_qos_lut = dpu_hw_sspp_setup_qos_lut;
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..4a77734e83a7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h
@@ -11,6 +11,7 @@
 #include "dpu_formats.h"
 
 struct dpu_hw_sspp;
+struct dpu_hw_mdp;
 
 /**
  * Flags
@@ -271,6 +272,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
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..045a4545a8c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.c
@@ -175,11 +175,20 @@ 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)
+{
+	if (!ctx->caps->clk_ctrl_reg)
+		return false;
+
+	return dpu_hw_clk_force_ctrl(&ctx->hw, ctx->caps->clk_ctrl_reg, enable);
+}
+
 static void _setup_wb_ops(struct dpu_hw_wb_ops *ops,
 		unsigned long features)
 {
 	ops->setup_outaddress = dpu_hw_wb_setup_outaddress;
 	ops->setup_outformat = dpu_hw_wb_setup_format;
+	ops->setup_clk_force_ctrl = dpu_hw_wb_setup_clk_force_ctrl;
 
 	if (test_bit(DPU_WB_XY_ROI_OFFSET, &features))
 		ops->setup_roi = dpu_hw_wb_roi;
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..3734ca435e01 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);
 };

-- 
2.34.1

[PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper

[Neil Armstrong]

Move the actual call to the MDP setup_clk_force_ctrl() op to
an helper which will call the correct op depending on the caps.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
index 1305e250b71e..2ae5cba1848b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
@@ -158,6 +158,13 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
 	return ot_lim;
 }
 
+static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
+					  unsigned int clk_ctrl,
+					  bool enable)
+{
+	return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
+}
+
 /**
  * dpu_vbif_set_ot_limit - set OT based on usecase & configuration parameters
  * @dpu_kms:	DPU handler
@@ -200,7 +207,7 @@ 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);
+	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
 
 	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
 
@@ -213,7 +220,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
 
 	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
 }
 
 void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
@@ -251,7 +258,7 @@ 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);
+	forced_on = dpu_vbif_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",
@@ -262,7 +269,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 	}
 
 	if (forced_on)
-		mdp->ops.setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
 }
 
 void dpu_vbif_clear_errors(struct dpu_kms *dpu_kms)

-- 
2.34.1

Re: [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper

[Dmitry Baryshkov]

On 09/10/2023 19:36, Neil Armstrong wrote:
> Move the actual call to the MDP setup_clk_force_ctrl() op to
> an helper which will call the correct op depending on the caps.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>

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


-- 
With best wishes
Dmitry

[PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control

[Neil Armstrong]

Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
qos & limit params then call the clk_force_ctrl() op accordingly.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
 drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
 4 files changed, 40 insertions(+), 22 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..e4dfe0be7207 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
@@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
 	struct dpu_vbif_set_ot_params ot_params;
 
 	memset(&ot_params, 0, sizeof(ot_params));
+	ot_params.wb = hw_wb;
 	ot_params.xin_id = hw_wb->caps->xin_id;
 	ot_params.num = hw_wb->idx - WB_0;
 	ot_params.width = phys_enc->cached_mode.hdisplay;
@@ -52,7 +53,6 @@ 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;
 
 	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
@@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap(
 	hw_wb = phys_enc->hw_wb;
 
 	memset(&qos_params, 0, sizeof(qos_params));
+	qos_params.wb = hw_wb;
 	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;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index c2aaaded07ed..b0b662068377 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
 	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
 
 	memset(&ot_params, 0, sizeof(ot_params));
+	ot_params.sspp = pipe->sspp;
 	ot_params.xin_id = pipe->sspp->cap->xin_id;
 	ot_params.num = pipe->sspp->idx - SSPP_NONE;
 	ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
@@ -357,7 +358,6 @@ 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;
 
 	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
@@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
 
 	memset(&qos_params, 0, sizeof(qos_params));
 	qos_params.vbif_idx = VBIF_RT;
-	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
+	qos_params.sspp = pipe->sspp;
 	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);
 
 	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
 }
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
index 2ae5cba1848b..a79559084a91 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
@@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
 	return ot_lim;
 }
 
-static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
-					  unsigned int clk_ctrl,
+static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
+					  struct dpu_hw_wb *wb,
+					  struct dpu_hw_mdp *mdp,
 					  bool enable)
 {
-	return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
+	if (sspp && sspp->cap->clk_ctrl_reg)
+		return sspp->ops.setup_clk_force_ctrl(sspp, enable);
+	else if (wb && wb->caps->clk_ctrl_reg)
+		return wb->ops.setup_clk_force_ctrl(wb, enable);
+	else
+		return mdp->ops.setup_clk_force_ctrl(mdp,
+				sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
+				enable);
 }
 
 /**
@@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	if (!mdp->ops.setup_clk_force_ctrl ||
-			!vbif->ops.set_limit_conf ||
-			!vbif->ops.set_halt_ctrl)
+	if ((!params->sspp && !params->wb) ||
+	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
+	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
+	    !mdp->ops.setup_clk_force_ctrl)
+		return;
+
+	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
 		return;
 
 	/* set write_gather_en for all write clients */
@@ -207,7 +219,7 @@ 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 = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
+	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
 
 	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
 
@@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
 	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
 
 	if (forced_on)
-		dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
 }
 
 void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
@@ -245,7 +257,10 @@ 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 ((!params->sspp && !params->wb) ||
+	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
+	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
+	    !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
 		DRM_DEBUG_ATOMIC("qos remap not supported\n");
 		return;
 	}
@@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 		return;
 	}
 
-	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
+	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
 
 	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
 		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
@@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
 	}
 
 	if (forced_on)
-		dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
+		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, 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..a4fe76e390d9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
@@ -7,7 +7,12 @@
 
 #include "dpu_kms.h"
 
+struct dpu_hw_sspp;
+struct dpu_hw_wb;
+
 struct dpu_vbif_set_ot_params {
+	struct dpu_hw_sspp *sspp;
+	struct dpu_hw_wb *wb;
 	u32 xin_id;
 	u32 num;
 	u32 width;
@@ -16,28 +21,27 @@ 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;
 };
 
 /**
  * struct dpu_vbif_set_qos_params - QoS remapper parameter
+ * @sspp: backing SSPP
  * @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 {
+	struct dpu_hw_sspp *sspp;
+	struct dpu_hw_wb *wb;
 	u32 vbif_idx;
 	u32 xin_id;
-	u32 clk_ctrl;
 	u32 num;
 	bool is_rt;
 };

-- 
2.34.1

Re: [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control

[Dmitry Baryshkov]

On 09/10/2023 19:36, Neil Armstrong wrote:
> Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
> on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
> qos & limit params then call the clk_force_ctrl() op accordingly.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>   4 files changed, 40 insertions(+), 22 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..e4dfe0be7207 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
> @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>   	struct dpu_vbif_set_ot_params ot_params;
>   
>   	memset(&ot_params, 0, sizeof(ot_params));
> +	ot_params.wb = hw_wb;
>   	ot_params.xin_id = hw_wb->caps->xin_id;
>   	ot_params.num = hw_wb->idx - WB_0;
>   	ot_params.width = phys_enc->cached_mode.hdisplay;
> @@ -52,7 +53,6 @@ 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;
>   
>   	dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
> @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>   	hw_wb = phys_enc->hw_wb;
>   
>   	memset(&qos_params, 0, sizeof(qos_params));
> +	qos_params.wb = hw_wb;
>   	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;
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c2aaaded07ed..b0b662068377 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>   	struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>   
>   	memset(&ot_params, 0, sizeof(ot_params));
> +	ot_params.sspp = pipe->sspp;
>   	ot_params.xin_id = pipe->sspp->cap->xin_id;
>   	ot_params.num = pipe->sspp->idx - SSPP_NONE;
>   	ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
> @@ -357,7 +358,6 @@ 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;
>   
>   	dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
> @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>   
>   	memset(&qos_params, 0, sizeof(qos_params));
>   	qos_params.vbif_idx = VBIF_RT;
> -	qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
> +	qos_params.sspp = pipe->sspp;
>   	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);
>   
>   	dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>   }
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> index 2ae5cba1848b..a79559084a91 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
> @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
>   	return ot_lim;
>   }
>   
> -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
> -					  unsigned int clk_ctrl,
> +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
> +					  struct dpu_hw_wb *wb,
> +					  struct dpu_hw_mdp *mdp,
>   					  bool enable)
>   {
> -	return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
> +	if (sspp && sspp->cap->clk_ctrl_reg)
> +		return sspp->ops.setup_clk_force_ctrl(sspp, enable);
> +	else if (wb && wb->caps->clk_ctrl_reg)
> +		return wb->ops.setup_clk_force_ctrl(wb, enable);
> +	else

This is what I wanted to avoid.

If we move the caller function to the sspp / WB, we will not need this 
kind of wrapper.

> +		return mdp->ops.setup_clk_force_ctrl(mdp,
> +				sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
> +				enable);
>   }
>   
>   /**
> @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>   		return;
>   	}
>   
> -	if (!mdp->ops.setup_clk_force_ctrl ||
> -			!vbif->ops.set_limit_conf ||
> -			!vbif->ops.set_halt_ctrl)
> +	if ((!params->sspp && !params->wb) ||
> +	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
> +	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
> +	    !mdp->ops.setup_clk_force_ctrl)
> +		return;
> +
> +	if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>   		return;
>   
>   	/* set write_gather_en for all write clients */
> @@ -207,7 +219,7 @@ 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 = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> +	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);

I'd suggest removing the setup_clk_force_ctrl from 
dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make 
dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which 
will enable the clock, call dpu_vbif then disable the clock.

In my opinion this is simpler than the condition in the previous chunk.

>   
>   	vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>   
> @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>   	vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
>   
>   	if (forced_on)
> -		dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
> +		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>   }
>   
>   void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
> @@ -245,7 +257,10 @@ 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 ((!params->sspp && !params->wb) ||
> +	    (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
> +	    (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
> +	    !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
>   		DRM_DEBUG_ATOMIC("qos remap not supported\n");
>   		return;
>   	}
> @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>   		return;
>   	}
>   
> -	forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
> +	forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
>   
>   	for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>   		DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
> @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>   	}
>   
>   	if (forced_on)
> -		dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
> +		dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, 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..a4fe76e390d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
> @@ -7,7 +7,12 @@
>   
>   #include "dpu_kms.h"
>   
> +struct dpu_hw_sspp;
> +struct dpu_hw_wb;
> +
>   struct dpu_vbif_set_ot_params {
> +	struct dpu_hw_sspp *sspp;
> +	struct dpu_hw_wb *wb;
>   	u32 xin_id;
>   	u32 num;
>   	u32 width;
> @@ -16,28 +21,27 @@ 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;
>   };
>   
>   /**
>    * struct dpu_vbif_set_qos_params - QoS remapper parameter
> + * @sspp: backing SSPP
>    * @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 {
> +	struct dpu_hw_sspp *sspp;
> +	struct dpu_hw_wb *wb;
>   	u32 vbif_idx;
>   	u32 xin_id;
> -	u32 clk_ctrl;
>   	u32 num;
>   	bool is_rt;
>   };
> 

-- 
With best wishes
Dmitry

Re: [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control

[Neil Armstrong]

On 09/10/2023 19:07, Dmitry Baryshkov wrote:
> On 09/10/2023 19:36, Neil Armstrong wrote:
>> Now clk_ctrl IDs can be optional and the clk_ctrl_reg can be specified
>> on the SSPP & WB caps directly, pass the SSPP & WB hw struct to the
>> qos & limit params then call the clk_force_ctrl() op accordingly.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 37 +++++++++++++++-------
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>>   4 files changed, 40 insertions(+), 22 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..e4dfe0be7207 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
>> @@ -45,6 +45,7 @@ static void dpu_encoder_phys_wb_set_ot_limit(
>>       struct dpu_vbif_set_ot_params ot_params;
>>       memset(&ot_params, 0, sizeof(ot_params));
>> +    ot_params.wb = hw_wb;
>>       ot_params.xin_id = hw_wb->caps->xin_id;
>>       ot_params.num = hw_wb->idx - WB_0;
>>       ot_params.width = phys_enc->cached_mode.hdisplay;
>> @@ -52,7 +53,6 @@ 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;
>>       dpu_vbif_set_ot_limit(phys_enc->dpu_kms, &ot_params);
>> @@ -81,9 +81,9 @@ static void dpu_encoder_phys_wb_set_qos_remap(
>>       hw_wb = phys_enc->hw_wb;
>>       memset(&qos_params, 0, sizeof(qos_params));
>> +    qos_params.wb = hw_wb;
>>       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;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> index c2aaaded07ed..b0b662068377 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
>> @@ -350,6 +350,7 @@ static void _dpu_plane_set_ot_limit(struct drm_plane *plane,
>>       struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
>>       memset(&ot_params, 0, sizeof(ot_params));
>> +    ot_params.sspp = pipe->sspp;
>>       ot_params.xin_id = pipe->sspp->cap->xin_id;
>>       ot_params.num = pipe->sspp->idx - SSPP_NONE;
>>       ot_params.width = drm_rect_width(&pipe_cfg->src_rect);
>> @@ -357,7 +358,6 @@ 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;
>>       dpu_vbif_set_ot_limit(dpu_kms, &ot_params);
>> @@ -377,16 +377,15 @@ static void _dpu_plane_set_qos_remap(struct drm_plane *plane,
>>       memset(&qos_params, 0, sizeof(qos_params));
>>       qos_params.vbif_idx = VBIF_RT;
>> -    qos_params.clk_ctrl = pipe->sspp->cap->clk_ctrl;
>> +    qos_params.sspp = pipe->sspp;
>>       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);
>>       dpu_vbif_set_qos_remap(dpu_kms, &qos_params);
>>   }
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> index 2ae5cba1848b..a79559084a91 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c
>> @@ -158,11 +158,19 @@ static u32 _dpu_vbif_get_ot_limit(struct dpu_hw_vbif *vbif,
>>       return ot_lim;
>>   }
>> -static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_mdp *mdp,
>> -                      unsigned int clk_ctrl,
>> +static bool dpu_vbif_setup_clk_force_ctrl(struct dpu_hw_sspp *sspp,
>> +                      struct dpu_hw_wb *wb,
>> +                      struct dpu_hw_mdp *mdp,
>>                         bool enable)
>>   {
>> -    return mdp->ops.setup_clk_force_ctrl(mdp, clk_ctrl, enable);
>> +    if (sspp && sspp->cap->clk_ctrl_reg)
>> +        return sspp->ops.setup_clk_force_ctrl(sspp, enable);
>> +    else if (wb && wb->caps->clk_ctrl_reg)
>> +        return wb->ops.setup_clk_force_ctrl(wb, enable);
>> +    else
> 
> This is what I wanted to avoid.
> 
> If we move the caller function to the sspp / WB, we will not need this kind of wrapper.

I tried it, but it requires passing the mdp pointer to the setup_clk_force_ctrl op,
which is IMHO not super clean... or if you have a way to get dpu_hw_mdp from
within hw_sspp/hw_wb it would help.

> 
>> +        return mdp->ops.setup_clk_force_ctrl(mdp,
>> +                sspp ? sspp->cap->clk_ctrl : wb->caps->clk_ctrl,
>> +                enable);
>>   }
>>   /**
>> @@ -190,9 +198,13 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>>           return;
>>       }
>> -    if (!mdp->ops.setup_clk_force_ctrl ||
>> -            !vbif->ops.set_limit_conf ||
>> -            !vbif->ops.set_halt_ctrl)
>> +    if ((!params->sspp && !params->wb) ||
>> +        (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
>> +        (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
>> +        !mdp->ops.setup_clk_force_ctrl)
>> +        return;
>> +
>> +    if (!vbif->ops.set_limit_conf || !vbif->ops.set_halt_ctrl)
>>           return;
>>       /* set write_gather_en for all write clients */
>> @@ -207,7 +219,7 @@ 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 = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
>> +    forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
> 
> I'd suggest removing the setup_clk_force_ctrl from dpu_vbif_set_ot_limit() and dpu_vbif_set_qos_remap(). Instead make dpu_plane / dpu_encoder_phys_wb call into dpu_hw_sspp / dpu_hw_wb, which will enable the clock, call dpu_vbif then disable the clock.
> 
> In my opinion this is simpler than the condition in the previous chunk.

Indeed this is a nice option, but the hw_mdp pointer requirement into hw_sspp/hw_wb
still puzzles me.

> 
>>       vbif->ops.set_limit_conf(vbif, params->xin_id, params->rd, ot_lim);
>> @@ -220,7 +232,7 @@ void dpu_vbif_set_ot_limit(struct dpu_kms *dpu_kms,
>>       vbif->ops.set_halt_ctrl(vbif, params->xin_id, false);
>>       if (forced_on)
>> -        dpu_vbif_setup_clk_force_ctrl(mdp,  params->clk_ctrl, false);
>> +        dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, false);
>>   }
>>   void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>> @@ -245,7 +257,10 @@ 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 ((!params->sspp && !params->wb) ||
>> +        (params->sspp && !params->sspp->ops.setup_clk_force_ctrl) ||
>> +        (params->wb && !params->wb->ops.setup_clk_force_ctrl) ||
>> +        !mdp->ops.setup_clk_force_ctrl || !vbif->ops.set_qos_remap) {
>>           DRM_DEBUG_ATOMIC("qos remap not supported\n");
>>           return;
>>       }
>> @@ -258,7 +273,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>>           return;
>>       }
>> -    forced_on = dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, true);
>> +    forced_on = dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, true);
>>       for (i = 0; i < qos_tbl->npriority_lvl; i++) {
>>           DRM_DEBUG_ATOMIC("%s xin:%d lvl:%d/%d\n",
>> @@ -269,7 +284,7 @@ void dpu_vbif_set_qos_remap(struct dpu_kms *dpu_kms,
>>       }
>>       if (forced_on)
>> -        dpu_vbif_setup_clk_force_ctrl(mdp, params->clk_ctrl, false);
>> +        dpu_vbif_setup_clk_force_ctrl(params->sspp, params->wb, mdp, 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..a4fe76e390d9 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h
>> @@ -7,7 +7,12 @@
>>   #include "dpu_kms.h"
>> +struct dpu_hw_sspp;
>> +struct dpu_hw_wb;
>> +
>>   struct dpu_vbif_set_ot_params {
>> +    struct dpu_hw_sspp *sspp;
>> +    struct dpu_hw_wb *wb;
>>       u32 xin_id;
>>       u32 num;
>>       u32 width;
>> @@ -16,28 +21,27 @@ 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;
>>   };
>>   /**
>>    * struct dpu_vbif_set_qos_params - QoS remapper parameter
>> + * @sspp: backing SSPP
>>    * @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 {
>> +    struct dpu_hw_sspp *sspp;
>> +    struct dpu_hw_wb *wb;
>>       u32 vbif_idx;
>>       u32 xin_id;
>> -    u32 clk_ctrl;
>>       u32 num;
>>       bool is_rt;
>>   };
>>
> 

Thanks,
Neil

[PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries

[Neil Armstrong]

The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them
out of the MDP top.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------
 1 file changed, 15 insertions(+), 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..527ec020fba4 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 },
 	},
 };
@@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = {
 	},
 };
 
+static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = {
+	.reg_off = 0x330,
+	.bit_off = 0
+};
+
 static const struct dpu_sspp_cfg sm8550_sspp[] = {
 	{
 		.name = "sspp_0", .id = SSPP_VIG0,
@@ -81,7 +76,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_1", .id = SSPP_VIG1,
 		.base = 0x6000, .len = 0x344,
@@ -89,7 +84,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_2", .id = SSPP_VIG2,
 		.base = 0x8000, .len = 0x344,
@@ -97,7 +92,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_3", .id = SSPP_VIG3,
 		.base = 0xa000, .len = 0x344,
@@ -105,7 +100,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_8", .id = SSPP_DMA0,
 		.base = 0x24000, .len = 0x344,
@@ -113,7 +108,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_9", .id = SSPP_DMA1,
 		.base = 0x26000, .len = 0x344,
@@ -121,7 +116,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_10", .id = SSPP_DMA2,
 		.base = 0x28000, .len = 0x344,
@@ -129,7 +124,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_11", .id = SSPP_DMA3,
 		.base = 0x2a000, .len = 0x344,
@@ -137,7 +132,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_12", .id = SSPP_DMA4,
 		.base = 0x2c000, .len = 0x344,
@@ -145,7 +140,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	}, {
 		.name = "sspp_13", .id = SSPP_DMA5,
 		.base = 0x2e000, .len = 0x344,
@@ -153,7 +148,7 @@ 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,
+		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
 	},
 };
 

-- 
2.34.1

Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries

[Dmitry Baryshkov]

On 09/10/2023 19:36, Neil Armstrong wrote:
> The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them
> out of the MDP top.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------
>   1 file changed, 15 insertions(+), 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..527ec020fba4 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 },

Hmm, interesting. I even double-checked this. SSPP and WB have their own 
clock registers now. But the REG_DMA uses the main area (0x2bc).

>   	},
>   };
> @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = {
>   	},
>   };
>   
> +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = {
> +	.reg_off = 0x330,
> +	.bit_off = 0
> +};

I don't think we even need this outside of dpu_hw_sspp. You can use 
core_major_rev to check whether the driver should use global clocks or 
per-SSPP / per-WB clocks register instead.

> +
>   static const struct dpu_sspp_cfg sm8550_sspp[] = {
>   	{
>   		.name = "sspp_0", .id = SSPP_VIG0,
> @@ -81,7 +76,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_1", .id = SSPP_VIG1,
>   		.base = 0x6000, .len = 0x344,
> @@ -89,7 +84,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_2", .id = SSPP_VIG2,
>   		.base = 0x8000, .len = 0x344,
> @@ -97,7 +92,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_3", .id = SSPP_VIG3,
>   		.base = 0xa000, .len = 0x344,
> @@ -105,7 +100,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_8", .id = SSPP_DMA0,
>   		.base = 0x24000, .len = 0x344,
> @@ -113,7 +108,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_9", .id = SSPP_DMA1,
>   		.base = 0x26000, .len = 0x344,
> @@ -121,7 +116,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_10", .id = SSPP_DMA2,
>   		.base = 0x28000, .len = 0x344,
> @@ -129,7 +124,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_11", .id = SSPP_DMA3,
>   		.base = 0x2a000, .len = 0x344,
> @@ -137,7 +132,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_12", .id = SSPP_DMA4,
>   		.base = 0x2c000, .len = 0x344,
> @@ -145,7 +140,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	}, {
>   		.name = "sspp_13", .id = SSPP_DMA5,
>   		.base = 0x2e000, .len = 0x344,
> @@ -153,7 +148,7 @@ 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,
> +		.clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>   	},
>   };
>   
> 

-- 
With best wishes
Dmitry

Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries

[Neil Armstrong]

On 09/10/2023 19:10, Dmitry Baryshkov wrote:
> On 09/10/2023 19:36, Neil Armstrong wrote:
>> The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them
>> out of the MDP top.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------
>>   1 file changed, 15 insertions(+), 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..527ec020fba4 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 },
> 
> Hmm, interesting. I even double-checked this. SSPP and WB have their own clock registers now. But the REG_DMA uses the main area (0x2bc).

yeah

> 
>>       },
>>   };
>> @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = {
>>       },
>>   };
>> +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = {
>> +    .reg_off = 0x330,
>> +    .bit_off = 0
>> +};
> 
> I don't think we even need this outside of dpu_hw_sspp. You can use core_major_rev to check whether the driver should use global clocks or per-SSPP / per-WB clocks register instead.

Ack

> 
>> +
>>   static const struct dpu_sspp_cfg sm8550_sspp[] = {
>>       {
>>           .name = "sspp_0", .id = SSPP_VIG0,
>> @@ -81,7 +76,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_1", .id = SSPP_VIG1,
>>           .base = 0x6000, .len = 0x344,
>> @@ -89,7 +84,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_2", .id = SSPP_VIG2,
>>           .base = 0x8000, .len = 0x344,
>> @@ -97,7 +92,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_3", .id = SSPP_VIG3,
>>           .base = 0xa000, .len = 0x344,
>> @@ -105,7 +100,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_8", .id = SSPP_DMA0,
>>           .base = 0x24000, .len = 0x344,
>> @@ -113,7 +108,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_9", .id = SSPP_DMA1,
>>           .base = 0x26000, .len = 0x344,
>> @@ -121,7 +116,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_10", .id = SSPP_DMA2,
>>           .base = 0x28000, .len = 0x344,
>> @@ -129,7 +124,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_11", .id = SSPP_DMA3,
>>           .base = 0x2a000, .len = 0x344,
>> @@ -137,7 +132,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_12", .id = SSPP_DMA4,
>>           .base = 0x2c000, .len = 0x344,
>> @@ -145,7 +140,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_13", .id = SSPP_DMA5,
>>           .base = 0x2e000, .len = 0x344,
>> @@ -153,7 +148,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       },
>>   };
>>
> 

Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries

[Neil Armstrong]

On 09/10/2023 19:10, Dmitry Baryshkov wrote:
> On 09/10/2023 19:36, Neil Armstrong wrote:
>> The SM8550 has the SSPP clk_ctrl in the SSPP registers, move them
>> out of the MDP top.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 ++++++++++------------
>>   1 file changed, 15 insertions(+), 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..527ec020fba4 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 },
> 
> Hmm, interesting. I even double-checked this. SSPP and WB have their own clock registers now. But the REG_DMA uses the main area (0x2bc).

yeah

> 
>>       },
>>   };
>> @@ -73,6 +63,11 @@ static const struct dpu_ctl_cfg sm8550_ctl[] = {
>>       },
>>   };
>> +static const struct dpu_clk_ctrl_reg sm8550_sspp_clk_ctrl = {
>> +    .reg_off = 0x330,
>> +    .bit_off = 0
>> +};
> 
> I don't think we even need this outside of dpu_hw_sspp. You can use core_major_rev to check whether the driver should use global clocks or per-SSPP / per-WB clocks register instead.

Ack

> 
>> +
>>   static const struct dpu_sspp_cfg sm8550_sspp[] = {
>>       {
>>           .name = "sspp_0", .id = SSPP_VIG0,
>> @@ -81,7 +76,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_1", .id = SSPP_VIG1,
>>           .base = 0x6000, .len = 0x344,
>> @@ -89,7 +84,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_2", .id = SSPP_VIG2,
>>           .base = 0x8000, .len = 0x344,
>> @@ -97,7 +92,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_3", .id = SSPP_VIG3,
>>           .base = 0xa000, .len = 0x344,
>> @@ -105,7 +100,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_8", .id = SSPP_DMA0,
>>           .base = 0x24000, .len = 0x344,
>> @@ -113,7 +108,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_9", .id = SSPP_DMA1,
>>           .base = 0x26000, .len = 0x344,
>> @@ -121,7 +116,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_10", .id = SSPP_DMA2,
>>           .base = 0x28000, .len = 0x344,
>> @@ -129,7 +124,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_11", .id = SSPP_DMA3,
>>           .base = 0x2a000, .len = 0x344,
>> @@ -137,7 +132,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_12", .id = SSPP_DMA4,
>>           .base = 0x2c000, .len = 0x344,
>> @@ -145,7 +140,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       }, {
>>           .name = "sspp_13", .id = SSPP_DMA5,
>>           .base = 0x2e000, .len = 0x344,
>> @@ -153,7 +148,7 @@ 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,
>> +        .clk_ctrl_reg = &sm8550_sspp_clk_ctrl,
>>       },
>>   };
>>
> 

Re: [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split

[Marijn Suijten]

On 2023-10-09 18:36:11, Neil Armstrong wrote:
> 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 from the vbif QoS
> and OT limit setup functions.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> Neil Armstrong (5):
>       drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper
>       drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb
>       drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper
>       drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
>       drm/msm: dpu1: sm8550: move split clock controls to sspp entries

Fyi we're all using drm/msm/dpu: now :)

- Marijn

> 
>  .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++-----------
>  .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |  4 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        |  9 +++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        |  9 +++++
>  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          |  9 +++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h          |  4 +++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 38 +++++++++++++++++-----
>  drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>  13 files changed, 120 insertions(+), 61 deletions(-)
> ---
> base-commit: 9119cf579b4432b36be9d33a92f4331922067d92
> change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa
> 
> Best regards,
> -- 
> Neil Armstrong <neil.armstrong@linaro.org>
> 

Re: [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split

[Neil Armstrong]

On 10/10/2023 10:10, Marijn Suijten wrote:
> On 2023-10-09 18:36:11, Neil Armstrong wrote:
>> 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 from the vbif QoS
>> and OT limit setup functions.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>> Neil Armstrong (5):
>>        drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper
>>        drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb
>>        drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper
>>        drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
>>        drm/msm: dpu1: sm8550: move split clock controls to sspp entries
> 
> Fyi we're all using drm/msm/dpu: now :)

Ack, thx, will change for v2

> 
> - Marijn
> 
>>
>>   .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 35 +++++++++-----------
>>   .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c    |  4 +--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h     |  4 +++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c        |  9 +++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.h        |  9 +++++
>>   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          |  9 +++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_wb.h          |  4 +++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c          |  9 +++--
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.c           | 38 +++++++++++++++++-----
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_vbif.h           | 12 ++++---
>>   13 files changed, 120 insertions(+), 61 deletions(-)
>> ---
>> base-commit: 9119cf579b4432b36be9d33a92f4331922067d92
>> change-id: 20231009-topic-sm8550-graphics-sspp-split-clk-43c32e37b6aa
>>
>> Best regards,
>> -- 
>> Neil Armstrong <neil.armstrong@linaro.org>
>>