Patchwork [igt] lib/i915: Update detection of GuC submission

login
register
mail settings
Submitter Chris Wilson
Date Dec. 7, 2017, 9:02 p.m.
Message ID <20171207210259.25986-1-chris@chris-wilson.co.uk>
Download mbox | patch
Permalink /patch/10101001/
State New
Headers show

Comments

Chris Wilson - Dec. 7, 2017, 9:02 p.m.
Recently the kernel has switched to using a combined i915.enable_guc
rather than multiple i915.enable_guc_submission parameters.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 lib/i915/gem_submission.c | 5 +++++
 1 file changed, 5 insertions(+)
Michal Wajdeczko - Dec. 7, 2017, 11:30 p.m.
On Thu, 07 Dec 2017 22:02:59 +0100, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Recently the kernel has switched to using a combined i915.enable_guc
> rather than multiple i915.enable_guc_submission parameters.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>  lib/i915/gem_submission.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
> index 8bff4844f..1414c7138 100644
> --- a/lib/i915/gem_submission.c
> +++ b/lib/i915/gem_submission.c
> @@ -75,6 +75,11 @@ unsigned gem_submission_method(int fd)
>  	if (dir < 0)
>  		return 0;
> +	if (igt_sysfs_get_u32(dir, "enable_guc") & 1) {
> +		flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;

Hmm, maybe I'm missing something, but as modparam represents actual driver
mode of operation where value '1' represents GuC submission mode and as we
no longer support fallback to execlist mode, I'm not sure that we should
turn on the second GEM_SUBMISSION_EXECLISTS bit here.

But since old modparam was interpreted in the similar way,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

> +		goto out;
> +	}
> +
>  	if (igt_sysfs_get_boolean(dir, "enable_guc_submission")) {
>  		flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;
>  		goto out;
Chris Wilson - Dec. 8, 2017, 12:01 a.m.
Quoting Michal Wajdeczko (2017-12-07 23:30:54)
> On Thu, 07 Dec 2017 22:02:59 +0100, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Recently the kernel has switched to using a combined i915.enable_guc
> > rather than multiple i915.enable_guc_submission parameters.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > ---
> >  lib/i915/gem_submission.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
> > index 8bff4844f..1414c7138 100644
> > --- a/lib/i915/gem_submission.c
> > +++ b/lib/i915/gem_submission.c
> > @@ -75,6 +75,11 @@ unsigned gem_submission_method(int fd)
> >       if (dir < 0)
> >               return 0;
> > +     if (igt_sysfs_get_u32(dir, "enable_guc") & 1) {
> > +             flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;
> 
> Hmm, maybe I'm missing something, but as modparam represents actual driver
> mode of operation where value '1' represents GuC submission mode and as we
> no longer support fallback to execlist mode, I'm not sure that we should
> turn on the second GEM_SUBMISSION_EXECLISTS bit here.

Execlists really means ring-per-context / lrc, as opposed to that we are
directly programming ELSP. If you feel creative, GEM_foo_LRC. (In tests
where is matters, it is used to determine if we can rely on having
independent rings between contexts.)
 
> But since old modparam was interpreted in the similar way,

But you were meant to say "time for a GETPARAM!" Which will be a sad day
as it means that GuC represents an ABI break... :-p
-Chris

Patch

diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 8bff4844f..1414c7138 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -75,6 +75,11 @@  unsigned gem_submission_method(int fd)
 	if (dir < 0)
 		return 0;
 
+	if (igt_sysfs_get_u32(dir, "enable_guc") & 1) {
+		flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;
+		goto out;
+	}
+
 	if (igt_sysfs_get_boolean(dir, "enable_guc_submission")) {
 		flags |= GEM_SUBMISSION_GUC | GEM_SUBMISSION_EXECLISTS;
 		goto out;