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

Daniel Vetter
2011-03-19T22:36:08Z

On Fri, Mar 18, 2011 at 10:35:16PM +0000, Chris Wilson wrote:
> This fixes a bookkeeping error causing an OOPS whilst waiting for an
> object to finish using a fence. Now we can simply wait for the fence to
> be written independent of the objects currently inhabiting it (past,
> present and future).

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Ah, this patch addresses my update label comment ;) Otherwise I think
tracking fences more as independent objects definitely results in clearer
semantics. A tiny nitpick below.

> @@ -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).
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48