From 9a3b530455380eed28e7a93121c46d7c334153d9 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 15 Jul 2012 12:34:24 +0100 Subject: [PATCH] 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 Reviewed-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_context.c | 57 +++++++++++++++------------------ 1 file changed, 26 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index da8b01fb1bf8..5d0d6ad489e2 100644 --- 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, -- 2.11.0