[Intel-gfx] [PATCH] drm/i915/display: Record the plane update times for debugging

* [Intel-gfx] [PATCH] drm/i915/display: Record the plane update times for debugging
@ 2020-11-23 14:55 Chris Wilson
  2020-11-23 15:55 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
                   ` (15 more replies)
  0 siblings, 16 replies; 30+ messages in thread
From: Chris Wilson @ 2020-11-23 14:55 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson

Since we try and estimate how long we require to update the registers to
perform a plane update, it is of vital importance that we measure the
distribution of plane updates to better guide our estimate. If we
underestimate how long it takes to perform the plane update, we may
slip into the next scanout frame causing a tear. If we overestimate, we
may unnecessarily delay the update to the next frame, causing visible
jitter.

Replace the warning that we exceed some arbitrary threshold for the
vblank update with a histogram for debugfs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  | 46 +++++++++++++++++++
 .../drm/i915/display/intel_display_types.h    |  6 +++
 drivers/gpu/drm/i915/display/intel_sprite.c   | 26 ++++++++---
 3 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index ca41e8c00ad7..aa7a2a679138 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -865,6 +865,50 @@ static void intel_scaler_info(struct seq_file *m, struct intel_crtc *crtc)
 	}
 }
 
+#ifdef CONFIG_DRM_I915_DEBUG_VBLANK_EVADE
+static void crtc_vblank_info(struct seq_file *m, struct intel_crtc *crtc)
+{
+	char buf[ARRAY_SIZE(crtc->debug.vbl_times) + 1] = {};
+	int h, row, max;
+	u64 count;
+
+	max = 0;
+	count = 0;
+	for (h = 0; h < ARRAY_SIZE(crtc->debug.vbl_times); h++) {
+		if (crtc->debug.vbl_times[h] > max)
+			max = crtc->debug.vbl_times[h];
+		count += crtc->debug.vbl_times[h];
+	}
+	seq_printf(m, "\tUpdates: %llu\n", count);
+	if (!count)
+		return;
+
+	memset(buf, '-', sizeof(buf) - 1);
+	seq_printf(m, "\t  |%s|\n", buf);
+
+	max = ilog2(max);
+	for (row = max - 1; row--; ) {
+		memset(buf, ' ', sizeof(buf) - 1);
+		for (h = 0; h < ARRAY_SIZE(crtc->debug.vbl_times); h++) {
+			if (ilog2(crtc->debug.vbl_times[h]) >= row)
+				buf[h] = '*';
+		}
+		seq_printf(m, "\t  |%s|\n", buf);
+	}
+
+	memset(buf, '-', sizeof(buf) - 1);
+	seq_printf(m, "\t  |%s|\n", buf);
+	seq_printf(m, "\t    1us     (log)     1ms\n");
+
+	seq_printf(m, "\tMin update: %lluns\n", crtc->debug.min_vbl_time);
+	seq_printf(m, "\tMax update: %lluns\n", crtc->debug.max_vbl_time);
+	seq_printf(m, "\tAverage update: %lluns\n",
+		   div64_u64(crtc->debug.sum_vbl_time,  count));
+}
+#else
+static void crtc_vblank_info(struct seq_file *m, struct intel_crtc *crtc) {}
+#endif
+
 static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -907,6 +951,8 @@ static void intel_crtc_info(struct seq_file *m, struct intel_crtc *crtc)
 	seq_printf(m, "\tunderrun reporting: cpu=%s pch=%s\n",
 		   yesno(!crtc->cpu_fifo_underrun_disabled),
 		   yesno(!crtc->pch_fifo_underrun_disabled));
+
+	crtc_vblank_info(m, crtc);
 }
 
 static int i915_display_info(struct seq_file *m, void *unused)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index ce82d654d0f2..01214636be56 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1186,6 +1186,12 @@ struct intel_crtc {
 		ktime_t start_vbl_time;
 		int min_vbl, max_vbl;
 		int scanline_start;
+#ifdef CONFIG_DRM_I915_DEBUG_VBLANK_EVADE
+		u64 min_vbl_time;
+		u64 max_vbl_time;
+		u64 sum_vbl_time;
+		unsigned int vbl_times[20]; /* [1us, 1ms] */
+#endif
 	} debug;
 
 	/* scalers available on this crtc */
diff --git a/drivers/gpu/drm/i915/display/intel_sprite.c b/drivers/gpu/drm/i915/display/intel_sprite.c
index 019a2d6d807a..fdb54a569224 100644
--- a/drivers/gpu/drm/i915/display/intel_sprite.c
+++ b/drivers/gpu/drm/i915/display/intel_sprite.c
@@ -250,13 +250,25 @@ void intel_pipe_update_end(struct intel_crtc_state *new_crtc_state)
 			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(&dev_priv->drm,
-			 "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);
+	{
+		unsigned int h;
+		u64 delta;
+
+		delta = ktime_to_ns(ktime_sub(end_vbl_time,
+					      crtc->debug.start_vbl_time));
+
+		h = ilog2(delta >> 9);
+		if (h >= ARRAY_SIZE(crtc->debug.vbl_times))
+			h = ARRAY_SIZE(crtc->debug.vbl_times) - 1;
+		crtc->debug.vbl_times[h]++;
+
+		crtc->debug.sum_vbl_time += delta;
+		if (!crtc->debug.min_vbl_time ||
+		    delta < crtc->debug.min_vbl_time)
+			crtc->debug.min_vbl_time = delta;
+		if (delta > crtc->debug.max_vbl_time)
+			crtc->debug.max_vbl_time = delta;
+	}
 #endif
 }
 
-- 
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] 30+ messages in thread