[Intel-gfx] [PATCH v2] drm/i915: Disable mmio debugging during user access

[Michal Wajdeczko]

On Thu, Sep 07, 2017 at 02:44:41PM +0100, Chris Wilson wrote:
> If the user bypasses i915 and accesses mmio directly, that easily
> confuses our automatic mmio debugging (any error we then detect is
> likely to be as a result of the user). Since we expect userspace to open
> debugfs/i915_forcewake_user if i915.ko is loaded and they want mmio
> access, that makes the opportune time to disable our debugging for
> duration of the bypass.
> 
> v2: Move the fiddling of uncore internals to uncore.c
> 
> References: https://bugs.freedesktop.org/show_bug.cgi?id=102543
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---

<snip>

> +/**
> + * intel_uncore_forcewake_user_get - claim forcewake on behalf of userspace
> + * @dev_priv: i915 device instance
> + *
> + * This function is a wrapper around intel_uncore_forcewake_get() to acquire
> + * the GT powerwell and in the process disable our debugging for the
> + * duration of userspace's bypass.
> + */
> +void intel_uncore_forcewake_user_get(struct drm_i915_private *dev_priv)

Maybe little off-topic: This file already uses in many places "i915" as a name
for the "drm_i915_private" param (which is the preferred name over "dev_priv").
Why for this new function you are still using legacy name ?

Is it due to the fact that you want to avoid name overload as you want to
access global "i915"? If so, what is our long term plan to avoid such clashes.

Thanks,
Michal

> +{
> +	spin_lock_irq(&dev_priv->uncore.lock);
> +	if (!dev_priv->uncore.user_forcewake.count++) {
> +		intel_uncore_forcewake_get__locked(dev_priv, FORCEWAKE_ALL);
> +
> +		/* Save and disable mmio debugging for the user bypass */
> +		dev_priv->uncore.user_forcewake.saved_mmio_check =
> +			dev_priv->uncore.unclaimed_mmio_check;
> +		dev_priv->uncore.user_forcewake.saved_mmio_debug =
> +			i915.mmio_debug;
                        ^^^^

> +
> +		dev_priv->uncore.unclaimed_mmio_check = 0;
> +		i915.mmio_debug = 0;
                ^^^^

> +	}
> +	spin_unlock_irq(&dev_priv->uncore.lock);
> +}
> +