[Intel-gfx] [PATCH 19/33] drm/i915: Only clflush the context object when binding

Joonas Lahtinen joonas.lahtinen at linux.intel.com
Wed Aug 10 10:50:11 UTC 2016


On ke, 2016-08-10 at 10:02 +0100, Chris Wilson wrote:
> On Wed, Aug 10, 2016 at 11:41:39AM +0300, Joonas Lahtinen wrote:
> > 
> > On su, 2016-08-07 at 15:45 +0100, Chris Wilson wrote:
> > > 
> > > @@ -771,6 +771,13 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
> > >  	if (skip_rcs_switch(ppgtt, engine, to))
> > >  		return 0;
> > >  
> > > +	if (!(to->engine[RCS].state->flags & I915_VMA_GLOBAL_BIND)) {
> > > +		ret = i915_gem_object_set_to_gtt_domain(to->engine[RCS].state->obj,
> > > +							false);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > > +
> > >  	/* Trying to pin first makes error handling easier. */
> > >  	ret = i915_vma_pin(to->engine[RCS].state,
> > >  			   0, to->ggtt_alignment,
> > This could be lifted inside the if?
> I'd rather not as that would be more unusual and we would then need a
> complicated error path.
> 
> If the compiler is feeling intelligent it could combine the two blocks
> into one, i.e.
> 
> if (unlikely((++vma->flags ^ flags) & I915_VMA_BIND_MASK)) {
> 	ret = i915_gem_object_set_to_gtt_domain(vma->obj, false);
> 	if (ret)
> 		return ret;
> 
> 	ret = __i915_vma_do_pin(vma, size, alignment, flags);
> 	if (ret)
> 		return ret;
> }
> 
> I don't suggest we do the expansion ourselves, and I don't have a bit to
> spare in the PIN_FLAGS.

With the comment moved/updated (mentioned in previous patch);

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

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


More information about the Intel-gfx mailing list