diff mbox series

[CI,1/2] drm/i915/uc: Extract common code from GuC stop/disable comm

Message ID 20190829174154.14675-1-fernando.pacheco@intel.com (mailing list archive)
State New, archived
Headers show
Series [CI,1/2] drm/i915/uc: Extract common code from GuC stop/disable comm | expand

Commit Message

Fernando Pacheco Aug. 29, 2019, 5:41 p.m. UTC 
During normal driver unload we attempt to disable GuC communication
while it is currently stopped. This results in a nop'd call to
intel_guc_ct_disable within guc_disable_communication because
stop/disable rely on the same flag to prevent further comms with CT.

We can avoid the call to disable and still leave communication in a
satisfactory state by extracting a set of shared steps from stop/disable.
This set can include guc_disable_interrupts as we do not require the
single caller of guc_stop_communication to be atomic:
"drm/i915/selftests: Fixup atomic reset checking".

This situation (stop -> disable) only occurs during intel_uc_fini_hw,
so during fini, call guc_disable_communication only if currently enabled.
The symmetric calls to enable/disable remain unmodified for all other
scenarios.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110943
Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 30 ++++++++++++++++-----------
 1 file changed, 18 insertions(+), 12 deletions(-)

Comments

Chris Wilson Aug. 29, 2019, 6:49 p.m. UTC | #1 
Quoting Fernando Pacheco (2019-08-29 18:41:53)
> During normal driver unload we attempt to disable GuC communication
> while it is currently stopped. This results in a nop'd call to
> intel_guc_ct_disable within guc_disable_communication because
> stop/disable rely on the same flag to prevent further comms with CT.
> 
> We can avoid the call to disable and still leave communication in a
> satisfactory state by extracting a set of shared steps from stop/disable.
> This set can include guc_disable_interrupts as we do not require the
> single caller of guc_stop_communication to be atomic:
> "drm/i915/selftests: Fixup atomic reset checking".
> 
> This situation (stop -> disable) only occurs during intel_uc_fini_hw,
> so during fini, call guc_disable_communication only if currently enabled.
> The symmetric calls to enable/disable remain unmodified for all other
> scenarios.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110943
> Signed-off-by: Fernando Pacheco <fernando.pacheco@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

Thanks for the bugfix and review, pushed.
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 71ee7ab035cc..29a9eec60d2e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -224,17 +224,7 @@  static int guc_enable_communication(struct intel_guc *guc)
 	return 0;
 }
 
-static void guc_stop_communication(struct intel_guc *guc)
-{
-	intel_guc_ct_stop(&guc->ct);
-
-	guc->send = intel_guc_send_nop;
-	guc->handler = intel_guc_to_host_event_handler_nop;
-
-	guc_clear_mmio_msg(guc);
-}
-
-static void guc_disable_communication(struct intel_guc *guc)
+static void __guc_stop_communication(struct intel_guc *guc)
 {
 	/*
 	 * Events generated during or after CT disable are logged by guc in
@@ -247,6 +237,20 @@  static void guc_disable_communication(struct intel_guc *guc)
 
 	guc->send = intel_guc_send_nop;
 	guc->handler = intel_guc_to_host_event_handler_nop;
+}
+
+static void guc_stop_communication(struct intel_guc *guc)
+{
+	intel_guc_ct_stop(&guc->ct);
+
+	__guc_stop_communication(guc);
+
+	DRM_INFO("GuC communication stopped\n");
+}
+
+static void guc_disable_communication(struct intel_guc *guc)
+{
+	__guc_stop_communication(guc);
 
 	intel_guc_ct_disable(&guc->ct);
 
@@ -537,7 +541,9 @@  void intel_uc_fini_hw(struct intel_uc *uc)
 	if (intel_uc_supports_guc_submission(uc))
 		intel_guc_submission_disable(guc);
 
-	guc_disable_communication(guc);
+	if (guc_communication_enabled(guc))
+		guc_disable_communication(guc);
+
 	__uc_sanitize(uc);
 }