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

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

On Mon, 18 Oct 2021 16:39:40 -0700, John Harrison wrote:
>
> On 10/15/2021 17:23, 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.
> >
> > Reported-by: John Harrison <john.c.harrison@intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> > ---
> >   lib/i915/gem_submission.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
> > index 2627b802cfb..b037d04cc4a 100644
> > --- a/lib/i915/gem_submission.c
> > +++ b/lib/i915/gem_submission.c
> > @@ -86,7 +86,7 @@ unsigned gem_submission_method(int fd)
> >		return 0;
> >		if (igt_sysfs_get_u32(dir, "enable_guc") & 1) {
> > -		flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;
> > +		flags |= GEM_SUBMISSION_GUC;
> >		goto out;
> >	}
> >
> Looks good to me, but as per the comments in the other thread, this might
> have unintended side effects. Have you gone through all instances of the
> submission query usages to check how it is used and whether this will break
> something? E.g. if something is explicitly testing for execlist support to
> mean 'something better than ring buffer' then it would now fail.

There are at present just these two instances of gem_has_execlists:

*** tests/i915/gem_ctx_shared.c:
disjoint_timelines[162]        igt_require(gem_has_execlists(i915));

*** tests/i915/gem_watchdog.c:
virtual[225]                   igt_require(gem_has_execlists(i915));

I'll try to see if I can figure out if they will be affected in any
way. Meanwhile since Tvrtko seems to have authored/reviewed these tests, if
he can chime in that would be great :) Thanks!