[Intel-gfx] [PATCH 6/6] drm/i915/gem: Drop lru bumping on display unpinning

[Chris Wilson]

Quoting Matthew Auld (2021-01-19 16:38:04)
> On Tue, 19 Jan 2021 at 14:49, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> >
> > Simplify the frontbuffer unpin by removing the lock requirement. The LRU
> > bumping was primarily to protect the GTT from being evicted and from
> > frontbuffers being eagerly shrunk. Now we protect frontbuffers from the
> > shrinker, and we avoid accidentally evicting from the GTT, so the
> > benefit from bumping LRU is no more, and we can save more time by not.
> 
> For the GTT evict case, where/how do we currently try to prevent
> accidental eviction for fb?

Our preference is to try with NOEVICT, and then use smaller partial
mappings, reducing the risk of evicting anything that may be reused in
the near future. The goal is to really only use the full GTT mapping for
when HW needs access to the whole object.

However, we could apply the same rule as we do for the shrinker as leave
frontbuffer objects until the second pass. Such as

--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -61,6 +61,19 @@ mark_free(struct drm_mm_scan *scan,
        return drm_mm_scan_add_block(scan, &vma->node);
 }

+static bool skip_vma(struct i915_vma *vma)
+{
+       if (i915_vma_is_active(vma))
+               return true;
+
+       if (i915_is_ggtt(vma) &&
+           vma->obj &&
+           i915_gem_object_is_framebuffer(vma->obj))
+               return true;
+
+       return false;
+}
+
 /**
  * i915_gem_evict_something - Evict vmas to make room for binding a new one
  * @vm: address space to evict from
@@ -150,7 +163,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
                 * To notice when we complete one full cycle, we record the
                 * first active element seen, before moving it to the tail.
                 */
-               if (active != ERR_PTR(-EAGAIN) && i915_vma_is_active(vma)) {
+               if (active != ERR_PTR(-EAGAIN) && skip_vma(vma)) {
                        if (!active)
                                active = vma;

That would be better if we mark the vma as being used by display.
-Chris