[Intel-gfx] [PATCH 2/3] drm/i915: Fix obj->map_and_fenceable for ppgtt

[Chris Wilson]

On Sat, Aug 09, 2014 at 12:29:17PM -0700, Ben Widawsky wrote:
> On Sat, Aug 09, 2014 at 05:37:23PM +0100, Chris Wilson wrote:
> > With ppgtt, it is no longer correct to mark an object as
> > map_and_fenceable if we simply unbind it from the global gtt. This has
> > consequences during execbuffer where we simply use
> > obj->map_and_fenceable in use_cpu_reloc() to decide which access method
> > to use for writing into the object. Now for a ppgtt object,
> > map_and_fenceable will be true when it is not bound into the ggtt but
> > only in a ppgtt, leading to an invalid access on a non-llc architecture.
> 
> The last sentence confused me for some reason. I think the first two
> sentences were perfectly succinct in describing the problem.

Actually I think the last sentence is obsolete. So long as we only tweak
map_and_fenceable when manipulating the ggtt, the current optimisation
still holds and the subject is wrong.
 
> > v2: Revamp and resend to ease future patches.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Ben Widawsky <benjamin.widawsky at intel.com>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> I didn't read the patch closely, but the problem is clear. It seems like
> the one hunk:
> > -     if (use_cpu_reloc(obj))
> > +     if (use_cpu_reloc(obj) || !i915_gem_obj_ggtt_bound(obj))
> 
> would be sufficient to fix the problem.

Right, I wanted this patch as later patches build upon the flags and
using the flags makes the code cleaner. This tweak was something that
occurred to me when reviewing the patch and looking at recent bug
reports.

Let me go back and look at whether we can keep the obj->map_and_fencceable
micro-optimisation.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre