Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [RFC 1/4] drm/i915: split out uncore_mmio_debug
Date: Wed, 26 Jun 2019 11:02:26 +0100	[thread overview]
Message-ID: <156154334639.2637.9988052891636686902@skylake-alporthouse-com> (raw)
In-Reply-To: <20190624203152.13725-2-daniele.ceraolospurio@intel.com>

Quoting Daniele Ceraolo Spurio (2019-06-24 21:31:49)
> @@ -605,18 +614,20 @@ void intel_uncore_forcewake_get(struct intel_uncore *uncore,
>  void intel_uncore_forcewake_user_get(struct intel_uncore *uncore)
>  {
>         spin_lock_irq(&uncore->lock);
> +       spin_lock(&uncore->debug->lock);
>         if (!uncore->user_forcewake.count++) {

Afaict, uncore->user_forcewake.count is only guarded by uncore->lock
and we only need to take debug->lock for the debug->unclaimed_mmio_check
manipulation. But there needs to be a shared usage counter around the
debug as it is shared state.

>                 intel_uncore_forcewake_get__locked(uncore, FORCEWAKE_ALL);
>  
>                 /* Save and disable mmio debugging for the user bypass */
>                 uncore->user_forcewake.saved_mmio_check =
> -                       uncore->unclaimed_mmio_check;
> +                       uncore->debug->unclaimed_mmio_check;
>                 uncore->user_forcewake.saved_mmio_debug =
>                         i915_modparams.mmio_debug;

Something more like

spin_lock_irq(&uncore->lock);
if (!uncore->user_forcewake_count++) {
	spin_lock(&uncore->debug->lock);
	if (!uncore->debug->usage_count++) {
		...
	}
	spin_unlock(&uncore->debug->lock);
}
...
spin_unlock_irq(&uncore->lock);
?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx