On Mon, Nov 09, 2015 at 03:36:10PM +0200, Imre Deak wrote: > On ma, 2015-11-09 at 13:25 +0000, Chris Wilson wrote: > > On Mon, Nov 09, 2015 at 03:09:18PM +0200, Imre Deak wrote: > > > Looked through it, it seems only i915_gem_set_tiling() could > > > release > > > the PTE's without waking up the hardware (if no need to unbind the > > > object). Otherwise it's true that all callers hold (or should hold) > > > already an RPM ref. To fix the set tiling case to work after your > > > optimization we could wake up the HW unconditionally there, use a > > > no_resume RPM ref+and RPM barrier or a separate new lock for the > > > fault > > > list. > > > > I was suggesting we move to the model where writes through gsm took > > the > > rpm reference itself. > > Yes, but even then you want to have a lock around updating the new > fault list, no? So if we go with your way and push down the RPM ref > where GSM is written, we wouldn't have a lock around the fault_list > update in i915_gem_set_tiling() (via i915_gem_release_mmap()). That's > where I meant we need an extra ref/lock above. Ok, I remember some of the specifics I had in mind about having to do it inside the if (vma->map_and_fencable) branch of i915_vma_unbind() as opposed to be able to push it into ggtt_unbind_vma... Hmm, pushing that itself down to ggtt_unbind_vma isn't too silly. Equally moving the vma->ggtt_view.pages = NULL would also help with symmetry. Plenty of opportunity here for cleanup that should pay off nicely wrt to rpm handling :) -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
