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 for remaining chipsets in catalog
drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
Kuogee Hsieh (5):
drm/msm/dpu: add DPU_PINGPONG_DSC feature bit
drm/msm/dpu: add DPU_PINGPONG_DSC bits into PP_BLK and PP_BLK_TE
marcos
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
drivers/gpu/drm/msm/Makefile | 1 +
.../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 23 +-
.../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +-
.../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +-
.../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 35 +-
.../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +-
.../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 | 16 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 36 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 15 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 15 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 385 +++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 +
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
22 files changed, 652 insertions(+), 76 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
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
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 15 ++++++++-------
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, 11 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..47cb9f3 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,16 @@ 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,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
DPU < 7.0.0 requires the PINGPONG block to be involved during
DSC setting up. Since DPU < 7.0.0, enabling and starting the DSC
encoder engine moved to INTF with the help of the flush mechanism.
Add a DPU_PINGPONG_DSC feature bit to restrict the availability of
dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() on the
PINGPONG block to DPU < 7.0.0 hardware, as the registers are not
available [in the PINGPONG block] on DPU 7.0.0 and higher anymore.
Existing call-sites to these callbacks already skip calling into
them if the function pointer is NULL.
Changes in v4:
-- add more details commit text
Changes in v5:
-- reword commit text suggested by Marijn
-- delet comma
-- use test_bit()
Reported-by: Marijn Suijten <[email protected]>
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 ++++++
2 files changed, 9 insertions(+), 1 deletion(-)
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 6ee48f0..dc0a4da 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -144,7 +144,8 @@ enum {
* @DPU_PINGPONG_TE2 Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
- * @DPU_PINGPONG_DITHER, Dither blocks
+ * @DPU_PINGPONG_DITHER Dither blocks
+ * @DPU_PINGPONG_DSC PP ops functions required for DSC
* @DPU_PINGPONG_MAX
*/
enum {
@@ -153,6 +154,7 @@ enum {
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
DPU_PINGPONG_DITHER,
+ DPU_PINGPONG_DSC,
DPU_PINGPONG_MAX
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 79e4576..e7f47a4 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -295,6 +295,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
+ if (test_bit(DPU_PINGPONG_DSC, &features)) {
+ c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+ c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+ c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
+ }
+
if (test_bit(DPU_PINGPONG_DITHER, &features))
c->ops.setup_dither = dpu_hw_pp_setup_dither;
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate
pingpong ops functions are required to complete DSC data path setup if
this chipset has DSC hardware block presented. This patch add
DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC
hardware block presented.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
.../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++--------
.../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++--------
.../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++-----------
.../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++-----------
.../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++-----------
5 files changed, 52 insertions(+), 52 deletions(-)
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 521cfd5..ef92545 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
@@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
};
static const struct dpu_pingpong_cfg msm8998_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+ PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
- PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
- PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
- PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
index b109757..697fbd8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
@@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = {
};
static const struct dpu_pingpong_cfg sdm845_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+ PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
- PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
- PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
- PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
+ 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
index 30aff2b..cb117ca 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
@@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = {
};
static const struct dpu_pingpong_cfg sm8150_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+ PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
-1),
- PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
-1),
- PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
-1),
- PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
-1),
- PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
+ PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
-1),
- PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
+ PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
-1),
};
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 fec1665..27eda6a 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
@@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = {
};
static const struct dpu_pingpong_cfg sc8180x_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+ PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
-1),
- PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
-1),
- PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
-1),
- PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
-1),
- PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
+ PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
-1),
- PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
+ PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
-1),
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
index 37716b8..70fdd4d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
@@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = {
};
static const struct dpu_pingpong_cfg sm8250_pp[] = {
- PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
+ PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
-1),
- PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
+ PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
-1),
- PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
+ PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
-1),
- PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
+ PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
-1),
- PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
+ PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
-1),
- PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
- DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
+ PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
+ MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
-1),
};
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Current DSC flush update is piggyback inside dpu_hw_ctl_intf_cfg_v1().
This patch separates 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.
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++++++++++
3 files changed, 38 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..94b805b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1834,12 +1834,18 @@ 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_encoder_virt *dpu_enc,
+ struct dpu_hw_dsc *hw_dsc,
struct dpu_hw_pingpong *hw_pp,
struct drm_dsc_config *dsc,
u32 common_mode,
u32 initial_lines)
{
+ struct dpu_encoder_phys *cur_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl;
+
+ ctl = cur_master->hw_ctl;
+
if (hw_dsc->ops.dsc_config)
hw_dsc->ops.dsc_config(hw_dsc, dsc, common_mode, initial_lines);
@@ -1854,6 +1860,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,
@@ -1898,7 +1907,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(dpu_enc, 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..832a6a7 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,8 @@ 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 +637,9 @@ 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..5762752 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
*/
@@ -245,6 +254,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;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
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 hard slice 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[]
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
.../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..c4c93c8 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", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_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 = sm8350_dsc,
+ .dsc_count = ARRAY_SIZE(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..42c66fe 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", DSC_0, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_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..508479f 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", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_1),
+ DSC_BLK_1_2("dce_2", DSC_4, 0x82000, 0x100, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_2", DSC_5, 0x82000, 0x100, 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 = sc8280xp_dsc,
+ .dsc_count = ARRAY_SIZE(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..741d03f 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", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_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 = sm8450_dsc,
+ .dsc_count = ARRAY_SIZE(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..3ee6dc8 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", DSC_0, 0x80000, 0x100, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0", DSC_1, 0x80000, 0x100, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_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 = sm8550_dsc,
+ .dsc_count = ARRAY_SIZE(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 82b58c6..7944481 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
*************************************************************/
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
From: Abhinav Kumar <[email protected]>
There are some platforms has DSC blocks but it is not declared at catalog.
For completeness, this patch adds DSC blocks for platforms which missed
them.
Signed-off-by: Abhinav Kumar <[email protected]>
Reviewed-by: Dmitry Baryshkov <[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),
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Add support for DSC 1.2 by providing the necessary hooks to program
the DPU DSC 1.2 encoder.
Changes in v3:
-- fixed kernel test rebot report that "__iomem *off" is declared but not
used at dpu_hw_dsc_config_1_2()
-- unrolling thresh loops
Changes in v4:
-- delete DPU_DSC_HW_REV_1_1
-- delete off and used real register name directly
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 32 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 385 +++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
5 files changed, 435 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index b814fc8..b9af5e4 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
disp/dpu1/dpu_hw_dsc.o \
+ disp/dpu1/dpu_hw_dsc_1_2.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
disp/dpu1/dpu_hw_lm.o \
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 dc0a4da..4eda2cc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
*/
@@ -244,12 +244,18 @@ enum {
};
/**
- * DSC features
+ * DSC sub-blocks/features
* @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
* the pixel output from this DSC.
+ * @DPU_DSC_HW_REV_1_2 DSC block supports dsc 1.1 and 1.2
+ * @DPU_DSC_NATIVE_422_EN Supports native422 and native420 encoding
+ * @DPU_DSC_MAX
*/
enum {
DPU_DSC_OUTPUT_CTRL = 0x1,
+ DPU_DSC_HW_REV_1_2,
+ DPU_DSC_NATIVE_422_EN,
+ DPU_DSC_MAX
};
/**
@@ -306,6 +312,14 @@ struct dpu_pp_blk {
};
/**
+ * struct dpu_dsc_blk - DSC Encoder sub-blk information
+ * @info: HW register and features supported by this sub-blk
+ */
+struct dpu_dsc_blk {
+ DPU_HW_SUBBLK_INFO;
+};
+
+/**
* enum dpu_qos_lut_usage - define QoS LUT use cases
*/
enum dpu_qos_lut_usage {
@@ -452,6 +466,17 @@ struct dpu_pingpong_sub_blks {
};
/**
+ * struct dpu_dsc_sub_blks - DSC sub-blks
+ * @enc: DSC encoder sub block
+ * @ctl: DSC controller sub block
+ *
+ */
+struct dpu_dsc_sub_blks {
+ struct dpu_dsc_blk enc;
+ struct dpu_dsc_blk ctl;
+};
+
+/**
* dpu_clk_ctrl_type - Defines top level clock control signals
*/
enum dpu_clk_ctrl_type {
@@ -605,10 +630,13 @@ struct dpu_merge_3d_cfg {
* struct dpu_dsc_cfg - information of DSC blocks
* @id enum identifying this block
* @base register offset of this block
+ * @len: length of hardware block
* @features bit mask identifying sub-blocks/features
+ * @sblk sub-blocks information
*/
struct dpu_dsc_cfg {
DPU_HW_BLK_INFO;
+ const struct dpu_dsc_sub_blks *sblk;
};
/**
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 138080a..bdff74d 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -1,5 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2020-2022, Linaro Limited */
+/*
+ * Copyright (c) 2020-2022, Linaro Limited
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
#ifndef _DPU_HW_DSC_H
#define _DPU_HW_DSC_H
@@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg,
void __iomem *addr);
/**
+ * dpu_hw_dsc_init_1_2 - initializes the v1.2 DSC hw driver block
+ * @cfg: DSC catalog entry for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
+ void __iomem *addr);
+
+/**
* dpu_hw_dsc_destroy - destroys dsc driver context
* @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
new file mode 100644
index 00000000..0c77c85
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include <drm/display/drm_dsc_helper.h>
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_CMN_MAIN_CNF 0x00
+
+/* DPU_DSC_ENC register offsets */
+#define ENC_DF_CTRL 0x00
+#define ENC_GENERAL_STATUS 0x04
+#define ENC_HSLICE_STATUS 0x08
+#define ENC_OUT_STATUS 0x0C
+#define ENC_INT_STAT 0x10
+#define ENC_INT_CLR 0x14
+#define ENC_INT_MASK 0x18
+#define DSC_MAIN_CONF 0x30
+#define DSC_PICTURE_SIZE 0x34
+#define DSC_SLICE_SIZE 0x38
+#define DSC_MISC_SIZE 0x3C
+#define DSC_HRD_DELAYS 0x40
+#define DSC_RC_SCALE 0x44
+#define DSC_RC_SCALE_INC_DEC 0x48
+#define DSC_RC_OFFSETS_1 0x4C
+#define DSC_RC_OFFSETS_2 0x50
+#define DSC_RC_OFFSETS_3 0x54
+#define DSC_RC_OFFSETS_4 0x58
+#define DSC_FLATNESS_QP 0x5C
+#define DSC_RC_MODEL_SIZE 0x60
+#define DSC_RC_CONFIG 0x64
+#define DSC_RC_BUF_THRESH_0 0x68
+#define DSC_RC_BUF_THRESH_1 0x6C
+#define DSC_RC_BUF_THRESH_2 0x70
+#define DSC_RC_BUF_THRESH_3 0x74
+#define DSC_RC_MIN_QP_0 0x78
+#define DSC_RC_MIN_QP_1 0x7C
+#define DSC_RC_MIN_QP_2 0x80
+#define DSC_RC_MAX_QP_0 0x84
+#define DSC_RC_MAX_QP_1 0x88
+#define DSC_RC_MAX_QP_2 0x8C
+#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
+#define DSC_RC_RANGE_BPG_OFFSETS_1 0x94
+#define DSC_RC_RANGE_BPG_OFFSETS_2 0x98
+
+/* DPU_DSC_CTL register offsets */
+#define DSC_CTL 0x00
+#define DSC_CFG 0x04
+#define DSC_DATA_IN_SWAP 0x08
+#define DSC_CLK_CTRL 0x0C
+
+static inline int _dsc_calc_ob_max_addr(struct dpu_hw_dsc *hw_dsc, int num_ss)
+{
+ int max_addr = 2400 / num_ss;
+
+ if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN))
+ max_addr /= 2;
+
+ return max_addr - 1;
+};
+
+static inline void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ u32 offset;
+
+ if (!hw_dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+ offset = hw_dsc->caps->sblk->ctl.base;
+ DPU_REG_WRITE(hw, offset + DSC_CFG, 0);
+
+ offset = hw_dsc->caps->sblk->enc.base;
+ DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, 0);
+ DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, 0);
+}
+
+static inline void dpu_hw_dsc_config_1_2(struct dpu_hw_dsc *hw_dsc,
+ struct drm_dsc_config *dsc,
+ u32 mode,
+ u32 initial_lines)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ u32 offset;
+ u32 data = 0;
+ u32 det_thresh_flatness;
+ u32 num_active_ss_per_enc;
+ u32 bpp;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+
+ offset = hw_dsc->caps->sblk->enc.base;
+
+ if (mode & DSC_MODE_SPLIT_PANEL)
+ data |= BIT(0);
+
+ if (mode & DSC_MODE_MULTIPLEX)
+ data |= BIT(1);
+
+ num_active_ss_per_enc = dsc->slice_count;
+ if (mode & DSC_MODE_MULTIPLEX)
+ num_active_ss_per_enc = dsc->slice_count >> 1;
+
+ data |= (num_active_ss_per_enc & 0x3) << 7;
+
+ DPU_REG_WRITE(hw, DSC_CMN_MAIN_CNF, data);
+
+ data = (initial_lines & 0xff);
+
+ if (mode & DSC_MODE_VIDEO)
+ data |= BIT(9);
+
+ data |= (_dsc_calc_ob_max_addr(hw_dsc, num_active_ss_per_enc) << 18);
+
+ DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, data);
+
+ data = (dsc->dsc_version_minor & 0xf) << 28;
+ if (dsc->dsc_version_minor == 0x2) {
+ if (dsc->native_422)
+ data |= BIT(22);
+ if (dsc->native_420)
+ data |= BIT(21);
+ }
+
+ bpp = dsc->bits_per_pixel;
+ /* as per hw requirement bpp should be programmed
+ * twice the actual value in case of 420 or 422 encoding
+ */
+ if (dsc->native_422 || dsc->native_420)
+ bpp = 2 * bpp;
+ data |= (dsc->block_pred_enable ? 1 : 0) << 20;
+ data |= bpp << 10;
+ data |= (dsc->line_buf_depth & 0xf) << 6;
+ data |= dsc->convert_rgb << 4;
+ data |= dsc->bits_per_component & 0xf;
+
+ DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, data);
+
+ data = (dsc->pic_width & 0xffff) |
+ ((dsc->pic_height & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_PICTURE_SIZE, data);
+
+ data = (dsc->slice_width & 0xffff) |
+ ((dsc->slice_height & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_SLICE_SIZE, data);
+
+ DPU_REG_WRITE(hw, offset + DSC_MISC_SIZE,
+ (dsc->slice_chunk_size) & 0xffff);
+
+ data = (dsc->initial_xmit_delay & 0xffff) |
+ ((dsc->initial_dec_delay & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_HRD_DELAYS, data);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_SCALE,
+ dsc->initial_scale_value & 0x3f);
+
+ data = (dsc->scale_increment_interval & 0xffff) |
+ ((dsc->scale_decrement_interval & 0x7ff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_SCALE_INC_DEC, data);
+
+ data = (dsc->first_line_bpg_offset & 0x1f) |
+ ((dsc->second_line_bpg_offset & 0x1f) << 5);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_1, data);
+
+ data = (dsc->nfl_bpg_offset & 0xffff) |
+ ((dsc->slice_bpg_offset & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_2, data);
+
+ data = (dsc->initial_offset & 0xffff) |
+ ((dsc->final_offset & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_3, data);
+
+ data = (dsc->nsl_bpg_offset & 0xffff) |
+ ((dsc->second_line_offset_adj & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_4, data);
+
+ data = (dsc->flatness_min_qp & 0x1f);
+ data |= (dsc->flatness_max_qp & 0x1f) << 5;
+
+ det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
+ data |= (det_thresh_flatness & 0xff) << 10;
+
+ DPU_REG_WRITE(hw, offset + DSC_FLATNESS_QP, data);
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_MODEL_SIZE,
+ (dsc->rc_model_size) & 0xffff);
+
+ data = dsc->rc_edge_factor & 0xf;
+ data |= (dsc->rc_quant_incr_limit0 & 0x1f) << 8;
+ data |= (dsc->rc_quant_incr_limit1 & 0x1f) << 13;
+ data |= (dsc->rc_tgt_offset_high & 0xf) << 20;
+ data |= (dsc->rc_tgt_offset_low & 0xf) << 24;
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_CONFIG, data);
+
+ /* program the dsc wrapper */
+ offset = hw_dsc->caps->sblk->ctl.base;
+
+ data = BIT(0); /* encoder enable */
+ if (dsc->native_422)
+ data |= BIT(8);
+ else if (dsc->native_420)
+ data |= BIT(9);
+ if (!dsc->convert_rgb)
+ data |= BIT(10);
+ if (dsc->bits_per_component == 8)
+ data |= BIT(11);
+ if (mode & DSC_MODE_SPLIT_PANEL)
+ data |= BIT(12);
+ if (mode & DSC_MODE_MULTIPLEX)
+ data |= BIT(13);
+ if (!(mode & DSC_MODE_VIDEO))
+ data |= BIT(17);
+
+ DPU_REG_WRITE(hw, offset + DSC_CFG, data);
+}
+
+static inline void dpu_hw_dsc_config_thresh_1_2(struct dpu_hw_dsc *hw_dsc,
+ struct drm_dsc_config *dsc)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ u32 offset;
+ struct drm_dsc_rc_range_parameters *rc;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ offset = hw_dsc->caps->sblk->enc.base;
+
+ hw = &hw_dsc->hw;
+
+ rc = dsc->rc_range_params;
+
+ /*
+ * With BUF_THRESH -- 14 in total
+ * each register contains 4 thresh values with the last register
+ * containing only 2 thresh values
+ */
+ DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0,
+ (dsc->rc_buf_thresh[0] << 0) |
+ (dsc->rc_buf_thresh[1] << 8) |
+ (dsc->rc_buf_thresh[2] << 16) |
+ (dsc->rc_buf_thresh[3] << 24));
+ DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_1,
+ (dsc->rc_buf_thresh[4] << 0) |
+ (dsc->rc_buf_thresh[5] << 8) |
+ (dsc->rc_buf_thresh[6] << 16) |
+ (dsc->rc_buf_thresh[7] << 24));
+ DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_2,
+ (dsc->rc_buf_thresh[8] << 0) |
+ (dsc->rc_buf_thresh[9] << 8) |
+ (dsc->rc_buf_thresh[10] << 16) |
+ (dsc->rc_buf_thresh[11] << 24));
+ DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_3,
+ (dsc->rc_buf_thresh[12] << 0) |
+ (dsc->rc_buf_thresh[13] << 8));
+
+ /*
+ * with min/max_QP -- 5 bits each
+ * each register contains 5 min_qp or max_qp for total of 15
+ *
+ * With BPG_OFFSET -- 6 bits each
+ * each register contains 5 BPG_offset for total of 15
+ */
+ DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0,
+ (rc[0].range_min_qp << 0) |
+ (rc[1].range_min_qp << 5) |
+ (rc[2].range_min_qp << 10) |
+ (rc[3].range_min_qp << 15) |
+ (rc[4].range_min_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0,
+ (rc[0].range_max_qp << 0) |
+ (rc[1].range_max_qp << 5) |
+ (rc[2].range_max_qp << 10) |
+ (rc[3].range_max_qp << 15) |
+ (rc[4].range_max_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_RANGE_BPG_OFFSETS_0,
+ (rc[0].range_bpg_offset << 0) |
+ (rc[1].range_bpg_offset << 6) |
+ (rc[2].range_bpg_offset << 12) |
+ (rc[3].range_bpg_offset << 18) |
+ (rc[4].range_bpg_offset << 24));
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_1,
+ (rc[5].range_min_qp << 0) |
+ (rc[6].range_min_qp << 5) |
+ (rc[7].range_min_qp << 10) |
+ (rc[8].range_min_qp << 15) |
+ (rc[9].range_min_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_1,
+ (rc[5].range_max_qp << 0) |
+ (rc[6].range_max_qp << 5) |
+ (rc[7].range_max_qp << 10) |
+ (rc[8].range_max_qp << 15) |
+ (rc[9].range_max_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_RANGE_BPG_OFFSETS_1,
+ (rc[5].range_bpg_offset << 0) |
+ (rc[6].range_bpg_offset << 6) |
+ (rc[7].range_bpg_offset << 12) |
+ (rc[8].range_bpg_offset << 18) |
+ (rc[9].range_bpg_offset << 24));
+
+ DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_2,
+ (rc[10].range_min_qp << 0) |
+ (rc[11].range_min_qp << 5) |
+ (rc[12].range_min_qp << 10) |
+ (rc[13].range_min_qp << 15) |
+ (rc[14].range_min_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_2,
+ (rc[10].range_max_qp << 0) |
+ (rc[11].range_max_qp << 5) |
+ (rc[12].range_max_qp << 10) |
+ (rc[13].range_max_qp << 15) |
+ (rc[14].range_max_qp << 20));
+ DPU_REG_WRITE(hw, offset + DSC_RC_RANGE_BPG_OFFSETS_2,
+ (rc[10].range_bpg_offset << 0) |
+ (rc[11].range_bpg_offset << 6) |
+ (rc[12].range_bpg_offset << 12) |
+ (rc[13].range_bpg_offset << 18) |
+ (rc[14].range_bpg_offset << 24));
+}
+
+static inline void dpu_hw_dsc_bind_pingpong_blk_1_2(
+ struct dpu_hw_dsc *hw_dsc,
+ const enum dpu_pingpong pp)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ int offset;
+ int mux_cfg = 0xf; /* Disabled */
+
+ offset = hw_dsc->caps->sblk->ctl.base;
+
+ hw = &hw_dsc->hw;
+ if (pp)
+ mux_cfg = (pp - PINGPONG_0) & 0x7;
+
+ DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
+}
+
+static void _setup_dcs_ops_1_2(struct dpu_hw_dsc_ops *ops,
+ const unsigned long features)
+{
+ ops->dsc_disable = dpu_hw_dsc_disable_1_2;
+ ops->dsc_config = dpu_hw_dsc_config_1_2;
+ ops->dsc_config_thresh = dpu_hw_dsc_config_thresh_1_2;
+ ops->dsc_bind_pingpong_blk = dpu_hw_dsc_bind_pingpong_blk_1_2;
+}
+
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
+ void __iomem *addr)
+{
+ struct dpu_hw_dsc *c;
+
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ c->hw.blk_addr = addr + cfg->base;
+ c->hw.log_mask = DPU_DBG_MASK_DSC;
+
+ c->idx = cfg->id;
+ c->caps = cfg;
+ _setup_dcs_ops_1_2(&c->ops, c->caps->features);
+
+ return c;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f0fc704..502dd60 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt) "[drm:%s] " fmt, __func__
@@ -246,7 +247,11 @@ int dpu_rm_init(struct dpu_rm *rm,
struct dpu_hw_dsc *hw;
const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
- hw = dpu_hw_dsc_init(dsc, mmio);
+ if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
+ hw = dpu_hw_dsc_init_1_2(dsc, mmio);
+ else
+ hw = dpu_hw_dsc_init(dsc, mmio);
+
if (IS_ERR_OR_NULL(hw)) {
rc = PTR_ERR(hw);
DPU_ERROR("failed dsc object creation: err %d\n", rc);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
On 2023-05-04 09:46:42, Kuogee Hsieh wrote:
> 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
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
Thanks for bearing with all the comments thus far!
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 | 15 ++++++++-------
> 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, 11 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..47cb9f3 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,16 @@ 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,
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On 2023-05-04 09:46:40, Kuogee Hsieh wrote:
> DPU < 7.0.0 requires the PINGPONG block to be involved during
Nit: double space.
> DSC setting up. Since DPU < 7.0.0, enabling and starting the DSC
That should be >=.
> encoder engine moved to INTF with the help of the flush mechanism.
> Add a DPU_PINGPONG_DSC feature bit to restrict the availability of
> dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() on the
> PINGPONG block to DPU < 7.0.0 hardware, as the registers are not
> available [in the PINGPONG block] on DPU 7.0.0 and higher anymore.
> Existing call-sites to these callbacks already skip calling into
> them if the function pointer is NULL.
>
> Changes in v4:
> -- add more details commit text
>
> Changes in v5:
> -- reword commit text suggested by Marijn
> -- delet comma
> -- use test_bit()
>
> Reported-by: Marijn Suijten <[email protected]>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
This patch isn't strictly bisectable, but I'll explain in patch 3/7.
After addressing the '<' -> '>=' typo above, this is:
Reviewed-by: Marijn Suijten <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 ++++++
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> 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 6ee48f0..dc0a4da 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -144,7 +144,8 @@ enum {
> * @DPU_PINGPONG_TE2 Additional tear check block for split pipes
> * @DPU_PINGPONG_SPLIT PP block supports split fifo
> * @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
> - * @DPU_PINGPONG_DITHER, Dither blocks
> + * @DPU_PINGPONG_DITHER Dither blocks
> + * @DPU_PINGPONG_DSC PP ops functions required for DSC
> * @DPU_PINGPONG_MAX
> */
> enum {
> @@ -153,6 +154,7 @@ enum {
> DPU_PINGPONG_SPLIT,
> DPU_PINGPONG_SLAVE,
> DPU_PINGPONG_DITHER,
> + DPU_PINGPONG_DSC,
> DPU_PINGPONG_MAX
> };
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> index 79e4576..e7f47a4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> @@ -295,6 +295,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
> c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
>
> + if (test_bit(DPU_PINGPONG_DSC, &features)) {
> + c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
> + c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> + c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
> + }
> +
> if (test_bit(DPU_PINGPONG_DITHER, &features))
> c->ops.setup_dither = dpu_hw_pp_setup_dither;
> };
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
PP_BLK_TE is no longer there.
marcos -> macros.
On 2023-05-04 09:46:41, Kuogee Hsieh wrote:
> At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate
I may have not made this clear, but the comments on patch 2/7
(introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify
DPU 7.0.0 exactly in favour of "legacy", which has no definition at all
and changes over time.
> pingpong ops functions are required to complete DSC data path setup if
> this chipset has DSC hardware block presented. This patch add
> DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC
> hardware block presented.
Strictly speaking this patch together with 2/7 is not bisectable, as 2/7
first disables the callbacks for _all_ hardware and then this patch adds
it back by adding the flag to all DPU < 7.0.0 catalog descriptions.
To solve that, as we do in other DPU patch-series, just squash this
patch into 2/7. That way you also don't have to spend extra time
rewording this commit message either to match the improvements we made
in 2/7 (for example, you mention that "ops functions are required to
complete DSC data path setup", but those were already available before
2/7, despite sounding as if this is a new thing that was previously
missing entirely).
But please wait at least a couple days before sending v6. I only have a
few hours every day/week but would appreciate to review and test all the
other patches.
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++--------
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++--------
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++-----------
> .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++-----------
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++-----------
> 5 files changed, 52 insertions(+), 52 deletions(-)
>
> 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 521cfd5..ef92545 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
> @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
> };
>
> static const struct dpu_pingpong_cfg msm8998_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
This should be added to the MASK (add new #define's where necessary).
- Marijn
> + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> };
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> index b109757..697fbd8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> @@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = {
> };
>
> static const struct dpu_pingpong_cfg sdm845_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> };
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> index 30aff2b..cb117ca 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> @@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = {
> };
>
> static const struct dpu_pingpong_cfg sm8150_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> -1),
> - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> -1),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> -1),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> -1),
> - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> -1),
> - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> -1),
> };
>
> 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 fec1665..27eda6a 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
> @@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = {
> };
>
> static const struct dpu_pingpong_cfg sc8180x_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> -1),
> - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> -1),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> -1),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> -1),
> - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> -1),
> - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> -1),
> };
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> index 37716b8..70fdd4d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> @@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = {
> };
>
> static const struct dpu_pingpong_cfg sm8250_pp[] = {
> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> -1),
> - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> -1),
> - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> -1),
> - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> -1),
> - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> -1),
> - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> -1),
> };
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
On Thu, 4 May 2023 at 20:49, Marijn Suijten
<[email protected]> wrote:
>
> PP_BLK_TE is no longer there.
>
> marcos -> macros.
>
> On 2023-05-04 09:46:41, Kuogee Hsieh wrote:
> > At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate
>
> I may have not made this clear, but the comments on patch 2/7
> (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify
> DPU 7.0.0 exactly in favour of "legacy", which has no definition at all
> and changes over time.
>
> > pingpong ops functions are required to complete DSC data path setup if
> > this chipset has DSC hardware block presented. This patch add
> > DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC
> > hardware block presented.
>
> Strictly speaking this patch together with 2/7 is not bisectable, as 2/7
> first disables the callbacks for _all_ hardware and then this patch adds
> it back by adding the flag to all DPU < 7.0.0 catalog descriptions.
I asked to split these into two patches, but I see your point and
partially agree with it. However if we mix the catalog changes with
functional changes, it is too easy to overlook or misjudge the
functional changes.
As you are correct about bisectability, I'd probably suggest either
having three patches (define flag, update catalog, handle flag in the
driver) or squashing first two patches to have two patches (add flag +
catalog, separate functional changes).
>
> To solve that, as we do in other DPU patch-series, just squash this
> patch into 2/7. That way you also don't have to spend extra time
> rewording this commit message either to match the improvements we made
> in 2/7 (for example, you mention that "ops functions are required to
> complete DSC data path setup", but those were already available before
> 2/7, despite sounding as if this is a new thing that was previously
> missing entirely).
>
> But please wait at least a couple days before sending v6. I only have a
> few hours every day/week but would appreciate to review and test all the
> other patches.
>
> > Signed-off-by: Kuogee Hsieh <[email protected]>
> > ---
> > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++--------
> > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++--------
> > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++-----------
> > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++-----------
> > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++-----------
> > 5 files changed, 52 insertions(+), 52 deletions(-)
> >
> > 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 521cfd5..ef92545 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
> > @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
> > };
> >
> > static const struct dpu_pingpong_cfg msm8998_pp[] = {
> > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
>
> This should be added to the MASK (add new #define's where necessary).
>
> - Marijn
>
> > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> > };
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> > index b109757..697fbd8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h
> > @@ -110,17 +110,17 @@ static const struct dpu_lm_cfg sdm845_lm[] = {
> > };
> >
> > static const struct dpu_pingpong_cfg sdm845_pp[] = {
> > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 12)),
> > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk_te, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 13)),
> > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 14)),
> > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK, 0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SDM845_MASK|BIT(DPU_PINGPONG_DSC),
> > + 0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> > };
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> > index 30aff2b..cb117ca 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h
> > @@ -128,23 +128,23 @@ static const struct dpu_dspp_cfg sm8150_dspp[] = {
> > };
> >
> > static const struct dpu_pingpong_cfg sm8150_pp[] = {
> > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > -1),
> > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > -1),
> > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > -1),
> > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > -1),
> > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > -1),
> > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > -1),
> > };
> >
> > 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 fec1665..27eda6a 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
> > @@ -116,23 +116,23 @@ static const struct dpu_lm_cfg sc8180x_lm[] = {
> > };
> >
> > static const struct dpu_pingpong_cfg sc8180x_pp[] = {
> > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > -1),
> > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > -1),
> > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > -1),
> > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > -1),
> > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > -1),
> > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > -1),
> > };
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> > index 37716b8..70fdd4d 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> > @@ -129,23 +129,23 @@ static const struct dpu_dspp_cfg sm8250_dspp[] = {
> > };
> >
> > static const struct dpu_pingpong_cfg sm8250_pp[] = {
> > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > -1),
> > - PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK, MERGE_3D_0, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > + PP_BLK("pingpong_1", PINGPONG_1, 0x70800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_0, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 9),
> > -1),
> > - PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > + PP_BLK("pingpong_2", PINGPONG_2, 0x71000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 10),
> > -1),
> > - PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK, MERGE_3D_1, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > + PP_BLK("pingpong_3", PINGPONG_3, 0x71800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_1, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 11),
> > -1),
> > - PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > + PP_BLK("pingpong_4", PINGPONG_4, 0x72000, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 30),
> > -1),
> > - PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK, MERGE_3D_2, sdm845_pp_sblk,
> > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > + PP_BLK("pingpong_5", PINGPONG_5, 0x72800, PINGPONG_SM8150_MASK|BIT(DPU_PINGPONG_DSC),
> > + MERGE_3D_2, sdm845_pp_sblk, DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 31),
> > -1),
> > };
> >
> > --
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> > a Linux Foundation Collaborative Project
> >
--
With best wishes
Dmitry
On 5/4/2023 11:23 AM, Marijn Suijten wrote:
> On 2023-05-04 20:53:33, Dmitry Baryshkov wrote:
>> On Thu, 4 May 2023 at 20:49, Marijn Suijten
>> <[email protected]> wrote:
>>>
>>> PP_BLK_TE is no longer there.
>>>
>>> marcos -> macros.
>>>
>>> On 2023-05-04 09:46:41, Kuogee Hsieh wrote:
>>>> At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate
>>>
>>> I may have not made this clear, but the comments on patch 2/7
>>> (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify
>>> DPU 7.0.0 exactly in favour of "legacy", which has no definition at all
>>> and changes over time.
>>>
>>>> pingpong ops functions are required to complete DSC data path setup if
>>>> this chipset has DSC hardware block presented. This patch add
>>>> DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC
>>>> hardware block presented.
>>>
>>> Strictly speaking this patch together with 2/7 is not bisectable, as 2/7
>>> first disables the callbacks for _all_ hardware and then this patch adds
>>> it back by adding the flag to all DPU < 7.0.0 catalog descriptions.
>>
>> I asked to split these into two patches, but I see your point and
>> partially agree with it. However if we mix the catalog changes with
>> functional changes, it is too easy to overlook or misjudge the
>> functional changes.
>
> I did the same in the INTF TE series for patches that have very little
> and/or very obvious functional changes: exactly this combination of
> guarding a few callbacks behind a feature bit, and setting that feature
> bit on a few specific catalog entries.
>
>> As you are correct about bisectability, I'd probably suggest either
>> having three patches (define flag, update catalog, handle flag in the
>> driver) or squashing first two patches to have two patches (add flag +
>> catalog, separate functional changes).
>
> Sure, if you really prefer a split I'd go for two patches:
> 1. Add the flag to the enum and catalog;
> 2. Add the ops guard (functional change).
>
> Then don't forget to reword the commit message, following the guidelines
> below and the suggestion for 2/7.
>
> - Marijn
Plan sounds good to me.
Marijn, we will wait for a couple of days to post the next rev but would
be hard more than that as we need to pick up other things which are
pending on top of this. Hence would appreciate if you can finish reviews
by then.
>
>>> To solve that, as we do in other DPU patch-series, just squash this
>>> patch into 2/7. That way you also don't have to spend extra time
>>> rewording this commit message either to match the improvements we made
>>> in 2/7 (for example, you mention that "ops functions are required to
>>> complete DSC data path setup", but those were already available before
>>> 2/7, despite sounding as if this is a new thing that was previously
>>> missing entirely).
>>>
>>> But please wait at least a couple days before sending v6. I only have a
>>> few hours every day/week but would appreciate to review and test all the
>>> other patches.
>>>
>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>> ---
>>>> .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++--------
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++--------
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++-----------
>>>> .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++-----------
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++-----------
>>>> 5 files changed, 52 insertions(+), 52 deletions(-)
>>>>
>>>> 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 521cfd5..ef92545 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
>>>> @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
>>>> };
>>>>
>>>> static const struct dpu_pingpong_cfg msm8998_pp[] = {
>>>> - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
>>>> - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
>>>> + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
>>>
>>> This should be added to the MASK (add new #define's where necessary).
>>>
>>> - Marijn
>
> <snip>
On 2023-05-04 20:53:33, Dmitry Baryshkov wrote:
> On Thu, 4 May 2023 at 20:49, Marijn Suijten
> <[email protected]> wrote:
> >
> > PP_BLK_TE is no longer there.
> >
> > marcos -> macros.
> >
> > On 2023-05-04 09:46:41, Kuogee Hsieh wrote:
> > > At legacy chipsets, it required DPU_PINGPONG_DSC bit be set to indicate
> >
> > I may have not made this clear, but the comments on patch 2/7
> > (introducing the DPU_PINGPONG_DSC bit) also apply to this patch: clarify
> > DPU 7.0.0 exactly in favour of "legacy", which has no definition at all
> > and changes over time.
> >
> > > pingpong ops functions are required to complete DSC data path setup if
> > > this chipset has DSC hardware block presented. This patch add
> > > DPU_PINGPONG_DSC bit to both PP_BLK and PP_BLK_TE marcos if it has DSC
> > > hardware block presented.
> >
> > Strictly speaking this patch together with 2/7 is not bisectable, as 2/7
> > first disables the callbacks for _all_ hardware and then this patch adds
> > it back by adding the flag to all DPU < 7.0.0 catalog descriptions.
>
> I asked to split these into two patches, but I see your point and
> partially agree with it. However if we mix the catalog changes with
> functional changes, it is too easy to overlook or misjudge the
> functional changes.
I did the same in the INTF TE series for patches that have very little
and/or very obvious functional changes: exactly this combination of
guarding a few callbacks behind a feature bit, and setting that feature
bit on a few specific catalog entries.
> As you are correct about bisectability, I'd probably suggest either
> having three patches (define flag, update catalog, handle flag in the
> driver) or squashing first two patches to have two patches (add flag +
> catalog, separate functional changes).
Sure, if you really prefer a split I'd go for two patches:
1. Add the flag to the enum and catalog;
2. Add the ops guard (functional change).
Then don't forget to reword the commit message, following the guidelines
below and the suggestion for 2/7.
- Marijn
> > To solve that, as we do in other DPU patch-series, just squash this
> > patch into 2/7. That way you also don't have to spend extra time
> > rewording this commit message either to match the improvements we made
> > in 2/7 (for example, you mention that "ops functions are required to
> > complete DSC data path setup", but those were already available before
> > 2/7, despite sounding as if this is a new thing that was previously
> > missing entirely).
> >
> > But please wait at least a couple days before sending v6. I only have a
> > few hours every day/week but would appreciate to review and test all the
> > other patches.
> >
> > > Signed-off-by: Kuogee Hsieh <[email protected]>
> > > ---
> > > .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 16 +++++++--------
> > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_4_0_sdm845.h | 16 +++++++--------
> > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_5_0_sm8150.h | 24 +++++++++++-----------
> > > .../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 24 +++++++++++-----------
> > > .../gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h | 24 +++++++++++-----------
> > > 5 files changed, 52 insertions(+), 52 deletions(-)
> > >
> > > 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 521cfd5..ef92545 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
> > > @@ -112,17 +112,17 @@ static const struct dpu_lm_cfg msm8998_lm[] = {
> > > };
> > >
> > > static const struct dpu_pingpong_cfg msm8998_pp[] = {
> > > - PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK, 0, sdm845_pp_sblk_te,
> > > - DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 8),
> > > + PP_BLK("pingpong_0", PINGPONG_0, 0x70000, PINGPONG_SDM845_TE2_MASK|BIT(DPU_PINGPONG_DSC),
> >
> > This should be added to the MASK (add new #define's where necessary).
> >
> > - Marijn
<snip>
On 2023-05-04 11:25:44, Abhinav Kumar wrote:
<snip>
> > Sure, if you really prefer a split I'd go for two patches:
> > 1. Add the flag to the enum and catalog;
> > 2. Add the ops guard (functional change).
> >
> > Then don't forget to reword the commit message, following the guidelines
> > below and the suggestion for 2/7.
> >
> > - Marijn
>
> Plan sounds good to me.
>
> Marijn, we will wait for a couple of days to post the next rev but would
> be hard more than that as we need to pick up other things which are
> pending on top of this. Hence would appreciate if you can finish reviews
> by then.
It depends on how many more revisions are needed after that, and not all
patches in this series have an r-b just yet. Given the amount of review
comments that are still trickling in (also on patches that already have
maintainer r-b) I don't think we're quite there to start thinging about
picking this up in drm-msm just yet. I doubt anyone wants a repeat of
the original DSC series, which went through many review rounds yet still
required multiple series of bugfixes (some of which were pointed out and
ignored in review) to be brought to a working state. But the split
across topics per series already makes this a lot less likely, many
thanks for that.
In other words, let's take it slow and do things properly this time. And
who knows, perhaps the rest of these patches are more straightforward.
- Marijn
<snip>
On 5/4/2023 12:59 PM, Dmitry Baryshkov wrote:
> On 04/05/2023 22:50, Abhinav Kumar wrote:
>>
>>
>> On 5/4/2023 12:36 PM, Marijn Suijten wrote:
>>> On 2023-05-04 11:25:44, Abhinav Kumar wrote:
>>> <snip>
>>>>> Sure, if you really prefer a split I'd go for two patches:
>>>>> 1. Add the flag to the enum and catalog;
>>>>> 2. Add the ops guard (functional change).
>>>>>
>>>>> Then don't forget to reword the commit message, following the
>>>>> guidelines
>>>>> below and the suggestion for 2/7.
>>>>>
>>>>> - Marijn
>>>>
>>>> Plan sounds good to me.
>>>>
>>>> Marijn, we will wait for a couple of days to post the next rev but
>>>> would
>>>> be hard more than that as we need to pick up other things which are
>>>> pending on top of this. Hence would appreciate if you can finish
>>>> reviews
>>>> by then.
>>>
>>> It depends on how many more revisions are needed after that, and not all
>>> patches in this series have an r-b just yet. Given the amount of review
>>> comments that are still trickling in (also on patches that already have
>>> maintainer r-b) I don't think we're quite there to start thinging about
>>> picking this up in drm-msm just yet. I doubt anyone wants a repeat of
>>> the original DSC series, which went through many review rounds yet still
>>> required multiple series of bugfixes (some of which were pointed out and
>>> ignored in review) to be brought to a working state. But the split
>>> across topics per series already makes this a lot less likely, many
>>> thanks for that.
>>>
>>
>> I think the outstanding comments shouldnt last more than 1-2 revs more
>> on this one as its mostly due to multiple patches on the list touching
>> catalog at the same time. I have been monitoring the comments closely
>> even though I dont respond to all of them.
>>
>> One of the major reasons of the number of issues with DSC 1.1 was QC
>> didn't really have the devices or panels to support it. Thats why I
>> changed that this time around to take more control of validation of
>> DSC 1.2 and ofcourse decided to break up of series into the least
>> amount of functionality needed to keep the DPU driver intact.
>>
>> All that being said, we still value your comments and would gladly
>> wait for a couple of days like I already wrote. But there are more
>> incremental series on top of this:
>>
>> -> DSI changes for DSC 1.2
>> -> proper teardown for DSC
>> -> DSC pair allocation support
>> -> DSC 1.2 over DP
>>
>> We will be posting all of these within next couple of weeks on top of
>> this.
>
> I'd say, it's fine to post them now, as we have more or less agreed on
> the helper series. The interface between the series should be stable then.
>
> The RM series is probably the one having bigger dependencies/conflicts
> on other pending patches (include virtual wide planes)
>
1 is already posted, will keep fixing review comments
2 will be posted pretty soon
DSC1.2 over DSI will be complete with this set.
I will finish up virtual planes review by early next week. Already
underway ...
3 & 4 will be posted soon after that.
>>
>>> In other words, let's take it slow and do things properly this time. And
>>> who knows, perhaps the rest of these patches are more straightforward.
>>>
>>
>> Ack. the intent is always to do things right the first time.
>>
>>> - Marijn
>>>
>>> <snip>
>
On 04/05/2023 22:50, Abhinav Kumar wrote:
>
>
> On 5/4/2023 12:36 PM, Marijn Suijten wrote:
>> On 2023-05-04 11:25:44, Abhinav Kumar wrote:
>> <snip>
>>>> Sure, if you really prefer a split I'd go for two patches:
>>>> 1. Add the flag to the enum and catalog;
>>>> 2. Add the ops guard (functional change).
>>>>
>>>> Then don't forget to reword the commit message, following the
>>>> guidelines
>>>> below and the suggestion for 2/7.
>>>>
>>>> - Marijn
>>>
>>> Plan sounds good to me.
>>>
>>> Marijn, we will wait for a couple of days to post the next rev but would
>>> be hard more than that as we need to pick up other things which are
>>> pending on top of this. Hence would appreciate if you can finish reviews
>>> by then.
>>
>> It depends on how many more revisions are needed after that, and not all
>> patches in this series have an r-b just yet. Given the amount of review
>> comments that are still trickling in (also on patches that already have
>> maintainer r-b) I don't think we're quite there to start thinging about
>> picking this up in drm-msm just yet. I doubt anyone wants a repeat of
>> the original DSC series, which went through many review rounds yet still
>> required multiple series of bugfixes (some of which were pointed out and
>> ignored in review) to be brought to a working state. But the split
>> across topics per series already makes this a lot less likely, many
>> thanks for that.
>>
>
> I think the outstanding comments shouldnt last more than 1-2 revs more
> on this one as its mostly due to multiple patches on the list touching
> catalog at the same time. I have been monitoring the comments closely
> even though I dont respond to all of them.
>
> One of the major reasons of the number of issues with DSC 1.1 was QC
> didn't really have the devices or panels to support it. Thats why I
> changed that this time around to take more control of validation of DSC
> 1.2 and ofcourse decided to break up of series into the least amount of
> functionality needed to keep the DPU driver intact.
>
> All that being said, we still value your comments and would gladly wait
> for a couple of days like I already wrote. But there are more
> incremental series on top of this:
>
> -> DSI changes for DSC 1.2
> -> proper teardown for DSC
> -> DSC pair allocation support
> -> DSC 1.2 over DP
>
> We will be posting all of these within next couple of weeks on top of this.
I'd say, it's fine to post them now, as we have more or less agreed on
the helper series. The interface between the series should be stable then.
The RM series is probably the one having bigger dependencies/conflicts
on other pending patches (include virtual wide planes)
>
>> In other words, let's take it slow and do things properly this time. And
>> who knows, perhaps the rest of these patches are more straightforward.
>>
>
> Ack. the intent is always to do things right the first time.
>
>> - Marijn
>>
>> <snip>
--
With best wishes
Dmitry
On 5/4/2023 12:36 PM, Marijn Suijten wrote:
> On 2023-05-04 11:25:44, Abhinav Kumar wrote:
> <snip>
>>> Sure, if you really prefer a split I'd go for two patches:
>>> 1. Add the flag to the enum and catalog;
>>> 2. Add the ops guard (functional change).
>>>
>>> Then don't forget to reword the commit message, following the guidelines
>>> below and the suggestion for 2/7.
>>>
>>> - Marijn
>>
>> Plan sounds good to me.
>>
>> Marijn, we will wait for a couple of days to post the next rev but would
>> be hard more than that as we need to pick up other things which are
>> pending on top of this. Hence would appreciate if you can finish reviews
>> by then.
>
> It depends on how many more revisions are needed after that, and not all
> patches in this series have an r-b just yet. Given the amount of review
> comments that are still trickling in (also on patches that already have
> maintainer r-b) I don't think we're quite there to start thinging about
> picking this up in drm-msm just yet. I doubt anyone wants a repeat of
> the original DSC series, which went through many review rounds yet still
> required multiple series of bugfixes (some of which were pointed out and
> ignored in review) to be brought to a working state. But the split
> across topics per series already makes this a lot less likely, many
> thanks for that.
>
I think the outstanding comments shouldnt last more than 1-2 revs more
on this one as its mostly due to multiple patches on the list touching
catalog at the same time. I have been monitoring the comments closely
even though I dont respond to all of them.
One of the major reasons of the number of issues with DSC 1.1 was QC
didn't really have the devices or panels to support it. Thats why I
changed that this time around to take more control of validation of DSC
1.2 and ofcourse decided to break up of series into the least amount of
functionality needed to keep the DPU driver intact.
All that being said, we still value your comments and would gladly wait
for a couple of days like I already wrote. But there are more
incremental series on top of this:
-> DSI changes for DSC 1.2
-> proper teardown for DSC
-> DSC pair allocation support
-> DSC 1.2 over DP
We will be posting all of these within next couple of weeks on top of this.
> In other words, let's take it slow and do things properly this time. And
> who knows, perhaps the rest of these patches are more straightforward.
>
Ack. the intent is always to do things right the first time.
> - Marijn
>
> <snip>
On 2023-05-04 12:50:57, Abhinav Kumar wrote:
>
>
> On 5/4/2023 12:36 PM, Marijn Suijten wrote:
> > On 2023-05-04 11:25:44, Abhinav Kumar wrote:
> > <snip>
> >>> Sure, if you really prefer a split I'd go for two patches:
> >>> 1. Add the flag to the enum and catalog;
> >>> 2. Add the ops guard (functional change).
> >>>
> >>> Then don't forget to reword the commit message, following the guidelines
> >>> below and the suggestion for 2/7.
> >>>
> >>> - Marijn
> >>
> >> Plan sounds good to me.
> >>
> >> Marijn, we will wait for a couple of days to post the next rev but would
> >> be hard more than that as we need to pick up other things which are
> >> pending on top of this. Hence would appreciate if you can finish reviews
> >> by then.
> >
> > It depends on how many more revisions are needed after that, and not all
> > patches in this series have an r-b just yet. Given the amount of review
> > comments that are still trickling in (also on patches that already have
> > maintainer r-b) I don't think we're quite there to start thinging about
> > picking this up in drm-msm just yet. I doubt anyone wants a repeat of
> > the original DSC series, which went through many review rounds yet still
> > required multiple series of bugfixes (some of which were pointed out and
> > ignored in review) to be brought to a working state. But the split
> > across topics per series already makes this a lot less likely, many
> > thanks for that.
> >
>
> I think the outstanding comments shouldnt last more than 1-2 revs more
> on this one as its mostly due to multiple patches on the list touching
> catalog at the same time. I have been monitoring the comments closely
> even though I dont respond to all of them.
>
> One of the major reasons of the number of issues with DSC 1.1 was QC
> didn't really have the devices or panels to support it. Thats why I
> changed that this time around to take more control of validation of DSC
> 1.2 and ofcourse decided to break up of series into the least amount of
> functionality needed to keep the DPU driver intact.
Really glad that you are able to test and validate it now, that goes a
long way. Does that also mean you can post the panel patches quickly,
so that everyone has a point of reference? As you said that is one of
the main points where DSC 1.1 "went wrong" (a misunderstanding of
drm_dsc_config).
> All that being said, we still value your comments and would gladly wait
> for a couple of days like I already wrote. But there are more
> incremental series on top of this:
>
> -> DSI changes for DSC 1.2
> -> proper teardown for DSC
> -> DSC pair allocation support
> -> DSC 1.2 over DP
Yeah, I'm familiar with the concept of having many dependent series, and
now DSC series are even rebasing on DPU (catalog) cleanups to preempt
conflicts, which is really hard to follow.
Dmitry just pushed v5 of "drm/i915/dsc: change DSC param tables to
follow the DSC model" [1] and seems to have dropped some patches that
some of these series are depending on, resulting in compilation
failures. Other series don't seem to fully mention all their
dependencies.
[1]: https://lore.kernel.org/linux-arm-msm/[email protected]/T/#u
So, for this to go as convenient as possible, do you have a list of
series, in a desired order that they should be reviewed and tested?
That way I can direct my priorities and help achieve the goal of picking
base dependencies as early as possible.
For example, one of the many series regresses DSC on the Xperia XZ3
(SDM845), but I have yet to bisect and understand which patch it is.
Will let you know as soon as I get my tree in order.
- Marijn
On 05/05/2023 00:39, Marijn Suijten wrote:
> On 2023-05-04 12:50:57, Abhinav Kumar wrote:
>>
>>
>> On 5/4/2023 12:36 PM, Marijn Suijten wrote:
>>> On 2023-05-04 11:25:44, Abhinav Kumar wrote:
>>> <snip>
>>>>> Sure, if you really prefer a split I'd go for two patches:
>>>>> 1. Add the flag to the enum and catalog;
>>>>> 2. Add the ops guard (functional change).
>>>>>
>>>>> Then don't forget to reword the commit message, following the guidelines
>>>>> below and the suggestion for 2/7.
>>>>>
>>>>> - Marijn
>>>>
>>>> Plan sounds good to me.
>>>>
>>>> Marijn, we will wait for a couple of days to post the next rev but would
>>>> be hard more than that as we need to pick up other things which are
>>>> pending on top of this. Hence would appreciate if you can finish reviews
>>>> by then.
>>>
>>> It depends on how many more revisions are needed after that, and not all
>>> patches in this series have an r-b just yet. Given the amount of review
>>> comments that are still trickling in (also on patches that already have
>>> maintainer r-b) I don't think we're quite there to start thinging about
>>> picking this up in drm-msm just yet. I doubt anyone wants a repeat of
>>> the original DSC series, which went through many review rounds yet still
>>> required multiple series of bugfixes (some of which were pointed out and
>>> ignored in review) to be brought to a working state. But the split
>>> across topics per series already makes this a lot less likely, many
>>> thanks for that.
>>>
>>
>> I think the outstanding comments shouldnt last more than 1-2 revs more
>> on this one as its mostly due to multiple patches on the list touching
>> catalog at the same time. I have been monitoring the comments closely
>> even though I dont respond to all of them.
>>
>> One of the major reasons of the number of issues with DSC 1.1 was QC
>> didn't really have the devices or panels to support it. Thats why I
>> changed that this time around to take more control of validation of DSC
>> 1.2 and ofcourse decided to break up of series into the least amount of
>> functionality needed to keep the DPU driver intact.
>
> Really glad that you are able to test and validate it now, that goes a
> long way. Does that also mean you can post the panel patches quickly,
> so that everyone has a point of reference? As you said that is one of
> the main points where DSC 1.1 "went wrong" (a misunderstanding of
> drm_dsc_config).
>
>> All that being said, we still value your comments and would gladly wait
>> for a couple of days like I already wrote. But there are more
>> incremental series on top of this:
>>
>> -> DSI changes for DSC 1.2
>> -> proper teardown for DSC
>> -> DSC pair allocation support
>> -> DSC 1.2 over DP
>
> Yeah, I'm familiar with the concept of having many dependent series, and
> now DSC series are even rebasing on DPU (catalog) cleanups to preempt
> conflicts, which is really hard to follow.
> Dmitry just pushed v5 of "drm/i915/dsc: change DSC param tables to
> follow the DSC model" [1] and seems to have dropped some patches that
> some of these series are depending on, resulting in compilation
> failures. Other series don't seem to fully mention all their
> dependencies.
We'd have to pick them into our code or submit directly to drm-misc. I
removed the patches which we can get in w/o Intel review.
>
> [1]: https://lore.kernel.org/linux-arm-msm/[email protected]/T/#u
>
> So, for this to go as convenient as possible, do you have a list of
> series, in a desired order that they should be reviewed and tested?
> That way I can direct my priorities and help achieve the goal of picking
> base dependencies as early as possible.
>
> For example, one of the many series regresses DSC on the Xperia XZ3
> (SDM845), but I have yet to bisect and understand which patch it is.
> Will let you know as soon as I get my tree in order.
>
> - Marijn
--
With best wishes
Dmitry