diff mbox

[12/21] drm/i915/slpc: Send shutdown event

Message ID 1461805865-212590-13-git-send-email-tom.orourke@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

tom.orourke@intel.com April 28, 2016, 1:10 a.m. UTC 
From: Tom O'Rourke <Tom.O'Rourke@intel.com>

Send SLPC shutdown event during disable, suspend, and reset
operations.  Sending shutdown event while already shutdown
is OK.

v2: return void instead of ignored error code (Paulo)

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
---
 drivers/gpu/drm/i915/intel_slpc.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Comments

Chris Wilson April 28, 2016, 7:07 a.m. UTC | #1 
On Wed, Apr 27, 2016 at 06:10:56PM -0700, tom.orourke@intel.com wrote:
> From: Tom O'Rourke <Tom.O'Rourke@intel.com>
> 
> Send SLPC shutdown event during disable, suspend, and reset
> operations.  Sending shutdown event while already shutdown
> is OK.
> 
> v2: return void instead of ignored error code (Paulo)
> 
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_slpc.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
> index 3fd46ac..076d07b 100644
> --- a/drivers/gpu/drm/i915/intel_slpc.c
> +++ b/drivers/gpu/drm/i915/intel_slpc.c
> @@ -56,6 +56,23 @@ static void host2guc_slpc_reset(struct drm_device *dev)
>  	host2guc_slpc(dev_priv, data, 4);
>  }
>  
> +static void host2guc_slpc_shutdown(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_i915_gem_object *obj = dev_priv->guc.slpc.shared_data_obj;
> +	u32 data[4];
> +	u64 shared_data_gtt_offset = i915_gem_obj_ggtt_offset(obj);
> +
> +	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
> +	data[1] = SLPC_EVENT(SLPC_EVENT_SHUTDOWN, 2);
> +	data[2] = lower_32_bits(shared_data_gtt_offset);
> +	data[3] = upper_32_bits(shared_data_gtt_offset);
> +
> +	WARN_ON(0 != data[3]);

Why WARN here? Why not WARN during setup and clamp the GGTT to what the
GuC supports? Shutting the stable door after the horse has bolted...

What happens to the guc if this WARN fires? Is it even sensible to be
programming the hardware to a value you know is bogus? These are the
questions you raise by having an unhandled WARN.
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_slpc.c b/drivers/gpu/drm/i915/intel_slpc.c
index 3fd46ac..076d07b 100644
--- a/drivers/gpu/drm/i915/intel_slpc.c
+++ b/drivers/gpu/drm/i915/intel_slpc.c
@@ -56,6 +56,23 @@  static void host2guc_slpc_reset(struct drm_device *dev)
 	host2guc_slpc(dev_priv, data, 4);
 }
 
+static void host2guc_slpc_shutdown(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_gem_object *obj = dev_priv->guc.slpc.shared_data_obj;
+	u32 data[4];
+	u64 shared_data_gtt_offset = i915_gem_obj_ggtt_offset(obj);
+
+	data[0] = HOST2GUC_ACTION_SLPC_REQUEST;
+	data[1] = SLPC_EVENT(SLPC_EVENT_SHUTDOWN, 2);
+	data[2] = lower_32_bits(shared_data_gtt_offset);
+	data[3] = upper_32_bits(shared_data_gtt_offset);
+
+	WARN_ON(0 != data[3]);
+
+	host2guc_slpc(dev_priv, data, 4);
+}
+
 static u8 slpc_get_platform_sku(struct drm_i915_gem_object *obj)
 {
 	struct drm_device *dev = obj->base.dev;
@@ -152,12 +169,14 @@  void intel_slpc_cleanup(struct drm_device *dev)
 
 void intel_slpc_suspend(struct drm_device *dev)
 {
-	return;
+	if (intel_slpc_active(dev))
+		host2guc_slpc_shutdown(dev);
 }
 
 void intel_slpc_disable(struct drm_device *dev)
 {
-	return;
+	if (intel_slpc_active(dev))
+		host2guc_slpc_shutdown(dev);
 }
 
 void intel_slpc_enable(struct drm_device *dev)
@@ -168,5 +187,8 @@  void intel_slpc_enable(struct drm_device *dev)
 
 void intel_slpc_reset(struct drm_device *dev)
 {
-	return;
+	if (intel_slpc_active(dev)) {
+		host2guc_slpc_shutdown(dev);
+		host2guc_slpc_reset(dev);
+	}
 }