Re: [PATCH 14/15] drm/i915: vlv: add runtime PM support

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Apr 08, 2014 at 07:57:55PM +0300, Imre Deak wrote:
> Add runtime PM support for VLV, but leave it disabled. The next patch
> enables it.
> 
> The suspend/resume sequence used is based on [1] and [2]. In practice we
> depend on the GT RC6 mechanism to save the HW context depending on the
> render and media power wells. By the time we run the runtime suspend
> callback the display side is also off and the HW context for that is
> managed by the display power domain framework.
> 
> Besides the above there are Gunit registers that depend on a system-wide
> power well. This power well goes off once the device enters any of the
> S0i[R123] states. To handle this scenario, save/restore these Gunit
> registers. Note that this is not the complete register set dictated by
> [2], to remove some overhead registers that are known not to be used are
> ignored. Also some registers are fully setup by initialization functions
> called during resume, these are not saved either. The list of registers
> can be further reduced, see the TODO note in the code.
> 
> [1] VLV_gfx_clocking_PM_reset_y12w21d3 / "Driver D3 entry/exit"
> [2] VLV2_S0IXRegs
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 170 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 166 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 9f9d0db..16ca37f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -891,19 +891,23 @@ static int i915_pm_poweroff(struct device *dev)
>  	return i915_drm_freeze(drm_dev);
>  }
>  
> -static void snb_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int snb_runtime_suspend(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
>  	intel_runtime_pm_disable_interrupts(dev);
> +
> +	return 0;
>  }
>  
> -static void hsw_runtime_suspend(struct drm_i915_private *dev_priv)
> +static int hsw_runtime_suspend(struct drm_i915_private *dev_priv)
>  {
>  	hsw_enable_pc8(dev_priv);
> +
> +	return 0;
>  }
>  
> -static void snb_runtime_resume(struct drm_i915_private *dev_priv)
> +static int snb_runtime_resume(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
>  
> @@ -913,11 +917,15 @@ static void snb_runtime_resume(struct drm_i915_private *dev_priv)
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	gen6_update_ring_freq(dev);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
> +
> +	return 0;
>  }
>  
> -static void hsw_runtime_resume(struct drm_i915_private *dev_priv)
> +static int hsw_runtime_resume(struct drm_i915_private *dev_priv)
>  {
>  	hsw_disable_pc8(dev_priv);
> +
> +	return 0;
>  }
>  
>  /*
> @@ -1144,11 +1152,155 @@ int vlv_force_gfx_clock(struct drm_i915_private *dev_priv, bool force_on)
>  #undef COND
>  }
>  
> +static int vlv_allow_gt_wake(struct drm_i915_private *dev_priv, bool allow)
> +{
> +	u32 val;
> +	int err = 0;
> +
> +	val = I915_READ(VLV_GTLC_WAKE_CTRL);
> +	val &= ~VLV_GTLC_ALLOWWAKEREQ;
> +	if (allow)
> +		val |= VLV_GTLC_ALLOWWAKEREQ;
> +	I915_WRITE(VLV_GTLC_WAKE_CTRL, val);
> +	POSTING_READ(VLV_GTLC_WAKE_CTRL);
> +
> +#define COND (!!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEACK) == \
> +	      allow)
> +	err = wait_for(COND, 1);
> +	if (err)
> +		DRM_ERROR("timeout disabling GT waking\n");
> +	return err;
> +#undef COND
> +}
> +
> +static int vlv_wait_for_gt_wells(struct drm_i915_private *dev_priv,
> +				 bool wait_for_on)
> +{
> +	u32 mask;
> +	u32 val;
> +	int err;
> +
> +	mask = VLV_GTLC_PW_MEDIA_STATUS_MASK | VLV_GTLC_PW_RENDER_STATUS_MASK;
> +	val = wait_for_on ? mask : 0;
> +#define COND ((I915_READ(VLV_GTLC_PW_STATUS) & mask) == val)
> +	if (COND)
> +		return 0;
> +
> +	DRM_DEBUG_KMS("waiting for GT wells to go %s (%08x)\n",
> +			wait_for_on ? "on" : "off",
> +			I915_READ(VLV_GTLC_PW_STATUS));
> +
> +	/*
> +	 * RC6 transitioning can be delayed up to 2 msec (see
> +	 * valleyview_enable_rps), use 3 msec for safety.
> +	 */
> +	err = wait_for(COND, 3);
> +	if (err)
> +		DRM_ERROR("timeout waiting for GT wells to go %s\n",
> +			  wait_for_on ? "on" : "off");
> +
> +	return err;
> +#undef COND
> +}
> +
> +static void vlv_check_no_gt_access(struct drm_i915_private *dev_priv)
> +{
> +	if (!(I915_READ(VLV_GTLC_PW_STATUS) & VLV_GTLC_ALLOWWAKEERR))
> +		return;
> +
> +	DRM_ERROR("GT register access while GT waking disabled\n");
> +	I915_WRITE(VLV_GTLC_PW_STATUS, VLV_GTLC_ALLOWWAKEERR);
> +}
> +
> +static int vlv_runtime_suspend(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	u32 mask;
> +	int err;
> +
> +	if (WARN_ON(!valleyview_rc6_enabled(dev)))
> +		return -ENODEV;
> +
> +	intel_runtime_pm_disable_interrupts(dev);
> +	cancel_work_sync(&dev_priv->rps.work);
> +
> +	/*
> +	 * Bspec defines the following GT well on flags as debug only, so
> +	 * don't treat them as hard failures.
> +	 */
> +	(void)vlv_wait_for_gt_wells(dev_priv, false);
> +
> +	mask = VLV_GTLC_RENDER_CTX_EXISTS | VLV_GTLC_MEDIA_CTX_EXISTS;
> +	WARN_ON((I915_READ(VLV_GTLC_WAKE_CTRL) & mask) != mask);
> +
> +	vlv_check_no_gt_access(dev_priv);
> +
> +	err = vlv_force_gfx_clock(dev_priv, true);
> +	if (err)
> +		goto err1;
> +
> +	err = vlv_allow_gt_wake(dev_priv, false);
> +	if (err)
> +		goto err2;
> +	vlv_save_gunit_s0ix_state(dev_priv);
> +
> +	err = vlv_force_gfx_clock(dev_priv, false);
> +	if (err)
> +		goto err2;
> +
> +	return 0;
> +
> +err2:
> +	/* For safety always re-enable waking and disable gfx clock forcing */
> +	vlv_allow_gt_wake(dev_priv, true);
> +err1:
> +	vlv_force_gfx_clock(dev_priv, false);
> +	intel_runtime_pm_restore_interrupts(dev);
> +
> +	return err;
> +}
> +
> +static int vlv_runtime_resume(struct drm_i915_private *dev_priv)
> +{
> +	struct drm_device *dev = dev_priv->dev;
> +	int err;
> +	int ret;
> +
> +	/*
> +	 * If any of the steps fail just try to continue, that's the best we
> +	 * can do at this point. Return the first error code (which will also
> +	 * leave RPM permanentyl disabled).
> +	 */
> +	ret = vlv_force_gfx_clock(dev_priv, true);
> +
> +	vlv_restore_gunit_s0ix_state(dev_priv);
> +
> +	err = vlv_allow_gt_wake(dev_priv, true);
> +	if (!ret)
> +		ret = err;
> +
> +	err = vlv_force_gfx_clock(dev_priv, false);
> +	if (!ret)
> +		ret = err;
> +
> +	vlv_check_no_gt_access(dev_priv);
> +
> +	intel_init_clock_gating(dev);
> +	intel_reset_gt_powersave(dev);
> +	i915_gem_init_swizzling(dev);
> +	i915_gem_restore_fences(dev);
> +
> +	intel_runtime_pm_restore_interrupts(dev);
> +
> +	return ret;
> +}
> +
>  static int intel_runtime_suspend(struct device *device)
>  {
>  	struct pci_dev *pdev = to_pci_dev(device);
>  	struct drm_device *dev = pci_get_drvdata(pdev);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	int ret = 0;
>  
>  	if (WARN_ON_ONCE(!dev_priv->rps.enabled))
>  		return -ENODEV;
> @@ -1162,9 +1314,17 @@ static int intel_runtime_suspend(struct device *device)
>  		snb_runtime_suspend(dev_priv);
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		hsw_runtime_suspend(dev_priv);
> +	else if (IS_VALLEYVIEW(dev))
> +		ret = vlv_runtime_suspend(dev_priv);
>  	else
>  		WARN_ON(1);
>  
> +	if (ret) {
> +		DRM_ERROR("Runtime suspend failed, disabling it\n");
> +
> +		return ret;
> +	}
> +
>  	i915_gem_release_all_mmaps(dev_priv);
>  
>  	del_timer_sync(&dev_priv->gpu_error.hangcheck_timer);
> @@ -1200,6 +1360,8 @@ static int intel_runtime_resume(struct device *device)
>  		snb_runtime_resume(dev_priv);
>  	else if (IS_HASWELL(dev) || IS_BROADWELL(dev))
>  		hsw_runtime_resume(dev_priv);
> +	else if (IS_VALLEYVIEW(dev))
> +		vlv_runtime_resume(dev_priv);

Golden rule of refactoring: The 3rd guy gets to cleanup the mess. Imo
it's time to refactor the common parts form these platform functions out
and move them into generic code, and only call down into platform code as
needed. If we don't do that we'll have completely hell due to slight
differences in ordering between platforms.

Also I think we should try to share as much code as possible with the
other setup/teardowns paths, i.e. driver load/unload, system
suspend/resume and gpu reset.

Thanks, Daniel

>  	else
>  		WARN_ON(1);
>  
> -- 
> 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