2022-09-02 10:07:28

by Kalyan Thota

[permalink] [raw]
Subject: [v2] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280

Flush mechanism for DSPP blocks has changed in sc7280 family, it
allows individual sub blocks to be flushed in coordination with
master flush control.

Representation: master_flush && (PCC_flush | IGC_flush .. etc )

This change adds necessary support for the above design.

Changes in v1:
- Few nits (Doug, Dmitry)
- Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry)
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 +++++++++++++++++++++++++-
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++++++--
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 7 ++++++
6 files changed, 56 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 601d687..ab38a52 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -766,7 +766,7 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc *crtc)

/* stage config flush mask */
ctl->ops.update_pending_flush_dspp(ctl,
- mixer[i].hw_dspp->idx);
+ mixer[i].hw_dspp->idx, DPU_DSPP_SUB_PCC);
}
}

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 27f029f..0eecb2f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -65,7 +65,10 @@
(PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))

#define CTL_SC7280_MASK \
- (BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG))
+ (BIT(DPU_CTL_ACTIVE_CFG) | \
+ BIT(DPU_CTL_FETCH_ACTIVE) | \
+ BIT(DPU_CTL_VM_CFG) | \
+ BIT(DPU_CTL_DSPP_SUB_BLOCK_FLUSH))

#define MERGE_3D_SM8150_MASK (0)

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 38aa38a..6a0b784 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -191,6 +191,7 @@ enum {
* @DPU_CTL_SPLIT_DISPLAY: CTL supports video mode split display
* @DPU_CTL_FETCH_ACTIVE: Active CTL for fetch HW (SSPPs)
* @DPU_CTL_VM_CFG: CTL config to support multiple VMs
+ * @DPU_CTL_DSPP_BLOCK_FLUSH: CTL config to support dspp sub-block flush
* @DPU_CTL_MAX
*/
enum {
@@ -198,6 +199,7 @@ enum {
DPU_CTL_ACTIVE_CFG,
DPU_CTL_FETCH_ACTIVE,
DPU_CTL_VM_CFG,
+ DPU_CTL_DSPP_SUB_BLOCK_FLUSH,
DPU_CTL_MAX
};

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 a35ecb6..3b14c30 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -33,6 +33,7 @@
#define CTL_INTF_FLUSH 0x110
#define CTL_INTF_MASTER 0x134
#define CTL_FETCH_PIPE_ACTIVE 0x0FC
+#define CTL_DSPP_n_FLUSH 0x13C

#define CTL_MIXER_BORDER_OUT BIT(24)
#define CTL_FLUSH_MASK_CTL BIT(17)
@@ -82,6 +83,31 @@ static int _mixer_stages(const struct dpu_lm_cfg *mixer, int count,
return stages;
}

+static u32 _set_dspp_sub_block_flush(struct dpu_hw_ctl *ctx,
+ enum dpu_dspp dspp, enum dpu_dspp_sub_blk dspp_sub_blk)
+{
+ uint32_t flushbits = 0, active;
+
+ switch (dspp_sub_blk) {
+ case DPU_DSPP_SUB_IGC:
+ flushbits = BIT(2);
+ break;
+ case DPU_DSPP_SUB_PCC:
+ flushbits = BIT(4);
+ break;
+ case DPU_DSPP_SUB_GC:
+ flushbits = BIT(5);
+ break;
+ default:
+ return 0;
+ }
+
+ active = DPU_REG_READ(&ctx->hw, CTL_DSPP_n_FLUSH + ((dspp - 1) * 4));
+ DPU_REG_WRITE(&ctx->hw, CTL_DSPP_n_FLUSH + ((dspp - 1) * 4), active | flushbits);
+
+ return BIT(29);
+}
+
static inline u32 dpu_hw_ctl_get_flush_register(struct dpu_hw_ctl *ctx)
{
struct dpu_hw_blk_reg_map *c = &ctx->hw;
@@ -287,8 +313,15 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
}

static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
- enum dpu_dspp dspp)
+ enum dpu_dspp dspp, enum dpu_dspp_sub_blk dspp_sub_blk)
{
+
+ if ((test_bit(DPU_CTL_DSPP_SUB_BLOCK_FLUSH, &ctx->caps->features))) {
+ ctx->pending_flush_mask |=
+ _set_dspp_sub_block_flush(ctx, dspp, dspp_sub_blk);
+ return;
+ }
+
switch (dspp) {
case DSPP_0:
ctx->pending_flush_mask |= BIT(13);
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 96c012e..227f1bd 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
@@ -149,12 +149,18 @@ struct dpu_hw_ctl_ops {

/**
* OR in the given flushbits to the cached pending_flush_mask
- * No effect on hardware
+ *
+ * If the hardware supports dspp sub block flush, then sub-block
+ * flushes are written to the hardware and main dspp flush will
+ * be cached in the pending_flush_mask.
+ *
* @ctx : ctl path ctx pointer
* @blk : DSPP block index
+ * @dspp_sub_blk : DSPP sub-block index
*/
void (*update_pending_flush_dspp)(struct dpu_hw_ctl *ctx,
- enum dpu_dspp blk);
+ enum dpu_dspp blk, enum dpu_dspp_sub_blk dspp_sub_blk);
+
/**
* Write the value of the pending_flush_mask to hardware
* @ctx : ctl path ctx pointer
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 d3b0ed0..c113d52 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
@@ -167,6 +167,13 @@ enum dpu_dspp {
DSPP_MAX
};

+enum dpu_dspp_sub_blk{
+ DPU_DSPP_SUB_PCC = 1,
+ DPU_DSPP_SUB_IGC,
+ DPU_DSPP_SUB_GC,
+ DPU_DSPP_SUB_MAX
+};
+
enum dpu_ctl {
CTL_0 = 1,
CTL_1,
--
2.7.4


2022-09-02 11:23:47

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [v2] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280

On Fri, 2 Sept 2022 at 12:38, Kalyan Thota <[email protected]> wrote:
>
> Flush mechanism for DSPP blocks has changed in sc7280 family, it
> allows individual sub blocks to be flushed in coordination with
> master flush control.
>
> Representation: master_flush && (PCC_flush | IGC_flush .. etc )
>
> This change adds necessary support for the above design.
>
> Changes in v1:
> - Few nits (Doug, Dmitry)
> - Restrict sub-block flush programming to dpu_hw_ctl file (Dmitry)
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 35 +++++++++++++++++++++++++-
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h | 10 ++++++--
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 7 ++++++
> 6 files changed, 56 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 601d687..ab38a52 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -766,7 +766,7 @@ static void _dpu_crtc_setup_cp_blocks(struct drm_crtc *crtc)
>
> /* stage config flush mask */
> ctl->ops.update_pending_flush_dspp(ctl,
> - mixer[i].hw_dspp->idx);
> + mixer[i].hw_dspp->idx, DPU_DSPP_SUB_PCC);
> }
> }
>
> 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 27f029f..0eecb2f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> @@ -65,7 +65,10 @@
> (PINGPONG_SDM845_MASK | BIT(DPU_PINGPONG_TE2))
>
> #define CTL_SC7280_MASK \
> - (BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE) | BIT(DPU_CTL_VM_CFG))
> + (BIT(DPU_CTL_ACTIVE_CFG) | \
> + BIT(DPU_CTL_FETCH_ACTIVE) | \
> + BIT(DPU_CTL_VM_CFG) | \
> + BIT(DPU_CTL_DSPP_SUB_BLOCK_FLUSH))
>
> #define MERGE_3D_SM8150_MASK (0)
>
> 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 38aa38a..6a0b784 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> @@ -191,6 +191,7 @@ enum {
> * @DPU_CTL_SPLIT_DISPLAY: CTL supports video mode split display
> * @DPU_CTL_FETCH_ACTIVE: Active CTL for fetch HW (SSPPs)
> * @DPU_CTL_VM_CFG: CTL config to support multiple VMs
> + * @DPU_CTL_DSPP_BLOCK_FLUSH: CTL config to support dspp sub-block flush
> * @DPU_CTL_MAX
> */
> enum {
> @@ -198,6 +199,7 @@ enum {
> DPU_CTL_ACTIVE_CFG,
> DPU_CTL_FETCH_ACTIVE,
> DPU_CTL_VM_CFG,
> + DPU_CTL_DSPP_SUB_BLOCK_FLUSH,
> DPU_CTL_MAX
> };
>
> 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 a35ecb6..3b14c30 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -33,6 +33,7 @@
> #define CTL_INTF_FLUSH 0x110
> #define CTL_INTF_MASTER 0x134
> #define CTL_FETCH_PIPE_ACTIVE 0x0FC
> +#define CTL_DSPP_n_FLUSH 0x13C

CTL_DSPP_n_FLUSH(n)

>
> #define CTL_MIXER_BORDER_OUT BIT(24)
> #define CTL_FLUSH_MASK_CTL BIT(17)
> @@ -82,6 +83,31 @@ static int _mixer_stages(const struct dpu_lm_cfg *mixer, int count,
> return stages;
> }
>
> +static u32 _set_dspp_sub_block_flush(struct dpu_hw_ctl *ctx,
> + enum dpu_dspp dspp, enum dpu_dspp_sub_blk dspp_sub_blk)
> +{
> + uint32_t flushbits = 0, active;
> +
> + switch (dspp_sub_blk) {
> + case DPU_DSPP_SUB_IGC:
> + flushbits = BIT(2);
> + break;
> + case DPU_DSPP_SUB_PCC:
> + flushbits = BIT(4);
> + break;
> + case DPU_DSPP_SUB_GC:
> + flushbits = BIT(5);
> + break;
> + default:
> + return 0;
> + }
> +
> + active = DPU_REG_READ(&ctx->hw, CTL_DSPP_n_FLUSH + ((dspp - 1) * 4));
> + DPU_REG_WRITE(&ctx->hw, CTL_DSPP_n_FLUSH + ((dspp - 1) * 4), active | flushbits);
> +
> + return BIT(29);
> +}
> +
> static inline u32 dpu_hw_ctl_get_flush_register(struct dpu_hw_ctl *ctx)
> {
> struct dpu_hw_blk_reg_map *c = &ctx->hw;
> @@ -287,8 +313,15 @@ static void dpu_hw_ctl_update_pending_flush_merge_3d_v1(struct dpu_hw_ctl *ctx,
> }
>
> static void dpu_hw_ctl_update_pending_flush_dspp(struct dpu_hw_ctl *ctx,
> - enum dpu_dspp dspp)
> + enum dpu_dspp dspp, enum dpu_dspp_sub_blk dspp_sub_blk)
> {
> +
> + if ((test_bit(DPU_CTL_DSPP_SUB_BLOCK_FLUSH, &ctx->caps->features))) {
> + ctx->pending_flush_mask |=
> + _set_dspp_sub_block_flush(ctx, dspp, dspp_sub_blk);
> + return;
> + }

Please inline _set_dspp_sub_block_flush(), split this if to a
separate function (e.g.
dpu_hw_ctl_update_pending_flush_dspp_subblocks()), and use it instead
of dpu_hw_ctl_update_pending_flush_dspp like it was done before.

> +
> switch (dspp) {
> case DSPP_0:
> ctx->pending_flush_mask |= BIT(13);
> 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 96c012e..227f1bd 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.h
> @@ -149,12 +149,18 @@ struct dpu_hw_ctl_ops {
>
> /**
> * OR in the given flushbits to the cached pending_flush_mask
> - * No effect on hardware
> + *
> + * If the hardware supports dspp sub block flush, then sub-block
> + * flushes are written to the hardware and main dspp flush will
> + * be cached in the pending_flush_mask.
> + *
> * @ctx : ctl path ctx pointer
> * @blk : DSPP block index
> + * @dspp_sub_blk : DSPP sub-block index
> */
> void (*update_pending_flush_dspp)(struct dpu_hw_ctl *ctx,
> - enum dpu_dspp blk);
> + enum dpu_dspp blk, enum dpu_dspp_sub_blk dspp_sub_blk);
> +
> /**
> * Write the value of the pending_flush_mask to hardware
> * @ctx : ctl path ctx pointer
> 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 d3b0ed0..c113d52 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h
> @@ -167,6 +167,13 @@ enum dpu_dspp {
> DSPP_MAX
> };
>
> +enum dpu_dspp_sub_blk{
> + DPU_DSPP_SUB_PCC = 1,
> + DPU_DSPP_SUB_IGC,
> + DPU_DSPP_SUB_GC,
> + DPU_DSPP_SUB_MAX
> +};

Can we please use the existing enum DPU_DSPP_foo instead of adding a
new enum for no particular reason?

> +
> enum dpu_ctl {
> CTL_0 = 1,
> CTL_1,
> --
> 2.7.4
>


--
With best wishes
Dmitry