[Intel-gfx] [PATCH] drm/i915: Re-enable preallocated top level PDPs support

[Michał Winiarski]

On Tue, Jan 17, 2017 at 10:06:12PM +0800, Zhi Wang wrote:
> After PPGTT page table is able to be shrinken, the preallocated PDPs and
> PDE pages can be freed even they are preallocated under 3-level PPGTT
> mode. This patch re-enables preallocated top level PDPs and PDE pages
> like before.
> 
> Cc: Michał Winiarski <michal.winiarski at intel.com>
> Cc: Michel Thierry <michel.thierry at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> Cc: Zhiyuan Lv <zhiyuan.lv at intel.com>
> Signed-off-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 10 +++++++++-
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c    |  2 +-
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 8aca11f..f0e1992 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -793,12 +793,18 @@ static bool gen8_ppgtt_clear_pdp(struct i915_address_space *vm,
>  	struct i915_hw_ppgtt *ppgtt = i915_vm_to_ppgtt(vm);
>  	struct i915_page_directory *pd;
>  	uint64_t pdpe;
> +	bool pd_is_empty;
>  
>  	gen8_for_each_pdpe(pd, pdp, start, length, pdpe) {
>  		if (WARN_ON(!pdp->page_directory[pdpe]))
>  			break;
>  
> -		if (gen8_ppgtt_clear_pd(vm, pd, start, length)) {
> +		pd_is_empty = gen8_ppgtt_clear_pd(vm, pd, start, length);

Why the extra pd_is_empty variable?
Just adding if (preallocate) continue; here is more readable imho.

We should also assert that we're not in 4-level paging mode when shrinking is
skipped.

With those changes:

Reviewed-by: Michał Winiarski <michal.winiarski at intel.com>

-Michał

> +		/* Do not free pd pages if pdps are preallocated. */
> +		if (ppgtt->preallocate_top_level_pdps)
> +			continue;
> +
> +		if (pd_is_empty) {
>  			__clear_bit(pdpe, pdp->used_pdpes);
>  			gen8_setup_pdpe(ppgtt, pdp, vm->scratch_pd, pdpe);
>  			free_pd(vm->i915, pd);
> @@ -1545,6 +1551,8 @@ static int gen8_preallocate_top_level_pdps(struct i915_hw_ppgtt *ppgtt)
>  
>  	free_gen8_temp_bitmaps(new_page_dirs, new_page_tables);
>  
> +	ppgtt->preallocate_top_level_pdps = true;
> +
>  	return ret;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 34a4fd5..f325cb8 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -349,6 +349,7 @@ struct i915_hw_ppgtt {
>  	struct kref ref;
>  	struct drm_mm_node node;
>  	unsigned long pd_dirty_rings;
> +	bool preallocate_top_level_pdps;
>  	union {
>  		struct i915_pml4 pml4;		/* GEN8+ & 48b PPGTT */
>  		struct i915_page_directory_pointer pdp;	/* GEN8+ */
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index db714dc..766a91a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1488,7 +1488,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	if (req->ctx->ppgtt &&
>  	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
>  		if (!USES_FULL_48BIT_PPGTT(req->i915) &&
> -		    !intel_vgpu_active(req->i915)) {
> +		    !req->ctx->ppgtt->preallocate_top_level_pdps) {
>  			ret = intel_logical_ring_emit_pdps(req);
>  			if (ret)
>  				return ret;
> -- 
> 1.9.1
>