drm/i915/bdw: Pin the context backing objects to GGTT on-demand

Up until now, we have pinned every logical ring context backing object
during creation, and left it pinned until destruction. This made my life
easier, but it's a harmful thing to do, because we cause fragmentation
of the GGTT (and, eventually, we would run out of space).

This patch makes the pinning on-demand: the backing objects of the two
contexts that are written to the ELSP are pinned right before submission
and unpinned once the hardware is done with them. The only context that
is still pinned regardless is the global default one, so that the HWS can
still be accessed in the same way (ring->status_page).

v2: In the early version of this patch, we were pinning the context as
we put it into the ELSP: on the one hand, this is very efficient because
only a maximum two contexts are pinned at any given time, but on the other
hand, we cannot really pin in interrupt time :(

v3: Use a mutex rather than atomic_t to protect pin count to avoid races.
Do not unpin default context in free_request.

v4: Break out pin and unpin into functions.  Fix style problems reported
by checkpatch

v5: Remove unpin_lock as all pinning and unpinning is done with the struct
mutex already locked.  Add WARN_ONs to make sure this is the case in future.

Issue: VIZ-4277
Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Reviewed-by: Akash Goel <akash.goels@gmail.com>
Reviewed-by: Deepak S<deepak.s@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 319da61354b09831dc9beef2ed23a70bd20cb3a9..bb1458381dc45d8704f86a950b7d9bc46879feb2 100644 (file)
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1799,10 +1799,16 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
                                continue;
 
                        if (ctx_obj) {
-                               struct page *page = i915_gem_object_get_page(ctx_obj, 1);
-                               uint32_t *reg_state = kmap_atomic(page);
+                               struct page *page;
+                               uint32_t *reg_state;
                                int j;
 
+                               i915_gem_obj_ggtt_pin(ctx_obj,
+                                               GEN8_LR_CONTEXT_ALIGN, 0);
+
+                               page = i915_gem_object_get_page(ctx_obj, 1);
+                               reg_state = kmap_atomic(page);
+
                                seq_printf(m, "CONTEXT: %s %u\n", ring->name,
                                                intel_execlists_ctx_id(ctx_obj));
 
@@ -1814,6 +1820,8 @@ static int i915_dump_lrc(struct seq_file *m, void *unused)
                                }
                                kunmap_atomic(reg_state);
 
+                               i915_gem_object_ggtt_unpin(ctx_obj);
+
                                seq_putc(m, '\n');
                        }
                }

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4f08160d6192b57358faec9667541ce08035e9f3..1097d20a4f35018264eb4184b0af99826d0551a4 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -685,6 +685,7 @@ struct intel_context {
        struct {
                struct drm_i915_gem_object *state;
                struct intel_ringbuffer *ringbuf;
+               int unpin_count;
        } engine[I915_NUM_RINGS];
 
        struct list_head link;

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2e85ef1650bfab5c1d534dacc092ae4b79b5fbce..7f95d8ff28af06a3b97d7f0a7fc71e82009223c2 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2576,12 +2576,18 @@ static void i915_set_reset_status(struct drm_i915_private *dev_priv,
 
 static void i915_gem_free_request(struct drm_i915_gem_request *request)
 {
+       struct intel_context *ctx = request->ctx;
+
        list_del(&request->list);
        i915_gem_request_remove_from_client(request);
 
-       if (request->ctx)
-               i915_gem_context_unreference(request->ctx);
+       if (i915.enable_execlists && ctx) {
+               struct intel_engine_cs *ring = request->ring;
 
+               if (ctx != ring->default_context)
+                       intel_lr_context_unpin(ring, ctx);
+               i915_gem_context_unreference(ctx);
+       }
        kfree(request);
 }
 
@@ -2635,6 +2641,23 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
                i915_gem_object_move_to_inactive(obj);
        }
 
+       /*
+        * Clear the execlists queue up before freeing the requests, as those
+        * are the ones that keep the context and ringbuffer backing objects
+        * pinned in place.
+        */
+       while (!list_empty(&ring->execlist_queue)) {
+               struct intel_ctx_submit_request *submit_req;
+
+               submit_req = list_first_entry(&ring->execlist_queue,
+                               struct intel_ctx_submit_request,
+                               execlist_link);
+               list_del(&submit_req->execlist_link);
+               intel_runtime_pm_put(dev_priv);
+               i915_gem_context_unreference(submit_req->ctx);
+               kfree(submit_req);
+       }
+
        /*
         * We must free the requests after all the corresponding objects have
         * been moved off active lists. Which is the same order as the normal
@@ -2652,18 +2675,6 @@ static void i915_gem_reset_ring_cleanup(struct drm_i915_private *dev_priv,
                i915_gem_free_request(request);
        }
 
-       while (!list_empty(&ring->execlist_queue)) {
-               struct intel_ctx_submit_request *submit_req;
-
-               submit_req = list_first_entry(&ring->execlist_queue,
-                               struct intel_ctx_submit_request,
-                               execlist_link);
-               list_del(&submit_req->execlist_link);
-               intel_runtime_pm_put(dev_priv);
-               i915_gem_context_unreference(submit_req->ctx);
-               kfree(submit_req);
-       }
-
        /* These may not have been flush before the reset, do so now */
        kfree(ring->preallocated_lazy_request);
        ring->preallocated_lazy_request = NULL;

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c855051ba18d65cb37abdafff37f28b6db1d70d8..9560e634c9b8e8793b6287c538c5bb91ff1b8f38 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -140,8 +140,6 @@
 #define GEN8_LR_CONTEXT_RENDER_SIZE (20 * PAGE_SIZE)
 #define GEN8_LR_CONTEXT_OTHER_SIZE (2 * PAGE_SIZE)
 
-#define GEN8_LR_CONTEXT_ALIGN 4096
-
 #define RING_EXECLIST_QFULL            (1 << 0x2)
 #define RING_EXECLIST1_VALID           (1 << 0x3)
 #define RING_EXECLIST0_VALID           (1 << 0x4)
@@ -814,9 +812,40 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf)
        execlists_context_queue(ring, ctx, ringbuf->tail);
 }
 
+static int intel_lr_context_pin(struct intel_engine_cs *ring,
+               struct intel_context *ctx)
+{
+       struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+       int ret = 0;
+
+       WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+       if (ctx->engine[ring->id].unpin_count++ == 0) {
+               ret = i915_gem_obj_ggtt_pin(ctx_obj,
+                               GEN8_LR_CONTEXT_ALIGN, 0);
+               if (ret)
+                       ctx->engine[ring->id].unpin_count = 0;
+       }
+
+       return ret;
+}
+
+void intel_lr_context_unpin(struct intel_engine_cs *ring,
+               struct intel_context *ctx)
+{
+       struct drm_i915_gem_object *ctx_obj = ctx->engine[ring->id].state;
+
+       if (ctx_obj) {
+               WARN_ON(!mutex_is_locked(&ring->dev->struct_mutex));
+               if (--ctx->engine[ring->id].unpin_count == 0)
+                       i915_gem_object_ggtt_unpin(ctx_obj);
+       }
+}
+
 static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
                                    struct intel_context *ctx)
 {
+       int ret;
+
        if (ring->outstanding_lazy_seqno)
                return 0;
 
@@ -827,6 +856,14 @@ static int logical_ring_alloc_seqno(struct intel_engine_cs *ring,
                if (request == NULL)
                        return -ENOMEM;
 
+               if (ctx != ring->default_context) {
+                       ret = intel_lr_context_pin(ring, ctx);
+                       if (ret) {
+                               kfree(request);
+                               return ret;
+                       }
+               }
+
                /* Hold a reference to the context this request belongs to
                 * (we will need it when the time comes to emit/retire the
                 * request).
@@ -1680,12 +1717,16 @@ void intel_lr_context_free(struct intel_context *ctx)
 
        for (i = 0; i < I915_NUM_RINGS; i++) {
                struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state;
-               struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf;
 
                if (ctx_obj) {
+                       struct intel_ringbuffer *ringbuf =
+                                       ctx->engine[i].ringbuf;
+                       struct intel_engine_cs *ring = ringbuf->ring;
+
                        intel_destroy_ringbuffer_obj(ringbuf);
                        kfree(ringbuf);
-                       i915_gem_object_ggtt_unpin(ctx_obj);
+                       if (ctx == ring->default_context)
+                               i915_gem_object_ggtt_unpin(ctx_obj);
                        drm_gem_object_unreference(&ctx_obj->base);
                }
        }
@@ -1748,6 +1789,7 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring,
 int intel_lr_context_deferred_create(struct intel_context *ctx,
                                     struct intel_engine_cs *ring)
 {
+       const bool is_global_default_ctx = (ctx == ring->default_context);
        struct drm_device *dev = ring->dev;
        struct drm_i915_gem_object *ctx_obj;
        uint32_t context_size;
@@ -1767,18 +1809,22 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
                return ret;
        }
 
-       ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
-       if (ret) {
-               DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n", ret);
-               drm_gem_object_unreference(&ctx_obj->base);
-               return ret;
+       if (is_global_default_ctx) {
+               ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, 0);
+               if (ret) {
+                       DRM_DEBUG_DRIVER("Pin LRC backing obj failed: %d\n",
+                                       ret);
+                       drm_gem_object_unreference(&ctx_obj->base);
+                       return ret;
+               }
        }
 
        ringbuf = kzalloc(sizeof(*ringbuf), GFP_KERNEL);
        if (!ringbuf) {
                DRM_DEBUG_DRIVER("Failed to allocate ringbuffer %s\n",
                                ring->name);
-               i915_gem_object_ggtt_unpin(ctx_obj);
+               if (is_global_default_ctx)
+                       i915_gem_object_ggtt_unpin(ctx_obj);
                drm_gem_object_unreference(&ctx_obj->base);
                ret = -ENOMEM;
                return ret;
@@ -1841,7 +1887,8 @@ int intel_lr_context_deferred_create(struct intel_context *ctx,
 
 error:
        kfree(ringbuf);
-       i915_gem_object_ggtt_unpin(ctx_obj);
+       if (is_global_default_ctx)
+               i915_gem_object_ggtt_unpin(ctx_obj);
        drm_gem_object_unreference(&ctx_obj->base);
        return ret;
 }

diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index 84bbf1916324a5b06e1052f59d91a4943dc729b7..14b216b9be7f2fb5d33dd01c664e4e96d705d6bb 100644 (file)
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -24,6 +24,8 @@
 #ifndef _INTEL_LRC_H_
 #define _INTEL_LRC_H_
 
+#define GEN8_LR_CONTEXT_ALIGN 4096
+
 /* Execlists regs */
 #define RING_ELSP(ring)                        ((ring)->mmio_base+0x230)
 #define RING_EXECLIST_STATUS(ring)     ((ring)->mmio_base+0x234)
@@ -67,6 +69,8 @@ int intel_lr_context_render_state_init(struct intel_engine_cs *ring,
 void intel_lr_context_free(struct intel_context *ctx);
 int intel_lr_context_deferred_create(struct intel_context *ctx,
                                     struct intel_engine_cs *ring);
+void intel_lr_context_unpin(struct intel_engine_cs *ring,
+               struct intel_context *ctx);
 
 /* Execlists */
 int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists);