[Intel-gfx] [PATCH 45/46] drm/i915: Exercise manipulate of single pages in the GGTT

[Chris Wilson]

On Wed, Feb 08, 2017 at 12:25:55PM +0000, Matthew Auld wrote:
> On 2 February 2017 at 09:09, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > Move a single page of an object around within the GGTT and check
> > coherency of writes and reads.
> >
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 91 +++++++++++++++++++++++++++
> >  1 file changed, 91 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > index 7ec6fb2208a6..27e380a3bae5 100644
> > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> > @@ -684,6 +684,96 @@ static int igt_ggtt_drunk(void *arg)
> >         return exercise_ggtt(arg, drunk_hole);
> >  }
> >
> > +static int igt_ggtt_page(void *arg)
> > +{
> > +       const unsigned int count = PAGE_SIZE/sizeof(u32);
> > +       I915_RND_STATE(prng);
> > +       struct drm_i915_private *i915 = arg;
> > +       struct i915_ggtt *ggtt = &i915->ggtt;
> > +       struct drm_i915_gem_object *obj;
> > +       struct drm_mm_node tmp;
> > +       unsigned int *order, n;
> > +       int err;
> > +
> > +       mutex_lock(&i915->drm.struct_mutex);
> > +
> > +       obj = i915_gem_object_create_internal(i915, PAGE_SIZE);
> > +       if (IS_ERR(obj)) {
> > +               err = PTR_ERR(obj);
> > +               goto out_unlock;
> > +       }
> > +
> > +       err = i915_gem_object_pin_pages(obj);
> > +       if (err)
> > +               goto out_free;
> > +
> > +       memset(&tmp, 0, sizeof(tmp));
> > +       err = drm_mm_insert_node_in_range(&ggtt->base.mm, &tmp,
> > +                                         1024 * PAGE_SIZE, 0,
> > +                                         I915_COLOR_UNEVICTABLE,
> > +                                         0, ggtt->mappable_end,
> > +                                         DRM_MM_INSERT_LOW);
> > +       if (err)
> > +               goto out_unpin;
> > +
> > +       order = i915_random_order(count, &prng);
> > +       if (!order) {
> > +               err = -ENOMEM;
> > +               goto out_remove;
> > +       }
> > +
> > +       for (n = 0; n < count; n++) {
> > +               u64 offset = tmp.start + order[n] * PAGE_SIZE;
> > +               u32 __iomem *vaddr;
> > +
> > +               ggtt->base.insert_page(&ggtt->base,
> > +                                      i915_gem_object_get_dma_address(obj, 0),
> > +                                      offset, I915_CACHE_NONE, 0);
> Forgive my ignorance but don't we need a write barrier here also?

After the insert, no. That's provided by insert_page() itself, so writes
cannot overtake the setup of the PTE. But we don't put a barrier before
insert_pages(), which is left to the caller to provide. It feels
inconsistent and we probably should just play safe and put all the
barriers inside the PTE maniupation routines. Mistakes are very hard to
detect - the easiest seem to be around relocation, where a wrong value
seen by the GPU can lead to a GPU hang.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre