Re: [PATCH 16/18] drm/i915/context: anonymous context interfaces

[Daniel Vetter <daniel@ffwll.ch>]

On Sun, Mar 18, 2012 at 01:39:56PM -0700, Ben Widawsky wrote:
> Ironlake RC6 needs to allocate a power context object which the hardware
> can automatically switch to. Since the new context code nicely handles
> contexts, create some interfaces so we can hook up the existing code to
> the new code.
> 
> The right way to do this is to move a bunch of code out of
> intel_display.c but I don't feel like doing it at this time. This patch
> is a step in the right direction though.
> 
> CC: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

I've looked through this and assuming I understand things correctly, this
is just to replace the power context on ilk. For the render context stuff
I expect that we can just directly re-use the default context.

Adding up the diffs I don't think the added indirection of these anonymous
contexts over simply using a plain gem object for the powercontext is
worth it. So I suggest we just rip this out.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |    3 +++
>  drivers/gpu/drm/i915/i915_gem_context.c |   40 +++++++++++++++++++++++++++++--
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d49615e..003b62e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -280,6 +280,7 @@ struct i915_hw_ppgtt {
>  
>  /* This must match up with the value previously used for execbuf2.rsvd1. */
>  #define DEFAULT_CONTEXT_ID 0
> +#define ANONYMOUS_CONTEXT_ID 1
>  struct i915_hw_context {
>  	struct drm_i915_file_private *file_priv;
>  	struct kref nref;
> @@ -1316,6 +1317,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file);
>  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  				   struct drm_file *file);
> +struct i915_hw_context *i915_context_alloc_anonymous(struct drm_device *dev);
> +void i915_context_destroy_anonymous(struct i915_hw_context *ctx);
>  
>  /* i915_gem_gtt.c */
>  int __must_check i915_gem_init_aliasing_ppgtt(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index accb3de..de1f3ce 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -122,7 +122,7 @@ again:
>  
>  	spin_lock(&file_priv->context_lock);
>  	ret = idr_get_new_above(&file_priv->context_idr, *ctx_out,
> -				DEFAULT_CONTEXT_ID + 1, &id);
> +				ANONYMOUS_CONTEXT_ID + 1, &id);
>  	if (ret == 0)
>  		(*ctx_out)->id = id;
>  	spin_unlock(&file_priv->context_lock);
> @@ -254,7 +254,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
>  	struct drm_i915_file_private *file_priv = file->driver_priv;
>  	struct i915_hw_context *ctx;
>  
> -	BUG_ON(id == DEFAULT_CONTEXT_ID);
> +	BUG_ON(id == DEFAULT_CONTEXT_ID || id == ANONYMOUS_CONTEXT_ID);
>  	ctx = i915_gem_context_get(file_priv, id);
>  	BUG_ON(ctx == NULL);
>  	kref_put(&ctx->nref, destroy_hw_context);
> @@ -380,6 +380,8 @@ int i915_switch_context(struct intel_ring_buffer *ring,
>  	if (ring != &dev_priv->ring[RCS])
>  		return 0;
>  
> +	BUG_ON(to_id == ANONYMOUS_CONTEXT_ID);
> +
>  	if (file)
>  		file_priv = file->driver_priv;
>  
> @@ -475,3 +477,37 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
>  	return 0;
>  }
> +
> +struct i915_hw_context *
> +i915_context_alloc_anonymous(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct i915_hw_context *ctx;
> +	int ret;
> +
> +	if (dev_priv->hw_contexts_disabled)
> +		return NULL;
> +
> +	ret = create_hw_context(dev, NULL, &ctx);
> +	if (ret)
> +		return NULL;
> +
> +	ctx->id = ANONYMOUS_CONTEXT_ID;
> +	ctx->obj->context_id = ANONYMOUS_CONTEXT_ID;
> +
> +	/* Anonymous contexts are assumed to be always pinned */
> +	ret = i915_gem_object_pin(ctx->obj, CONTEXT_ALIGN, false);
> +	if (ret)
> +		do_destroy(ctx);
> +
> +	return ctx;
> +}
> +
> +void i915_context_destroy_anonymous(struct i915_hw_context *ctx)
> +{
> +	BUG_ON(ctx->id != ANONYMOUS_CONTEXT_ID);
> +	BUG_ON(ctx->obj->context_id != ANONYMOUS_CONTEXT_ID);
> +
> +	i915_gem_object_unpin(ctx->obj);
> +	kref_put(&ctx->nref, destroy_hw_context);
> +}
> -- 
> 1.7.9.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48