[Intel-gfx] [PATCH 1/2] drm/i915/guc: reorder enable/disable communication steps

[Michal Wajdeczko]

On Wed, 19 Jun 2019 23:43:50 +0200, Daniele Ceraolo Spurio  
<daniele.ceraolospurio at intel.com> 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 at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>

Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>

with some nit below

> ---
>  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;

now you can drop above early return and always 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)