What if FBC is disabled? Or FBC can not be activate by any of the
reasons in intel_fbc_can_activate(). The hardware tracking would never
trigger a PSR exit by it self?!

On Tue, 2018-03-06 at 19:34 -0800, Dhinakaran Pandiyan wrote:
> From: Rodrigo Vivi <rodrigo.v...@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.pandi...@intel.com>
> Cc: Jim Bride <jim.br...@linux.intel.com>
> Cc: Vathsala Nagaraju <vathsala.nagar...@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.v...@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandi...@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 7eec99d7fad4..6aab929f354c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -772,6 +772,7 @@ struct i915_psr {
>       bool y_cord_support;
>       bool colorimetry_support;
>       bool alpm;
> +     bool has_hw_tracking;
>  
>       void (*enable_source)(struct intel_dp *,
>                             const struct intel_crtc_state *);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1a7c5addcec3..fdf7dd100dfe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1873,7 +1873,8 @@ void intel_psr_enable(struct intel_dp
> *intel_dp,
>  void intel_psr_disable(struct intel_dp *intel_dp,
>                     const struct intel_crtc_state
> *old_crtc_state);
>  void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> -                       unsigned frontbuffer_bits);
> +                       unsigned frontbuffer_bits,
> +                       enum fb_op_origin origin);
>  void intel_psr_flush(struct drm_i915_private *dev_priv,
>                    unsigned frontbuffer_bits,
>                    enum fb_op_origin origin);
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c
> b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index 3a8d3d06c26a..7fff0a0eceb4 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -80,7 +80,7 @@ void __intel_fb_obj_invalidate(struct
> drm_i915_gem_object *obj,
>       }
>  
>       might_sleep();
> -     intel_psr_invalidate(dev_priv, frontbuffer_bits);
> +     intel_psr_invalidate(dev_priv, frontbuffer_bits, origin);
>       intel_edp_drrs_invalidate(dev_priv, frontbuffer_bits);
>       intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 05770790a4e9..16b77e19c28e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -948,6 +948,7 @@ void intel_psr_single_frame_update(struct
> drm_i915_private *dev_priv,
>   * intel_psr_invalidate - Invalidade PSR
>   * @dev_priv: i915 device
>   * @frontbuffer_bits: frontbuffer plane tracking bits
> + * @origin: which operation caused the invalidate
>   *
>   * Since the hardware frontbuffer tracking has gaps we need to
> integrate
>   * with the software frontbuffer tracking. This function gets called
> every
> @@ -957,7 +958,7 @@ void intel_psr_single_frame_update(struct
> drm_i915_private *dev_priv,
>   * Dirty frontbuffers relevant to PSR are tracked in
> busy_frontbuffer_bits."
>   */
>  void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> -                       unsigned frontbuffer_bits)
> +                       unsigned frontbuffer_bits, enum
> fb_op_origin origin)
>  {
>       struct drm_crtc *crtc;
>       enum pipe pipe;
> @@ -965,6 +966,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);
> @@ -1005,6 +1009,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;
> +
>       mutex_lock(&dev_priv->psr.lock);
>       if (!dev_priv->psr.enabled) {
>               mutex_unlock(&dev_priv->psr.lock);
> @@ -1081,6 +1088,7 @@ void intel_psr_init(struct drm_i915_private
> *dev_priv)
>               dev_priv->psr.activate = vlv_psr_activate;
>               dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
>       } else {
> +             dev_priv->psr.has_hw_tracking = true;
>               dev_priv->psr.enable_source = hsw_psr_enable_source;
>               dev_priv->psr.disable_source = hsw_psr_disable;
>               dev_priv->psr.enable_sink = hsw_psr_enable_sink;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to