Re: [PATCH v2] drm/i915/guc: updated suspend/resume protocol

[John Spotswood <john.a.spotswood@intel.com>]

On Fri, 2019-04-12 at 17:20 -0700, Ceraolo Spurio, Daniele wrote:
> From: Michal Wajdeczko <michal.wajdeczko@intel.com>
> 
> New GuC firmwares use updated sleep status definitions.
> The polling on scratch register 14 is also now required only on
> suspend
> and there is no need to provide the shared page.
> 
> v2: include changes for polling and shared page
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.co
> m>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>

Reviewed-by: John Spotswood <john.a.spotswood@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_guc.c      | 50 +++++++++++------------
> ----
>  drivers/gpu/drm/i915/intel_guc_fwif.h |  6 ++--
>  2 files changed, 24 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c
> b/drivers/gpu/drm/i915/intel_guc.c
> index 483c7019f817..cf943eb7537c 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -539,25 +539,33 @@ int intel_guc_auth_huc(struct intel_guc *guc,
> u32 rsa_offset)
>  	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
> -/*
> - * The ENTER/EXIT_S_STATE actions queue the save/restore operation
> in GuC FW and
> - * then return, so waiting on the H2G is not enough to guarantee GuC
> is done.
> - * When all the processing is done, GuC writes
> INTEL_GUC_SLEEP_STATE_SUCCESS to
> - * scratch register 14, so we can poll on that. Note that GuC does
> not ensure
> - * that the value in the register is different from
> - * INTEL_GUC_SLEEP_STATE_SUCCESS while the action is in progress so
> we need to
> - * take care of that ourselves as well.
> +/**
> + * intel_guc_suspend() - notify GuC entering suspend state
> + * @guc:	the guc
>   */
> -static int guc_sleep_state_action(struct intel_guc *guc,
> -				  const u32 *action, u32 len)
> +int intel_guc_suspend(struct intel_guc *guc)
>  {
>  	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>  	int ret;
>  	u32 status;
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_ENTER_S_STATE,
> +		GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> */
> +	};
> +
> +	/*
> +	 * The ENTER_S_STATE action queues the save/restore
> operation in GuC FW
> +	 * and then returns, so waiting on the H2G is not enough to
> guarantee
> +	 * GuC is done. When all the processing is done, GuC writes
> +	 * INTEL_GUC_SLEEP_STATE_SUCCESS to scratch register 14, so
> we can poll
> +	 * on that. Note that GuC does not ensure that the value in
> the register
> +	 * is different from INTEL_GUC_SLEEP_STATE_SUCCESS while the
> action is
> +	 * in progress so we need to take care of that ourselves as
> well.
> +	 */
>  
>  	I915_WRITE(SOFT_SCRATCH(14),
> INTEL_GUC_SLEEP_STATE_INVALID_MASK);
>  
> -	ret = intel_guc_send(guc, action, len);
> +	ret = intel_guc_send(guc, action, ARRAY_SIZE(action));
>  	if (ret)
>  		return ret;
>  
> @@ -577,21 +585,6 @@ static int guc_sleep_state_action(struct
> intel_guc *guc,
>  	return 0;
>  }
>  
> -/**
> - * intel_guc_suspend() - notify GuC entering suspend state
> - * @guc:	the guc
> - */
> -int intel_guc_suspend(struct intel_guc *guc)
> -{
> -	u32 data[] = {
> -		INTEL_GUC_ACTION_ENTER_S_STATE,
> -		GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> */
> -		intel_guc_ggtt_offset(guc, guc->shared_data)
> -	};
> -
> -	return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
> -}
> -
>  /**
>   * intel_guc_reset_engine() - ask GuC to reset an engine
>   * @guc:	intel_guc structure
> @@ -621,13 +614,12 @@ int intel_guc_reset_engine(struct intel_guc
> *guc,
>   */
>  int intel_guc_resume(struct intel_guc *guc)
>  {
> -	u32 data[] = {
> +	u32 action[] = {
>  		INTEL_GUC_ACTION_EXIT_S_STATE,
>  		GUC_POWER_D0,
> -		intel_guc_ggtt_offset(guc, guc->shared_data)
>  	};
>  
> -	return guc_sleep_state_action(guc, data, ARRAY_SIZE(data));
> +	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 64b56da9775c..25d57c819e3f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -648,9 +648,9 @@ enum intel_guc_report_status {
>  };
>  
>  enum intel_guc_sleep_state_status {
> -	INTEL_GUC_SLEEP_STATE_SUCCESS = 0x0,
> -	INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x1,
> -	INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x2
> +	INTEL_GUC_SLEEP_STATE_SUCCESS = 0x1,
> +	INTEL_GUC_SLEEP_STATE_PREEMPT_TO_IDLE_FAILED = 0x2,
> +	INTEL_GUC_SLEEP_STATE_ENGINE_RESET_FAILED = 0x3
>  #define INTEL_GUC_SLEEP_STATE_INVALID_MASK 0x80000000
>  };
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx