Re: [PATCH] RFC: drm/i915: Take runtime pm wakelock during hangcheck

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ke, 2015-12-09 at 12:22 +0000, Chris Wilson wrote:
> During hangcheck we access the hardware registers, for which we must
> hold a runtime pm reference. Hangcheck also should only be running
> whilst the GPU is active, and we hold a runtime pm whilst the GPU is
> busy. Therefore, if the runtime pm is suspended (no wakelocks held
> anywhere) we know the GPU is already idle and we can skip the
> hangcheck
> (and all further hangchecks until the next request is submitted to
> the
> GPU, waking it up).
> 
> Currently, hangcheck relies upon being flushed during
> intel_runtime_suspend() but is being done so too late causing invalid
> hardware access whilst the device is being suspend. By taking an
> explicit wakelock (albeit only if already awake) inside hangcheck we
> can
> remove the synchronous cancellation from the suspend function.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93121
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |  1 -
>  drivers/gpu/drm/i915/i915_irq.c         |  9 +++++++++
>  drivers/gpu/drm/i915/intel_drv.h        |  1 +
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 13 +++++++++++++
>  4 files changed, 23 insertions(+), 1 deletion(-)
> 

<SNIP>

> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -2233,6 +2233,19 @@ void intel_runtime_pm_get(struct
> drm_i915_private *dev_priv)
>  	WARN(dev_priv->pm.suspended, "Device still suspended.\n");
>  }
>  
> +bool intel_runtime_pm_tryget(struct drm_i915_private *dev_priv)
> +{
> +	/* XXX Ideally we would push this to pm_runtime_tryget() */

I think we first need to have that function upstream, I sent another
patch which fixes the same bug in a cleaner manner.

This could be incorporated to the series Imre is working on.

Regards, Joonas

> +#ifdef CONFIG_PM
> +	if (HAS_RUNTIME_PM(dev_priv)) {
> +		struct device *device = &dev_priv->dev->pdev->dev;
> +		return atomic_inc_unless_zero(&dev
> ->power.usage_count);
> +	}
> +#else
> +	return true;
> +#endif
> +}
> +
>  /**
>   * intel_runtime_pm_get_noresume - grab a runtime pm reference
>   * @dev_priv: i915 device instance
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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