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

[Chris Wilson]

On Thu, Feb 09, 2017 at 06:02:21PM +0200, Mika Kuoppala wrote:
> 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?

It's mixed results from the compiler vs total == BIT_ULL(48).

I think once the dust has settled we probably really want a
4lvl flag, which is smallest of all. So this is a placeholder for the
bigger tidy up to come.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre