[Intel-gfx] [PATCH 06/17] drm/i915: Move aliasing_ppgtt underneath its i915_ggtt

Tvrtko Ursulin tvrtko.ursulin at linux.intel.com
Tue Jul 30 13:58:07 UTC 2019


On 30/07/2019 14:30, Chris Wilson wrote:
> The aliasing_ppgtt provides a PIN_USER alias for the global gtt, so move
> it under the i915_ggtt to simplify later transformations to enable
> intel_context.vm.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>

 From few days back:

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

Regards,

Tvrtko

> ---
>   drivers/gpu/drm/i915/gem/i915_gem_context.c   |  7 +-
>   .../drm/i915/gem/selftests/i915_gem_context.c |  2 +-
>   drivers/gpu/drm/i915/gt/intel_ringbuffer.c    | 69 ++++++++++++-------
>   drivers/gpu/drm/i915/i915_drv.h               |  3 -
>   drivers/gpu/drm/i915/i915_gem_gtt.c           | 36 +++++-----
>   drivers/gpu/drm/i915/i915_gem_gtt.h           |  3 +
>   drivers/gpu/drm/i915/i915_vma.c               |  2 +-
>   7 files changed, 71 insertions(+), 51 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> index ffb59d96d4d8..0f6b0678f548 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> @@ -459,8 +459,7 @@ __create_context(struct drm_i915_private *i915)
>   	i915_gem_context_set_recoverable(ctx);
>   
>   	ctx->ring_size = 4 * PAGE_SIZE;
> -	ctx->desc_template =
> -		default_desc_template(i915, &i915->mm.aliasing_ppgtt->vm);
> +	ctx->desc_template = default_desc_template(i915, NULL);
>   
>   	for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++)
>   		ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES;
> @@ -2258,8 +2257,8 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
>   		args->size = 0;
>   		if (ctx->vm)
>   			args->value = ctx->vm->total;
> -		else if (to_i915(dev)->mm.aliasing_ppgtt)
> -			args->value = to_i915(dev)->mm.aliasing_ppgtt->vm.total;
> +		else if (to_i915(dev)->ggtt.alias)
> +			args->value = to_i915(dev)->ggtt.alias->vm.total;
>   		else
>   			args->value = to_i915(dev)->ggtt.vm.total;
>   		break;
> diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> index db7856f0f31e..bbd17d4b8ffd 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c
> @@ -1190,7 +1190,7 @@ static int igt_ctx_readonly(void *arg)
>   		goto out_unlock;
>   	}
>   
> -	vm = ctx->vm ?: &i915->mm.aliasing_ppgtt->vm;
> +	vm = ctx->vm ?: &i915->ggtt.alias->vm;
>   	if (!vm || !vm->has_read_only) {
>   		err = 0;
>   		goto out_unlock;
> diff --git a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> index 5c7f2fdc5ec3..11afe64cc256 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ringbuffer.c
> @@ -1376,30 +1376,41 @@ static void ring_context_destroy(struct kref *ref)
>   	intel_context_free(ce);
>   }
>   
> -static int __context_pin_ppgtt(struct i915_gem_context *ctx)
> +static struct i915_address_space *vm_alias(struct intel_context *ce)
> +{
> +	struct i915_address_space *vm;
> +
> +	vm = ce->gem_context->vm;
> +	if (!vm)
> +		vm = &ce->engine->gt->ggtt->alias->vm;
> +
> +	return vm;
> +}
> +
> +static int __context_pin_ppgtt(struct intel_context *ce)
>   {
>   	struct i915_address_space *vm;
>   	int err = 0;
>   
> -	vm = ctx->vm ?: &ctx->i915->mm.aliasing_ppgtt->vm;
> +	vm = vm_alias(ce);
>   	if (vm)
>   		err = gen6_ppgtt_pin(i915_vm_to_ppgtt((vm)));
>   
>   	return err;
>   }
>   
> -static void __context_unpin_ppgtt(struct i915_gem_context *ctx)
> +static void __context_unpin_ppgtt(struct intel_context *ce)
>   {
>   	struct i915_address_space *vm;
>   
> -	vm = ctx->vm ?: &ctx->i915->mm.aliasing_ppgtt->vm;
> +	vm = vm_alias(ce);
>   	if (vm)
>   		gen6_ppgtt_unpin(i915_vm_to_ppgtt(vm));
>   }
>   
>   static void ring_context_unpin(struct intel_context *ce)
>   {
> -	__context_unpin_ppgtt(ce->gem_context);
> +	__context_unpin_ppgtt(ce);
>   }
>   
>   static struct i915_vma *
> @@ -1493,7 +1504,7 @@ static int ring_context_pin(struct intel_context *ce)
>   	if (err)
>   		return err;
>   
> -	err = __context_pin_ppgtt(ce->gem_context);
> +	err = __context_pin_ppgtt(ce);
>   	if (err)
>   		goto err_active;
>   
> @@ -1685,7 +1696,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
>   	return 0;
>   }
>   
> -static int remap_l3(struct i915_request *rq, int slice)
> +static int remap_l3_slice(struct i915_request *rq, int slice)
>   {
>   	u32 *cs, *remap_info = rq->i915->l3_parity.remap_info[slice];
>   	int i;
> @@ -1713,15 +1724,34 @@ static int remap_l3(struct i915_request *rq, int slice)
>   	return 0;
>   }
>   
> +static int remap_l3(struct i915_request *rq)
> +{
> +	struct i915_gem_context *ctx = rq->gem_context;
> +	int i, err;
> +
> +	if (!ctx->remap_slice)
> +		return 0;
> +
> +	for (i = 0; i < MAX_L3_SLICES; i++) {
> +		if (!(ctx->remap_slice & BIT(i)))
> +			continue;
> +
> +		err = remap_l3_slice(rq, i);
> +		if (err)
> +			return err;
> +	}
> +
> +	ctx->remap_slice = 0;
> +	return 0;
> +}
> +
>   static int switch_context(struct i915_request *rq)
>   {
>   	struct intel_engine_cs *engine = rq->engine;
> -	struct i915_gem_context *ctx = rq->gem_context;
> -	struct i915_address_space *vm =
> -		ctx->vm ?: &rq->i915->mm.aliasing_ppgtt->vm;
> +	struct i915_address_space *vm = vm_alias(rq->hw_context);
>   	unsigned int unwind_mm = 0;
>   	u32 hw_flags = 0;
> -	int ret, i;
> +	int ret;
>   
>   	GEM_BUG_ON(HAS_EXECLISTS(rq->i915));
>   
> @@ -1765,7 +1795,7 @@ static int switch_context(struct i915_request *rq)
>   		 * as nothing actually executes using the kernel context; it
>   		 * is purely used for flushing user contexts.
>   		 */
> -		if (i915_gem_context_is_kernel(ctx))
> +		if (i915_gem_context_is_kernel(rq->gem_context))
>   			hw_flags = MI_RESTORE_INHIBIT;
>   
>   		ret = mi_set_context(rq, hw_flags);
> @@ -1799,18 +1829,9 @@ static int switch_context(struct i915_request *rq)
>   			goto err_mm;
>   	}
>   
> -	if (ctx->remap_slice) {
> -		for (i = 0; i < MAX_L3_SLICES; i++) {
> -			if (!(ctx->remap_slice & BIT(i)))
> -				continue;
> -
> -			ret = remap_l3(rq, i);
> -			if (ret)
> -				goto err_mm;
> -		}
> -
> -		ctx->remap_slice = 0;
> -	}
> +	ret = remap_l3(rq);
> +	if (ret)
> +		goto err_mm;
>   
>   	return 0;
>   
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b516e6a02b61..3ddeba5c7b8d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -784,9 +784,6 @@ struct i915_gem_mm {
>   	 */
>   	struct vfsmount *gemfs;
>   
> -	/** PPGTT used for aliasing the PPGTT with the GTT */
> -	struct i915_ppgtt *aliasing_ppgtt;
> -
>   	struct notifier_block oom_notifier;
>   	struct notifier_block vmap_notifier;
>   	struct shrinker shrinker;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 423c890d03bf..c3028722d4e3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2446,18 +2446,18 @@ static int aliasing_gtt_bind_vma(struct i915_vma *vma,
>   		pte_flags |= PTE_READ_ONLY;
>   
>   	if (flags & I915_VMA_LOCAL_BIND) {
> -		struct i915_ppgtt *appgtt = i915->mm.aliasing_ppgtt;
> +		struct i915_ppgtt *alias = i915_vm_to_ggtt(vma->vm)->alias;
>   
>   		if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
> -			ret = appgtt->vm.allocate_va_range(&appgtt->vm,
> -							   vma->node.start,
> -							   vma->size);
> +			ret = alias->vm.allocate_va_range(&alias->vm,
> +							  vma->node.start,
> +							  vma->size);
>   			if (ret)
>   				return ret;
>   		}
>   
> -		appgtt->vm.insert_entries(&appgtt->vm, vma, cache_level,
> -					  pte_flags);
> +		alias->vm.insert_entries(&alias->vm, vma,
> +					 cache_level, pte_flags);
>   	}
>   
>   	if (flags & I915_VMA_GLOBAL_BIND) {
> @@ -2485,7 +2485,8 @@ static void aliasing_gtt_unbind_vma(struct i915_vma *vma)
>   	}
>   
>   	if (vma->flags & I915_VMA_LOCAL_BIND) {
> -		struct i915_address_space *vm = &i915->mm.aliasing_ppgtt->vm;
> +		struct i915_address_space *vm =
> +			&i915_vm_to_ggtt(vma->vm)->alias->vm;
>   
>   		vm->clear_range(vm, vma->node.start, vma->size);
>   	}
> @@ -2542,13 +2543,12 @@ static void i915_gtt_color_adjust(const struct drm_mm_node *node,
>   		*end -= I915_GTT_PAGE_SIZE;
>   }
>   
> -static int init_aliasing_ppgtt(struct drm_i915_private *i915)
> +static int init_aliasing_ppgtt(struct i915_ggtt *ggtt)
>   {
> -	struct i915_ggtt *ggtt = &i915->ggtt;
>   	struct i915_ppgtt *ppgtt;
>   	int err;
>   
> -	ppgtt = i915_ppgtt_create(i915);
> +	ppgtt = i915_ppgtt_create(ggtt->vm.i915);
>   	if (IS_ERR(ppgtt))
>   		return PTR_ERR(ppgtt);
>   
> @@ -2567,7 +2567,7 @@ static int init_aliasing_ppgtt(struct drm_i915_private *i915)
>   	if (err)
>   		goto err_ppgtt;
>   
> -	i915->mm.aliasing_ppgtt = ppgtt;
> +	ggtt->alias = ppgtt;
>   
>   	GEM_BUG_ON(ggtt->vm.vma_ops.bind_vma != ggtt_bind_vma);
>   	ggtt->vm.vma_ops.bind_vma = aliasing_gtt_bind_vma;
> @@ -2582,14 +2582,14 @@ static int init_aliasing_ppgtt(struct drm_i915_private *i915)
>   	return err;
>   }
>   
> -static void fini_aliasing_ppgtt(struct drm_i915_private *i915)
> +static void fini_aliasing_ppgtt(struct i915_ggtt *ggtt)
>   {
> -	struct i915_ggtt *ggtt = &i915->ggtt;
> +	struct drm_i915_private *i915 = ggtt->vm.i915;
>   	struct i915_ppgtt *ppgtt;
>   
>   	mutex_lock(&i915->drm.struct_mutex);
>   
> -	ppgtt = fetch_and_zero(&i915->mm.aliasing_ppgtt);
> +	ppgtt = fetch_and_zero(&ggtt->alias);
>   	if (!ppgtt)
>   		goto out;
>   
> @@ -2706,7 +2706,7 @@ int i915_init_ggtt(struct drm_i915_private *i915)
>   		return ret;
>   
>   	if (INTEL_PPGTT(i915) == INTEL_PPGTT_ALIASING) {
> -		ret = init_aliasing_ppgtt(i915);
> +		ret = init_aliasing_ppgtt(&i915->ggtt);
>   		if (ret)
>   			cleanup_init_ggtt(&i915->ggtt);
>   	}
> @@ -2755,7 +2755,7 @@ void i915_ggtt_driver_release(struct drm_i915_private *i915)
>   {
>   	struct pagevec *pvec;
>   
> -	fini_aliasing_ppgtt(i915);
> +	fini_aliasing_ppgtt(&i915->ggtt);
>   
>   	ggtt_cleanup_hw(&i915->ggtt);
>   
> @@ -3588,7 +3588,7 @@ int i915_gem_gtt_reserve(struct i915_address_space *vm,
>   	GEM_BUG_ON(!IS_ALIGNED(size, I915_GTT_PAGE_SIZE));
>   	GEM_BUG_ON(!IS_ALIGNED(offset, I915_GTT_MIN_ALIGNMENT));
>   	GEM_BUG_ON(range_overflows(offset, size, vm->total));
> -	GEM_BUG_ON(vm == &vm->i915->mm.aliasing_ppgtt->vm);
> +	GEM_BUG_ON(vm == &vm->i915->ggtt.alias->vm);
>   	GEM_BUG_ON(drm_mm_node_allocated(node));
>   
>   	node->size = size;
> @@ -3685,7 +3685,7 @@ int i915_gem_gtt_insert(struct i915_address_space *vm,
>   	GEM_BUG_ON(start >= end);
>   	GEM_BUG_ON(start > 0  && !IS_ALIGNED(start, I915_GTT_PAGE_SIZE));
>   	GEM_BUG_ON(end < U64_MAX && !IS_ALIGNED(end, I915_GTT_PAGE_SIZE));
> -	GEM_BUG_ON(vm == &vm->i915->mm.aliasing_ppgtt->vm);
> +	GEM_BUG_ON(vm == &vm->i915->ggtt.alias->vm);
>   	GEM_BUG_ON(drm_mm_node_allocated(node));
>   
>   	if (unlikely(range_overflows(start, size, end)))
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index cea59ef1a365..51274483502e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -394,6 +394,9 @@ struct i915_ggtt {
>   	void __iomem *gsm;
>   	void (*invalidate)(struct i915_ggtt *ggtt);
>   
> +	/** PPGTT used for aliasing the PPGTT with the GTT */
> +	struct i915_ppgtt *alias;
> +
>   	bool do_idle_maps;
>   
>   	int mtrr;
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index ee73baf29415..eb16a1a93bbc 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -104,7 +104,7 @@ vma_create(struct drm_i915_gem_object *obj,
>   	struct rb_node *rb, **p;
>   
>   	/* The aliasing_ppgtt should never be used directly! */
> -	GEM_BUG_ON(vm == &vm->i915->mm.aliasing_ppgtt->vm);
> +	GEM_BUG_ON(vm == &vm->i915->ggtt.alias->vm);
>   
>   	vma = i915_vma_alloc();
>   	if (vma == NULL)
> 


More information about the Intel-gfx mailing list