Re: [PATCH 1/4] drm/i915: Track fence setup separately from fenced object lifetime

[Chris Wilson <chris@chris-wilson.co.uk>]

On Sat, 19 Mar 2011 23:35:55 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> > @@ -2647,9 +2638,13 @@ i915_gem_object_get_fence(struct drm_i915_gem_object *obj,
> >  			pipelined = NULL;
> >  
> >  		old->fence_reg = I915_FENCE_REG_NONE;
> > -		old->last_fenced_ring = pipelined;
> > -		old->last_fenced_seqno =
> > -			pipelined ? i915_gem_next_request_seqno(pipelined) : 0;
> > +		old->last_fenced_seqno = 0;
> > +		if (pipelined) {
> > +			old->last_fenced_seqno =
> > +				i915_gem_next_request_seqno(pipelined);
> > +			i915_gem_object_move_to_active(old, pipelined,
> > +						       old->last_fenced_seqno);
> > +		}
> >  
> >  		drm_gem_object_unreference(&old->base);
> >  	} else if (obj->last_fenced_seqno == 0)
> 
> This special case of the last_fenced_seqno tracking slightly annoys me. I
> _think_ the flush_ring in flush_fence does already take care of this (but
> I'm not too shure, and this is definitely the safe option).

It's meant to be an optimisation where we note that even though this might
be a pipelined request, the object does not have any outstanding GPU
fenced access and so we can write the fence register immediately.

Worth a comment after cleaning it up (see the later patch).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre