diff mbox series

drm/i915/display: Add an extra vblank wait before fbc activation

Message ID 20200817074418.24045-1-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: Add an extra vblank wait before fbc activation | expand

Commit Message

Shankar, Uma Aug. 17, 2020, 7:44 a.m. UTC 
Add an extra vblank before fbc is activated.
WA: 1409689360
Corruption with FBC around plane 1A enabling. In the Frame Buffer
Compression programming sequence "Display Plane Enabling with FBC"
add a wait for vblank between plane enabling step 1 and FBC enabling
step 2.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Ville Syrjälä Aug. 24, 2020, 6:16 p.m. UTC | #1 
On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> Add an extra vblank before fbc is activated.
> WA: 1409689360
> Corruption with FBC around plane 1A enabling. In the Frame Buffer
> Compression programming sequence "Display Plane Enabling with FBC"
> add a wait for vblank between plane enabling step 1 and FBC enabling
> step 2.

Already there due to drm_atomic_helper_wait_for_flip_done().

> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index 2ab32e6532ff..0ed252ff2c53 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct intel_crtc *crtc)
>  	if (!intel_fbc_can_activate(crtc))
>  		return;
>  
> -	if (!fbc->busy_bits)
> +	if (!fbc->busy_bits) {
> +		intel_wait_for_vblank(dev_priv, crtc->pipe);
>  		intel_fbc_hw_activate(dev_priv);
> -	else
> +	} else {
>  		intel_fbc_deactivate(dev_priv, "frontbuffer write");
> +	}
>  }
>  
>  void intel_fbc_post_update(struct intel_atomic_state *state,
> -- 
> 2.22.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shankar, Uma Aug. 24, 2020, 7:46 p.m. UTC | #2 
> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Monday, August 24, 2020 11:46 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait
> before fbc activation
> 
> On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> > Add an extra vblank before fbc is activated.
> > WA: 1409689360
> > Corruption with FBC around plane 1A enabling. In the Frame Buffer
> > Compression programming sequence "Display Plane Enabling with FBC"
> > add a wait for vblank between plane enabling step 1 and FBC enabling
> > step 2.
> 
> Already there due to drm_atomic_helper_wait_for_flip_done().

Hi Ville,
__intel_fbc_post_update is also called through intel_fbc_flush. The extra wait at that point seem
to be taking care of this case as well.

We can add it in vblank worker as suggested by Maarten or do you feel this should be handled differently.

Regards,
Uma Shankar

> >
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index 2ab32e6532ff..0ed252ff2c53 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct
> intel_crtc *crtc)
> >  	if (!intel_fbc_can_activate(crtc))
> >  		return;
> >
> > -	if (!fbc->busy_bits)
> > +	if (!fbc->busy_bits) {
> > +		intel_wait_for_vblank(dev_priv, crtc->pipe);
> >  		intel_fbc_hw_activate(dev_priv);
> > -	else
> > +	} else {
> >  		intel_fbc_deactivate(dev_priv, "frontbuffer write");
> > +	}
> >  }
> >
> >  void intel_fbc_post_update(struct intel_atomic_state *state,
> > --
> > 2.22.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel
Ville Syrjälä Aug. 24, 2020, 8 p.m. UTC | #3 
On Mon, Aug 24, 2020 at 07:46:30PM +0000, Shankar, Uma wrote:
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Monday, August 24, 2020 11:46 PM
> > To: Shankar, Uma <uma.shankar@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Add an extra vblank wait
> > before fbc activation
> > 
> > On Mon, Aug 17, 2020 at 01:14:18PM +0530, Uma Shankar wrote:
> > > Add an extra vblank before fbc is activated.
> > > WA: 1409689360
> > > Corruption with FBC around plane 1A enabling. In the Frame Buffer
> > > Compression programming sequence "Display Plane Enabling with FBC"
> > > add a wait for vblank between plane enabling step 1 and FBC enabling
> > > step 2.
> > 
> > Already there due to drm_atomic_helper_wait_for_flip_done().
> 
> Hi Ville,
> __intel_fbc_post_update is also called through intel_fbc_flush. The extra wait at that point seem
> to be taking care of this case as well.
> 
> We can add it in vblank worker as suggested by Maarten or do you feel this should be handled differently.

There's already supposed to be something that prevents the frontbuffer
stuff from racing with plane updates.

> 
> Regards,
> Uma Shankar
> 
> > >
> > > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_fbc.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > index 2ab32e6532ff..0ed252ff2c53 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > > @@ -1085,10 +1085,12 @@ static void __intel_fbc_post_update(struct
> > intel_crtc *crtc)
> > >  	if (!intel_fbc_can_activate(crtc))
> > >  		return;
> > >
> > > -	if (!fbc->busy_bits)
> > > +	if (!fbc->busy_bits) {
> > > +		intel_wait_for_vblank(dev_priv, crtc->pipe);
> > >  		intel_fbc_hw_activate(dev_priv);
> > > -	else
> > > +	} else {
> > >  		intel_fbc_deactivate(dev_priv, "frontbuffer write");
> > > +	}
> > >  }
> > >
> > >  void intel_fbc_post_update(struct intel_atomic_state *state,
> > > --
> > > 2.22.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > --
> > Ville Syrjälä
> > Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 2ab32e6532ff..0ed252ff2c53 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1085,10 +1085,12 @@  static void __intel_fbc_post_update(struct intel_crtc *crtc)
 	if (!intel_fbc_can_activate(crtc))
 		return;
 
-	if (!fbc->busy_bits)
+	if (!fbc->busy_bits) {
+		intel_wait_for_vblank(dev_priv, crtc->pipe);
 		intel_fbc_hw_activate(dev_priv);
-	else
+	} else {
 		intel_fbc_deactivate(dev_priv, "frontbuffer write");
+	}
 }
 
 void intel_fbc_post_update(struct intel_atomic_state *state,