From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id AAE5389B48 for ; Tue, 15 Jun 2021 23:03:52 +0000 (UTC) Date: Tue, 15 Jun 2021 16:03:50 -0700 Message-ID: <87bl86u3vt.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210614163704.365989-4-jason@jlekstrand.net> References: <20210614163704.365989-1-jason@jlekstrand.net> <20210614163704.365989-4-jason@jlekstrand.net> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t 03/77] tests/i915/gem_exec_schedule: Convert to intel_ctx_t List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: Jason Ekstrand Cc: igt-dev@lists.freedesktop.org List-ID: On Mon, 14 Jun 2021 09:36:18 -0700, Jason Ekstrand wrote: > > @@ -1166,24 +1188,24 @@ noreorder(int i915, unsigned int engine, int prio, unsigned int flags) > .buffers_ptr = to_user_pointer(&obj), > .buffer_count = 1, > .flags = engine, > - .rsvd1 = gem_context_clone_with_engines(i915, 0), > }; > + intel_ctx_cfg_t vm_cfg = *cfg; > + const intel_ctx_t *ctx; > IGT_CORK_FENCE(cork); > uint32_t *map, *cs; > igt_spin_t *slice; > igt_spin_t *spin; > int fence = -1; > uint64_t addr; > - uint32_t ctx; > > if (flags & CORKED) > fence = igt_cork_plug(&cork, i915); > > - ctx = gem_context_clone(i915, execbuf.rsvd1, > - I915_CONTEXT_CLONE_ENGINES | > - I915_CONTEXT_CLONE_VM, > - 0); > - spin = igt_spin_new(i915, ctx, > + vm_cfg.vm = gem_vm_create(i915); I don't know a lot about this but anyway here goes: Not sure what the purpose of the origin I915_CONTEXT_CLONE_VM was but if we are creating a new VM and adding to the context that is probably not equivalent to cloning the VM. Also, i915_gem_vm_create_ioctl() in i915 has this: if (!HAS_FULL_PPGTT(i915)) return -ENODEV; Considering all this, I'm wondering if we are better off just skipping the gem_vm_create() above (and just use the default VM for the new context)? Since in the new API there will no way to clone a VM, correct? gem_vm_destroy() is also missing at the end of the function. > @@ -2455,8 +2491,10 @@ static void *iova_high(void *arg) > return iova_thread(arg, MAX_PRIO); > } > > -static void test_pi_iova(int i915, unsigned int engine, unsigned int flags) > +static void test_pi_iova(int i915, const intel_ctx_cfg_t *cfg, > + unsigned int engine, unsigned int flags) > { > + intel_ctx_cfg_t ufd_cfg = *cfg; > struct uffdio_api api = { .api = UFFD_API }; > struct uffdio_register reg; > struct uffdio_copy copy; > @@ -2490,9 +2528,12 @@ static void test_pi_iova(int i915, unsigned int engine, unsigned int flags) > igt_require_f(ioctl(ufd, UFFDIO_API, &api) == 0 && api.api == UFFD_API, > "userfaultfd API v%lld:%lld\n", UFFD_API, api.api); > > + if (flags & SHARED) > + ufd_cfg.vm = gem_vm_create(i915); This once again is the same vm create/clone issue described above. Apart from this, everything else seems good afaict and I can give a R-b after we close on the vm_create/clone issue. Thanks! _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev