All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: matthew.auld@intel.com
Subject: Re: [PATCH 19/43] drm/i915: Move over to intel_context_lookup()
Date: Wed, 6 Mar 2019 14:40:16 +0000	[thread overview]
Message-ID: <765ec66d-2976-fbc7-0c68-7c7ed85313b1@linux.intel.com> (raw)
In-Reply-To: <20190306142517.22558-20-chris@chris-wilson.co.uk>


On 06/03/2019 14:24, Chris Wilson wrote:
> In preparation for an ever growing number of engines and so ever
> increasing static array of HW contexts within the GEM context, move the
> array over to an rbtree, allocated upon first use.
> 
> Unfortunately, this imposes an rbtree lookup at a few frequent callsites,
> but we should be able to mitigate those by moving over to using the HW
> context as our primary type and so only incur the lookup on the boundary
> with the user GEM context and engines.
> 
> v2: Check for no HW context in guc_stage_desc_init
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                 |   1 +
>   drivers/gpu/drm/i915/gvt/mmio_context.c       |   3 +-
>   drivers/gpu/drm/i915/i915_gem.c               |   9 +-
>   drivers/gpu/drm/i915/i915_gem_context.c       |  91 ++++-----
>   drivers/gpu/drm/i915/i915_gem_context.h       |   1 -
>   drivers/gpu/drm/i915/i915_gem_context_types.h |   7 +-
>   drivers/gpu/drm/i915/i915_gem_execbuffer.c    |   4 +-
>   drivers/gpu/drm/i915/i915_globals.c           |   1 +
>   drivers/gpu/drm/i915/i915_globals.h           |   1 +
>   drivers/gpu/drm/i915/i915_perf.c              |   4 +-
>   drivers/gpu/drm/i915/intel_context.c          | 180 ++++++++++++++++++
>   drivers/gpu/drm/i915/intel_context.h          |  40 +++-
>   drivers/gpu/drm/i915/intel_context_types.h    |   2 +
>   drivers/gpu/drm/i915/intel_engine_cs.c        |   2 +-
>   drivers/gpu/drm/i915/intel_engine_types.h     |   5 +
>   drivers/gpu/drm/i915/intel_guc_ads.c          |   4 +-
>   drivers/gpu/drm/i915/intel_guc_submission.c   |   6 +-
>   drivers/gpu/drm/i915/intel_lrc.c              |  29 ++-
>   drivers/gpu/drm/i915/intel_ringbuffer.c       |  23 ++-
>   drivers/gpu/drm/i915/selftests/mock_context.c |   7 +-
>   drivers/gpu/drm/i915/selftests/mock_engine.c  |   6 +-
>   21 files changed, 324 insertions(+), 102 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/intel_context.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 524cace37d85..60de05f3fa60 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -95,6 +95,7 @@ i915-y += \
>   	  i915_trace_points.o \
>   	  i915_vma.o \
>   	  intel_breadcrumbs.o \
> +	  intel_context.o \
>   	  intel_engine_cs.o \
>   	  intel_hangcheck.o \
>   	  intel_lrc.o \
> diff --git a/drivers/gpu/drm/i915/gvt/mmio_context.c b/drivers/gpu/drm/i915/gvt/mmio_context.c
> index 0209d27fcaf0..f64c76dd11d4 100644
> --- a/drivers/gpu/drm/i915/gvt/mmio_context.c
> +++ b/drivers/gpu/drm/i915/gvt/mmio_context.c
> @@ -494,7 +494,8 @@ static void switch_mmio(struct intel_vgpu *pre,
>   			 * itself.
>   			 */
>   			if (mmio->in_context &&
> -			    !is_inhibit_context(&s->shadow_ctx->__engine[ring_id]))
> +			    !is_inhibit_context(intel_context_lookup(s->shadow_ctx,
> +								     dev_priv->engine[ring_id])))
>   				continue;
>   
>   			if (mmio->mask)
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8d5918b99fc9..71d8ce38f42c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4650,15 +4650,20 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
>   	}
>   
>   	for_each_engine(engine, i915, id) {
> +		struct intel_context *ce;
>   		struct i915_vma *state;
>   		void *vaddr;
>   
> -		GEM_BUG_ON(to_intel_context(ctx, engine)->pin_count);
> +		ce = intel_context_lookup(ctx, engine);
> +		if (!ce)
> +			continue;
>   
> -		state = to_intel_context(ctx, engine)->state;
> +		state = ce->state;
>   		if (!state)
>   			continue;
>   
> +		GEM_BUG_ON(ce->pin_count);
> +
>   		/*
>   		 * As we will hold a reference to the logical state, it will
>   		 * not be torn down with the context, and importantly the
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index bc89d01d338d..f7a075efccad 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -96,7 +96,7 @@
>   
>   #define ALL_L3_SLICES(dev) (1 << NUM_L3_SLICES(dev)) - 1
>   
> -static struct i915_global_context {
> +static struct i915_global_gem_context {
>   	struct i915_global base;
>   	struct kmem_cache *slab_luts;
>   } global;
> @@ -240,7 +240,7 @@ static void release_hw_id(struct i915_gem_context *ctx)
>   
>   static void i915_gem_context_free(struct i915_gem_context *ctx)
>   {
> -	unsigned int n;
> +	struct intel_context *it, *n;
>   
>   	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
>   	GEM_BUG_ON(!i915_gem_context_is_closed(ctx));
> @@ -251,12 +251,8 @@ static void i915_gem_context_free(struct i915_gem_context *ctx)
>   
>   	kfree(ctx->engines);
>   
> -	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++) {
> -		struct intel_context *ce = &ctx->__engine[n];
> -
> -		if (ce->ops)
> -			ce->ops->destroy(ce);
> -	}
> +	rbtree_postorder_for_each_entry_safe(it, n, &ctx->hw_contexts, node)
> +		it->ops->destroy(it);
>   
>   	if (ctx->timeline)
>   		i915_timeline_put(ctx->timeline);
> @@ -363,40 +359,11 @@ static u32 default_desc_template(const struct drm_i915_private *i915,
>   	return desc;
>   }
>   
> -static void intel_context_retire(struct i915_active_request *active,
> -				 struct i915_request *rq)
> -{
> -	struct intel_context *ce =
> -		container_of(active, typeof(*ce), active_tracker);
> -
> -	intel_context_unpin(ce);
> -}
> -
> -void
> -intel_context_init(struct intel_context *ce,
> -		   struct i915_gem_context *ctx,
> -		   struct intel_engine_cs *engine)
> -{
> -	ce->gem_context = ctx;
> -	ce->engine = engine;
> -	ce->ops = engine->cops;
> -
> -	INIT_LIST_HEAD(&ce->signal_link);
> -	INIT_LIST_HEAD(&ce->signals);
> -
> -	/* Use the whole device by default */
> -	ce->sseu = intel_device_default_sseu(ctx->i915);
> -
> -	i915_active_request_init(&ce->active_tracker,
> -				 NULL, intel_context_retire);
> -}
> -
>   static struct i915_gem_context *
>   __create_hw_context(struct drm_i915_private *dev_priv,
>   		    struct drm_i915_file_private *file_priv)
>   {
>   	struct i915_gem_context *ctx;
> -	unsigned int n;
>   	int ret;
>   	int i;
>   
> @@ -411,8 +378,8 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>   	INIT_LIST_HEAD(&ctx->active_engines);
>   	mutex_init(&ctx->mutex);
>   
> -	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++)
> -		intel_context_init(&ctx->__engine[n], ctx, dev_priv->engine[n]);
> +	ctx->hw_contexts = RB_ROOT;
> +	spin_lock_init(&ctx->hw_contexts_lock);
>   
>   	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
>   	INIT_LIST_HEAD(&ctx->handles_list);
> @@ -961,8 +928,6 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915,
>   		struct intel_ring *ring;
>   		struct i915_request *rq;
>   
> -		GEM_BUG_ON(!to_intel_context(i915->kernel_context, engine));
> -
>   		rq = i915_request_alloc(engine, i915->kernel_context);
>   		if (IS_ERR(rq))
>   			return PTR_ERR(rq);
> @@ -1197,9 +1162,13 @@ __i915_gem_context_reconfigure_sseu(struct i915_gem_context *ctx,
>   				    struct intel_engine_cs *engine,
>   				    struct intel_sseu sseu)
>   {
> -	struct intel_context *ce = to_intel_context(ctx, engine);
> +	struct intel_context *ce;
>   	int ret = 0;
>   
> +	ce = intel_context_instance(ctx, engine);
> +	if (IS_ERR(ce))
> +		return PTR_ERR(ce);
> +
>   	GEM_BUG_ON(INTEL_GEN(ctx->i915) < 8);
>   	GEM_BUG_ON(engine->id != RCS0);
>   
> @@ -1633,8 +1602,8 @@ static int create_setparam(struct i915_user_extension __user *ext, void *data)
>   	return ctx_setparam(data, &local.setparam);
>   }
>   
> -static void clone_sseu(struct i915_gem_context *dst,
> -		       struct i915_gem_context *src)
> +static int clone_sseu(struct i915_gem_context *dst,
> +		      struct i915_gem_context *src)
>   {
>   	const struct intel_sseu default_sseu =
>   		intel_device_default_sseu(dst->i915);
> @@ -1645,7 +1614,7 @@ static void clone_sseu(struct i915_gem_context *dst,
>   		struct intel_context *ce;
>   		struct intel_sseu sseu;
>   
> -		ce = to_intel_context(src, engine);
> +		ce = intel_context_lookup(src, engine);
>   		if (!ce)
>   			continue;
>   
> @@ -1653,9 +1622,14 @@ static void clone_sseu(struct i915_gem_context *dst,
>   		if (!memcmp(&sseu, &default_sseu, sizeof(sseu)))
>   			continue;
>   
> -		ce = to_intel_context(dst, engine);
> +		ce = intel_context_instance(dst, engine);
> +		if (IS_ERR(ce))
> +			return PTR_ERR(ce);
> +
>   		ce->sseu = sseu;
>   	}
> +
> +	return 0;
>   }
>   
>   static int clone_engines(struct i915_gem_context *dst,
> @@ -1702,8 +1676,11 @@ static int create_clone(struct i915_user_extension __user *ext, void *data)
>   	if (local.flags & I915_CONTEXT_CLONE_SCHED)
>   		dst->sched = src->sched;
>   
> -	if (local.flags & I915_CONTEXT_CLONE_SSEU)
> -		clone_sseu(dst, src);
> +	if (local.flags & I915_CONTEXT_CLONE_SSEU) {
> +		err = clone_sseu(dst, src);
> +		if (err)
> +			return err;
> +	}
>   
>   	if (local.flags & I915_CONTEXT_CLONE_TIMELINE && src->timeline) {
>   		if (dst->timeline)
> @@ -1856,13 +1833,15 @@ static int get_sseu(struct i915_gem_context *ctx,
>   	if (!engine)
>   		return -EINVAL;
>   
> +	ce = intel_context_instance(ctx, engine);
> +	if (IS_ERR(ce))
> +		return PTR_ERR(ce);
> +
>   	/* Only use for mutex here is to serialize get_param and set_param. */
>   	ret = mutex_lock_interruptible(&ctx->i915->drm.struct_mutex);
>   	if (ret)
>   		return ret;
>   
> -	ce = to_intel_context(ctx, engine);
> -
>   	user_sseu.slice_mask = ce->sseu.slice_mask;
>   	user_sseu.subslice_mask = ce->sseu.subslice_mask;
>   	user_sseu.min_eus_per_subslice = ce->sseu.min_eus_per_subslice;
> @@ -2038,22 +2017,22 @@ int __i915_gem_context_pin_hw_id(struct i915_gem_context *ctx)
>   #include "selftests/i915_gem_context.c"
>   #endif
>   
> -static void i915_global_context_shrink(void)
> +static void i915_global_gem_context_shrink(void)
>   {
>   	kmem_cache_shrink(global.slab_luts);
>   }
>   
> -static void i915_global_context_exit(void)
> +static void i915_global_gem_context_exit(void)
>   {
>   	kmem_cache_destroy(global.slab_luts);
>   }
>   
> -static struct i915_global_context global = { {
> -	.shrink = i915_global_context_shrink,
> -	.exit = i915_global_context_exit,
> +static struct i915_global_gem_context global = { {
> +	.shrink = i915_global_gem_context_shrink,
> +	.exit = i915_global_gem_context_exit,
>   } };
>   
> -int __init i915_global_context_init(void)
> +int __init i915_global_gem_context_init(void)
>   {
>   	global.slab_luts = KMEM_CACHE(i915_lut_handle, 0);
>   	if (!global.slab_luts)
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h
> index ed9a2447bba9..1e670372892c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context.h
> @@ -31,7 +31,6 @@
>   #include "i915_scheduler.h"
>   #include "intel_context.h"
>   #include "intel_device_info.h"
> -#include "intel_ringbuffer.h"
>   
>   struct drm_device;
>   struct drm_file;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context_types.h b/drivers/gpu/drm/i915/i915_gem_context_types.h
> index 16e2518439a3..8a89f3053f73 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context_types.h
> +++ b/drivers/gpu/drm/i915/i915_gem_context_types.h
> @@ -13,10 +13,10 @@
>   #include <linux/kref.h>
>   #include <linux/mutex.h>
>   #include <linux/radix-tree.h>
> +#include <linux/rbtree.h>
>   #include <linux/rcupdate.h>
>   #include <linux/types.h>
>   
> -#include "i915_gem.h" /* I915_NUM_ENGINES */
>   #include "i915_scheduler.h"
>   #include "intel_context_types.h"
>   
> @@ -146,8 +146,9 @@ struct i915_gem_context {
>   
>   	struct i915_sched_attr sched;
>   
> -	/** engine: per-engine logical HW state */
> -	struct intel_context __engine[I915_NUM_ENGINES];
> +	/** hw_contexts: per-engine logical HW state */
> +	struct rb_root hw_contexts;
> +	spinlock_t hw_contexts_lock;
>   
>   	/** ring_size: size for allocating the per-engine ring buffer */
>   	u32 ring_size;
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 4b0af9a8000f..8fc5bbdba9fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -775,8 +775,8 @@ static int eb_wait_for_ring(const struct i915_execbuffer *eb)
>   	 * keeping all of their resources pinned.
>   	 */
>   
> -	ce = to_intel_context(eb->ctx, eb->engine);
> -	if (!ce->ring) /* first use, assume empty! */
> +	ce = intel_context_lookup(eb->ctx, eb->engine);
> +	if (!ce || !ce->ring) /* first use, assume empty! */
>   		return 0;
>   
>   	rq = __eb_wait_for_ring(ce->ring);
> diff --git a/drivers/gpu/drm/i915/i915_globals.c b/drivers/gpu/drm/i915/i915_globals.c
> index 1cf4e8bc8ec6..2f5c72e2a9d1 100644
> --- a/drivers/gpu/drm/i915/i915_globals.c
> +++ b/drivers/gpu/drm/i915/i915_globals.c
> @@ -36,6 +36,7 @@ static void __i915_globals_cleanup(void)
>   static __initconst int (* const initfn[])(void) = {
>   	i915_global_active_init,
>   	i915_global_context_init,
> +	i915_global_gem_context_init,
>   	i915_global_objects_init,
>   	i915_global_request_init,
>   	i915_global_scheduler_init,
> diff --git a/drivers/gpu/drm/i915/i915_globals.h b/drivers/gpu/drm/i915/i915_globals.h
> index a45529022a42..04c1ce107fc0 100644
> --- a/drivers/gpu/drm/i915/i915_globals.h
> +++ b/drivers/gpu/drm/i915/i915_globals.h
> @@ -26,6 +26,7 @@ void i915_globals_exit(void);
>   /* constructors */
>   int i915_global_active_init(void);
>   int i915_global_context_init(void);
> +int i915_global_gem_context_init(void);
>   int i915_global_objects_init(void);
>   int i915_global_request_init(void);
>   int i915_global_scheduler_init(void);
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index a0d145f976ec..e19a89e4df64 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1740,11 +1740,11 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
>   
>   	/* Update all contexts now that we've stalled the submission. */
>   	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
> -		struct intel_context *ce = to_intel_context(ctx, engine);
> +		struct intel_context *ce = intel_context_lookup(ctx, engine);
>   		u32 *regs;
>   
>   		/* OA settings will be set upon first use */
> -		if (!ce->state)
> +		if (!ce || !ce->state)
>   			continue;
>   
>   		regs = i915_gem_object_pin_map(ce->state->obj, map_type);
> diff --git a/drivers/gpu/drm/i915/intel_context.c b/drivers/gpu/drm/i915/intel_context.c
> new file mode 100644
> index 000000000000..d04a1f51a90c
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_context.c
> @@ -0,0 +1,180 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2019 Intel Corporation
> + */
> +
> +#include "i915_drv.h"
> +#include "i915_gem_context.h"
> +#include "i915_globals.h"
> +#include "intel_context.h"
> +#include "intel_ringbuffer.h"
> +
> +static struct i915_global_context {
> +	struct i915_global base;
> +	struct kmem_cache *slab_ce;
> +} global;
> +
> +struct intel_context *intel_context_alloc(void)
> +{
> +	return kmem_cache_zalloc(global.slab_ce, GFP_KERNEL);
> +}
> +
> +void intel_context_free(struct intel_context *ce)
> +{
> +	kmem_cache_free(global.slab_ce, ce);
> +}
> +
> +struct intel_context *
> +intel_context_lookup(struct i915_gem_context *ctx,
> +		     struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce = NULL;
> +	struct rb_node *p;
> +
> +	spin_lock(&ctx->hw_contexts_lock);
> +	p = ctx->hw_contexts.rb_node;
> +	while (p) {
> +		struct intel_context *this =
> +			rb_entry(p, struct intel_context, node);
> +
> +		if (this->engine == engine) {
> +			GEM_BUG_ON(this->gem_context != ctx);
> +			ce = this;
> +			break;
> +		}
> +
> +		if (this->engine < engine)
> +			p = p->rb_right;
> +		else
> +			p = p->rb_left;
> +	}
> +	spin_unlock(&ctx->hw_contexts_lock);
> +
> +	return ce;
> +}
> +
> +struct intel_context *
> +__intel_context_insert(struct i915_gem_context *ctx,
> +		       struct intel_engine_cs *engine,
> +		       struct intel_context *ce)
> +{
> +	struct rb_node **p, *parent;
> +	int err = 0;
> +
> +	spin_lock(&ctx->hw_contexts_lock);
> +
> +	parent = NULL;
> +	p = &ctx->hw_contexts.rb_node;
> +	while (*p) {
> +		struct intel_context *this;
> +
> +		parent = *p;
> +		this = rb_entry(parent, struct intel_context, node);
> +
> +		if (this->engine == engine) {
> +			err = -EEXIST;
> +			ce = this;
> +			break;
> +		}
> +
> +		if (this->engine < engine)
> +			p = &parent->rb_right;
> +		else
> +			p = &parent->rb_left;
> +	}
> +	if (!err) {
> +		rb_link_node(&ce->node, parent, p);
> +		rb_insert_color(&ce->node, &ctx->hw_contexts);
> +	}
> +
> +	spin_unlock(&ctx->hw_contexts_lock);
> +
> +	return ce;
> +}
> +
> +void __intel_context_remove(struct intel_context *ce)
> +{
> +	struct i915_gem_context *ctx = ce->gem_context;
> +
> +	spin_lock(&ctx->hw_contexts_lock);
> +	rb_erase(&ce->node, &ctx->hw_contexts);
> +	spin_unlock(&ctx->hw_contexts_lock);
> +}
> +
> +struct intel_context *
> +intel_context_instance(struct i915_gem_context *ctx,
> +		       struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce, *pos;
> +
> +	ce = intel_context_lookup(ctx, engine);
> +	if (likely(ce))
> +		return ce;
> +
> +	ce = intel_context_alloc();
> +	if (!ce)
> +		return ERR_PTR(-ENOMEM);
> +
> +	intel_context_init(ce, ctx, engine);
> +
> +	pos = __intel_context_insert(ctx, engine, ce);
> +	if (unlikely(pos != ce)) /* Beaten! Use their HW context instead */
> +		intel_context_free(ce);
> +
> +	GEM_BUG_ON(intel_context_lookup(ctx, engine) != pos);
> +	return pos;
> +}
> +
> +static void intel_context_retire(struct i915_active_request *active,
> +				 struct i915_request *rq)
> +{
> +	struct intel_context *ce =
> +		container_of(active, typeof(*ce), active_tracker);
> +
> +	intel_context_unpin(ce);
> +}
> +
> +void
> +intel_context_init(struct intel_context *ce,
> +		   struct i915_gem_context *ctx,
> +		   struct intel_engine_cs *engine)
> +{
> +	ce->gem_context = ctx;
> +	ce->engine = engine;
> +	ce->ops = engine->cops;
> +
> +	INIT_LIST_HEAD(&ce->signal_link);
> +	INIT_LIST_HEAD(&ce->signals);
> +
> +	/* Use the whole device by default */
> +	ce->sseu = intel_device_default_sseu(ctx->i915);
> +
> +	i915_active_request_init(&ce->active_tracker,
> +				 NULL, intel_context_retire);
> +}
> +
> +static void i915_global_context_shrink(void)
> +{
> +	kmem_cache_shrink(global.slab_ce);
> +}
> +
> +static void i915_global_context_exit(void)
> +{
> +	kmem_cache_destroy(global.slab_ce);
> +}
> +
> +static struct i915_global_context global = { {
> +	.shrink = i915_global_context_shrink,
> +	.exit = i915_global_context_exit,
> +} };
> +
> +int __init i915_global_context_init(void)
> +{
> +	global.slab_ce = KMEM_CACHE(intel_context, SLAB_HWCACHE_ALIGN);
> +	if (!global.slab_ce)
> +		return -ENOMEM;
> +
> +	i915_global_register(&global.base);
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_context.h b/drivers/gpu/drm/i915/intel_context.h
> index dd947692bb0b..2c910628acaf 100644
> --- a/drivers/gpu/drm/i915/intel_context.h
> +++ b/drivers/gpu/drm/i915/intel_context.h
> @@ -7,20 +7,46 @@
>   #ifndef __INTEL_CONTEXT_H__
>   #define __INTEL_CONTEXT_H__
>   
> -#include "i915_gem_context_types.h"
>   #include "intel_context_types.h"
>   #include "intel_engine_types.h"
>   
> +struct intel_context *intel_context_alloc(void);
> +void intel_context_free(struct intel_context *ce);
> +
>   void intel_context_init(struct intel_context *ce,
>   			struct i915_gem_context *ctx,
>   			struct intel_engine_cs *engine);
>   
> -static inline struct intel_context *
> -to_intel_context(struct i915_gem_context *ctx,
> -		 const struct intel_engine_cs *engine)
> -{
> -	return &ctx->__engine[engine->id];
> -}
> +/**
> + * intel_context_lookup - Find the matching HW context for this (ctx, engine)
> + * @ctx - the parent GEM context
> + * @engine - the target HW engine
> + *
> + * May return NULL if the HW context hasn't been instantiated (i.e. unused).
> + */
> +struct intel_context *
> +intel_context_lookup(struct i915_gem_context *ctx,
> +		     struct intel_engine_cs *engine);
> +
> +/**
> + * intel_context_instance - Lookup or allocate the HW context for (ctx, engine)
> + * @ctx - the parent GEM context
> + * @engine - the target HW engine
> + *
> + * Returns the existing HW context for this pair of (GEM context, engine), or
> + * allocates and initialises a fresh context. Once allocated, the HW context
> + * remains resident until the GEM context is destroyed.
> + */
> +struct intel_context *
> +intel_context_instance(struct i915_gem_context *ctx,
> +		       struct intel_engine_cs *engine);
> +
> +struct intel_context *
> +__intel_context_insert(struct i915_gem_context *ctx,
> +		       struct intel_engine_cs *engine,
> +		       struct intel_context *ce);
> +void
> +__intel_context_remove(struct intel_context *ce);
>   
>   static inline struct intel_context *
>   intel_context_pin(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
> diff --git a/drivers/gpu/drm/i915/intel_context_types.h b/drivers/gpu/drm/i915/intel_context_types.h
> index 16e1306e9595..857f5c335324 100644
> --- a/drivers/gpu/drm/i915/intel_context_types.h
> +++ b/drivers/gpu/drm/i915/intel_context_types.h
> @@ -8,6 +8,7 @@
>   #define __INTEL_CONTEXT_TYPES__
>   
>   #include <linux/list.h>
> +#include <linux/rbtree.h>
>   #include <linux/types.h>
>   
>   #include "i915_active_types.h"
> @@ -52,6 +53,7 @@ struct intel_context {
>   	struct i915_active_request active_tracker;
>   
>   	const struct intel_context_ops *ops;
> +	struct rb_node node;
>   
>   	/** sseu: Control eu/slice partitioning */
>   	struct intel_sseu sseu;
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 8e326556499e..1937128ea267 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -644,7 +644,7 @@ void intel_engines_set_scheduler_caps(struct drm_i915_private *i915)
>   static void __intel_context_unpin(struct i915_gem_context *ctx,
>   				  struct intel_engine_cs *engine)
>   {
> -	intel_context_unpin(to_intel_context(ctx, engine));
> +	intel_context_unpin(intel_context_lookup(ctx, engine));
>   }
>   
>   struct measure_breadcrumb {
> diff --git a/drivers/gpu/drm/i915/intel_engine_types.h b/drivers/gpu/drm/i915/intel_engine_types.h
> index 4d2c0ba58e6e..6a570e4e51c3 100644
> --- a/drivers/gpu/drm/i915/intel_engine_types.h
> +++ b/drivers/gpu/drm/i915/intel_engine_types.h
> @@ -231,6 +231,11 @@ struct intel_engine_execlists {
>   	 */
>   	u32 *csb_status;
>   
> +	/**
> +	 * @preempt_context: the HW context for injecting preempt-to-idle
> +	 */
> +	struct intel_context *preempt_context;
> +
>   	/**
>   	 * @preempt_complete_status: expected CSB upon completing preemption
>   	 */
> diff --git a/drivers/gpu/drm/i915/intel_guc_ads.c b/drivers/gpu/drm/i915/intel_guc_ads.c
> index c51d558fd431..b4ff0045a605 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ads.c
> @@ -121,8 +121,8 @@ int intel_guc_ads_create(struct intel_guc *guc)
>   	 * to find it. Note that we have to skip our header (1 page),
>   	 * because our GuC shared data is there.
>   	 */
> -	kernel_ctx_vma = to_intel_context(dev_priv->kernel_context,
> -					  dev_priv->engine[RCS0])->state;
> +	kernel_ctx_vma = intel_context_lookup(dev_priv->kernel_context,
> +					      dev_priv->engine[RCS0])->state;
>   	blob->ads.golden_context_lrca =
>   		intel_guc_ggtt_offset(guc, kernel_ctx_vma) + skipped_offset;
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
> index e3afc91f0e7b..4a5727233419 100644
> --- a/drivers/gpu/drm/i915/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/intel_guc_submission.c
> @@ -382,7 +382,7 @@ static void guc_stage_desc_init(struct intel_guc_client *client)
>   	desc->db_id = client->doorbell_id;
>   
>   	for_each_engine_masked(engine, dev_priv, client->engines, tmp) {
> -		struct intel_context *ce = to_intel_context(ctx, engine);
> +		struct intel_context *ce = intel_context_lookup(ctx, engine);
>   		u32 guc_engine_id = engine->guc_id;
>   		struct guc_execlist_context *lrc = &desc->lrc[guc_engine_id];
>   
> @@ -393,7 +393,7 @@ static void guc_stage_desc_init(struct intel_guc_client *client)
>   		 * for now who owns a GuC client. But for future owner of GuC
>   		 * client, need to make sure lrc is pinned prior to enter here.
>   		 */
> -		if (!ce->state)
> +		if (!ce || !ce->state)
>   			break;	/* XXX: continue? */
>   
>   		/*
> @@ -567,7 +567,7 @@ static void inject_preempt_context(struct work_struct *work)
>   					     preempt_work[engine->id]);
>   	struct intel_guc_client *client = guc->preempt_client;
>   	struct guc_stage_desc *stage_desc = __get_stage_desc(client);
> -	struct intel_context *ce = to_intel_context(client->owner, engine);
> +	struct intel_context *ce = intel_context_lookup(client->owner, engine);
>   	u32 data[7];
>   
>   	if (!ce->ring->emit) { /* recreate upon load/resume */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 31cc4a82b928..9231efb7a82d 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -628,8 +628,7 @@ static void port_assign(struct execlist_port *port, struct i915_request *rq)
>   static void inject_preempt_context(struct intel_engine_cs *engine)
>   {
>   	struct intel_engine_execlists *execlists = &engine->execlists;
> -	struct intel_context *ce =
> -		to_intel_context(engine->i915->preempt_context, engine);
> +	struct intel_context *ce = execlists->preempt_context;
>   	unsigned int n;
>   
>   	GEM_BUG_ON(execlists->preempt_complete_status !=
> @@ -1237,19 +1236,24 @@ static void execlists_submit_request(struct i915_request *request)
>   	spin_unlock_irqrestore(&engine->timeline.lock, flags);
>   }
>   
> -static void execlists_context_destroy(struct intel_context *ce)
> +static void __execlists_context_fini(struct intel_context *ce)
>   {
> -	GEM_BUG_ON(ce->pin_count);
> -
> -	if (!ce->state)
> -		return;
> -
>   	intel_ring_free(ce->ring);
>   
>   	GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj));
>   	i915_gem_object_put(ce->state->obj);
>   }
>   
> +static void execlists_context_destroy(struct intel_context *ce)
> +{
> +	GEM_BUG_ON(ce->pin_count);
> +
> +	if (ce->state)
> +		__execlists_context_fini(ce);
> +
> +	intel_context_free(ce);
> +}
> +
>   static void execlists_context_unpin(struct intel_context *ce)
>   {
>   	struct intel_engine_cs *engine;
> @@ -1390,7 +1394,11 @@ static struct intel_context *
>   execlists_context_pin(struct intel_engine_cs *engine,
>   		      struct i915_gem_context *ctx)
>   {
> -	struct intel_context *ce = to_intel_context(ctx, engine);
> +	struct intel_context *ce;
> +
> +	ce = intel_context_instance(ctx, engine);
> +	if (IS_ERR(ce))
> +		return ce;
>   
>   	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
>   	GEM_BUG_ON(!ctx->ppgtt);
> @@ -2441,8 +2449,9 @@ static int logical_ring_init(struct intel_engine_cs *engine)
>   	execlists->preempt_complete_status = ~0u;
>   	if (i915->preempt_context) {
>   		struct intel_context *ce =
> -			to_intel_context(i915->preempt_context, engine);
> +			intel_context_lookup(i915->preempt_context, engine);
>   
> +		execlists->preempt_context = ce;
>   		execlists->preempt_complete_status =
>   			upper_32_bits(ce->lrc_desc);
>   	}
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 78e3c03aab4e..602babbd737f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1349,15 +1349,20 @@ intel_ring_free(struct intel_ring *ring)
>   	kfree(ring);
>   }
>   
> +static void __ring_context_fini(struct intel_context *ce)
> +{
> +	GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj));
> +	i915_gem_object_put(ce->state->obj);
> +}
> +
>   static void ring_context_destroy(struct intel_context *ce)
>   {
>   	GEM_BUG_ON(ce->pin_count);
>   
> -	if (!ce->state)
> -		return;
> +	if (ce->state)
> +		__ring_context_fini(ce);
>   
> -	GEM_BUG_ON(i915_gem_object_is_active(ce->state->obj));
> -	i915_gem_object_put(ce->state->obj);
> +	intel_context_free(ce);
>   }
>   
>   static int __context_pin_ppgtt(struct i915_gem_context *ctx)
> @@ -1552,7 +1557,11 @@ __ring_context_pin(struct intel_engine_cs *engine,
>   static struct intel_context *
>   ring_context_pin(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
>   {
> -	struct intel_context *ce = to_intel_context(ctx, engine);
> +	struct intel_context *ce;
> +
> +	ce = intel_context_instance(ctx, engine);
> +	if (IS_ERR(ce))
> +		return ce;
>   
>   	lockdep_assert_held(&ctx->i915->drm.struct_mutex);
>   
> @@ -1754,8 +1763,8 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
>   		 * placeholder we use to flush other contexts.
>   		 */
>   		*cs++ = MI_SET_CONTEXT;
> -		*cs++ = i915_ggtt_offset(to_intel_context(i915->kernel_context,
> -							  engine)->state) |
> +		*cs++ = i915_ggtt_offset(intel_context_lookup(i915->kernel_context,
> +							      engine)->state) |
>   			MI_MM_SPACE_GTT |
>   			MI_RESTORE_INHIBIT;
>   	}
> diff --git a/drivers/gpu/drm/i915/selftests/mock_context.c b/drivers/gpu/drm/i915/selftests/mock_context.c
> index 5d0ff2293abc..1d6dc2fe36ab 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_context.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_context.c
> @@ -30,7 +30,6 @@ mock_context(struct drm_i915_private *i915,
>   	     const char *name)
>   {
>   	struct i915_gem_context *ctx;
> -	unsigned int n;
>   	int ret;
>   
>   	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
> @@ -41,15 +40,15 @@ mock_context(struct drm_i915_private *i915,
>   	INIT_LIST_HEAD(&ctx->link);
>   	ctx->i915 = i915;
>   
> +	ctx->hw_contexts = RB_ROOT;
> +	spin_lock_init(&ctx->hw_contexts_lock);
> +
>   	INIT_RADIX_TREE(&ctx->handles_vma, GFP_KERNEL);
>   	INIT_LIST_HEAD(&ctx->handles_list);
>   	INIT_LIST_HEAD(&ctx->hw_id_link);
>   	INIT_LIST_HEAD(&ctx->active_engines);
>   	mutex_init(&ctx->mutex);
>   
> -	for (n = 0; n < ARRAY_SIZE(ctx->__engine); n++)
> -		intel_context_init(&ctx->__engine[n], ctx, i915->engine[n]);
> -
>   	ret = i915_gem_context_pin_hw_id(ctx);
>   	if (ret < 0)
>   		goto err_handles;
> diff --git a/drivers/gpu/drm/i915/selftests/mock_engine.c b/drivers/gpu/drm/i915/selftests/mock_engine.c
> index 856ec2706f3e..d9bbb102677f 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_engine.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_engine.c
> @@ -141,9 +141,13 @@ static struct intel_context *
>   mock_context_pin(struct intel_engine_cs *engine,
>   		 struct i915_gem_context *ctx)
>   {
> -	struct intel_context *ce = to_intel_context(ctx, engine);
> +	struct intel_context *ce;
>   	int err = -ENOMEM;
>   
> +	ce = intel_context_instance(ctx, engine);
> +	if (IS_ERR(ce))
> +		return ce;
> +
>   	if (ce->pin_count++)
>   		return ce;
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-03-06 14:40 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-06 14:24 RFC Breaking up GEM struct_mutex for async-pages Chris Wilson
2019-03-06 14:24 ` [PATCH 01/43] drm/i915/selftests: Canonicalise gen8 addresses Chris Wilson
2019-03-06 14:24 ` [PATCH 02/43] drm/i915: Force GPU idle on suspend Chris Wilson
2019-03-07  9:38   ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 03/43] drm/i915/selftests: Improve switch-to-kernel-context checking Chris Wilson
2019-03-07 12:40   ` Tvrtko Ursulin
2019-03-07 13:17     ` Chris Wilson
2019-03-07 13:21       ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 04/43] drm/i915: Do a synchronous switch-to-kernel-context on idling Chris Wilson
2019-03-07 13:07   ` Tvrtko Ursulin
2019-03-07 13:29     ` Chris Wilson
2019-03-07 17:06       ` Tvrtko Ursulin
2019-03-07 22:24         ` Chris Wilson
2019-03-08  6:46           ` Tvrtko Ursulin
2019-03-08  8:59             ` Chris Wilson
2019-03-06 14:24 ` [PATCH 05/43] drm/i915: Refactor common code to load initial power context Chris Wilson
2019-03-07 13:19   ` Tvrtko Ursulin
2019-03-07 22:26     ` Chris Wilson
2019-03-08  6:48       ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 06/43] drm/i915: Reduce presumption of request ordering for barriers Chris Wilson
2019-03-07 17:26   ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 07/43] drm/i915: Remove has-kernel-context Chris Wilson
2019-03-07 17:29   ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 08/43] drm/i915: Introduce the i915_user_extension_method Chris Wilson
2019-03-06 14:24 ` [PATCH 09/43] drm/i915: Track active engines within a context Chris Wilson
2019-03-06 14:24 ` [PATCH 10/43] drm/i915: Introduce a context barrier callback Chris Wilson
2019-03-06 14:24 ` [PATCH 11/43] drm/i915: Create/destroy VM (ppGTT) for use with contexts Chris Wilson
2019-03-06 14:24 ` [PATCH 12/43] drm/i915: Extend CONTEXT_CREATE to set parameters upon construction Chris Wilson
2019-03-06 14:24 ` [PATCH 13/43] drm/i915: Allow contexts to share a single timeline across all engines Chris Wilson
2019-03-06 14:24 ` [PATCH 14/43] drm/i915: Allow userspace to clone contexts on creation Chris Wilson
2019-03-06 14:24 ` [PATCH 15/43] drm/i915: Allow a context to define its set of engines Chris Wilson
2019-03-06 14:24 ` [PATCH 16/43] drm/i915: Extend I915_CONTEXT_PARAM_SSEU to support local ctx->engine[] Chris Wilson
2019-03-06 14:24 ` [PATCH 17/43] drm/i915: Split struct intel_context definition to its own header Chris Wilson
2019-03-06 14:24 ` [PATCH 18/43] drm/i915: Store the intel_context_ops in the intel_engine_cs Chris Wilson
2019-03-06 14:39   ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 19/43] drm/i915: Move over to intel_context_lookup() Chris Wilson
2019-03-06 14:40   ` Tvrtko Ursulin [this message]
2019-03-06 14:24 ` [PATCH 20/43] drm/i915: Make context pinning part of intel_context_ops Chris Wilson
2019-03-06 14:24 ` [PATCH 21/43] drm/i915: Track the pinned kernel contexts on each engine Chris Wilson
2019-03-08  9:26   ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 22/43] drm/i915: Introduce intel_context.pin_mutex for pin management Chris Wilson
2019-03-06 14:43   ` Tvrtko Ursulin
2019-03-06 14:51     ` Chris Wilson
2019-03-06 15:46       ` Tvrtko Ursulin
2019-03-06 14:24 ` [PATCH 23/43] drm/i915: Load balancing across a virtual engine Chris Wilson
2019-03-06 14:24 ` [PATCH 24/43] drm/i915: Extend execution fence to support a callback Chris Wilson
2019-03-06 14:24 ` [PATCH 25/43] drm/i915/execlists: Virtual engine bonding Chris Wilson
2019-03-06 14:25 ` [PATCH 26/43] drm/i915: Allow specification of parallel execbuf Chris Wilson
2019-03-06 14:25 ` [PATCH 27/43] drm/i915/selftests: Check preemption support on each engine Chris Wilson
2019-03-08  9:31   ` Tvrtko Ursulin
2019-03-06 14:25 ` [PATCH 28/43] drm/i915/execlists: Skip direct submission if only lite-restore Chris Wilson
2019-03-06 14:25 ` [PATCH 29/43] drm/i915: Split GEM object type definition to its own header Chris Wilson
2019-03-06 15:46   ` Matthew Auld
2019-03-06 14:25 ` [PATCH 30/43] drm/i915: Pull GEM ioctls interface to its own file Chris Wilson
2019-03-06 15:48   ` Matthew Auld
2019-03-06 14:25 ` [PATCH 31/43] drm/i915: Move object->pages API to i915_gem_object.[ch] Chris Wilson
2019-03-06 16:23   ` Matthew Auld
2019-03-06 16:29     ` Chris Wilson
2019-03-06 14:25 ` [PATCH 32/43] drm/i915: Move shmem object setup to its own file Chris Wilson
2019-03-06 17:05   ` Matthew Auld
2019-03-06 17:24     ` Chris Wilson
2019-03-06 14:25 ` [PATCH 33/43] drm/i915: Move phys objects " Chris Wilson
2019-03-07 12:31   ` Matthew Auld
2019-03-06 14:25 ` [PATCH 34/43] drm/i915: Move mmap and friends " Chris Wilson
2019-03-07 12:42   ` Matthew Auld
2019-03-06 14:25 ` [PATCH 35/43] drm/i915: Move GEM domain management " Chris Wilson
2019-03-07 13:00   ` Matthew Auld
2019-03-06 14:25 ` [PATCH 36/43] drm/i915: Move more GEM objects under gem/ Chris Wilson
2019-03-06 14:25 ` [PATCH 37/43] drm/i915: Pull scatterlist utils out of i915_gem.h Chris Wilson
2019-03-06 14:25 ` [PATCH 38/43] drm/i915: Move GEM object domain management from struct_mutex to local Chris Wilson
2019-03-06 14:25 ` [PATCH 39/43] drm/i915: Move GEM object waiting to its own file Chris Wilson
2019-03-06 14:25 ` [PATCH 40/43] drm/i915: Move GEM object busy checking " Chris Wilson
2019-03-06 14:25 ` [PATCH 41/43] drm/i915: Move GEM client throttling " Chris Wilson
2019-03-06 14:25 ` [PATCH 42/43] drm/i915: Drop the deferred active reference Chris Wilson
2019-03-06 14:25 ` [PATCH 43/43] drm/i915: Move object close under its own lock Chris Wilson
2019-03-06 15:08 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [01/43] drm/i915/selftests: Canonicalise gen8 addresses Patchwork
2019-03-06 15:25 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-03-06 16:19 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=765ec66d-2976-fbc7-0c68-7c7ed85313b1@linux.intel.com \
    --to=tvrtko.ursulin@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=matthew.auld@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.