[Intel-gfx] [PATCH 8/8] drm/i915: Wait on external rendering for GEM objects

[Joonas Lahtinen]

On ti, 2016-07-19 at 15:27 +0100, Chris Wilson wrote:
> On Tue, Jul 19, 2016 at 05:12:22PM +0300, Joonas Lahtinen wrote:
> > 
> > On ti, 2016-07-19 at 11:51 +0100, Chris Wilson wrote:
> > > 
> > > +	resv = i915_gem_object_get_dmabuf_resv(obj);
> > > +	if (resv) {
> > > +		long err;
> > We already have ret in the function scope.
> ret is int, we need a long. At least I can attest to our test coverage!
> 
> > 
> > > 
> > > @@ -3402,13 +3414,13 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write)
> > >  	struct i915_vma *vma;
> > >  	int ret;
> > >  
> > > -	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
> > > -		return 0;
> > > -
> > >  	ret = i915_gem_object_wait_rendering(obj, !write);
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	if (obj->base.write_domain == I915_GEM_DOMAIN_GTT)
> > > +		return 0;
> > > +
> > Not sure I follow this change, wait rendering would only be issued if
> > DOMAIN_CPU was in place, this function was about moving to gtt_domain
> > so should really be NOP if in GTT domain already, no? Maybe calling
> > site should do an explicit wait_rendering if needed.
> No, this is where we do the wait rendering. The API says
> set-to-gtt-domain implies that it is out of the GPU domain (no
> rendering) and out of the CPU domain. (Similarly for set-to-cpu-domain.)
> The relaxation I've applied here is to try and catch third parties that
> have not updated our domain tracking for their access.
> 
> Moving the wait_rendering to the parent is a fair amount of burden.
> 
> > 
> > > 
> > >  		vma->pin_count = 0;
> > > -		ret = i915_vma_unbind(vma);
> > Maybe add a comment/TODO/FIXME: about the potential WARN.
> __i915_vma_unbind_no_wait() is itself an automatic FIXME - it only
> exists to cover up a WARN. (I wish we had this series in place first so
> that such a hack wasn't applied.)

OK then,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>

> -Chris
> 
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation