Re: [PATCH v2 08/25] drm/i915: get a runtime PM ref for the deferred GT powersave enabling

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Apr 14, 2014 at 08:24:29PM +0300, Imre Deak wrote:
> At least on VLV but probably on other platforms too we depend on RC6
> being enabled for RPM, so disable RPM until the delayed RC6 enabling
> completes.
> 
> v2:
> - explain the reason for the _noresume version of RPM get (Daniel)
> - use the simpler 'if (schedule_work()) rpm_get();' instead of
>   'if (!cancel_work_sync()) rpm_get(); schedule_work();'
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Merged up to this patch, thanks.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.c  |  5 ++++-
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  drivers/gpu/drm/i915/intel_pm.c  | 34 ++++++++++++++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a821608..a20d2d1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -754,7 +754,7 @@ int i915_reset(struct drm_device *dev)
>  		 * previous concerns that it doesn't respond well to some forms
>  		 * of re-init after reset. */
>  		if (INTEL_INFO(dev)->gen > 5)
> -			intel_enable_gt_powersave(dev);
> +			intel_reset_gt_powersave(dev);
>  
>  		intel_hpd_init(dev);
>  	} else {
> @@ -923,6 +923,9 @@ static int intel_runtime_suspend(struct device *device)
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> +	if (WARN_ON_ONCE(!dev_priv->rps.enabled))
> +		return -ENODEV;
> +
>  	WARN_ON(!HAS_RUNTIME_PM(dev));
>  	assert_force_wake_inactive(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c551472..618d05a 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -924,6 +924,7 @@ 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_disable_gt_powersave(struct drm_device *dev);
> +void intel_reset_gt_powersave(struct drm_device *dev);
>  void ironlake_teardown_rc6(struct drm_device *dev);
>  void gen6_update_ring_freq(struct drm_device *dev);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
> @@ -931,6 +932,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>  void intel_init_runtime_pm(struct drm_i915_private *dev_priv);
>  void intel_fini_runtime_pm(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e5b9f08..0e8b263 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -4549,6 +4549,8 @@ static void intel_gen6_powersave_work(struct work_struct *work)
>  	}
>  	dev_priv->rps.enabled = true;
>  	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	intel_runtime_pm_put(dev_priv);
>  }
>  
>  void intel_enable_gt_powersave(struct drm_device *dev)
> @@ -4566,12 +4568,28 @@ void intel_enable_gt_powersave(struct drm_device *dev)
>  		 * PCU communication is slow and this doesn't need to be
>  		 * done at any specific time, so do this out of our fast path
>  		 * to make resume and init faster.
> +		 *
> +		 * We depend on the HW RC6 power context save/restore
> +		 * mechanism when entering D3 through runtime PM suspend. So
> +		 * disable RPM until RPS/RC6 is properly setup. We can only
> +		 * get here via the driver load/system resume/runtime resume
> +		 * paths, so the _noresume version is enough (and in case of
> +		 * runtime resume it's necessary).
>  		 */
> -		schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
> -				      round_jiffies_up_relative(HZ));
> +		if (schedule_delayed_work(&dev_priv->rps.delayed_resume_work,
> +					   round_jiffies_up_relative(HZ)))
> +			intel_runtime_pm_get_noresume(dev_priv);
>  	}
>  }
>  
> +void intel_reset_gt_powersave(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->rps.enabled = false;
> +	intel_enable_gt_powersave(dev);
> +}
> +
>  static void ibx_init_clock_gating(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6025,6 +6043,18 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
>  	WARN(dev_priv->pm.suspended, "Device still suspended.\n");
>  }
>  
> +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	struct device *device = &dev->pdev->dev;
> +
> +	if (!HAS_RUNTIME_PM(dev))
> +		return;
> +
> +	WARN(dev_priv->pm.suspended, "Getting nosync-ref while suspended.\n");
> +	pm_runtime_get_noresume(device);
> +}
> +
>  void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> -- 
> 1.8.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch