Re: [Intel-gfx] [PATCH v12 2/4] drm/i915: Reserve some kernel space per vm

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 20.09.2023 23:07, Jonathan Cavitt wrote:
> Reserve two pages in each vm for kernel space to use for things
> such as workarounds.
> 
> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> Suggested-by: Chris Wilson <chris.p.wilson@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 7 +++++++
>   drivers/gpu/drm/i915/gt/intel_gtt.h  | 1 +
>   2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> index 84aa29715e0ac..6344d733fb2c4 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c
> @@ -230,6 +230,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
>   				     gen8_pd_top_count(vm), vm->top);
>   
>   	free_scratch(vm);
> +	drm_mm_remove_node(&vm->rsvd);
>   }
>   
>   static u64 __gen8_ppgtt_clear(struct i915_address_space * const vm,
> @@ -1014,6 +1015,12 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt,
>   	ppgtt->vm.foreach = gen8_ppgtt_foreach;
>   	ppgtt->vm.cleanup = gen8_ppgtt_cleanup;
>   
> +	ppgtt->vm.rsvd.start = ppgtt->vm.total - (SZ_4K * 2);
> +	ppgtt->vm.rsvd.size = (SZ_4K * 2);
> +	ppgtt->vm.rsvd.color = I915_COLOR_UNEVICTABLE;
> +	GEM_BUG_ON(drm_mm_reserve_node(&ppgtt->vm.mm, &ppgtt->vm.rsvd));
> +	ppgtt->vm.total -= (SZ_4K * 2);

I suspect you shouldn't decrease vm.total, otherwise api_intel_bb CI 
tests will complain and I guess vm.total means total, ie it should 
include reserved nodes as well.
Btw why not use i915_gem_gtt_reserve instead of hardcoding, I do not 
know this helper but it looks like good fit.

Regards
Andrzej


> +
>   	err = gen8_init_scratch(&ppgtt->vm);
>   	if (err)
>   		goto err_put;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 153ddfca0ae18..680ce27dda40c 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -247,6 +247,7 @@ struct i915_address_space {
>   	struct work_struct release_work;
>   
>   	struct drm_mm mm;
> +	struct drm_mm_node rsvd;
>   	struct intel_gt *gt;
>   	struct drm_i915_private *i915;
>   	struct device *dma;