[PATCH] drm/i915: Replace the fixed vblank evasion with a ewma of past update times

* [PATCH] drm/i915: Replace the fixed vblank evasion with a ewma of past update times
@ 2019-02-14 13:29 Chris Wilson
  2019-02-14 17:33 ` ✗ Fi.CI.SPARSE: warning for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Chris Wilson @ 2019-02-14 13:29 UTC (permalink / raw)
  To: intel-gfx

Currently we have a fixed estimate for how long we think it will take to
rewrite the registers for the modeset to apply, as we have to ensure
that we complete the update before the next vblank. We can improve upon
our estimate by keeping track of the average for the past few updates,
as each will typically require rewriting the same amount of state. In
case there is a dramatic difference, or the system is slow for whatever
reason, we factor in a small bit of safety.

As we now accommodate variances between systems and automatically update
such circumstances change, no longer log it as an outright error in the
driver, but as a significant, but unfortunate common occurrence.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  9 ++++++
 drivers/gpu/drm/i915/intel_drv.h     |  4 +++
 drivers/gpu/drm/i915/intel_sprite.c  | 41 +++++++++++++---------------
 3 files changed, 32 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0a8913b2059e..c2f28913bfa3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -60,6 +60,12 @@
 #include "i915_reset.h"
 #include "i915_trace.h"
 
+#if IS_ENABLED(CONFIG_PROVE_LOCKING)
+#define VBLANK_EVASION_TIME_US 250
+#else
+#define VBLANK_EVASION_TIME_US 100
+#endif
+
 /* Primary plane formats for gen <= 3 */
 static const u32 i8xx_primary_formats[] = {
 	DRM_FORMAT_C8,
@@ -14374,6 +14380,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	intel_crtc->base.state = &crtc_state->base;
 	crtc_state->base.crtc = &intel_crtc->base;
 
+	ewma_evade_init(&intel_crtc->evasion);
+	ewma_evade_add(&intel_crtc->evasion, VBLANK_EVASION_TIME_US);
+
 	primary = intel_primary_plane_create(dev_priv, pipe);
 	if (IS_ERR(primary)) {
 		ret = PTR_ERR(primary);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3398b28c053b..80d30fe5d50b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -26,6 +26,7 @@
 #define __INTEL_DRV_H__
 
 #include <linux/async.h>
+#include <linux/average.h>
 #include <linux/i2c.h>
 #include <linux/hdmi.h>
 #include <linux/sched/clock.h>
@@ -984,6 +985,8 @@ struct intel_crtc_state {
 	bool fec_enable;
 };
 
+DECLARE_EWMA(evade, 2, 4);
+
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
@@ -1014,6 +1017,7 @@ struct intel_crtc {
 	} wm;
 
 	int scanline_offset;
+	struct ewma_evade evasion;
 
 	struct {
 		unsigned start_vbl_count;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 610398607e8e..cc116c2d0959 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -54,11 +54,6 @@ int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 
 /* FIXME: We should instead only take spinlocks once for the entire update
  * instead of once per mmio. */
-#if IS_ENABLED(CONFIG_PROVE_LOCKING)
-#define VBLANK_EVASION_TIME_US 250
-#else
-#define VBLANK_EVASION_TIME_US 100
-#endif
 
 /**
  * intel_pipe_update_start() - start update of a set of display registers
@@ -84,14 +79,18 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
 		intel_crtc_has_type(new_crtc_state, INTEL_OUTPUT_DSI);
 	DEFINE_WAIT(wait);
 	u32 psr_status;
+	int evasion;
 
 	vblank_start = adjusted_mode->crtc_vblank_start;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
 		vblank_start = DIV_ROUND_UP(vblank_start, 2);
 
-	/* FIXME needs to be calibrated sensibly */
-	min = vblank_start - intel_usecs_to_scanlines(adjusted_mode,
-						      VBLANK_EVASION_TIME_US);
+	evasion = max(2 * ewma_evade_read(&crtc->evasion), 100ul);
+	DRM_DEBUG_KMS("Predicted evasion time: %lu, using %u\n",
+		      ewma_evade_read(&crtc->evasion),
+		      evasion);
+
+	min = vblank_start - intel_usecs_to_scanlines(adjusted_mode, evasion);
 	max = vblank_start - 1;
 
 	if (min <= 0 || max <= 0)
@@ -213,21 +212,19 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 
 	if (crtc->debug.start_vbl_count &&
 	    crtc->debug.start_vbl_count != end_vbl_count) {
-		DRM_ERROR("Atomic update failure on pipe %c (start=%u end=%u) time %lld us, min %d, max %d, scanline start %d, end %d\n",
-			  pipe_name(pipe), crtc->debug.start_vbl_count,
-			  end_vbl_count,
-			  ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
-			  crtc->debug.min_vbl, crtc->debug.max_vbl,
-			  crtc->debug.scanline_start, scanline_end);
+		dev_notice(dev_priv->drm.dev,
+			   "Atomic update failure on pipe %c (start=%u end=%u) time %lld us (estimated %lu us), min %d, max %d, scanline start %d, end %d\n",
+			   pipe_name(pipe), crtc->debug.start_vbl_count,
+			   end_vbl_count,
+			   ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
+			   ewma_evade_read(&crtc->evasion),
+			   crtc->debug.min_vbl, crtc->debug.max_vbl,
+			   crtc->debug.scanline_start, scanline_end);
 	}
-#ifdef CONFIG_DRM_I915_DEBUG_VBLANK_EVADE
-	else if (ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time) >
-		 VBLANK_EVASION_TIME_US)
-		DRM_WARN("Atomic update on pipe (%c) took %lld us, max time under evasion is %u us\n",
-			 pipe_name(pipe),
-			 ktime_us_delta(end_vbl_time, crtc->debug.start_vbl_time),
-			 VBLANK_EVASION_TIME_US);
-#endif
+
+	ewma_evade_add(&crtc->evasion,
+		       ktime_us_delta(end_vbl_time,
+				      crtc->debug.start_vbl_time));
 }
 
 int intel_plane_check_stride(const struct intel_plane_state *plane_state)
-- 
2.20.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 15+ messages in thread