[PATCH 1/2] drm/i915/gtt: Avoid calling non-existent allocate_va_range

[PATCH 1/2] drm/i915/gtt: Avoid calling non-existent allocate_va_range

[Chris Wilson]

On hsw and older, we do not need to allocate the ppgtt on the fly and so
ppgtt->allocate_va_range() is NULL. Fixup ppgtt_bind_vma not to call it,
in that case!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 35 +++++++++++++++++++----------
 1 file changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 21d72f695adb..b454f3489767 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -189,19 +189,30 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 	return 1;
 }
 
-static int ppgtt_bind_vma(struct i915_vma *vma,
-			  enum i915_cache_level cache_level,
-			  u32 unused)
+static int gen8_ppgtt_bind_vma(struct i915_vma *vma,
+			       enum i915_cache_level cache_level,
+			       u32 unused)
 {
-	u32 pte_flags;
 	int ret;
 
-	if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
-		ret = vma->vm->allocate_va_range(vma->vm, vma->node.start,
-						 vma->size);
-		if (ret)
-			return ret;
-	}
+	GEM_BUG_ON(vma->flags & I915_VMA_LOCAL_BIND);
+
+	ret = vma->vm->allocate_va_range(vma->vm, vma->node.start, vma->size);
+	if (ret)
+		return ret;
+
+	vma->vm->insert_entries(vma->vm, vma, cache_level, 0);
+
+	return 0;
+}
+
+static int gen6_ppgtt_bind_vma(struct i915_vma *vma,
+			       enum i915_cache_level cache_level,
+			       u32 unused)
+{
+	u32 pte_flags;
+
+	GEM_BUG_ON(vma->flags & I915_VMA_LOCAL_BIND);
 
 	/* Currently applicable only to VLV */
 	pte_flags = 0;
@@ -1635,8 +1646,8 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
 	ppgtt->base.cleanup = gen8_ppgtt_cleanup;
+	ppgtt->base.bind_vma = gen8_ppgtt_bind_vma;
 	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
 	ppgtt->base.set_pages = ppgtt_set_pages;
 	ppgtt->base.clear_pages = clear_pages;
 	ppgtt->debug_dump = gen8_dump_ppgtt;
@@ -2078,8 +2089,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 	ppgtt->base.clear_range = gen6_ppgtt_clear_range;
 	ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
+	ppgtt->base.bind_vma = gen6_ppgtt_bind_vma;
 	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
 	ppgtt->base.set_pages = ppgtt_set_pages;
 	ppgtt->base.clear_pages = clear_pages;
 	ppgtt->base.cleanup = gen6_ppgtt_cleanup;
-- 
2.17.0

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

[PATCH 2/2] drm/i915/gtt: Enable full-ppgtt by default for HSW

[Chris Wilson]

Let's see if we have all the kinks worked out and full-ppgtt now works
reliably on 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 and more secure (due to better mm
segregation).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b454f3489767..eef37ce184eb 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -178,7 +178,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 		return 0;
 	}
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv)) {
+	if (HAS_LOGICAL_RING_CONTEXTS(dev_priv) || IS_HASWELL(dev_priv)) {
 		if (has_full_48bit_ppgtt)
 			return 3;
 
-- 
2.17.0

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

Re: [PATCH 1/2] drm/i915/gtt: Avoid calling non-existent allocate_va_range

[Chris Wilson]

Quoting Chris Wilson (2018-04-28 21:04:23)
> On hsw and older, we do not need to allocate the ppgtt on the fly and so
> ppgtt->allocate_va_range() is NULL. Fixup ppgtt_bind_vma not to call it,
> in that case!
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 35 +++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 21d72f695adb..b454f3489767 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -189,19 +189,30 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>         return 1;
>  }
>  
> -static int ppgtt_bind_vma(struct i915_vma *vma,
> -                         enum i915_cache_level cache_level,
> -                         u32 unused)
> +static int gen8_ppgtt_bind_vma(struct i915_vma *vma,
> +                              enum i915_cache_level cache_level,
> +                              u32 unused)
>  {
> -       u32 pte_flags;
>         int ret;
>  
> -       if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
> -               ret = vma->vm->allocate_va_range(vma->vm, vma->node.start,
> -                                                vma->size);
> -               if (ret)
> -                       return ret;
> -       }
> +       GEM_BUG_ON(vma->flags & I915_VMA_LOCAL_BIND);

I keep forgetting about PIN_UPDATE. Hopefully this fails in igt and not
just the selftests.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915/gtt: Avoid calling non-existent allocate_va_range

[Chris Wilson]

On hsw and older, we do not need to allocate the ppgtt on the fly and so
ppgtt->allocate_va_range() is NULL. Fixup ppgtt_bind_vma not to call it,
in that case!

v2: PIN_UPDATE still exists.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 21d72f695adb..b5517c66c653 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -189,20 +189,30 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
 	return 1;
 }
 
-static int ppgtt_bind_vma(struct i915_vma *vma,
-			  enum i915_cache_level cache_level,
-			  u32 unused)
+static int gen8_ppgtt_bind_vma(struct i915_vma *vma,
+			       enum i915_cache_level cache_level,
+			       u32 unused)
 {
-	u32 pte_flags;
 	int ret;
 
 	if (!(vma->flags & I915_VMA_LOCAL_BIND)) {
-		ret = vma->vm->allocate_va_range(vma->vm, vma->node.start,
-						 vma->size);
+		ret = vma->vm->allocate_va_range(vma->vm,
+						 vma->node.start, vma->size);
 		if (ret)
 			return ret;
 	}
 
+	vma->vm->insert_entries(vma->vm, vma, cache_level, 0);
+
+	return 0;
+}
+
+static int gen6_ppgtt_bind_vma(struct i915_vma *vma,
+			       enum i915_cache_level cache_level,
+			       u32 unused)
+{
+	u32 pte_flags;
+
 	/* Currently applicable only to VLV */
 	pte_flags = 0;
 	if (vma->obj->gt_ro)
@@ -1635,8 +1645,8 @@ static int gen8_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 		gen8_ppgtt_notify_vgt(ppgtt, true);
 
 	ppgtt->base.cleanup = gen8_ppgtt_cleanup;
+	ppgtt->base.bind_vma = gen8_ppgtt_bind_vma;
 	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
 	ppgtt->base.set_pages = ppgtt_set_pages;
 	ppgtt->base.clear_pages = clear_pages;
 	ppgtt->debug_dump = gen8_dump_ppgtt;
@@ -2078,8 +2088,8 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
 
 	ppgtt->base.clear_range = gen6_ppgtt_clear_range;
 	ppgtt->base.insert_entries = gen6_ppgtt_insert_entries;
+	ppgtt->base.bind_vma = gen6_ppgtt_bind_vma;
 	ppgtt->base.unbind_vma = ppgtt_unbind_vma;
-	ppgtt->base.bind_vma = ppgtt_bind_vma;
 	ppgtt->base.set_pages = ppgtt_set_pages;
 	ppgtt->base.clear_pages = clear_pages;
 	ppgtt->base.cleanup = gen6_ppgtt_cleanup;
-- 
2.17.0

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

✗ Fi.CI.BAT: failure for series starting with drm/i915/gtt: Avoid calling non-existent allocate_va_range (rev2)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915/gtt: Avoid calling non-existent allocate_va_range (rev2)
URL   : https://patchwork.freedesktop.org/series/42448/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4112 -> Patchwork_8836 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42448/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_exec_suspend@basic-s3:
      fi-hsw-4770:        PASS -> INCOMPLETE
      fi-hsw-4770r:       PASS -> INCOMPLETE
      fi-hsw-4200u:       PASS -> INCOMPLETE

    
    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-ivb-3520m:       DMESG-WARN (fdo#106084) -> PASS

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


== Participating hosts (39 -> 36) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4112 -> Patchwork_8836

  CI_DRM_4112: 423a00794c9d9610a71d8a02cd3bc17c6fe5fae1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4452: 29ae12bd764e3b1876356e7628a32192b4ec9066 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8836: ca781d9f5b0b91d13dbb49ac4555d40f4ee2ed36 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4452: 04a2952c5b3782eb03cb136bb16d89daaf243f14 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

ca781d9f5b0b drm/i915/gtt: Enable full-ppgtt by default for HSW
65f39934a0be drm/i915/gtt: Avoid calling non-existent allocate_va_range

== Logs ==

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

✗ Fi.CI.BAT: failure for series starting with drm/i915/gtt: Avoid calling non-existent allocate_va_range (rev2)

[Patchwork]

== Series Details ==

Series: series starting with drm/i915/gtt: Avoid calling non-existent allocate_va_range (rev2)
URL   : https://patchwork.freedesktop.org/series/42448/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4238 -> Patchwork_9118 =

== Summary - FAILURE ==

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/42448/revisions/2/mbox/

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@gem_ctx_create@basic-files:
      fi-hsw-4770:        PASS -> FAIL
      fi-hsw-peppy:       PASS -> FAIL
      fi-hsw-4770r:       PASS -> FAIL
      fi-hsw-4200u:       PASS -> FAIL

    igt@gem_exec_suspend@basic-s3:
      fi-hsw-4770:        PASS -> INCOMPLETE
      fi-hsw-4770r:       PASS -> INCOMPLETE
      fi-hsw-4200u:       PASS -> INCOMPLETE
      fi-hsw-peppy:       PASS -> INCOMPLETE

    
    ==== Warnings ====

    igt@gem_exec_gttfill@basic:
      fi-pnv-d510:        PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@gem_mmap_gtt@basic-small-bo-tiledx:
      fi-gdg-551:         FAIL (fdo#102575) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713


== Participating hosts (44 -> 39) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4238 -> Patchwork_9118

  CI_DRM_4238: 2771a5e6347eb63e43fdfc432a9f15ffb55ef209 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4498: f9ecb79ad8b02278cfdb5b82495df47061c04f8f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9118: 12c27379ac2c679528884723799a560164b98387 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

12c27379ac2c drm/i915/gtt: Enable full-ppgtt by default for HSW
df89deb5e312 drm/i915/gtt: Avoid calling non-existent allocate_va_range

== Logs ==

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