[igt-dev] [PATCH i-g-t v21 20/35] tests/api_intel_bb: Add subtest to check render batch on the last page

[Zbigniew Kempczyński]

On Fri, Mar 05, 2021 at 02:45:07PM +0000, Chris Wilson wrote:
> Quoting Zbigniew Kempczyński (2021-03-01 16:13:48)
> > Last page (on 48-bit ppgtt) seems to be problematic when full
> > 3D pipeline batch is inserted and executed from it. Try to find out
> > which generations are still prone to hang on it.
> > 
> > Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >  tests/i915/api_intel_bb.c | 74 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 74 insertions(+)
> > 
> > diff --git a/tests/i915/api_intel_bb.c b/tests/i915/api_intel_bb.c
> > index d5ed77314..207a70fab 100644
> > --- a/tests/i915/api_intel_bb.c
> > +++ b/tests/i915/api_intel_bb.c
> > @@ -36,6 +36,7 @@
> >  #include <glib.h>
> >  #include <zlib.h>
> >  #include "intel_bufops.h"
> > +#include "sw_sync.h"
> >  
> >  #define PAGE_SIZE 4096
> >  
> > @@ -1391,6 +1392,76 @@ static void render_ccs(struct buf_ops *bops)
> >         igt_assert_f(fails == 0, "render-ccs fails: %d\n", fails);
> >  }
> >  
> > +static void last_page(struct buf_ops *bops, uint32_t width, uint32_t height)
> > +{
> > +       struct intel_bb *ibb1, *ibb2;
> > +       struct intel_buf src, dst;
> > +       int i915 = buf_ops_get_fd(bops);
> > +       uint32_t devid = intel_get_drm_devid(i915);
> > +       igt_render_copyfunc_t render_copy = NULL;
> > +       uint64_t gtt_size;
> > +       uint64_t ctx;
> > +       int ret;
> > +
> > +       igt_require(gem_uses_full_ppgtt(i915));
> > +       gtt_size = gem_aperture_size(i915);
> > +
> > +       ctx = gem_context_create(i915);
> > +
> > +       ibb1 = intel_bb_create_full(i915, ctx, PAGE_SIZE,
> > +                                  0, gtt_size, INTEL_ALLOCATOR_SIMPLE,
> > +                                  ALLOC_STRATEGY_LOW_TO_HIGH);
> > +
> > +       ibb2 = intel_bb_create_full(i915, 0, PAGE_SIZE,
> > +                                  0, gtt_size, INTEL_ALLOCATOR_SIMPLE,
> > +                                  ALLOC_STRATEGY_HIGH_TO_LOW);
> > +
> > +       if (debug_bb) {
> > +               intel_bb_set_debug(ibb1, true);
> > +               intel_bb_set_debug(ibb2, true);
> > +       }
> > +
> > +       scratch_buf_init(bops, &src, width, height, I915_TILING_NONE,
> > +                        I915_COMPRESSION_NONE);
> > +       scratch_buf_init(bops, &dst, width, height, I915_TILING_NONE,
> > +                        I915_COMPRESSION_NONE);
> > +       scratch_buf_draw_pattern(bops, &src,
> > +                                0, 0, width, height,
> > +                                0, 0, width, height, 0);
> > +
> > +       render_copy = igt_get_render_copyfunc(devid);
> > +       igt_assert(render_copy);
> > +
> > +       render_copy(ibb1,
> > +                   &src,
> > +                   0, 0, width, height,
> > +                   &dst,
> > +                   0, 0);
> > +       gem_sync(i915, dst.handle);
> > +       igt_assert(sync_fence_status(ibb1->fence) == 1);
> 
> If you don't want to mix apis, then
> s/gem_sync/sync_fence_wait(ibb1->fence, -1));
> 
> Also igt_assert_eq(sync_fence_status(ibb1->fence), 1));

Ok, I'll use sync_fence_wait().
> 
> > +       intel_bb_destroy(ibb1);
> > +
> > +       intel_buf_close(bops, &dst);
> > +       scratch_buf_init(bops, &dst, width, height, I915_TILING_NONE,
> > +                        I915_COMPRESSION_NONE);
> > +
> > +       render_copy(ibb2,
> > +                   &src,
> > +                   0, 0, width, height,
> > +                   &dst,
> > +                   0, 0);
> > +
> > +       /* We likely got a hang here, so free resources before assert */
> > +       gem_sync(i915, dst.handle);
> > +       ret = sync_fence_status(ibb2->fence);
> > +
> > +       intel_bb_destroy(ibb2);
> > +       intel_buf_close(bops, &src);
> > +       intel_buf_close(bops, &dst);
> > +
> > +       igt_assert_f(ret == 1, "Batch in last page in rcs leads to hang\n");
> 
> Now I always thought that the position of the state inside the last half
> of the rendercopy batch page would provide sufficient padding to prevent
> the prefetch from crossing the domain boundary.
> 
> If you want to push the test further you would move the aux buffers out
> of the batch page and use execbuf.batch_start_offset to position the
> batch at the very end of the page (and possibly a binary search therein
> to find the failure position).
> -Chris

This is not easy task. 3d pipeline built for rendercopy has hard assumptions
(split 2KB code + 2KB aux data). So we have to change rendercopy pipelines
to allow code + aux separation. I don't think then fail in last page should
be part of allocator series. We exposed the problem there and for allocator
IMO that's enough to limit the ppgtt space.

--
Zbigniew