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

Chris Wilson
2011-03-19T22:42:14Z

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