Re: [igt-dev] [PATCH i-g-t] lib/i915: Return actual submission method from gem_submission_method

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

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.