diff mbox series

[6/7] drm/i915/uc: Stop GuC submission during reset prepare

Message ID 20190517162227.6436-7-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series GuC fixes | expand

Commit Message

Michal Wajdeczko May 17, 2019, 4:22 p.m. UTC 
Knowing that GuC will be reset soon, perform only minimal
cleanup actions (ie. doorbells) without talking with GuC.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 25 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_submission.h |  1 +
 drivers/gpu/drm/i915/intel_uc.c             |  3 +++
 3 files changed, 29 insertions(+)

Comments

Chris Wilson May 17, 2019, 4:36 p.m. UTC | #1 
Quoting Michal Wajdeczko (2019-05-17 17:22:26)
> +void intel_guc_submission_stop(struct intel_guc *guc)
> +{
> +       struct drm_i915_private *i915 = guc_to_i915(guc);
> +
> +       GEM_BUG_ON(i915->gt.awake); /* GT should be parked first */

How is this true for reset? Note, it's an unlocked read, so READ_ONCE()
or something along GEM_BUG_ON(intel_wakeref_active(&i915->gt.wakeref))

Though I think this is wrong!

i915_reset or i915_gem_set_wedged
 -> reset_prepare
    -> intel_uc_reset_prepare
       -> intel_guc_submission_stop

No locks, no parking. So how does this survive selftests? :(
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index ea0e3734d37c..8889442a31df 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1208,6 +1208,12 @@  static void __guc_client_disable(struct intel_guc_client *client)
 	guc_proc_desc_fini(client);
 }
 
+static void __guc_client_stop(struct intel_guc_client *client)
+{
+	__fini_doorbell(client);
+	__update_doorbell_desc(client, GUC_DOORBELL_INVALID);
+}
+
 static int guc_clients_enable(struct intel_guc *guc)
 {
 	int ret;
@@ -1236,6 +1242,15 @@  static void guc_clients_disable(struct intel_guc *guc)
 		__guc_client_disable(guc->execbuf_client);
 }
 
+static void guc_clients_stop(struct intel_guc *guc)
+{
+	if (guc->preempt_client)
+		__guc_client_stop(guc->preempt_client);
+
+	if (guc->execbuf_client)
+		__guc_client_stop(guc->execbuf_client);
+}
+
 /*
  * Set up the memory resources to be shared with the GuC (via the GGTT)
  * at firmware loading time.
@@ -1455,6 +1470,16 @@  void intel_guc_submission_disable(struct intel_guc *guc)
 	guc_clients_disable(guc);
 }
 
+void intel_guc_submission_stop(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+
+	GEM_BUG_ON(i915->gt.awake); /* GT should be parked first */
+
+	guc_interrupts_release(i915);
+	guc_clients_stop(guc);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/intel_guc.c"
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.h b/drivers/gpu/drm/i915/intel_guc_submission.h
index 7d823a513b9c..fbfe7b3b0957 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.h
+++ b/drivers/gpu/drm/i915/intel_guc_submission.h
@@ -81,6 +81,7 @@  struct intel_guc_client {
 
 int intel_guc_submission_init(struct intel_guc *guc);
 int intel_guc_submission_enable(struct intel_guc *guc);
+void intel_guc_submission_stop(struct intel_guc *guc);
 void intel_guc_submission_disable(struct intel_guc *guc);
 void intel_guc_submission_fini(struct intel_guc *guc);
 int intel_guc_preempt_work_create(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 36c53a42927c..51dcdb2764ce 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -502,6 +502,9 @@  void intel_uc_reset_prepare(struct drm_i915_private *i915)
 	if (!intel_guc_is_alive(guc))
 		return;
 
+	if (USES_GUC_SUBMISSION(i915))
+		intel_guc_submission_stop(guc);
+
 	guc_stop_communication(guc);
 	__uc_sanitize(i915);
 }