diff mbox series

[06/11] drm/msm/disp/dpu1: Add DSC support in hw_ctl

Message ID 20210715065203.709914-7-vkoul@kernel.org
State New
Headers show
Series [01/11] drm/msm/dsi: add support for dsc data | expand

Commit Message

Vinod Koul July 15, 2021, 6:51 a.m. UTC 
Later gens of hardware have DSC bits moved to hw_ctl, so configure these
bits so that DSC would work there as well

Signed-off-by: Vinod Koul <vkoul@kernel.org>

---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

-- 
2.31.1

Comments

Dmitry Baryshkov July 29, 2021, 10:15 p.m. UTC | #1 
On 15/07/2021 09:51, Vinod Koul wrote:
> Later gens of hardware have DSC bits moved to hw_ctl, so configure these
> bits so that DSC would work there as well
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 2d4645e01ebf..aeea6add61ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -25,6 +25,8 @@
>   #define   CTL_MERGE_3D_ACTIVE           0x0E4
>   #define   CTL_INTF_ACTIVE               0x0F4
>   #define   CTL_MERGE_3D_FLUSH            0x100
> +#define   CTL_DSC_ACTIVE                0x0E8
> +#define   CTL_DSC_FLUSH                0x104
>   #define   CTL_INTF_FLUSH                0x110
>   #define   CTL_INTF_MASTER               0x134
>   #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
> @@ -34,6 +36,7 @@
>   
>   #define DPU_REG_RESET_TIMEOUT_US        2000
>   #define  MERGE_3D_IDX   23
> +#define  DSC_IDX        22
>   #define  INTF_IDX       31
>   #define CTL_INVALID_BIT                 0xffff
>   
> @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
>   
>   static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>   {
> +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));

Please pass DSC indices using intf cfg and use them to configure 
register writes.

>   
>   	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
>   		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
> @@ -128,7 +132,7 @@ static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>   		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
>   				ctx->pending_intf_flush_mask);
>   
> -	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  BIT(DSC_IDX));

Only if DSCs are used

>   }
>   
>   static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
> @@ -507,6 +511,7 @@ 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));
> +	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));

And here

>   }
>   
>   static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
>
Abhinav Kumar Aug. 3, 2021, midnight UTC | #2 
On 2021-07-14 23:51, Vinod Koul wrote:
> Later gens of hardware have DSC bits moved to hw_ctl, so configure 
> these
> bits so that DSC would work there as well
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
Please correct me if wrong but here you seem to be flushing all the DSC 
bits
even the unused ones. This will end-up enabling DSC even when DSC is 
unused on
the newer targets.
If so, thats wrong.
We need to implement bit-mask based approach to avoid this change and 
only enable
those DSCs which are used.

> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> index 2d4645e01ebf..aeea6add61ee 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -25,6 +25,8 @@
>  #define   CTL_MERGE_3D_ACTIVE           0x0E4
>  #define   CTL_INTF_ACTIVE               0x0F4
>  #define   CTL_MERGE_3D_FLUSH            0x100
> +#define   CTL_DSC_ACTIVE                0x0E8
> +#define   CTL_DSC_FLUSH                0x104
>  #define   CTL_INTF_FLUSH                0x110
>  #define   CTL_INTF_MASTER               0x134
>  #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
> @@ -34,6 +36,7 @@
> 
>  #define DPU_REG_RESET_TIMEOUT_US        2000
>  #define  MERGE_3D_IDX   23
> +#define  DSC_IDX        22
>  #define  INTF_IDX       31
>  #define CTL_INVALID_BIT                 0xffff
> 
> @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct
> dpu_hw_ctl *ctx)
> 
>  static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>  {
> +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | 
> BIT(3));
> 
>  	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
>  		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
> @@ -128,7 +132,7 @@ static inline void
> dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
>  		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
>  				ctx->pending_intf_flush_mask);
> 
> -	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  
> BIT(DSC_IDX));
>  }
> 
>  static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
> @@ -507,6 +511,7 @@ 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));
> +	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
>  }
> 
>  static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,
Vinod Koul Oct. 6, 2021, 12:21 p.m. UTC | #3 
On 30-07-21, 01:15, Dmitry Baryshkov wrote:
> On 15/07/2021 09:51, Vinod Koul wrote:

> > Later gens of hardware have DSC bits moved to hw_ctl, so configure these

> > bits so that DSC would work there as well

> > 

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> > ---

> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 ++++++-

> >   1 file changed, 6 insertions(+), 1 deletion(-)

> > 

> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c

> > index 2d4645e01ebf..aeea6add61ee 100644

> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c

> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c

> > @@ -25,6 +25,8 @@

> >   #define   CTL_MERGE_3D_ACTIVE           0x0E4

> >   #define   CTL_INTF_ACTIVE               0x0F4

> >   #define   CTL_MERGE_3D_FLUSH            0x100

> > +#define   CTL_DSC_ACTIVE                0x0E8

> > +#define   CTL_DSC_FLUSH                0x104

> >   #define   CTL_INTF_FLUSH                0x110

> >   #define   CTL_INTF_MASTER               0x134

> >   #define   CTL_FETCH_PIPE_ACTIVE         0x0FC

> > @@ -34,6 +36,7 @@

> >   #define DPU_REG_RESET_TIMEOUT_US        2000

> >   #define  MERGE_3D_IDX   23

> > +#define  DSC_IDX        22

> >   #define  INTF_IDX       31

> >   #define CTL_INVALID_BIT                 0xffff

> > @@ -120,6 +123,7 @@ static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)

> >   static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)

> >   {

> > +	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));

> 

> Please pass DSC indices using intf cfg and use them to configure register

> writes.


Yes I have modified the intf cfg dsc from bool to pass actual indices.
So this patch goes next (as a dependency reorder) and we use this only
when DSC is enabled and use the indices set. Thanks for the suggestion

-- 
~Vinod
Vinod Koul Oct. 6, 2021, 12:21 p.m. UTC | #4 
On 02-08-21, 17:00, abhinavk@codeaurora.org wrote:
> On 2021-07-14 23:51, Vinod Koul wrote:

> > Later gens of hardware have DSC bits moved to hw_ctl, so configure these

> > bits so that DSC would work there as well

> > 

> > Signed-off-by: Vinod Koul <vkoul@kernel.org>

> Please correct me if wrong but here you seem to be flushing all the DSC bits

> even the unused ones. This will end-up enabling DSC even when DSC is unused

> on

> the newer targets.

> If so, thats wrong.

> We need to implement bit-mask based approach to avoid this change and only

> enable

> those DSCs which are used.


Yes as Dimitry suggested I have done that by passing indices

-- 
~Vinod
diff mbox series

Patch

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 2d4645e01ebf..aeea6add61ee 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -25,6 +25,8 @@ 
 #define   CTL_MERGE_3D_ACTIVE           0x0E4
 #define   CTL_INTF_ACTIVE               0x0F4
 #define   CTL_MERGE_3D_FLUSH            0x100
+#define   CTL_DSC_ACTIVE                0x0E8
+#define   CTL_DSC_FLUSH                0x104
 #define   CTL_INTF_FLUSH                0x110
 #define   CTL_INTF_MASTER               0x134
 #define   CTL_FETCH_PIPE_ACTIVE         0x0FC
@@ -34,6 +36,7 @@ 
 
 #define DPU_REG_RESET_TIMEOUT_US        2000
 #define  MERGE_3D_IDX   23
+#define  DSC_IDX        22
 #define  INTF_IDX       31
 #define CTL_INVALID_BIT                 0xffff
 
@@ -120,6 +123,7 @@  static u32 dpu_hw_ctl_get_pending_flush(struct dpu_hw_ctl *ctx)
 
 static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 {
+	DPU_REG_WRITE(&ctx->hw, CTL_DSC_FLUSH, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 
 	if (ctx->pending_flush_mask & BIT(MERGE_3D_IDX))
 		DPU_REG_WRITE(&ctx->hw, CTL_MERGE_3D_FLUSH,
@@ -128,7 +132,7 @@  static inline void dpu_hw_ctl_trigger_flush_v1(struct dpu_hw_ctl *ctx)
 		DPU_REG_WRITE(&ctx->hw, CTL_INTF_FLUSH,
 				ctx->pending_intf_flush_mask);
 
-	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
+	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask |  BIT(DSC_IDX));
 }
 
 static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
@@ -507,6 +511,7 @@  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));
+	DPU_REG_WRITE(c, CTL_DSC_ACTIVE, BIT(0) | BIT(1) | BIT(2) | BIT(3));
 }
 
 static void dpu_hw_ctl_intf_cfg(struct dpu_hw_ctl *ctx,