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.
Abhinav Kumar (1):
drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
Kuogee Hsieh (4):
drm/msm/dpu: add support for DSC encoder v1.2 engine
drm/msm/dpu: separate DSC flush update out of interface
drm/msm/dpu: save dpu topology configuration
drm/msm/dpu: calculate DSC encoder parameters dynamically
drivers/gpu/drm/msm/Makefile | 1 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21 ++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19 +
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19 +
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 102 +++---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 38 +-
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.h | 17 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388 +++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
14 files changed, 629 insertions(+), 57 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
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.
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Kuogee Hsieh <[email protected]>
---
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19 +++++++++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21 +++++++++++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19 +++++++++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19 +++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++--
6 files changed, 99 insertions(+), 2 deletions(-)
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 ca107ca..3f2dcc0 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,23 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+static const struct dpu_dsc_sub_blks sm8350_dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sm8350_dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sm8350_dsc[] = {
+ DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8350_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8350_dsc_sblk_1),
+ DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_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, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
@@ -205,6 +222,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 5957de1..858577c 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,15 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
PP_BLK("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
};
+static const struct dpu_dsc_sub_blks sc7280_dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sc7280_dsc[] = {
+ DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sc7280_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, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x35000, 0x2c4, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
@@ -142,6 +151,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 9aab110..28443e2 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,25 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+static const struct dpu_dsc_sub_blks sc8280xp_dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sc8280xp_dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
+ DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_1),
+ DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8350_dsc_sblk_1),
+ DSC_BLK_1_2("dsc_2", DSC_4, 0x82000, 0x100, 0, sm8350_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_2", DSC_5, 0x82000, 0x100, 0, sm8350_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, MDP_SSPP_TOP0_INTR, 24, 25),
@@ -196,6 +215,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 02a259b..7c25786 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,23 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
};
+static const struct dpu_dsc_sub_blks sm8450_dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sm8450_dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sm8450_dsc[] = {
+ DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8450_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8450_dsc_sblk_1),
+ DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8450_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8450_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, MDP_SSPP_TOP0_INTR, 24, 25),
INTF_BLK("intf_1", INTF_1, 0x35000, 0x300, INTF_DSI, 0, 24, INTF_SC7280_MASK, MDP_SSPP_TOP0_INTR, 26, 27),
@@ -213,6 +230,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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
};
+static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
+static const struct dpu_dsc_cfg sm8550_dsc[] = {
+ DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8550_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8550_dsc_sblk_1),
+ DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
+ DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24, 25),
/* TODO TE sub-blocks for intf1 & intf2 */
@@ -218,6 +235,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 03f162a..be08158 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__
@@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
{\
.name = _name, .id = _id, \
.base = _base, .len = 0x140, \
- .features = _features, \
+ .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
+ }
+
+#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, \
}
/*************************************************************
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
At current implementation, topology configuration is thrown away after
dpu_rm_reserve(). This patch save the topology so that it can be used
for DSC related calculation later.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 ++++++++++++++---------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index ecb87bc..2fdacf1 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
return (num_dsc > 0) && (num_dsc > intf_count);
}
-static struct msm_display_topology dpu_encoder_get_topology(
+static void dpu_encoder_get_topology(
struct dpu_encoder_virt *dpu_enc,
struct dpu_kms *dpu_kms,
struct drm_display_mode *mode,
- struct drm_crtc_state *crtc_state)
+ struct drm_crtc_state *crtc_state,
+ struct msm_display_topology *topology)
{
- struct msm_display_topology topology = {0};
int i, intf_count = 0;
for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
@@ -567,16 +567,16 @@ static struct msm_display_topology dpu_encoder_get_topology(
* Add dspps to the reservation requirements if ctm is requested
*/
if (intf_count == 2)
- topology.num_lm = 2;
+ topology->num_lm = 2;
else if (!dpu_kms->catalog->caps->has_3d_merge)
- topology.num_lm = 1;
+ topology->num_lm = 1;
else
- topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
+ topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
if (crtc_state->ctm)
- topology.num_dspp = topology.num_lm;
+ topology->num_dspp = topology->num_lm;
- topology.num_intf = intf_count;
+ topology->num_intf = intf_count;
if (dpu_enc->dsc) {
/*
@@ -585,12 +585,10 @@ static struct msm_display_topology dpu_encoder_get_topology(
* this is power optimal and can drive up to (including) 4k
* screens
*/
- topology.num_dsc = 2;
- topology.num_lm = 2;
- topology.num_intf = 1;
+ topology->num_dsc = 2;
+ topology->num_lm = 2;
+ topology->num_intf = 1;
}
-
- return topology;
}
static int dpu_encoder_virt_atomic_check(
@@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
struct msm_drm_private *priv;
struct dpu_kms *dpu_kms;
struct drm_display_mode *adj_mode;
- struct msm_display_topology topology;
+ struct msm_display_topology *topology;
struct dpu_global_state *global_state;
int i = 0;
int ret = 0;
@@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
}
}
- topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
+ topology = &dpu_enc->topology;
+ memset(topology, 0, sizeof (*topology));
+ dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, topology);
/*
* Release and Allocate resources on every modeset
@@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
if (!crtc_state->active_changed || crtc_state->enable)
ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
- drm_enc, crtc_state, topology);
+ drm_enc, crtc_state, *topology);
}
trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
--
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 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.
Signed-off-by: Kuogee Hsieh <[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 1dc5dbe..ecb87bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1823,12 +1823,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);
@@ -1843,6 +1849,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,
@@ -1887,7 +1896,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 bbdc95c..d2a1608 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -156,6 +156,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);
}
@@ -302,6 +307,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)
{
@@ -519,9 +531,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);
@@ -541,10 +550,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,
@@ -647,6 +654,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 78611a8..8330550 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
During DSC preparation, add run time calculation to figure out what
usage modes, split mode and merge mode, is going to be setup.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 ++++++++++++++++-------------
1 file changed, 32 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 2fdacf1..5677728 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
{
struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
- int i, intf_count = 0, num_dsc = 0;
+ struct msm_display_topology *topology = &dpu_enc->topology;
- for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
- if (dpu_enc->phys_encs[i])
- intf_count++;
-
- /* See dpu_encoder_get_topology, we only support 2:2:1 topology */
- if (dpu_enc->dsc)
- num_dsc = 2;
-
- return (num_dsc > 0) && (num_dsc > intf_count);
+ return (topology->num_dsc > topology->num_intf);
}
static void dpu_encoder_get_topology(
@@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+ struct msm_display_topology *topology = &dpu_enc->topology;
int this_frame_slices;
int intf_ip_w, enc_ip_w;
- int dsc_common_mode;
+ int dsc_common_mode = 0;
int pic_width;
u32 initial_lines;
+ int num_dsc, num_intf;
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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
- return;
- }
}
- dsc_common_mode = 0;
+ num_dsc = topology->num_dsc;
+ num_intf = topology->num_intf;
+
pic_width = dsc->pic_width;
- dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
if (enc_master->intf_mode == INTF_MODE_VIDEO)
dsc_common_mode |= DSC_MODE_VIDEO;
+ /*
+ * If this encoder is driving more than one DSC encoder, they
+ * operate in tandem, same pic dimension needs to be used by
+ * each of them.(pp-split is assumed to be not supported)
+ *
+ */
+
this_frame_slices = pic_width / dsc->slice_width;
intf_ip_w = this_frame_slices * dsc->slice_width;
+ enc_ip_w = intf_ip_w;
+
+ intf_ip_w /= num_intf;
+
+ if (num_dsc > 1)
+ dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
+
+ if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
+ dsc_common_mode |= DSC_MODE_MULTIPLEX;
+ /*
+ * in dsc merge case: when using 2 encoders for the same
+ * stream, no. of slices need to be same on both the
+ * encoders.
+ */
+ enc_ip_w = intf_ip_w / 2;
+ }
- /*
- * dsc merge case: when using 2 encoders for the same stream,
- * no. of slices need to be same on both the encoders.
- */
- enc_ip_w = intf_ip_w / 2;
initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
- for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+ for (i = 0; i < num_dsc; i++)
dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
dsc_common_mode, initial_lines);
}
--
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.
Signed-off-by: Kuogee Hsieh <[email protected]>
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 38 ++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 17 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388 +++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
5 files changed, 444 insertions(+), 7 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 71584cd..22421b9 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.
*/
@@ -241,12 +241,20 @@ enum {
};
/**
- * DSC features
- * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
- * the pixel output from this DSC.
+ * DSC sub-blocks/features
+ * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
+ * the pixel output from this DSC.
+ * @DPU_DSC_HW_REV_1_1 DSC block supports dsc 1.1 only
+ * @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_1,
+ DPU_DSC_HW_REV_1_2,
+ DPU_DSC_NATIVE_422_EN,
+ DPU_DSC_MAX
};
/**
@@ -311,6 +319,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 {
@@ -459,6 +475,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 {
@@ -612,10 +639,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 ae9b5db..d0f8b8b 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
@@ -61,7 +64,7 @@ struct dpu_hw_dsc {
};
/**
- * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
+ * dpu_hw_dsc_init - initializes the v1.1 dsc block for the passed dsc idx.
* @idx: DSC index for which driver object is required
* @addr: Mapped register io address of MDP
* @m: Pointer to mdss catalog data
@@ -71,6 +74,16 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
const struct dpu_mdss_cfg *m);
/**
+ * dpu_hw_dsc_init_1_2 - initializes the v1.2 dsc block for the passed dsc idx.
+ * @idx: DSC index for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * @m: Pointer to mdss catalog data
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(enum dpu_dsc idx, void __iomem *addr,
+ const struct dpu_mdss_cfg *m);
+
+/**
* 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..455d7f2
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
@@ -0,0 +1,388 @@
+// 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
+
+/*
+ * @DPU_DSC_ENC DSC encoder sub block
+ * @DPU_DSC_CTL DSC controller sub block
+ */
+enum {
+ DPU_DSC_ENC,
+ DPU_DSC_CTL,
+};
+
+static 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 _dsc_subblk_offset(struct dpu_hw_dsc *hw_dsc, int s_id,
+ u32 *offset)
+{
+ const struct dpu_dsc_sub_blks *sblk;
+
+ sblk = hw_dsc->caps->sblk;
+
+ if (s_id == DPU_DSC_ENC)
+ *offset = sblk->enc.base;
+ else if (s_id == DPU_DSC_CTL)
+ *offset = sblk->ctl.base;
+ else
+ DPU_ERROR("invalid DSC sub block=%d\n", s_id);
+}
+
+static 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;
+
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
+
+ hw = &hw_dsc->hw;
+ DPU_REG_WRITE(hw, offset + DSC_CFG, 0);
+
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
+
+ DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, 0);
+ DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, 0);
+}
+
+static 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;
+ void __iomem *off;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
+
+ 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 */
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
+
+ off = hw->blk_addr + offset;
+
+ 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 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, off;
+ int i, j = 0;
+ struct drm_dsc_rc_range_parameters *rc;
+ u32 data = 0, min_qp = 0, max_qp = 0, bpg_off = 0;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
+
+ hw = &hw_dsc->hw;
+
+ rc = dsc->rc_range_params;
+
+ off = 0;
+ for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
+ data |= dsc->rc_buf_thresh[i] << (8 * j);
+ j++;
+ if ((j == 4) || (i == DSC_NUM_BUF_RANGES - 2)) {
+ DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0 + off,
+ data);
+ off += 4;
+ j = 0;
+ data = 0;
+ }
+ }
+
+ off = 0;
+ for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
+ min_qp |= rc[i].range_min_qp << (5 * j);
+ max_qp |= rc[i].range_max_qp << (5 * j);
+ bpg_off |= rc[i].range_bpg_offset << (6 * j);
+ j++;
+ if (j == 5) {
+ DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0 + off,
+ min_qp);
+ DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0 + off,
+ max_qp);
+ DPU_REG_WRITE(hw,
+ offset + DSC_RC_RANGE_BPG_OFFSETS_0 + off,
+ bpg_off);
+ off += 4;
+ j = 0;
+ min_qp = 0;
+ max_qp = 0;
+ bpg_off = 0;
+ }
+ }
+}
+
+static void dpu_hw_dsc_bind_pingpong_blk_1_2(
+ struct dpu_hw_dsc *hw_dsc,
+ bool enable,
+ const enum dpu_pingpong pp)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ int offset;
+ int mux_cfg = 0xf; /* Disabled */
+
+ _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
+
+ hw = &hw_dsc->hw;
+ if (enable)
+ mux_cfg = (pp - PINGPONG_0) & 0x7;
+
+ DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
+}
+
+static const struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
+ const struct dpu_mdss_cfg *m,
+ void __iomem *addr,
+ struct dpu_hw_blk_reg_map *b)
+{
+ int i;
+
+ for (i = 0; i < m->dsc_count; i++) {
+ if (dsc == m->dsc[i].id) {
+ b->blk_addr = addr + m->dsc[i].base;
+ b->log_mask = DPU_DBG_MASK_DSC;
+ return &m->dsc[i];
+ }
+ }
+
+ return NULL;
+}
+
+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(enum dpu_dsc idx, void __iomem *addr,
+ const struct dpu_mdss_cfg *m)
+{
+ struct dpu_hw_dsc *c;
+ const struct dpu_dsc_cfg *cfg;
+
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ cfg = _dsc_offset(idx, m, addr, &c->hw);
+ if (IS_ERR_OR_NULL(cfg)) {
+ kfree(c);
+ return ERR_PTR(-EINVAL);
+ }
+
+ c->idx = idx;
+ 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 f4dda88..f7014f5 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__
@@ -250,7 +251,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->id, mmio, cat);
+ if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
+ hw = dpu_hw_dsc_init_1_2(dsc->id, mmio, cat);
+ else
+ hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
+
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 21/04/2023 02:25, Kuogee Hsieh wrote:
> Add support for DSC 1.2 by providing the necessary hooks to program
> the DPU DSC 1.2 encoder.
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 38 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 17 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388 +++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
> 5 files changed, 444 insertions(+), 7 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 71584cd..22421b9 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.
> */
>
> @@ -241,12 +241,20 @@ enum {
> };
>
> /**
> - * DSC features
> - * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
> - * the pixel output from this DSC.
> + * DSC sub-blocks/features
> + * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
> + * the pixel output from this DSC.
Any reason to change the alignment
> + * @DPU_DSC_HW_REV_1_1 DSC block supports dsc 1.1 only
> + * @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_1,
> + DPU_DSC_HW_REV_1_2,
> + DPU_DSC_NATIVE_422_EN,
> + DPU_DSC_MAX
> };
>
> /**
> @@ -311,6 +319,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 {
> @@ -459,6 +475,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 {
> @@ -612,10 +639,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 ae9b5db..d0f8b8b 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
> @@ -61,7 +64,7 @@ struct dpu_hw_dsc {
> };
>
> /**
> - * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
> + * dpu_hw_dsc_init - initializes the v1.1 dsc block for the passed dsc idx.
> * @idx: DSC index for which driver object is required
> * @addr: Mapped register io address of MDP
> * @m: Pointer to mdss catalog data
> @@ -71,6 +74,16 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc idx, void __iomem *addr,
> const struct dpu_mdss_cfg *m);
>
> /**
> + * dpu_hw_dsc_init_1_2 - initializes the v1.2 dsc block for the passed dsc idx.
> + * @idx: DSC index for which driver object is required
> + * @addr: Mapped register io address of MDP
> + * @m: Pointer to mdss catalog data
> + * Returns: Error code or allocated dpu_hw_dsc context
> + */
> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(enum dpu_dsc idx, void __iomem *addr,
> + const struct dpu_mdss_cfg *m);
> +
> +/**
> * 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..455d7f2
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
> @@ -0,0 +1,388 @@
> +// 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
As you are adding new definitions, could please make them aligned from
the beginning?
> +#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
> +
> +/*
> + * @DPU_DSC_ENC DSC encoder sub block
> + * @DPU_DSC_CTL DSC controller sub block
> + */
> +enum {
> + DPU_DSC_ENC,
> + DPU_DSC_CTL,
> +};
> +
> +static int _dsc_calc_ob_max_addr(struct dpu_hw_dsc *hw_dsc, int num_ss)
If this is used from a single place, please inline this function.
> +{
> + int max_addr = 2400 / num_ss;
> +
> + if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN))
> + max_addr /= 2;
Is this really guarded by the hw block feature or by native_422 being
enabled?
> +
> + return max_addr - 1;
> +};
> +
>
[skipped]
> +static 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, off;
> + int i, j = 0;
> + struct drm_dsc_rc_range_parameters *rc;
> + u32 data = 0, min_qp = 0, max_qp = 0, bpg_off = 0;
> +
> + if (!hw_dsc || !dsc)
> + return;
> +
> + _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
> +
> + hw = &hw_dsc->hw;
> +
> + rc = dsc->rc_range_params;
> +
> + off = 0;
> + for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
> + data |= dsc->rc_buf_thresh[i] << (8 * j);
> + j++;
> + if ((j == 4) || (i == DSC_NUM_BUF_RANGES - 2)) {
> + DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0 + off,
> + data);
> + off += 4;
> + j = 0;
> + data = 0;
> + }
> + }
> +
> + off = 0;
> + for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
> + min_qp |= rc[i].range_min_qp << (5 * j);
> + max_qp |= rc[i].range_max_qp << (5 * j);
> + bpg_off |= rc[i].range_bpg_offset << (6 * j);
> + j++;
> + if (j == 5) {
> + DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0 + off,
> + min_qp);
> + DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0 + off,
> + max_qp);
> + DPU_REG_WRITE(hw,
> + offset + DSC_RC_RANGE_BPG_OFFSETS_0 + off,
> + bpg_off);
> + off += 4;
> + j = 0;
> + min_qp = 0;
> + max_qp = 0;
> + bpg_off = 0;
> + }
> + }
Unrolling these loops would make it easier to understand them.
> +}
> +
> +static void dpu_hw_dsc_bind_pingpong_blk_1_2(
> + struct dpu_hw_dsc *hw_dsc,
> + bool enable,
> + const enum dpu_pingpong pp)
> +{
> + struct dpu_hw_blk_reg_map *hw;
> + int offset;
> + int mux_cfg = 0xf; /* Disabled */
> +
> + _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
> +
> + hw = &hw_dsc->hw;
> + if (enable)
> + mux_cfg = (pp - PINGPONG_0) & 0x7;
> +
> + DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
> +}
> +
> +static const struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
> + const struct dpu_mdss_cfg *m,
> + void __iomem *addr,
> + struct dpu_hw_blk_reg_map *b)
> +{
> + int i;
> +
> + for (i = 0; i < m->dsc_count; i++) {
> + if (dsc == m->dsc[i].id) {
> + b->blk_addr = addr + m->dsc[i].base;
> + b->log_mask = DPU_DBG_MASK_DSC;
> + return &m->dsc[i];
> + }
> + }
> +
> + return NULL;
> +}
> +
> +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(enum dpu_dsc idx, void __iomem *addr,
> + const struct dpu_mdss_cfg *m)
> +{
Please rebase on top of https://patchwork.freedesktop.org/series/116615/
> + struct dpu_hw_dsc *c;
> + const struct dpu_dsc_cfg *cfg;
> +
> + c = kzalloc(sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + cfg = _dsc_offset(idx, m, addr, &c->hw);
> + if (IS_ERR_OR_NULL(cfg)) {
> + kfree(c);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + c->idx = idx;
> + 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 f4dda88..f7014f5 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__
> @@ -250,7 +251,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->id, mmio, cat);
> + if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
> + hw = dpu_hw_dsc_init_1_2(dsc->id, mmio, cat);
> + else
> + hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
> +
> if (IS_ERR_OR_NULL(hw)) {
> rc = PTR_ERR(hw);
> DPU_ERROR("failed dsc object creation: err %d\n", rc);
--
With best wishes
Dmitry
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> Current DSC flush update is piggyback inside dpu_hw_ctl_intf_cfg_v1().
> This patch 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.
>
> Signed-off-by: Kuogee Hsieh <[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(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> From: Abhinav Kumar <[email protected]>
>
> Add DSC 1.2 hardware blocks to the catalog with necessary
> sub-block and feature flag information.
> Each display compression engine (DCE) contains dual hard
> slice DSC encoders so both share same base address but with
> its own different sub block address.
Please correct line wrapping. 72-75 is usually the preferred width
>
> Signed-off-by: Abhinav Kumar <[email protected]>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19 +++++++++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21 +++++++++++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19 +++++++++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19 +++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++--
> 6 files changed, 99 insertions(+), 2 deletions(-)
>
[I commented on sm8550, it applies to all the rest of platforms]
> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
> };
>
> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
> + .enc = {.base = 0x100, .len = 0x100},
> + .ctl = {.base = 0xF00, .len = 0x10},
> +};
> +
> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
> + .enc = {.base = 0x200, .len = 0x100},
> + .ctl = {.base = 0xF80, .len = 0x10},
> +};
Please keep sblk in dpu_hw_catalog for now.
> +
> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8550_dsc_sblk_0),
> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8550_dsc_sblk_1),
Is there a reason why index in "dsc_N" doesn't match the DSC_n which
comes next to it?
> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100, BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24, 25),
> /* TODO TE sub-blocks for intf1 & intf2 */
> @@ -218,6 +235,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 03f162a..be08158 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__
> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> {\
> .name = _name, .id = _id, \
> .base = _base, .len = 0x140, \
> - .features = _features, \
> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
> + }
> +
> +#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, \
> }
>
> /*************************************************************
--
With best wishes
Dmitry
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> At current implementation, topology configuration is thrown away after
> dpu_rm_reserve(). This patch save the topology so that it can be used
> for DSC related calculation later.
Please take a look at
https://patchwork.freedesktop.org/patch/527960/?series=115283&rev=2 .
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32 ++++++++++++++---------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ecb87bc..2fdacf1 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
> return (num_dsc > 0) && (num_dsc > intf_count);
> }
>
> -static struct msm_display_topology dpu_encoder_get_topology(
> +static void dpu_encoder_get_topology(
> struct dpu_encoder_virt *dpu_enc,
> struct dpu_kms *dpu_kms,
> struct drm_display_mode *mode,
> - struct drm_crtc_state *crtc_state)
> + struct drm_crtc_state *crtc_state,
> + struct msm_display_topology *topology)
> {
> - struct msm_display_topology topology = {0};
> int i, intf_count = 0;
>
> for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
> @@ -567,16 +567,16 @@ static struct msm_display_topology dpu_encoder_get_topology(
> * Add dspps to the reservation requirements if ctm is requested
> */
> if (intf_count == 2)
> - topology.num_lm = 2;
> + topology->num_lm = 2;
> else if (!dpu_kms->catalog->caps->has_3d_merge)
> - topology.num_lm = 1;
> + topology->num_lm = 1;
> else
> - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
> + topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
>
> if (crtc_state->ctm)
> - topology.num_dspp = topology.num_lm;
> + topology->num_dspp = topology->num_lm;
>
> - topology.num_intf = intf_count;
> + topology->num_intf = intf_count;
>
> if (dpu_enc->dsc) {
> /*
> @@ -585,12 +585,10 @@ static struct msm_display_topology dpu_encoder_get_topology(
> * this is power optimal and can drive up to (including) 4k
> * screens
> */
> - topology.num_dsc = 2;
> - topology.num_lm = 2;
> - topology.num_intf = 1;
> + topology->num_dsc = 2;
> + topology->num_lm = 2;
> + topology->num_intf = 1;
> }
> -
> - return topology;
> }
>
> static int dpu_encoder_virt_atomic_check(
> @@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
> struct msm_drm_private *priv;
> struct dpu_kms *dpu_kms;
> struct drm_display_mode *adj_mode;
> - struct msm_display_topology topology;
> + struct msm_display_topology *topology;
> struct dpu_global_state *global_state;
> int i = 0;
> int ret = 0;
> @@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
> }
> }
>
> - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state);
> + topology = &dpu_enc->topology;
> + memset(topology, 0, sizeof (*topology));
> + dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state, topology);
>
> /*
> * Release and Allocate resources on every modeset
> @@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
>
> if (!crtc_state->active_changed || crtc_state->enable)
> ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
> - drm_enc, crtc_state, topology);
> + drm_enc, crtc_state, *topology);
> }
>
> trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
--
With best wishes
Dmitry
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> During DSC preparation, add run time calculation to figure out what
> usage modes, split mode and merge mode, is going to be setup.
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56 ++++++++++++++++-------------
> 1 file changed, 32 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 2fdacf1..5677728 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
> {
> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> - int i, intf_count = 0, num_dsc = 0;
> + struct msm_display_topology *topology = &dpu_enc->topology;
>
> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
> - if (dpu_enc->phys_encs[i])
> - intf_count++;
> -
> - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */
> - if (dpu_enc->dsc)
> - num_dsc = 2;
> -
> - return (num_dsc > 0) && (num_dsc > intf_count);
> + return (topology->num_dsc > topology->num_intf);
> }
>
> static void dpu_encoder_get_topology(
> @@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> + struct msm_display_topology *topology = &dpu_enc->topology;
> int this_frame_slices;
> int intf_ip_w, enc_ip_w;
> - int dsc_common_mode;
> + int dsc_common_mode = 0;
> int pic_width;
> u32 initial_lines;
> + int num_dsc, num_intf;
> 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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
> - return;
> - }
Why?
> }
>
> - dsc_common_mode = 0;
> + num_dsc = topology->num_dsc;
> + num_intf = topology->num_intf;
> +
> pic_width = dsc->pic_width;
>
> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
> if (enc_master->intf_mode == INTF_MODE_VIDEO)
> dsc_common_mode |= DSC_MODE_VIDEO;
>
> + /*
> + * If this encoder is driving more than one DSC encoder, they
> + * operate in tandem, same pic dimension needs to be used by
> + * each of them.(pp-split is assumed to be not supported)
> + *
> + */
> +
> this_frame_slices = pic_width / dsc->slice_width;
> intf_ip_w = this_frame_slices * dsc->slice_width;
> + enc_ip_w = intf_ip_w;
> +
> + intf_ip_w /= num_intf;
> +
> + if (num_dsc > 1)
> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
> +
> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
> + dsc_common_mode |= DSC_MODE_MULTIPLEX;
> + /*
> + * in dsc merge case: when using 2 encoders for the same
> + * stream, no. of slices need to be same on both the
> + * encoders.
> + */
> + enc_ip_w = intf_ip_w / 2;
So do you want to get enc_ip_w / 2 or enc_ip_w / num_intf / 2 here?
> + }
>
> - /*
> - * dsc merge case: when using 2 encoders for the same stream,
> - * no. of slices need to be same on both the encoders.
> - */
> - enc_ip_w = intf_ip_w / 2;
> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>
> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> + for (i = 0; i < num_dsc; i++)
> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
> dsc_common_mode, initial_lines);
> }
--
With best wishes
Dmitry
On 4/20/2023 5:12 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>> At current implementation, topology configuration is thrown away after
>> dpu_rm_reserve(). This patch save the topology so that it can be used
>> for DSC related calculation later.
>
> Please take a look at
> https://patchwork.freedesktop.org/patch/527960/?series=115283&rev=2 .
Let the review of this series go on and lets try to get the acks on the
non-topology related pieces. I think 2/5 patches in this series are
conflicting in the design. We will resolve that in a weeks time.
Meanwhile, I think we can keep the reviews / versions going on the rest.
I think we can move patch 5 of this series to patch 3. That way we get
acks on patches 1-3 and patches 4 & 5 which deal with topology are dealt
with together with virtual planes.
I will review virtual planes next week and we will decide the best
course of action. Moving resource allocation to CRTC needs to be thought
of a bit deeper for DSC as that one is directly tied to encoder.
>
>>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 32
>> ++++++++++++++---------------
>> 1 file changed, 16 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index ecb87bc..2fdacf1 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -542,13 +542,13 @@ bool dpu_encoder_use_dsc_merge(struct
>> drm_encoder *drm_enc)
>> return (num_dsc > 0) && (num_dsc > intf_count);
>> }
>> -static struct msm_display_topology dpu_encoder_get_topology(
>> +static void dpu_encoder_get_topology(
>> struct dpu_encoder_virt *dpu_enc,
>> struct dpu_kms *dpu_kms,
>> struct drm_display_mode *mode,
>> - struct drm_crtc_state *crtc_state)
>> + struct drm_crtc_state *crtc_state,
>> + struct msm_display_topology *topology)
>> {
>> - struct msm_display_topology topology = {0};
>> int i, intf_count = 0;
>> for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>> @@ -567,16 +567,16 @@ static struct msm_display_topology
>> dpu_encoder_get_topology(
>> * Add dspps to the reservation requirements if ctm is requested
>> */
>> if (intf_count == 2)
>> - topology.num_lm = 2;
>> + topology->num_lm = 2;
>> else if (!dpu_kms->catalog->caps->has_3d_merge)
>> - topology.num_lm = 1;
>> + topology->num_lm = 1;
>> else
>> - topology.num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2 : 1;
>> + topology->num_lm = (mode->hdisplay > MAX_HDISPLAY_SPLIT) ? 2
>> : 1;
>> if (crtc_state->ctm)
>> - topology.num_dspp = topology.num_lm;
>> + topology->num_dspp = topology->num_lm;
>> - topology.num_intf = intf_count;
>> + topology->num_intf = intf_count;
>> if (dpu_enc->dsc) {
>> /*
>> @@ -585,12 +585,10 @@ static struct msm_display_topology
>> dpu_encoder_get_topology(
>> * this is power optimal and can drive up to (including) 4k
>> * screens
>> */
>> - topology.num_dsc = 2;
>> - topology.num_lm = 2;
>> - topology.num_intf = 1;
>> + topology->num_dsc = 2;
>> + topology->num_lm = 2;
>> + topology->num_intf = 1;
>> }
>> -
>> - return topology;
>> }
>> static int dpu_encoder_virt_atomic_check(
>> @@ -602,7 +600,7 @@ static int dpu_encoder_virt_atomic_check(
>> struct msm_drm_private *priv;
>> struct dpu_kms *dpu_kms;
>> struct drm_display_mode *adj_mode;
>> - struct msm_display_topology topology;
>> + struct msm_display_topology *topology;
>> struct dpu_global_state *global_state;
>> int i = 0;
>> int ret = 0;
>> @@ -639,7 +637,9 @@ static int dpu_encoder_virt_atomic_check(
>> }
>> }
>> - topology = dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode,
>> crtc_state);
>> + topology = &dpu_enc->topology;
>> + memset(topology, 0, sizeof (*topology));
>> + dpu_encoder_get_topology(dpu_enc, dpu_kms, adj_mode, crtc_state,
>> topology);
>> /*
>> * Release and Allocate resources on every modeset
>> @@ -650,7 +650,7 @@ static int dpu_encoder_virt_atomic_check(
>> if (!crtc_state->active_changed || crtc_state->enable)
>> ret = dpu_rm_reserve(&dpu_kms->rm, global_state,
>> - drm_enc, crtc_state, topology);
>> + drm_enc, crtc_state, *topology);
>> }
>> trace_dpu_enc_atomic_check_flags(DRMID(drm_enc), adj_mode->flags);
>
Hi Kuogee,
kernel test robot noticed the following build warnings:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.3-rc7 next-20230420]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dpu-add-support-for-DSC-encoder-v1-2-engine/20230421-072925
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/1682033114-28483-2-git-send-email-quic_khsieh%40quicinc.com
patch subject: [PATCH v1 1/5] drm/msm/dpu: add support for DSC encoder v1.2 engine
config: sparc-allyesconfig (https://download.01.org/0day-ci/archive/20230421/[email protected]/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/1c3eede9e4f8fc63f52eddb0c55f63d59fad4b68
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kuogee-Hsieh/drm-msm-dpu-add-support-for-DSC-encoder-v1-2-engine/20230421-072925
git checkout 1c3eede9e4f8fc63f52eddb0c55f63d59fad4b68
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sparc SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c: In function 'dpu_hw_dsc_config_1_2':
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c:227:31: error: implicit declaration of function 'drm_dsc_calculate_flatness_det_thresh' [-Werror=implicit-function-declaration]
227 | det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c:124:23: warning: variable 'off' set but not used [-Wunused-but-set-variable]
124 | void __iomem *off;
| ^~~
cc1: some warnings being treated as errors
vim +/off +124 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
112
113 static void dpu_hw_dsc_config_1_2(struct dpu_hw_dsc *hw_dsc,
114 struct drm_dsc_config *dsc,
115 u32 mode,
116 u32 initial_lines)
117 {
118 struct dpu_hw_blk_reg_map *hw;
119 u32 offset;
120 u32 data = 0;
121 u32 det_thresh_flatness;
122 u32 num_active_ss_per_enc;
123 u32 bpp;
> 124 void __iomem *off;
125
126 if (!hw_dsc || !dsc)
127 return;
128
129 hw = &hw_dsc->hw;
130
131 _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
132
133 if (mode & DSC_MODE_SPLIT_PANEL)
134 data |= BIT(0);
135
136 if (mode & DSC_MODE_MULTIPLEX)
137 data |= BIT(1);
138
139 num_active_ss_per_enc = dsc->slice_count;
140 if (mode & DSC_MODE_MULTIPLEX)
141 num_active_ss_per_enc = dsc->slice_count >> 1;
142
143 data |= (num_active_ss_per_enc & 0x3) << 7;
144
145 DPU_REG_WRITE(hw, DSC_CMN_MAIN_CNF, data);
146
147 data = (initial_lines & 0xff);
148
149 if (mode & DSC_MODE_VIDEO)
150 data |= BIT(9);
151
152 data |= (_dsc_calc_ob_max_addr(hw_dsc, num_active_ss_per_enc) << 18);
153
154 DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, data);
155
156 data = (dsc->dsc_version_minor & 0xf) << 28;
157 if (dsc->dsc_version_minor == 0x2) {
158 if (dsc->native_422)
159 data |= BIT(22);
160 if (dsc->native_420)
161 data |= BIT(21);
162 }
163
164 bpp = dsc->bits_per_pixel;
165 /* as per hw requirement bpp should be programmed
166 * twice the actual value in case of 420 or 422 encoding
167 */
168 if (dsc->native_422 || dsc->native_420)
169 bpp = 2 * bpp;
170 data |= (dsc->block_pred_enable ? 1 : 0) << 20;
171 data |= bpp << 10;
172 data |= (dsc->line_buf_depth & 0xf) << 6;
173 data |= dsc->convert_rgb << 4;
174 data |= dsc->bits_per_component & 0xf;
175
176 DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, data);
177
178 data = (dsc->pic_width & 0xffff) |
179 ((dsc->pic_height & 0xffff) << 16);
180
181 DPU_REG_WRITE(hw, offset + DSC_PICTURE_SIZE, data);
182
183 data = (dsc->slice_width & 0xffff) |
184 ((dsc->slice_height & 0xffff) << 16);
185
186 DPU_REG_WRITE(hw, offset + DSC_SLICE_SIZE, data);
187
188 DPU_REG_WRITE(hw, offset + DSC_MISC_SIZE,
189 (dsc->slice_chunk_size) & 0xffff);
190
191 data = (dsc->initial_xmit_delay & 0xffff) |
192 ((dsc->initial_dec_delay & 0xffff) << 16);
193
194 DPU_REG_WRITE(hw, offset + DSC_HRD_DELAYS, data);
195
196 DPU_REG_WRITE(hw, offset + DSC_RC_SCALE,
197 dsc->initial_scale_value & 0x3f);
198
199 data = (dsc->scale_increment_interval & 0xffff) |
200 ((dsc->scale_decrement_interval & 0x7ff) << 16);
201
202 DPU_REG_WRITE(hw, offset + DSC_RC_SCALE_INC_DEC, data);
203
204 data = (dsc->first_line_bpg_offset & 0x1f) |
205 ((dsc->second_line_bpg_offset & 0x1f) << 5);
206
207 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_1, data);
208
209 data = (dsc->nfl_bpg_offset & 0xffff) |
210 ((dsc->slice_bpg_offset & 0xffff) << 16);
211
212 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_2, data);
213
214 data = (dsc->initial_offset & 0xffff) |
215 ((dsc->final_offset & 0xffff) << 16);
216
217 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_3, data);
218
219 data = (dsc->nsl_bpg_offset & 0xffff) |
220 ((dsc->second_line_offset_adj & 0xffff) << 16);
221
222 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_4, data);
223
224 data = (dsc->flatness_min_qp & 0x1f);
225 data |= (dsc->flatness_max_qp & 0x1f) << 5;
226
227 det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
228 data |= (det_thresh_flatness & 0xff) << 10;
229
230 DPU_REG_WRITE(hw, offset + DSC_FLATNESS_QP, data);
231
232 DPU_REG_WRITE(hw, offset + DSC_RC_MODEL_SIZE,
233 (dsc->rc_model_size) & 0xffff);
234
235 data = dsc->rc_edge_factor & 0xf;
236 data |= (dsc->rc_quant_incr_limit0 & 0x1f) << 8;
237 data |= (dsc->rc_quant_incr_limit1 & 0x1f) << 13;
238 data |= (dsc->rc_tgt_offset_high & 0xf) << 20;
239 data |= (dsc->rc_tgt_offset_low & 0xf) << 24;
240
241 DPU_REG_WRITE(hw, offset + DSC_RC_CONFIG, data);
242
243 /* program the dsc wrapper */
244 _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
245
246 off = hw->blk_addr + offset;
247
248 data = BIT(0); /* encoder enable */
249 if (dsc->native_422)
250 data |= BIT(8);
251 else if (dsc->native_420)
252 data |= BIT(9);
253 if (!dsc->convert_rgb)
254 data |= BIT(10);
255 if (dsc->bits_per_component == 8)
256 data |= BIT(11);
257 if (mode & DSC_MODE_SPLIT_PANEL)
258 data |= BIT(12);
259 if (mode & DSC_MODE_MULTIPLEX)
260 data |= BIT(13);
261 if (!(mode & DSC_MODE_VIDEO))
262 data |= BIT(17);
263
264 DPU_REG_WRITE(hw, offset + DSC_CFG, data);
265 }
266
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
Hi Kuogee,
kernel test robot noticed the following build errors:
[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.3-rc7 next-20230420]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kuogee-Hsieh/drm-msm-dpu-add-support-for-DSC-encoder-v1-2-engine/20230421-072925
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link: https://lore.kernel.org/r/1682033114-28483-2-git-send-email-quic_khsieh%40quicinc.com
patch subject: [PATCH v1 1/5] drm/msm/dpu: add support for DSC encoder v1.2 engine
config: riscv-randconfig-r013-20230421 (https://download.01.org/0day-ci/archive/20230422/[email protected]/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 437b7602e4a998220871de78afcb020b9c14a661)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/1c3eede9e4f8fc63f52eddb0c55f63d59fad4b68
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kuogee-Hsieh/drm-msm-dpu-add-support-for-DSC-encoder-v1-2-engine/20230421-072925
git checkout 1c3eede9e4f8fc63f52eddb0c55f63d59fad4b68
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/gpu/drm/msm/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All error/warnings (new ones prefixed by >>):
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c:227:24: error: call to undeclared function 'drm_dsc_calculate_flatness_det_thresh'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
^
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c:124:16: warning: variable 'off' set but not used [-Wunused-but-set-variable]
void __iomem *off;
^
1 warning and 1 error generated.
vim +/drm_dsc_calculate_flatness_det_thresh +227 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
112
113 static void dpu_hw_dsc_config_1_2(struct dpu_hw_dsc *hw_dsc,
114 struct drm_dsc_config *dsc,
115 u32 mode,
116 u32 initial_lines)
117 {
118 struct dpu_hw_blk_reg_map *hw;
119 u32 offset;
120 u32 data = 0;
121 u32 det_thresh_flatness;
122 u32 num_active_ss_per_enc;
123 u32 bpp;
> 124 void __iomem *off;
125
126 if (!hw_dsc || !dsc)
127 return;
128
129 hw = &hw_dsc->hw;
130
131 _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
132
133 if (mode & DSC_MODE_SPLIT_PANEL)
134 data |= BIT(0);
135
136 if (mode & DSC_MODE_MULTIPLEX)
137 data |= BIT(1);
138
139 num_active_ss_per_enc = dsc->slice_count;
140 if (mode & DSC_MODE_MULTIPLEX)
141 num_active_ss_per_enc = dsc->slice_count >> 1;
142
143 data |= (num_active_ss_per_enc & 0x3) << 7;
144
145 DPU_REG_WRITE(hw, DSC_CMN_MAIN_CNF, data);
146
147 data = (initial_lines & 0xff);
148
149 if (mode & DSC_MODE_VIDEO)
150 data |= BIT(9);
151
152 data |= (_dsc_calc_ob_max_addr(hw_dsc, num_active_ss_per_enc) << 18);
153
154 DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, data);
155
156 data = (dsc->dsc_version_minor & 0xf) << 28;
157 if (dsc->dsc_version_minor == 0x2) {
158 if (dsc->native_422)
159 data |= BIT(22);
160 if (dsc->native_420)
161 data |= BIT(21);
162 }
163
164 bpp = dsc->bits_per_pixel;
165 /* as per hw requirement bpp should be programmed
166 * twice the actual value in case of 420 or 422 encoding
167 */
168 if (dsc->native_422 || dsc->native_420)
169 bpp = 2 * bpp;
170 data |= (dsc->block_pred_enable ? 1 : 0) << 20;
171 data |= bpp << 10;
172 data |= (dsc->line_buf_depth & 0xf) << 6;
173 data |= dsc->convert_rgb << 4;
174 data |= dsc->bits_per_component & 0xf;
175
176 DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, data);
177
178 data = (dsc->pic_width & 0xffff) |
179 ((dsc->pic_height & 0xffff) << 16);
180
181 DPU_REG_WRITE(hw, offset + DSC_PICTURE_SIZE, data);
182
183 data = (dsc->slice_width & 0xffff) |
184 ((dsc->slice_height & 0xffff) << 16);
185
186 DPU_REG_WRITE(hw, offset + DSC_SLICE_SIZE, data);
187
188 DPU_REG_WRITE(hw, offset + DSC_MISC_SIZE,
189 (dsc->slice_chunk_size) & 0xffff);
190
191 data = (dsc->initial_xmit_delay & 0xffff) |
192 ((dsc->initial_dec_delay & 0xffff) << 16);
193
194 DPU_REG_WRITE(hw, offset + DSC_HRD_DELAYS, data);
195
196 DPU_REG_WRITE(hw, offset + DSC_RC_SCALE,
197 dsc->initial_scale_value & 0x3f);
198
199 data = (dsc->scale_increment_interval & 0xffff) |
200 ((dsc->scale_decrement_interval & 0x7ff) << 16);
201
202 DPU_REG_WRITE(hw, offset + DSC_RC_SCALE_INC_DEC, data);
203
204 data = (dsc->first_line_bpg_offset & 0x1f) |
205 ((dsc->second_line_bpg_offset & 0x1f) << 5);
206
207 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_1, data);
208
209 data = (dsc->nfl_bpg_offset & 0xffff) |
210 ((dsc->slice_bpg_offset & 0xffff) << 16);
211
212 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_2, data);
213
214 data = (dsc->initial_offset & 0xffff) |
215 ((dsc->final_offset & 0xffff) << 16);
216
217 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_3, data);
218
219 data = (dsc->nsl_bpg_offset & 0xffff) |
220 ((dsc->second_line_offset_adj & 0xffff) << 16);
221
222 DPU_REG_WRITE(hw, offset + DSC_RC_OFFSETS_4, data);
223
224 data = (dsc->flatness_min_qp & 0x1f);
225 data |= (dsc->flatness_max_qp & 0x1f) << 5;
226
> 227 det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
228 data |= (det_thresh_flatness & 0xff) << 10;
229
230 DPU_REG_WRITE(hw, offset + DSC_FLATNESS_QP, data);
231
232 DPU_REG_WRITE(hw, offset + DSC_RC_MODEL_SIZE,
233 (dsc->rc_model_size) & 0xffff);
234
235 data = dsc->rc_edge_factor & 0xf;
236 data |= (dsc->rc_quant_incr_limit0 & 0x1f) << 8;
237 data |= (dsc->rc_quant_incr_limit1 & 0x1f) << 13;
238 data |= (dsc->rc_tgt_offset_high & 0xf) << 20;
239 data |= (dsc->rc_tgt_offset_low & 0xf) << 24;
240
241 DPU_REG_WRITE(hw, offset + DSC_RC_CONFIG, data);
242
243 /* program the dsc wrapper */
244 _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
245
246 off = hw->blk_addr + offset;
247
248 data = BIT(0); /* encoder enable */
249 if (dsc->native_422)
250 data |= BIT(8);
251 else if (dsc->native_420)
252 data |= BIT(9);
253 if (!dsc->convert_rgb)
254 data |= BIT(10);
255 if (dsc->bits_per_component == 8)
256 data |= BIT(11);
257 if (mode & DSC_MODE_SPLIT_PANEL)
258 data |= BIT(12);
259 if (mode & DSC_MODE_MULTIPLEX)
260 data |= BIT(13);
261 if (!(mode & DSC_MODE_VIDEO))
262 data |= BIT(17);
263
264 DPU_REG_WRITE(hw, offset + DSC_CFG, data);
265 }
266
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
On 4/20/2023 5:27 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>> During DSC preparation, add run time calculation to figure out what
>> usage modes, split mode and merge mode, is going to be setup.
>>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> ---
>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56
>> ++++++++++++++++-------------
>> 1 file changed, 32 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index 2fdacf1..5677728 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
>> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>> {
>> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>> - int i, intf_count = 0, num_dsc = 0;
>> + struct msm_display_topology *topology = &dpu_enc->topology;
>> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>> - if (dpu_enc->phys_encs[i])
>> - intf_count++;
>> -
>> - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */
>> - if (dpu_enc->dsc)
>> - num_dsc = 2;
>> -
>> - return (num_dsc > 0) && (num_dsc > intf_count);
>> + return (topology->num_dsc > topology->num_intf);
>> }
>> static void dpu_encoder_get_topology(
>> @@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct
>> dpu_encoder_virt *dpu_enc,
>> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
>> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>> + struct msm_display_topology *topology = &dpu_enc->topology;
>> int this_frame_slices;
>> int intf_ip_w, enc_ip_w;
>> - int dsc_common_mode;
>> + int dsc_common_mode = 0;
>> int pic_width;
>> u32 initial_lines;
>> + int num_dsc, num_intf;
>> 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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
>> - return;
>> - }
>
Why?
MAX_CHANNELS_PER_ENC == 2
This works for dsi since it use 2 dsc encoder.
Since DP only use one dsc encoder, this will cause it return at loop 2
without execute dpu_encoder_dsc_pipe_cfg().
>
>> }
>> - dsc_common_mode = 0;
>> + num_dsc = topology->num_dsc;
>> + num_intf = topology->num_intf;
>> +
>> pic_width = dsc->pic_width;
>> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
>> if (enc_master->intf_mode == INTF_MODE_VIDEO)
>> dsc_common_mode |= DSC_MODE_VIDEO;
>> + /*
>> + * If this encoder is driving more than one DSC encoder, they
>> + * operate in tandem, same pic dimension needs to be used by
>> + * each of them.(pp-split is assumed to be not supported)
>> + *
>> + */
>> +
>> this_frame_slices = pic_width / dsc->slice_width;
>> intf_ip_w = this_frame_slices * dsc->slice_width;
>> + enc_ip_w = intf_ip_w;
>> +
>> + intf_ip_w /= num_intf;
>> +
>> + if (num_dsc > 1)
>> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
>> +
>> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
>> + dsc_common_mode |= DSC_MODE_MULTIPLEX;
>> + /*
>> + * in dsc merge case: when using 2 encoders for the same
>> + * stream, no. of slices need to be same on both the
>> + * encoders.
>> + */
>> + enc_ip_w = intf_ip_w / 2;
>
> So do you want to get enc_ip_w / 2 or enc_ip_w / num_intf / 2 here?
enc_ip_w / num_intf / 2
>
>> + }
>> - /*
>> - * dsc merge case: when using 2 encoders for the same stream,
>> - * no. of slices need to be same on both the encoders.
>> - */
>> - enc_ip_w = intf_ip_w / 2;
>> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>> + for (i = 0; i < num_dsc; i++)
>> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
>> dsc_common_mode, initial_lines);
>> }
>
On 22/04/2023 00:07, Kuogee Hsieh wrote:
>
> On 4/20/2023 5:27 PM, Dmitry Baryshkov wrote:
>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>> During DSC preparation, add run time calculation to figure out what
>>> usage modes, split mode and merge mode, is going to be setup.
>>>
>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>> ---
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56
>>> ++++++++++++++++-------------
>>> 1 file changed, 32 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> index 2fdacf1..5677728 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
>>> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>>> {
>>> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>>> - int i, intf_count = 0, num_dsc = 0;
>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>>> - if (dpu_enc->phys_encs[i])
>>> - intf_count++;
>>> -
>>> - /* See dpu_encoder_get_topology, we only support 2:2:1 topology */
>>> - if (dpu_enc->dsc)
>>> - num_dsc = 2;
>>> -
>>> - return (num_dsc > 0) && (num_dsc > intf_count);
>>> + return (topology->num_dsc > topology->num_intf);
>>> }
>>> static void dpu_encoder_get_topology(
>>> @@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct
>>> dpu_encoder_virt *dpu_enc,
>>> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
>>> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>>> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>> int this_frame_slices;
>>> int intf_ip_w, enc_ip_w;
>>> - int dsc_common_mode;
>>> + int dsc_common_mode = 0;
>>> int pic_width;
>>> u32 initial_lines;
>>> + int num_dsc, num_intf;
>>> 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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
>>> - return;
>>> - }
>>
> Why?
>
> MAX_CHANNELS_PER_ENC == 2
>
> This works for dsi since it use 2 dsc encoder.
>
> Since DP only use one dsc encoder, this will cause it return at loop 2
> without execute dpu_encoder_dsc_pipe_cfg().
Then the loop should go up to num_dsc rather than MAX_CHANNELS_PER_ENC
>
>>
>>> }
>>> - dsc_common_mode = 0;
>>> + num_dsc = topology->num_dsc;
>>> + num_intf = topology->num_intf;
>>> +
>>> pic_width = dsc->pic_width;
>>> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
>>> if (enc_master->intf_mode == INTF_MODE_VIDEO)
>>> dsc_common_mode |= DSC_MODE_VIDEO;
>>> + /*
>>> + * If this encoder is driving more than one DSC encoder, they
>>> + * operate in tandem, same pic dimension needs to be used by
>>> + * each of them.(pp-split is assumed to be not supported)
>>> + *
>>> + */
>>> +
>>> this_frame_slices = pic_width / dsc->slice_width;
>>> intf_ip_w = this_frame_slices * dsc->slice_width;
>>> + enc_ip_w = intf_ip_w;
>>> +
>>> + intf_ip_w /= num_intf;
>>> +
>>> + if (num_dsc > 1)
>>> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
>>> +
>>> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
>>> + dsc_common_mode |= DSC_MODE_MULTIPLEX;
>>> + /*
>>> + * in dsc merge case: when using 2 encoders for the same
>>> + * stream, no. of slices need to be same on both the
>>> + * encoders.
>>> + */
>>> + enc_ip_w = intf_ip_w / 2;
>>
>> So do you want to get enc_ip_w / 2 or enc_ip_w / num_intf / 2 here?
> enc_ip_w / num_intf / 2
But previously we had enc_ip_w = intf_ip_w / 2. Was it because of the
assumption that num_intf = 1?
>>
>>> + }
>>> - /*
>>> - * dsc merge case: when using 2 encoders for the same stream,
>>> - * no. of slices need to be same on both the encoders.
>>> - */
>>> - enc_ip_w = intf_ip_w / 2;
>>> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>>> + for (i = 0; i < num_dsc; i++)
>>> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
>>> dsc_common_mode, initial_lines);
>>> }
>>
--
With best wishes
Dmitry
On 4/21/2023 2:13 PM, Dmitry Baryshkov wrote:
> On 22/04/2023 00:07, Kuogee Hsieh wrote:
>>
>> On 4/20/2023 5:27 PM, Dmitry Baryshkov wrote:
>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>> During DSC preparation, add run time calculation to figure out what
>>>> usage modes, split mode and merge mode, is going to be setup.
>>>>
>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>> ---
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56
>>>> ++++++++++++++++-------------
>>>> 1 file changed, 32 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> index 2fdacf1..5677728 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
>>>> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>>>> {
>>>> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>>>> - int i, intf_count = 0, num_dsc = 0;
>>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>>> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>>>> - if (dpu_enc->phys_encs[i])
>>>> - intf_count++;
>>>> -
>>>> - /* See dpu_encoder_get_topology, we only support 2:2:1
>>>> topology */
>>>> - if (dpu_enc->dsc)
>>>> - num_dsc = 2;
>>>> -
>>>> - return (num_dsc > 0) && (num_dsc > intf_count);
>>>> + return (topology->num_dsc > topology->num_intf);
>>>> }
>>>> static void dpu_encoder_get_topology(
>>>> @@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct
>>>> dpu_encoder_virt *dpu_enc,
>>>> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
>>>> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>>>> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>>> int this_frame_slices;
>>>> int intf_ip_w, enc_ip_w;
>>>> - int dsc_common_mode;
>>>> + int dsc_common_mode = 0;
>>>> int pic_width;
>>>> u32 initial_lines;
>>>> + int num_dsc, num_intf;
>>>> 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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
>>>> - return;
>>>> - }
>>>
>> Why?
>>
>> MAX_CHANNELS_PER_ENC == 2
>>
>> This works for dsi since it use 2 dsc encoder.
>>
>> Since DP only use one dsc encoder, this will cause it return at loop
>> 2 without execute dpu_encoder_dsc_pipe_cfg().
>
> Then the loop should go up to num_dsc rather than MAX_CHANNELS_PER_ENC
>
>>
>>>
>>>> }
>>>> - dsc_common_mode = 0;
>>>> + num_dsc = topology->num_dsc;
>>>> + num_intf = topology->num_intf;
>>>> +
>>>> pic_width = dsc->pic_width;
>>>> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
>>>> if (enc_master->intf_mode == INTF_MODE_VIDEO)
>>>> dsc_common_mode |= DSC_MODE_VIDEO;
>>>> + /*
>>>> + * If this encoder is driving more than one DSC encoder, they
>>>> + * operate in tandem, same pic dimension needs to be used by
>>>> + * each of them.(pp-split is assumed to be not supported)
>>>> + *
>>>> + */
>>>> +
>>>> this_frame_slices = pic_width / dsc->slice_width;
>>>> intf_ip_w = this_frame_slices * dsc->slice_width;
>>>> + enc_ip_w = intf_ip_w;
>>>> +
>>>> + intf_ip_w /= num_intf;
>>>> +
>>>> + if (num_dsc > 1)
>>>> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
>>>> +
>>>> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
>>>> + dsc_common_mode |= DSC_MODE_MULTIPLEX;
>>>> + /*
>>>> + * in dsc merge case: when using 2 encoders for the same
>>>> + * stream, no. of slices need to be same on both the
>>>> + * encoders.
>>>> + */
>>>> + enc_ip_w = intf_ip_w / 2;
>>>
>>> So do you want to get enc_ip_w / 2 or enc_ip_w / num_intf / 2 here?
>> enc_ip_w / num_intf / 2
>
> But previously we had enc_ip_w = intf_ip_w / 2. Was it because of the
> assumption that num_intf = 1?
i think so since there is no num_intf involve at previous code.
>
>>>
>>>> + }
>>>> - /*
>>>> - * dsc merge case: when using 2 encoders for the same stream,
>>>> - * no. of slices need to be same on both the encoders.
>>>> - */
>>>> - enc_ip_w = intf_ip_w / 2;
>>>> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc,
>>>> enc_ip_w);
>>>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>>>> + for (i = 0; i < num_dsc; i++)
>>>> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
>>>> dsc_common_mode, initial_lines);
>>>> }
>>>
>
On 22/04/2023 00:30, Kuogee Hsieh wrote:
>
> On 4/21/2023 2:13 PM, Dmitry Baryshkov wrote:
>> On 22/04/2023 00:07, Kuogee Hsieh wrote:
>>>
>>> On 4/20/2023 5:27 PM, Dmitry Baryshkov wrote:
>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>>> During DSC preparation, add run time calculation to figure out what
>>>>> usage modes, split mode and merge mode, is going to be setup.
>>>>>
>>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>>> ---
>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 56
>>>>> ++++++++++++++++-------------
>>>>> 1 file changed, 32 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>>> index 2fdacf1..5677728 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>>>>> @@ -529,17 +529,9 @@ void dpu_encoder_helper_split_config(
>>>>> bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
>>>>> {
>>>>> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>>>>> - int i, intf_count = 0, num_dsc = 0;
>>>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>>>> - for (i = 0; i < MAX_PHYS_ENCODERS_PER_VIRTUAL; i++)
>>>>> - if (dpu_enc->phys_encs[i])
>>>>> - intf_count++;
>>>>> -
>>>>> - /* See dpu_encoder_get_topology, we only support 2:2:1
>>>>> topology */
>>>>> - if (dpu_enc->dsc)
>>>>> - num_dsc = 2;
>>>>> -
>>>>> - return (num_dsc > 0) && (num_dsc > intf_count);
>>>>> + return (topology->num_dsc > topology->num_intf);
>>>>> }
>>>>> static void dpu_encoder_get_topology(
>>>>> @@ -1861,41 +1853,57 @@ static void dpu_encoder_prep_dsc(struct
>>>>> dpu_encoder_virt *dpu_enc,
>>>>> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
>>>>> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
>>>>> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
>>>>> + struct msm_display_topology *topology = &dpu_enc->topology;
>>>>> int this_frame_slices;
>>>>> int intf_ip_w, enc_ip_w;
>>>>> - int dsc_common_mode;
>>>>> + int dsc_common_mode = 0;
>>>>> int pic_width;
>>>>> u32 initial_lines;
>>>>> + int num_dsc, num_intf;
>>>>> 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_ERROR_ENC(dpu_enc, "invalid params for DSC\n");
>>>>> - return;
>>>>> - }
>>>>
>>> Why?
>>>
>>> MAX_CHANNELS_PER_ENC == 2
>>>
>>> This works for dsi since it use 2 dsc encoder.
>>>
>>> Since DP only use one dsc encoder, this will cause it return at loop
>>> 2 without execute dpu_encoder_dsc_pipe_cfg().
>>
>> Then the loop should go up to num_dsc rather than MAX_CHANNELS_PER_ENC
>>
>>>
>>>>
>>>>> }
>>>>> - dsc_common_mode = 0;
>>>>> + num_dsc = topology->num_dsc;
>>>>> + num_intf = topology->num_intf;
>>>>> +
>>>>> pic_width = dsc->pic_width;
>>>>> - dsc_common_mode = DSC_MODE_MULTIPLEX | DSC_MODE_SPLIT_PANEL;
>>>>> if (enc_master->intf_mode == INTF_MODE_VIDEO)
>>>>> dsc_common_mode |= DSC_MODE_VIDEO;
>>>>> + /*
>>>>> + * If this encoder is driving more than one DSC encoder, they
>>>>> + * operate in tandem, same pic dimension needs to be used by
>>>>> + * each of them.(pp-split is assumed to be not supported)
>>>>> + *
>>>>> + */
>>>>> +
>>>>> this_frame_slices = pic_width / dsc->slice_width;
>>>>> intf_ip_w = this_frame_slices * dsc->slice_width;
>>>>> + enc_ip_w = intf_ip_w;
>>>>> +
>>>>> + intf_ip_w /= num_intf;
>>>>> +
>>>>> + if (num_dsc > 1)
>>>>> + dsc_common_mode |= DSC_MODE_SPLIT_PANEL;
>>>>> +
>>>>> + if (dpu_encoder_use_dsc_merge(&dpu_enc->base)) {
>>>>> + dsc_common_mode |= DSC_MODE_MULTIPLEX;
>>>>> + /*
>>>>> + * in dsc merge case: when using 2 encoders for the same
>>>>> + * stream, no. of slices need to be same on both the
>>>>> + * encoders.
>>>>> + */
>>>>> + enc_ip_w = intf_ip_w / 2;
>>>>
>>>> So do you want to get enc_ip_w / 2 or enc_ip_w / num_intf / 2 here?
>>> enc_ip_w / num_intf / 2
>>
>> But previously we had enc_ip_w = intf_ip_w / 2. Was it because of the
>> assumption that num_intf = 1?
> i think so since there is no num_intf involve at previous code.
Ack.
>>
>>>>
>>>>> + }
>>>>> - /*
>>>>> - * dsc merge case: when using 2 encoders for the same stream,
>>>>> - * no. of slices need to be same on both the encoders.
>>>>> - */
>>>>> - enc_ip_w = intf_ip_w / 2;
>>>>> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc,
>>>>> enc_ip_w);
>>>>> - for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
>>>>> + for (i = 0; i < num_dsc; i++)
>>>>> dpu_encoder_dsc_pipe_cfg(dpu_enc, hw_dsc[i], hw_pp[i], dsc,
>>>>> dsc_common_mode, initial_lines);
>>>>> }
>>>>
>>
--
With best wishes
Dmitry
On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>> From: Abhinav Kumar <[email protected]>
>>
>> Add DSC 1.2 hardware blocks to the catalog with necessary
>> sub-block and feature flag information.
>> Each display compression engine (DCE) contains dual hard
>> slice DSC encoders so both share same base address but with
>> its own different sub block address.
>
> Please correct line wrapping. 72-75 is usually the preferred width
>
>>
>> Signed-off-by: Abhinav Kumar <[email protected]>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> ---
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>> +++++++++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>> +++++++++++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>> +++++++++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>> +++++++++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++--
>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>
>
>
> [I commented on sm8550, it applies to all the rest of platforms]
>
>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>> sm8550_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>> };
>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>> + .enc = {.base = 0x100, .len = 0x100},
>> + .ctl = {.base = 0xF00, .len = 0x10},
>> +};
>> +
>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>> + .enc = {.base = 0x200, .len = 0x100},
>> + .ctl = {.base = 0xF80, .len = 0x10},
>> +};
>
> Please keep sblk in dpu_hw_catalog for now.
>
>> +
>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8550_dsc_sblk_0),
>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8550_dsc_sblk_1),
>
> Is there a reason why index in "dsc_N" doesn't match the DSC_n which
> comes next to it?
usually each DCE (display compression engine) contains two hard slice
encoders.
DSC_0 and DSC_1 (index) is belong to dsc_0.
If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>
>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24, 25),
>> /* TODO TE sub-blocks for intf1 & intf2 */
>> @@ -218,6 +235,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 03f162a..be08158 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__
>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>> sc7280_pp_sblk = {
>> {\
>> .name = _name, .id = _id, \
>> .base = _base, .len = 0x140, \
>> - .features = _features, \
>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>> + }
>> +
>> +#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, \
>> }
>> /*************************************************************
>
On 22/04/2023 01:05, Kuogee Hsieh wrote:
>
> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>> From: Abhinav Kumar <[email protected]>
>>>
>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>> sub-block and feature flag information.
>>> Each display compression engine (DCE) contains dual hard
>>> slice DSC encoders so both share same base address but with
>>> its own different sub block address.
>>
>> Please correct line wrapping. 72-75 is usually the preferred width
>>
>>>
>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>> ---
>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>> +++++++++++++++++++
>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>> +++++++++++++++++++++
>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>> +++++++++++++++++++
>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>> +++++++++++++++++++
>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++--
>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>
>>
>>
>> [I commented on sm8550, it applies to all the rest of platforms]
>>
>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>> sm8550_merge_3d[] = {
>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>> };
>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>> + .enc = {.base = 0x100, .len = 0x100},
>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>> +};
>>> +
>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>> + .enc = {.base = 0x200, .len = 0x100},
>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>> +};
>>
>> Please keep sblk in dpu_hw_catalog for now.
>>
>>> +
>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0, sm8550_dsc_sblk_0),
>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0, sm8550_dsc_sblk_1),
>>
>> Is there a reason why index in "dsc_N" doesn't match the DSC_n which
>> comes next to it?
>
> usually each DCE (display compression engine) contains two hard slice
> encoders.
>
> DSC_0 and DSC_1 (index) is belong to dsc_0.
>
> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
Ah, I see now. So, the block register space is the following:
DCEi ->
common
dsc0_enc
dsc1_enc
dsc0_ctl
dsc1_ctl
Instead of declaring a single DCE unit with two DSC blocks, we declare
two distinct DSC blocks. This raises a question, how independent are
these two parts of a single DCE block? For example, can we use them to
perform compression with different parameters? Or use one of them for
the DP DSC and another one for DSI DSC? Can we have the following
configuration:
DSC_0 => DP DSC
DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
>
>>
>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24, 25),
>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>> sc7280_pp_sblk = {
>>> {\
>>> .name = _name, .id = _id, \
>>> .base = _base, .len = 0x140, \
>>> - .features = _features, \
>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>> + }
>>> +
>>> +#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, \
>>> }
>>> /*************************************************************
>>
--
With best wishes
Dmitry
On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote:
> On 22/04/2023 01:05, Kuogee Hsieh wrote:
>>
>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>> From: Abhinav Kumar <[email protected]>
>>>>
>>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>>> sub-block and feature flag information.
>>>> Each display compression engine (DCE) contains dual hard
>>>> slice DSC encoders so both share same base address but with
>>>> its own different sub block address.
>>>
>>> Please correct line wrapping. 72-75 is usually the preferred width
>>>
>>>>
>>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>> ---
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>>> +++++++++++++++++++
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>>> +++++++++++++++++++++
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>>> +++++++++++++++++++
>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>>> +++++++++++++++++++
>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12
>>>> ++++++++++--
>>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>>
>>>
>>>
>>> [I commented on sm8550, it applies to all the rest of platforms]
>>>
>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>>> sm8550_merge_3d[] = {
>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>>> };
>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>>> + .enc = {.base = 0x100, .len = 0x100},
>>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>>> +};
>>>> +
>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>>> + .enc = {.base = 0x200, .len = 0x100},
>>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>>> +};
>>>
>>> Please keep sblk in dpu_hw_catalog for now.
>>>
>>>> +
>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0,
>>>> sm8550_dsc_sblk_0),
>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0,
>>>> sm8550_dsc_sblk_1),
>>>
>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n which
>>> comes next to it?
>>
>> usually each DCE (display compression engine) contains two hard slice
>> encoders.
>>
>> DSC_0 and DSC_1 (index) is belong to dsc_0.
>>
>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>
> Ah, I see now. So, the block register space is the following:
> DCEi ->
> common
> dsc0_enc
> dsc1_enc
> dsc0_ctl
> dsc1_ctl
>
> Instead of declaring a single DCE unit with two DSC blocks, we declare
> two distinct DSC blocks. This raises a question, how independent are
> these two parts of a single DCE block? For example, can we use them to
> perform compression with different parameters? Or use one of them for
> the DP DSC and another one for DSI DSC? Can we have the following
> configuration:
>
> DSC_0 => DP DSC
> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
no, For merge mode you have to use same DCE, such as DSC_2 and DSC3 (pair)
>
>>
>>>
>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24,
>>>> 25),
>>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>>> sc7280_pp_sblk = {
>>>> {\
>>>> .name = _name, .id = _id, \
>>>> .base = _base, .len = 0x140, \
>>>> - .features = _features, \
>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>>> + }
>>>> +
>>>> +#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, \
>>>> }
>>>> /*************************************************************
>>>
>
On 22/04/2023 02:08, Kuogee Hsieh wrote:
>
> On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote:
>> On 22/04/2023 01:05, Kuogee Hsieh wrote:
>>>
>>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>>> From: Abhinav Kumar <[email protected]>
>>>>>
>>>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>>>> sub-block and feature flag information.
>>>>> Each display compression engine (DCE) contains dual hard
>>>>> slice DSC encoders so both share same base address but with
>>>>> its own different sub block address.
>>>>
>>>> Please correct line wrapping. 72-75 is usually the preferred width
>>>>
>>>>>
>>>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>>> ---
>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>>>> +++++++++++++++++++
>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11 +++++++++++
>>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>>>> +++++++++++++++++++++
>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>>>> +++++++++++++++++++
>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>>>> +++++++++++++++++++
>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12
>>>>> ++++++++++--
>>>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>>
>>>> [I commented on sm8550, it applies to all the rest of platforms]
>>>>
>>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>>>> sm8550_merge_3d[] = {
>>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>>>> };
>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>>>> + .enc = {.base = 0x100, .len = 0x100},
>>>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>>>> +};
>>>>> +
>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>>>> + .enc = {.base = 0x200, .len = 0x100},
>>>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>>>> +};
>>>>
>>>> Please keep sblk in dpu_hw_catalog for now.
>>>>
>>>>> +
>>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0,
>>>>> sm8550_dsc_sblk_0),
>>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0,
>>>>> sm8550_dsc_sblk_1),
>>>>
>>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n which
>>>> comes next to it?
>>>
>>> usually each DCE (display compression engine) contains two hard slice
>>> encoders.
>>>
>>> DSC_0 and DSC_1 (index) is belong to dsc_0.
>>>
>>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>>
>> Ah, I see now. So, the block register space is the following:
>> DCEi ->
>> common
>> dsc0_enc
>> dsc1_enc
>> dsc0_ctl
>> dsc1_ctl
>>
>> Instead of declaring a single DCE unit with two DSC blocks, we declare
>> two distinct DSC blocks. This raises a question, how independent are
>> these two parts of a single DCE block? For example, can we use them to
>> perform compression with different parameters? Or use one of them for
>> the DP DSC and another one for DSI DSC? Can we have the following
>> configuration:
>>
>> DSC_0 => DP DSC
>> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
>
> no, For merge mode you have to use same DCE, such as DSC_2 and DSC3 (pair)
Ok, this is for the merge mode. So the dpu_rm should be extended to
allocate them in pairs if merge mode is requested.
What about using DSC_0 for DP and DSC_1 for DSI? Is it possible?
>
>>
>>>
>>>>
>>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR, 24,
>>>>> 25),
>>>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>>>> sc7280_pp_sblk = {
>>>>> {\
>>>>> .name = _name, .id = _id, \
>>>>> .base = _base, .len = 0x140, \
>>>>> - .features = _features, \
>>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>>>> + }
>>>>> +
>>>>> +#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, \
>>>>> }
>>>>> /*************************************************************
>>>>
>>
--
With best wishes
Dmitry
On 4/21/2023 4:11 PM, Dmitry Baryshkov wrote:
> On 22/04/2023 02:08, Kuogee Hsieh wrote:
>>
>> On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote:
>>> On 22/04/2023 01:05, Kuogee Hsieh wrote:
>>>>
>>>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>>>> From: Abhinav Kumar <[email protected]>
>>>>>>
>>>>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>>>>> sub-block and feature flag information.
>>>>>> Each display compression engine (DCE) contains dual hard
>>>>>> slice DSC encoders so both share same base address but with
>>>>>> its own different sub block address.
>>>>>
>>>>> Please correct line wrapping. 72-75 is usually the preferred width
>>>>>
>>>>>>
>>>>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>>>> ---
>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>>>>> +++++++++++++++++++
>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11
>>>>>> +++++++++++
>>>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>>>>> +++++++++++++++++++++
>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>>>>> +++++++++++++++++++
>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>>>>> +++++++++++++++++++
>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12
>>>>>> ++++++++++--
>>>>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>>>>
>>>>>
>>>>>
>>>>> [I commented on sm8550, it applies to all the rest of platforms]
>>>>>
>>>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>>>>> sm8550_merge_3d[] = {
>>>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>>>>> };
>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>>>>> + .enc = {.base = 0x100, .len = 0x100},
>>>>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>>>>> +};
>>>>>> +
>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>>>>> + .enc = {.base = 0x200, .len = 0x100},
>>>>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>>>>> +};
>>>>>
>>>>> Please keep sblk in dpu_hw_catalog for now.
>>>>>
>>>>>> +
>>>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0,
>>>>>> sm8550_dsc_sblk_0),
>>>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0,
>>>>>> sm8550_dsc_sblk_1),
>>>>>
>>>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n
>>>>> which comes next to it?
>>>>
>>>> usually each DCE (display compression engine) contains two hard
>>>> slice encoders.
>>>>
>>>> DSC_0 and DSC_1 (index) is belong to dsc_0.
>>>>
>>>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>>>
>>> Ah, I see now. So, the block register space is the following:
>>> DCEi ->
>>> common
>>> dsc0_enc
>>> dsc1_enc
>>> dsc0_ctl
>>> dsc1_ctl
>>>
>>> Instead of declaring a single DCE unit with two DSC blocks, we
>>> declare two distinct DSC blocks. This raises a question, how
>>> independent are these two parts of a single DCE block? For example,
>>> can we use them to perform compression with different parameters? Or
>>> use one of them for the DP DSC and another one for DSI DSC? Can we
>>> have the following configuration:
>>>
>>> DSC_0 => DP DSC
>>> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
>>
>> no, For merge mode you have to use same DCE, such as DSC_2 and DSC3
>> (pair)
>
> Ok, this is for the merge mode. So the dpu_rm should be extended to
> allocate them in pairs if merge mode is requested.
>
> What about using DSC_0 for DP and DSC_1 for DSI? Is it possible?
I never do that, but i think it should works since they can work
independently.
>
>>
>>>
>>>>
>>>>>
>>>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR,
>>>>>> 24, 25),
>>>>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>>>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>>>>> sc7280_pp_sblk = {
>>>>>> {\
>>>>>> .name = _name, .id = _id, \
>>>>>> .base = _base, .len = 0x140, \
>>>>>> - .features = _features, \
>>>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>>>>> + }
>>>>>> +
>>>>>> +#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, \
>>>>>> }
>>>>>> /*************************************************************
>>>>>
>>>
>
On 22/04/2023 02:16, Kuogee Hsieh wrote:
>
> On 4/21/2023 4:11 PM, Dmitry Baryshkov wrote:
>> On 22/04/2023 02:08, Kuogee Hsieh wrote:
>>>
>>> On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote:
>>>> On 22/04/2023 01:05, Kuogee Hsieh wrote:
>>>>>
>>>>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>>>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>>>>> From: Abhinav Kumar <[email protected]>
>>>>>>>
>>>>>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>>>>>> sub-block and feature flag information.
>>>>>>> Each display compression engine (DCE) contains dual hard
>>>>>>> slice DSC encoders so both share same base address but with
>>>>>>> its own different sub block address.
>>>>>>
>>>>>> Please correct line wrapping. 72-75 is usually the preferred width
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>>>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>>>>> ---
>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>>>>>> +++++++++++++++++++
>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11
>>>>>>> +++++++++++
>>>>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>>>>>> +++++++++++++++++++++
>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>>>>>> +++++++++++++++++++
>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>>>>>> +++++++++++++++++++
>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12
>>>>>>> ++++++++++--
>>>>>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>
>>>>>>
>>>>>> [I commented on sm8550, it applies to all the rest of platforms]
>>>>>>
>>>>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>>>>>> sm8550_merge_3d[] = {
>>>>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>>>>>> };
>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>>>>>> + .enc = {.base = 0x100, .len = 0x100},
>>>>>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>>>>>> +};
>>>>>>> +
>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>>>>>> + .enc = {.base = 0x200, .len = 0x100},
>>>>>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>>>>>> +};
>>>>>>
>>>>>> Please keep sblk in dpu_hw_catalog for now.
>>>>>>
>>>>>>> +
>>>>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0,
>>>>>>> sm8550_dsc_sblk_0),
>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0,
>>>>>>> sm8550_dsc_sblk_1),
>>>>>>
>>>>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n
>>>>>> which comes next to it?
>>>>>
>>>>> usually each DCE (display compression engine) contains two hard
>>>>> slice encoders.
>>>>>
>>>>> DSC_0 and DSC_1 (index) is belong to dsc_0.
>>>>>
>>>>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>>>>
>>>> Ah, I see now. So, the block register space is the following:
>>>> DCEi ->
>>>> common
>>>> dsc0_enc
>>>> dsc1_enc
>>>> dsc0_ctl
>>>> dsc1_ctl
>>>>
>>>> Instead of declaring a single DCE unit with two DSC blocks, we
>>>> declare two distinct DSC blocks. This raises a question, how
>>>> independent are these two parts of a single DCE block? For example,
>>>> can we use them to perform compression with different parameters? Or
>>>> use one of them for the DP DSC and another one for DSI DSC? Can we
>>>> have the following configuration:
>>>>
>>>> DSC_0 => DP DSC
>>>> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
>>>
>>> no, For merge mode you have to use same DCE, such as DSC_2 and DSC3
>>> (pair)
>>
>> Ok, this is for the merge mode. So the dpu_rm should be extended to
>> allocate them in pairs if merge mode is requested.
>>
>> What about using DSC_0 for DP and DSC_1 for DSI? Is it possible?
>
> I never do that, but i think it should works since they can work
> independently.
Good, thanks for the confirmation. For v2, could you please describe
this arrangement of DCE -> 2xDSC in a comment close to DSC_BLK_1_2 and
corresponding sblk definitions?
Also could you please fix dpu_rm to allocate DSC blocks in pairs for
DSC_MERGE mode.
Last, but not least, would it make sense to name the blocks as "dceN"
instead of "dscN"?
>
>
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR,
>>>>>>> 24, 25),
>>>>>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>>>>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>>>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>>>>>> sc7280_pp_sblk = {
>>>>>>> {\
>>>>>>> .name = _name, .id = _id, \
>>>>>>> .base = _base, .len = 0x140, \
>>>>>>> - .features = _features, \
>>>>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>>>>>> + }
>>>>>>> +
>>>>>>> +#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, \
>>>>>>> }
>>>>>>> /*************************************************************
>>>>>>
>>>>
>>
--
With best wishes
Dmitry
On 21/04/2023 02:25, Kuogee Hsieh wrote:
> Add support for DSC 1.2 by providing the necessary hooks to program
> the DPU DSC 1.2 encoder.
>
> Signed-off-by: Kuogee Hsieh <[email protected]>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 38 ++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 17 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388 +++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
> 5 files changed, 444 insertions(+), 7 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>
[skipped]
> +static inline void _dsc_subblk_offset(struct dpu_hw_dsc *hw_dsc, int s_id,
> + u32 *offset)
> +{
> + const struct dpu_dsc_sub_blks *sblk;
> +
> + sblk = hw_dsc->caps->sblk;
> +
> + if (s_id == DPU_DSC_ENC)
> + *offset = sblk->enc.base;
> + else if (s_id == DPU_DSC_CTL)
> + *offset = sblk->ctl.base;
> + else
> + DPU_ERROR("invalid DSC sub block=%d\n", s_id);
> +}
I have just sent a patchset removing the _sspp_subblk_offset. Could you
please inline this function too?
> +
> +static 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;
> +
> + _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
> +
> + hw = &hw_dsc->hw;
> + DPU_REG_WRITE(hw, offset + DSC_CFG, 0);
> +
> + _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
> +
> + DPU_REG_WRITE(hw, offset + ENC_DF_CTRL, 0);
> + DPU_REG_WRITE(hw, offset + DSC_MAIN_CONF, 0);
> +}
> +
--
With best wishes
Dmitry
On 4/21/2023 4:22 PM, Dmitry Baryshkov wrote:
> On 22/04/2023 02:16, Kuogee Hsieh wrote:
>>
>> On 4/21/2023 4:11 PM, Dmitry Baryshkov wrote:
>>> On 22/04/2023 02:08, Kuogee Hsieh wrote:
>>>>
>>>> On 4/21/2023 3:16 PM, Dmitry Baryshkov wrote:
>>>>> On 22/04/2023 01:05, Kuogee Hsieh wrote:
>>>>>>
>>>>>> On 4/20/2023 5:07 PM, Dmitry Baryshkov wrote:
>>>>>>> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>>>>>>>> From: Abhinav Kumar <[email protected]>
>>>>>>>>
>>>>>>>> Add DSC 1.2 hardware blocks to the catalog with necessary
>>>>>>>> sub-block and feature flag information.
>>>>>>>> Each display compression engine (DCE) contains dual hard
>>>>>>>> slice DSC encoders so both share same base address but with
>>>>>>>> its own different sub block address.
>>>>>>>
>>>>>>> Please correct line wrapping. 72-75 is usually the preferred width
>>>>>>>
>>>>>>>>
>>>>>>>> Signed-off-by: Abhinav Kumar <[email protected]>
>>>>>>>> Signed-off-by: Kuogee Hsieh <[email protected]>
>>>>>>>> ---
>>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 19
>>>>>>>> +++++++++++++++++++
>>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 11
>>>>>>>> +++++++++++
>>>>>>>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 21
>>>>>>>> +++++++++++++++++++++
>>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 19
>>>>>>>> +++++++++++++++++++
>>>>>>>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 19
>>>>>>>> +++++++++++++++++++
>>>>>>>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++++++++++--
>>>>>>>> 6 files changed, 99 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> [I commented on sm8550, it applies to all the rest of platforms]
>>>>>>>
>>>>>>>> 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 9e40303..72a7bcf 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,23 @@ static const struct dpu_merge_3d_cfg
>>>>>>>> sm8550_merge_3d[] = {
>>>>>>>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>>>>>>>> };
>>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_0 = {
>>>>>>>> + .enc = {.base = 0x100, .len = 0x100},
>>>>>>>> + .ctl = {.base = 0xF00, .len = 0x10},
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +static const struct dpu_dsc_sub_blks sm8550_dsc_sblk_1 = {
>>>>>>>> + .enc = {.base = 0x200, .len = 0x100},
>>>>>>>> + .ctl = {.base = 0xF80, .len = 0x10},
>>>>>>>> +};
>>>>>>>
>>>>>>> Please keep sblk in dpu_hw_catalog for now.
>>>>>>>
>>>>>>>> +
>>>>>>>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_0, 0x80000, 0x100, 0,
>>>>>>>> sm8550_dsc_sblk_0),
>>>>>>>> + DSC_BLK_1_2("dsc_0", DSC_1, 0x80000, 0x100, 0,
>>>>>>>> sm8550_dsc_sblk_1),
>>>>>>>
>>>>>>> Is there a reason why index in "dsc_N" doesn't match the DSC_n
>>>>>>> which comes next to it?
>>>>>>
>>>>>> usually each DCE (display compression engine) contains two hard
>>>>>> slice encoders.
>>>>>>
>>>>>> DSC_0 and DSC_1 (index) is belong to dsc_0.
>>>>>>
>>>>>> If there are two DCE, then DSC_2 and DSC_3 belong to dsc_1
>>>>>
>>>>> Ah, I see now. So, the block register space is the following:
>>>>> DCEi ->
>>>>> common
>>>>> dsc0_enc
>>>>> dsc1_enc
>>>>> dsc0_ctl
>>>>> dsc1_ctl
>>>>>
>>>>> Instead of declaring a single DCE unit with two DSC blocks, we
>>>>> declare two distinct DSC blocks. This raises a question, how
>>>>> independent are these two parts of a single DCE block? For
>>>>> example, can we use them to perform compression with different
>>>>> parameters? Or use one of them for the DP DSC and another one for
>>>>> DSI DSC? Can we have the following configuration:
>>>>>
>>>>> DSC_0 => DP DSC
>>>>> DSC_1, DSC_2 => DSI DSC in DSC_MERGE topology?
>>>>
>>>> no, For merge mode you have to use same DCE, such as DSC_2 and DSC3
>>>> (pair)
>>>
>>> Ok, this is for the merge mode. So the dpu_rm should be extended to
>>> allocate them in pairs if merge mode is requested.
>>>
>>> What about using DSC_0 for DP and DSC_1 for DSI? Is it possible?
>>
>> I never do that, but i think it should works since they can work
>> independently.
>
> Good, thanks for the confirmation. For v2, could you please describe
> this arrangement of DCE -> 2xDSC in a comment close to DSC_BLK_1_2 and
> corresponding sblk definitions?
>
> Also could you please fix dpu_rm to allocate DSC blocks in pairs for
> DSC_MERGE mode.
yes, I will fix DSC_MERGE mode at next patch serial.
>
> Last, but not least, would it make sense to name the blocks as "dceN"
> instead of "dscN"?
>
>>
>>
>>>
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_2, 0x81000, 0x100,
>>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_dsc_sblk_0),
>>>>>>>> + DSC_BLK_1_2("dsc_1", DSC_3, 0x81000, 0x100,
>>>>>>>> BIT(DPU_DSC_NATIVE_422_EN), sm8550_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, MDP_SSPP_TOP0_INTR,
>>>>>>>> 24, 25),
>>>>>>>> /* TODO TE sub-blocks for intf1 & intf2 */
>>>>>>>> @@ -218,6 +235,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 03f162a..be08158 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__
>>>>>>>> @@ -540,7 +540,15 @@ static const struct dpu_pingpong_sub_blks
>>>>>>>> sc7280_pp_sblk = {
>>>>>>>> {\
>>>>>>>> .name = _name, .id = _id, \
>>>>>>>> .base = _base, .len = 0x140, \
>>>>>>>> - .features = _features, \
>>>>>>>> + .features = BIT(DPU_DSC_HW_REV_1_1) | _features, \
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> +#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, \
>>>>>>>> }
>>>>>>>> /*************************************************************
>>>>>>>
>>>>>
>>>
>
On 4/20/2023 4:54 PM, Dmitry Baryshkov wrote:
> On 21/04/2023 02:25, Kuogee Hsieh wrote:
>> Add support for DSC 1.2 by providing the necessary hooks to program
>> the DPU DSC 1.2 encoder.
>>
>> Signed-off-by: Kuogee Hsieh <[email protected]>
>> ---
>> drivers/gpu/drm/msm/Makefile | 1 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 38 ++-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 17 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 388
>> +++++++++++++++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
>> 5 files changed, 444 insertions(+), 7 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 71584cd..22421b9 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.
>> */
>> @@ -241,12 +241,20 @@ enum {
>> };
>> /**
>> - * DSC features
>> - * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
>> - * the pixel output from this DSC.
>> + * DSC sub-blocks/features
>> + * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
>> + * the pixel output from this DSC.
>
> Any reason to change the alignment
>
>> + * @DPU_DSC_HW_REV_1_1 DSC block supports dsc 1.1 only
>> + * @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_1,
>> + DPU_DSC_HW_REV_1_2,
>> + DPU_DSC_NATIVE_422_EN,
>> + DPU_DSC_MAX
>> };
>> /**
>> @@ -311,6 +319,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 {
>> @@ -459,6 +475,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 {
>> @@ -612,10 +639,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 ae9b5db..d0f8b8b 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
>> @@ -61,7 +64,7 @@ struct dpu_hw_dsc {
>> };
>> /**
>> - * dpu_hw_dsc_init - initializes the dsc block for the passed dsc idx.
>> + * dpu_hw_dsc_init - initializes the v1.1 dsc block for the passed
>> dsc idx.
>> * @idx: DSC index for which driver object is required
>> * @addr: Mapped register io address of MDP
>> * @m: Pointer to mdss catalog data
>> @@ -71,6 +74,16 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(enum dpu_dsc
>> idx, void __iomem *addr,
>> const struct dpu_mdss_cfg *m);
>> /**
>> + * dpu_hw_dsc_init_1_2 - initializes the v1.2 dsc block for the
>> passed dsc idx.
>> + * @idx: DSC index for which driver object is required
>> + * @addr: Mapped register io address of MDP
>> + * @m: Pointer to mdss catalog data
>> + * Returns: Error code or allocated dpu_hw_dsc context
>> + */
>> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(enum dpu_dsc idx, void
>> __iomem *addr,
>> + const struct dpu_mdss_cfg *m);
>> +
>> +/**
>> * 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..455d7f2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>> @@ -0,0 +1,388 @@
>> +// 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
>
> As you are adding new definitions, could please make them aligned from
> the beginning?
>
>> +#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
>> +
>> +/*
>> + * @DPU_DSC_ENC DSC encoder sub block
>> + * @DPU_DSC_CTL DSC controller sub block
>> + */
>> +enum {
>> + DPU_DSC_ENC,
>> + DPU_DSC_CTL,
>> +};
>> +
>> +static int _dsc_calc_ob_max_addr(struct dpu_hw_dsc *hw_dsc, int num_ss)
>
> If this is used from a single place, please inline this function.
>
>> +{
>> + int max_addr = 2400 / num_ss;
>> +
>> + if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_422_EN))
>> + max_addr /= 2;
>
> Is this really guarded by the hw block feature or by native_422 being
> enabled?
>
>> +
>> + return max_addr - 1;
>> +};
>> +
>>
>
> [skipped]
>
>> +static 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, off;
>> + int i, j = 0;
>> + struct drm_dsc_rc_range_parameters *rc;
>> + u32 data = 0, min_qp = 0, max_qp = 0, bpg_off = 0;
>> +
>> + if (!hw_dsc || !dsc)
>> + return;
>> +
>> + _dsc_subblk_offset(hw_dsc, DPU_DSC_ENC, &offset);
>> +
>> + hw = &hw_dsc->hw;
>> +
>> + rc = dsc->rc_range_params;
>> +
>> + off = 0;
>> + for (i = 0; i < DSC_NUM_BUF_RANGES - 1; i++) {
>> + data |= dsc->rc_buf_thresh[i] << (8 * j);
>> + j++;
>> + if ((j == 4) || (i == DSC_NUM_BUF_RANGES - 2)) {
>> + DPU_REG_WRITE(hw, offset + DSC_RC_BUF_THRESH_0 + off,
>> + data);
>> + off += 4;
>> + j = 0;
>> + data = 0;
>> + }
>> + }
>> +
>> + off = 0;
>> + for (i = 0; i < DSC_NUM_BUF_RANGES; i++) {
>> + min_qp |= rc[i].range_min_qp << (5 * j);
>> + max_qp |= rc[i].range_max_qp << (5 * j);
>> + bpg_off |= rc[i].range_bpg_offset << (6 * j);
>> + j++;
>> + if (j == 5) {
>> + DPU_REG_WRITE(hw, offset + DSC_RC_MIN_QP_0 + off,
>> + min_qp);
>> + DPU_REG_WRITE(hw, offset + DSC_RC_MAX_QP_0 + off,
>> + max_qp);
>> + DPU_REG_WRITE(hw,
>> + offset + DSC_RC_RANGE_BPG_OFFSETS_0 + off,
>> + bpg_off);
>> + off += 4;
>> + j = 0;
>> + min_qp = 0;
>> + max_qp = 0;
>> + bpg_off = 0;
>> + }
>> + }
>
> Unrolling these loops would make it easier to understand them.
it is kind of difficult to unrolling the for loop.
I will add comments to explain the loop.
>
>> +}
>> +
>> +static void dpu_hw_dsc_bind_pingpong_blk_1_2(
>> + struct dpu_hw_dsc *hw_dsc,
>> + bool enable,
>> + const enum dpu_pingpong pp)
>> +{
>> + struct dpu_hw_blk_reg_map *hw;
>> + int offset;
>> + int mux_cfg = 0xf; /* Disabled */
>> +
>> + _dsc_subblk_offset(hw_dsc, DPU_DSC_CTL, &offset);
>> +
>> + hw = &hw_dsc->hw;
>> + if (enable)
>> + mux_cfg = (pp - PINGPONG_0) & 0x7;
>> +
>> + DPU_REG_WRITE(hw, offset + DSC_CTL, mux_cfg);
>> +}
>> +
>> +static const struct dpu_dsc_cfg *_dsc_offset(enum dpu_dsc dsc,
>> + const struct dpu_mdss_cfg *m,
>> + void __iomem *addr,
>> + struct dpu_hw_blk_reg_map *b)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < m->dsc_count; i++) {
>> + if (dsc == m->dsc[i].id) {
>> + b->blk_addr = addr + m->dsc[i].base;
>> + b->log_mask = DPU_DBG_MASK_DSC;
>> + return &m->dsc[i];
>> + }
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +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(enum dpu_dsc idx, void
>> __iomem *addr,
>> + const struct dpu_mdss_cfg *m)
>> +{
>
> Please rebase on top of https://patchwork.freedesktop.org/series/116615/
>
>> + struct dpu_hw_dsc *c;
>> + const struct dpu_dsc_cfg *cfg;
>> +
>> + c = kzalloc(sizeof(*c), GFP_KERNEL);
>> + if (!c)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + cfg = _dsc_offset(idx, m, addr, &c->hw);
>> + if (IS_ERR_OR_NULL(cfg)) {
>> + kfree(c);
>> + return ERR_PTR(-EINVAL);
>> + }
>> +
>> + c->idx = idx;
>> + 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 f4dda88..f7014f5 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__
>> @@ -250,7 +251,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->id, mmio, cat);
>> + if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
>> + hw = dpu_hw_dsc_init_1_2(dsc->id, mmio, cat);
>> + else
>> + hw = dpu_hw_dsc_init(dsc->id, mmio, cat);
>> +
>> if (IS_ERR_OR_NULL(hw)) {
>> rc = PTR_ERR(hw);
>> DPU_ERROR("failed dsc object creation: err %d\n", rc);
>