[Intel-gfx] [PATCH 1/3] drm/i915: Retire the VMA's fence tracker before unbinding

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-20 16:55:12)
> 
> On 20/06/2017 13:43, Chris Wilson wrote:
> > Since we may track unfenced access (GPU access to the vma that
> > explicitly requires no fence), vma->last_fence may be set without any
> 
> Is this the gen < 4 code path? There is no actual fence in this case?

Yes, this is for old hw that used a fence for GPU access, and in this
case indeed there is no fence and we are tracking the access for when
the fence is not wanted to ensure that don't install a fence before the
GPU finished accessing the region.

> > attached fence (vma->fence) and so will not be flushed when we call
> > i915_vma_put_fence(). Since we stopped doing a full retire of the
> > activity trackers for unbind, we need to explicitly retire each tracker.
> > 
> > Fixes: b0decaf75bd9 ("drm/i915: Track active vma requests")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_vma.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> > index 532c709febbd..1cfe137cdc32 100644
> > --- a/drivers/gpu/drm/i915/i915_vma.c
> > +++ b/drivers/gpu/drm/i915/i915_vma.c
> > @@ -672,6 +672,11 @@ int i915_vma_unbind(struct i915_vma *vma)
> >                               break;
> >               }
> >   
> > +             if (!ret) {
> > +                     ret = i915_gem_active_retire(&vma->last_fence,
> > +                                                  &vma->vm->i915->drm.struct_mutex);
> > +             }
> > +
> >               __i915_vma_unpin(vma);
> >               if (ret)
> >                       return ret;
> > 
> 
> Looks safe anyway, but I'd like to understand how exactly it happens and 
> if it is still possible after patch 2/3.

For gen2/3, we still use last_fence. For others it was accidental and
should be no more after patch 2. I considered add a GEM_BUG_ON to
i915_move_to_active but didn't have a convenient i915 pointer, and I
have grander plans :)
-Chris