Re: [PATCH 2/4] drm/i915: leave rc6 enabled at suspend time

[Imre Deak <imre.deak@intel.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3897 bytes --]

On Thu, 2014-05-29 at 14:11 -0700, Jesse Barnes wrote:
> From: Kristen Carlson Accardi <kristen@linux.intel.com>
> 
> This allows the system to enter the lowest power mode during system freeze.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  3 ---
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_pm.c  | 16 +++++++++++-----
>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 66c6ffb..433bdfa 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -521,8 +521,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>  		drm_irq_uninstall(dev);
>  		dev_priv->enable_hotplug_processing = false;
>  
> -		intel_disable_gt_powersave(dev);
> -

I wonder what was the reason for this call. One possibility is that
i915_save_state() depends on it to save the correct registers, but it
would be good to clarify this.

It also cancels some deferred works which we do need here. But we could
also add that to intel_enable_gt_powersave_sync() in this patch.

>  		/*
>  		 * Disable CRTCs directly since we want to preserve sw state
>  		 * for _thaw.
> @@ -543,7 +541,6 @@ static int i915_drm_freeze(struct drm_device *dev)
>  	i915_save_state(dev);
>  
>  	intel_opregion_fini(dev);
> -	intel_uncore_fini(dev);

Some stuff in the above call is unrelated to this patch, like
intel_uncore_forcewake_reset(). At least canceling force_wake_timer
there seems to be needed here. In any case it would be better to have
the above two changes in a separate patch.

With that fixed this patch looks ok to me. The original patch was from
me, so fwiw:
Reviewed-by: Imre Deak <imre.deak@intel.com>
 
>  	console_lock();
>  	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c597b0d..bf90e7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -956,6 +956,7 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv);
>  void intel_init_gt_powersave(struct drm_device *dev);
>  void intel_cleanup_gt_powersave(struct drm_device *dev);
>  void intel_enable_gt_powersave(struct drm_device *dev);
> +void intel_enable_gt_powersave_sync(struct drm_device *dev);
>  void intel_disable_gt_powersave(struct drm_device *dev);
>  void intel_reset_gt_powersave(struct drm_device *dev);
>  void ironlake_teardown_rc6(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 1840d15..8d9e036 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4891,12 +4891,9 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  	}
>  }
>  
> -static void intel_gen6_powersave_work(struct work_struct *work)
> +void intel_enable_gt_powersave_sync(struct drm_device *dev)
>  {
> -	struct drm_i915_private *dev_priv =
> -		container_of(work, struct drm_i915_private,
> -			     rps.delayed_resume_work.work);
> -	struct drm_device *dev = dev_priv->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  
> @@ -4917,6 +4914,15 @@ static void intel_gen6_powersave_work(struct work_struct *work)
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> +static void intel_gen6_powersave_work(struct work_struct *work)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(work, struct drm_i915_private,
> +			     rps.delayed_resume_work.work);
> +
> +	intel_enable_gt_powersave_sync(dev_priv->dev);
> +}
> +
>  void intel_enable_gt_powersave(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx