2023-05-22 20:05:05

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 00/10] add DSC 1.2 dpu supports

This series adds the DPU side changes to support DSC 1.2 encoder. This
was validated with both DSI DSC 1.2 panel and DP DSC 1.2 monitor.
The DSI and DP parts will be pushed later on top of this change.
This seriel is rebase on [1], [2] and catalog fixes from rev-4 of [3].

[1]: https://patchwork.freedesktop.org/series/116851/
[2]: https://patchwork.freedesktop.org/series/116615/
[3]: https://patchwork.freedesktop.org/series/112332/

Abhinav Kumar (2):
drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X
drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets

Kuogee Hsieh (8):
drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register
drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0
drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit
drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG
drm/msm/dpu: add support for DSC encoder v1.2 engine
drm/msm/dpu: always clear every individual pending flush mask
drm/msm/dpu: separate DSC flush update out of interface
drm/msm/dpu: tear down DSC data path when DSC disabled

drivers/gpu/drm/msm/Makefile | 1 +
.../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +
.../drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 +
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 51 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 24 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 35 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 34 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 11 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 15 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 387 +++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
19 files changed, 644 insertions(+), 30 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c

--
2.7.4



2023-05-22 20:13:15

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 03/10] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0

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 was 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 on DPU 7.0.0 and higher anymore.
Add DPU_PINGPONG_DSC to PINGPONG_SDM845_MASK, PINGPONG_SDM845_TE2_MASK
and PINGPONG_SM8150_MASK which is used for all DPU < 7.0 chipsets.

changes in v6:
-- split patches and rearrange to keep catalog related files at this patch

changes in v7:
-- rewording commit text as suggested at review comments

changes in v9:
-- delete BIT(DPU_PINGPONG_DSC) from PINGPONG_SDM845_TE2_MASK

changes in v10:
-- correct order of commit text

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)

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..f2a1535 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -76,13 +76,13 @@
(BIT(DPU_DIM_LAYER) | BIT(DPU_MIXER_COMBINED_ALPHA))

#define PINGPONG_SDM845_MASK \
- (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE))
+ (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE) | BIT(DPU_PINGPONG_DSC))

#define PINGPONG_SDM845_TE2_MASK \
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))

#define PINGPONG_SM8150_MASK \
- (BIT(DPU_PINGPONG_DITHER))
+ (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_DSC))

#define CTL_SC7280_MASK \
(BIT(DPU_CTL_ACTIVE_CFG) | \
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..83854e8 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 block supports 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
};

--
2.7.4


2023-05-22 20:15:34

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 04/10] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit

DPU < 7.0.0 has DPU_PINGPONG_DSC feature bit set to indicate it requires
both dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() to be
executed to complete DSC configuration if DSC hardware block is present.
Hence test DPU_PINGPONG_DSC feature bit and assign DSC related functions
to the ops of PINGPONG block accordingly if DPU_PINGPONG_DSC bit is set.

Changes in v6:
-- split patches, this patch has function handles DPU_PINGPONG_DSC bit

Changes in v9:
-- the original code of assigning dsc related functions to the ops of
pingpong block without testing the DPU_PINGPONG_DSC feature bit was
restored back due to rebase error which defeat the purpose of this
patch. Remove those error code.

Changes in v10:
-- change commit title
-- correct texts at changes in v9

Changes in v12:
-- fixed length too long at Changes in v9

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)

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..437d9e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -291,9 +291,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
c->ops.get_line_count = dpu_hw_pp_get_line_count;
c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
}
- 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_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;
--
2.7.4


2023-05-22 20:16:10

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 07/10] drm/msm/dpu: always clear every individual pending flush mask

There are two tiers of pending flush control, main controller and
individual hardware block. Currently only the main controller of
flush mask is reset to 0 but leave out some individual pending flush
mask of particular hardware block keep previous value at
clear_pending_flush(). Reset all individual hardware blocks flush
mask to 0 to avoid individual hardware block be triggered accidentally.

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

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 69d0ea2..069c6e5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -100,7 +100,9 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct dpu_hw_ctl *ctx)
trace_dpu_hw_ctl_clear_pending_flush(ctx->pending_flush_mask,
dpu_hw_ctl_get_flush_register(ctx));
ctx->pending_flush_mask = 0x0;
-
+ ctx->pending_intf_flush_mask = 0;
+ ctx->pending_wb_flush_mask = 0;
+ ctx->pending_merge_3d_flush_mask = 0;
memset(ctx->pending_dspp_flush_mask, 0,
sizeof(ctx->pending_dspp_flush_mask));
}
--
2.7.4


2023-05-22 20:17:21

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 05/10] drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG

Disabling the crossbar mux between DSC and PINGPONG currently
requires a bogus enum dpu_pingpong value to be passed when calling
dsc_bind_pingpong_blk() with enable=false, even though the register
value written is independent of the current PINGPONG block. Replace
that `bool enable` parameter with a new PINGPONG_NONE dpu_pingpong
flag that triggers the write of the "special" 0xF "crossbar
disabled" value to the register instead.

Changes in v4:
-- more details to commit text

Changes in v5:
-- rewording commit text suggested by Marijn
-- add DRM_DEBUG_KMS for DSC unbinding case

Changes in v8:
-- fix checkpatch warning

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +++++++-------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++-
4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index cf1de5d..ffa6f04 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1850,7 +1850,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
hw_pp->ops.setup_dsc(hw_pp);

if (hw_dsc->ops.dsc_bind_pingpong_blk)
- hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, true, hw_pp->idx);
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, hw_pp->idx);

if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 8deedeae..509dbaa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -157,7 +157,6 @@ static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,

static void dpu_hw_dsc_bind_pingpong_blk(
struct dpu_hw_dsc *hw_dsc,
- bool enable,
const enum dpu_pingpong pp)
{
struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
@@ -166,14 +165,15 @@ static void dpu_hw_dsc_bind_pingpong_blk(

dsc_ctl_offset = DSC_CTL(hw_dsc->idx);

- if (enable)
+ if (pp)
mux_cfg = (pp - PINGPONG_0) & 0x7;

- DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n",
- enable ? "Binding" : "Unbinding",
- hw_dsc->idx - DSC_0,
- enable ? "to" : "from",
- pp - PINGPONG_0);
+ if (pp)
+ DRM_DEBUG_KMS("Binding dsc:%d to pp:%d\n",
+ hw_dsc->idx - DSC_0, pp - PINGPONG_0);
+ else
+ DRM_DEBUG_KMS("Unbinding dsc:%d from any pp\n",
+ hw_dsc->idx - DSC_0);

DPU_REG_WRITE(c, dsc_ctl_offset, mux_cfg);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index 287ec5f..138080a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -44,7 +44,6 @@ struct dpu_hw_dsc_ops {
struct drm_dsc_config *dsc);

void (*dsc_bind_pingpong_blk)(struct dpu_hw_dsc *hw_dsc,
- bool enable,
enum dpu_pingpong pp);
};

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 1913a19..02a0f48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -191,7 +191,8 @@ enum dpu_dsc {
};

enum dpu_pingpong {
- PINGPONG_0 = 1,
+ PINGPONG_NONE,
+ PINGPONG_0,
PINGPONG_1,
PINGPONG_2,
PINGPONG_3,
--
2.7.4


2023-05-22 20:17:37

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 10/10] drm/msm/dpu: tear down DSC data path when DSC disabled

Unset DSC_ACTIVE bit at dpu_hw_ctl_reset_intf_cfg_v1(),
dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() functions
to tear down DSC data path if DSC data path was setup previous.

Changes in V10:
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
-- move both dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() to above phys_cleanup()

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
Reviewed-by: Marijn Suijten <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++
2 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7fca09e..3b416e1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2036,6 +2036,41 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
}
}

+static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
+ struct dpu_hw_pingpong *hw_pp)
+{
+ if (hw_dsc->ops.dsc_disable)
+ hw_dsc->ops.dsc_disable(hw_dsc);
+
+ if (hw_pp->ops.disable_dsc)
+ hw_pp->ops.disable_dsc(hw_pp);
+
+ if (hw_dsc->ops.dsc_bind_pingpong_blk)
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, PINGPONG_NONE);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
+}
+
+static void dpu_encoder_unprep_dsc(struct dpu_encoder_virt *dpu_enc)
+{
+ /* coding only for 2LM, 2enc, 1 dsc config */
+ struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
+ struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+ int i;
+
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ hw_pp[i] = dpu_enc->hw_pp[i];
+ hw_dsc[i] = dpu_enc->hw_dsc[i];
+
+ if (hw_pp[i] && hw_dsc[i])
+ dpu_encoder_dsc_pipe_clr(ctl, hw_dsc[i], hw_pp[i]);
+ }
+}
+
void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
@@ -2086,8 +2121,12 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}

+ if (dpu_enc->dsc)
+ dpu_encoder_unprep_dsc(dpu_enc);
+
intf_cfg.stream_sel = 0; /* Don't care value for video mode */
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);

if (phys_enc->hw_intf)
intf_cfg.intf = phys_enc->hw_intf->idx;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 20e08c6..c2cb426 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -581,6 +581,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
u32 intf_active = 0;
u32 wb_active = 0;
u32 merge3d_active = 0;
+ u32 dsc_active;

/*
* This API resets each portion of the CTL path namely,
@@ -610,6 +611,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
wb_active &= ~BIT(cfg->wb - WB_0);
DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
}
+
+ if (cfg->dsc) {
+ dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
+ dsc_active &= ~cfg->dsc;
+ DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
+ }
}

static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
--
2.7.4


2023-05-22 20:18:01

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
to be written to CTL flush register to indicates that DSC is ready for
flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current
implementation. Fix this problem by writing BIT(22) to CTL flush register
for DSC to work properly.

Changes in V12:
-- split this patch out of "separate DSC flush update out of interface"

Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

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..69d0ea2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
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);
+ DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, BIT(DSC_IDX));
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
}
}
--
2.7.4


2023-05-22 20:18:01

by Kuogee Hsieh

[permalink] [raw]
Subject: [PATCH v12 08/10] drm/msm/dpu: separate DSC flush update out of interface

Currently DSC flushing happens during interface configuration at
dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
to handle both per-DSC engine and DSC flush bits at same time to make it
consistent with the location of flush programming of other DPU sub-blocks.

Changes in v10:
-- rewording commit text
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
-- ctx->pending_dsc_flush_mask = 0;

Changes in v11:
-- add Fixes tag
-- capitalize MERGE_3D, DSPP and DSC at struct dpu_hw_ctl_ops{}

Changes in v12:
-- move dsc parameter to next line at dpu_encoder_dsc_pipe_cfg()

Signed-off-by: Kuogee Hsieh <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 23 +++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 11 +++++++++++
3 files changed, 36 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..7fca09e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
return DIV_ROUND_UP(total_pixels, dsc->slice_width);
}

-static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
+static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
struct dpu_hw_pingpong *hw_pp,
struct drm_dsc_config *dsc,
u32 common_mode,
@@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,

if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
}

static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
@@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
{
/* coding only for 2LM, 2enc, 1 dsc config */
struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
int this_frame_slices;
@@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);

for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
- dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
+ dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i],
+ dsc, dsc_common_mode, initial_lines);
}

void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 069c6e5..20e08c6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -103,6 +103,7 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct dpu_hw_ctl *ctx)
ctx->pending_intf_flush_mask = 0;
ctx->pending_wb_flush_mask = 0;
ctx->pending_merge_3d_flush_mask = 0;
+ ctx->pending_dsc_flush_mask = 0;
memset(ctx->pending_dspp_flush_mask, 0,
sizeof(ctx->pending_dspp_flush_mask));
}
@@ -141,6 +142,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);
}

@@ -287,6 +293,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)
{
@@ -504,9 +517,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);

@@ -526,10 +536,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
BIT(cfg->merge_3d - MERGE_3D_0));
- if (cfg->dsc) {
- DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, BIT(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,
@@ -632,6 +641,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
ops->update_pending_flush_merge_3d =
dpu_hw_ctl_update_pending_flush_merge_3d_v1;
ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
+ ops->update_pending_flush_dsc =
+ dpu_hw_ctl_update_pending_flush_dsc_v1;
} else {
ops->trigger_flush = dpu_hw_ctl_trigger_flush;
ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 6292002..1c24229 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
enum dpu_dspp blk, u32 dspp_sub_blk);

/**
+ * OR in the given flushbits to the cached pending_(dsc_)flush_mask
+ * No effect on hardware
+ * @ctx: ctl path ctx pointer
+ * @blk: interface block index
+ */
+ void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
+ enum dpu_dsc blk);
+
+ /**
* Write the value of the pending_flush_mask to hardware
* @ctx : ctl path ctx pointer
*/
@@ -229,6 +238,7 @@ struct dpu_hw_ctl_ops {
* @pending_flush_mask: storage for pending ctl_flush managed via ops
* @pending_intf_flush_mask: pending INTF flush
* @pending_wb_flush_mask: pending WB flush
+ * @pending_dsc_flush_mask: pending DSC flush
* @ops: operation list
*/
struct dpu_hw_ctl {
@@ -245,6 +255,7 @@ struct dpu_hw_ctl {
u32 pending_wb_flush_mask;
u32 pending_merge_3d_flush_mask;
u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
+ u32 pending_dsc_flush_mask;

/* ops */
struct dpu_hw_ctl_ops ops;
--
2.7.4


2023-05-22 20:20:37

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

On 22/05/2023 22:30, Kuogee Hsieh wrote:
> DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
> to be written to CTL flush register to indicates that DSC is ready for
> flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current

DSC_IDX is 22, not 0x22.

> implementation. Fix this problem by writing BIT(22) to CTL flush register
> for DSC to work properly.
>
> Changes in V12:
> -- split this patch out of "separate DSC flush update out of interface"
>
> Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> 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..69d0ea2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> 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);
> + DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, BIT(DSC_IDX));
> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> }
> }

--
With best wishes
Dmitry


2023-05-22 21:33:42

by Marijn Suijten

[permalink] [raw]
Subject: Re: [PATCH v12 01/10] drm/msm/dpu: set DSC flush bit correctly at MDP CTL flush register

On 2023-05-22 23:18:02, Dmitry Baryshkov wrote:
> On 22/05/2023 22:30, Kuogee Hsieh wrote:
> > DSC flush bit is the bit 22 of CTL flush register. BIT(22) is expected
> > to be written to CTL flush register to indicates that DSC is ready for
> > flush. However 0x22 (BIT(1) and BIT(5)) was written mistakenly at current
>
> DSC_IDX is 22, not 0x22.
>
> > implementation. Fix this problem by writing BIT(22) to CTL flush register
> > for DSC to work properly.

Perhaps this whole comment needs a rewrite anyway, instead of pointing
out grammar errors and confusion:

The DSC CTL_FLUSH register should be programmed with the 22th bit
(DSC_IDX) to flush the DSC hardware blocks, not the literal value of
22 (which corresponds to flushing VIG1, VIG2 and RGB1 instead).

Patch contents are good though, thanks!

- Marijn

> > Changes in V12:
> > -- split this patch out of "separate DSC flush update out of interface"
> >
> > Fixes: 77f6da90487c ("drm/msm/disp/dpu1: Add DSC support in hw_ctl")
> > Signed-off-by: Kuogee Hsieh <[email protected]>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > 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..69d0ea2 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > @@ -525,7 +525,7 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> > 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);
> > + DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, BIT(DSC_IDX));
> > DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> > }
> > }
>
> --
> With best wishes
> Dmitry
>

2023-05-22 21:46:27

by Marijn Suijten

[permalink] [raw]
Subject: Re: [PATCH v12 07/10] drm/msm/dpu: always clear every individual pending flush mask

On 2023-05-22 12:30:50, Kuogee Hsieh wrote:
> There are two tiers of pending flush control, main controller and
> individual hardware block. Currently only the main controller of

I would call "main controller" "top level" instead, but not sure how the
hardware manual calls this?

> flush mask is reset to 0 but leave out some individual pending flush
> mask of particular hardware block keep previous value at
> clear_pending_flush().

... but the individual pending flush masks of particular hardware blocks
are left at their previous values, eventually accumulating all possible
bit values and typically flushing more than necessary.

> Reset all individual hardware blocks flush
> mask to 0 to avoid individual hardware block be triggered accidentally.

maskS*

>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>

Thanks! Does this need any fixes tag?

Reviewed-by: Marijn Suijten <[email protected]>

> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> 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 69d0ea2..069c6e5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -100,7 +100,9 @@ static inline void dpu_hw_ctl_clear_pending_flush(struct dpu_hw_ctl *ctx)
> trace_dpu_hw_ctl_clear_pending_flush(ctx->pending_flush_mask,
> dpu_hw_ctl_get_flush_register(ctx));
> ctx->pending_flush_mask = 0x0;
> -
> + ctx->pending_intf_flush_mask = 0;
> + ctx->pending_wb_flush_mask = 0;
> + ctx->pending_merge_3d_flush_mask = 0;

I wouldn't mind keeping an empty line here.

> memset(ctx->pending_dspp_flush_mask, 0,
> sizeof(ctx->pending_dspp_flush_mask));
> }
> --
> 2.7.4
>