drm/i915: Cleanup context switching through do_switch()

When bug hunting, I found the interface to do_switch() overly
complicated and I believe festered the earlier bug. This aims to make
the code a little clearer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index da8b01f..5d0d6ad 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -97,8 +97,7 @@
 
 static struct i915_hw_context *
 i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
-static int do_switch(struct drm_i915_gem_object *from_obj,
-                    struct i915_hw_context *to, u32 seqno);
+static int do_switch(struct i915_hw_context *to);
 
 static int get_context_size(struct drm_device *dev)
 {
@@ -220,19 +219,20 @@ static int create_default_context(struct drm_i915_private *dev_priv)
         */
        dev_priv->ring[RCS].default_context = ctx;
        ret = i915_gem_object_pin(ctx->obj, CONTEXT_ALIGN, false);
-       if (ret) {
-               do_destroy(ctx);
-               return ret;
-       }
+       if (ret)
+               goto err_destroy;
 
-       ret = do_switch(NULL, ctx, 0);
-       if (ret) {
-               i915_gem_object_unpin(ctx->obj);
-               do_destroy(ctx);
-       } else {
-               DRM_DEBUG_DRIVER("Default HW context loaded\n");
-       }
+       ret = do_switch(ctx);
+       if (ret)
+               goto err_unpin;
 
+       DRM_DEBUG_DRIVER("Default HW context loaded\n");
+       return 0;
+
+err_unpin:
+       i915_gem_object_unpin(ctx->obj);
+err_destroy:
+       do_destroy(ctx);
        return ret;
 }
 
@@ -359,17 +359,18 @@ mi_set_context(struct intel_ring_buffer *ring,
        return ret;
 }
 
-static int do_switch(struct drm_i915_gem_object *from_obj,
-                    struct i915_hw_context *to,
-                    u32 seqno)
+static int do_switch(struct i915_hw_context *to)
 {
-       struct intel_ring_buffer *ring = NULL;
+       struct intel_ring_buffer *ring = to->ring;
+       struct drm_i915_gem_object *from_obj = ring->last_context_obj;
        u32 hw_flags = 0;
        int ret;
 
-       BUG_ON(to == NULL);
        BUG_ON(from_obj != NULL && from_obj->pin_count == 0);
 
+       if (from_obj == to->obj)
+               return 0;
+
        ret = i915_gem_object_pin(to->obj, CONTEXT_ALIGN, false);
        if (ret)
                return ret;
@@ -393,7 +394,6 @@ static int do_switch(struct drm_i915_gem_object *from_obj,
        else if (WARN_ON_ONCE(from_obj == to->obj)) /* not yet expected */
                hw_flags |= MI_FORCE_RESTORE;
 
-       ring = to->ring;
        ret = mi_set_context(ring, to, hw_flags);
        if (ret) {
                i915_gem_object_unpin(to->obj);
@@ -407,6 +407,7 @@ static int do_switch(struct drm_i915_gem_object *from_obj,
         * MI_SET_CONTEXT instead of when the next seqno has completed.
         */
        if (from_obj != NULL) {
+               u32 seqno = i915_gem_next_request_seqno(ring);
                from_obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
                i915_gem_object_move_to_active(from_obj, ring, seqno);
                /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the
@@ -417,7 +418,7 @@ static int do_switch(struct drm_i915_gem_object *from_obj,
                 * swapped, but there is no way to do that yet.
                 */
                from_obj->dirty = 1;
-               BUG_ON(from_obj->ring != to->ring);
+               BUG_ON(from_obj->ring != ring);
                i915_gem_object_unpin(from_obj);
 
                drm_gem_object_unreference(&from_obj->base);
@@ -448,10 +449,7 @@ int i915_switch_context(struct intel_ring_buffer *ring,
                        int to_id)
 {
        struct drm_i915_private *dev_priv = ring->dev->dev_private;
-       struct drm_i915_file_private *file_priv = NULL;
        struct i915_hw_context *to;
-       struct drm_i915_gem_object *from_obj = ring->last_context_obj;
-       int ret;
 
        if (dev_priv->hw_contexts_disabled)
                return 0;
@@ -459,21 +457,18 @@ int i915_switch_context(struct intel_ring_buffer *ring,
        if (ring != &dev_priv->ring[RCS])
                return 0;
 
-       if (file)
-               file_priv = file->driver_priv;
-
        if (to_id == DEFAULT_CONTEXT_ID) {
                to = ring->default_context;
        } else {
-               to = i915_gem_context_get(file_priv, to_id);
+               if (file == NULL)
+                       return -EINVAL;
+
+               to = i915_gem_context_get(file->driver_priv, to_id);
                if (to == NULL)
                        return -ENOENT;
        }
 
-       if (from_obj == to->obj)
-               return 0;
-
-       return do_switch(from_obj, to, i915_gem_next_request_seqno(to->ring));
+       return do_switch(to);
 }
 
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,