[Intel-gfx] [PATCH 08/11] drm/i915/gt: Only wait for GPU activity before unbinding a GGTT fence

[Chris Wilson]

Quoting Matthew Auld (2020-04-01 19:56:23)
> On Tue, 31 Mar 2020 at 22:31, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Only GPU activity via the GGTT fence is asynchronous, we know that we
> > control the CPU access directly, so we only need to wait for the GPU to
> > stop using the fence before we relinquish it.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c | 12 ++++++++----
> >  drivers/gpu/drm/i915/gt/intel_ggtt_fencing.h |  3 +++
> >  drivers/gpu/drm/i915/i915_vma.c              |  4 ++++
> >  3 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> > index 225970f4a4ef..74f8201486b2 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt_fencing.c
> > @@ -239,15 +239,18 @@ static int fence_update(struct i915_fence_reg *fence,
> >                 if (!i915_vma_is_map_and_fenceable(vma))
> >                         return -EINVAL;
> >
> > -               ret = i915_vma_sync(vma);
> > -               if (ret)
> > -                       return ret;
> > +               if (INTEL_GEN(fence_to_i915(fence)) < 4) {
> > +                       /* implicit 'unfenced' GPU blits */
> > +                       ret = i915_vma_sync(vma);
> 
> What was the strangeness with gen < 4 again?

>From gen4, all gpu ops have implicit fences and never reference the
global fence registers.

if (gpu_uses_fence_registers())

worksforme

> > +                       if (ret)
> > +                               return ret;
> > +               }
> >         }
> >
> >         old = xchg(&fence->vma, NULL);
> >         if (old) {
> >                 /* XXX Ideally we would move the waiting to outside the mutex */
> > -               ret = i915_vma_sync(old);
> > +               ret = i915_active_wait(&fence->active);
> >                 if (ret) {
> >                         fence->vma = old;
> >                         return ret;
> > @@ -869,6 +872,7 @@ void intel_ggtt_init_fences(struct i915_ggtt *ggtt)
> >         for (i = 0; i < num_fences; i++) {
> >                 struct i915_fence_reg *fence = &ggtt->fence_regs[i];
> >
> > +               i915_active_init(&fence->active, NULL, NULL);
> 
> Some active_fini?

For debug peace of mind, I think we added fini_fences so should be easy
to type up.
-Chris