On Thu, 26 Jun 2014 14:24:15 +0100
oscar.mateo@xxxxxxxxx wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> 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 <oscar.mateo@xxxxxxxxx>
> ---
> 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 <jbarnes@xxxxxxxxxxxxxxxx>
--
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
