Re: [PATCH] drm/i915: Unifiy GT powersave suspend logic

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 12 Jun 2014 17:53:45 +0200
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> Jesse's patch to only quiescent our rps work and Imre's fix to address
> a race with runtime pm and the forcewake reference held by the used
> diverging means to address the same bug: Jesse's patch uses
> flush_delayed_work while (since we want to make sure rps is set up)
> while Imre's used a cancel+manuel refcount adjustment.
> 
> Unify them again by simply reusing intel_suspend_gt_powersave in
> intel_disable_gt_powersave.
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 685b4910eb93..49122204a001 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4706,10 +4706,8 @@ void intel_disable_gt_powersave(struct drm_device *dev)
>  		ironlake_disable_drps(dev);
>  		ironlake_disable_rc6(dev);
>  	} else if (IS_GEN6(dev) || IS_GEN7(dev) || IS_BROADWELL(dev)) {
> -		if (cancel_delayed_work_sync(&dev_priv->rps.delayed_resume_work))
> -			intel_runtime_pm_put(dev_priv);
> +		intel_suspend_gt_powersave(dev);
>  
> -		cancel_work_sync(&dev_priv->rps.work);
>  		mutex_lock(&dev_priv->rps.hw_lock);
>  		if (IS_VALLEYVIEW(dev))
>  			valleyview_disable_rps(dev);

Yeah looks good, though we may end up toggling RC6 on rather than
canceling an oustanding enable, but that doesn't matter much on the
disable path.

We can do some more unification of the various freeze/suspend/resume
paths too, though we'll want lots of testing...

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center