On Thu, 26 Jun 2014 14:24:15 +0100 [email protected] wrote: > From: Oscar Mateo <[email protected]> > > This is an Execlists preparatory patch, since they make context ID become an > overloaded term: > > - In the software, it was used to distinguish which context userspace was > trying to use. > - In the BSpec, the term is used to describe the 20-bits long field the > hardware uses to it to discriminate the contexts that are submitted to > the ELSP and inform the driver about their current status (via Context > Switch Interrupts and Context Status Buffers). > > Initially, I tried to make the different meanings converge, but it proved > impossible: > > - The software ctx->id is per-filp, while the hardware one needs to be > globally unique. > - Also, we multiplex several backing states objects per intel_context, > and all of them need unique HW IDs. > - I tried adding a per-filp ID and then composing the HW context ID as: > ctx->id + file_priv->id + ring->id, but the fact that the hardware only > uses 20-bits means we have to artificially limit the number of filps or > contexts the userspace can create. > > The ctx->handle bits are done with this Cocci patch (plus manual frobbing > of the struct declaration): > > @@ > struct intel_context c; > @@ > - (c).id > + c.handle > > @@ > struct intel_context *c; > @@ > - (c)->id > + c->handle > > Also, while we are at it, s/DEFAULT_CONTEXT_ID/DEFAULT_CONTEXT_HANDLE. > > Signed-off-by: Oscar Mateo <[email protected]> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 6 +++--- > drivers/gpu/drm/i915/i915_gem_context.c | 12 ++++++------ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/intel_uncore.c | 2 +- > 5 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index d4b8391..7484e22 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1867,7 +1867,7 @@ static int per_file_ctx(int id, void *ptr, void *data) > if (i915_gem_context_is_default(ctx)) > seq_puts(m, " default context:\n"); > else > - seq_printf(m, " context %d:\n", ctx->id); > + seq_printf(m, " context %d:\n", ctx->handle); > ppgtt->debug_dump(ppgtt, m); > > return 0; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 122e942..5d2b6ab 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -584,10 +584,10 @@ struct i915_ctx_hang_stats { > }; > > /* This must match up with the value previously used for execbuf2.rsvd1. */ > -#define DEFAULT_CONTEXT_ID 0 > +#define DEFAULT_CONTEXT_HANDLE 0 > struct intel_context { > struct kref ref; > - int id; > + int handle; > bool rcs_is_initialized; > uint8_t remap_slice; > struct drm_i915_file_private *file_priv; > @@ -2458,7 +2458,7 @@ static inline void i915_gem_context_unreference(struct > intel_context *ctx) > > static inline bool i915_gem_context_is_default(const struct intel_context *c) > { > - return c->id == DEFAULT_CONTEXT_ID; > + return c->handle == DEFAULT_CONTEXT_HANDLE; > } > > int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c > b/drivers/gpu/drm/i915/i915_gem_context.c > index b8b9859..75e903f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -276,14 +276,14 @@ __create_hw_context(struct drm_device *dev, > /* Default context will never have a file_priv */ > if (file_priv != NULL) { > ret = idr_alloc(&file_priv->context_idr, ctx, > - DEFAULT_CONTEXT_ID, 0, GFP_KERNEL); > + DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL); > if (ret < 0) > goto err_out; > } else > - ret = DEFAULT_CONTEXT_ID; > + ret = DEFAULT_CONTEXT_HANDLE; > > ctx->file_priv = file_priv; > - ctx->id = ret; > + ctx->handle = ret; > /* NB: Mark all slices as needing a remap so that when the context first > * loads it will restore whatever remap state already exists. If there > * is no remap info, it will be a NOP. */ > @@ -787,7 +787,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, > void *data, > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > - args->ctx_id = ctx->id; > + args->ctx_id = ctx->handle; > DRM_DEBUG_DRIVER("HW context %d created\n", args->ctx_id); > > return 0; > @@ -801,7 +801,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device > *dev, void *data, > struct intel_context *ctx; > int ret; > > - if (args->ctx_id == DEFAULT_CONTEXT_ID) > + if (args->ctx_id == DEFAULT_CONTEXT_HANDLE) > return -ENOENT; > > ret = i915_mutex_lock_interruptible(dev); > @@ -814,7 +814,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device > *dev, void *data, > return PTR_ERR(ctx); > } > > - idr_remove(&ctx->file_priv->context_idr, ctx->id); > + idr_remove(&ctx->file_priv->context_idr, ctx->handle); > i915_gem_context_unreference(ctx); > mutex_unlock(&dev->struct_mutex); > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index d815ef5..c97178e 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -938,7 +938,7 @@ i915_gem_validate_context(struct drm_device *dev, struct > drm_file *file, > struct intel_context *ctx = NULL; > struct i915_ctx_hang_stats *hs; > > - if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) > + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_HANDLE) > return ERR_PTR(-EINVAL); > > ctx = i915_gem_context_get(file->driver_priv, ctx_id); > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 29145df..e0f0843 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -1010,7 +1010,7 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev, > if (args->flags || args->pad) > return -EINVAL; > > - if (args->ctx_id == DEFAULT_CONTEXT_ID && !capable(CAP_SYS_ADMIN)) > + if (args->ctx_id == DEFAULT_CONTEXT_HANDLE && !capable(CAP_SYS_ADMIN)) > return -EPERM; > > ret = mutex_lock_interruptible(&dev->struct_mutex);
So this handle is just the sw tracking ID right? Would be nice to have a patch on top commenting that in the context struct (well commenting the whole struct really). Reviewed-by: Jesse Barnes <[email protected]> -- Jesse Barnes, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
