From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6D7266E1F6 for ; Tue, 19 Oct 2021 20:40:23 +0000 (UTC) Date: Tue, 19 Oct 2021 13:40:22 -0700 Message-ID: <87lf2oyci1.wl-ashutosh.dixit@intel.com> From: "Dixit, Ashutosh" In-Reply-To: References: <20211019031706.6992-1-ashutosh.dixit@intel.com> MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII Subject: Re: [igt-dev] [PATCH i-g-t] lib/i915: Return actual submission method from gem_submission_method List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" To: John Harrison Cc: igt-dev@lists.freedesktop.org, Tvrtko Ursulin List-ID: On Tue, 19 Oct 2021 11:19:36 -0700, John Harrison wrote: > > On 10/18/2021 20:17, Ashutosh Dixit wrote: > > gem_submission_method() purports to return the currently used submission > > method by the driver, as evidenced by its callers. Therefore remove the > > GEM_SUBMISSION_EXECLISTS flag when GuC submission is detected. > > > > This also fixes gem_has_execlists() to match its description, previously > > gem_has_execlists() would return true even if GuC submission was actually > > being used in the driver. > > The thought occurs that while the behaviour might now match the description > (and the naming of the underlying flag), the function name really matches > the old behaviour. Maybe the function should also be renamed to > gem_has_execlist_submission()? Maybe even rename both to > gem_using_(execlist|guc)_submission() to be properly accurate? > > Or is that just over polishing things? No I agree, might as well change the names too while we are at it. We could probably have both _has_ and _using_ but I am not see much use for _has_ so will probably just keep _using_ as you suggest. > > diff --git a/tests/i915/gem_ctx_shared.c b/tests/i915/gem_ctx_shared.c > > index ea1b5dd1b8c..f50ef13263f 100644 > > --- a/tests/i915/gem_ctx_shared.c > > +++ b/tests/i915/gem_ctx_shared.c > > @@ -159,7 +159,7 @@ static void disjoint_timelines(int i915, const intel_ctx_cfg_t *cfg) > > uint32_t plug; > > uint64_t ahnd; > > - igt_require(gem_has_execlists(i915)); > > + igt_require(gem_has_execlists(i915) || > > gem_has_guc_submission(i915)); > > I believe Tvrtko is suggesting this one should be a different test entirely: > > Gem_ctx_shared/disjoint-timlines could perhaps use gem_uses_ppgtt() (for > create vm) > > and gem_scheduler_enabled() for context re-ordering OK, let me go through the mails and see what I come up with in the next version. Thanks.