[Intel-gfx] [PATCH] drm/i915: get runtime PM reference around GEM set_caching IOCTL

[Chris Wilson]

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