TODO from irc: change create_hw_context to return the pointer and use PTR_ERR as appropriate.
On Mon, 4 Jun 2012 14:42:43 -0700 Ben Widawsky <[email protected]> wrote: > Invent an abstraction for a hw context which is passed around through > the core functions. The main bit a hw context holds is the buffer object > which backs the context. The rest of the members are just helper > functions. Specifically the ring member, which could likely go away if > we decide to never implement whatever other hw context support exists. > > Of note here is the introduction of the 64k alignment constraint for the > BO. If contexts become heavily used, we should consider tweaking this > down to 4k. Until the contexts are merged and tested a bit though, I > think 64k is a nice start (based on docs). > > Since we don't yet switch contexts, there is really not much complexity > here. Creation/destruction works pretty much as one would expect. An idr > is used to generate the context id numbers which are unique per file > descriptor. > > v2: add DRM_DEBUG_DRIVERS to distinguish ENOMEM failures (ben) > convert a BUG_ON to WARN_ON, default destruction is still fatal (ben) > > Signed-off-by: Ben Widawsky <[email protected]> > --- > drivers/gpu/drm/i915/i915_drv.h | 11 +++ > drivers/gpu/drm/i915/i915_gem_context.c | 142 > ++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 + > 3 files changed, 153 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 432b44f..f543679 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -308,6 +308,16 @@ struct i915_hw_ppgtt { > dma_addr_t scratch_page_dma_addr; > }; > > + > +/* This must match up with the value previously used for execbuf2.rsvd1. */ > +#define DEFAULT_CONTEXT_ID 0 > +struct i915_hw_context { > + int id; > + struct drm_i915_file_private *file_priv; > + struct intel_ring_buffer *ring; > + struct drm_i915_gem_object *obj; > +}; > + > enum no_fbc_reason { > FBC_NO_OUTPUT, /* no outputs enabled to compress */ > FBC_STOLEN_TOO_SMALL, /* not enough space to hold compressed buffers */ > @@ -1023,6 +1033,7 @@ struct drm_i915_file_private { > struct spinlock lock; > struct list_head request_list; > } mm; > + struct idr context_idr; > }; > > #define INTEL_INFO(dev) (((struct drm_i915_private *) > (dev)->dev_private)->info) > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index e39808e..2aca002 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -89,6 +89,15 @@ > #include "i915_drm.h" > #include "i915_drv.h" > > +/* This is a HW constraint. The value below is the largest known requirement > + * I've seen in a spec to date, and that was a workaround for a non-shipping > + * part. It should be safe to decrease this, but it's more future proof as > is. > + */ > +#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 get_context_size(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = dev->dev_private; > @@ -111,6 +120,76 @@ static int get_context_size(struct drm_device *dev) > return ret; > } > > +static void do_destroy(struct i915_hw_context *ctx) > +{ > + struct drm_device *dev = ctx->obj->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (ctx->file_priv) > + idr_remove(&ctx->file_priv->context_idr, ctx->id); > + else > + BUG_ON(ctx != dev_priv->ring[RCS].default_context); > + > + drm_gem_object_unreference(&ctx->obj->base); > + kfree(ctx); > +} > + > +static int > +create_hw_context(struct drm_device *dev, > + struct drm_i915_file_private *file_priv, > + struct i915_hw_context **ctx_out) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + int ret, id; > + > + *ctx_out = kzalloc(sizeof(struct drm_i915_file_private), GFP_KERNEL); > + if (*ctx_out == NULL) > + return -ENOMEM; > + > + (*ctx_out)->obj = i915_gem_alloc_object(dev, > + dev_priv->hw_context_size); > + if ((*ctx_out)->obj == NULL) { > + kfree(*ctx_out); > + DRM_DEBUG_DRIVER("Context object allocated failed\n"); > + return -ENOMEM; > + } > + > + /* The ring associated with the context object is handled by the normal > + * object tracking code. We give an initial ring value simple to pass an > + * assertion in the context switch code. > + */ > + (*ctx_out)->ring = &dev_priv->ring[RCS]; > + > + /* Default context will never have a file_priv */ > + if (file_priv == NULL) > + return 0; > + > + (*ctx_out)->file_priv = file_priv; > + > +again: > + if (idr_pre_get(&file_priv->context_idr, GFP_KERNEL) == 0) { > + ret = -ENOMEM; > + DRM_DEBUG_DRIVER("idr allocation failed\n"); > + goto err_out; > + } > + > + ret = idr_get_new_above(&file_priv->context_idr, *ctx_out, > + DEFAULT_CONTEXT_ID + 1, &id); > + if (ret == 0) > + (*ctx_out)->id = id; > + > + if (ret == -EAGAIN) > + goto again; > + else if (ret) > + goto err_out; > + > + return 0; > + > +err_out: > + do_destroy(*ctx_out); > + return ret; > +} > + > /** > * The default context needs to exist per ring that uses contexts. It stores > the > * context state of the GPU for applications that don't utilize HW contexts, > as > @@ -118,7 +197,30 @@ static int get_context_size(struct drm_device *dev) > */ > static int create_default_context(struct drm_i915_private *dev_priv) > { > - return 0; > + struct i915_hw_context *ctx; > + int ret; > + > + BUG_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); > + > + ret = create_hw_context(dev_priv->dev, NULL, > + &dev_priv->ring[RCS].default_context); > + if (ret) > + return 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. > + */ > + ctx = dev_priv->ring[RCS].default_context; > + ret = i915_gem_object_pin(ctx->obj, CONTEXT_ALIGN, false); > + if (ret) { > + do_destroy(ctx); > + return ret; > + } > + > + return ret; > } > > void i915_gem_context_init(struct drm_device *dev) > @@ -130,7 +232,8 @@ void i915_gem_context_init(struct drm_device *dev) > return; > > /* If called from reset, or thaw... we've been here already */ > - if (dev_priv->hw_contexts_disabled) > + if (dev_priv->hw_contexts_disabled || > + dev_priv->ring[RCS].default_context) > return; > > ctx_size = get_context_size(dev); > @@ -156,20 +259,55 @@ void i915_gem_context_fini(struct drm_device *dev) > > if (dev_priv->hw_contexts_disabled) > return; > + > + i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj); > + > + do_destroy(dev_priv->ring[RCS].default_context); > } > > void i915_gem_context_open(struct drm_device *dev, struct drm_file *file) > { > struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_file_private *file_priv = file->driver_priv; > > if (dev_priv->hw_contexts_disabled) > return; > + > + idr_init(&file_priv->context_idr); > +} > + > +static int context_idr_cleanup(int id, void *p, void *data) > +{ > + struct drm_file *file = (struct drm_file *)data; > + struct drm_i915_file_private *file_priv = file->driver_priv; > + struct i915_hw_context *ctx; > + > + BUG_ON(id == DEFAULT_CONTEXT_ID); > + ctx = i915_gem_context_get(file_priv, id); > + if (WARN_ON(ctx == NULL)) > + return -ENXIO; > + > + do_destroy(ctx); > + > + return 0; > } > > void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) > { > struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_i915_file_private *file_priv = file->driver_priv; > > if (dev_priv->hw_contexts_disabled) > return; > + > + mutex_lock(&dev->struct_mutex); > + idr_for_each(&file_priv->context_idr, context_idr_cleanup, file); > + idr_destroy(&file_priv->context_idr); > + mutex_unlock(&dev->struct_mutex); > +} > + > +static __used struct i915_hw_context * > +i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) > +{ > + return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id); > } > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h > b/drivers/gpu/drm/i915/intel_ringbuffer.h > index 55d3da2..bb19bec 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h > @@ -116,6 +116,8 @@ struct intel_ring_buffer { > > wait_queue_head_t irq_queue; > > + struct i915_hw_context *default_context; > + > void *private; > }; > -- Ben Widawsky, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
