On Wed, 2016-04-20 at 13:04 +0100, Chris Wilson wrote:
> On Wed, Apr 20, 2016 at 04:47:28PM +0530, ankitprasad.r.sharma@xxxxxxxxx wrote:
> > From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx>
> > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > ---
> > 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...
I was trying to test the stolen memory patches on SKL with the removed
FLSH_CNTL write. Apparently, the insert_page routine does not seem to be
working. As I was verifying if the stolen-backed object is zeroed out
(using i915_gem_object_clear, which makes use of insert_page) on
creation but it failed (I was getting a garbage value instead of zeros).
And on adding this write it works as expected.
I think this write cannot be removed after all.
Thanks,
Ankit
>
> 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
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
