drm/i915: Move legacy kernel context pinning to intel_ringbuffer.c

This is so that we have symmetry with intel_lrc.c and avoid a source of
if (i915.enable_execlists) layering violation within i915_gem_context.c -
that is we move the specific handling of the dev_priv->kernel_context
for legacy submission into the legacy submission code.

This depends upon the init/fini ordering between contexts and engines
already defined by intel_lrc.c, and also exporting the context alignment
required for pinning the legacy context.

v2: Separate out pin/unpin context funcs for greater symmetry with
intel_lrc. One more step towards unifying behaviour between the two
classes of engines and towards fixing another bug in i915_switch_context
vs requests.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1466776558-21516-2-git-send-email-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5b68c07b39217cedfdd1bbf207e2b8876a81f6a6..2b01bbc1fb6e88a6d85e14d0412c76745f56a07e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -872,6 +872,8 @@ struct i915_gem_context {
        u32 user_handle;
 #define CONTEXT_NO_ZEROMAP             (1<<0)
 
+       u32 ggtt_alignment;
+
        struct intel_context {
                struct drm_i915_gem_object *state;
                struct intel_ringbuffer *ringbuf;

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 30d9b4fd30f32b636d304263ea4acb1e14d2eebd..419d2551dc6bc6c5e5c4008f065b1bdd03b0a3ab 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -268,6 +268,8 @@ __create_hw_context(struct drm_device *dev,
        list_add_tail(&ctx->link, &dev_priv->context_list);
        ctx->i915 = dev_priv;
 
+       ctx->ggtt_alignment = get_context_alignment(dev_priv);
+
        if (dev_priv->hw_context_size) {
                struct drm_i915_gem_object *obj =
                                i915_gem_alloc_context_obj(dev, dev_priv->hw_context_size);
@@ -451,26 +453,6 @@ int i915_gem_context_init(struct drm_device *dev)
                return PTR_ERR(ctx);
        }
 
-       if (!i915.enable_execlists && ctx->engine[RCS].state) {
-               int ret;
-
-               /* We may need to do things with the shrinker which
-                * require us to immediately switch back to the default
-                * context. This can cause a problem as pinning the
-                * default context also requires GTT space which may not
-                * be available. To avoid this we always pin the default
-                * context.
-                */
-               ret = i915_gem_obj_ggtt_pin(ctx->engine[RCS].state,
-                                           get_context_alignment(dev_priv), 0);
-               if (ret) {
-                       DRM_ERROR("Failed to pinned default global context (error %d)\n",
-                                 ret);
-                       i915_gem_context_unreference(ctx);
-                       return ret;
-               }
-       }
-
        dev_priv->kernel_context = ctx;
 
        DRM_DEBUG_DRIVER("%s context support initialized\n",
@@ -507,9 +489,6 @@ void i915_gem_context_fini(struct drm_device *dev)
 
        lockdep_assert_held(&dev->struct_mutex);
 
-       if (!i915.enable_execlists && dctx->engine[RCS].state)
-               i915_gem_object_ggtt_unpin(dctx->engine[RCS].state);
-
        i915_gem_context_unreference(dctx);
        dev_priv->kernel_context = NULL;
 
@@ -759,7 +738,7 @@ static int do_rcs_switch(struct drm_i915_gem_request *req)
 
        /* Trying to pin first makes error handling easier. */
        ret = i915_gem_obj_ggtt_pin(to->engine[RCS].state,
-                                   get_context_alignment(engine->i915),
+                                   to->ggtt_alignment,
                                    0);
        if (ret)
                return ret;

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index fedd270498140945d6b4167c8b2844833b46e2a1..86b80b4727fb2c8c4e533ee4ffeb23a4a780c922 100644 (file)
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2321,6 +2321,47 @@ intel_ringbuffer_free(struct intel_ringbuffer *ring)
        kfree(ring);
 }
 
+static int intel_ring_context_pin(struct i915_gem_context *ctx,
+                                 struct intel_engine_cs *engine)
+{
+       struct intel_context *ce = &ctx->engine[engine->id];
+       int ret;
+
+       lockdep_assert_held(&ctx->i915->dev->struct_mutex);
+
+       if (ce->pin_count++)
+               return 0;
+
+       if (ce->state) {
+               ret = i915_gem_obj_ggtt_pin(ce->state, ctx->ggtt_alignment, 0);
+               if (ret)
+                       goto error;
+       }
+
+       i915_gem_context_reference(ctx);
+       return 0;
+
+error:
+       ce->pin_count = 0;
+       return ret;
+}
+
+static void intel_ring_context_unpin(struct i915_gem_context *ctx,
+                                    struct intel_engine_cs *engine)
+{
+       struct intel_context *ce = &ctx->engine[engine->id];
+
+       lockdep_assert_held(&ctx->i915->dev->struct_mutex);
+
+       if (--ce->pin_count)
+               return;
+
+       if (ce->state)
+               i915_gem_object_ggtt_unpin(ce->state);
+
+       i915_gem_context_unreference(ctx);
+}
+
 static int intel_init_ring_buffer(struct drm_device *dev,
                                  struct intel_engine_cs *engine)
 {
@@ -2341,6 +2382,17 @@ static int intel_init_ring_buffer(struct drm_device *dev,
 
        init_waitqueue_head(&engine->irq_queue);
 
+       /* We may need to do things with the shrinker which
+        * require us to immediately switch back to the default
+        * context. This can cause a problem as pinning the
+        * default context also requires GTT space which may not
+        * be available. To avoid this we always pin the default
+        * context.
+        */
+       ret = intel_ring_context_pin(dev_priv->kernel_context, engine);
+       if (ret)
+               goto error;
+
        ringbuf = intel_engine_create_ringbuffer(engine, 32 * PAGE_SIZE);
        if (IS_ERR(ringbuf)) {
                ret = PTR_ERR(ringbuf);
@@ -2408,6 +2460,9 @@ void intel_cleanup_engine(struct intel_engine_cs *engine)
 
        i915_cmd_parser_fini_ring(engine);
        i915_gem_batch_pool_fini(&engine->batch_pool);
+
+       intel_ring_context_unpin(dev_priv->kernel_context, engine);
+
        engine->i915 = NULL;
 }