[Intel-gfx] [PATCH 05/10] drm/i915: add assert_rpm_wakelock_held helper

[Chris Wilson]

On Tue, Dec 15, 2015 at 08:10:33PM +0200, Imre Deak wrote:
> As a preparation for follow-up patches add a new helper that checks
> whether we hold an RPM reference, since this is what we want most of
> the cases. Atm this helper will only check for the HW suspended state, a
> follow-up patch will do the actual change to check the refcount instead.
> One exception is the forcewake release timer function, where it's
> guaranteed that the HW is on even though the RPM refcount drops to zero.
> This guarantee is provided by flushing the timer in the runtime suspend
> handler. So leave the assert_device_not_suspended check in place there.
> 
> Also rename assert_device_suspended for consistency and export these
> helpers as a preparation for the follow-up patches.
> 
> No functional change.
> 
> v3:
> - change the assert warning message to be more meaningful (Chris)
> 
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_drv.h    | 14 ++++++++++++++
>  drivers/gpu/drm/i915/intel_uncore.c | 20 +++++++-------------
>  2 files changed, 21 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 798463e..9837a25 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1430,6 +1430,20 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
>  			     enum intel_display_power_domain domain);
>  void intel_display_power_put(struct drm_i915_private *dev_priv,
>  			     enum intel_display_power_domain domain);
> +
> +static inline void
> +assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
> +{
> +	WARN_ONCE(dev_priv->pm.suspended,
> +		  "Device suspended during HW access\n");
> +}

On irc, Joonas expressed a wish to see all errors during an igt run,
i.e. something like

static inline void
assert_rpm_device_not_suspended(struct drm_i915_private *dev_priv)
{
	WARN(dev_priv->pm.suspended &&
	     atomic_inc_return(&dev_priv->pm.errors) < 0,
		  "Device suspended during HW access\n");
}

with

static int
i915_pm_errors_get(void *data, u64 *val)
{
	struct drm_device *dev = data;

	*val = atomic_read(&to_i915(dev)->pm.erors);
	return 0;
}

static int
i915_pm_errors_set(void *data, u64 val)
{
	struct drm_device *dev = data;

	atomic_set(&to_i915(dev)->pm.errors, val);
	return 0;
}

DEFINE_SIMPLE_ATTRIBUTE(i915_pm_errors_fops,
                        i915_pm_errors_get,
			i915_pm_errors_set,
			"0x%llx\n");


Then users only see the first WARN (and not swamped when it does go
wrong) and igt can echo INT_MIN > /sys/kernel/debug/dri/0/i915_pm_errors
to generate a WARN for each failure (and can even do a quick check for
any errors during a test by reading back i915_pm_errors).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre