This series adds the DPU side changes to support DSC 1.2 encoder. This
was validated with both DSI DSC 1.2 panel and DP DSC 1.2 monitor.
The DSI and DP parts will be pushed later on top of this change.
This seriel is rebase on [1], [2] and catalog fixes from rev-4 of [3].
[1]: https://patchwork.freedesktop.org/series/116851/
[2]: https://patchwork.freedesktop.org/series/116615/
[3]: https://patchwork.freedesktop.org/series/112332/
Abhinav Kumar (2):
drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X
drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
Kuogee Hsieh (6):
drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0
drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit
drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG
drm/msm/dpu: add support for DSC encoder v1.2 engine
drm/msm/dpu: separate DSC flush update out of interface
drm/msm/dpu: tear down DSC data path when DSC disabled
drivers/gpu/drm/msm/Makefile | 1 +
.../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +
.../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 +
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 51 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 29 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 35 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 29 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 15 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 386 +++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
19 files changed, 646 insertions(+), 29 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
--
2.7.4
From: Abhinav Kumar <[email protected]>
Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
feature flag information. Each display compression engine (DCE) contains
dual DSC encoders so both share same base address but with
its own different sub block address.
changes in v4:
-- delete DPU_DSC_HW_REV_1_1
-- re arrange sc8280xp_dsc[]
changes in v4:
-- fix checkpatch warning
changes in v10:
-- remove hard slice from commit text
-- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
-- change DSC_BLK_1_2 .len from 0x100 to 0x29c
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
kuogee: catalog.h
---
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
6 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
index 500cfd0..d90486f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
@@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8350_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8350_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
.dspp = sm8350_dspp,
.pingpong_count = ARRAY_SIZE(sm8350_pp),
.pingpong = sm8350_pp,
+ .dsc_count = ARRAY_SIZE(sm8350_dsc),
+ .dsc = sm8350_dsc,
.merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
.merge_3d = sm8350_merge_3d,
.intf_count = ARRAY_SIZE(sm8350_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
index 5646713..52609b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
@@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
};
+/* NOTE: sc7280 only has one DSC hard slice encoder */
+static const struct dpu_dsc_cfg sc7280_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+};
+
static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
.mixer = sc7280_lm,
.pingpong_count = ARRAY_SIZE(sc7280_pp),
.pingpong = sc7280_pp,
+ .dsc_count = ARRAY_SIZE(sc7280_dsc),
+ .dsc = sc7280_dsc,
.intf_count = ARRAY_SIZE(sc7280_intf),
.intf = sc7280_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index 808aacd..a84cf36 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+ DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
+};
+
/* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
static const struct dpu_intf_cfg sc8280xp_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
@@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
.dspp = sc8280xp_dspp,
.pingpong_count = ARRAY_SIZE(sc8280xp_pp),
.pingpong = sc8280xp_pp,
+ .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
+ .dsc = sc8280xp_dsc,
.merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
.merge_3d = sc8280xp_merge_3d,
.intf_count = ARRAY_SIZE(sc8280xp_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
index 1a89ff9..1620622 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
@@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8450_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8450_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
.dspp = sm8450_dspp,
.pingpong_count = ARRAY_SIZE(sm8450_pp),
.pingpong = sm8450_pp,
+ .dsc_count = ARRAY_SIZE(sm8450_dsc),
+ .dsc = sm8450_dsc,
.merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
.merge_3d = sm8450_merge_3d,
.intf_count = ARRAY_SIZE(sm8450_intf),
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 497b34c..6582a14 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
@@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8550_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8550_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
.dspp = sm8550_dspp,
.pingpong_count = ARRAY_SIZE(sm8550_pp),
.pingpong = sm8550_pp,
+ .dsc_count = ARRAY_SIZE(sm8550_dsc),
+ .dsc = sm8550_dsc,
.merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
.merge_3d = sm8550_merge_3d,
.intf_count = ARRAY_SIZE(sm8550_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index f2a1535..9612ab5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
- * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
@@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
/*************************************************************
* DSC sub blocks config
*************************************************************/
+static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
#define DSC_BLK(_name, _id, _base, _features) \
{\
.name = _name, .id = _id, \
@@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
.features = _features, \
}
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
+ {\
+ .name = _name, .id = _id, \
+ .base = _base, .len = _len, \
+ .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
+ .sblk = &_sblk, \
+ }
+
/*************************************************************
* INTF sub blocks config
*************************************************************/
--
2.7.4
Disabling the crossbar mux between DSC and PINGPONG currently
requires a bogus enum dpu_pingpong value to be passed when calling
dsc_bind_pingpong_blk() with enable=false, even though the register
value written is independent of the current PINGPONG block. Replace
that `bool enable` parameter with a new PINGPONG_NONE dpu_pingpong
flag that triggers the write of the "special" 0xF "crossbar
disabled" value to the register instead.
Changes in v4:
-- more details to commit text
Changes in v5:
-- rewording commit text suggested by Marijn
-- add DRM_DEBUG_KMS for DSC unbinding case
Changes in v8:
-- fix checkpatch warning
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +++++++-------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++-
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index cf1de5d..ffa6f04 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1850,7 +1850,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
hw_pp->ops.setup_dsc(hw_pp);
if (hw_dsc->ops.dsc_bind_pingpong_blk)
- hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, true, hw_pp->idx);
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, hw_pp->idx);
if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 4a6bbcc..f134fb0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -157,7 +157,6 @@ static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
static void dpu_hw_dsc_bind_pingpong_blk(
struct dpu_hw_dsc *hw_dsc,
- bool enable,
const enum dpu_pingpong pp)
{
struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
@@ -166,14 +165,15 @@ static void dpu_hw_dsc_bind_pingpong_blk(
dsc_ctl_offset = DSC_CTL(hw_dsc->idx);
- if (enable)
+ if (pp)
mux_cfg = (pp - PINGPONG_0) & 0x7;
- DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n",
- enable ? "Binding" : "Unbinding",
- hw_dsc->idx - DSC_0,
- enable ? "to" : "from",
- pp - PINGPONG_0);
+ if (pp)
+ DRM_DEBUG_KMS("Binding dsc:%d to pp:%d\n",
+ hw_dsc->idx - DSC_0, pp - PINGPONG_0);
+ else
+ DRM_DEBUG_KMS("Unbinding dsc:%d from any pp\n",
+ hw_dsc->idx - DSC_0);
DPU_REG_WRITE(c, dsc_ctl_offset, mux_cfg);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index 287ec5f..138080a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -44,7 +44,6 @@ struct dpu_hw_dsc_ops {
struct drm_dsc_config *dsc);
void (*dsc_bind_pingpong_blk)(struct dpu_hw_dsc *hw_dsc,
- bool enable,
enum dpu_pingpong pp);
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 1913a19..02a0f48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -191,7 +191,8 @@ enum dpu_dsc {
};
enum dpu_pingpong {
- PINGPONG_0 = 1,
+ PINGPONG_NONE,
+ PINGPONG_0,
PINGPONG_1,
PINGPONG_2,
PINGPONG_3,
--
2.7.4
Unset DSC_ACTIVE bit at dpu_hw_ctl_reset_intf_cfg_v1(),
dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() functions
to tear down DSC data path if DSC data path was setup previous.
Changes in V10:
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
-- move both dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() to above phys_cleanup()
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1957545..ec2a55a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2036,6 +2036,41 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
}
}
+static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
+ struct dpu_hw_pingpong *hw_pp)
+{
+ if (hw_dsc->ops.dsc_disable)
+ hw_dsc->ops.dsc_disable(hw_dsc);
+
+ if (hw_pp->ops.disable_dsc)
+ hw_pp->ops.disable_dsc(hw_pp);
+
+ if (hw_dsc->ops.dsc_bind_pingpong_blk)
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, PINGPONG_NONE);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
+}
+
+static void dpu_encoder_unprep_dsc(struct dpu_encoder_virt *dpu_enc)
+{
+ /* coding only for 2LM, 2enc, 1 dsc config */
+ struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
+ struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+ int i;
+
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ hw_pp[i] = dpu_enc->hw_pp[i];
+ hw_dsc[i] = dpu_enc->hw_dsc[i];
+
+ if (hw_pp[i] && hw_dsc[i])
+ dpu_encoder_dsc_pipe_clr(ctl, hw_dsc[i], hw_pp[i]);
+ }
+}
+
void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
@@ -2086,8 +2121,12 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}
+ if (dpu_enc->dsc)
+ dpu_encoder_unprep_dsc(dpu_enc);
+
intf_cfg.stream_sel = 0; /* Don't care value for video mode */
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);
if (phys_enc->hw_intf)
intf_cfg.intf = phys_enc->hw_intf->idx;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4e132d9..2663626 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -578,6 +578,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
u32 intf_active = 0;
u32 wb_active = 0;
u32 merge3d_active = 0;
+ u32 dsc_active;
/*
* This API resets each portion of the CTL path namely,
@@ -607,6 +608,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
wb_active &= ~BIT(cfg->wb - WB_0);
DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
}
+
+ if (cfg->dsc) {
+ dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
+ dsc_active &= ~cfg->dsc;
+ DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
+ }
}
static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
--
2.7.4
Currently DSC flushing happens during interface configuration at
dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
to handle both per-DSC engine and DSC flush bits at same time to make it
consistent with the location of flush programming of other DPU sub-blocks.
Changes in v10:
-- rewording commit text
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index ffa6f04..1957545 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
return DIV_ROUND_UP(total_pixels, dsc->slice_width);
}
-static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
+static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
struct dpu_hw_pingpong *hw_pp,
struct drm_dsc_config *dsc,
u32 common_mode,
@@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
}
static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
@@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
{
/* coding only for 2LM, 2enc, 1 dsc config */
struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
int this_frame_slices;
@@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
- dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
+ dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
+ dsc_common_mode, initial_lines);
}
void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4f7cfa9..4e132d9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
CTL_DSPP_n_FLUSH(dspp - DSPP_0),
ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
}
+
+ if (ctx->pending_flush_mask & BIT(DSC_IDX))
+ DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
+ ctx->pending_dsc_flush_mask);
+
DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
}
@@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
}
+static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
+ enum dpu_dsc dsc_num)
+{
+ ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
+ ctx->pending_flush_mask |= BIT(DSC_IDX);
+}
+
static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
enum dpu_dspp dspp, u32 dspp_sub_blk)
{
@@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
mode_sel = CTL_DEFAULT_GROUP_ID << 28;
- if (cfg->dsc)
- DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
-
if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
mode_sel |= BIT(17);
@@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
BIT(cfg->merge_3d - MERGE_3D_0));
- if (cfg->dsc) {
- DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
+
+ if (cfg->dsc)
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
- }
}
static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
@@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
ops->update_pending_flush_merge_3d =
dpu_hw_ctl_update_pending_flush_merge_3d_v1;
ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
+ ops->update_pending_flush_dsc =
+ dpu_hw_ctl_update_pending_flush_dsc_v1;
} else {
ops->trigger_flush = dpu_hw_ctl_trigger_flush;
ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 6292002..d5f3ef8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
enum dpu_dspp blk, u32 dspp_sub_blk);
/**
+ * OR in the given flushbits to the cached pending_(dsc_)flush_mask
+ * No effect on hardware
+ * @ctx: ctl path ctx pointer
+ * @blk: interface block index
+ */
+ void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
+ enum dpu_dsc blk);
+
+ /**
* Write the value of the pending_flush_mask to hardware
* @ctx : ctl path ctx pointer
*/
@@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
* @pending_flush_mask: storage for pending ctl_flush managed via ops
* @pending_intf_flush_mask: pending INTF flush
* @pending_wb_flush_mask: pending WB flush
+ * @pending_merge_3d_flush_mask: pending merge_3d flush
+ * @pending_dspp_flush_mask: pending dspp flush
+ * @pending_dsc_flush_mask: pending dsc flush
* @ops: operation list
*/
struct dpu_hw_ctl {
@@ -245,6 +257,7 @@ struct dpu_hw_ctl {
u32 pending_wb_flush_mask;
u32 pending_merge_3d_flush_mask;
u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
+ u32 pending_dsc_flush_mask;
/* ops */
struct dpu_hw_ctl_ops ops;
--
2.7.4
From: Abhinav Kumar <[email protected]>
Some platforms have DSC blocks which have not been declared in the catalog.
Complete DSC 1.1 support for all platforms by adding the missing blocks to
MSM8998 and SC8180X.
Changes in v9:
-- add MSM8998 and SC8180x to commit titil
Changes in v10:
-- fix grammar at commit text
Signed-off-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++++++
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++++++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index c0dd477..521cfd5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = {
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
};
+static const struct dpu_dsc_cfg msm8998_dsc[] = {
+ DSC_BLK("dsc_0", DSC_0, 0x80000, 0),
+ DSC_BLK("dsc_1", DSC_1, 0x80400, 0),
+};
+
static const struct dpu_dspp_cfg msm8998_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&msm8998_dspp_sblk),
@@ -199,6 +204,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = {
.dspp = msm8998_dspp,
.pingpong_count = ARRAY_SIZE(msm8998_pp),
.pingpong = msm8998_pp,
+ .dsc_count = ARRAY_SIZE(msm8998_dsc),
+ .dsc = msm8998_dsc,
.intf_count = ARRAY_SIZE(msm8998_intf),
.intf = msm8998_intf,
.vbif_count = ARRAY_SIZE(msm8998_vbif),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
index e8057a1..fec1665 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
@@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200),
};
+static const struct dpu_dsc_cfg sc8180x_dsc[] = {
+ DSC_BLK("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)),
+};
+
static const struct dpu_intf_cfg sc8180x_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -206,6 +215,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = {
.mixer = sc8180x_lm,
.pingpong_count = ARRAY_SIZE(sc8180x_pp),
.pingpong = sc8180x_pp,
+ .dsc_count = ARRAY_SIZE(sc8180x_dsc),
+ .dsc = sc8180x_dsc,
.merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d),
.merge_3d = sc8180x_merge_3d,
.intf_count = ARRAY_SIZE(sc8180x_intf),
--
2.7.4
DSC* in the title.
On 2023-05-17 15:01:52, Kuogee Hsieh wrote:
> From: Abhinav Kumar <[email protected]>
>
> Some platforms have DSC blocks which have not been declared in the catalog.
> Complete DSC 1.1 support for all platforms by adding the missing blocks to
> MSM8998 and SC8180X.
>
> Changes in v9:
> -- add MSM8998 and SC8180x to commit titil
title*
(couldn't be caught in v9... because this changelog wasn't present then)
- Marijn
>
> Changes in v10:
> -- fix grammar at commit text
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Reviewed-by: Marijn Suijten <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++++++
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index c0dd477..521cfd5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = {
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> };
>
> +static const struct dpu_dsc_cfg msm8998_dsc[] = {
> + DSC_BLK("dsc_0", DSC_0, 0x80000, 0),
> + DSC_BLK("dsc_1", DSC_1, 0x80400, 0),
> +};
> +
> static const struct dpu_dspp_cfg msm8998_dspp[] = {
> DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
> &msm8998_dspp_sblk),
> @@ -199,6 +204,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = {
> .dspp = msm8998_dspp,
> .pingpong_count = ARRAY_SIZE(msm8998_pp),
> .pingpong = msm8998_pp,
> + .dsc_count = ARRAY_SIZE(msm8998_dsc),
> + .dsc = msm8998_dsc,
> .intf_count = ARRAY_SIZE(msm8998_intf),
> .intf = msm8998_intf,
> .vbif_count = ARRAY_SIZE(msm8998_vbif),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> index e8057a1..fec1665 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> @@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200),
> };
>
> +static const struct dpu_dsc_cfg sc8180x_dsc[] = {
> + DSC_BLK("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)),
> +};
> +
> static const struct dpu_intf_cfg sc8180x_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -206,6 +215,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = {
> .mixer = sc8180x_lm,
> .pingpong_count = ARRAY_SIZE(sc8180x_pp),
> .pingpong = sc8180x_pp,
> + .dsc_count = ARRAY_SIZE(sc8180x_dsc),
> + .dsc = sc8180x_dsc,
> .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d),
> .merge_3d = sc8180x_merge_3d,
> .intf_count = ARRAY_SIZE(sc8180x_intf),
> --
> 2.7.4
>
On 2023-05-17 15:01:57, Kuogee Hsieh wrote:
> Currently DSC flushing happens during interface configuration at
> dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
> dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
> to handle both per-DSC engine and DSC flush bits at same time to make it
> consistent with the location of flush programming of other DPU sub-blocks.
>
> Changes in v10:
> -- rewording commit text
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
There are a few things missing from v8 review, see below.
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
> 3 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ffa6f04..1957545 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
> return DIV_ROUND_UP(total_pixels, dsc->slice_width);
> }
>
> -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> struct dpu_hw_pingpong *hw_pp,
> struct drm_dsc_config *dsc,
> u32 common_mode,
> @@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
>
> if (hw_pp->ops.enable_dsc)
> hw_pp->ops.enable_dsc(hw_pp);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> }
>
> static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> @@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> {
> /* coding only for 2LM, 2enc, 1 dsc config */
> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> int this_frame_slices;
> @@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>
> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
> + dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
> + dsc_common_mode, initial_lines);
> }
>
> void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4f7cfa9..4e132d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> }
> +
> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> + ctx->pending_dsc_flush_mask);
Again, when do we reset this mask to 0? (v8 review)
> +
> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> }
>
> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> }
>
> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc dsc_num)
> +{
> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> + ctx->pending_flush_mask |= BIT(DSC_IDX);
> +}
> +
> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
> enum dpu_dspp dspp, u32 dspp_sub_blk)
> {
> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>
> - if (cfg->dsc)
> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> -
> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> mode_sel |= BIT(17);
>
> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if (cfg->merge_3d)
> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> BIT(cfg->merge_3d - MERGE_3D_0));
> - if (cfg->dsc) {
> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
separate Fixes: patch to have this semantic change documented. (v8
review)
> +
> + if (cfg->dsc)
> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> - }
> }
>
> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
> ops->update_pending_flush_merge_3d =
> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
> + ops->update_pending_flush_dsc =
> + dpu_hw_ctl_update_pending_flush_dsc_v1;
> } else {
> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 6292002..d5f3ef8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> enum dpu_dspp blk, u32 dspp_sub_blk);
>
> /**
> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> + * No effect on hardware
> + * @ctx: ctl path ctx pointer
> + * @blk: interface block index
> + */
> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc blk);
> +
> + /**
> * Write the value of the pending_flush_mask to hardware
> * @ctx : ctl path ctx pointer
> */
> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
> * @pending_flush_mask: storage for pending ctl_flush managed via ops
> * @pending_intf_flush_mask: pending INTF flush
> * @pending_wb_flush_mask: pending WB flush
The above is all capitalized, so...:
> + * @pending_merge_3d_flush_mask: pending merge_3d flush
MERGE_3D?
> + * @pending_dspp_flush_mask: pending dspp flush
DSPP
> + * @pending_dsc_flush_mask: pending dsc flush
DSC
- Marijn
> * @ops: operation list
> */
> struct dpu_hw_ctl {
> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
> u32 pending_wb_flush_mask;
> u32 pending_merge_3d_flush_mask;
> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
> + u32 pending_dsc_flush_mask;
>
> /* ops */
> struct dpu_hw_ctl_ops ops;
> --
> 2.7.4
>
Title: Tear down DSC datapath* on encoder cleanup*
On 2023-05-17 15:01:59, Kuogee Hsieh wrote:
> Unset DSC_ACTIVE bit at dpu_hw_ctl_reset_intf_cfg_v1(),
> dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() functions
> to tear down DSC data path if DSC data path was setup previous.
>
> Changes in V10:
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
> -- move both dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() to above phys_cleanup()
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1957545..ec2a55a6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2036,6 +2036,41 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
> }
> }
>
> +static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> + struct dpu_hw_pingpong *hw_pp)
> +{
> + if (hw_dsc->ops.dsc_disable)
> + hw_dsc->ops.dsc_disable(hw_dsc);
> +
> + if (hw_pp->ops.disable_dsc)
> + hw_pp->ops.disable_dsc(hw_pp);
> +
> + if (hw_dsc->ops.dsc_bind_pingpong_blk)
> + hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, PINGPONG_NONE);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> +}
> +
> +static void dpu_encoder_unprep_dsc(struct dpu_encoder_virt *dpu_enc)
> +{
> + /* coding only for 2LM, 2enc, 1 dsc config */
> + struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> + struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> + struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> + int i;
> +
> + for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> + hw_pp[i] = dpu_enc->hw_pp[i];
> + hw_dsc[i] = dpu_enc->hw_dsc[i];
> +
> + if (hw_pp[i] && hw_dsc[i])
> + dpu_encoder_dsc_pipe_clr(ctl, hw_dsc[i], hw_pp[i]);
> + }
> +}
> +
> void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
> {
> struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
> @@ -2086,8 +2121,12 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
> phys_enc->hw_pp->merge_3d->idx);
> }
>
> + if (dpu_enc->dsc)
> + dpu_encoder_unprep_dsc(dpu_enc);
> +
> intf_cfg.stream_sel = 0; /* Don't care value for video mode */
> intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);
>
> if (phys_enc->hw_intf)
> intf_cfg.intf = phys_enc->hw_intf->idx;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4e132d9..2663626 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -578,6 +578,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> u32 intf_active = 0;
> u32 wb_active = 0;
> u32 merge3d_active = 0;
> + u32 dsc_active;
>
> /*
> * This API resets each portion of the CTL path namely,
> @@ -607,6 +608,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> wb_active &= ~BIT(cfg->wb - WB_0);
> DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
> }
> +
> + if (cfg->dsc) {
> + dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
> + dsc_active &= ~cfg->dsc;
> + DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
> + }
> }
>
> static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
> --
> 2.7.4
>
Title: "DPU >= 7.0" instead of "relevant chipsets" to match the others.
On 2023-05-17 15:01:58, Kuogee Hsieh wrote:
> From: Abhinav Kumar <[email protected]>
>
> Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
> feature flag information. Each display compression engine (DCE) contains
> dual DSC encoders so both share same base address but with
> its own different sub block address.
If you reword it, also reflow this line.
>
> changes in v4:
> -- delete DPU_DSC_HW_REV_1_1
> -- re arrange sc8280xp_dsc[]
>
> changes in v4:
> -- fix checkpatch warning
>
> changes in v10:
> -- remove hard slice from commit text
It is still mentioned in the diff though, that's why I originally
requested a better place to describe it.
> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
> -- change DSC_BLK_1_2 .len from 0x100 to 0x29c
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
>
> kuogee: catalog.h
What's this for? This file isn't touched in this patch.
> ---
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
> 6 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> index 500cfd0..d90486f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8350_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8350_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
> .dspp = sm8350_dspp,
> .pingpong_count = ARRAY_SIZE(sm8350_pp),
> .pingpong = sm8350_pp,
> + .dsc_count = ARRAY_SIZE(sm8350_dsc),
> + .dsc = sm8350_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
> .merge_3d = sm8350_merge_3d,
> .intf_count = ARRAY_SIZE(sm8350_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> index 5646713..52609b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
> PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
> };
>
> +/* NOTE: sc7280 only has one DSC hard slice encoder */
> +static const struct dpu_dsc_cfg sc7280_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> +};
> +
> static const struct dpu_intf_cfg sc7280_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
> .mixer = sc7280_lm,
> .pingpong_count = ARRAY_SIZE(sc7280_pp),
> .pingpong = sc7280_pp,
> + .dsc_count = ARRAY_SIZE(sc7280_dsc),
> + .dsc = sc7280_dsc,
> .intf_count = ARRAY_SIZE(sc7280_intf),
> .intf = sc7280_intf,
> .vbif_count = ARRAY_SIZE(sdm845_vbif),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> index 808aacd..a84cf36 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> + DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
> +};
> +
> /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
> static const struct dpu_intf_cfg sc8280xp_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> @@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
> .dspp = sc8280xp_dspp,
> .pingpong_count = ARRAY_SIZE(sc8280xp_pp),
> .pingpong = sc8280xp_pp,
> + .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
> + .dsc = sc8280xp_dsc,
> .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
> .merge_3d = sc8280xp_merge_3d,
> .intf_count = ARRAY_SIZE(sc8280xp_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> index 1a89ff9..1620622 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> @@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8450_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8450_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
> .dspp = sm8450_dspp,
> .pingpong_count = ARRAY_SIZE(sm8450_pp),
> .pingpong = sm8450_pp,
> + .dsc_count = ARRAY_SIZE(sm8450_dsc),
> + .dsc = sm8450_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
> .merge_3d = sm8450_merge_3d,
> .intf_count = ARRAY_SIZE(sm8450_intf),
> 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 497b34c..6582a14 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
> @@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8550_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
> .dspp = sm8550_dspp,
> .pingpong_count = ARRAY_SIZE(sm8550_pp),
> .pingpong = sm8550_pp,
> + .dsc_count = ARRAY_SIZE(sm8550_dsc),
> + .dsc = sm8550_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
> .merge_3d = sm8550_merge_3d,
> .intf_count = ARRAY_SIZE(sm8550_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index f2a1535..9612ab5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
> @@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> /*************************************************************
> * DSC sub blocks config
> *************************************************************/
> +static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
> + .enc = {.base = 0x100, .len = 0x100},
> + .ctl = {.base = 0xF00, .len = 0x10},
> +};
> +
> +static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
> + .enc = {.base = 0x200, .len = 0x100},
> + .ctl = {.base = 0xF80, .len = 0x10},
> +};
> +
> #define DSC_BLK(_name, _id, _base, _features) \
> {\
> .name = _name, .id = _id, \
> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> .features = _features, \
> }
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
I still think this comment is superfluous (and doesn't even apply
generically, see i.e. sc7280) and should best be kept exclusively in the
SoC-specific catalog files.
- Marijn
> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
> + {\
> + .name = _name, .id = _id, \
> + .base = _base, .len = _len, \
> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
> + .sblk = &_sblk, \
> + }
> +
> /*************************************************************
> * INTF sub blocks config
> *************************************************************/
> --
> 2.7.4
>
On 5/17/2023 3:47 PM, Marijn Suijten wrote:
> Title: "DPU >= 7.0" instead of "relevant chipsets" to match the others.
>
> On 2023-05-17 15:01:58, Kuogee Hsieh wrote:
>> From: Abhinav Kumar <[email protected]>
>>
>> Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
>> feature flag information. Each display compression engine (DCE) contains
>> dual DSC encoders so both share same base address but with
>> its own different sub block address.
>
> If you reword it, also reflow this line.
>
>>
>> changes in v4:
>> -- delete DPU_DSC_HW_REV_1_1
>> -- re arrange sc8280xp_dsc[]
>>
>> changes in v4:
>> -- fix checkpatch warning
>>
>> changes in v10:
>> -- remove hard slice from commit text
>
> It is still mentioned in the diff though, that's why I originally
> requested a better place to describe it.
>
>> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
>> -- change DSC_BLK_1_2 .len from 0x100 to 0x29c
>>
>> Signed-off-by: Abhinav Kumar <[email protected]>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> Reviewed-by: Dmitry Baryshkov <[email protected]>
>
> Reviewed-by: Marijn Suijten <[email protected]>
>
>>
>> kuogee: catalog.h
>
> What's this for? This file isn't touched in this patch.
>
>> ---
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
>> 6 files changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> index 500cfd0..d90486f 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8350_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8350_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
>> .dspp = sm8350_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8350_pp),
>> .pingpong = sm8350_pp,
>> + .dsc_count = ARRAY_SIZE(sm8350_dsc),
>> + .dsc = sm8350_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
>> .merge_3d = sm8350_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8350_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> index 5646713..52609b8 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
>> PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
>> };
>>
>> +/* NOTE: sc7280 only has one DSC hard slice encoder */
>> +static const struct dpu_dsc_cfg sc7280_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +};
>> +
>> static const struct dpu_intf_cfg sc7280_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
>> .mixer = sc7280_lm,
>> .pingpong_count = ARRAY_SIZE(sc7280_pp),
>> .pingpong = sc7280_pp,
>> + .dsc_count = ARRAY_SIZE(sc7280_dsc),
>> + .dsc = sc7280_dsc,
>> .intf_count = ARRAY_SIZE(sc7280_intf),
>> .intf = sc7280_intf,
>> .vbif_count = ARRAY_SIZE(sdm845_vbif),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> index 808aacd..a84cf36 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> + DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
>> +};
>> +
>> /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
>> static const struct dpu_intf_cfg sc8280xp_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> @@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
>> .dspp = sc8280xp_dspp,
>> .pingpong_count = ARRAY_SIZE(sc8280xp_pp),
>> .pingpong = sc8280xp_pp,
>> + .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
>> + .dsc = sc8280xp_dsc,
>> .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
>> .merge_3d = sc8280xp_merge_3d,
>> .intf_count = ARRAY_SIZE(sc8280xp_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> index 1a89ff9..1620622 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> @@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8450_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8450_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
>> .dspp = sm8450_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8450_pp),
>> .pingpong = sm8450_pp,
>> + .dsc_count = ARRAY_SIZE(sm8450_dsc),
>> + .dsc = sm8450_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
>> .merge_3d = sm8450_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8450_intf),
>> 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 497b34c..6582a14 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
>> @@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8550_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
>> .dspp = sm8550_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8550_pp),
>> .pingpong = sm8550_pp,
>> + .dsc_count = ARRAY_SIZE(sm8550_dsc),
>> + .dsc = sm8550_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
>> .merge_3d = sm8550_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8550_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> index f2a1535..9612ab5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1,6 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
>> @@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>> /*************************************************************
>> * DSC sub blocks config
>> *************************************************************/
>> +static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
>> + .enc = {.base = 0x100, .len = 0x100},
>> + .ctl = {.base = 0xF00, .len = 0x10},
>> +};
>> +
>> +static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
>> + .enc = {.base = 0x200, .len = 0x100},
>> + .ctl = {.base = 0xF80, .len = 0x10},
>> +};
>> +
>> #define DSC_BLK(_name, _id, _base, _features) \
>> {\
>> .name = _name, .id = _id, \
>> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>> .features = _features, \
>> }
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>
> I still think this comment is superfluous (and doesn't even apply
> generically, see i.e. sc7280) and should best be kept exclusively in the
> SoC-specific catalog files.
>
> - Marijn
>
sc7280 is the only exception as it has only one encoder. But, by and
large, for all other chipsets this is true and hence kept here.
The main reason for this comment is people should not get confused that
how come two DSC encoders have the same base address.
>> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
>> + {\
>> + .name = _name, .id = _id, \
>> + .base = _base, .len = _len, \
>> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
>> + .sblk = &_sblk, \
>> + }
>> +
>> /*************************************************************
>> * INTF sub blocks config
>> *************************************************************/
>> --
>> 2.7.4
>>
On 18/05/2023 01:01, Kuogee Hsieh wrote:
> Currently DSC flushing happens during interface configuration at
> dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
> dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
> to handle both per-DSC engine and DSC flush bits at same time to make it
> consistent with the location of flush programming of other DPU sub-blocks.
>
> Changes in v10:
> -- rewording commit text
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
> 3 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ffa6f04..1957545 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
> return DIV_ROUND_UP(total_pixels, dsc->slice_width);
> }
>
> -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> struct dpu_hw_pingpong *hw_pp,
> struct drm_dsc_config *dsc,
> u32 common_mode,
> @@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
>
> if (hw_pp->ops.enable_dsc)
> hw_pp->ops.enable_dsc(hw_pp);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> }
>
> static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> @@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> {
> /* coding only for 2LM, 2enc, 1 dsc config */
> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> int this_frame_slices;
> @@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>
> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
> + dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
> + dsc_common_mode, initial_lines);
Moving dsc to the next line would be more logical.
> }
>
> void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4f7cfa9..4e132d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> }
> +
> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> + ctx->pending_dsc_flush_mask);
> +
> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> }
>
> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> }
>
> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc dsc_num)
> +{
> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> + ctx->pending_flush_mask |= BIT(DSC_IDX);
> +}
> +
> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
> enum dpu_dspp dspp, u32 dspp_sub_blk)
> {
> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>
> - if (cfg->dsc)
> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> -
> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> mode_sel |= BIT(17);
>
> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if (cfg->merge_3d)
> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> BIT(cfg->merge_3d - MERGE_3D_0));
> - if (cfg->dsc) {
> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> +
> + if (cfg->dsc)
> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> - }
> }
>
> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
> ops->update_pending_flush_merge_3d =
> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
> + ops->update_pending_flush_dsc =
> + dpu_hw_ctl_update_pending_flush_dsc_v1;
> } else {
> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 6292002..d5f3ef8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> enum dpu_dspp blk, u32 dspp_sub_blk);
>
> /**
> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> + * No effect on hardware
> + * @ctx: ctl path ctx pointer
> + * @blk: interface block index
> + */
> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc blk);
Please align to the opening parenthesis.
> +
> + /**
> * Write the value of the pending_flush_mask to hardware
> * @ctx : ctl path ctx pointer
> */
> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
> * @pending_flush_mask: storage for pending ctl_flush managed via ops
> * @pending_intf_flush_mask: pending INTF flush
> * @pending_wb_flush_mask: pending WB flush
> + * @pending_merge_3d_flush_mask: pending merge_3d flush
> + * @pending_dspp_flush_mask: pending dspp flush
These two can go to a separate patch.
> + * @pending_dsc_flush_mask: pending dsc flush
> * @ops: operation list
> */
> struct dpu_hw_ctl {
> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
> u32 pending_wb_flush_mask;
> u32 pending_merge_3d_flush_mask;
> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
> + u32 pending_dsc_flush_mask;
>
> /* ops */
> struct dpu_hw_ctl_ops ops;
--
With best wishes
Dmitry
On 2023-05-18 03:35:31, Dmitry Baryshkov wrote:
<snip>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > index 6292002..d5f3ef8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> > enum dpu_dspp blk, u32 dspp_sub_blk);
> >
> > /**
> > + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> > + * No effect on hardware
> > + * @ctx: ctl path ctx pointer
> > + * @blk: interface block index
> > + */
> > + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> > + enum dpu_dsc blk);
>
> Please align to the opening parenthesis.
I requested this change to a single tab specifically to match the rest
of the indentation of these callbacks. Perhaps we should submit a
followup patch realigning all of them at once (and fixing the doc
comments, and and and...).
- Marijn
> > +
> > + /**
> > * Write the value of the pending_flush_mask to hardware
> > * @ctx : ctl path ctx pointer
> > */
On 2023-05-17 16:22:37, Abhinav Kumar wrote:
<snip>
> >> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> >> .features = _features, \
> >> }
> >>
> >> +/*
> >> + * NOTE: Each display compression engine (DCE) contains dual hard
> >> + * slice DSC encoders so both share same base address but with
> >> + * its own different sub block address.
> >> + */
> >
> > I still think this comment is superfluous (and doesn't even apply
> > generically, see i.e. sc7280) and should best be kept exclusively in the
> > SoC-specific catalog files.
> >
> > - Marijn
> >
>
> sc7280 is the only exception as it has only one encoder. But, by and
> large, for all other chipsets this is true and hence kept here.
>
> The main reason for this comment is people should not get confused that
> how come two DSC encoders have the same base address.
And that's why the comment is already placed in the SoC-specific catalog
files where a duplicate base address is visible. It is not visible
here.
- Marijn
> >> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
> >> + {\
> >> + .name = _name, .id = _id, \
> >> + .base = _base, .len = _len, \
> >> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
> >> + .sblk = &_sblk, \
> >> + }
> >> +
> >> /*************************************************************
> >> * INTF sub blocks config
> >> *************************************************************/
> >> --
> >> 2.7.4
> >>
On 5/18/2023 12:05 AM, Marijn Suijten wrote:
> On 2023-05-17 16:22:37, Abhinav Kumar wrote:
> <snip>
>>>> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>>>> .features = _features, \
>>>> }
>>>>
>>>> +/*
>>>> + * NOTE: Each display compression engine (DCE) contains dual hard
>>>> + * slice DSC encoders so both share same base address but with
>>>> + * its own different sub block address.
>>>> + */
>>>
>>> I still think this comment is superfluous (and doesn't even apply
>>> generically, see i.e. sc7280) and should best be kept exclusively in the
>>> SoC-specific catalog files.
>>>
>>> - Marijn
>>>
>>
>> sc7280 is the only exception as it has only one encoder. But, by and
>> large, for all other chipsets this is true and hence kept here.
>>
>> The main reason for this comment is people should not get confused that
>> how come two DSC encoders have the same base address.
>
> And that's why the comment is already placed in the SoC-specific catalog
> files where a duplicate base address is visible. It is not visible
> here.
>
> - Marijn
>
ok, if kuogee pushes a v11, we can drop it.
>>>> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
>>>> + {\
>>>> + .name = _name, .id = _id, \
>>>> + .base = _base, .len = _len, \
>>>> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
>>>> + .sblk = &_sblk, \
>>>> + }
>>>> +
>>>> /*************************************************************
>>>> * INTF sub blocks config
>>>> *************************************************************/
>>>> --
>>>> 2.7.4
>>>>
On 5/17/2023 3:31 PM, Marijn Suijten wrote:
>
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
>> }
>> +
>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
>> + ctx->pending_dsc_flush_mask);
> Again, when do we reset this mask to 0? (v8 review)
can not find it.
let me add a separate patch to fix this.
>
>> +
>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>> }
>>
>> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
>> }
>>
>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
>> + enum dpu_dsc dsc_num)
>> +{
>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
>> +}
>> +
>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
>> enum dpu_dspp dspp, u32 dspp_sub_blk)
>> {
>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>>
>> - if (cfg->dsc)
>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
>> -
>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
>> mode_sel |= BIT(17);
>>
>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
>> if (cfg->merge_3d)
>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>> BIT(cfg->merge_3d - MERGE_3D_0));
>> - if (cfg->dsc) {
>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
> separate Fixes: patch to have this semantic change documented. (v8
> review)
That will be this patch. let me add Fixes at this patch
>
>> +
>> + if (cfg->dsc)
>> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
>> - }
>> }
>>
>> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
>> ops->update_pending_flush_merge_3d =
>> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
>> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
>> + ops->update_pending_flush_dsc =
>> + dpu_hw_ctl_update_pending_flush_dsc_v1;
>> } else {
>> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
>> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> index 6292002..d5f3ef8 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
>> enum dpu_dspp blk, u32 dspp_sub_blk);
>>
>> /**
>> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
>> + * No effect on hardware
>> + * @ctx: ctl path ctx pointer
>> + * @blk: interface block index
>> + */
>> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
>> + enum dpu_dsc blk);
>> +
>> + /**
>> * Write the value of the pending_flush_mask to hardware
>> * @ctx : ctl path ctx pointer
>> */
>> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
>> * @pending_flush_mask: storage for pending ctl_flush managed via ops
>> * @pending_intf_flush_mask: pending INTF flush
>> * @pending_wb_flush_mask: pending WB flush
> The above is all capitalized, so...:
>
>> + * @pending_merge_3d_flush_mask: pending merge_3d flush
> MERGE_3D?
>
>> + * @pending_dspp_flush_mask: pending dspp flush
> DSPP
>
>> + * @pending_dsc_flush_mask: pending dsc flush
> DSC
>
> - Marijn
>
>> * @ops: operation list
>> */
>> struct dpu_hw_ctl {
>> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
>> u32 pending_wb_flush_mask;
>> u32 pending_merge_3d_flush_mask;
>> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
>> + u32 pending_dsc_flush_mask;
>>
>> /* ops */
>> struct dpu_hw_ctl_ops ops;
>> --
>> 2.7.4
>>
On 19/05/2023 01:09, Kuogee Hsieh wrote:
>
> On 5/17/2023 3:31 PM, Marijn Suijten wrote:
>>
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> @@ -139,6 +139,11 @@ static inline void
>>> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
>>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
>>> }
>>> +
>>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
>>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
>>> + ctx->pending_dsc_flush_mask);
>> Again, when do we reset this mask to 0? (v8 review)
>
> can not find it.
>
> let me add a separate patch to fix this.
The pending_dsc_flush_mask was added in this patch, so the reset should
be a part of this patch too.
>
>>
>>> +
>>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>>> }
>>> @@ -285,6 +290,13 @@ static void
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
>>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
>>> }
>>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl
>>> *ctx,
>>> + enum dpu_dsc dsc_num)
>>> +{
>>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
>>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
>>> +}
>>> +
>>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
>>> *ctx,
>>> enum dpu_dspp dspp, u32 dspp_sub_blk)
>>> {
>>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
>>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>>> - if (cfg->dsc)
>>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
>>> -
>>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
>>> mode_sel |= BIT(17);
>>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if (cfg->merge_3d)
>>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>>> BIT(cfg->merge_3d - MERGE_3D_0));
>>> - if (cfg->dsc) {
>>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
>> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
>> separate Fixes: patch to have this semantic change documented. (v8
>> review)
> That will be this patch. let me add Fixes at this patch
_separate_ patch.
>>
>>> +
>>> + if (cfg->dsc)
>>> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
>>> - }
>>> }
>>> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>>> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops
>>> *ops,
>>> ops->update_pending_flush_merge_3d =
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
>>> ops->update_pending_flush_wb =
>>> dpu_hw_ctl_update_pending_flush_wb_v1;
>>> + ops->update_pending_flush_dsc =
>>> + dpu_hw_ctl_update_pending_flush_dsc_v1;
>>> } else {
>>> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
>>> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> index 6292002..d5f3ef8 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
>>> enum dpu_dspp blk, u32 dspp_sub_blk);
>>> /**
>>> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
>>> + * No effect on hardware
>>> + * @ctx: ctl path ctx pointer
>>> + * @blk: interface block index
>>> + */
>>> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
>>> + enum dpu_dsc blk);
>>> +
>>> + /**
>>> * Write the value of the pending_flush_mask to hardware
>>> * @ctx : ctl path ctx pointer
>>> */
>>> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
>>> * @pending_flush_mask: storage for pending ctl_flush managed via ops
>>> * @pending_intf_flush_mask: pending INTF flush
>>> * @pending_wb_flush_mask: pending WB flush
>> The above is all capitalized, so...:
>>
>>> + * @pending_merge_3d_flush_mask: pending merge_3d flush
>> MERGE_3D?
>>
>>> + * @pending_dspp_flush_mask: pending dspp flush
>> DSPP
>>
>>> + * @pending_dsc_flush_mask: pending dsc flush
>> DSC
>>
>> - Marijn
>>
>>> * @ops: operation list
>>> */
>>> struct dpu_hw_ctl {
>>> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
>>> u32 pending_wb_flush_mask;
>>> u32 pending_merge_3d_flush_mask;
>>> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
>>> + u32 pending_dsc_flush_mask;
>>> /* ops */
>>> struct dpu_hw_ctl_ops ops;
>>> --
>>> 2.7.4
>>>
--
With best wishes
Dmitry
On 2023-05-19 01:40:19, Dmitry Baryshkov wrote:
>
> On 19/05/2023 01:09, Kuogee Hsieh wrote:
> >
> > On 5/17/2023 3:31 PM, Marijn Suijten wrote:
> >>
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> >>> @@ -139,6 +139,11 @@ static inline void
> >>> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> >>> ????????????????? CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> >>> ????????????????? ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> >>> ????????? }
> >>> +
> >>> +??? if (ctx->pending_flush_mask & BIT(DSC_IDX))
> >>> +??????? DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> >>> +????????????????? ctx->pending_dsc_flush_mask);
> >> Again, when do we reset this mask to 0?? (v8 review)
> >
> > can not find it.
> >
> > let me add a separate? patch to fix this.
>
> The pending_dsc_flush_mask was added in this patch, so the reset should
> be a part of this patch too.
Yes, same patch.
Related question I asked in v8: only the global pending_flush_mask and
pending_dspp_flush_mask are reset in dpu_hw_ctl_clear_pending_flush().
Shall I send a patch to clear the other missing ones (e.g. merge_3d etc)
as well?
> >>> +
> >>> ????? DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> >>> ? }
> >>> @@ -285,6 +290,13 @@ static void
> >>> dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> >>> ????? ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> >>> ? }
> >>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl
> >>> *ctx,
> >>> +?????????????????????????? enum dpu_dsc dsc_num)
> >>> +{
> >>> +??? ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> >>> +??? ctx->pending_flush_mask |= BIT(DSC_IDX);
> >>> +}
> >>> +
> >>> ? static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
> >>> *ctx,
> >>> ????? enum dpu_dspp dspp, u32 dspp_sub_blk)
> >>> ? {
> >>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
> >>> dpu_hw_ctl *ctx,
> >>> ????? if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> >>> ????????? mode_sel = CTL_DEFAULT_GROUP_ID? << 28;
> >>> -??? if (cfg->dsc)
> >>> -??????? DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> >>> -
> >>> ????? if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> >>> ????????? mode_sel |= BIT(17);
> >>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
> >>> dpu_hw_ctl *ctx,
> >>> ????? if (cfg->merge_3d)
> >>> ????????? DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> >>> ??????????????????? BIT(cfg->merge_3d - MERGE_3D_0));
> >>> -??? if (cfg->dsc) {
> >>> -??????? DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> >> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
> >> separate Fixes: patch to have this semantic change documented.? (v8
> >> review)
> > That will be this patch. let me add Fixes at this patch
>
> _separate_ patch.
Separate patch, and documenting clearly what happens and why. Kuogee, I
can send this as well if it makes things more clear, since it doesn't
seem (from the patch description) that anyone noticed the
implication/bugfix in this change as a drive-by effect of porting
sde_hw_ctl_update_bitmask_dsc_v1() from downstream.
- Marijn
<snip>