[v12,08/17] drm/i915/guc/slpc: Send SHUTDOWN event to stop SLPC tasks
diff mbox

Message ID 1522398722-12161-9-git-send-email-sagar.a.kamble@intel.com
State New
Headers show

Commit Message

sagar.a.kamble@intel.com March 30, 2018, 8:31 a.m. UTC
From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Send SLPC shutdown event during uc_fini_hw or prior to enabling SLPC
done while communicating updated parameters in shared data.

v1: Return void instead of ignored error code (Paulo). Removed WARN_ON
    for checking msb of gtt address of shared gem obj. (Chris)
    Added SLPC state update during disable, suspend and reset. Changed
    semantics of reset. It is supposed to just disable. (Sagar)

v2-v4: Rebase.

v5: Updated the input data structure. (Sagar)

v6: Rebase.

v7: s/i915_ggtt_offset/guc_ggtt_offset.

v8: Updated the status check post disabling to wait for 20us. (Sagar)

v9: Updated the status check wait time to 5ms for safe margin as it is
    handled similar to reset by SLPC. s/slpc_disabled/slpc_stopped

v10: Rebase.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_slpc.c | 38 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.c       |  6 +++---
 2 files changed, 41 insertions(+), 3 deletions(-)

Comments

Michal Wajdeczko May 14, 2018, 10:29 a.m. UTC | #1
On Fri, 30 Mar 2018 10:31:53 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
>
> Send SLPC shutdown event during uc_fini_hw or prior to enabling SLPC
> done while communicating updated parameters in shared data.
>
> v1: Return void instead of ignored error code (Paulo). Removed WARN_ON
>     for checking msb of gtt address of shared gem obj. (Chris)
>     Added SLPC state update during disable, suspend and reset. Changed
>     semantics of reset. It is supposed to just disable. (Sagar)
>
> v2-v4: Rebase.
>
> v5: Updated the input data structure. (Sagar)
>
> v6: Rebase.
>
> v7: s/i915_ggtt_offset/guc_ggtt_offset.
>
> v8: Updated the status check post disabling to wait for 20us. (Sagar)
>
> v9: Updated the status check wait time to 5ms for safe margin as it is
>     handled similar to reset by SLPC. s/slpc_disabled/slpc_stopped
>
> v10: Rebase.
>
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_slpc.c | 38  
> +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.c       |  6 +++---
>  2 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c  
> b/drivers/gpu/drm/i915/intel_guc_slpc.c
> index bc2c717..7f75d218 100644
> --- a/drivers/gpu/drm/i915/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_guc_slpc.c
> @@ -368,6 +368,28 @@ static bool slpc_running(struct intel_guc_slpc  
> *slpc)
>  	return (data.global_state == SLPC_GLOBAL_STATE_RUNNING);
>  }
> +static void host2guc_slpc_shutdown(struct intel_guc_slpc *slpc)
> +{
> +	struct intel_guc *guc = slpc_to_guc(slpc);
> +	u32 shared_data_gtt_offset = intel_guc_ggtt_offset(guc, slpc->vma);
> +	struct slpc_event_input data = {0};
> +
> +	data.header.value = SLPC_EVENT(SLPC_EVENT_SHUTDOWN, 2);
> +	data.args[0] = shared_data_gtt_offset;

	data.args[0] = intel_guc_ggtt_offset(guc, slpc->vma);

> +	data.args[1] = 0;

hmm, this '0' means something ? or it is MBZ ?
but since events num_args is flexible, maybe we can drop it ?

> +
> +	slpc_send(slpc, &data, 4);
> +}
> +
> +static bool slpc_stopped(struct intel_guc_slpc *slpc)
> +{
> +	struct slpc_shared_data data;
> +
> +	slpc_read_shared_data(slpc, &data);
> +
> +	return (data.global_state == SLPC_GLOBAL_STATE_NOT_RUNNING);
> +}
> +
>  /**
>   * intel_guc_slpc_init() - Initialize the SLPC shared data structure.
>   * @slpc: pointer to intel_guc_slpc.
> @@ -448,8 +470,24 @@ int intel_guc_slpc_enable(struct intel_guc_slpc  
> *slpc)
>  	return 0;
>  }
> +/**
> + * intel_guc_slpc_disable() - Stop SLPC tasks.
> + * @slpc: pointer to intel_guc_slpc.
> + *
> + * This function will stop GuC SLPC tasks by sending Host to GuC action.
> + */
>  void intel_guc_slpc_disable(struct intel_guc_slpc *slpc)
>  {
> +	mutex_lock(&slpc->lock);
> +
> +	host2guc_slpc_shutdown(slpc);
> +
> +	/* Ensure SLPC is not running */
> +	if (wait_for(slpc_stopped(slpc), 5))
> +		DRM_ERROR("SLPC not disabled! State = %s\n",
> +			  slpc_get_state(slpc));
> +
> +	mutex_unlock(&slpc->lock);
>  }
> /**
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 5bf33c8..ece6687 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -359,6 +359,9 @@ void intel_uc_sanitize(struct drm_i915_private *i915)
> 	GEM_BUG_ON(!HAS_GUC(i915));
> +	if (USES_GUC_SLPC(dev_priv))
> +		intel_guc_slpc_disable(&guc->slpc);
> +
>  	guc_disable_communication(guc);
> 	intel_huc_sanitize(huc);
> @@ -484,9 +487,6 @@ void intel_uc_fini_hw(struct drm_i915_private  
> *dev_priv)
>  	if (USES_GUC_SUBMISSION(dev_priv))
>  		intel_guc_submission_disable(guc);
> -	if (USES_GUC_SLPC(dev_priv))
> -		intel_guc_slpc_disable(&guc->slpc);
> -

Can we avoid such code movement within one series ?

>  	guc_disable_communication(guc);
>  }

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.c b/drivers/gpu/drm/i915/intel_guc_slpc.c
index bc2c717..7f75d218 100644
--- a/drivers/gpu/drm/i915/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/intel_guc_slpc.c
@@ -368,6 +368,28 @@  static bool slpc_running(struct intel_guc_slpc *slpc)
 	return (data.global_state == SLPC_GLOBAL_STATE_RUNNING);
 }
 
+static void host2guc_slpc_shutdown(struct intel_guc_slpc *slpc)
+{
+	struct intel_guc *guc = slpc_to_guc(slpc);
+	u32 shared_data_gtt_offset = intel_guc_ggtt_offset(guc, slpc->vma);
+	struct slpc_event_input data = {0};
+
+	data.header.value = SLPC_EVENT(SLPC_EVENT_SHUTDOWN, 2);
+	data.args[0] = shared_data_gtt_offset;
+	data.args[1] = 0;
+
+	slpc_send(slpc, &data, 4);
+}
+
+static bool slpc_stopped(struct intel_guc_slpc *slpc)
+{
+	struct slpc_shared_data data;
+
+	slpc_read_shared_data(slpc, &data);
+
+	return (data.global_state == SLPC_GLOBAL_STATE_NOT_RUNNING);
+}
+
 /**
  * intel_guc_slpc_init() - Initialize the SLPC shared data structure.
  * @slpc: pointer to intel_guc_slpc.
@@ -448,8 +470,24 @@  int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
 	return 0;
 }
 
+/**
+ * intel_guc_slpc_disable() - Stop SLPC tasks.
+ * @slpc: pointer to intel_guc_slpc.
+ *
+ * This function will stop GuC SLPC tasks by sending Host to GuC action.
+ */
 void intel_guc_slpc_disable(struct intel_guc_slpc *slpc)
 {
+	mutex_lock(&slpc->lock);
+
+	host2guc_slpc_shutdown(slpc);
+
+	/* Ensure SLPC is not running */
+	if (wait_for(slpc_stopped(slpc), 5))
+		DRM_ERROR("SLPC not disabled! State = %s\n",
+			  slpc_get_state(slpc));
+
+	mutex_unlock(&slpc->lock);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 5bf33c8..ece6687 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -359,6 +359,9 @@  void intel_uc_sanitize(struct drm_i915_private *i915)
 
 	GEM_BUG_ON(!HAS_GUC(i915));
 
+	if (USES_GUC_SLPC(dev_priv))
+		intel_guc_slpc_disable(&guc->slpc);
+
 	guc_disable_communication(guc);
 
 	intel_huc_sanitize(huc);
@@ -484,9 +487,6 @@  void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	if (USES_GUC_SUBMISSION(dev_priv))
 		intel_guc_submission_disable(guc);
 
-	if (USES_GUC_SLPC(dev_priv))
-		intel_guc_slpc_disable(&guc->slpc);
-
 	guc_disable_communication(guc);
 }