Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
On Thu, Jul 03, 2014 at 12:01:35PM +, Mateo Lozano, Oscar wrote: -Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Thursday, July 03, 2014 10:53 AM To: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle On Thu, Jul 03, 2014 at 10:30:52AM +0100, Chris Wilson wrote: On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com Let's go whole hog here and call it ctx-user_handle. ACK And it's unsigned and only 32bits... ACK, I´ll change the type to unit32_t Aside when resending please pull in all the r-b tags from Jesse so that lazy me has less hassle when merging this ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com Let's go whole hog here and call it ctx-user_handle. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
-Original Message- From: Chris Wilson [mailto:ch...@chris-wilson.co.uk] Sent: Thursday, July 03, 2014 10:53 AM To: Mateo Lozano, Oscar; intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle On Thu, Jul 03, 2014 at 10:30:52AM +0100, Chris Wilson wrote: On Thu, Jun 26, 2014 at 02:24:15PM +0100, oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com Let's go whole hog here and call it ctx-user_handle. ACK And it's unsigned and only 32bits... ACK, I´ll change the type to unit32_t ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
-Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Monday, June 30, 2014 9:54 PM To: Mateo Lozano, Oscar Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle On Thu, 26 Jun 2014 14:24:15 +0100 oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com --- 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
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
On Tue, 1 Jul 2014 15:46:51 + Mateo Lozano, Oscar oscar.ma...@intel.com wrote: -Original Message- From: Jesse Barnes [mailto:jbar...@virtuousgeek.org] Sent: Monday, June 30, 2014 9:54 PM To: Mateo Lozano, Oscar Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle On Thu, 26 Jun 2014 14:24:15 +0100 oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com --- 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
Re: [Intel-gfx] [PATCH 4/8] drm/i915: Rename ctx-id to ctx-handle
On Thu, 26 Jun 2014 14:24:15 +0100 oscar.ma...@intel.com wrote: From: Oscar Mateo oscar.ma...@intel.com 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.ma...@intel.com --- 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 =