Quoting Tvrtko Ursulin (2017-06-21 10:17:49)
>
> On 20/06/2017 17:05, Chris Wilson wrote:
> > 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@xxxxxxxxxxxxxxxxxx>
> >>> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> >>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> >>> ---
> >>> 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 :)
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Thanks for accepting my poor grammar :)
Pushed the remaining pair,
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
