On Fri, Mar 18, 2011 at 08:02:10AM +0000, Chris Wilson wrote: > ... even though it was disabled. A mistake in the handling of fence reuse > caused us to skip the vital delay of waiting for the object to finish > rendering before changing the register.
Reviewed-by: Daniel Vetter <daniel.vet...@ffwll.ch> Nice catch and good simplification of the code-flow. One nitpick about a possible further cleanup below. > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index d4bf061..c5dfb59 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2581,8 +2581,23 @@ i915_gem_object_get_fence(struct drm_i915_gem_object > *obj, > reg = &dev_priv->fence_regs[obj->fence_reg]; > list_move_tail(®->lru_list, &dev_priv->mm.fence_list); > > - if (!obj->fenced_gpu_access && !obj->last_fenced_seqno) > - pipelined = NULL; > + if (obj->tiling_changed) { > + ret = i915_gem_object_flush_fence(obj, pipelined); > + if (ret) > + return ret; > + > + if (!obj->fenced_gpu_access && !obj->last_fenced_seqno) > + pipelined = NULL; > + > + if (pipelined) { > + reg->setup_seqno = > + i915_gem_next_request_seqno(pipelined); > + obj->last_fenced_seqno = reg->setup_seqno; > + obj->last_fenced_ring = pipelined; > + } > + > + goto update; I think we could move the update label 3 lines up, which would make the above if(pipelined) clause unnecessary. Maybe even drop the goto and extract the tail of get_fence as a function of its own. > + } > > if (!pipelined) { > if (reg->setup_seqno) { > @@ -2601,31 +2616,6 @@ i915_gem_object_get_fence(struct drm_i915_gem_object > *obj, > ret = i915_gem_object_flush_fence(obj, pipelined); > if (ret) > return ret; > - } else if (obj->tiling_changed) { > - if (obj->fenced_gpu_access) { > - if (obj->base.write_domain & > I915_GEM_GPU_DOMAINS) { > - ret = i915_gem_flush_ring(obj->ring, > - 0, > obj->base.write_domain); > - if (ret) > - return ret; > - } > - > - obj->fenced_gpu_access = false; > - } > - } > - > - if (!obj->fenced_gpu_access && !obj->last_fenced_seqno) > - pipelined = NULL; > - BUG_ON(!pipelined && reg->setup_seqno); > - > - if (obj->tiling_changed) { > - if (pipelined) { > - reg->setup_seqno = > - i915_gem_next_request_seqno(pipelined); > - obj->last_fenced_seqno = reg->setup_seqno; > - obj->last_fenced_ring = pipelined; > - } > - goto update; > } > > return 0; > -- > 1.7.2.3 > -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx