[Intel-gfx] [PATCH] drm/i915: Fix obj->map_and_fenceable across tiling changes

[Chris Wilson]

On Fri, Nov 07, 2014 at 11:05:05AM +0100, Daniel Vetter wrote:
> On Thu, Nov 06, 2014 at 08:40:35AM +0000, Chris Wilson wrote:
> > As obj->map_and_fenceable computation has changed to only be set when
> > the object is bound inside the global GTT (and is suitable aligned to a
> > fence region) we need to accommodate those changes when the tiling is
> > adjusted. The easiest solution is to unbind from the global GTT if we
> > are currently fenceable, but will not be after the tiling change.
> 
> QA failed to supply the bisect for this regression, but most likely this
> has been introduced due to the change in handling obj->map_and_fenceable
> in
> 
> commit e6a844687cf929ec053c7578d5ecc794a8a6c5cf
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date:   Mon Aug 11 12:00:12 2014 +0200
> 
>     drm/i915: Force CPU relocations if not GTT mapped

I think it also took

commit f8fcadba218fe6d23b2e353fea1cf0a4be4c9454
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Fri Oct 31 13:53:52 2014 +0000

    drm/i915: Only mark as map-and-fenceable when bound into the GGTT

to expose the bug in testing.

> Note that the alignment check is a vestige from our (unsuccessful)
> attempts to reduce the alignment requirements of tiled but unfenced
> buffers on gen2/3.

Also, that was when unbinding from the GTT meant UC writes and clflushing,
so we went to great pains to avoid such.
 
> That leaves the actual bug of setting map_and_fenceable to true if we're
> not bound to ggtt, which violates the change introduced in the above
> patch. Unbinding in that case really looks like the simplest and safest
> option, we have to do it anyway.
> 
> If Chris agrees, please add the above analysis to the commit message when
> merging to -fixes. 
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=85896
> > Tested-by: huax.lu at intel.com
> 
> Testcase: igt/gem_concurrent_blit
Testcase: igt/gem_concurrent_blit/gttX*

It was also only triggered by recent additions to
gem_concurrent_blit (which itself was trying to stress test our
fence-vs-GPU serialisation for testing requests - so I can claim it was
intentional!). However, it turns out to be easier to hit in practice
than in testing. :|
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre