[Intel-gfx] [PATCH] drm/i915/psr: Display WA 0884 applied broadly for more HW tracking.

[Pandiyan, Dhinakaran]

On Mon, 2018-03-12 at 11:45 -0700, Rodrigo Vivi wrote:
> On Mon, Mar 12, 2018 at 11:07:55AM -0700, Pandiyan, Dhinakaran wrote:
> > 
> > 
> > 
> > On Thu, 2018-03-08 at 16:52 -0800, Rodrigo Vivi wrote:
> > > WA 0884:bxt:all,cnl:*:A - "When FBC is enabled with eDP PSR,
> > > the CPU host modify writes may not get updated on the Display
> > > as expected.
> > > WA: Write 0x00000000 to CUR_SURFLIVE_A with every CPU
> > > host modify write to trigger PSR exit."
> > > 
> > > We can also find on spec other cases where they describe
> > > bogus writes to cursor registers to force PSR exit with
> > > HW tracking. And it was confirmed by HW engineers that
> > > this Wa can be safely applied for any frontbuffer activity.
> > > 
> > > So let's use this more and more here instead of forcibly
> > > disable and re-enable PSR everytime that we have a simple
> > > reliable flush case.
> > > 
> > > Other commits improve the fbcon/fbdev use a lot, but this
> > > approach is the only when where we can get a fully reliable
> > > console with no slowness or missed frames and PSR still
> > > enabled and active.
> > > 
> > > v2: - Rebase on drm-tip
> > >     - (DK) Add a comment to explain that WA
> > >     tells about writing 0 to CUR_SURFLIVE_A but we write to
> > >     CUR_SURFLIVE(pipe).
> > > v3: Wa doesn't work on PSR2.
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h  |  3 +++
> > >  drivers/gpu/drm/i915/intel_psr.c | 19 +++++++++++++++++--
> > >  2 files changed, 20 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index e6a8c0ee7df1..abdc513a9edd 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -6032,6 +6032,9 @@ enum {
> > >  #define IVB_CURSOR_B_OFFSET 0x71080
> > >  #define IVB_CURSOR_C_OFFSET 0x72080
> > >  
> > > +#define _CUR_SURLIVE		0x700AC
> > > +#define CUR_SURLIVE(pipe)	_CURSOR2(pipe, _CUR_SURLIVE)
> > > +
> > >  /* Display A control */
> > >  #define _DSPACNTR				0x70180
> > >  #define   DISPLAY_PLANE_ENABLE			(1<<31)
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > > index 30932527e663..b0286722a72f 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1027,8 +1027,23 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> > >  	dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
> > >  
> > >  	/* By definition flush = invalidate + flush */
> > > -	if (frontbuffer_bits)
> > > -		intel_psr_exit(dev_priv);
> > > +	if (frontbuffer_bits) {
> > > +		if (dev_priv->psr.psr2_support ||
> > > +		    IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> > 
> > I forgot if I asked you already about this, why not
> > psr.has_hw_tracking? 
> 
> because we don't have that yet ;)
> 
> I wonder if we should do this change in a separated patch
> 
> > 
> > My interpretation of the flag is - any platform that can exit PSR by 
> > - driver writing to some pipe/plane MMIO
> > - HW tracking frontbuffer modifications
> > - enabling vblanks 
> > 
> > The patch works well on my SKL laptop with fbcon.
> > 
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> 
> Thanks. Pushed to dinq.
> 
Can the frontbuffer patches
(https://patchwork.freedesktop.org/series/39502/) be pushed too? We
should have PSR usable on SKL with the frontbuffer patches on top of
this one.



> > 
> > 
> > 
> > 
> > 
> > > +			intel_psr_exit(dev_priv);
> > > +		} else {
> > > +			/*
> > > +			 * Display WA #0884: all
> > > +			 * This documented WA for bxt can be safely applied
> > > +			 * broadly so we can force HW tracking to exit PSR
> > > +			 * instead of disabling and re-enabling.
> > > +			 * Workaround tells us to write 0 to CUR_SURLIVE_A,
> > > +			 * but it makes more sense write to the current active
> > > +			 * pipe.
> > > +			 */
> > > +			I915_WRITE(CUR_SURLIVE(pipe), 0);
> > > +		}
> > > +	}
> > >  
> > >  	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> > >  		if (!work_busy(&dev_priv->psr.work.work))
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx