[igt-dev] [PATCH i-g-t v21 26/35] tests/api_intel_bb: Check switching vm in intel-bb

[Chris Wilson]

Quoting Zbigniew Kempczyński (2021-03-01 16:13:54)
> For more vm-controlled scenarios we have to support changing vm
> in the intel-bb. Test verifies allocator is able to provide
> two vm's which can be assigned to used within intel-bb context.
> 
> 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 | 91 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 91 insertions(+)
> 
> diff --git a/tests/i915/api_intel_bb.c b/tests/i915/api_intel_bb.c
> index b62957b34..57a99b360 100644
> --- a/tests/i915/api_intel_bb.c
> +++ b/tests/i915/api_intel_bb.c
> @@ -37,6 +37,7 @@
>  #include <zlib.h>
>  #include "intel_bufops.h"
>  #include "sw_sync.h"
> +#include "i915/gem_vm.h"
>  
>  #define PAGE_SIZE 4096
>  
> @@ -237,6 +238,93 @@ static void bb_with_allocator(struct buf_ops *bops)
>         intel_bb_destroy(ibb);
>  }
>  
> +static void bb_with_vm(struct buf_ops *bops)
> +{
> +       int i915 = buf_ops_get_fd(bops);
> +       struct drm_i915_gem_context_param arg = {
> +               .param = I915_CONTEXT_PARAM_VM,
> +       };
> +       struct intel_bb *ibb;
> +       struct intel_buf *src, *dst, *gap;
> +       uint32_t ctx = 0, vm_id1, vm_id2;
> +       uint64_t prev_vm, vm;
> +       uint64_t src1_addr, dst1_addr;
> +       uint64_t src2_addr, dst2_addr;
> +       uint64_t src3_addr, dst3_addr;
> +       uint64_t src4_addr, dst4_addr;
> +
> +       igt_require(gem_uses_full_ppgtt(i915));
> +
> +       ibb = intel_bb_create_with_allocator(i915, ctx, PAGE_SIZE,
> +                                            INTEL_ALLOCATOR_SIMPLE);
> +       if (debug_bb)
> +               intel_bb_set_debug(ibb, true);
> +
> +       src = intel_buf_create(bops, 4096/32, 32, 8, 0, I915_TILING_NONE,
> +                              I915_COMPRESSION_NONE);
> +       dst = intel_buf_create(bops, 4096/32, 32, 8, 0, I915_TILING_NONE,
> +                              I915_COMPRESSION_NONE);
> +       gap = intel_buf_create(bops, 4096, 128, 8, 0, I915_TILING_NONE,
> +                              I915_COMPRESSION_NONE);
> +
> +       /* vm for second blit */
> +       vm_id1 = gem_vm_create(i915);
> +
> +       /* Get vm_id for default vm */
> +       arg.ctx_id = ctx;
> +       gem_context_get_param(i915, &arg);
> +       vm_id2 = arg.value;
> +
> +       igt_debug("Vm_id1: %u\n", vm_id1);
> +       igt_debug("Vm_id2: %u\n", vm_id2);
> +
> +       /* First blit without set calling setparam */
> +       intel_bb_copy_intel_buf(ibb, dst, src, 4096);
> +       src1_addr = src->addr.offset;
> +       dst1_addr = dst->addr.offset;
> +       igt_debug("step1: src: 0x%llx, dst: 0x%llx\n",
> +                 (long long) src1_addr, (long long) dst1_addr);
> +
> +       /* Open new allocator with vm_id */
> +       vm = intel_allocator_open_vm(i915, vm_id1, INTEL_ALLOCATOR_SIMPLE);
> +       prev_vm = intel_bb_assign_vm(ibb, vm, vm_id1, false);
> +
> +       intel_bb_add_intel_buf(ibb, gap, false);
> +       intel_bb_copy_intel_buf(ibb, dst, src, 4096);
> +       src2_addr = src->addr.offset;
> +       dst2_addr = dst->addr.offset;
> +       igt_debug("step2: src: 0x%llx, dst: 0x%llx\n",
> +                 (long long) src2_addr, (long long) dst2_addr);
> +
> +       /* Back with default vm */
> +       intel_bb_assign_vm(ibb, prev_vm, vm_id2, false);
> +       intel_bb_add_intel_buf(ibb, gap, false);
> +       intel_bb_copy_intel_buf(ibb, dst, src, 4096);
> +       src3_addr = src->addr.offset;
> +       dst3_addr = dst->addr.offset;
> +       igt_debug("step3: src: 0x%llx, dst: 0x%llx\n",
> +                 (long long) src3_addr, (long long) dst3_addr);
> +
> +       /* And exchange one more time */
> +       intel_bb_assign_vm(ibb, vm, vm_id1, false);
> +       intel_bb_copy_intel_buf(ibb, dst, src, 4096);
> +       src4_addr = src->addr.offset;
> +       dst4_addr = dst->addr.offset;
> +       igt_debug("step4: src: 0x%llx, dst: 0x%llx\n",
> +                 (long long) src4_addr, (long long) dst4_addr);
> +
> +       /* Close vm allocator after assigning prev_vm */
> +       intel_bb_assign_vm(ibb, prev_vm, vm_id2, true);

Can we avoid the bool parameter with an extra function to free an
intel_bb vm?

> +
> +       /* Addresses should match for vm and prev_vm blits */
> +       igt_assert_eq(src1_addr, src3_addr);
> +       igt_assert_eq(dst1_addr, dst3_addr);
> +       igt_assert_eq(src2_addr, src4_addr);
> +       igt_assert_eq(dst2_addr, dst4_addr);
> +
> +       intel_bb_destroy(ibb);

Finish with gem_vm_destroy(i915, vm_id1); gem_vm_destroy(i915, vm_id2);
or the ppGTT will be kept until close(i915), and so might interfere with
later tests.

If intel_bb_assign_vm(true) is calling gem_vm_destroy() on the vm_id2,
I'm not keen on the stealing of ownership.
-Chris