Re: [PATCH 1/2] drm/i915/guc: reorder enable/disable communication steps

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Jun 19, 2019 at 02:43:50PM -0700, Daniele Ceraolo Spurio wrote:
>Make sure we always have CT buffers enabled when the interrupts are
>enabled, so we can always handle interrupts from GuC. Also move the
>setting of the guc->send and guc->handler functions to the GuC
>communication control functions for consistency.
>
>The reorder also fixes the onion unwinding of intel_uc_init_hw, because
>guc_enable_communication would've left interrupts enabled when failing
>to enable CTB.
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110943
>Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>Cc: Chris Wilson <chris@chris-wilson.co.uk>
>Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>---

Reviewed-by: Matthew Brost

> drivers/gpu/drm/i915/intel_guc_ct.c | 15 +++------------
> drivers/gpu/drm/i915/intel_guc_ct.h |  4 ++++
> drivers/gpu/drm/i915/intel_uc.c     | 19 ++++++++++++++++---
> 3 files changed, 23 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
>index 3921809f812b..92eb40aadd9b 100644
>--- a/drivers/gpu/drm/i915/intel_guc_ct.c
>+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>@@ -529,8 +529,8 @@ static int ctch_send(struct intel_guc_ct *ct,
> /*
>  * Command Transport (CT) buffer based GuC send function.
>  */
>-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>-			     u32 *response_buf, u32 response_buf_size)
>+int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>+		      u32 *response_buf, u32 response_buf_size)
> {
> 	struct intel_guc_ct *ct = &guc->ct;
> 	struct intel_guc_ct_channel *ctch = &ct->host_channel;
>@@ -834,7 +834,7 @@ static void ct_process_host_channel(struct intel_guc_ct *ct)
>  * When we're communicating with the GuC over CT, GuC uses events
>  * to notify us about new messages being posted on the RECV buffer.
>  */
>-static void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
>+void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
> {
> 	struct intel_guc_ct *ct = &guc->ct;
>
>@@ -901,10 +901,6 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
> 	if (unlikely(err))
> 		return err;
>
>-	/* Switch into cmd transport buffer based send() */
>-	guc->send = intel_guc_send_ct;
>-	guc->handler = intel_guc_to_host_event_handler_ct;
>-	DRM_INFO("CT: %s\n", enableddisabled(true));
> 	return 0;
> }
>
>@@ -921,9 +917,4 @@ void intel_guc_ct_disable(struct intel_guc_ct *ct)
> 		return;
>
> 	ctch_disable(guc, ctch);
>-
>-	/* Disable send */
>-	guc->send = intel_guc_send_nop;
>-	guc->handler = intel_guc_to_host_event_handler_nop;
>-	DRM_INFO("CT: %s\n", enableddisabled(false));
> }
>diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
>index 41ba593a4df7..0ec17493d83b 100644
>--- a/drivers/gpu/drm/i915/intel_guc_ct.h
>+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
>@@ -101,4 +101,8 @@ static inline void intel_guc_ct_stop(struct intel_guc_ct *ct)
> 	ct->host_channel.enabled = false;
> }
>
>+int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
>+		      u32 *response_buf, u32 response_buf_size);
>+void intel_guc_to_host_event_handler_ct(struct intel_guc *guc);
>+
> #endif /* _INTEL_GUC_CT_H_ */
>diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
>index ae45651ac73c..c7f82c944dd6 100644
>--- a/drivers/gpu/drm/i915/intel_uc.c
>+++ b/drivers/gpu/drm/i915/intel_uc.c
>@@ -235,9 +235,20 @@ static void guc_disable_interrupts(struct intel_guc *guc)
>
> static int guc_enable_communication(struct intel_guc *guc)
> {
>+	int ret;
>+
>+	ret = intel_guc_ct_enable(&guc->ct);
>+	if (ret)
>+		return ret;
>+
>+	guc->send = intel_guc_send_ct;
>+	guc->handler = intel_guc_to_host_event_handler_ct;
>+
> 	guc_enable_interrupts(guc);
>
>-	return intel_guc_ct_enable(&guc->ct);
>+	DRM_INFO("GuC communication enabled\n");
>+
>+	return 0;
> }
>
> static void guc_stop_communication(struct intel_guc *guc)
>@@ -250,12 +261,14 @@ static void guc_stop_communication(struct intel_guc *guc)
>
> static void guc_disable_communication(struct intel_guc *guc)
> {
>-	intel_guc_ct_disable(&guc->ct);
>-
> 	guc_disable_interrupts(guc);
>
> 	guc->send = intel_guc_send_nop;
> 	guc->handler = intel_guc_to_host_event_handler_nop;
>+
>+	intel_guc_ct_disable(&guc->ct);
>+
>+	DRM_INFO("GuC communication disabled\n");
> }
>
> int intel_uc_init_misc(struct drm_i915_private *i915)
>-- 
>2.20.1
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx