[Intel-gfx] [PATCH 01/12] drm/i915: Add support for mapping an object page by page

[Chris Wilson]

On Wed, Apr 20, 2016 at 04:47:28PM +0530, ankitprasad.r.sharma at intel.com wrote:
> From: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Introduced a new vm specfic callback insert_page() to program a single pte in
> ggtt or ppgtt. This allows us to map a single page in to the mappable aperture
> space. This can be iterated over to access the whole object by using space as
> meagre as page size.
> 
> v2: Added low level rpm assertions to insert_page routines (Chris)
> 
> v3: Added POSTING_READ post register write (Tvrtko)
> 
> v4: Rebase (Ankit)
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
>  drivers/char/agp/intel-gtt.c        |  9 +++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 67 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.h |  5 +++
>  include/drm/intel-gtt.h             |  3 ++
>  4 files changed, 84 insertions(+)
> 
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index aef87fd..cea0ae3 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -840,6 +840,15 @@ static bool i830_check_flags(unsigned int flags)
>  	return false;
>  }
>  
> +void intel_gtt_insert_page(dma_addr_t addr,
> +			   unsigned int pg,
> +			   unsigned int flags)
> +{
> +	intel_private.driver->write_entry(addr, pg, flags);
> +	wmb();
> +}
> +EXPORT_SYMBOL(intel_gtt_insert_page);
> +
>  void intel_gtt_insert_sg_entries(struct sg_table *st,
>  				 unsigned int pg_start,
>  				 unsigned int flags)
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 9f165fe..da1819d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2353,6 +2353,29 @@ static void gen8_set_pte(void __iomem *addr, gen8_pte_t pte)
>  #endif
>  }
>  
> +static void gen8_ggtt_insert_page(struct i915_address_space *vm,
> +				  dma_addr_t addr,
> +				  uint64_t offset,
> +				  enum i915_cache_level level,
> +				  u32 unused)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(vm->dev);
> +	gen8_pte_t __iomem *pte =
> +		(gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
> +		(offset >> PAGE_SHIFT);
> +	int rpm_atomic_seq;
> +
> +	rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
> +
> +	gen8_set_pte(pte, gen8_pte_encode(addr, level, true));
> +	wmb();
> +
> +	I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> +	POSTING_READ(GFX_FLSH_CNTL_GEN6);

Actually, we don't need these at all for the insert-page API (current
users in pread/pwrite/reloc/gpu-error-capture) as they go through the
GTT and not the GPU TLB. I would much rather we punt these to the caller
with a vm->flush() which we can add later. When I say later, we already
have the GUC pretending to be the GGTT and adding code where it doesn't
belong...

All we need here is the wmb() to order the PTE write (and flush the WCB)
with the use afterwards. The caller has to provide the wmb() to order
its writes before a subsequent PTE change.

In a few patches time we can also kill the rpm_atomic asserts and the
dev_priv local.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre