[PATCH 1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[PATCH 1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Chris Wilson]

We should we have all the kinks worked out and full-ppgtt now works
reliably on gen7 (Ivybridge, Valleyview/Baytrail and Haswell). If we can
let userspace have full control over their own ppgtt, it makes softpinning
far more effective, in turn making GPU dispatch far more efficient by
virtue of better mm segregation.  On the other hand, switching over to a
different GTT for every client does incur noticeable overhead, but only
for very lightweight tasks.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Matthew Auld <matthew.william.auld@gmail.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Jason Ekstrand <jason.ekstrand@intel.com>
Cc: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index f00c7fbef79e..9bad73332ce7 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -179,13 +179,11 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 		return 0;
 	}
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
-		if (has_full_48bit_ppgtt)
-			return 3;
+	if (has_full_48bit_ppgtt)
+		return 3;
 
-		if (has_full_ppgtt)
-			return 2;
-	}
+	if (has_full_ppgtt)
+		return 2;
 
 	return 1;
 }
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/gtt: Full ppgtt everywhere, no excuses

[Chris Wilson]

We believe we have all the kinks worked out, even for the early
Valleyview devices, for whom we currently disable all ppgtt.

References: 62942ed7279d ("drm/i915/vlv: disable PPGTT on early revs v3")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 9bad73332ce7..cff0e6430994 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -173,12 +173,6 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 		return 0;
 	}
 
-	/* Early VLV doesn't have this */
-	if (IS_VALLEYVIEW(dev_priv) && dev_priv->drm.pdev->revision < 0xb) {
-		DRM_DEBUG_DRIVER("disabling PPGTT on pre-B3 step VLV\n");
-		return 0;
-	}
-
 	if (has_full_48bit_ppgtt)
 		return 3;
 
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere
URL   : https://patchwork.freedesktop.org/series/46685/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
b8b34765e511 drm/i915/gtt: Enable full-ppgtt by default everywhere
0f8a1fa2eccd drm/i915/gtt: Full ppgtt everywhere, no excuses
-:12: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 62942ed7279d ("drm/i915/vlv: disable PPGTT on early revs v3")'
#12: 
References: 62942ed7279d ("drm/i915/vlv: disable PPGTT on early revs v3")

total: 1 errors, 0 warnings, 0 checks, 12 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere
URL   : https://patchwork.freedesktop.org/series/46685/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/gtt: Enable full-ppgtt by default everywhere
Okay!

Commit: drm/i915/gtt: Full ppgtt everywhere, no excuses
-drivers/gpu/drm/i915/i915_gem_gtt.c:996:9: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/i915_gem_gtt.c:996:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:996:9: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gem_gtt.c:996:9: warning: expression using sizeof(void)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere
URL   : https://patchwork.freedesktop.org/series/46685/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4501 -> Patchwork_9687 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46685/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9687 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7567u:       PASS -> DMESG-FAIL (fdo#106560, fdo#106947)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998) +1

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       DMESG-WARN (fdo#105128, fdo#107139) -> PASS

    igt@kms_busy@basic-flip-b:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS +1

    
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139


== Participating hosts (46 -> 42) ==

  Additional (1): fi-cfl-8109u 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4501 -> Patchwork_9687

  CI_DRM_4501: 692d13f7b75baf0bb8c58b9784569c52d68f01e2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4559: 6d341aac2124836443ce74e8e97a4508ac8d5095 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9687: 0f8a1fa2eccd64f7e804829d1ce34f0c547ac418 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0f8a1fa2eccd drm/i915/gtt: Full ppgtt everywhere, no excuses
b8b34765e511 drm/i915/gtt: Enable full-ppgtt by default everywhere

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9687/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere
URL   : https://patchwork.freedesktop.org/series/46685/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4501_full -> Patchwork_9687_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9687_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9687_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9687_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd1:
      shard-kbl:          SKIP -> PASS +1

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_ppgtt@flink-and-close-vma-leak:
      shard-hsw:          SKIP -> PASS +2

    
== Known issues ==

  Here are the changes found in Patchwork_9687_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@plain-flip-fb-recreate-interruptible:
      shard-glk:          PASS -> FAIL (fdo#100368)

    
    ==== Possible fixes ====

    igt@kms_flip@plain-flip-fb-recreate:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@perf_pmu@rc6-runtime-pm-long:
      shard-hsw:          FAIL (fdo#105010) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#105010 https://bugs.freedesktop.org/show_bug.cgi?id=105010


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4501 -> Patchwork_9687

  CI_DRM_4501: 692d13f7b75baf0bb8c58b9784569c52d68f01e2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4559: 6d341aac2124836443ce74e8e97a4508ac8d5095 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9687: 0f8a1fa2eccd64f7e804829d1ce34f0c547ac418 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9687/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Kenneth Graunke]

[-- Attachment #1.1: Type: text/plain, Size: 1805 bytes --]

On Tuesday, July 17, 2018 2:57:50 AM PDT Chris Wilson wrote:
> We should we have all the kinks worked out and full-ppgtt now works
> reliably on gen7 (Ivybridge, Valleyview/Baytrail and Haswell). If we can
> let userspace have full control over their own ppgtt, it makes softpinning
> far more effective, in turn making GPU dispatch far more efficient by
> virtue of better mm segregation.  On the other hand, switching over to a
> different GTT for every client does incur noticeable overhead, but only
> for very lightweight tasks.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Matthew Auld <matthew.william.auld@gmail.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Jason Ekstrand <jason.ekstrand@intel.com>
> Cc: Kenneth Graunke <kenneth@whitecape.org>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index f00c7fbef79e..9bad73332ce7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -179,13 +179,11 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>  		return 0;
>  	}
>  
> -	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
> -		if (has_full_48bit_ppgtt)
> -			return 3;
> +	if (has_full_48bit_ppgtt)
> +		return 3;
>  
> -		if (has_full_ppgtt)
> -			return 2;
> -	}
> +	if (has_full_ppgtt)
> +		return 2;
>  
>  	return 1;
>  }
> 

I'm very glad to see this land, PPGTT is really important for security.
It may also enable us to do more interesting things on Gen7.x.

Acked-by: Kenneth Graunke <kenneth@whitecape.org>

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/gtt: Enable full-ppgtt by default everywhere

[Chris Wilson]

Quoting Kenneth Graunke (2018-07-17 21:02:33)
> On Tuesday, July 17, 2018 2:57:50 AM PDT Chris Wilson wrote:
> > We should we have all the kinks worked out and full-ppgtt now works
> > reliably on gen7 (Ivybridge, Valleyview/Baytrail and Haswell). If we can
> > let userspace have full control over their own ppgtt, it makes softpinning
> > far more effective, in turn making GPU dispatch far more efficient by
> > virtue of better mm segregation.  On the other hand, switching over to a
> > different GTT for every client does incur noticeable overhead, but only
> > for very lightweight tasks.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Matthew Auld <matthew.william.auld@gmail.com>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Jason Ekstrand <jason.ekstrand@intel.com>
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index f00c7fbef79e..9bad73332ce7 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -179,13 +179,11 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
> >               return 0;
> >       }
> >  
> > -     if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
> > -             if (has_full_48bit_ppgtt)
> > -                     return 3;
> > +     if (has_full_48bit_ppgtt)
> > +             return 3;
> >  
> > -             if (has_full_ppgtt)
> > -                     return 2;
> > -     }
> > +     if (has_full_ppgtt)
> > +             return 2;
> >  
> >       return 1;
> >  }
> > 
> 
> I'm very glad to see this land, PPGTT is really important for security.
> It may also enable us to do more interesting things on Gen7.x.
> 
> Acked-by: Kenneth Graunke <kenneth@whitecape.org>

Plonked in. If I timed it right, it should have just missed the 4.19
cutoff, so we have the best part of 6 months to detect any damage.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx