diff mbox

[2/7] drm/i915: PSR: Flush means invalidate + flush

Message ID 1436311737-18270-2-git-send-email-rodrigo.vivi@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi July 7, 2015, 11:28 p.m. UTC
Since flush actually means invalidate + flush we need to force psr
exit on PSR flush.

On Core platforms there is no way to do the 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.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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         | 51 +++++++++++++++++++++-----------
 3 files changed, 36 insertions(+), 20 deletions(-)

Comments

Paulo Zanoni July 8, 2015, 1:58 p.m. UTC | #1
2015-07-07 20:28 GMT-03:00 Rodrigo Vivi <rodrigo.vivi@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 do the sw tracking so we

There is no way to do the _HW_ tracking?

> 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.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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         | 51 +++++++++++++++++++++-----------
>  3 files changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1f3f786..c5005f2 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..dd174ae 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,38 @@ 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);
> +       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);
>
> -       /*
> -        * 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);
> +               /*
> +                * 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);

With this, we'll call intel_psr_exit() twice for sprites. It seems we
never get ORIGIN_FLIP for sprites, so you can just kill these lines.
Everything else looks correct.

> +
> +       } 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, not just when we've seen a
> +                * preceding invalidation through frontbuffer rendering.
> +                */
> +               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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1f3f786..c5005f2 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..dd174ae 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,38 @@  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);
+	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);
 
-	/*
-	 * 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);
+		/*
+		 * 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);
+
+	} 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, not just when we've seen a
+		 * preceding invalidation through frontbuffer rendering.
+		 */
+		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,