On Sat, 19 Mar 2011 23:17:56 +0100, Daniel Vetter <dan...@ffwll.ch> wrote:
> 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.
Hints for further cleanups are always appreciated. :)
>
> > 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.
Right, I did this in a later patch. I want to keep this as a simple
rearrangement of the code since that is the minimal fix and improve upon
that with further patches. Sound good?
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
