[Intel-gfx] [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT

[Chris Wilson]

On Thu, May 25, 2017 at 11:54:03AM +0100, Tvrtko Ursulin wrote:
> 
> On 24/05/2017 16:54, Jon Bloomfield wrote:
> >BXT has a H/W issue with IOMMU which can lead to system hangs when
> >Aperture accesses are queued within the GAM behind GTT Accesses.
> >
> >This patch avoids the condition by wrapping all GTT updates in stop_machine
> >and using a flushing read prior to restarting the machine.
> >
> >The stop_machine guarantees no new Aperture accesses can begin while
> >the PTE writes are being emmitted. The flushing read ensures that
> >any following Aperture accesses cannot begin until the PTE writes
> >have been cleared out of the GAM's fifo.
> >
> >Only FOLLOWING Aperture accesses need to be separated from in flight
> >PTE updates. PTE Writes may follow tightly behind already in flight
> >Aperture accesses, so no flushing read is required at the start of
> >a PTE update sequence.
> >
> >This issue was reproduced by running
> >	igt/gem_readwrite and
> >	igt/gem_render_copy
> >simultaneously from different processes, each in a tight loop,
> >with INTEL_IOMMU enabled.
> >
> >This patch was originally published as:
> >	drm/i915: Serialize GTT Updates on BXT
> >
> >v2: Move bxt/iommu detection into static function
> >    Remove #ifdef CONFIG_INTEL_IOMMU protection
> >    Make function names more reflective of purpose
> >    Move flushing read into static function
> >
> >Signed-off-by: Jon Bloomfield <jon.bloomfield at intel.com>
> >Cc: John Harrison <john.C.Harrison at intel.com>
> >Cc: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Daniel Vetter <daniel.vetter at intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

> >+static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
> >+{
> >+	struct insert_page *arg = _arg;
> >+
> 
> I don't remember seeing blank lines in declaration blocks anywhere
> else so I'd remove this (and below).
> 
> But FWIW, in essence the patch looks good to me. I was even
> surprised that GCC understands not to complain about defined but
> unused functions when IOMMU is compiled out. One learns something
> new every day! (Until one forgets it again!)
> 
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Tidied up the whitespace to appease checkpatch, and pushed.
Thanks for the fix and review,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre