From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <igt-dev-bounces@lists.freedesktop.org>
Received: from mga04.intel.com (mga04.intel.com [192.55.52.120])
 by gabe.freedesktop.org (Postfix) with ESMTPS id 5F6AF6EAB7
 for <igt-dev@lists.freedesktop.org>; Mon, 18 Oct 2021 23:59:45 +0000 (UTC)
Date: Mon, 18 Oct 2021 16:59:43 -0700
Message-ID: <87czo1995c.wl-ashutosh.dixit@intel.com>
From: "Dixit, Ashutosh" <ashutosh.dixit@intel.com>
In-Reply-To: <15873c81-caec-c822-867f-7f0392606126@intel.com>
References: <20211016002334.14580-1-ashutosh.dixit@intel.com>
 <15873c81-caec-c822-867f-7f0392606126@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: <https://lists.freedesktop.org/mailman/options/igt-dev>,
 <mailto:igt-dev-request@lists.freedesktop.org?subject=unsubscribe>
List-Archive: <https://lists.freedesktop.org/archives/igt-dev>
List-Post: <mailto:igt-dev@lists.freedesktop.org>
List-Help: <mailto:igt-dev-request@lists.freedesktop.org?subject=help>
List-Subscribe: <https://lists.freedesktop.org/mailman/listinfo/igt-dev>,
 <mailto:igt-dev-request@lists.freedesktop.org?subject=subscribe>
Errors-To: igt-dev-bounces@lists.freedesktop.org
Sender: "igt-dev" <igt-dev-bounces@lists.freedesktop.org>
To: John Harrison <john.c.harrison@intel.com>
Cc: igt-dev@lists.freedesktop.org, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>, Michal Wajdeczko <michal.wajdeczko@intel.com>
List-ID: <igt-dev@lists.freedesktop.org>

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!