* [PATCH v10 1/8] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:14 ` Marijn Suijten
2023-05-17 22:01 ` [PATCH v10 2/8] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0 Kuogee Hsieh
` (6 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Abhinav Kumar, quic_khsieh, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
Some platforms have DSC blocks which have not been declared in the catalog.
Complete DSC 1.1 support for all platforms by adding the missing blocks to
MSM8998 and SC8180X.
Changes in v9:
-- add MSM8998 and SC8180x to commit titil
Changes in v10:
-- fix grammar at commit text
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
---
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++++++
drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++++++++++
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
index c0dd477..521cfd5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
@@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = {
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
};
+static const struct dpu_dsc_cfg msm8998_dsc[] = {
+ DSC_BLK("dsc_0", DSC_0, 0x80000, 0),
+ DSC_BLK("dsc_1", DSC_1, 0x80400, 0),
+};
+
static const struct dpu_dspp_cfg msm8998_dspp[] = {
DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
&msm8998_dspp_sblk),
@@ -199,6 +204,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = {
.dspp = msm8998_dspp,
.pingpong_count = ARRAY_SIZE(msm8998_pp),
.pingpong = msm8998_pp,
+ .dsc_count = ARRAY_SIZE(msm8998_dsc),
+ .dsc = msm8998_dsc,
.intf_count = ARRAY_SIZE(msm8998_intf),
.intf = msm8998_intf,
.vbif_count = ARRAY_SIZE(msm8998_vbif),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
index e8057a1..fec1665 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
@@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200),
};
+static const struct dpu_dsc_cfg sc8180x_dsc[] = {
+ DSC_BLK("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)),
+ DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)),
+};
+
static const struct dpu_intf_cfg sc8180x_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -206,6 +215,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = {
.mixer = sc8180x_lm,
.pingpong_count = ARRAY_SIZE(sc8180x_pp),
.pingpong = sc8180x_pp,
+ .dsc_count = ARRAY_SIZE(sc8180x_dsc),
+ .dsc = sc8180x_dsc,
.merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d),
.merge_3d = sc8180x_merge_3d,
.intf_count = ARRAY_SIZE(sc8180x_intf),
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 1/8] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X
2023-05-17 22:01 ` [PATCH v10 1/8] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X Kuogee Hsieh
@ 2023-05-17 22:14 ` Marijn Suijten
0 siblings, 0 replies; 24+ messages in thread
From: Marijn Suijten @ 2023-05-17 22:14 UTC (permalink / raw)
To: Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, Abhinav Kumar,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
DSC* in the title.
On 2023-05-17 15:01:52, Kuogee Hsieh wrote:
> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> Some platforms have DSC blocks which have not been declared in the catalog.
> Complete DSC 1.1 support for all platforms by adding the missing blocks to
> MSM8998 and SC8180X.
>
> Changes in v9:
> -- add MSM8998 and SC8180x to commit titil
title*
(couldn't be caught in v9... because this changelog wasn't present then)
- Marijn
>
> Changes in v10:
> -- fix grammar at commit text
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h | 7 +++++++
> drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h | 11 +++++++++++
> 2 files changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> index c0dd477..521cfd5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> @@ -126,6 +126,11 @@ static const struct dpu_pingpong_cfg msm8998_pp[] = {
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 15)),
> };
>
> +static const struct dpu_dsc_cfg msm8998_dsc[] = {
> + DSC_BLK("dsc_0", DSC_0, 0x80000, 0),
> + DSC_BLK("dsc_1", DSC_1, 0x80400, 0),
> +};
> +
> static const struct dpu_dspp_cfg msm8998_dspp[] = {
> DSPP_BLK("dspp_0", DSPP_0, 0x54000, DSPP_MSM8998_MASK,
> &msm8998_dspp_sblk),
> @@ -199,6 +204,8 @@ const struct dpu_mdss_cfg dpu_msm8998_cfg = {
> .dspp = msm8998_dspp,
> .pingpong_count = ARRAY_SIZE(msm8998_pp),
> .pingpong = msm8998_pp,
> + .dsc_count = ARRAY_SIZE(msm8998_dsc),
> + .dsc = msm8998_dsc,
> .intf_count = ARRAY_SIZE(msm8998_intf),
> .intf = msm8998_intf,
> .vbif_count = ARRAY_SIZE(msm8998_vbif),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> index e8057a1..fec1665 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_5_1_sc8180x.h
> @@ -142,6 +142,15 @@ static const struct dpu_merge_3d_cfg sc8180x_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x83200),
> };
>
> +static const struct dpu_dsc_cfg sc8180x_dsc[] = {
> + DSC_BLK("dsc_0", DSC_0, 0x80000, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_1", DSC_1, 0x80400, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_2", DSC_2, 0x80800, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_3", DSC_3, 0x80c00, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_4", DSC_4, 0x81000, BIT(DPU_DSC_OUTPUT_CTRL)),
> + DSC_BLK("dsc_5", DSC_5, 0x81400, BIT(DPU_DSC_OUTPUT_CTRL)),
> +};
> +
> static const struct dpu_intf_cfg sc8180x_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x6a000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7180_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -206,6 +215,8 @@ const struct dpu_mdss_cfg dpu_sc8180x_cfg = {
> .mixer = sc8180x_lm,
> .pingpong_count = ARRAY_SIZE(sc8180x_pp),
> .pingpong = sc8180x_pp,
> + .dsc_count = ARRAY_SIZE(sc8180x_dsc),
> + .dsc = sc8180x_dsc,
> .merge_3d_count = ARRAY_SIZE(sc8180x_merge_3d),
> .merge_3d = sc8180x_merge_3d,
> .intf_count = ARRAY_SIZE(sc8180x_intf),
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v10 2/8] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 1/8] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 3/8] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit Kuogee Hsieh
` (5 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
DPU < 7.0.0 requires the PINGPONG block to be involved during
DSC setting up. Since DPU >= 7.0.0, enabling and starting the DSC
encoder engine was moved to INTF with the help of the flush mechanism.
Add a DPU_PINGPONG_DSC feature bit to restrict the availability of
dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() on the
PINGPONG block to DPU < 7.0.0 hardware, as the registers are not
available on DPU 7.0.0 and higher anymore.
Add DPU_PINGPONG_DSC to PINGPONG_SDM845_MASK, PINGPONG_SDM845_TE2_MASK
and PINGPONG_SM8150_MASK which is used for all DPU < 7.0 chipsets.
changes in v6:
-- split patches and rearrange to keep catalog related files at this patch
changes in v7:
-- rewording commit text as suggested at review comments
changes in v9:
-- delete BIT(DPU_PINGPONG_DSC) from PINGPONG_SDM845_TE2_MASK
changes in v10:
-- correct order of commit text
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 ++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 +++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 82b58c6..f2a1535 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -76,13 +76,13 @@
(BIT(DPU_DIM_LAYER) | BIT(DPU_MIXER_COMBINED_ALPHA))
#define PINGPONG_SDM845_MASK \
- (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE))
+ (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_TE) | BIT(DPU_PINGPONG_DSC))
#define PINGPONG_SDM845_TE2_MASK \
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
#define PINGPONG_SM8150_MASK \
- (BIT(DPU_PINGPONG_DITHER))
+ (BIT(DPU_PINGPONG_DITHER) | BIT(DPU_PINGPONG_DSC))
#define CTL_SC7280_MASK \
(BIT(DPU_CTL_ACTIVE_CFG) | \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 6ee48f0..83854e8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -144,7 +144,8 @@ enum {
* @DPU_PINGPONG_TE2 Additional tear check block for split pipes
* @DPU_PINGPONG_SPLIT PP block supports split fifo
* @DPU_PINGPONG_SLAVE PP block is a suitable slave for split fifo
- * @DPU_PINGPONG_DITHER, Dither blocks
+ * @DPU_PINGPONG_DITHER Dither blocks
+ * @DPU_PINGPONG_DSC PP block supports DSC
* @DPU_PINGPONG_MAX
*/
enum {
@@ -153,6 +154,7 @@ enum {
DPU_PINGPONG_SPLIT,
DPU_PINGPONG_SLAVE,
DPU_PINGPONG_DITHER,
+ DPU_PINGPONG_DSC,
DPU_PINGPONG_MAX
};
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v10 3/8] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 1/8] drm/msm/dpu: add dsc blocks to the catalog of MSM8998 and SC8180X Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 2/8] drm/msm/dpu: add DPU_PINGPONG_DSC feature bit for DPU < 7.0.0 Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-18 0:29 ` Dmitry Baryshkov
2023-05-17 22:01 ` [PATCH v10 4/8] drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG Kuogee Hsieh
` (4 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
DPU < 7.0.0 has DPU_PINGPONG_DSC feature bit set to indicate it requires
both dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() to be
executed to complete DSC configuration if DSC hardware block is present.
Hence test DPU_PINGPONG_DSC feature bit and assign DSC related functions
to the ops of PINGPONG block accordingly if DPU_PINGPONG_DSC bit is set.
Changes in v6:
-- split patches, this patch has function handles DPU_PINGPONG_DSC bit
Changes in v9:
-- the original code of assigning dsc related functions to the ops of pingpong
block without testing the DPU_PINGPONG_DSC feature bit was restored back
due to rebase error which defeat the purpose of this patch. Remove those
error code.
Changes in v10:
-- change commit title
-- correct texts at changes in v9
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
index 79e4576..437d9e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
@@ -291,9 +291,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
c->ops.get_line_count = dpu_hw_pp_get_line_count;
c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
}
- c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
- c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
- c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
+
+ if (test_bit(DPU_PINGPONG_DSC, &features)) {
+ c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
+ c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
+ c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
+ }
if (test_bit(DPU_PINGPONG_DITHER, &features))
c->ops.setup_dither = dpu_hw_pp_setup_dither;
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 3/8] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit
2023-05-17 22:01 ` [PATCH v10 3/8] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit Kuogee Hsieh
@ 2023-05-18 0:29 ` Dmitry Baryshkov
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Baryshkov @ 2023-05-18 0:29 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, andersson
Cc: quic_abhinavk, quic_jesszhan, quic_sbillaka, marijn.suijten,
freedreno, linux-arm-msm, linux-kernel
On 18/05/2023 01:01, Kuogee Hsieh wrote:
> DPU < 7.0.0 has DPU_PINGPONG_DSC feature bit set to indicate it requires
> both dpu_hw_pp_setup_dsc() and dpu_hw_pp_dsc_{enable,disable}() to be
> executed to complete DSC configuration if DSC hardware block is present.
> Hence test DPU_PINGPONG_DSC feature bit and assign DSC related functions
> to the ops of PINGPONG block accordingly if DPU_PINGPONG_DSC bit is set.
>
> Changes in v6:
> -- split patches, this patch has function handles DPU_PINGPONG_DSC bit
>
> Changes in v9:
> -- the original code of assigning dsc related functions to the ops of pingpong
line too long
> block without testing the DPU_PINGPONG_DSC feature bit was restored back
> due to rebase error which defeat the purpose of this patch. Remove those
> error code.
>
> Changes in v10:
> -- change commit title
> -- correct texts at changes in v9
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> index 79e4576..437d9e6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c
> @@ -291,9 +291,12 @@ static void _setup_pingpong_ops(struct dpu_hw_pingpong *c,
> c->ops.get_line_count = dpu_hw_pp_get_line_count;
> c->ops.disable_autorefresh = dpu_hw_pp_disable_autorefresh;
> }
> - c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
> - c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> - c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
> +
> + if (test_bit(DPU_PINGPONG_DSC, &features)) {
> + c->ops.setup_dsc = dpu_hw_pp_setup_dsc;
> + c->ops.enable_dsc = dpu_hw_pp_dsc_enable;
> + c->ops.disable_dsc = dpu_hw_pp_dsc_disable;
> + }
>
> if (test_bit(DPU_PINGPONG_DITHER, &features))
> c->ops.setup_dither = dpu_hw_pp_setup_dither;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v10 4/8] drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
` (2 preceding siblings ...)
2023-05-17 22:01 ` [PATCH v10 3/8] drm/msm/dpu: Guard PINGPONG DSC ops behind DPU_PINGPONG_DSC bit Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine Kuogee Hsieh
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
Disabling the crossbar mux between DSC and PINGPONG currently
requires a bogus enum dpu_pingpong value to be passed when calling
dsc_bind_pingpong_blk() with enable=false, even though the register
value written is independent of the current PINGPONG block. Replace
that `bool enable` parameter with a new PINGPONG_NONE dpu_pingpong
flag that triggers the write of the "special" 0xF "crossbar
disabled" value to the register instead.
Changes in v4:
-- more details to commit text
Changes in v5:
-- rewording commit text suggested by Marijn
-- add DRM_DEBUG_KMS for DSC unbinding case
Changes in v8:
-- fix checkpatch warning
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c | 14 +++++++-------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 1 -
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 ++-
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index cf1de5d..ffa6f04 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1850,7 +1850,7 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
hw_pp->ops.setup_dsc(hw_pp);
if (hw_dsc->ops.dsc_bind_pingpong_blk)
- hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, true, hw_pp->idx);
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, hw_pp->idx);
if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
index 4a6bbcc..f134fb0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.c
@@ -157,7 +157,6 @@ static void dpu_hw_dsc_config_thresh(struct dpu_hw_dsc *hw_dsc,
static void dpu_hw_dsc_bind_pingpong_blk(
struct dpu_hw_dsc *hw_dsc,
- bool enable,
const enum dpu_pingpong pp)
{
struct dpu_hw_blk_reg_map *c = &hw_dsc->hw;
@@ -166,14 +165,15 @@ static void dpu_hw_dsc_bind_pingpong_blk(
dsc_ctl_offset = DSC_CTL(hw_dsc->idx);
- if (enable)
+ if (pp)
mux_cfg = (pp - PINGPONG_0) & 0x7;
- DRM_DEBUG_KMS("%s dsc:%d %s pp:%d\n",
- enable ? "Binding" : "Unbinding",
- hw_dsc->idx - DSC_0,
- enable ? "to" : "from",
- pp - PINGPONG_0);
+ if (pp)
+ DRM_DEBUG_KMS("Binding dsc:%d to pp:%d\n",
+ hw_dsc->idx - DSC_0, pp - PINGPONG_0);
+ else
+ DRM_DEBUG_KMS("Unbinding dsc:%d from any pp\n",
+ hw_dsc->idx - DSC_0);
DPU_REG_WRITE(c, dsc_ctl_offset, mux_cfg);
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index 287ec5f..138080a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -44,7 +44,6 @@ struct dpu_hw_dsc_ops {
struct drm_dsc_config *dsc);
void (*dsc_bind_pingpong_blk)(struct dpu_hw_dsc *hw_dsc,
- bool enable,
enum dpu_pingpong pp);
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
index 1913a19..02a0f48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -191,7 +191,8 @@ enum dpu_dsc {
};
enum dpu_pingpong {
- PINGPONG_0 = 1,
+ PINGPONG_NONE,
+ PINGPONG_0,
PINGPONG_1,
PINGPONG_2,
PINGPONG_3,
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
` (3 preceding siblings ...)
2023-05-17 22:01 ` [PATCH v10 4/8] drm/msm/dpu: Introduce PINGPONG_NONE to disconnect DSC from PINGPONG Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:22 ` Marijn Suijten
2023-05-17 22:01 ` [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface Kuogee Hsieh
` (2 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
Add support for DSC 1.2 by providing the necessary hooks to program
the DPU DSC 1.2 encoder.
Changes in v3:
-- fixed kernel test rebot report that "__iomem *off" is declared but not
used at dpu_hw_dsc_config_1_2()
-- unrolling thresh loops
Changes in v4:
-- delete DPU_DSC_HW_REV_1_1
-- delete off and used real register name directly
Changes in v7:
-- replace offset with sblk->enc.base
-- replace ss with slice
Changes in v8:
-- fixed checkpatch warning
Changes in v9:
-- replaced __dsc_calc_ob_max_addr() with __dsc_calc_output_buf_max_addr()
-- replaced variable num_ss with num_softslice
-- remove inline from function declaration
changes in v10:
-- rewording text of changes in v9
-- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 31 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 386 +++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
5 files changed, 435 insertions(+), 4 deletions(-)
create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index b814fc8..b9af5e4 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
disp/dpu1/dpu_hw_catalog.o \
disp/dpu1/dpu_hw_ctl.o \
disp/dpu1/dpu_hw_dsc.o \
+ disp/dpu1/dpu_hw_dsc_1_2.o \
disp/dpu1/dpu_hw_interrupts.o \
disp/dpu1/dpu_hw_intf.o \
disp/dpu1/dpu_hw_lm.o \
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 83854e8..11610f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
*/
@@ -244,12 +244,18 @@ enum {
};
/**
- * DSC features
+ * DSC sub-blocks/features
* @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
* the pixel output from this DSC.
+ * @DPU_DSC_HW_REV_1_2 DSC block supports DSC 1.1 and 1.2
+ * @DPU_DSC_NATIVE_42x_EN Supports NATIVE_422_EN and NATIVE_420_EN encoding
+ * @DPU_DSC_MAX
*/
enum {
DPU_DSC_OUTPUT_CTRL = 0x1,
+ DPU_DSC_HW_REV_1_2,
+ DPU_DSC_NATIVE_42x_EN,
+ DPU_DSC_MAX
};
/**
@@ -306,6 +312,14 @@ struct dpu_pp_blk {
};
/**
+ * struct dpu_dsc_blk - DSC Encoder sub-blk information
+ * @info: HW register and features supported by this sub-blk
+ */
+struct dpu_dsc_blk {
+ DPU_HW_SUBBLK_INFO;
+};
+
+/**
* enum dpu_qos_lut_usage - define QoS LUT use cases
*/
enum dpu_qos_lut_usage {
@@ -452,6 +466,16 @@ struct dpu_pingpong_sub_blks {
};
/**
+ * struct dpu_dsc_sub_blks - DSC sub-blks
+ * @enc: DSC encoder sub-block
+ * @ctl: DSC controller sub-block
+ */
+struct dpu_dsc_sub_blks {
+ struct dpu_dsc_blk enc;
+ struct dpu_dsc_blk ctl;
+};
+
+/**
* dpu_clk_ctrl_type - Defines top level clock control signals
*/
enum dpu_clk_ctrl_type {
@@ -605,10 +629,13 @@ struct dpu_merge_3d_cfg {
* struct dpu_dsc_cfg - information of DSC blocks
* @id enum identifying this block
* @base register offset of this block
+ * @len: length of hardware block
* @features bit mask identifying sub-blocks/features
+ * @sblk: sub-blocks information
*/
struct dpu_dsc_cfg {
DPU_HW_BLK_INFO;
+ const struct dpu_dsc_sub_blks *sblk;
};
/**
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
index 138080a..41e39d0 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
@@ -1,5 +1,8 @@
/* SPDX-License-Identifier: GPL-2.0-only */
-/* Copyright (c) 2020-2022, Linaro Limited */
+/*
+ * Copyright (c) 2020-2022, Linaro Limited
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
#ifndef _DPU_HW_DSC_H
#define _DPU_HW_DSC_H
@@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg,
void __iomem *addr);
/**
+ * dpu_hw_dsc_init_1_2() - initializes the v1.2 DSC hw driver block
+ * @cfg: DSC catalog entry for which driver object is required
+ * @addr: Mapped register io address of MDP
+ * Returns: Error code or allocated dpu_hw_dsc context
+ */
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
+ void __iomem *addr);
+
+/**
* dpu_hw_dsc_destroy - destroys dsc driver context
* @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
*/
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
new file mode 100644
index 00000000..c3f1998
--- /dev/null
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
+ */
+
+#include <drm/display/drm_dsc_helper.h>
+
+#include "dpu_kms.h"
+#include "dpu_hw_catalog.h"
+#include "dpu_hwio.h"
+#include "dpu_hw_mdss.h"
+#include "dpu_hw_dsc.h"
+
+#define DSC_CMN_MAIN_CNF 0x00
+
+/* DPU_DSC_ENC register offsets */
+#define ENC_DF_CTRL 0x00
+#define ENC_GENERAL_STATUS 0x04
+#define ENC_HSLICE_STATUS 0x08
+#define ENC_OUT_STATUS 0x0C
+#define ENC_INT_STAT 0x10
+#define ENC_INT_CLR 0x14
+#define ENC_INT_MASK 0x18
+#define DSC_MAIN_CONF 0x30
+#define DSC_PICTURE_SIZE 0x34
+#define DSC_SLICE_SIZE 0x38
+#define DSC_MISC_SIZE 0x3C
+#define DSC_HRD_DELAYS 0x40
+#define DSC_RC_SCALE 0x44
+#define DSC_RC_SCALE_INC_DEC 0x48
+#define DSC_RC_OFFSETS_1 0x4C
+#define DSC_RC_OFFSETS_2 0x50
+#define DSC_RC_OFFSETS_3 0x54
+#define DSC_RC_OFFSETS_4 0x58
+#define DSC_FLATNESS_QP 0x5C
+#define DSC_RC_MODEL_SIZE 0x60
+#define DSC_RC_CONFIG 0x64
+#define DSC_RC_BUF_THRESH_0 0x68
+#define DSC_RC_BUF_THRESH_1 0x6C
+#define DSC_RC_BUF_THRESH_2 0x70
+#define DSC_RC_BUF_THRESH_3 0x74
+#define DSC_RC_MIN_QP_0 0x78
+#define DSC_RC_MIN_QP_1 0x7C
+#define DSC_RC_MIN_QP_2 0x80
+#define DSC_RC_MAX_QP_0 0x84
+#define DSC_RC_MAX_QP_1 0x88
+#define DSC_RC_MAX_QP_2 0x8C
+#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
+#define DSC_RC_RANGE_BPG_OFFSETS_1 0x94
+#define DSC_RC_RANGE_BPG_OFFSETS_2 0x98
+
+/* DPU_DSC_CTL register offsets */
+#define DSC_CTL 0x00
+#define DSC_CFG 0x04
+#define DSC_DATA_IN_SWAP 0x08
+#define DSC_CLK_CTRL 0x0C
+
+static int _dsc_calc_output_buf_max_addr(struct dpu_hw_dsc *hw_dsc, int num_softslice)
+{
+ int max_addr = 2400 / num_softslice;
+
+ if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_42x_EN))
+ max_addr /= 2;
+
+ return max_addr - 1;
+};
+
+static void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ const struct dpu_dsc_sub_blks *sblk;
+
+ if (!hw_dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+ sblk = hw_dsc->caps->sblk;
+ DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CFG, 0);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + ENC_DF_CTRL, 0);
+ DPU_REG_WRITE(hw, sblk->enc.base + 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;
+ const struct dpu_dsc_sub_blks *sblk;
+ u32 data = 0;
+ u32 det_thresh_flatness;
+ u32 num_active_slice_per_enc;
+ u32 bpp;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+
+ sblk = hw_dsc->caps->sblk;
+
+ if (mode & DSC_MODE_SPLIT_PANEL)
+ data |= BIT(0);
+
+ if (mode & DSC_MODE_MULTIPLEX)
+ data |= BIT(1);
+
+ num_active_slice_per_enc = dsc->slice_count;
+ if (mode & DSC_MODE_MULTIPLEX)
+ num_active_slice_per_enc = dsc->slice_count / 2;
+
+ data |= (num_active_slice_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_output_buf_max_addr(hw_dsc, num_active_slice_per_enc) << 18);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + 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 |= bpp << 10;
+
+ if (dsc->block_pred_enable)
+ data |= BIT(20);
+
+ if (dsc->convert_rgb)
+ data |= BIT(4);
+
+ data |= (dsc->line_buf_depth & 0xf) << 6;
+ data |= dsc->bits_per_component & 0xf;
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_MAIN_CONF, data);
+
+ data = (dsc->pic_width & 0xffff) |
+ ((dsc->pic_height & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_PICTURE_SIZE, data);
+
+ data = (dsc->slice_width & 0xffff) |
+ ((dsc->slice_height & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_SLICE_SIZE, data);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_MISC_SIZE,
+ (dsc->slice_chunk_size) & 0xffff);
+
+ data = (dsc->initial_xmit_delay & 0xffff) |
+ ((dsc->initial_dec_delay & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_HRD_DELAYS, data);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE,
+ dsc->initial_scale_value & 0x3f);
+
+ data = (dsc->scale_increment_interval & 0xffff) |
+ ((dsc->scale_decrement_interval & 0x7ff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE_INC_DEC, data);
+
+ data = (dsc->first_line_bpg_offset & 0x1f) |
+ ((dsc->second_line_bpg_offset & 0x1f) << 5);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_1, data);
+
+ data = (dsc->nfl_bpg_offset & 0xffff) |
+ ((dsc->slice_bpg_offset & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_2, data);
+
+ data = (dsc->initial_offset & 0xffff) |
+ ((dsc->final_offset & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_3, data);
+
+ data = (dsc->nsl_bpg_offset & 0xffff) |
+ ((dsc->second_line_offset_adj & 0xffff) << 16);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_4, data);
+
+ det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
+ data = (dsc->flatness_min_qp & 0x1f) | ((dsc->flatness_max_qp & 0x1f) << 5) |
+ ((det_thresh_flatness & 0xff) << 10);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_FLATNESS_QP, data);
+
+ DPU_REG_WRITE(hw, sblk->enc.base + 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, sblk->enc.base + DSC_RC_CONFIG, data);
+
+ /* program the dsc wrapper */
+ 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, sblk->ctl.base + 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;
+ const struct dpu_dsc_sub_blks *sblk;
+ struct drm_dsc_rc_range_parameters *rc;
+
+ if (!hw_dsc || !dsc)
+ return;
+
+ hw = &hw_dsc->hw;
+
+ sblk = hw_dsc->caps->sblk;
+
+ rc = dsc->rc_range_params;
+
+ /*
+ * With BUF_THRESH -- 14 in total
+ * each register contains 4 thresh values with the last register
+ * containing only 2 thresh values
+ */
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_0,
+ (dsc->rc_buf_thresh[0] << 0) |
+ (dsc->rc_buf_thresh[1] << 8) |
+ (dsc->rc_buf_thresh[2] << 16) |
+ (dsc->rc_buf_thresh[3] << 24));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_1,
+ (dsc->rc_buf_thresh[4] << 0) |
+ (dsc->rc_buf_thresh[5] << 8) |
+ (dsc->rc_buf_thresh[6] << 16) |
+ (dsc->rc_buf_thresh[7] << 24));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_2,
+ (dsc->rc_buf_thresh[8] << 0) |
+ (dsc->rc_buf_thresh[9] << 8) |
+ (dsc->rc_buf_thresh[10] << 16) |
+ (dsc->rc_buf_thresh[11] << 24));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_3,
+ (dsc->rc_buf_thresh[12] << 0) |
+ (dsc->rc_buf_thresh[13] << 8));
+
+ /*
+ * with min/max_QP -- 5 bits
+ * each register contains 5 min_qp or max_qp for total of 15
+ *
+ * With BPG_OFFSET -- 6 bits
+ * each register contains 5 BPG_offset for total of 15
+ */
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_0,
+ (rc[0].range_min_qp << 0) |
+ (rc[1].range_min_qp << 5) |
+ (rc[2].range_min_qp << 10) |
+ (rc[3].range_min_qp << 15) |
+ (rc[4].range_min_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_0,
+ (rc[0].range_max_qp << 0) |
+ (rc[1].range_max_qp << 5) |
+ (rc[2].range_max_qp << 10) |
+ (rc[3].range_max_qp << 15) |
+ (rc[4].range_max_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_0,
+ (rc[0].range_bpg_offset << 0) |
+ (rc[1].range_bpg_offset << 6) |
+ (rc[2].range_bpg_offset << 12) |
+ (rc[3].range_bpg_offset << 18) |
+ (rc[4].range_bpg_offset << 24));
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_1,
+ (rc[5].range_min_qp << 0) |
+ (rc[6].range_min_qp << 5) |
+ (rc[7].range_min_qp << 10) |
+ (rc[8].range_min_qp << 15) |
+ (rc[9].range_min_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_1,
+ (rc[5].range_max_qp << 0) |
+ (rc[6].range_max_qp << 5) |
+ (rc[7].range_max_qp << 10) |
+ (rc[8].range_max_qp << 15) |
+ (rc[9].range_max_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_1,
+ (rc[5].range_bpg_offset << 0) |
+ (rc[6].range_bpg_offset << 6) |
+ (rc[7].range_bpg_offset << 12) |
+ (rc[8].range_bpg_offset << 18) |
+ (rc[9].range_bpg_offset << 24));
+
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_2,
+ (rc[10].range_min_qp << 0) |
+ (rc[11].range_min_qp << 5) |
+ (rc[12].range_min_qp << 10) |
+ (rc[13].range_min_qp << 15) |
+ (rc[14].range_min_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_2,
+ (rc[10].range_max_qp << 0) |
+ (rc[11].range_max_qp << 5) |
+ (rc[12].range_max_qp << 10) |
+ (rc[13].range_max_qp << 15) |
+ (rc[14].range_max_qp << 20));
+ DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_2,
+ (rc[10].range_bpg_offset << 0) |
+ (rc[11].range_bpg_offset << 6) |
+ (rc[12].range_bpg_offset << 12) |
+ (rc[13].range_bpg_offset << 18) |
+ (rc[14].range_bpg_offset << 24));
+}
+
+static void dpu_hw_dsc_bind_pingpong_blk_1_2(struct dpu_hw_dsc *hw_dsc,
+ const enum dpu_pingpong pp)
+{
+ struct dpu_hw_blk_reg_map *hw;
+ const struct dpu_dsc_sub_blks *sblk;
+ int mux_cfg = 0xf; /* Disabled */
+
+ hw = &hw_dsc->hw;
+
+ sblk = hw_dsc->caps->sblk;
+
+ if (pp)
+ mux_cfg = (pp - PINGPONG_0) & 0x7;
+
+ DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CTL, mux_cfg);
+}
+
+static void _setup_dcs_ops_1_2(struct dpu_hw_dsc_ops *ops,
+ const unsigned long features)
+{
+ ops->dsc_disable = dpu_hw_dsc_disable_1_2;
+ ops->dsc_config = dpu_hw_dsc_config_1_2;
+ ops->dsc_config_thresh = dpu_hw_dsc_config_thresh_1_2;
+ ops->dsc_bind_pingpong_blk = dpu_hw_dsc_bind_pingpong_blk_1_2;
+}
+
+struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
+ void __iomem *addr)
+{
+ struct dpu_hw_dsc *c;
+
+ c = kzalloc(sizeof(*c), GFP_KERNEL);
+ if (!c)
+ return ERR_PTR(-ENOMEM);
+
+ c->hw.blk_addr = addr + cfg->base;
+ c->hw.log_mask = DPU_DBG_MASK_DSC;
+
+ c->idx = cfg->id;
+ c->caps = cfg;
+ _setup_dcs_ops_1_2(&c->ops, c->caps->features);
+
+ return c;
+}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
index f0fc704..502dd60 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt) "[drm:%s] " fmt, __func__
@@ -246,7 +247,11 @@ int dpu_rm_init(struct dpu_rm *rm,
struct dpu_hw_dsc *hw;
const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
- hw = dpu_hw_dsc_init(dsc, mmio);
+ if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
+ hw = dpu_hw_dsc_init_1_2(dsc, mmio);
+ else
+ hw = dpu_hw_dsc_init(dsc, mmio);
+
if (IS_ERR_OR_NULL(hw)) {
rc = PTR_ERR(hw);
DPU_ERROR("failed dsc object creation: err %d\n", rc);
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine
2023-05-17 22:01 ` [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine Kuogee Hsieh
@ 2023-05-17 22:22 ` Marijn Suijten
2023-05-18 0:32 ` Dmitry Baryshkov
0 siblings, 1 reply; 24+ messages in thread
From: Marijn Suijten @ 2023-05-17 22:22 UTC (permalink / raw)
To: Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
On 2023-05-17 15:01:56, Kuogee Hsieh wrote:
> Add support for DSC 1.2 by providing the necessary hooks to program
> the DPU DSC 1.2 encoder.
>
> Changes in v3:
> -- fixed kernel test rebot report that "__iomem *off" is declared but not
> used at dpu_hw_dsc_config_1_2()
> -- unrolling thresh loops
>
> Changes in v4:
> -- delete DPU_DSC_HW_REV_1_1
> -- delete off and used real register name directly
>
> Changes in v7:
> -- replace offset with sblk->enc.base
> -- replace ss with slice
>
> Changes in v8:
> -- fixed checkpatch warning
>
> Changes in v9:
> -- replaced __dsc_calc_ob_max_addr() with __dsc_calc_output_buf_max_addr()
> -- replaced variable num_ss with num_softslice
> -- remove inline from function declaration
>
> changes in v10:
> -- rewording text of changes in v9
> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
Besides this rename and commit-body fix, you skipped all my other
comments in v9. Do I really have to repeat them?
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/Makefile | 1 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 31 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 386 +++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
> 5 files changed, 435 insertions(+), 4 deletions(-)
> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>
> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
> index b814fc8..b9af5e4 100644
> --- a/drivers/gpu/drm/msm/Makefile
> +++ b/drivers/gpu/drm/msm/Makefile
> @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
> disp/dpu1/dpu_hw_catalog.o \
> disp/dpu1/dpu_hw_ctl.o \
> disp/dpu1/dpu_hw_dsc.o \
> + disp/dpu1/dpu_hw_dsc_1_2.o \
> disp/dpu1/dpu_hw_interrupts.o \
> disp/dpu1/dpu_hw_intf.o \
> disp/dpu1/dpu_hw_lm.o \
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> index 83854e8..11610f7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> /*
> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
> * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
> */
>
> @@ -244,12 +244,18 @@ enum {
> };
>
> /**
> - * DSC features
> + * DSC sub-blocks/features
> * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
> * the pixel output from this DSC.
> + * @DPU_DSC_HW_REV_1_2 DSC block supports DSC 1.1 and 1.2
> + * @DPU_DSC_NATIVE_42x_EN Supports NATIVE_422_EN and NATIVE_420_EN encoding
> + * @DPU_DSC_MAX
> */
> enum {
> DPU_DSC_OUTPUT_CTRL = 0x1,
> + DPU_DSC_HW_REV_1_2,
> + DPU_DSC_NATIVE_42x_EN,
> + DPU_DSC_MAX
> };
>
> /**
> @@ -306,6 +312,14 @@ struct dpu_pp_blk {
> };
>
> /**
> + * struct dpu_dsc_blk - DSC Encoder sub-blk information
> + * @info: HW register and features supported by this sub-blk
> + */
> +struct dpu_dsc_blk {
> + DPU_HW_SUBBLK_INFO;
> +};
> +
> +/**
> * enum dpu_qos_lut_usage - define QoS LUT use cases
> */
> enum dpu_qos_lut_usage {
> @@ -452,6 +466,16 @@ struct dpu_pingpong_sub_blks {
> };
>
> /**
> + * struct dpu_dsc_sub_blks - DSC sub-blks
> + * @enc: DSC encoder sub-block
> + * @ctl: DSC controller sub-block
> + */
> +struct dpu_dsc_sub_blks {
> + struct dpu_dsc_blk enc;
> + struct dpu_dsc_blk ctl;
> +};
> +
> +/**
> * dpu_clk_ctrl_type - Defines top level clock control signals
> */
> enum dpu_clk_ctrl_type {
> @@ -605,10 +629,13 @@ struct dpu_merge_3d_cfg {
> * struct dpu_dsc_cfg - information of DSC blocks
> * @id enum identifying this block
> * @base register offset of this block
> + * @len: length of hardware block
> * @features bit mask identifying sub-blocks/features
> + * @sblk: sub-blocks information
> */
> struct dpu_dsc_cfg {
> DPU_HW_BLK_INFO;
> + const struct dpu_dsc_sub_blks *sblk;
> };
>
> /**
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> index 138080a..41e39d0 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
> @@ -1,5 +1,8 @@
> /* SPDX-License-Identifier: GPL-2.0-only */
> -/* Copyright (c) 2020-2022, Linaro Limited */
> +/*
> + * Copyright (c) 2020-2022, Linaro Limited
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
> + */
>
> #ifndef _DPU_HW_DSC_H
> #define _DPU_HW_DSC_H
> @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg,
> void __iomem *addr);
>
> /**
> + * dpu_hw_dsc_init_1_2() - initializes the v1.2 DSC hw driver block
DSC hw driver object. (v8 review)
> + * @cfg: DSC catalog entry for which driver object is required
> + * @addr: Mapped register io address of MDP
> + * Returns: Error code or allocated dpu_hw_dsc context
> + */
> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
> + void __iomem *addr);
> +
> +/**
> * dpu_hw_dsc_destroy - destroys dsc driver context
> * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
> */
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
> new file mode 100644
> index 00000000..c3f1998
> --- /dev/null
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
> @@ -0,0 +1,386 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
> + */
> +
> +#include <drm/display/drm_dsc_helper.h>
> +
> +#include "dpu_kms.h"
> +#include "dpu_hw_catalog.h"
> +#include "dpu_hwio.h"
> +#include "dpu_hw_mdss.h"
> +#include "dpu_hw_dsc.h"
> +
> +#define DSC_CMN_MAIN_CNF 0x00
> +
> +/* DPU_DSC_ENC register offsets */
> +#define ENC_DF_CTRL 0x00
> +#define ENC_GENERAL_STATUS 0x04
> +#define ENC_HSLICE_STATUS 0x08
> +#define ENC_OUT_STATUS 0x0C
> +#define ENC_INT_STAT 0x10
> +#define ENC_INT_CLR 0x14
> +#define ENC_INT_MASK 0x18
> +#define DSC_MAIN_CONF 0x30
> +#define DSC_PICTURE_SIZE 0x34
> +#define DSC_SLICE_SIZE 0x38
> +#define DSC_MISC_SIZE 0x3C
> +#define DSC_HRD_DELAYS 0x40
> +#define DSC_RC_SCALE 0x44
> +#define DSC_RC_SCALE_INC_DEC 0x48
> +#define DSC_RC_OFFSETS_1 0x4C
> +#define DSC_RC_OFFSETS_2 0x50
> +#define DSC_RC_OFFSETS_3 0x54
> +#define DSC_RC_OFFSETS_4 0x58
> +#define DSC_FLATNESS_QP 0x5C
> +#define DSC_RC_MODEL_SIZE 0x60
> +#define DSC_RC_CONFIG 0x64
> +#define DSC_RC_BUF_THRESH_0 0x68
> +#define DSC_RC_BUF_THRESH_1 0x6C
> +#define DSC_RC_BUF_THRESH_2 0x70
> +#define DSC_RC_BUF_THRESH_3 0x74
> +#define DSC_RC_MIN_QP_0 0x78
> +#define DSC_RC_MIN_QP_1 0x7C
> +#define DSC_RC_MIN_QP_2 0x80
> +#define DSC_RC_MAX_QP_0 0x84
> +#define DSC_RC_MAX_QP_1 0x88
> +#define DSC_RC_MAX_QP_2 0x8C
> +#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
> +#define DSC_RC_RANGE_BPG_OFFSETS_1 0x94
> +#define DSC_RC_RANGE_BPG_OFFSETS_2 0x98
> +
> +/* DPU_DSC_CTL register offsets */
> +#define DSC_CTL 0x00
> +#define DSC_CFG 0x04
> +#define DSC_DATA_IN_SWAP 0x08
> +#define DSC_CLK_CTRL 0x0C
> +
> +static int _dsc_calc_output_buf_max_addr(struct dpu_hw_dsc *hw_dsc, int num_softslice)
> +{
> + int max_addr = 2400 / num_softslice;
> +
> + if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_42x_EN))
> + max_addr /= 2;
> +
> + return max_addr - 1;
> +};
> +
> +static void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
> +{
> + struct dpu_hw_blk_reg_map *hw;
> + const struct dpu_dsc_sub_blks *sblk;
> +
> + if (!hw_dsc)
> + return;
> +
> + hw = &hw_dsc->hw;
> + sblk = hw_dsc->caps->sblk;
> + DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CFG, 0);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + ENC_DF_CTRL, 0);
> + DPU_REG_WRITE(hw, sblk->enc.base + 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;
> + const struct dpu_dsc_sub_blks *sblk;
> + u32 data = 0;
> + u32 det_thresh_flatness;
> + u32 num_active_slice_per_enc;
> + u32 bpp;
> +
> + if (!hw_dsc || !dsc)
> + return;
> +
> + hw = &hw_dsc->hw;
> +
> + sblk = hw_dsc->caps->sblk;
> +
> + if (mode & DSC_MODE_SPLIT_PANEL)
> + data |= BIT(0);
> +
> + if (mode & DSC_MODE_MULTIPLEX)
> + data |= BIT(1);
> +
> + num_active_slice_per_enc = dsc->slice_count;
> + if (mode & DSC_MODE_MULTIPLEX)
> + num_active_slice_per_enc = dsc->slice_count / 2;
> +
> + data |= (num_active_slice_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_output_buf_max_addr(hw_dsc, num_active_slice_per_enc) << 18);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + 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 |= bpp << 10;
> +
> + if (dsc->block_pred_enable)
> + data |= BIT(20);
Use tabs for indentation like everywhere else. (v9 review)
> +
> + if (dsc->convert_rgb)
> + data |= BIT(4);
> +
> + data |= (dsc->line_buf_depth & 0xf) << 6;
> + data |= dsc->bits_per_component & 0xf;
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_MAIN_CONF, data);
> +
> + data = (dsc->pic_width & 0xffff) |
> + ((dsc->pic_height & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_PICTURE_SIZE, data);
> +
> + data = (dsc->slice_width & 0xffff) |
> + ((dsc->slice_height & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_SLICE_SIZE, data);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_MISC_SIZE,
> + (dsc->slice_chunk_size) & 0xffff);
> +
> + data = (dsc->initial_xmit_delay & 0xffff) |
> + ((dsc->initial_dec_delay & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_HRD_DELAYS, data);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE,
> + dsc->initial_scale_value & 0x3f);
> +
> + data = (dsc->scale_increment_interval & 0xffff) |
> + ((dsc->scale_decrement_interval & 0x7ff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE_INC_DEC, data);
> +
> + data = (dsc->first_line_bpg_offset & 0x1f) |
> + ((dsc->second_line_bpg_offset & 0x1f) << 5);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_1, data);
> +
> + data = (dsc->nfl_bpg_offset & 0xffff) |
> + ((dsc->slice_bpg_offset & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_2, data);
> +
> + data = (dsc->initial_offset & 0xffff) |
> + ((dsc->final_offset & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_3, data);
> +
> + data = (dsc->nsl_bpg_offset & 0xffff) |
> + ((dsc->second_line_offset_adj & 0xffff) << 16);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_4, data);
> +
> + det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
Don't forget that Jessica renamed this to drm_dsc_flatness_det_thresh()
in v10. (v9 review)
> + data = (dsc->flatness_min_qp & 0x1f) | ((dsc->flatness_max_qp & 0x1f) << 5) |
> + ((det_thresh_flatness & 0xff) << 10);
Thanks for updating this, but while it might fit under 100 chars this is
perhaps clearer to read when split out over 3 lines. (v9 review):
data = (dsc->flatness_min_qp & 0x1f) |
((dsc->flatness_max_qp & 0x1f) << 5) |
((det_thresh_flatness & 0xff) << 10);
- Marijn
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_FLATNESS_QP, data);
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + 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, sblk->enc.base + DSC_RC_CONFIG, data);
> +
> + /* program the dsc wrapper */
> + 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, sblk->ctl.base + 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;
> + const struct dpu_dsc_sub_blks *sblk;
> + struct drm_dsc_rc_range_parameters *rc;
> +
> + if (!hw_dsc || !dsc)
> + return;
> +
> + hw = &hw_dsc->hw;
> +
> + sblk = hw_dsc->caps->sblk;
> +
> + rc = dsc->rc_range_params;
> +
> + /*
> + * With BUF_THRESH -- 14 in total
> + * each register contains 4 thresh values with the last register
> + * containing only 2 thresh values
> + */
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_0,
> + (dsc->rc_buf_thresh[0] << 0) |
> + (dsc->rc_buf_thresh[1] << 8) |
> + (dsc->rc_buf_thresh[2] << 16) |
> + (dsc->rc_buf_thresh[3] << 24));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_1,
> + (dsc->rc_buf_thresh[4] << 0) |
> + (dsc->rc_buf_thresh[5] << 8) |
> + (dsc->rc_buf_thresh[6] << 16) |
> + (dsc->rc_buf_thresh[7] << 24));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_2,
> + (dsc->rc_buf_thresh[8] << 0) |
> + (dsc->rc_buf_thresh[9] << 8) |
> + (dsc->rc_buf_thresh[10] << 16) |
> + (dsc->rc_buf_thresh[11] << 24));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_3,
> + (dsc->rc_buf_thresh[12] << 0) |
> + (dsc->rc_buf_thresh[13] << 8));
> +
> + /*
> + * with min/max_QP -- 5 bits
> + * each register contains 5 min_qp or max_qp for total of 15
> + *
> + * With BPG_OFFSET -- 6 bits
> + * each register contains 5 BPG_offset for total of 15
> + */
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_0,
> + (rc[0].range_min_qp << 0) |
> + (rc[1].range_min_qp << 5) |
> + (rc[2].range_min_qp << 10) |
> + (rc[3].range_min_qp << 15) |
> + (rc[4].range_min_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_0,
> + (rc[0].range_max_qp << 0) |
> + (rc[1].range_max_qp << 5) |
> + (rc[2].range_max_qp << 10) |
> + (rc[3].range_max_qp << 15) |
> + (rc[4].range_max_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_0,
> + (rc[0].range_bpg_offset << 0) |
> + (rc[1].range_bpg_offset << 6) |
> + (rc[2].range_bpg_offset << 12) |
> + (rc[3].range_bpg_offset << 18) |
> + (rc[4].range_bpg_offset << 24));
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_1,
> + (rc[5].range_min_qp << 0) |
> + (rc[6].range_min_qp << 5) |
> + (rc[7].range_min_qp << 10) |
> + (rc[8].range_min_qp << 15) |
> + (rc[9].range_min_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_1,
> + (rc[5].range_max_qp << 0) |
> + (rc[6].range_max_qp << 5) |
> + (rc[7].range_max_qp << 10) |
> + (rc[8].range_max_qp << 15) |
> + (rc[9].range_max_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_1,
> + (rc[5].range_bpg_offset << 0) |
> + (rc[6].range_bpg_offset << 6) |
> + (rc[7].range_bpg_offset << 12) |
> + (rc[8].range_bpg_offset << 18) |
> + (rc[9].range_bpg_offset << 24));
> +
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_2,
> + (rc[10].range_min_qp << 0) |
> + (rc[11].range_min_qp << 5) |
> + (rc[12].range_min_qp << 10) |
> + (rc[13].range_min_qp << 15) |
> + (rc[14].range_min_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_2,
> + (rc[10].range_max_qp << 0) |
> + (rc[11].range_max_qp << 5) |
> + (rc[12].range_max_qp << 10) |
> + (rc[13].range_max_qp << 15) |
> + (rc[14].range_max_qp << 20));
> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_2,
> + (rc[10].range_bpg_offset << 0) |
> + (rc[11].range_bpg_offset << 6) |
> + (rc[12].range_bpg_offset << 12) |
> + (rc[13].range_bpg_offset << 18) |
> + (rc[14].range_bpg_offset << 24));
> +}
> +
> +static void dpu_hw_dsc_bind_pingpong_blk_1_2(struct dpu_hw_dsc *hw_dsc,
> + const enum dpu_pingpong pp)
> +{
> + struct dpu_hw_blk_reg_map *hw;
> + const struct dpu_dsc_sub_blks *sblk;
> + int mux_cfg = 0xf; /* Disabled */
> +
> + hw = &hw_dsc->hw;
> +
> + sblk = hw_dsc->caps->sblk;
> +
> + if (pp)
> + mux_cfg = (pp - PINGPONG_0) & 0x7;
> +
> + DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CTL, mux_cfg);
> +}
> +
> +static void _setup_dcs_ops_1_2(struct dpu_hw_dsc_ops *ops,
> + const unsigned long features)
> +{
> + ops->dsc_disable = dpu_hw_dsc_disable_1_2;
> + ops->dsc_config = dpu_hw_dsc_config_1_2;
> + ops->dsc_config_thresh = dpu_hw_dsc_config_thresh_1_2;
> + ops->dsc_bind_pingpong_blk = dpu_hw_dsc_bind_pingpong_blk_1_2;
> +}
> +
> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
> + void __iomem *addr)
> +{
> + struct dpu_hw_dsc *c;
> +
> + c = kzalloc(sizeof(*c), GFP_KERNEL);
> + if (!c)
> + return ERR_PTR(-ENOMEM);
> +
> + c->hw.blk_addr = addr + cfg->base;
> + c->hw.log_mask = DPU_DBG_MASK_DSC;
> +
> + c->idx = cfg->id;
> + c->caps = cfg;
> + _setup_dcs_ops_1_2(&c->ops, c->caps->features);
> +
> + return c;
> +}
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> index f0fc704..502dd60 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
> @@ -1,6 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #define pr_fmt(fmt) "[drm:%s] " fmt, __func__
> @@ -246,7 +247,11 @@ int dpu_rm_init(struct dpu_rm *rm,
> struct dpu_hw_dsc *hw;
> const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
>
> - hw = dpu_hw_dsc_init(dsc, mmio);
> + if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
> + hw = dpu_hw_dsc_init_1_2(dsc, mmio);
> + else
> + hw = dpu_hw_dsc_init(dsc, mmio);
> +
> if (IS_ERR_OR_NULL(hw)) {
> rc = PTR_ERR(hw);
> DPU_ERROR("failed dsc object creation: err %d\n", rc);
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine
2023-05-17 22:22 ` Marijn Suijten
@ 2023-05-18 0:32 ` Dmitry Baryshkov
0 siblings, 0 replies; 24+ messages in thread
From: Dmitry Baryshkov @ 2023-05-18 0:32 UTC (permalink / raw)
To: Marijn Suijten, Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, andersson, quic_abhinavk, quic_jesszhan,
quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 18/05/2023 01:22, Marijn Suijten wrote:
> On 2023-05-17 15:01:56, Kuogee Hsieh wrote:
>> Add support for DSC 1.2 by providing the necessary hooks to program
>> the DPU DSC 1.2 encoder.
>>
>> Changes in v3:
>> -- fixed kernel test rebot report that "__iomem *off" is declared but not
>> used at dpu_hw_dsc_config_1_2()
>> -- unrolling thresh loops
>>
>> Changes in v4:
>> -- delete DPU_DSC_HW_REV_1_1
>> -- delete off and used real register name directly
>>
>> Changes in v7:
>> -- replace offset with sblk->enc.base
>> -- replace ss with slice
>>
>> Changes in v8:
>> -- fixed checkpatch warning
>>
>> Changes in v9:
>> -- replaced __dsc_calc_ob_max_addr() with __dsc_calc_output_buf_max_addr()
>> -- replaced variable num_ss with num_softslice
>> -- remove inline from function declaration
>>
>> changes in v10:
>> -- rewording text of changes in v9
>> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
>
> Besides this rename and commit-body fix, you skipped all my other
> comments in v9. Do I really have to repeat them?
>
>>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> drivers/gpu/drm/msm/Makefile | 1 +
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 31 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h | 14 +-
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 386 +++++++++++++++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c | 7 +-
>> 5 files changed, 435 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index b814fc8..b9af5e4 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -65,6 +65,7 @@ msm-$(CONFIG_DRM_MSM_DPU) += \
>> disp/dpu1/dpu_hw_catalog.o \
>> disp/dpu1/dpu_hw_ctl.o \
>> disp/dpu1/dpu_hw_dsc.o \
>> + disp/dpu1/dpu_hw_dsc_1_2.o \
>> disp/dpu1/dpu_hw_interrupts.o \
>> disp/dpu1/dpu_hw_intf.o \
>> disp/dpu1/dpu_hw_lm.o \
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> index 83854e8..11610f7 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
>> @@ -1,6 +1,6 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>> /*
>> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved.
>> */
>>
>> @@ -244,12 +244,18 @@ enum {
>> };
>>
>> /**
>> - * DSC features
>> + * DSC sub-blocks/features
>> * @DPU_DSC_OUTPUT_CTRL Configure which PINGPONG block gets
>> * the pixel output from this DSC.
>> + * @DPU_DSC_HW_REV_1_2 DSC block supports DSC 1.1 and 1.2
>> + * @DPU_DSC_NATIVE_42x_EN Supports NATIVE_422_EN and NATIVE_420_EN encoding
>> + * @DPU_DSC_MAX
>> */
>> enum {
>> DPU_DSC_OUTPUT_CTRL = 0x1,
>> + DPU_DSC_HW_REV_1_2,
>> + DPU_DSC_NATIVE_42x_EN,
>> + DPU_DSC_MAX
>> };
>>
>> /**
>> @@ -306,6 +312,14 @@ struct dpu_pp_blk {
>> };
>>
>> /**
>> + * struct dpu_dsc_blk - DSC Encoder sub-blk information
>> + * @info: HW register and features supported by this sub-blk
>> + */
>> +struct dpu_dsc_blk {
>> + DPU_HW_SUBBLK_INFO;
>> +};
>> +
>> +/**
>> * enum dpu_qos_lut_usage - define QoS LUT use cases
>> */
>> enum dpu_qos_lut_usage {
>> @@ -452,6 +466,16 @@ struct dpu_pingpong_sub_blks {
>> };
>>
>> /**
>> + * struct dpu_dsc_sub_blks - DSC sub-blks
>> + * @enc: DSC encoder sub-block
>> + * @ctl: DSC controller sub-block
>> + */
>> +struct dpu_dsc_sub_blks {
>> + struct dpu_dsc_blk enc;
>> + struct dpu_dsc_blk ctl;
>> +};
>> +
>> +/**
>> * dpu_clk_ctrl_type - Defines top level clock control signals
>> */
>> enum dpu_clk_ctrl_type {
>> @@ -605,10 +629,13 @@ struct dpu_merge_3d_cfg {
>> * struct dpu_dsc_cfg - information of DSC blocks
>> * @id enum identifying this block
>> * @base register offset of this block
>> + * @len: length of hardware block
>> * @features bit mask identifying sub-blocks/features
>> + * @sblk: sub-blocks information
>> */
>> struct dpu_dsc_cfg {
>> DPU_HW_BLK_INFO;
>> + const struct dpu_dsc_sub_blks *sblk;
>> };
>>
>> /**
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>> index 138080a..41e39d0 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc.h
>> @@ -1,5 +1,8 @@
>> /* SPDX-License-Identifier: GPL-2.0-only */
>> -/* Copyright (c) 2020-2022, Linaro Limited */
>> +/*
>> + * Copyright (c) 2020-2022, Linaro Limited
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
>> + */
>>
>> #ifndef _DPU_HW_DSC_H
>> #define _DPU_HW_DSC_H
>> @@ -69,6 +72,15 @@ struct dpu_hw_dsc *dpu_hw_dsc_init(const struct dpu_dsc_cfg *cfg,
>> void __iomem *addr);
>>
>> /**
>> + * dpu_hw_dsc_init_1_2() - initializes the v1.2 DSC hw driver block
>
> DSC hw driver object. (v8 review)
>
>> + * @cfg: DSC catalog entry for which driver object is required
>> + * @addr: Mapped register io address of MDP
>> + * Returns: Error code or allocated dpu_hw_dsc context
>> + */
>> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
>> + void __iomem *addr);
>> +
>> +/**
>> * dpu_hw_dsc_destroy - destroys dsc driver context
>> * @dsc: Pointer to dsc driver context returned by dpu_hw_dsc_init
>> */
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>> new file mode 100644
>> index 00000000..c3f1998
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
>> @@ -0,0 +1,386 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2020-2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved
>> + */
>> +
>> +#include <drm/display/drm_dsc_helper.h>
>> +
>> +#include "dpu_kms.h"
>> +#include "dpu_hw_catalog.h"
>> +#include "dpu_hwio.h"
>> +#include "dpu_hw_mdss.h"
>> +#include "dpu_hw_dsc.h"
>> +
>> +#define DSC_CMN_MAIN_CNF 0x00
>> +
>> +/* DPU_DSC_ENC register offsets */
>> +#define ENC_DF_CTRL 0x00
>> +#define ENC_GENERAL_STATUS 0x04
>> +#define ENC_HSLICE_STATUS 0x08
>> +#define ENC_OUT_STATUS 0x0C
>> +#define ENC_INT_STAT 0x10
>> +#define ENC_INT_CLR 0x14
>> +#define ENC_INT_MASK 0x18
>> +#define DSC_MAIN_CONF 0x30
>> +#define DSC_PICTURE_SIZE 0x34
>> +#define DSC_SLICE_SIZE 0x38
>> +#define DSC_MISC_SIZE 0x3C
>> +#define DSC_HRD_DELAYS 0x40
>> +#define DSC_RC_SCALE 0x44
>> +#define DSC_RC_SCALE_INC_DEC 0x48
>> +#define DSC_RC_OFFSETS_1 0x4C
>> +#define DSC_RC_OFFSETS_2 0x50
>> +#define DSC_RC_OFFSETS_3 0x54
>> +#define DSC_RC_OFFSETS_4 0x58
>> +#define DSC_FLATNESS_QP 0x5C
>> +#define DSC_RC_MODEL_SIZE 0x60
>> +#define DSC_RC_CONFIG 0x64
>> +#define DSC_RC_BUF_THRESH_0 0x68
>> +#define DSC_RC_BUF_THRESH_1 0x6C
>> +#define DSC_RC_BUF_THRESH_2 0x70
>> +#define DSC_RC_BUF_THRESH_3 0x74
>> +#define DSC_RC_MIN_QP_0 0x78
>> +#define DSC_RC_MIN_QP_1 0x7C
>> +#define DSC_RC_MIN_QP_2 0x80
>> +#define DSC_RC_MAX_QP_0 0x84
>> +#define DSC_RC_MAX_QP_1 0x88
>> +#define DSC_RC_MAX_QP_2 0x8C
>> +#define DSC_RC_RANGE_BPG_OFFSETS_0 0x90
>> +#define DSC_RC_RANGE_BPG_OFFSETS_1 0x94
>> +#define DSC_RC_RANGE_BPG_OFFSETS_2 0x98
>> +
>> +/* DPU_DSC_CTL register offsets */
>> +#define DSC_CTL 0x00
>> +#define DSC_CFG 0x04
>> +#define DSC_DATA_IN_SWAP 0x08
>> +#define DSC_CLK_CTRL 0x0C
>> +
>> +static int _dsc_calc_output_buf_max_addr(struct dpu_hw_dsc *hw_dsc, int num_softslice)
>> +{
>> + int max_addr = 2400 / num_softslice;
>> +
>> + if (hw_dsc->caps->features & BIT(DPU_DSC_NATIVE_42x_EN))
>> + max_addr /= 2;
>> +
>> + return max_addr - 1;
>> +};
>> +
>> +static void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
>> +{
>> + struct dpu_hw_blk_reg_map *hw;
>> + const struct dpu_dsc_sub_blks *sblk;
>> +
>> + if (!hw_dsc)
>> + return;
>> +
>> + hw = &hw_dsc->hw;
>> + sblk = hw_dsc->caps->sblk;
>> + DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CFG, 0);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + ENC_DF_CTRL, 0);
>> + DPU_REG_WRITE(hw, sblk->enc.base + 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;
>> + const struct dpu_dsc_sub_blks *sblk;
>> + u32 data = 0;
>> + u32 det_thresh_flatness;
>> + u32 num_active_slice_per_enc;
>> + u32 bpp;
>> +
>> + if (!hw_dsc || !dsc)
>> + return;
>> +
>> + hw = &hw_dsc->hw;
>> +
>> + sblk = hw_dsc->caps->sblk;
>> +
>> + if (mode & DSC_MODE_SPLIT_PANEL)
>> + data |= BIT(0);
>> +
>> + if (mode & DSC_MODE_MULTIPLEX)
>> + data |= BIT(1);
>> +
>> + num_active_slice_per_enc = dsc->slice_count;
>> + if (mode & DSC_MODE_MULTIPLEX)
>> + num_active_slice_per_enc = dsc->slice_count / 2;
>> +
>> + data |= (num_active_slice_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_output_buf_max_addr(hw_dsc, num_active_slice_per_enc) << 18);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + 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 |= bpp << 10;
>> +
>> + if (dsc->block_pred_enable)
>> + data |= BIT(20);
>
> Use tabs for indentation like everywhere else. (v9 review)
I'd say: please use checkpatch.pl --strict when preparing the patchset.
>
>> +
>> + if (dsc->convert_rgb)
>> + data |= BIT(4);
>> +
>> + data |= (dsc->line_buf_depth & 0xf) << 6;
>> + data |= dsc->bits_per_component & 0xf;
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_MAIN_CONF, data);
>> +
>> + data = (dsc->pic_width & 0xffff) |
>> + ((dsc->pic_height & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_PICTURE_SIZE, data);
>> +
>> + data = (dsc->slice_width & 0xffff) |
>> + ((dsc->slice_height & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_SLICE_SIZE, data);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_MISC_SIZE,
>> + (dsc->slice_chunk_size) & 0xffff);
>> +
>> + data = (dsc->initial_xmit_delay & 0xffff) |
>> + ((dsc->initial_dec_delay & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_HRD_DELAYS, data);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE,
>> + dsc->initial_scale_value & 0x3f);
>> +
>> + data = (dsc->scale_increment_interval & 0xffff) |
>> + ((dsc->scale_decrement_interval & 0x7ff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_SCALE_INC_DEC, data);
>> +
>> + data = (dsc->first_line_bpg_offset & 0x1f) |
>> + ((dsc->second_line_bpg_offset & 0x1f) << 5);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_1, data);
>> +
>> + data = (dsc->nfl_bpg_offset & 0xffff) |
>> + ((dsc->slice_bpg_offset & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_2, data);
>> +
>> + data = (dsc->initial_offset & 0xffff) |
>> + ((dsc->final_offset & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_3, data);
>> +
>> + data = (dsc->nsl_bpg_offset & 0xffff) |
>> + ((dsc->second_line_offset_adj & 0xffff) << 16);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_OFFSETS_4, data);
>> +
>> + det_thresh_flatness = drm_dsc_calculate_flatness_det_thresh(dsc);
>
> Don't forget that Jessica renamed this to drm_dsc_flatness_det_thresh()
> in v10. (v9 review)
>
>> + data = (dsc->flatness_min_qp & 0x1f) | ((dsc->flatness_max_qp & 0x1f) << 5) |
>> + ((det_thresh_flatness & 0xff) << 10);
>
> Thanks for updating this, but while it might fit under 100 chars this is
> perhaps clearer to read when split out over 3 lines. (v9 review):
>
> data = (dsc->flatness_min_qp & 0x1f) |
> ((dsc->flatness_max_qp & 0x1f) << 5) |
> ((det_thresh_flatness & 0xff) << 10);
With the proper alignment, please.
>
> - Marijn
>
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_FLATNESS_QP, data);
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + 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, sblk->enc.base + DSC_RC_CONFIG, data);
>> +
>> + /* program the dsc wrapper */
>> + 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, sblk->ctl.base + 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;
>> + const struct dpu_dsc_sub_blks *sblk;
>> + struct drm_dsc_rc_range_parameters *rc;
>> +
>> + if (!hw_dsc || !dsc)
>> + return;
>> +
>> + hw = &hw_dsc->hw;
>> +
>> + sblk = hw_dsc->caps->sblk;
>> +
>> + rc = dsc->rc_range_params;
>> +
>> + /*
>> + * With BUF_THRESH -- 14 in total
>> + * each register contains 4 thresh values with the last register
>> + * containing only 2 thresh values
>> + */
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_0,
>> + (dsc->rc_buf_thresh[0] << 0) |
>> + (dsc->rc_buf_thresh[1] << 8) |
>> + (dsc->rc_buf_thresh[2] << 16) |
>> + (dsc->rc_buf_thresh[3] << 24));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_1,
>> + (dsc->rc_buf_thresh[4] << 0) |
>> + (dsc->rc_buf_thresh[5] << 8) |
>> + (dsc->rc_buf_thresh[6] << 16) |
>> + (dsc->rc_buf_thresh[7] << 24));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_2,
>> + (dsc->rc_buf_thresh[8] << 0) |
>> + (dsc->rc_buf_thresh[9] << 8) |
>> + (dsc->rc_buf_thresh[10] << 16) |
>> + (dsc->rc_buf_thresh[11] << 24));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_BUF_THRESH_3,
>> + (dsc->rc_buf_thresh[12] << 0) |
>> + (dsc->rc_buf_thresh[13] << 8));
>> +
>> + /*
>> + * with min/max_QP -- 5 bits
>> + * each register contains 5 min_qp or max_qp for total of 15
>> + *
>> + * With BPG_OFFSET -- 6 bits
>> + * each register contains 5 BPG_offset for total of 15
>> + */
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_0,
>> + (rc[0].range_min_qp << 0) |
>> + (rc[1].range_min_qp << 5) |
>> + (rc[2].range_min_qp << 10) |
>> + (rc[3].range_min_qp << 15) |
>> + (rc[4].range_min_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_0,
>> + (rc[0].range_max_qp << 0) |
>> + (rc[1].range_max_qp << 5) |
>> + (rc[2].range_max_qp << 10) |
>> + (rc[3].range_max_qp << 15) |
>> + (rc[4].range_max_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_0,
>> + (rc[0].range_bpg_offset << 0) |
>> + (rc[1].range_bpg_offset << 6) |
>> + (rc[2].range_bpg_offset << 12) |
>> + (rc[3].range_bpg_offset << 18) |
>> + (rc[4].range_bpg_offset << 24));
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_1,
>> + (rc[5].range_min_qp << 0) |
>> + (rc[6].range_min_qp << 5) |
>> + (rc[7].range_min_qp << 10) |
>> + (rc[8].range_min_qp << 15) |
>> + (rc[9].range_min_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_1,
>> + (rc[5].range_max_qp << 0) |
>> + (rc[6].range_max_qp << 5) |
>> + (rc[7].range_max_qp << 10) |
>> + (rc[8].range_max_qp << 15) |
>> + (rc[9].range_max_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_1,
>> + (rc[5].range_bpg_offset << 0) |
>> + (rc[6].range_bpg_offset << 6) |
>> + (rc[7].range_bpg_offset << 12) |
>> + (rc[8].range_bpg_offset << 18) |
>> + (rc[9].range_bpg_offset << 24));
>> +
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MIN_QP_2,
>> + (rc[10].range_min_qp << 0) |
>> + (rc[11].range_min_qp << 5) |
>> + (rc[12].range_min_qp << 10) |
>> + (rc[13].range_min_qp << 15) |
>> + (rc[14].range_min_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_MAX_QP_2,
>> + (rc[10].range_max_qp << 0) |
>> + (rc[11].range_max_qp << 5) |
>> + (rc[12].range_max_qp << 10) |
>> + (rc[13].range_max_qp << 15) |
>> + (rc[14].range_max_qp << 20));
>> + DPU_REG_WRITE(hw, sblk->enc.base + DSC_RC_RANGE_BPG_OFFSETS_2,
>> + (rc[10].range_bpg_offset << 0) |
>> + (rc[11].range_bpg_offset << 6) |
>> + (rc[12].range_bpg_offset << 12) |
>> + (rc[13].range_bpg_offset << 18) |
>> + (rc[14].range_bpg_offset << 24));
>> +}
>> +
>> +static void dpu_hw_dsc_bind_pingpong_blk_1_2(struct dpu_hw_dsc *hw_dsc,
>> + const enum dpu_pingpong pp)
>> +{
>> + struct dpu_hw_blk_reg_map *hw;
>> + const struct dpu_dsc_sub_blks *sblk;
>> + int mux_cfg = 0xf; /* Disabled */
>> +
>> + hw = &hw_dsc->hw;
>> +
>> + sblk = hw_dsc->caps->sblk;
>> +
>> + if (pp)
>> + mux_cfg = (pp - PINGPONG_0) & 0x7;
>> +
>> + DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CTL, mux_cfg);
>> +}
>> +
>> +static void _setup_dcs_ops_1_2(struct dpu_hw_dsc_ops *ops,
>> + const unsigned long features)
>> +{
>> + ops->dsc_disable = dpu_hw_dsc_disable_1_2;
>> + ops->dsc_config = dpu_hw_dsc_config_1_2;
>> + ops->dsc_config_thresh = dpu_hw_dsc_config_thresh_1_2;
>> + ops->dsc_bind_pingpong_blk = dpu_hw_dsc_bind_pingpong_blk_1_2;
>> +}
>> +
>> +struct dpu_hw_dsc *dpu_hw_dsc_init_1_2(const struct dpu_dsc_cfg *cfg,
>> + void __iomem *addr)
>> +{
>> + struct dpu_hw_dsc *c;
>> +
>> + c = kzalloc(sizeof(*c), GFP_KERNEL);
>> + if (!c)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + c->hw.blk_addr = addr + cfg->base;
>> + c->hw.log_mask = DPU_DBG_MASK_DSC;
>> +
>> + c->idx = cfg->id;
>> + c->caps = cfg;
>> + _setup_dcs_ops_1_2(&c->ops, c->caps->features);
>> +
>> + return c;
>> +}
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> index f0fc704..502dd60 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_rm.c
>> @@ -1,6 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /*
>> * Copyright (c) 2016-2018, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> #define pr_fmt(fmt) "[drm:%s] " fmt, __func__
>> @@ -246,7 +247,11 @@ int dpu_rm_init(struct dpu_rm *rm,
>> struct dpu_hw_dsc *hw;
>> const struct dpu_dsc_cfg *dsc = &cat->dsc[i];
>>
>> - hw = dpu_hw_dsc_init(dsc, mmio);
>> + if (test_bit(DPU_DSC_HW_REV_1_2, &dsc->features))
>> + hw = dpu_hw_dsc_init_1_2(dsc, mmio);
>> + else
>> + hw = dpu_hw_dsc_init(dsc, mmio);
>> +
>> if (IS_ERR_OR_NULL(hw)) {
>> rc = PTR_ERR(hw);
>> DPU_ERROR("failed dsc object creation: err %d\n", rc);
>> --
>> 2.7.4
>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
` (4 preceding siblings ...)
2023-05-17 22:01 ` [PATCH v10 5/8] drm/msm/dpu: add support for DSC encoder v1.2 engine Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:31 ` Marijn Suijten
2023-05-18 0:35 ` Dmitry Baryshkov
2023-05-17 22:01 ` [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Kuogee Hsieh
2023-05-17 22:01 ` [PATCH v10 8/8] drm/msm/dpu: tear down DSC data path when DSC disabled Kuogee Hsieh
7 siblings, 2 replies; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
Currently DSC flushing happens during interface configuration at
dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
to handle both per-DSC engine and DSC flush bits at same time to make it
consistent with the location of flush programming of other DPU sub-blocks.
Changes in v10:
-- rewording commit text
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index ffa6f04..1957545 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
return DIV_ROUND_UP(total_pixels, dsc->slice_width);
}
-static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
+static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
struct dpu_hw_pingpong *hw_pp,
struct drm_dsc_config *dsc,
u32 common_mode,
@@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
if (hw_pp->ops.enable_dsc)
hw_pp->ops.enable_dsc(hw_pp);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
}
static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
@@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
{
/* coding only for 2LM, 2enc, 1 dsc config */
struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
int this_frame_slices;
@@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
- dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
+ dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
+ dsc_common_mode, initial_lines);
}
void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4f7cfa9..4e132d9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
CTL_DSPP_n_FLUSH(dspp - DSPP_0),
ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
}
+
+ if (ctx->pending_flush_mask & BIT(DSC_IDX))
+ DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
+ ctx->pending_dsc_flush_mask);
+
DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
}
@@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
}
+static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
+ enum dpu_dsc dsc_num)
+{
+ ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
+ ctx->pending_flush_mask |= BIT(DSC_IDX);
+}
+
static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
enum dpu_dspp dspp, u32 dspp_sub_blk)
{
@@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
mode_sel = CTL_DEFAULT_GROUP_ID << 28;
- if (cfg->dsc)
- DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
-
if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
mode_sel |= BIT(17);
@@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
if (cfg->merge_3d)
DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
BIT(cfg->merge_3d - MERGE_3D_0));
- if (cfg->dsc) {
- DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
+
+ if (cfg->dsc)
DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
- }
}
static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
@@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
ops->update_pending_flush_merge_3d =
dpu_hw_ctl_update_pending_flush_merge_3d_v1;
ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
+ ops->update_pending_flush_dsc =
+ dpu_hw_ctl_update_pending_flush_dsc_v1;
} else {
ops->trigger_flush = dpu_hw_ctl_trigger_flush;
ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
index 6292002..d5f3ef8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
enum dpu_dspp blk, u32 dspp_sub_blk);
/**
+ * OR in the given flushbits to the cached pending_(dsc_)flush_mask
+ * No effect on hardware
+ * @ctx: ctl path ctx pointer
+ * @blk: interface block index
+ */
+ void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
+ enum dpu_dsc blk);
+
+ /**
* Write the value of the pending_flush_mask to hardware
* @ctx : ctl path ctx pointer
*/
@@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
* @pending_flush_mask: storage for pending ctl_flush managed via ops
* @pending_intf_flush_mask: pending INTF flush
* @pending_wb_flush_mask: pending WB flush
+ * @pending_merge_3d_flush_mask: pending merge_3d flush
+ * @pending_dspp_flush_mask: pending dspp flush
+ * @pending_dsc_flush_mask: pending dsc flush
* @ops: operation list
*/
struct dpu_hw_ctl {
@@ -245,6 +257,7 @@ struct dpu_hw_ctl {
u32 pending_wb_flush_mask;
u32 pending_merge_3d_flush_mask;
u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
+ u32 pending_dsc_flush_mask;
/* ops */
struct dpu_hw_ctl_ops ops;
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-17 22:01 ` [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface Kuogee Hsieh
@ 2023-05-17 22:31 ` Marijn Suijten
2023-05-18 22:09 ` Kuogee Hsieh
2023-05-18 0:35 ` Dmitry Baryshkov
1 sibling, 1 reply; 24+ messages in thread
From: Marijn Suijten @ 2023-05-17 22:31 UTC (permalink / raw)
To: Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
On 2023-05-17 15:01:57, Kuogee Hsieh wrote:
> Currently DSC flushing happens during interface configuration at
> dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
> dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
> to handle both per-DSC engine and DSC flush bits at same time to make it
> consistent with the location of flush programming of other DPU sub-blocks.
>
> Changes in v10:
> -- rewording commit text
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
There are a few things missing from v8 review, see below.
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
> 3 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ffa6f04..1957545 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
> return DIV_ROUND_UP(total_pixels, dsc->slice_width);
> }
>
> -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> struct dpu_hw_pingpong *hw_pp,
> struct drm_dsc_config *dsc,
> u32 common_mode,
> @@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
>
> if (hw_pp->ops.enable_dsc)
> hw_pp->ops.enable_dsc(hw_pp);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> }
>
> static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> @@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> {
> /* coding only for 2LM, 2enc, 1 dsc config */
> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> int this_frame_slices;
> @@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>
> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
> + dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
> + dsc_common_mode, initial_lines);
> }
>
> void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4f7cfa9..4e132d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> }
> +
> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> + ctx->pending_dsc_flush_mask);
Again, when do we reset this mask to 0? (v8 review)
> +
> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> }
>
> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> }
>
> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc dsc_num)
> +{
> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> + ctx->pending_flush_mask |= BIT(DSC_IDX);
> +}
> +
> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
> enum dpu_dspp dspp, u32 dspp_sub_blk)
> {
> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>
> - if (cfg->dsc)
> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> -
> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> mode_sel |= BIT(17);
>
> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if (cfg->merge_3d)
> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> BIT(cfg->merge_3d - MERGE_3D_0));
> - if (cfg->dsc) {
> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
separate Fixes: patch to have this semantic change documented. (v8
review)
> +
> + if (cfg->dsc)
> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> - }
> }
>
> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
> ops->update_pending_flush_merge_3d =
> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
> + ops->update_pending_flush_dsc =
> + dpu_hw_ctl_update_pending_flush_dsc_v1;
> } else {
> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 6292002..d5f3ef8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> enum dpu_dspp blk, u32 dspp_sub_blk);
>
> /**
> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> + * No effect on hardware
> + * @ctx: ctl path ctx pointer
> + * @blk: interface block index
> + */
> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc blk);
> +
> + /**
> * Write the value of the pending_flush_mask to hardware
> * @ctx : ctl path ctx pointer
> */
> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
> * @pending_flush_mask: storage for pending ctl_flush managed via ops
> * @pending_intf_flush_mask: pending INTF flush
> * @pending_wb_flush_mask: pending WB flush
The above is all capitalized, so...:
> + * @pending_merge_3d_flush_mask: pending merge_3d flush
MERGE_3D?
> + * @pending_dspp_flush_mask: pending dspp flush
DSPP
> + * @pending_dsc_flush_mask: pending dsc flush
DSC
- Marijn
> * @ops: operation list
> */
> struct dpu_hw_ctl {
> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
> u32 pending_wb_flush_mask;
> u32 pending_merge_3d_flush_mask;
> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
> + u32 pending_dsc_flush_mask;
>
> /* ops */
> struct dpu_hw_ctl_ops ops;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-17 22:31 ` Marijn Suijten
@ 2023-05-18 22:09 ` Kuogee Hsieh
2023-05-18 22:40 ` Dmitry Baryshkov
0 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-18 22:09 UTC (permalink / raw)
To: Marijn Suijten
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
On 5/17/2023 3:31 PM, Marijn Suijten wrote:
>
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
>> }
>> +
>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
>> + ctx->pending_dsc_flush_mask);
> Again, when do we reset this mask to 0? (v8 review)
can not find it.
let me add a separate patch to fix this.
>
>> +
>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>> }
>>
>> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
>> }
>>
>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
>> + enum dpu_dsc dsc_num)
>> +{
>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
>> +}
>> +
>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
>> enum dpu_dspp dspp, u32 dspp_sub_blk)
>> {
>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>>
>> - if (cfg->dsc)
>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
>> -
>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
>> mode_sel |= BIT(17);
>>
>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
>> if (cfg->merge_3d)
>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>> BIT(cfg->merge_3d - MERGE_3D_0));
>> - if (cfg->dsc) {
>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
> separate Fixes: patch to have this semantic change documented. (v8
> review)
That will be this patch. let me add Fixes at this patch
>
>> +
>> + if (cfg->dsc)
>> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
>> - }
>> }
>>
>> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
>> ops->update_pending_flush_merge_3d =
>> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
>> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
>> + ops->update_pending_flush_dsc =
>> + dpu_hw_ctl_update_pending_flush_dsc_v1;
>> } else {
>> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
>> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> index 6292002..d5f3ef8 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
>> enum dpu_dspp blk, u32 dspp_sub_blk);
>>
>> /**
>> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
>> + * No effect on hardware
>> + * @ctx: ctl path ctx pointer
>> + * @blk: interface block index
>> + */
>> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
>> + enum dpu_dsc blk);
>> +
>> + /**
>> * Write the value of the pending_flush_mask to hardware
>> * @ctx : ctl path ctx pointer
>> */
>> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
>> * @pending_flush_mask: storage for pending ctl_flush managed via ops
>> * @pending_intf_flush_mask: pending INTF flush
>> * @pending_wb_flush_mask: pending WB flush
> The above is all capitalized, so...:
>
>> + * @pending_merge_3d_flush_mask: pending merge_3d flush
> MERGE_3D?
>
>> + * @pending_dspp_flush_mask: pending dspp flush
> DSPP
>
>> + * @pending_dsc_flush_mask: pending dsc flush
> DSC
>
> - Marijn
>
>> * @ops: operation list
>> */
>> struct dpu_hw_ctl {
>> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
>> u32 pending_wb_flush_mask;
>> u32 pending_merge_3d_flush_mask;
>> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
>> + u32 pending_dsc_flush_mask;
>>
>> /* ops */
>> struct dpu_hw_ctl_ops ops;
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-18 22:09 ` Kuogee Hsieh
@ 2023-05-18 22:40 ` Dmitry Baryshkov
2023-05-19 12:04 ` Marijn Suijten
0 siblings, 1 reply; 24+ messages in thread
From: Dmitry Baryshkov @ 2023-05-18 22:40 UTC (permalink / raw)
To: Kuogee Hsieh, Marijn Suijten
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, andersson, quic_abhinavk, quic_jesszhan,
quic_sbillaka, freedreno, linux-arm-msm, linux-kernel
On 19/05/2023 01:09, Kuogee Hsieh wrote:
>
> On 5/17/2023 3:31 PM, Marijn Suijten wrote:
>>
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>>> @@ -139,6 +139,11 @@ static inline void
>>> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
>>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
>>> }
>>> +
>>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
>>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
>>> + ctx->pending_dsc_flush_mask);
>> Again, when do we reset this mask to 0? (v8 review)
>
> can not find it.
>
> let me add a separate patch to fix this.
The pending_dsc_flush_mask was added in this patch, so the reset should
be a part of this patch too.
>
>>
>>> +
>>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>>> }
>>> @@ -285,6 +290,13 @@ static void
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
>>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
>>> }
>>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl
>>> *ctx,
>>> + enum dpu_dsc dsc_num)
>>> +{
>>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
>>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
>>> +}
>>> +
>>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
>>> *ctx,
>>> enum dpu_dspp dspp, u32 dspp_sub_blk)
>>> {
>>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
>>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>>> - if (cfg->dsc)
>>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
>>> -
>>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
>>> mode_sel |= BIT(17);
>>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
>>> dpu_hw_ctl *ctx,
>>> if (cfg->merge_3d)
>>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
>>> BIT(cfg->merge_3d - MERGE_3D_0));
>>> - if (cfg->dsc) {
>>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
>> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
>> separate Fixes: patch to have this semantic change documented. (v8
>> review)
> That will be this patch. let me add Fixes at this patch
_separate_ patch.
>>
>>> +
>>> + if (cfg->dsc)
>>> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
>>> - }
>>> }
>>> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>>> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops
>>> *ops,
>>> ops->update_pending_flush_merge_3d =
>>> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
>>> ops->update_pending_flush_wb =
>>> dpu_hw_ctl_update_pending_flush_wb_v1;
>>> + ops->update_pending_flush_dsc =
>>> + dpu_hw_ctl_update_pending_flush_dsc_v1;
>>> } else {
>>> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
>>> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> index 6292002..d5f3ef8 100644
>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
>>> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
>>> enum dpu_dspp blk, u32 dspp_sub_blk);
>>> /**
>>> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
>>> + * No effect on hardware
>>> + * @ctx: ctl path ctx pointer
>>> + * @blk: interface block index
>>> + */
>>> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
>>> + enum dpu_dsc blk);
>>> +
>>> + /**
>>> * Write the value of the pending_flush_mask to hardware
>>> * @ctx : ctl path ctx pointer
>>> */
>>> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
>>> * @pending_flush_mask: storage for pending ctl_flush managed via ops
>>> * @pending_intf_flush_mask: pending INTF flush
>>> * @pending_wb_flush_mask: pending WB flush
>> The above is all capitalized, so...:
>>
>>> + * @pending_merge_3d_flush_mask: pending merge_3d flush
>> MERGE_3D?
>>
>>> + * @pending_dspp_flush_mask: pending dspp flush
>> DSPP
>>
>>> + * @pending_dsc_flush_mask: pending dsc flush
>> DSC
>>
>> - Marijn
>>
>>> * @ops: operation list
>>> */
>>> struct dpu_hw_ctl {
>>> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
>>> u32 pending_wb_flush_mask;
>>> u32 pending_merge_3d_flush_mask;
>>> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
>>> + u32 pending_dsc_flush_mask;
>>> /* ops */
>>> struct dpu_hw_ctl_ops ops;
>>> --
>>> 2.7.4
>>>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-18 22:40 ` Dmitry Baryshkov
@ 2023-05-19 12:04 ` Marijn Suijten
0 siblings, 0 replies; 24+ messages in thread
From: Marijn Suijten @ 2023-05-19 12:04 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
On 2023-05-19 01:40:19, Dmitry Baryshkov wrote:
>
> On 19/05/2023 01:09, Kuogee Hsieh wrote:
> >
> > On 5/17/2023 3:31 PM, Marijn Suijten wrote:
> >>
> >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> >>> @@ -139,6 +139,11 @@ static inline void
> >>> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> >>> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> >>> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> >>> }
> >>> +
> >>> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
> >>> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> >>> + ctx->pending_dsc_flush_mask);
> >> Again, when do we reset this mask to 0? (v8 review)
> >
> > can not find it.
> >
> > let me add a separate patch to fix this.
>
> The pending_dsc_flush_mask was added in this patch, so the reset should
> be a part of this patch too.
Yes, same patch.
Related question I asked in v8: only the global pending_flush_mask and
pending_dspp_flush_mask are reset in dpu_hw_ctl_clear_pending_flush().
Shall I send a patch to clear the other missing ones (e.g. merge_3d etc)
as well?
> >>> +
> >>> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> >>> }
> >>> @@ -285,6 +290,13 @@ static void
> >>> dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> >>> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> >>> }
> >>> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl
> >>> *ctx,
> >>> + enum dpu_dsc dsc_num)
> >>> +{
> >>> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> >>> + ctx->pending_flush_mask |= BIT(DSC_IDX);
> >>> +}
> >>> +
> >>> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl
> >>> *ctx,
> >>> enum dpu_dspp dspp, u32 dspp_sub_blk)
> >>> {
> >>> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
> >>> dpu_hw_ctl *ctx,
> >>> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> >>> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
> >>> - if (cfg->dsc)
> >>> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> >>> -
> >>> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> >>> mode_sel |= BIT(17);
> >>> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct
> >>> dpu_hw_ctl *ctx,
> >>> if (cfg->merge_3d)
> >>> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> >>> BIT(cfg->merge_3d - MERGE_3D_0));
> >>> - if (cfg->dsc) {
> >>> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> >> Again, this bugfix of now wrapping DSC_IDX in BIT() should go in a
> >> separate Fixes: patch to have this semantic change documented. (v8
> >> review)
> > That will be this patch. let me add Fixes at this patch
>
> _separate_ patch.
Separate patch, and documenting clearly what happens and why. Kuogee, I
can send this as well if it makes things more clear, since it doesn't
seem (from the patch description) that anyone noticed the
implication/bugfix in this change as a drive-by effect of porting
sde_hw_ctl_update_bitmask_dsc_v1() from downstream.
- Marijn
<snip>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-17 22:01 ` [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface Kuogee Hsieh
2023-05-17 22:31 ` Marijn Suijten
@ 2023-05-18 0:35 ` Dmitry Baryshkov
2023-05-18 7:03 ` Marijn Suijten
1 sibling, 1 reply; 24+ messages in thread
From: Dmitry Baryshkov @ 2023-05-18 0:35 UTC (permalink / raw)
To: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, andersson
Cc: quic_abhinavk, quic_jesszhan, quic_sbillaka, marijn.suijten,
freedreno, linux-arm-msm, linux-kernel
On 18/05/2023 01:01, Kuogee Hsieh wrote:
> Currently DSC flushing happens during interface configuration at
> dpu_hw_ctl_intf_cfg_v1(). Separate DSC flush away from
> dpu_hw_ctl_intf_cfg_v1() by adding dpu_hw_ctl_update_pending_flush_dsc_v1()
> to handle both per-DSC engine and DSC flush bits at same time to make it
> consistent with the location of flush programming of other DPU sub-blocks.
>
> Changes in v10:
> -- rewording commit text
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 10 ++++++++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 22 ++++++++++++++++------
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 13 +++++++++++++
> 3 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index ffa6f04..1957545 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1834,7 +1834,8 @@ dpu_encoder_dsc_initial_line_calc(struct drm_dsc_config *dsc,
> return DIV_ROUND_UP(total_pixels, dsc->slice_width);
> }
>
> -static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
> +static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> struct dpu_hw_pingpong *hw_pp,
> struct drm_dsc_config *dsc,
> u32 common_mode,
> @@ -1854,6 +1855,9 @@ static void dpu_encoder_dsc_pipe_cfg(struct dpu_hw_dsc *hw_dsc,
>
> if (hw_pp->ops.enable_dsc)
> hw_pp->ops.enable_dsc(hw_pp);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> }
>
> static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> @@ -1861,6 +1865,7 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> {
> /* coding only for 2LM, 2enc, 1 dsc config */
> struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> int this_frame_slices;
> @@ -1898,7 +1903,8 @@ static void dpu_encoder_prep_dsc(struct dpu_encoder_virt *dpu_enc,
> initial_lines = dpu_encoder_dsc_initial_line_calc(dsc, enc_ip_w);
>
> for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> - dpu_encoder_dsc_pipe_cfg(hw_dsc[i], hw_pp[i], dsc, dsc_common_mode, initial_lines);
> + dpu_encoder_dsc_pipe_cfg(ctl, hw_dsc[i], hw_pp[i], dsc,
> + dsc_common_mode, initial_lines);
Moving dsc to the next line would be more logical.
> }
>
> void dpu_encoder_prepare_for_kickoff(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4f7cfa9..4e132d9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -139,6 +139,11 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
> CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> ctx->pending_dspp_flush_mask[dspp - DSPP_0]);
> }
> +
> + if (ctx->pending_flush_mask & BIT(DSC_IDX))
> + DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH,
> + ctx->pending_dsc_flush_mask);
> +
> DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> }
>
> @@ -285,6 +290,13 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> ctx->pending_flush_mask |= BIT(MERGE_3D_IDX);
> }
>
> +static void dpu_hw_ctl_update_pending_flush_dsc_v1(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc dsc_num)
> +{
> + ctx->pending_dsc_flush_mask |= BIT(dsc_num - DSC_0);
> + ctx->pending_flush_mask |= BIT(DSC_IDX);
> +}
> +
> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
> enum dpu_dspp dspp, u32 dspp_sub_blk)
> {
> @@ -502,9 +514,6 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if ((test_bit(DPU_CTL_VM_CFG, &ctx->caps->features)))
> mode_sel = CTL_DEFAULT_GROUP_ID << 28;
>
> - if (cfg->dsc)
> - DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, cfg->dsc);
> -
> if (cfg->intf_mode_sel == DPU_CTL_MODE_SEL_CMD)
> mode_sel |= BIT(17);
>
> @@ -524,10 +533,9 @@ static void dpu_hw_ctl_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> if (cfg->merge_3d)
> DPU_REG_WRITE(c, CTL_MERGE_3D_ACTIVE,
> BIT(cfg->merge_3d - MERGE_3D_0));
> - if (cfg->dsc) {
> - DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, DSC_IDX);
> +
> + if (cfg->dsc)
> DPU_REG_WRITE(c, CTL_DSC_ACTIVE, cfg->dsc);
> - }
> }
>
> static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
> @@ -630,6 +638,8 @@ static void _setup_ctl_ops(struct dpu_hw_ctl_ops *ops,
> ops->update_pending_flush_merge_3d =
> dpu_hw_ctl_update_pending_flush_merge_3d_v1;
> ops->update_pending_flush_wb = dpu_hw_ctl_update_pending_flush_wb_v1;
> + ops->update_pending_flush_dsc =
> + dpu_hw_ctl_update_pending_flush_dsc_v1;
> } else {
> ops->trigger_flush = dpu_hw_ctl_trigger_flush;
> ops->setup_intf_cfg = dpu_hw_ctl_intf_cfg;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> index 6292002..d5f3ef8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> enum dpu_dspp blk, u32 dspp_sub_blk);
>
> /**
> + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> + * No effect on hardware
> + * @ctx: ctl path ctx pointer
> + * @blk: interface block index
> + */
> + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> + enum dpu_dsc blk);
Please align to the opening parenthesis.
> +
> + /**
> * Write the value of the pending_flush_mask to hardware
> * @ctx : ctl path ctx pointer
> */
> @@ -229,6 +238,9 @@ struct dpu_hw_ctl_ops {
> * @pending_flush_mask: storage for pending ctl_flush managed via ops
> * @pending_intf_flush_mask: pending INTF flush
> * @pending_wb_flush_mask: pending WB flush
> + * @pending_merge_3d_flush_mask: pending merge_3d flush
> + * @pending_dspp_flush_mask: pending dspp flush
These two can go to a separate patch.
> + * @pending_dsc_flush_mask: pending dsc flush
> * @ops: operation list
> */
> struct dpu_hw_ctl {
> @@ -245,6 +257,7 @@ struct dpu_hw_ctl {
> u32 pending_wb_flush_mask;
> u32 pending_merge_3d_flush_mask;
> u32 pending_dspp_flush_mask[DSPP_MAX - DSPP_0];
> + u32 pending_dsc_flush_mask;
>
> /* ops */
> struct dpu_hw_ctl_ops ops;
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface
2023-05-18 0:35 ` Dmitry Baryshkov
@ 2023-05-18 7:03 ` Marijn Suijten
0 siblings, 0 replies; 24+ messages in thread
From: Marijn Suijten @ 2023-05-18 7:03 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Kuogee Hsieh, dri-devel, robdclark, sean, swboyd, dianders,
vkoul, daniel, airlied, agross, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
On 2023-05-18 03:35:31, Dmitry Baryshkov wrote:
<snip>
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > index 6292002..d5f3ef8 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> > @@ -158,6 +158,15 @@ struct dpu_hw_ctl_ops {
> > enum dpu_dspp blk, u32 dspp_sub_blk);
> >
> > /**
> > + * OR in the given flushbits to the cached pending_(dsc_)flush_mask
> > + * No effect on hardware
> > + * @ctx: ctl path ctx pointer
> > + * @blk: interface block index
> > + */
> > + void (*update_pending_flush_dsc)(struct dpu_hw_ctl *ctx,
> > + enum dpu_dsc blk);
>
> Please align to the opening parenthesis.
I requested this change to a single tab specifically to match the rest
of the indentation of these callbacks. Perhaps we should submit a
followup patch realigning all of them at once (and fixing the doc
comments, and and and...).
- Marijn
> > +
> > + /**
> > * Write the value of the pending_flush_mask to hardware
> > * @ctx : ctl path ctx pointer
> > */
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
` (5 preceding siblings ...)
2023-05-17 22:01 ` [PATCH v10 6/8] drm/msm/dpu: separate DSC flush update out of interface Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:47 ` Marijn Suijten
2023-05-17 22:01 ` [PATCH v10 8/8] drm/msm/dpu: tear down DSC data path when DSC disabled Kuogee Hsieh
7 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Abhinav Kumar, quic_khsieh, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
From: Abhinav Kumar <quic_abhinavk@quicinc.com>
Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
feature flag information. Each display compression engine (DCE) contains
dual DSC encoders so both share same base address but with
its own different sub block address.
changes in v4:
-- delete DPU_DSC_HW_REV_1_1
-- re arrange sc8280xp_dsc[]
changes in v4:
-- fix checkpatch warning
changes in v10:
-- remove hard slice from commit text
-- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
-- change DSC_BLK_1_2 .len from 0x100 to 0x29c
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
kuogee: catalog.h
---
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
.../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
.../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
6 files changed, 89 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
index 500cfd0..d90486f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
@@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8350_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8350_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
.dspp = sm8350_dspp,
.pingpong_count = ARRAY_SIZE(sm8350_pp),
.pingpong = sm8350_pp,
+ .dsc_count = ARRAY_SIZE(sm8350_dsc),
+ .dsc = sm8350_dsc,
.merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
.merge_3d = sm8350_merge_3d,
.intf_count = ARRAY_SIZE(sm8350_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
index 5646713..52609b8 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
@@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
};
+/* NOTE: sc7280 only has one DSC hard slice encoder */
+static const struct dpu_dsc_cfg sc7280_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+};
+
static const struct dpu_intf_cfg sc7280_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
.mixer = sc7280_lm,
.pingpong_count = ARRAY_SIZE(sc7280_pp),
.pingpong = sc7280_pp,
+ .dsc_count = ARRAY_SIZE(sc7280_dsc),
+ .dsc = sc7280_dsc,
.intf_count = ARRAY_SIZE(sc7280_intf),
.intf = sc7280_intf,
.vbif_count = ARRAY_SIZE(sdm845_vbif),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
index 808aacd..a84cf36 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
@@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+ DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
+};
+
/* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
static const struct dpu_intf_cfg sc8280xp_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
@@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
.dspp = sc8280xp_dspp,
.pingpong_count = ARRAY_SIZE(sc8280xp_pp),
.pingpong = sc8280xp_pp,
+ .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
+ .dsc = sc8280xp_dsc,
.merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
.merge_3d = sc8280xp_merge_3d,
.intf_count = ARRAY_SIZE(sc8280xp_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
index 1a89ff9..1620622 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
@@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8450_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8450_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
.dspp = sm8450_dspp,
.pingpong_count = ARRAY_SIZE(sm8450_pp),
.pingpong = sm8450_pp,
+ .dsc_count = ARRAY_SIZE(sm8450_dsc),
+ .dsc = sm8450_dsc,
.merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
.merge_3d = sm8450_merge_3d,
.intf_count = ARRAY_SIZE(sm8450_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
index 497b34c..6582a14 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
@@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
};
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+static const struct dpu_dsc_cfg sm8550_dsc[] = {
+ DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
+ DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
+ DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
+ DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
+};
+
static const struct dpu_intf_cfg sm8550_intf[] = {
INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
@@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
.dspp = sm8550_dspp,
.pingpong_count = ARRAY_SIZE(sm8550_pp),
.pingpong = sm8550_pp,
+ .dsc_count = ARRAY_SIZE(sm8550_dsc),
+ .dsc = sm8550_dsc,
.merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
.merge_3d = sm8550_merge_3d,
.intf_count = ARRAY_SIZE(sm8550_intf),
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index f2a1535..9612ab5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
- * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
*/
#define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
@@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
/*************************************************************
* DSC sub blocks config
*************************************************************/
+static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
+ .enc = {.base = 0x100, .len = 0x100},
+ .ctl = {.base = 0xF00, .len = 0x10},
+};
+
+static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
+ .enc = {.base = 0x200, .len = 0x100},
+ .ctl = {.base = 0xF80, .len = 0x10},
+};
+
#define DSC_BLK(_name, _id, _base, _features) \
{\
.name = _name, .id = _id, \
@@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
.features = _features, \
}
+/*
+ * NOTE: Each display compression engine (DCE) contains dual hard
+ * slice DSC encoders so both share same base address but with
+ * its own different sub block address.
+ */
+#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
+ {\
+ .name = _name, .id = _id, \
+ .base = _base, .len = _len, \
+ .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
+ .sblk = &_sblk, \
+ }
+
/*************************************************************
* INTF sub blocks config
*************************************************************/
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
2023-05-17 22:01 ` [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Kuogee Hsieh
@ 2023-05-17 22:47 ` Marijn Suijten
2023-05-17 23:22 ` Abhinav Kumar
0 siblings, 1 reply; 24+ messages in thread
From: Marijn Suijten @ 2023-05-17 22:47 UTC (permalink / raw)
To: Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, Abhinav Kumar,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
Title: "DPU >= 7.0" instead of "relevant chipsets" to match the others.
On 2023-05-17 15:01:58, Kuogee Hsieh wrote:
> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>
> Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
> feature flag information. Each display compression engine (DCE) contains
> dual DSC encoders so both share same base address but with
> its own different sub block address.
If you reword it, also reflow this line.
>
> changes in v4:
> -- delete DPU_DSC_HW_REV_1_1
> -- re arrange sc8280xp_dsc[]
>
> changes in v4:
> -- fix checkpatch warning
>
> changes in v10:
> -- remove hard slice from commit text
It is still mentioned in the diff though, that's why I originally
requested a better place to describe it.
> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
> -- change DSC_BLK_1_2 .len from 0x100 to 0x29c
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
> kuogee: catalog.h
What's this for? This file isn't touched in this patch.
> ---
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
> 6 files changed, 89 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> index 500cfd0..d90486f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
> @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8350_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8350_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
> .dspp = sm8350_dspp,
> .pingpong_count = ARRAY_SIZE(sm8350_pp),
> .pingpong = sm8350_pp,
> + .dsc_count = ARRAY_SIZE(sm8350_dsc),
> + .dsc = sm8350_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
> .merge_3d = sm8350_merge_3d,
> .intf_count = ARRAY_SIZE(sm8350_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> index 5646713..52609b8 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
> PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
> };
>
> +/* NOTE: sc7280 only has one DSC hard slice encoder */
> +static const struct dpu_dsc_cfg sc7280_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> +};
> +
> static const struct dpu_intf_cfg sc7280_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
> .mixer = sc7280_lm,
> .pingpong_count = ARRAY_SIZE(sc7280_pp),
> .pingpong = sc7280_pp,
> + .dsc_count = ARRAY_SIZE(sc7280_dsc),
> + .dsc = sc7280_dsc,
> .intf_count = ARRAY_SIZE(sc7280_intf),
> .intf = sc7280_intf,
> .vbif_count = ARRAY_SIZE(sdm845_vbif),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> index 808aacd..a84cf36 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
> @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> + DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
> +};
> +
> /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
> static const struct dpu_intf_cfg sc8280xp_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> @@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
> .dspp = sc8280xp_dspp,
> .pingpong_count = ARRAY_SIZE(sc8280xp_pp),
> .pingpong = sc8280xp_pp,
> + .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
> + .dsc = sc8280xp_dsc,
> .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
> .merge_3d = sc8280xp_merge_3d,
> .intf_count = ARRAY_SIZE(sc8280xp_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> index 1a89ff9..1620622 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
> @@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8450_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8450_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
> .dspp = sm8450_dspp,
> .pingpong_count = ARRAY_SIZE(sm8450_pp),
> .pingpong = sm8450_pp,
> + .dsc_count = ARRAY_SIZE(sm8450_dsc),
> + .dsc = sm8450_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
> .merge_3d = sm8450_merge_3d,
> .intf_count = ARRAY_SIZE(sm8450_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> index 497b34c..6582a14 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> @@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
> };
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
> +};
> +
> static const struct dpu_intf_cfg sm8550_intf[] = {
> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
> @@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
> .dspp = sm8550_dspp,
> .pingpong_count = ARRAY_SIZE(sm8550_pp),
> .pingpong = sm8550_pp,
> + .dsc_count = ARRAY_SIZE(sm8550_dsc),
> + .dsc = sm8550_dsc,
> .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
> .merge_3d = sm8550_merge_3d,
> .intf_count = ARRAY_SIZE(sm8550_intf),
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> index f2a1535..9612ab5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -1,6 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
> */
>
> #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
> @@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> /*************************************************************
> * DSC sub blocks config
> *************************************************************/
> +static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
> + .enc = {.base = 0x100, .len = 0x100},
> + .ctl = {.base = 0xF00, .len = 0x10},
> +};
> +
> +static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
> + .enc = {.base = 0x200, .len = 0x100},
> + .ctl = {.base = 0xF80, .len = 0x10},
> +};
> +
> #define DSC_BLK(_name, _id, _base, _features) \
> {\
> .name = _name, .id = _id, \
> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> .features = _features, \
> }
>
> +/*
> + * NOTE: Each display compression engine (DCE) contains dual hard
> + * slice DSC encoders so both share same base address but with
> + * its own different sub block address.
> + */
I still think this comment is superfluous (and doesn't even apply
generically, see i.e. sc7280) and should best be kept exclusively in the
SoC-specific catalog files.
- Marijn
> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
> + {\
> + .name = _name, .id = _id, \
> + .base = _base, .len = _len, \
> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
> + .sblk = &_sblk, \
> + }
> +
> /*************************************************************
> * INTF sub blocks config
> *************************************************************/
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
2023-05-17 22:47 ` Marijn Suijten
@ 2023-05-17 23:22 ` Abhinav Kumar
2023-05-18 7:05 ` Marijn Suijten
0 siblings, 1 reply; 24+ messages in thread
From: Abhinav Kumar @ 2023-05-17 23:22 UTC (permalink / raw)
To: Marijn Suijten, Kuogee Hsieh
Cc: freedreno, quic_sbillaka, andersson, dri-devel, dianders, vkoul,
agross, linux-arm-msm, dmitry.baryshkov, quic_jesszhan, swboyd,
sean, linux-kernel
On 5/17/2023 3:47 PM, Marijn Suijten wrote:
> Title: "DPU >= 7.0" instead of "relevant chipsets" to match the others.
>
> On 2023-05-17 15:01:58, Kuogee Hsieh wrote:
>> From: Abhinav Kumar <quic_abhinavk@quicinc.com>
>>
>> Add DSC 1.2 hardware blocks to the catalog with necessary sub-block and
>> feature flag information. Each display compression engine (DCE) contains
>> dual DSC encoders so both share same base address but with
>> its own different sub block address.
>
> If you reword it, also reflow this line.
>
>>
>> changes in v4:
>> -- delete DPU_DSC_HW_REV_1_1
>> -- re arrange sc8280xp_dsc[]
>>
>> changes in v4:
>> -- fix checkpatch warning
>>
>> changes in v10:
>> -- remove hard slice from commit text
>
> It is still mentioned in the diff though, that's why I originally
> requested a better place to describe it.
>
>> -- replace DPU_DSC_NATIVE_422_EN with DPU_DSC_NATIVE_42x_EN
>> -- change DSC_BLK_1_2 .len from 0x100 to 0x29c
>>
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
>
>>
>> kuogee: catalog.h
>
> What's this for? This file isn't touched in this patch.
>
>> ---
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h | 14 ++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h | 7 ++++++
>> .../drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h | 16 ++++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h | 14 ++++++++++++
>> .../gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h | 14 ++++++++++++
>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 25 +++++++++++++++++++++-
>> 6 files changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> index 500cfd0..d90486f 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_0_sm8350.h
>> @@ -153,6 +153,18 @@ static const struct dpu_merge_3d_cfg sm8350_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8350_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8350_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -215,6 +227,8 @@ const struct dpu_mdss_cfg dpu_sm8350_cfg = {
>> .dspp = sm8350_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8350_pp),
>> .pingpong = sm8350_pp,
>> + .dsc_count = ARRAY_SIZE(sm8350_dsc),
>> + .dsc = sm8350_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8350_merge_3d),
>> .merge_3d = sm8350_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8350_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> index 5646713..52609b8 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
>> @@ -93,6 +93,11 @@ static const struct dpu_pingpong_cfg sc7280_pp[] = {
>> PP_BLK_DITHER("pingpong_3", PINGPONG_3, 0x6c000, 0, sc7280_pp_sblk, -1, -1),
>> };
>>
>> +/* NOTE: sc7280 only has one DSC hard slice encoder */
>> +static const struct dpu_dsc_cfg sc7280_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> +};
>> +
>> static const struct dpu_intf_cfg sc7280_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -149,6 +154,8 @@ const struct dpu_mdss_cfg dpu_sc7280_cfg = {
>> .mixer = sc7280_lm,
>> .pingpong_count = ARRAY_SIZE(sc7280_pp),
>> .pingpong = sc7280_pp,
>> + .dsc_count = ARRAY_SIZE(sc7280_dsc),
>> + .dsc = sc7280_dsc,
>> .intf_count = ARRAY_SIZE(sc7280_intf),
>> .intf = sc7280_intf,
>> .vbif_count = ARRAY_SIZE(sdm845_vbif),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> index 808aacd..a84cf36 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_0_sc8280xp.h
>> @@ -141,6 +141,20 @@ static const struct dpu_merge_3d_cfg sc8280xp_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_2", MERGE_3D_2, 0x50000),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sc8280xp_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> + DSC_BLK_1_2("dce_2_0", DSC_4, 0x82000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_2_1", DSC_5, 0x82000, 0x29c, 0, dsc_sblk_1),
>> +};
>> +
>> /* TODO: INTF 3, 8 and 7 are used for MST, marked as INTF_NONE for now */
>> static const struct dpu_intf_cfg sc8280xp_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> @@ -216,6 +230,8 @@ const struct dpu_mdss_cfg dpu_sc8280xp_cfg = {
>> .dspp = sc8280xp_dspp,
>> .pingpong_count = ARRAY_SIZE(sc8280xp_pp),
>> .pingpong = sc8280xp_pp,
>> + .dsc_count = ARRAY_SIZE(sc8280xp_dsc),
>> + .dsc = sc8280xp_dsc,
>> .merge_3d_count = ARRAY_SIZE(sc8280xp_merge_3d),
>> .merge_3d = sc8280xp_merge_3d,
>> .intf_count = ARRAY_SIZE(sc8280xp_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> index 1a89ff9..1620622 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_8_1_sm8450.h
>> @@ -161,6 +161,18 @@ static const struct dpu_merge_3d_cfg sm8450_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x65f00),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8450_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8450_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -223,6 +235,8 @@ const struct dpu_mdss_cfg dpu_sm8450_cfg = {
>> .dspp = sm8450_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8450_pp),
>> .pingpong = sm8450_pp,
>> + .dsc_count = ARRAY_SIZE(sm8450_dsc),
>> + .dsc = sm8450_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8450_merge_3d),
>> .merge_3d = sm8450_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8450_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> index 497b34c..6582a14 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
>> @@ -165,6 +165,18 @@ static const struct dpu_merge_3d_cfg sm8550_merge_3d[] = {
>> MERGE_3D_BLK("merge_3d_3", MERGE_3D_3, 0x66700),
>> };
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>> +static const struct dpu_dsc_cfg sm8550_dsc[] = {
>> + DSC_BLK_1_2("dce_0_0", DSC_0, 0x80000, 0x29c, 0, dsc_sblk_0),
>> + DSC_BLK_1_2("dce_0_1", DSC_1, 0x80000, 0x29c, 0, dsc_sblk_1),
>> + DSC_BLK_1_2("dce_1_0", DSC_2, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_0),
>> + DSC_BLK_1_2("dce_1_1", DSC_3, 0x81000, 0x29c, BIT(DPU_DSC_NATIVE_42x_EN), dsc_sblk_1),
>> +};
>> +
>> static const struct dpu_intf_cfg sm8550_intf[] = {
>> INTF_BLK("intf_0", INTF_0, 0x34000, 0x280, INTF_DP, MSM_DP_CONTROLLER_0, 24, INTF_SC7280_MASK,
>> DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR, 24),
>> @@ -227,6 +239,8 @@ const struct dpu_mdss_cfg dpu_sm8550_cfg = {
>> .dspp = sm8550_dspp,
>> .pingpong_count = ARRAY_SIZE(sm8550_pp),
>> .pingpong = sm8550_pp,
>> + .dsc_count = ARRAY_SIZE(sm8550_dsc),
>> + .dsc = sm8550_dsc,
>> .merge_3d_count = ARRAY_SIZE(sm8550_merge_3d),
>> .merge_3d = sm8550_merge_3d,
>> .intf_count = ARRAY_SIZE(sm8550_intf),
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> index f2a1535..9612ab5 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
>> @@ -1,6 +1,6 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved.
>> - * Copyright (c) 2022. Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2022-2023, Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> #define pr_fmt(fmt) "[drm:%s:%d] " fmt, __func__, __LINE__
>> @@ -522,6 +522,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>> /*************************************************************
>> * DSC sub blocks config
>> *************************************************************/
>> +static const struct dpu_dsc_sub_blks dsc_sblk_0 = {
>> + .enc = {.base = 0x100, .len = 0x100},
>> + .ctl = {.base = 0xF00, .len = 0x10},
>> +};
>> +
>> +static const struct dpu_dsc_sub_blks dsc_sblk_1 = {
>> + .enc = {.base = 0x200, .len = 0x100},
>> + .ctl = {.base = 0xF80, .len = 0x10},
>> +};
>> +
>> #define DSC_BLK(_name, _id, _base, _features) \
>> {\
>> .name = _name, .id = _id, \
>> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>> .features = _features, \
>> }
>>
>> +/*
>> + * NOTE: Each display compression engine (DCE) contains dual hard
>> + * slice DSC encoders so both share same base address but with
>> + * its own different sub block address.
>> + */
>
> I still think this comment is superfluous (and doesn't even apply
> generically, see i.e. sc7280) and should best be kept exclusively in the
> SoC-specific catalog files.
>
> - Marijn
>
sc7280 is the only exception as it has only one encoder. But, by and
large, for all other chipsets this is true and hence kept here.
The main reason for this comment is people should not get confused that
how come two DSC encoders have the same base address.
>> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
>> + {\
>> + .name = _name, .id = _id, \
>> + .base = _base, .len = _len, \
>> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
>> + .sblk = &_sblk, \
>> + }
>> +
>> /*************************************************************
>> * INTF sub blocks config
>> *************************************************************/
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
2023-05-17 23:22 ` Abhinav Kumar
@ 2023-05-18 7:05 ` Marijn Suijten
2023-05-18 19:00 ` Abhinav Kumar
0 siblings, 1 reply; 24+ messages in thread
From: Marijn Suijten @ 2023-05-18 7:05 UTC (permalink / raw)
To: Abhinav Kumar
Cc: Kuogee Hsieh, freedreno, quic_sbillaka, andersson, dri-devel,
dianders, vkoul, agross, linux-arm-msm, dmitry.baryshkov,
quic_jesszhan, swboyd, sean, linux-kernel
On 2023-05-17 16:22:37, Abhinav Kumar wrote:
<snip>
> >> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> >> .features = _features, \
> >> }
> >>
> >> +/*
> >> + * NOTE: Each display compression engine (DCE) contains dual hard
> >> + * slice DSC encoders so both share same base address but with
> >> + * its own different sub block address.
> >> + */
> >
> > I still think this comment is superfluous (and doesn't even apply
> > generically, see i.e. sc7280) and should best be kept exclusively in the
> > SoC-specific catalog files.
> >
> > - Marijn
> >
>
> sc7280 is the only exception as it has only one encoder. But, by and
> large, for all other chipsets this is true and hence kept here.
>
> The main reason for this comment is people should not get confused that
> how come two DSC encoders have the same base address.
And that's why the comment is already placed in the SoC-specific catalog
files where a duplicate base address is visible. It is not visible
here.
- Marijn
> >> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
> >> + {\
> >> + .name = _name, .id = _id, \
> >> + .base = _base, .len = _len, \
> >> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
> >> + .sblk = &_sblk, \
> >> + }
> >> +
> >> /*************************************************************
> >> * INTF sub blocks config
> >> *************************************************************/
> >> --
> >> 2.7.4
> >>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets
2023-05-18 7:05 ` Marijn Suijten
@ 2023-05-18 19:00 ` Abhinav Kumar
0 siblings, 0 replies; 24+ messages in thread
From: Abhinav Kumar @ 2023-05-18 19:00 UTC (permalink / raw)
To: Marijn Suijten
Cc: Kuogee Hsieh, freedreno, quic_sbillaka, andersson, dri-devel,
dianders, vkoul, agross, linux-arm-msm, dmitry.baryshkov,
quic_jesszhan, swboyd, sean, linux-kernel
On 5/18/2023 12:05 AM, Marijn Suijten wrote:
> On 2023-05-17 16:22:37, Abhinav Kumar wrote:
> <snip>
>>>> @@ -529,6 +539,19 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
>>>> .features = _features, \
>>>> }
>>>>
>>>> +/*
>>>> + * NOTE: Each display compression engine (DCE) contains dual hard
>>>> + * slice DSC encoders so both share same base address but with
>>>> + * its own different sub block address.
>>>> + */
>>>
>>> I still think this comment is superfluous (and doesn't even apply
>>> generically, see i.e. sc7280) and should best be kept exclusively in the
>>> SoC-specific catalog files.
>>>
>>> - Marijn
>>>
>>
>> sc7280 is the only exception as it has only one encoder. But, by and
>> large, for all other chipsets this is true and hence kept here.
>>
>> The main reason for this comment is people should not get confused that
>> how come two DSC encoders have the same base address.
>
> And that's why the comment is already placed in the SoC-specific catalog
> files where a duplicate base address is visible. It is not visible
> here.
>
> - Marijn
>
ok, if kuogee pushes a v11, we can drop it.
>>>> +#define DSC_BLK_1_2(_name, _id, _base, _len, _features, _sblk) \
>>>> + {\
>>>> + .name = _name, .id = _id, \
>>>> + .base = _base, .len = _len, \
>>>> + .features = BIT(DPU_DSC_HW_REV_1_2) | _features, \
>>>> + .sblk = &_sblk, \
>>>> + }
>>>> +
>>>> /*************************************************************
>>>> * INTF sub blocks config
>>>> *************************************************************/
>>>> --
>>>> 2.7.4
>>>>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v10 8/8] drm/msm/dpu: tear down DSC data path when DSC disabled
2023-05-17 22:01 [PATCH v10 0/8] add DSC 1.2 dpu supports Kuogee Hsieh
` (6 preceding siblings ...)
2023-05-17 22:01 ` [PATCH v10 7/8] drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Kuogee Hsieh
@ 2023-05-17 22:01 ` Kuogee Hsieh
2023-05-17 22:53 ` Marijn Suijten
7 siblings, 1 reply; 24+ messages in thread
From: Kuogee Hsieh @ 2023-05-17 22:01 UTC (permalink / raw)
To: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson
Cc: Kuogee Hsieh, quic_abhinavk, quic_jesszhan, quic_sbillaka,
marijn.suijten, freedreno, linux-arm-msm, linux-kernel
Unset DSC_ACTIVE bit at dpu_hw_ctl_reset_intf_cfg_v1(),
dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() functions
to tear down DSC data path if DSC data path was setup previous.
Changes in V10:
-- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
-- move both dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() to above phys_cleanup()
Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++++++++++++++++++++++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++
2 files changed, 46 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 1957545..ec2a55a6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2036,6 +2036,41 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
}
}
+static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,
+ struct dpu_hw_dsc *hw_dsc,
+ struct dpu_hw_pingpong *hw_pp)
+{
+ if (hw_dsc->ops.dsc_disable)
+ hw_dsc->ops.dsc_disable(hw_dsc);
+
+ if (hw_pp->ops.disable_dsc)
+ hw_pp->ops.disable_dsc(hw_pp);
+
+ if (hw_dsc->ops.dsc_bind_pingpong_blk)
+ hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, PINGPONG_NONE);
+
+ if (ctl->ops.update_pending_flush_dsc)
+ ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
+}
+
+static void dpu_encoder_unprep_dsc(struct dpu_encoder_virt *dpu_enc)
+{
+ /* coding only for 2LM, 2enc, 1 dsc config */
+ struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
+ struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
+ struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
+ struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
+ int i;
+
+ for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+ hw_pp[i] = dpu_enc->hw_pp[i];
+ hw_dsc[i] = dpu_enc->hw_dsc[i];
+
+ if (hw_pp[i] && hw_dsc[i])
+ dpu_encoder_dsc_pipe_clr(ctl, hw_dsc[i], hw_pp[i]);
+ }
+}
+
void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
{
struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
@@ -2086,8 +2121,12 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
phys_enc->hw_pp->merge_3d->idx);
}
+ if (dpu_enc->dsc)
+ dpu_encoder_unprep_dsc(dpu_enc);
+
intf_cfg.stream_sel = 0; /* Don't care value for video mode */
intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+ intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);
if (phys_enc->hw_intf)
intf_cfg.intf = phys_enc->hw_intf->idx;
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
index 4e132d9..2663626 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -578,6 +578,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
u32 intf_active = 0;
u32 wb_active = 0;
u32 merge3d_active = 0;
+ u32 dsc_active;
/*
* This API resets each portion of the CTL path namely,
@@ -607,6 +608,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
wb_active &= ~BIT(cfg->wb - WB_0);
DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
}
+
+ if (cfg->dsc) {
+ dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
+ dsc_active &= ~cfg->dsc;
+ DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
+ }
}
static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
--
2.7.4
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH v10 8/8] drm/msm/dpu: tear down DSC data path when DSC disabled
2023-05-17 22:01 ` [PATCH v10 8/8] drm/msm/dpu: tear down DSC data path when DSC disabled Kuogee Hsieh
@ 2023-05-17 22:53 ` Marijn Suijten
0 siblings, 0 replies; 24+ messages in thread
From: Marijn Suijten @ 2023-05-17 22:53 UTC (permalink / raw)
To: Kuogee Hsieh
Cc: dri-devel, robdclark, sean, swboyd, dianders, vkoul, daniel,
airlied, agross, dmitry.baryshkov, andersson, quic_abhinavk,
quic_jesszhan, quic_sbillaka, freedreno, linux-arm-msm,
linux-kernel
Title: Tear down DSC datapath* on encoder cleanup*
On 2023-05-17 15:01:59, Kuogee Hsieh wrote:
> Unset DSC_ACTIVE bit at dpu_hw_ctl_reset_intf_cfg_v1(),
> dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() functions
> to tear down DSC data path if DSC data path was setup previous.
>
> Changes in V10:
> -- pass ctl directly instead of dpu_enc to dsc_pipe_cfg()
> -- move both dpu_encoder_unprep_dsc() and dpu_encoder_dsc_pipe_clr() to above phys_cleanup()
>
> Signed-off-by: Kuogee Hsieh <quic_khsieh@quicinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org>
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 39 +++++++++++++++++++++++++++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 1957545..ec2a55a6 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2036,6 +2036,41 @@ static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
> }
> }
>
> +static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,
> + struct dpu_hw_dsc *hw_dsc,
> + struct dpu_hw_pingpong *hw_pp)
> +{
> + if (hw_dsc->ops.dsc_disable)
> + hw_dsc->ops.dsc_disable(hw_dsc);
> +
> + if (hw_pp->ops.disable_dsc)
> + hw_pp->ops.disable_dsc(hw_pp);
> +
> + if (hw_dsc->ops.dsc_bind_pingpong_blk)
> + hw_dsc->ops.dsc_bind_pingpong_blk(hw_dsc, PINGPONG_NONE);
> +
> + if (ctl->ops.update_pending_flush_dsc)
> + ctl->ops.update_pending_flush_dsc(ctl, hw_dsc->idx);
> +}
> +
> +static void dpu_encoder_unprep_dsc(struct dpu_encoder_virt *dpu_enc)
> +{
> + /* coding only for 2LM, 2enc, 1 dsc config */
> + struct dpu_encoder_phys *enc_master = dpu_enc->cur_master;
> + struct dpu_hw_ctl *ctl = enc_master->hw_ctl;
> + struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
> + struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
> + int i;
> +
> + for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
> + hw_pp[i] = dpu_enc->hw_pp[i];
> + hw_dsc[i] = dpu_enc->hw_dsc[i];
> +
> + if (hw_pp[i] && hw_dsc[i])
> + dpu_encoder_dsc_pipe_clr(ctl, hw_dsc[i], hw_pp[i]);
> + }
> +}
> +
> void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
> {
> struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
> @@ -2086,8 +2121,12 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
> phys_enc->hw_pp->merge_3d->idx);
> }
>
> + if (dpu_enc->dsc)
> + dpu_encoder_unprep_dsc(dpu_enc);
> +
> intf_cfg.stream_sel = 0; /* Don't care value for video mode */
> intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
> + intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);
>
> if (phys_enc->hw_intf)
> intf_cfg.intf = phys_enc->hw_intf->idx;
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 4e132d9..2663626 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -578,6 +578,7 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> u32 intf_active = 0;
> u32 wb_active = 0;
> u32 merge3d_active = 0;
> + u32 dsc_active;
>
> /*
> * This API resets each portion of the CTL path namely,
> @@ -607,6 +608,12 @@ static void dpu_hw_ctl_reset_intf_cfg_v1(struct dpu_hw_ctl *ctx,
> wb_active &= ~BIT(cfg->wb - WB_0);
> DPU_REG_WRITE(c, CTL_WB_ACTIVE, wb_active);
> }
> +
> + if (cfg->dsc) {
> + dsc_active = DPU_REG_READ(c, CTL_DSC_ACTIVE);
> + dsc_active &= ~cfg->dsc;
> + DPU_REG_WRITE(c, CTL_DSC_ACTIVE, dsc_active);
> + }
> }
>
> static void dpu_hw_ctl_set_fetch_pipe_active(struct dpu_hw_ctl *ctx,
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 24+ messages in thread