[Intel-gfx] [PATCH] drm/i915: Use the size/type of address space to make decisions

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Once the address space has been created (using 3 or 4 levels of page
> tables), we should use that to program the appropriate type into the
> contexts. This gives us the flexibility to handle different types of
> address spaces at runtime.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Matthew Auld <matthew.auld at intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 19 ++++++++++++-------
>  drivers/gpu/drm/i915/i915_gem_gtt.h     |  6 ++++++
>  drivers/gpu/drm/i915/i915_reg.h         |  3 ---
>  drivers/gpu/drm/i915/intel_lrc.c        |  6 +++---
>  4 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 0c9acd29c4d1..b72301488a0b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -236,16 +236,20 @@ static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
>  	return 0;
>  }
>  
> -static u32 default_desc_template(const struct drm_i915_private *dev_priv)
> +static u32 default_desc_template(const struct drm_i915_private *i915,
> +				 const struct i915_hw_ppgtt *ppgtt)
>  {
> +	u32 address_mode;
>  	u32 desc;
>  
> -	desc = GEN8_CTX_VALID |
> -		GEN8_CTX_PRIVILEGE |
> -		GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
> -		GEN8_CTX_ADDRESSING_MODE_SHIFT;
> +	desc = GEN8_CTX_VALID | GEN8_CTX_PRIVILEGE;
>  
> -	if (IS_GEN8(dev_priv))
> +	address_mode = INTEL_LEGACY_32B_CONTEXT;
> +	if (ppgtt && i915_vm_is_48bit(&ppgtt->base))
> +		address_mode = INTEL_LEGACY_64B_CONTEXT;
> +	desc |= address_mode << GEN8_CTX_ADDRESSING_MODE_SHIFT;
> +
> +	if (IS_GEN8(i915))
>  		desc |= GEN8_CTX_L3LLC_COHERENT;
>  
>  	/* TODO: WaDisableLiteRestore when we start using semaphore
> @@ -329,7 +333,7 @@ __create_hw_context(struct drm_i915_private *dev_priv,
>  
>  	i915_gem_context_set_bannable(ctx);
>  	ctx->ring_size = 4 * PAGE_SIZE;
> -	ctx->desc_template = default_desc_template(dev_priv);
> +	ctx->desc_template = default_desc_template(dev_priv, NULL);
>  	ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier);
>  
>  	/* GuC requires the ring to be placed above GUC_WOPCM_TOP. If GuC is not
> @@ -387,6 +391,7 @@ i915_gem_create_context(struct drm_i915_private *dev_priv,
>  		}
>  
>  		ctx->ppgtt = ppgtt;
> +		ctx->desc_template = default_desc_template(dev_priv, ppgtt);
>  	}
>  
>  	trace_i915_context_create(ctx);
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 3c5ef5358cef..7e678ce5a9c7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -525,6 +525,12 @@ i915_vm_to_ggtt(struct i915_address_space *vm)
>  	return container_of(vm, struct i915_ggtt, base);
>  }
>  
> +static inline bool
> +i915_vm_is_48bit(const struct i915_address_space *vm)
> +{
> +	return (vm->total - 1) >> 32;

Works but I guess you had your reasons to avoid direct
comparison for size?

Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>

> +}
> +
>  int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv);
>  int i915_ggtt_init_hw(struct drm_i915_private *dev_priv);
>  int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 07b1a2d67653..141a5c1e3895 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3389,9 +3389,6 @@ enum {
>  #define GEN8_CTX_L3LLC_COHERENT (1<<5)
>  #define GEN8_CTX_PRIVILEGE (1<<8)
>  #define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> -#define GEN8_CTX_ADDRESSING_MODE(dev_priv) (USES_FULL_48BIT_PPGTT(dev_priv) ?\
> -				INTEL_LEGACY_64B_CONTEXT : \
> -				INTEL_LEGACY_32B_CONTEXT)
>  
>  #define GEN8_CTX_ID_SHIFT 32
>  #define GEN8_CTX_ID_WIDTH 21
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index e42990b56fa8..b21dbd44045e 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -332,7 +332,7 @@ static u64 execlists_update_context(struct drm_i915_gem_request *rq)
>  	 * PML4 is allocated during ppgtt init, so this is not needed
>  	 * in 48-bit mode.
>  	 */
> -	if (ppgtt && !USES_FULL_48BIT_PPGTT(ppgtt->base.dev))
> +	if (ppgtt && !i915_vm_is_48bit(&ppgtt->base))
>  		execlists_update_context_pdps(ppgtt, reg_state);
>  
>  	return ce->lrc_desc;
> @@ -1447,7 +1447,7 @@ static int gen8_emit_bb_start(struct drm_i915_gem_request *req,
>  	 * not needed in 48-bit.*/
>  	if (req->ctx->ppgtt &&
>  	    (intel_engine_flag(req->engine) & req->ctx->ppgtt->pd_dirty_rings)) {
> -		if (!USES_FULL_48BIT_PPGTT(req->i915) &&
> +		if (!i915_vm_is_48bit(&req->ctx->ppgtt->base) &&
>  		    !intel_vgpu_active(req->i915)) {
>  			ret = intel_logical_ring_emit_pdps(req);
>  			if (ret)
> @@ -2045,7 +2045,7 @@ static void execlists_init_reg_state(u32 *reg_state,
>  	ASSIGN_CTX_REG(reg_state, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(engine, 0),
>  		       0);
>  
> -	if (ppgtt && USES_FULL_48BIT_PPGTT(ppgtt->base.dev)) {
> +	if (ppgtt && i915_vm_is_48bit(&ppgtt->base)) {
>  		/* 64b PPGTT (48bit canonical)
>  		 * PDP0_DESCRIPTOR contains the base address to PML4 and
>  		 * other PDP Descriptors are ignored.
> -- 
> 2.11.0