On Thu, Apr 04, 2013 at 04:41:48PM -0700, Ben Widawsky wrote:
> This allows us to make upcoming refcounting code a bit simpler, and
> cleaner. In addition, I think it makes the interface a bit nicer if the
> caller doesn't need to figure out default contexts and such.
>
> The interface works very similarly to the gem object ref counting, and
> I believe it makes sense to do so as we'll use it in a very similar way
> to objects (we currently use objects as a somewhat hackish way to manage
> context lifecycles).
>
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_context.c | 34 +++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index aa080ea..6211637 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -95,8 +95,6 @@
> */
> #define CONTEXT_ALIGN (64<<10)
>
> -static struct i915_hw_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id);
> static int do_switch(struct i915_hw_context *to);
>
> static int get_context_size(struct drm_device *dev)
> @@ -291,9 +289,18 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
> }
>
> static struct i915_hw_context *
> -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id)
> +i915_gem_context_get(struct intel_ring_buffer *ring,
> + struct drm_i915_file_private *file_priv, u32 id)
> {
> - return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id);
> + struct i915_hw_context *ctx;
> +
> + if (id == DEFAULT_CONTEXT_ID)
> + ctx = ring->default_context;
> + else
> + ctx = (struct i915_hw_context *)
> + idr_find(&file_priv->context_idr, id);
> +
> + return ctx;
> }
>
> static inline int
> @@ -440,16 +447,12 @@ int i915_switch_context(struct intel_ring_buffer *ring,
> if (ring != &dev_priv->ring[RCS])
> return 0;
>
> - if (to_id == DEFAULT_CONTEXT_ID) {
> - to = ring->default_context;
> - } else {
> - if (file == NULL)
> - return -EINVAL;
> + if (file == NULL)
> + return -EINVAL;
This looks wrong. Before the NULL check was only done when to_id !=
DEFAULT_CONTEXT_ID. Now it's done always, which means the call from
i915_gpu_idle() will always fail.
>
> - to = i915_gem_context_get(file->driver_priv, to_id);
> - if (to == NULL)
> - return -ENOENT;
> - }
> + to = i915_gem_context_get(ring, file->driver_priv, to_id);
> + if (to == NULL)
> + return -ENOENT;
>
> return do_switch(to);
> }
> @@ -495,11 +498,14 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> if (!(dev->driver->driver_features & DRIVER_GEM))
> return -ENODEV;
>
> + if (args->ctx_id == DEFAULT_CONTEXT_ID)
> + return -ENOENT;
> +
> ret = i915_mutex_lock_interruptible(dev);
> if (ret)
> return ret;
>
> - ctx = i915_gem_context_get(file_priv, args->ctx_id);
> + ctx = i915_gem_context_get(NULL, file_priv, args->ctx_id);
> if (!ctx) {
> mutex_unlock(&dev->struct_mutex);
> return -ENOENT;
> --
> 1.8.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrj?l?
Intel OTC
