From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6AF236E39B for ; Fri, 18 Jun 2021 04:55:10 +0000 (UTC) Date: Thu, 17 Jun 2021 21:55:05 -0700 Message-ID: <87h7hvepqu.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: <20210617191256.577244-32-jason@jlekstrand.net> References: <20210617191256.577244-1-jason@jlekstrand.net> <20210617191256.577244-32-jason@jlekstrand.net> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t 31/79] tests/i915/gem_ctx_switch: 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 Thu, 17 Jun 2021 12:12:38 -0700, Jason Ekstrand wrote: > > @@ -127,12 +130,12 @@ static void single(int fd, uint32_t handle, > shared = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0); > igt_assert(shared != MAP_FAILED); > > - for (n = 0; n < 64; n++) { > - if (flags & QUEUE) > - contexts[n] = gem_queue_clone_with_engines(fd, 0); > - else > - contexts[n] = gem_context_clone_with_engines(fd, 0); > - } > + cfg = *base_cfg; > + if (flags & QUEUE) > + cfg.vm = gem_vm_create(fd); As we did in gem_exec_whisper.c and gem_ctx_shared.c, do we also need to handle I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE here? > +static void all(int fd, uint32_t handle, const intel_ctx_cfg_t *base_cfg, > + unsigned flags, int timeout) > { > struct drm_i915_gem_execbuffer2 execbuf; > struct drm_i915_gem_exec_object2 obj[2]; > struct intel_engine_data engines = { }; > - uint32_t contexts[65]; > + intel_ctx_cfg_t cfg; > + const intel_ctx_t *contexts[65]; > int n, qlen; > > - engines = intel_init_engine_list(fd, 0); > + engines = intel_engine_list_for_ctx_cfg(fd, base_cfg); > igt_require(engines.nengines); > > - for (n = 0; n < ARRAY_SIZE(contexts); n++) { > - if (flags & QUEUE) > - contexts[n] = gem_queue_clone_with_engines(fd, 0); > - else > - contexts[n] = gem_context_clone_with_engines(fd, 0); > - } > + cfg = *base_cfg; > + if (flags & QUEUE) > + cfg.vm = gem_vm_create(fd); I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE? > @@ -244,13 +249,13 @@ static void all(int fd, uint32_t handle, unsigned flags, int timeout) > memset(&execbuf, 0, sizeof(execbuf)); > execbuf.buffers_ptr = to_user_pointer(obj + 1); > execbuf.buffer_count = 1; > - execbuf.rsvd1 = contexts[0]; > + execbuf.rsvd1 = contexts[0]->id; > execbuf.flags |= I915_EXEC_HANDLE_LUT; > execbuf.flags |= I915_EXEC_NO_RELOC; > igt_require(__gem_execbuf(fd, &execbuf) == 0); > gem_sync(fd, handle); > > - qlen = measure_qlen(fd, &execbuf, &engines, timeout); > + qlen = measure_qlen(fd, base_cfg, &execbuf, &engines, timeout); Because the cfg in the QUEUE case is different, should we just pass cfg instead of base_cfg here? > @@ -315,8 +322,8 @@ igt_main > } phases[] = { > { "", 0, NULL }, > { "-interruptible", INTERRUPTIBLE, NULL }, > - { "-queue", QUEUE, gem_has_queues }, > - { "-queue-interruptible", QUEUE | INTERRUPTIBLE, gem_has_queues }, > + { "-queue", QUEUE, gem_has_vm }, > + { "-queue-interruptible", QUEUE | INTERRUPTIBLE, gem_has_vm }, My suggestion would be to resurrect gem_has_queues() here where gem_has_queues == gem_has_vm && gem_context_has_single_timeline. (In general maybe we can see if we can reinstate the queue concept itself where a queue is a context with a shared VM and shared timeline, though not sure about this because we now have context configs and flags. Anyway this is a general observation, nothing specifically needed in this patch.) _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev