[v2,1/3] drm/msm: dpu: Mask inactive pending flushes

Submitted by Sean Paul on Oct. 30, 2018, 4 p.m.

Details

Message ID 20181030160033.18464-1-sean@poorly.run
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in DRI devel

Not browsing as part of any series.

Commit Message

Sean Paul Oct. 30, 2018, 4 p.m.
From: Sean Paul <seanpaul@chromium.org>

This patch masks any pending flushes which have not been latched for a
commit. This will catch the case where an asynchronous update is
nullified by a disable in the same frame.

Changes in v2:
- Added to the set

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++++++
 1 file changed, 7 insertions(+)

Patch hide | download patch | download mbox

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 8fa601a9abbf..d7a7fedc09f7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
@@ -28,6 +28,7 @@ 
 #define   CTL_TOP                       0x014
 #define   CTL_FLUSH                     0x018
 #define   CTL_START                     0x01C
+#define   CTL_FLUSH_MASK                0x090
 #define   CTL_PREPARE                   0x0d0
 #define   CTL_SW_RESET                  0x030
 #define   CTL_LAYER_EXTN_OFFSET         0x40
@@ -121,6 +122,12 @@  static inline void dpu_hw_ctl_trigger_flush(struct dpu_hw_ctl *ctx)
 {
 	trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
 				     dpu_hw_ctl_get_flush_register(ctx));
+
+	/*
+	 * Async updates could have changed CTL_FLUSH since it was last latched.
+	 * Mask anything not involved in this latest commit.
+	 */
+	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
 	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
 }
 

Comments

On 2018-10-30 09:00, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch masks any pending flushes which have not been latched for a
> commit. This will catch the case where an asynchronous update is
> nullified by a disable in the same frame.
> 
> Changes in v2:
> - Added to the set
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> 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 8fa601a9abbf..d7a7fedc09f7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> @@ -28,6 +28,7 @@
>  #define   CTL_TOP                       0x014
>  #define   CTL_FLUSH                     0x018
>  #define   CTL_START                     0x01C
> +#define   CTL_FLUSH_MASK                0x090
>  #define   CTL_PREPARE                   0x0d0
>  #define   CTL_SW_RESET                  0x030
>  #define   CTL_LAYER_EXTN_OFFSET         0x40
> @@ -121,6 +122,12 @@ static inline void dpu_hw_ctl_trigger_flush(struct
> dpu_hw_ctl *ctx)
>  {
>  	trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
>  				     dpu_hw_ctl_get_flush_register(ctx));
> +
> +	/*
> +	 * Async updates could have changed CTL_FLUSH since it was last
> latched.
> +	 * Mask anything not involved in this latest commit.
> +	 */
> +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
Do we need this change for adding the current async cursor support?
We are not masking any bit by default. So there is no need for updating 
it here.

The usage of flush_mask is not completely explored yet. Maybe we can add
this register support when we revisit this async logic as we discussed.

Thanks and Regards,
Jeykumar S.
>  	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>  }
On Thu, Nov 08, 2018 at 01:03:03PM -0800, Jeykumar Sankaran wrote:
> On 2018-10-30 09:00, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > This patch masks any pending flushes which have not been latched for a
> > commit. This will catch the case where an asynchronous update is
> > nullified by a disable in the same frame.
> > 
> > Changes in v2:
> > - Added to the set
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > 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 8fa601a9abbf..d7a7fedc09f7 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
> > @@ -28,6 +28,7 @@
> >  #define   CTL_TOP                       0x014
> >  #define   CTL_FLUSH                     0x018
> >  #define   CTL_START                     0x01C
> > +#define   CTL_FLUSH_MASK                0x090
> >  #define   CTL_PREPARE                   0x0d0
> >  #define   CTL_SW_RESET                  0x030
> >  #define   CTL_LAYER_EXTN_OFFSET         0x40
> > @@ -121,6 +122,12 @@ static inline void dpu_hw_ctl_trigger_flush(struct
> > dpu_hw_ctl *ctx)
> >  {
> >  	trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
> >  				     dpu_hw_ctl_get_flush_register(ctx));
> > +
> > +	/*
> > +	 * Async updates could have changed CTL_FLUSH since it was last
> > latched.
> > +	 * Mask anything not involved in this latest commit.
> > +	 */
> > +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
> Do we need this change for adding the current async cursor support?

Hmm, I think you asked me to implement this at the weekly meeting a little
while ago. Apparently HW team requested that we mask off the bits for
planes which have been disabled-but-not-flushed?

Sean

> We are not masking any bit by default. So there is no need for updating it
> here.
> 
> The usage of flush_mask is not completely explored yet. Maybe we can add
> this register support when we revisit this async logic as we discussed.
> 
> Thanks and Regards,
> Jeykumar S.
> >  	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
> >  }
> 
> -- 
> Jeykumar S
On 2018-11-08 13:40, Sean Paul wrote:
> On Thu, Nov 08, 2018 at 01:03:03PM -0800, Jeykumar Sankaran wrote:
>> On 2018-10-30 09:00, Sean Paul wrote:
>> > From: Sean Paul <seanpaul@chromium.org>
>> >
>> > This patch masks any pending flushes which have not been latched for a
>> > commit. This will catch the case where an asynchronous update is
>> > nullified by a disable in the same frame.
>> >
>> > Changes in v2:
>> > - Added to the set
>> >
>> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> > ---
>> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 7 +++++++
>> >  1 file changed, 7 insertions(+)
>> >
>> > 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 8fa601a9abbf..d7a7fedc09f7 100644
>> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c
>> > @@ -28,6 +28,7 @@
>> >  #define   CTL_TOP                       0x014
>> >  #define   CTL_FLUSH                     0x018
>> >  #define   CTL_START                     0x01C
>> > +#define   CTL_FLUSH_MASK                0x090
>> >  #define   CTL_PREPARE                   0x0d0
>> >  #define   CTL_SW_RESET                  0x030
>> >  #define   CTL_LAYER_EXTN_OFFSET         0x40
>> > @@ -121,6 +122,12 @@ static inline void
> dpu_hw_ctl_trigger_flush(struct
>> > dpu_hw_ctl *ctx)
>> >  {
>> >  	trace_dpu_hw_ctl_trigger_pending_flush(ctx->pending_flush_mask,
>> >  				     dpu_hw_ctl_get_flush_register(ctx));
>> > +
>> > +	/*
>> > +	 * Async updates could have changed CTL_FLUSH since it was last
>> > latched.
>> > +	 * Mask anything not involved in this latest commit.
>> > +	 */
>> > +	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH_MASK, ~ctx->pending_flush_mask);
>> Do we need this change for adding the current async cursor support?
> 
> Hmm, I think you asked me to implement this at the weekly meeting a 
> little
> while ago. Apparently HW team requested that we mask off the bits for
> planes which have been disabled-but-not-flushed?
> 
OK. If you want to implement the HW team recommendation, you should 
block the
FLUSH writes until both FLUSH and FLUSH_MASK writes goes through. We can 
do
that by writing 0xFFFFFFFF to the FLUSH_MASK indicating "hardware is not 
ready"
at the beginnging of the new vsync window. Since async updates dont wait
for commit_done (vsync), we can do that only for sync commits. Once we 
are
done programming all the registers and the final flush bits are ready, 
the
order of writing has to be reversed by writing FLUSH first and then 
FLUSH_MASK
to the inverse of FLUSH to unblock the hardware programming on vsync.

Still, there is a small window of error where vsync can happen between 
FLUSH
and FLUSH_MASK writes where we will end up missing the vsync but no 
partial
frame registers will be programmed.

I believe we have decided to try out this approach with a fresh set of 
patches
and let the current cursor support get in as such. In that case, we can 
drop
this patch from this series.

Thanks,
Jeykumar S.

> Sean
> 
>> We are not masking any bit by default. So there is no need for 
>> updating
> it
>> here.
>> 
>> The usage of flush_mask is not completely explored yet. Maybe we can 
>> add
>> this register support when we revisit this async logic as we 
>> discussed.
>> 
>> Thanks and Regards,
>> Jeykumar S.
>> >  	DPU_REG_WRITE(&ctx->hw, CTL_FLUSH, ctx->pending_flush_mask);
>> >  }
>> 
>> --
>> Jeykumar S