[Intel-gfx] [PATCH] drm/i915: Remove guard page insertion around unevictable nodes

[Chris Wilson]

Quoting Matthew Auld (2021-01-25 11:35:22)
> On Mon, 25 Jan 2021 at 11:28, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Quoting Chris Wilson (2021-01-25 11:24:22)
> > > Quoting Matthew Auld (2021-01-25 11:16:13)
> > > > On Sun, 24 Jan 2021 at 13:57, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > > >
> > > > > Assume that unevictable nodes are not in the GTT and so we can ignore
> > > > > page boundary concerns, and so allow regular nodes to abutt against
> > > > > irregular unevictable nodes.
> > > > >
> > > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.h       |  2 --
> > > > >  drivers/gpu/drm/i915/i915_gem_evict.c |  6 ++++--
> > > > >  drivers/gpu/drm/i915/i915_vma.h       | 10 +++++++++-
> > > > >  drivers/gpu/drm/i915/i915_vma_types.h |  2 ++
> > > > >  4 files changed, 15 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index 99cf861df92d..69c5a185ecff 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -357,8 +357,6 @@ enum i915_cache_level {
> > > > >         I915_CACHE_WT, /* hsw:gt3e WriteThrough for scanouts */
> > > > >  };
> > > > >
> > > > > -#define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
> > > > > -
> > > > >  struct intel_fbc {
> > > > >         /* This is always the inner lock when overlapping with struct_mutex and
> > > > >          * it's the outer lock when overlapping with stolen_lock. */
> > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > index 4d2d59a9942b..aef88fdb9f66 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> > > > > @@ -313,11 +313,13 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> > > > >                  */
> > > > >                 if (i915_vm_has_cache_coloring(vm)) {
> > > > >                         if (node->start + node->size == target->start) {
> > > > > -                               if (node->color == target->color)
> > > > > +                               if (i915_node_color_matches(node,
> > > > > +                                                           target->color))
> > > > >                                         continue;
> > > > >                         }
> > > > >                         if (node->start == target->start + target->size) {
> > > > > -                               if (node->color == target->color)
> > > > > +                               if (i915_node_color_matches(node,
> > > > > +                                                           target->color))
> > > > >                                         continue;
> > > > >                         }
> > > > >                 }
> > > >
> > > > Since we bail early on seeing COLOR_UNEVICTABLE, and since we have to
> > > > enlarge our search space by a page on both ends, do we need something
> > > > like:
> > >
> > > Are we not doing the opposite here, and skipping the evict if either
> > > node/target is unevictable?
> >
> > Oh, you mean the earlier abort if we try to evict an unevictable node
> > inside the target range.
> 
> Yeah, if it only abuts and is COLOR_UNEVICTABLE we can ignore the node
> now, but if it's actually within our range then we abort like before.
> And then there is some strangeness with the head node.

Hmm. On second thought, the reservation is using the direct reserve now
and not entering i915_gem_evict_for_now() so for the moment we don't
have to worry about any changes here. We can ponder whether we can
remove guard pages around foreign nodes later.
-Chris