[RFC,7/8] drm/i915: Allow reset without error capture

Commit Message

[e-mail] March 16, 2018, 6:31 p.m. UTC

From: Jeff McGee <[e-mail]>

Pull the reset handling out of i915_handle_error() so that it can be
called by that function and directly by the upcoming force preemption
handler. This allows the force preemption handler to bypass the error
capture that i915_handle_error() does before getting on with the
reset. We do not want error capture for force preemption because it
adds significant latency (~10 msecs measured on APL).

This patch is required to support the force preemption feature.

Change-Id: I41b4fae1adc197f0e70cec47cb960a0d7fa55f48
Signed-off-by: Jeff McGee <[e-mail]>
---
 drivers/gpu/drm/i915/i915_drv.h |  2 ++
 drivers/gpu/drm/i915/i915_irq.c | 75 +++++++++++++++++++++++------------------
 2 files changed, 45 insertions(+), 32 deletions(-)

Comments

Chris Wilson March 16, 2018, 8:33 p.m. UTC | #1

Quoting [e-mail] (2018-03-16 18:31:04)
> From: Jeff McGee <[e-mail]>
> 
> Pull the reset handling out of i915_handle_error() so that it can be
> called by that function and directly by the upcoming force preemption
> handler. This allows the force preemption handler to bypass the error
> capture that i915_handle_error() does before getting on with the
> reset. We do not want error capture for force preemption because it
> adds significant latency (~10 msecs measured on APL).

And we don't need to capture the error in many other cases; just add a
control flag.
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d8524357373e..ade09f97be5c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3245,6 +3245,8 @@  __printf(3, 4)
 void i915_handle_error(struct drm_i915_private *dev_priv,
 		       u32 engine_mask,
 		       const char *fmt, ...);
+void i915_handle_reset(struct drm_i915_private *dev_priv,
+		       u32 engine_mask);
 
 extern void intel_irq_init(struct drm_i915_private *dev_priv);
 extern void intel_irq_fini(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a34f459f8ac1..ab5d4d40083d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2673,41 +2673,17 @@  static void i915_clear_error_registers(struct drm_i915_private *dev_priv)
 }
 
 /**
- * i915_handle_error - handle a gpu error
+ * i915_handle_reset - handle a gpu reset
  * @dev_priv: i915 device private
- * @engine_mask: mask representing engines that are hung
- * @fmt: Error message format string
+ * @engine_mask: mask representing engines that require reset
  *
- * Do some basic checking of register state at error time and
- * dump it to the syslog.  Also call i915_capture_error_state() to make
- * sure we get a record and make it available in debugfs.  Fire a uevent
- * so userspace knows something bad happened (should trigger collection
- * of a ring dump etc.).
+ * Executes reset on the given engines.
  */
-void i915_handle_error(struct drm_i915_private *dev_priv,
-		       u32 engine_mask,
-		       const char *fmt, ...)
+void i915_handle_reset(struct drm_i915_private *dev_priv,
+		       u32 engine_mask)
 {
 	struct intel_engine_cs *engine;
 	unsigned int tmp;
-	va_list args;
-	char error_msg[80];
-
-	va_start(args, fmt);
-	vscnprintf(error_msg, sizeof(error_msg), fmt, args);
-	va_end(args);
-
-	/*
-	 * In most cases it's guaranteed that we get here with an RPM
-	 * reference held, for example because there is a pending GPU
-	 * request that won't finish until the reset is done. This
-	 * isn't the case at least when we get here by doing a
-	 * simulated reset via debugfs, so get an RPM reference.
-	 */
-	intel_runtime_pm_get(dev_priv);
-
-	i915_capture_error_state(dev_priv, engine_mask, error_msg);
-	i915_clear_error_registers(dev_priv);
 
 	/*
 	 * Try engine reset when available. We fall back to full reset if
@@ -2731,14 +2707,14 @@  void i915_handle_error(struct drm_i915_private *dev_priv,
 	}
 
 	if (!engine_mask)
-		goto out;
+		return;
 
 	/* Full reset needs the mutex, stop any other user trying to do so. */
 	if (test_and_set_bit(I915_RESET_BACKOFF, &dev_priv->gpu_error.flags)) {
 		wait_event(dev_priv->gpu_error.reset_queue,
 			   !test_bit(I915_RESET_BACKOFF,
 				     &dev_priv->gpu_error.flags));
-		goto out;
+		return;
 	}
 
 	/* Prevent any other reset-engine attempt. */
@@ -2759,8 +2735,43 @@  void i915_handle_error(struct drm_i915_private *dev_priv,
 
 	clear_bit(I915_RESET_BACKOFF, &dev_priv->gpu_error.flags);
 	wake_up_all(&dev_priv->gpu_error.reset_queue);
+}
+/**
+ * i915_handle_error - handle a gpu error
+ * @dev_priv: i915 device private
+ * @engine_mask: mask representing engines that are hung
+ * @fmt: Error message format string
+ *
+ * Do some basic checking of register state at error time and
+ * dump it to the syslog.  Also call i915_capture_error_state() to make
+ * sure we get a record and make it available in debugfs.  Fire a uevent
+ * so userspace knows something bad happened (should trigger collection
+ * of a ring dump etc.).
+ */
+void i915_handle_error(struct drm_i915_private *dev_priv,
+		       u32 engine_mask,
+		       const char *fmt, ...)
+{
+	va_list args;
+	char error_msg[80];
+
+	va_start(args, fmt);
+	vscnprintf(error_msg, sizeof(error_msg), fmt, args);
+	va_end(args);
+
+	/*
+	 * In most cases it's guaranteed that we get here with an RPM
+	 * reference held, for example because there is a pending GPU
+	 * request that won't finish until the reset is done. This
+	 * isn't the case at least when we get here by doing a
+	 * simulated reset via debugfs, so get an RPM reference.
+	 */
+	intel_runtime_pm_get(dev_priv);
+
+	i915_capture_error_state(dev_priv, engine_mask, error_msg);
+	i915_clear_error_registers(dev_priv);
+	i915_handle_reset(dev_priv, engine_mask);
 
-out:
 	intel_runtime_pm_put(dev_priv);
 }