From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by gabe.freedesktop.org (Postfix) with ESMTPS id 027466E9C8 for ; Thu, 24 Jun 2021 02:34:03 +0000 (UTC) Date: Wed, 23 Jun 2021 19:34:02 -0700 Message-ID: <87lf70j8it.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" References: <20210617191256.577244-32-jason@jlekstrand.net> <20210618164714.629477-1-jason@jlekstrand.net> <87bl83dlq8.wl-ashutosh.dixit@intel.com> <87lf73iy5q.wl-ashutosh.dixit@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Subject: Re: [igt-dev] [PATCH i-g-t] 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 GPU Tools List-ID: Some time ago, Dixit, Ashutosh wrote: > > On Tue, 22 Jun 2021 22:31:14 -0700, Jason Ekstrand wrote: > > > > On Mon, Jun 21, 2021 at 12:28 AM Dixit, Ashutosh > > wrote: > > > > > > On Sun, 20 Jun 2021 20:48:32 -0700, Jason Ekstrand wrote: > > > > > > > > On Fri, Jun 18, 2021 at 2:19 PM Dixit, Ashutosh > > > > wrote: > > > > > > > > > > On Fri, 18 Jun 2021 09:47:14 -0700, Jason Ekstrand wrote: > > > > > > > > > > > > @@ -315,8 +326,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 }, > > > > > > > > > > As I was saying ealier I think here we should replace gem_has_vm with > > > > > gem_has_queues() where: > > > > > > > > > > gem_has_queues == gem_has_vm && gem_context_has_single_timeline > > > > > > > > Oh, I see what you mean now! I've added this locally: > > > > > > > > static bool > > > > has_queues(int fd) > > > > { > > > > return gem_has_vm(fd) && gem_context_has_single_timeline(fd); > > > > } > > > > > > > > and used it instead of gem_has_vm. How's that sound? > > > > > > Thanks, though maybe add to the lib where we added > > > gem_context_has_single_timeline() (lib/i915/gem_context.c) since there are > > > other places too where this would be useful. > > > > I'm a little hesitant to do that since we don't have a quick-and-easy > > "create a queue" thing anymore. It doesn't make sense to me to have a > > central query for something that isn't a central concept, just because > > it exists more than once. My concern here generally has been that the 'queue' concept (a context with a shared VM and shared timeline) which existed previously in IGT has now been lost as a result of these changes. So I was thinking if it's possible to restore it. One way might be to have two functions such as: void intel_ctx_cfg_for_queues(int fd, intel_ctx_cfg_t *cfg) { cfg->vm = gem_vm_create(fd); cfg->flags |= I915_CONTEXT_CREATE_FLAGS_SINGLE_TIMELINE; } bool gem_has_queues(int fd) { return gem_has_vm(fd) && gem_context_has_single_timeline(fd); } And then we could call these from the couple of places where we've implicitly used the queue concept (gem_ctx_switch.c and gem_exec_whisper.c). In any case, just an idea. But the patch has a R-b as is so feel free to apply as is. We can debate and tweak such things after we merge this series. Thanks. > > > > --Jason > > > > > > > > > > > > --Jason > > > > > > > > > But even otherwise this is: > > > > > > > > > > Reviewed-by: Ashutosh Dixit _______________________________________________ igt-dev mailing list igt-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/igt-dev