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

[Matthew Auld]

On Wed, 1 Apr 2020 at 20:02, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> 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.

Ok,
Reviewed-by: Matthew Auld <matthew.auld at intel.com>

> -Chris