[Intel-gfx] [PATCH] drm/i915: PSR: Flush means invalidate + flush

[Paulo Zanoni]

2015-07-08 20:21 GMT-03:00 Rodrigo Vivi <rodrigo.vivi at intel.com>:
> Since flush actually means invalidate + flush we need to force psr
> exit on PSR flush.
>
> On Core platforms there is no way to disable hw tracking and
> do the pure sw tracking so we simulate it by fully disable psr and
> reschedule a enable back.
> So a good idea is to minimize sequential disable/enable in cases we
> know that HW tracking like when flush has been originated by a flip.
> Also flip had just invalidated it already.
>
> It also uses origin to minimize the a bit the amount of
> disable/enabled, mainly when flip already had invalidated.
>
> With this patch in place it is possible to do a flush on dirty areas
> properly in a following patch.
>
> v2: Remove duplicated exit on HSW+Sprites as pointed out by Paulo.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h         |  3 ++-
>  drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
>  drivers/gpu/drm/i915/intel_psr.c         | 40 +++++++++++++++++---------------
>  3 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index beeb4d3..c863511 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1329,7 +1329,8 @@ void intel_psr_disable(struct intel_dp *intel_dp);
>  void intel_psr_invalidate(struct drm_device *dev,
>                           unsigned frontbuffer_bits);
>  void intel_psr_flush(struct drm_device *dev,
> -                    unsigned frontbuffer_bits);
> +                    unsigned frontbuffer_bits,
> +                    enum fb_op_origin origin);
>  void intel_psr_init(struct drm_device *dev);
>  void intel_psr_single_frame_update(struct drm_device *dev,
>                                    unsigned frontbuffer_bits);
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index cb5a6f0..e73d2ff0 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -128,7 +128,7 @@ void intel_frontbuffer_flush(struct drm_device *dev,
>                 return;
>
>         intel_edp_drrs_flush(dev, frontbuffer_bits);
> -       intel_psr_flush(dev, frontbuffer_bits);
> +       intel_psr_flush(dev, frontbuffer_bits, origin);
>         intel_fbc_flush(dev_priv, frontbuffer_bits);
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index d79ba58..6db043f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -680,6 +680,7 @@ void intel_psr_invalidate(struct drm_device *dev,
>   * intel_psr_flush - Flush PSR
>   * @dev: DRM device
>   * @frontbuffer_bits: frontbuffer plane tracking bits
> + * @origin: which operation caused the flush
>   *
>   * Since the hardware frontbuffer tracking has gaps we need to integrate
>   * with the software frontbuffer tracking. This function gets called every
> @@ -689,7 +690,7 @@ void intel_psr_invalidate(struct drm_device *dev,
>   * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits.
>   */
>  void intel_psr_flush(struct drm_device *dev,
> -                    unsigned frontbuffer_bits)
> +                    unsigned frontbuffer_bits, enum fb_op_origin origin)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_crtc *crtc;
> @@ -707,24 +708,25 @@ void intel_psr_flush(struct drm_device *dev,
>         frontbuffer_bits &= INTEL_FRONTBUFFER_ALL_MASK(pipe);
>         dev_priv->psr.busy_frontbuffer_bits &= ~frontbuffer_bits;
>
> -       /*
> -        * On Haswell sprite plane updates don't result in a psr invalidating
> -        * signal in the hardware. Which means we need to manually fake this in
> -        * software for all flushes, not just when we've seen a preceding
> -        * invalidation through frontbuffer rendering.
> -        */
> -       if (IS_HASWELL(dev) &&
> -           (frontbuffer_bits & INTEL_FRONTBUFFER_SPRITE(pipe)))
> -               intel_psr_exit(dev);
> -
> -       /*
> -        * On Valleyview and Cherryview we don't use hardware tracking so
> -        * any plane updates or cursor moves don't result in a PSR
> -        * invalidating. Which means we need to manually fake this in
> -        * software for all flushes, not just when we've seen a preceding
> -        * invalidation through frontbuffer rendering. */
> -       if (frontbuffer_bits && !HAS_DDI(dev))
> -               intel_psr_exit(dev);
> +       if (HAS_DDI(dev)) {
> +               /*
> +                * By definition every flush should mean invalidate + flush,
> +                * however on core platforms let's minimize the
> +                * disable/re-enable so we can avoid the invalidate when flip
> +                * originated the flush.
> +                */
> +               if (frontbuffer_bits && origin != ORIGIN_FLIP)
> +                       intel_psr_exit(dev);
> +       } else {
> +               /*
> +                * On Valleyview and Cherryview we don't use hardware tracking
> +                * so any plane updates or cursor moves don't result in a PSR
> +                * invalidating. Which means we need to manually fake this in
> +                * software for all flushes.
> +                */
> +               if (frontbuffer_bits)
> +                       intel_psr_exit(dev);
> +       }
>
>         if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
>                 schedule_delayed_work(&dev_priv->psr.work,
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni