drm/i915: Move timeline from GTT to ring

In the future, we want to move a request between engines. To achieve
this, we first realise that we have two timelines in effect here. The
first runs through the GTT is required for ordering vma access, which is
tracked currently by engine. The second is implied by sequential
execution of commands inside the ringbuffer. This timeline is one that
maps to userspace's expectations when submitting requests (i.e. given the
same context, batch A is executed before batch B). As the rings's
timelines map to userspace and the GTT timeline an implementation
detail, move the timeline from the GTT into the ring itself (per-context
in logical-ring-contexts/execlists, or a global per-engine timeline for
the shared ringbuffers in legacy submission.

The two timelines are still assumed to be equivalent at the moment (no
migrating requests between engines yet) and so we can simply move from
one to the other without adding extra ordering.

v2: Reinforce that one isn't allowed to mix the engine execution
timeline with the client timeline from userspace (on the ring).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180502163839.3248-1-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 6268a5103dba6eda7494e49e4cc3e02cba30f83b..ffa87aef31e501c0d1cd3570d68694a8bda2d827 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2059,7 +2059,8 @@ struct drm_i915_private {
                void (*resume)(struct drm_i915_private *);
                void (*cleanup_engine)(struct intel_engine_cs *engine);
 
-               struct i915_gem_timeline global_timeline;
+               struct i915_gem_timeline execution_timeline;
+               struct i915_gem_timeline legacy_timeline;
                struct list_head timelines;
 
                struct list_head active_rings;
@@ -3235,16 +3236,6 @@ i915_gem_context_lookup(struct drm_i915_file_private *file_priv, u32 id)
        return ctx;
 }
 
-static inline struct intel_timeline *
-i915_gem_context_lookup_timeline(struct i915_gem_context *ctx,
-                                struct intel_engine_cs *engine)
-{
-       struct i915_address_space *vm;
-
-       vm = ctx->ppgtt ? &ctx->ppgtt->base : &ctx->i915->ggtt.base;
-       return &vm->timeline.engine[engine->id];
-}
-
 int i915_perf_open_ioctl(struct drm_device *dev, void *data,
                         struct drm_file *file);
 int i915_perf_add_config_ioctl(struct drm_device *dev, void *data,

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa1d94a4eb5fe49a1ac937c2e9b3b852716718a6..438a2fc5bba0a503765c247a3d8195c3911afe42 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3110,10 +3110,10 @@ static void engine_skip_context(struct i915_request *request)
 {
        struct intel_engine_cs *engine = request->engine;
        struct i915_gem_context *hung_ctx = request->ctx;
-       struct intel_timeline *timeline;
+       struct intel_timeline *timeline = request->timeline;
        unsigned long flags;
 
-       timeline = i915_gem_context_lookup_timeline(hung_ctx, engine);
+       GEM_BUG_ON(timeline == engine->timeline);
 
        spin_lock_irqsave(&engine->timeline->lock, flags);
        spin_lock(&timeline->lock);
@@ -3782,7 +3782,7 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915, unsigned int flags)
 
                ret = wait_for_engines(i915);
        } else {
-               ret = wait_for_timeline(&i915->gt.global_timeline, flags);
+               ret = wait_for_timeline(&i915->gt.execution_timeline, flags);
        }
 
        return ret;
@@ -5652,7 +5652,8 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
        WARN_ON(dev_priv->mm.object_count);
 
        mutex_lock(&dev_priv->drm.struct_mutex);
-       i915_gem_timeline_fini(&dev_priv->gt.global_timeline);
+       i915_gem_timeline_fini(&dev_priv->gt.legacy_timeline);
+       i915_gem_timeline_fini(&dev_priv->gt.execution_timeline);
        WARN_ON(!list_empty(&dev_priv->gt.timelines));
        mutex_unlock(&dev_priv->drm.struct_mutex);
 

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 59d4bd4a7b734db0324323c1e78c8e2eb35a51b1..1f4987dc6616101056f08f18a5f303c0849b1d3a 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -122,6 +122,7 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
        lockdep_assert_held(&ctx->i915->drm.struct_mutex);
        GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
 
+       i915_gem_timeline_free(ctx->timeline);
        i915_ppgtt_put(ctx->ppgtt);
 
        for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) {
@@ -376,6 +377,18 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
                ctx->desc_template = default_desc_template(dev_priv, ppgtt);
        }
 
+       if (HAS_EXECLISTS(dev_priv)) {
+               struct i915_gem_timeline *timeline;
+
+               timeline = i915_gem_timeline_create(dev_priv, ctx->name);
+               if (IS_ERR(timeline)) {
+                       __destroy_hw_context(ctx, file_priv);
+                       return ERR_CAST(timeline);
+               }
+
+               ctx->timeline = timeline;
+       }
+
        trace_i915_context_create(ctx);
 
        return ctx;
@@ -584,7 +597,7 @@ static bool engine_has_idle_kernel_context(struct intel_engine_cs *engine)
        list_for_each_entry(timeline, &engine->i915->gt.timelines, link) {
                struct intel_timeline *tl;
 
-               if (timeline == &engine->i915->gt.global_timeline)
+               if (timeline == &engine->i915->gt.execution_timeline)
                        continue;
 
                tl = &timeline->engine[engine->id];

diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
index ace3b129c18966f9c76d88f11bc75b5b463e9f6c..ec53ba06f836d7f22d55ce1fa12c4f98e6f058ad 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.h
+++ b/drivers/gpu/drm/i915/i915_gem_context.h
@@ -58,6 +58,8 @@ struct i915_gem_context {
        /** file_priv: owning file descriptor */
        struct drm_i915_file_private *file_priv;
 
+       struct i915_gem_timeline *timeline;
+
        /**
         * @ppgtt: unique address space (GTT)
         *

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 21d72f695adb717bb2b980001c7eb2db91b72d5b..e9d828324f678141d1877807b156bc9c82cdb0c4 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2111,8 +2111,6 @@ static void i915_address_space_init(struct i915_address_space *vm,
                                    struct drm_i915_private *dev_priv,
                                    const char *name)
 {
-       i915_gem_timeline_init(dev_priv, &vm->timeline, name);
-
        drm_mm_init(&vm->mm, 0, vm->total);
        vm->mm.head_node.color = I915_COLOR_UNEVICTABLE;
 
@@ -2129,7 +2127,6 @@ static void i915_address_space_fini(struct i915_address_space *vm)
        if (pagevec_count(&vm->free_pages))
                vm_free_pages_release(vm, true);
 
-       i915_gem_timeline_fini(&vm->timeline);
        drm_mm_takedown(&vm->mm);
        list_del(&vm->global_link);
 }

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 6efc017e8bb3ff6a99a70d4dda7c7019c8688689..98107925de48df56841060d30ad0c426c3a2fe0f 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -257,7 +257,6 @@ struct i915_pml4 {
 
 struct i915_address_space {
        struct drm_mm mm;
-       struct i915_gem_timeline timeline;
        struct drm_i915_private *i915;
        struct device *dma;
        /* Every address space belongs to a struct file - except for the global

diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.c b/drivers/gpu/drm/i915/i915_gem_timeline.c
index e9fd8760406716b8a0ed1de3dbe01511ecb58101..24f4068cc137c69af449eb3dc84315a3e6660c13 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_timeline.c
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.c
@@ -95,12 +95,28 @@ int i915_gem_timeline_init(struct drm_i915_private *i915,
 
 int i915_gem_timeline_init__global(struct drm_i915_private *i915)
 {
-       static struct lock_class_key class;
+       static struct lock_class_key class1, class2;
+       int err;
+
+       err = __i915_gem_timeline_init(i915,
+                                      &i915->gt.execution_timeline,
+                                      "[execution]", &class1,
+                                      "i915_execution_timeline");
+       if (err)
+               return err;
+
+       err = __i915_gem_timeline_init(i915,
+                                      &i915->gt.legacy_timeline,
+                                      "[global]", &class2,
+                                      "i915_global_timeline");
+       if (err)
+               goto err_exec_timeline;
+
+       return 0;
 
-       return __i915_gem_timeline_init(i915,
-                                       &i915->gt.global_timeline,
-                                       "[execution]",
-                                       &class, "&global_timeline->lock");
+err_exec_timeline:
+       i915_gem_timeline_fini(&i915->gt.execution_timeline);
+       return err;
 }
 
 /**
@@ -148,6 +164,34 @@ void i915_gem_timeline_fini(struct i915_gem_timeline *timeline)
        kfree(timeline->name);
 }
 
+struct i915_gem_timeline *
+i915_gem_timeline_create(struct drm_i915_private *i915, const char *name)
+{
+       struct i915_gem_timeline *timeline;
+       int err;
+
+       timeline = kzalloc(sizeof(*timeline), GFP_KERNEL);
+       if (!timeline)
+               return ERR_PTR(-ENOMEM);
+
+       err = i915_gem_timeline_init(i915, timeline, name);
+       if (err) {
+               kfree(timeline);
+               return ERR_PTR(err);
+       }
+
+       return timeline;
+}
+
+void i915_gem_timeline_free(struct i915_gem_timeline *timeline)
+{
+       if (!timeline)
+               return;
+
+       i915_gem_timeline_fini(timeline);
+       kfree(timeline);
+}
+
 #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
 #include "selftests/mock_timeline.c"
 #include "selftests/i915_gem_timeline.c"

diff --git a/drivers/gpu/drm/i915/i915_gem_timeline.h b/drivers/gpu/drm/i915/i915_gem_timeline.h
index 6e82119e2cd88d1ba7cf5b0ae4d17829dfc1bc86..780ed465c4fcc8829f1ebf2310440f2989b6c6e2 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_timeline.h
+++ b/drivers/gpu/drm/i915/i915_gem_timeline.h
@@ -90,6 +90,10 @@ int i915_gem_timeline_init__global(struct drm_i915_private *i915);
 void i915_gem_timelines_park(struct drm_i915_private *i915);
 void i915_gem_timeline_fini(struct i915_gem_timeline *tl);
 
+struct i915_gem_timeline *
+i915_gem_timeline_create(struct drm_i915_private *i915, const char *name);
+void i915_gem_timeline_free(struct i915_gem_timeline *timeline);
+
 static inline int __intel_timeline_sync_set(struct intel_timeline *tl,
                                            u64 context, u32 seqno)
 {

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index c8fc4b323e6240d3b6c7d96d674f48c2a647f73d..7bb613c00cc33990c7505381ed1c56019c799863 100644 (file)
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -758,7 +758,12 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
                }
        }
 
-       rq->timeline = i915_gem_context_lookup_timeline(ctx, engine);
+       INIT_LIST_HEAD(&rq->active_list);
+       rq->i915 = i915;
+       rq->engine = engine;
+       rq->ctx = ctx;
+       rq->ring = ring;
+       rq->timeline = ring->timeline;
        GEM_BUG_ON(rq->timeline == engine->timeline);
 
        spin_lock_init(&rq->lock);
@@ -774,12 +779,6 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
 
        i915_sched_node_init(&rq->sched);
 
-       INIT_LIST_HEAD(&rq->active_list);
-       rq->i915 = i915;
-       rq->engine = engine;
-       rq->ctx = ctx;
-       rq->ring = ring;
-
        /* No zalloc, must clear what we need by hand */
        rq->global_seqno = 0;
        rq->signaling.wait.seqno = 0;

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 9164e6d665f8535ff8fefe795adba9b072264c60..7af5fe85612d718fb9965c9e37e262ca305cc82f 100644 (file)
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -453,7 +453,8 @@ void intel_engine_init_global_seqno(struct intel_engine_cs *engine, u32 seqno)
 
 static void intel_engine_init_timeline(struct intel_engine_cs *engine)
 {
-       engine->timeline = &engine->i915->gt.global_timeline.engine[engine->id];
+       engine->timeline =
+               &engine->i915->gt.execution_timeline.engine[engine->id];
 }
 
 static void intel_engine_init_batch_pool(struct intel_engine_cs *engine)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 57396a2a6ea206d176a2e1c70cb9af95b21438bd..9b2407753ebd0ea051effbf9e4c1a399d1f3dc16 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2624,7 +2624,7 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
                goto error_deref_obj;
        }
 
-       ring = intel_engine_create_ring(engine, ctx->ring_size);
+       ring = intel_engine_create_ring(engine, ctx->timeline, ctx->ring_size);
        if (IS_ERR(ring)) {
                ret = PTR_ERR(ring);
                goto error_deref_obj;

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 007449cfa22ba1fc01e41f13940c3eb19b929287..b73e700c30485c06e1629f9f9af2c1b77289a24e 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1117,13 +1117,16 @@ err:
 }
 
 struct intel_ring *
-intel_engine_create_ring(struct intel_engine_cs *engine, int size)
+intel_engine_create_ring(struct intel_engine_cs *engine,
+                        struct i915_gem_timeline *timeline,
+                        int size)
 {
        struct intel_ring *ring;
        struct i915_vma *vma;
 
        GEM_BUG_ON(!is_power_of_2(size));
        GEM_BUG_ON(RING_CTL_SIZE(size) & ~RING_NR_PAGES);
+       GEM_BUG_ON(&timeline->engine[engine->id] == engine->timeline);
        lockdep_assert_held(&engine->i915->drm.struct_mutex);
 
        ring = kzalloc(sizeof(*ring), GFP_KERNEL);
@@ -1131,6 +1134,7 @@ intel_engine_create_ring(struct intel_engine_cs *engine, int size)
                return ERR_PTR(-ENOMEM);
 
        INIT_LIST_HEAD(&ring->request_list);
+       ring->timeline = &timeline->engine[engine->id];
 
        ring->size = size;
        /* Workaround an erratum on the i830 which causes a hang if
@@ -1327,7 +1331,9 @@ static int intel_init_ring_buffer(struct intel_engine_cs *engine)
        if (err)
                goto err;
 
-       ring = intel_engine_create_ring(engine, 32 * PAGE_SIZE);
+       ring = intel_engine_create_ring(engine,
+                                       &engine->i915->gt.legacy_timeline,
+                                       32 * PAGE_SIZE);
        if (IS_ERR(ring)) {
                err = PTR_ERR(ring);
                goto err;

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index fd679cec9ac657942f8149419931a578fade8a15..da53aa2973a735120d112a3d8415b2437cc2c34b 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -129,6 +129,7 @@ struct intel_ring {
        struct i915_vma *vma;
        void *vaddr;
 
+       struct intel_timeline *timeline;
        struct list_head request_list;
        struct list_head active_link;
 
@@ -768,7 +769,9 @@ intel_write_status_page(struct intel_engine_cs *engine, int reg, u32 value)
 #define CNL_HWS_CSB_WRITE_INDEX                0x2f
 
 struct intel_ring *
-intel_engine_create_ring(struct intel_engine_cs *engine, int size);
+intel_engine_create_ring(struct intel_engine_cs *engine,
+                        struct i915_gem_timeline *timeline,
+                        int size);
 int intel_ring_pin(struct intel_ring *ring,
                   struct drm_i915_private *i915,
                   unsigned int offset_bias);

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 7ecaed50d0b99a7ec1a0aadac8f6f7d706ef86d3..24ac648dc83a64fa19821b9df4f47de276b65f0b 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -355,6 +355,18 @@ static int igt_ctx_exec(void *arg)
 
                if (first_shared_gtt) {
                        ctx = __create_hw_context(i915, file->driver_priv);
+                       if (!IS_ERR(ctx) && HAS_EXECLISTS(i915)) {
+                               struct i915_gem_timeline *timeline;
+
+                               timeline = i915_gem_timeline_create(i915, ctx->name);
+                               if (IS_ERR(timeline)) {
+                                       __destroy_hw_context(ctx, file->driver_priv);
+                                       ctx = ERR_CAST(timeline);
+                               } else {
+                                       ctx->timeline = timeline;
+                               }
+                       }
+
                        first_shared_gtt = false;
                } else {
                        ctx = i915_gem_create_context(i915, file->driver_priv);

diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
index 19175ddcb45b8d773e2f349d3b959acfc4908a21..6752498e2c73de8af5b01fce660978163203c816 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/mock_engine.c
+++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
@@ -140,6 +140,8 @@ static struct intel_ring *mock_ring(struct intel_engine_cs *engine)
        if (!ring)
                return NULL;
 
+       ring->timeline = &engine->i915->gt.legacy_timeline.engine[engine->id];
+
        ring->size = sz;
        ring->effective_size = sz;
        ring->vaddr = (void *)(ring + 1);
@@ -180,8 +182,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
        engine->base.emit_breadcrumb = mock_emit_breadcrumb;
        engine->base.submit_request = mock_submit_request;
 
-       engine->base.timeline =
-               &i915->gt.global_timeline.engine[engine->base.id];
+       intel_engine_init_timeline(&engine->base);
 
        intel_engine_init_breadcrumbs(&engine->base);
        engine->base.breadcrumbs.mock = true; /* prevent touching HW for irqs */

diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
index f22a2b35a283c14cb424dfaa470d43ff6e601ecb..f11c83e8ff324a43e4ca72722f55daf34673d9be 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@ -73,7 +73,9 @@ static void mock_device_release(struct drm_device *dev)
 
        mutex_lock(&i915->drm.struct_mutex);
        mock_fini_ggtt(i915);
-       i915_gem_timeline_fini(&i915->gt.global_timeline);
+       i915_gem_timeline_fini(&i915->gt.legacy_timeline);
+       i915_gem_timeline_fini(&i915->gt.execution_timeline);
+       WARN_ON(!list_empty(&i915->gt.timelines));
        mutex_unlock(&i915->drm.struct_mutex);
 
        destroy_workqueue(i915->wq);

diff --git a/drivers/gpu/drm/i915/selftests/mock_gtt.c b/drivers/gpu/drm/i915/selftests/mock_gtt.c
index e96873f96116ecddabfac25b1804b39be9daf2b4..36c112088940585c8bcf0bcdb66027f3ec9e61da 100644 (file)
--- a/drivers/gpu/drm/i915/selftests/mock_gtt.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gtt.c
@@ -76,7 +76,6 @@ mock_ppgtt(struct drm_i915_private *i915,
 
        INIT_LIST_HEAD(&ppgtt->base.global_link);
        drm_mm_init(&ppgtt->base.mm, 0, ppgtt->base.total);
-       i915_gem_timeline_init(i915, &ppgtt->base.timeline, name);
 
        ppgtt->base.clear_range = nop_clear_range;
        ppgtt->base.insert_page = mock_insert_page;