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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxxxxx>
> > Cc: Ben Widawsky <benjamin.widawsky@xxxxxxxxx>
> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> 
> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux