Re: [PATCH v2] drm/i915: Track the last-active inside the i915_vma

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 05/07/2018 13:02, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-07-05 12:38:46)

On 04/07/2018 09:34, Chris Wilson wrote:
Using a VMA on more than one timeline concurrently is the exception
rather than the rule (using it concurrently on multiple engines). As we
expect to only use one active tracker, store the most recently used
tracker inside the i915_vma itself and only fallback to the rbtree if
we need a second or more concurrent active trackers.

v2: Comments on how we overwrite any existing last_active cache.

Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_vma.c | 50 +++++++++++++++++++++++++++++++--
   drivers/gpu/drm/i915/i915_vma.h |  1 +
   2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index cd94ffc7f079..33925e00f7e8 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -119,6 +119,12 @@ i915_vma_retire(struct i915_gem_active *base, struct i915_request *rq)
       __i915_vma_retire(active->vma, rq);
   }
+static void
+i915_vma_last_retire(struct i915_gem_active *base, struct i915_request *rq)
+{
+     __i915_vma_retire(container_of(base, struct i915_vma, last_active), rq);
+}
+
   static struct i915_vma *
   vma_create(struct drm_i915_gem_object *obj,
          struct i915_address_space *vm,
@@ -136,6 +142,7 @@ vma_create(struct drm_i915_gem_object *obj,
vma->active = RB_ROOT; + init_request_active(&vma->last_active, i915_vma_last_retire);
       init_request_active(&vma->last_fence, NULL);
       vma->vm = vm;
       vma->ops = &vm->vma_ops;
@@ -895,6 +902,22 @@ static struct i915_gem_active *lookup_active(struct i915_vma *vma, u64 idx)
   {
       struct i915_vma_active *active;
       struct rb_node **p, *parent;
+     struct i915_request *old;
+
+     /*
+      * We track the most recently used timeline to skip a rbtree search
+      * for the common case, under typical loads we never need the rbtree
+      * at all. We can reuse the last_active slot if it is empty, that is
+      * after the previous activity has been retired, or if the active
+      * matches the current timeline.
+      */
+     old = i915_gem_active_raw(&vma->last_active,
+                               &vma->vm->i915->drm.struct_mutex);
+     if (!old || old->fence.context == idx)
+             goto out;

Is the situation that retire can be out of order relative to
move_to_active? In other words, last_active can retire before the rbtree
record, and so the following new move_to_active will find last_active
empty and so could create a double entry for the same timeline?

We don't mind a double entry, and do expect that last_active and the
rbtree entry will still be active, tracking different requests.

Maybe I mind double entries, if avoiding them would make the code easier to understand. :) Or not that we don't mind, but need them? Different requests you say, but on same timeline or?

Regards,

Tvrtko


_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux