[Intel-gfx] [PATCH 2/5] drm/i915/psr: Use more PSR HW tracking.

[Pandiyan, Dhinakaran]

On Fri, 2018-02-16 at 08:54 +0000, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-02-16 04:33:19)
> > From: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > 
> > So far we are using frontbuffer tracking for everything
> > and ignoring that PSR has a HW capable HW tracking for many
> > modern usages of GPU on Core platforms and newer Atom ones.
> > 
> > One reason for that is that we were trying to keep same
> > infrastructure in place for VLV/CHV than the rest of platforms.
> > But also because when this infrastructure was created
> > the front-buffer-tracking origin wasn't that good and stable
> > how it is today after Paulo reworked it to attend FBC cases.
> > 
> > However this PSR implementation without HW tracking died
> > on gen8LP. And newer platforms are starting to demand more HW
> > tracking specially with PSR2 cases in mind.
> > 
> > By disabling and re-enabling PSR totally every time we believe
> > someone is going to change the front buffer content we don't
> > allow PSR HW tracking to do this job and specially compromising
> > the whole idea of PSR2 case where the HW tracking detect only
> > the damaged area and do a partial screen update.
> > 
> > So, from now on, on the platforms that has hw_tracking let's
> > rely more on HW tracking.
> > 
> > This also is the case in used by other drivers and more validated
> > by SV teams. So I hope that this will lead us to less misterious
> > bugs.
> > 
> > v2: Only do this for platform that actually has hw tracking.
> > 
> > v3 from DK
> > Do this only for flips, small gradual changes are better.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > Cc: Jim Bride <jim.bride at linux.intel.com>
> > Cc: Vathsala Nagaraju <vathsala.nagaraju at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  drivers/gpu/drm/i915/intel_drv.h         |  3 ++-
> >  drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
> >  drivers/gpu/drm/i915/intel_psr.c         | 10 +++++++++-
> >  4 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 90eca42ab2b8..31aae988d515 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -770,6 +770,7 @@ struct i915_psr {
> >         bool y_cord_support;
> >         bool colorimetry_support;
> >         bool alpm;
> > +       bool has_hw_tracking;
> 
> Time for some bool:1 compaction?

Oddly it increases the binary size, so I didn't make this change in the
new version I sent out -
https://patchwork.freedesktop.org/series/39502/

I also left out
Patch 5/5: The mod_timer patch that Andy sent avoids the problem this
patch works around.
Patch 4/5: frontbuffer flush during prepare_fb() might be necessary to
exit PSR early (before we start updating pipe registers) 


> 
> > @@ -841,6 +842,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> >         if (!CAN_PSR(dev_priv))
> >                 return;
> >  
> > +       if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
> > +               return;
> > +
> >         mutex_lock(&dev_priv->psr.lock);
> >         if (!dev_priv->psr.enabled) {
> >                 mutex_unlock(&dev_priv->psr.lock);
> > @@ -881,6 +885,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >         if (!CAN_PSR(dev_priv))
> >                 return;
> >  
> > +       if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
> > +               return;
> > +
> 
> Much easier for the causal reader to understand :)
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx