Hi,
On Thu, 2016-02-11 at 10:50 +0000, Tvrtko Ursulin wrote:
> 
> On 04/02/16 09:30, ankitprasad.r.sha...@intel.com wrote:
> > From: Chris Wilson <ch...@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)
> >
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sha...@intel.com>
> > ---
> >   drivers/char/agp/intel-gtt.c        |  9 +++++
> >   drivers/gpu/drm/i915/i915_gem_gtt.c | 65 
> > +++++++++++++++++++++++++++++++++++++
> >   drivers/gpu/drm/i915/i915_gem_gtt.h |  5 +++
> >   include/drm/intel-gtt.h             |  3 ++
> >   4 files changed, 82 insertions(+)
> >
> > diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> > index 1341a94..7c68576 100644
> > --- a/drivers/char/agp/intel-gtt.c
> > +++ b/drivers/char/agp/intel-gtt.c
> > @@ -838,6 +838,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 715a771..a64018f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2341,6 +2341,28 @@ 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->gtt.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();
> 
> gen8_ggtt_insert_entries does a read-back of the PTE after having 
> written it with a big fat comment talking about how that could be really 
> important. This is not needed in this path?
As per our discussion with Chris, wmb() is faster than doing a memory
access for reading the PTE.
So, I guess a barrier here should be better to keep things in sync.
> 
> > +
> > +   I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> 
> Why is the posting read not required here as in gen8_ggtt_insert_entries?
I agree with this, a POSTING_READ is required.
> 
> > +
> > +   assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
> > +}
> > +
> >   static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> >                                  struct sg_table *st,
> >                                  uint64_t start,
> > @@ -2412,6 +2434,28 @@ static void gen8_ggtt_insert_entries__BKL(struct 
> > i915_address_space *vm,
> >     stop_machine(gen8_ggtt_insert_entries__cb, &arg, NULL);
> >   }
> >
> > +static void gen6_ggtt_insert_page(struct i915_address_space *vm,
> > +                             dma_addr_t addr,
> > +                             uint64_t offset,
> > +                             enum i915_cache_level level,
> > +                             u32 flags)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(vm->dev);
> > +   gen6_pte_t __iomem *pte =
> > +           (gen6_pte_t __iomem *)dev_priv->gtt.gsm +
> > +           (offset >> PAGE_SHIFT);
> > +   int rpm_atomic_seq;
> > +
> > +   rpm_atomic_seq = assert_rpm_atomic_begin(dev_priv);
> > +
> > +   iowrite32(vm->pte_encode(addr, level, true, flags), pte);
> > +   wmb();
> > +
> > +   I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> > +
> > +   assert_rpm_atomic_end(dev_priv, rpm_atomic_seq);
> > +}
> 
> Same questions as for the gen8 version.
> 
> Regards,
> 
> Tvrtko

Thanks,
Ankit

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to