* [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split
@ 2023-10-09 16:36 Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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 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>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
@ 2023-10-09 16:36 ` Neil Armstrong
2023-10-09 16:57 ` Dmitry Baryshkov
2023-10-09 16:36 ` [PATCH RFC 2/5] drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb Neil Armstrong
` (4 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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.
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] 15+ messages in thread
* [PATCH RFC 2/5] drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
@ 2023-10-09 16:36 ` Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper Neil Armstrong
` (3 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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.
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 2/5] drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb Neil Armstrong
@ 2023-10-09 16:36 ` Neil Armstrong
2023-10-09 17:02 ` Dmitry Baryshkov
2023-10-09 16:36 ` [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control Neil Armstrong
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
` (2 preceding siblings ...)
2023-10-09 16:36 ` [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper Neil Armstrong
@ 2023-10-09 16:36 ` Neil Armstrong
2023-10-09 17:07 ` Dmitry Baryshkov
2023-10-09 16:36 ` [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries Neil Armstrong
2023-10-10 8:10 ` [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Marijn Suijten
5 siblings, 1 reply; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
` (3 preceding siblings ...)
2023-10-09 16:36 ` [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control Neil Armstrong
@ 2023-10-09 16:36 ` Neil Armstrong
2023-10-09 17:10 ` Dmitry Baryshkov
2023-10-10 8:10 ` [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Marijn Suijten
5 siblings, 1 reply; 15+ messages in thread
From: Neil Armstrong @ 2023-10-09 16:36 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, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper
2023-10-09 16:36 ` [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
@ 2023-10-09 16:57 ` Dmitry Baryshkov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-10-09 16:57 UTC (permalink / raw)
To: Neil Armstrong, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper
2023-10-09 16:36 ` [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper Neil Armstrong
@ 2023-10-09 17:02 ` Dmitry Baryshkov
0 siblings, 0 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-10-09 17:02 UTC (permalink / raw)
To: Neil Armstrong, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
2023-10-09 16:36 ` [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control Neil Armstrong
@ 2023-10-09 17:07 ` Dmitry Baryshkov
2023-10-10 7:58 ` Neil Armstrong
0 siblings, 1 reply; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-10-09 17:07 UTC (permalink / raw)
To: Neil Armstrong, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries
2023-10-09 16:36 ` [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries Neil Armstrong
@ 2023-10-09 17:10 ` Dmitry Baryshkov
2023-10-10 8:00 ` Neil Armstrong
2023-10-10 8:00 ` Neil Armstrong
0 siblings, 2 replies; 15+ messages in thread
From: Dmitry Baryshkov @ 2023-10-09 17:10 UTC (permalink / raw)
To: Neil Armstrong, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control
2023-10-09 17:07 ` Dmitry Baryshkov
@ 2023-10-10 7:58 ` Neil Armstrong
0 siblings, 0 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-10 7:58 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries
2023-10-09 17:10 ` Dmitry Baryshkov
2023-10-10 8:00 ` Neil Armstrong
@ 2023-10-10 8:00 ` Neil Armstrong
1 sibling, 0 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-10 8:00 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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,
>> },
>> };
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries
2023-10-09 17:10 ` Dmitry Baryshkov
@ 2023-10-10 8:00 ` Neil Armstrong
2023-10-10 8:00 ` Neil Armstrong
1 sibling, 0 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-10 8:00 UTC (permalink / raw)
To: Dmitry Baryshkov, Rob Clark, Abhinav Kumar, Sean Paul,
Marijn Suijten, David Airlie, Daniel Vetter
Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel
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,
>> },
>> };
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
` (4 preceding siblings ...)
2023-10-09 16:36 ` [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries Neil Armstrong
@ 2023-10-10 8:10 ` Marijn Suijten
2023-10-10 8:12 ` Neil Armstrong
5 siblings, 1 reply; 15+ messages in thread
From: Marijn Suijten @ 2023-10-10 8:10 UTC (permalink / raw)
To: Neil Armstrong
Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
linux-kernel
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>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split
2023-10-10 8:10 ` [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Marijn Suijten
@ 2023-10-10 8:12 ` Neil Armstrong
0 siblings, 0 replies; 15+ messages in thread
From: Neil Armstrong @ 2023-10-10 8:12 UTC (permalink / raw)
To: Marijn Suijten
Cc: Rob Clark, Abhinav Kumar, Dmitry Baryshkov, Sean Paul,
David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno,
linux-kernel
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>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-10-10 8:12 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-09 16:36 [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 1/5] drm/msm: dpu1: create a dpu_hw_clk_force_ctrl() helper Neil Armstrong
2023-10-09 16:57 ` Dmitry Baryshkov
2023-10-09 16:36 ` [PATCH RFC 2/5] drm/msm: dpu1: add setup_clk_force_ctrl() op to sspp & wb Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 3/5] drm/msm: dpu1: vbif: add dpu_vbif_setup_clk_force_ctrl() helper Neil Armstrong
2023-10-09 17:02 ` Dmitry Baryshkov
2023-10-09 16:36 ` [PATCH RFC 4/5] drm/msm: dpu1: call wb & sspp clk_force_ctrl op if split clock control Neil Armstrong
2023-10-09 17:07 ` Dmitry Baryshkov
2023-10-10 7:58 ` Neil Armstrong
2023-10-09 16:36 ` [PATCH RFC 5/5] drm/msm: dpu1: sm8550: move split clock controls to sspp entries Neil Armstrong
2023-10-09 17:10 ` Dmitry Baryshkov
2023-10-10 8:00 ` Neil Armstrong
2023-10-10 8:00 ` Neil Armstrong
2023-10-10 8:10 ` [PATCH RFC 0/5] drm/msm: dpu1: correctly implement SSPP & WB Clock Control Split Marijn Suijten
2023-10-10 8:12 ` Neil Armstrong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).