* [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT
@ 2017-05-24 15:54 Jon Bloomfield
2017-05-24 16:11 ` ✓ Fi.CI.BAT: success for " Patchwork
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Jon Bloomfield @ 2017-05-24 15:54 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
BXT has a H/W issue with IOMMU which can lead to system hangs when
Aperture accesses are queued within the GAM behind GTT Accesses.
This patch avoids the condition by wrapping all GTT updates in stop_machine
and using a flushing read prior to restarting the machine.
The stop_machine guarantees no new Aperture accesses can begin while
the PTE writes are being emmitted. The flushing read ensures that
any following Aperture accesses cannot begin until the PTE writes
have been cleared out of the GAM's fifo.
Only FOLLOWING Aperture accesses need to be separated from in flight
PTE updates. PTE Writes may follow tightly behind already in flight
Aperture accesses, so no flushing read is required at the start of
a PTE update sequence.
This issue was reproduced by running
igt/gem_readwrite and
igt/gem_render_copy
simultaneously from different processes, each in a tight loop,
with INTEL_IOMMU enabled.
This patch was originally published as:
drm/i915: Serialize GTT Updates on BXT
v2: Move bxt/iommu detection into static function
Remove #ifdef CONFIG_INTEL_IOMMU protection
Make function names more reflective of purpose
Move flushing read into static function
Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: John Harrison <john.C.Harrison@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 9 +++
drivers/gpu/drm/i915/i915_gem_gtt.c | 114 ++++++++++++++++++++++++++++++++++++
2 files changed, 123 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17883a8..b2783b2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3006,6 +3006,15 @@ static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
return false;
}
+static inline bool intel_gtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
+{
+#ifdef CONFIG_INTEL_IOMMU
+ if (IS_BROXTON(dev_priv) && intel_iommu_gfx_mapped)
+ return true;
+#endif
+ return false;
+}
+
int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
int enable_ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7c769d7..ab91178 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2191,6 +2191,110 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
gen8_set_pte(>t_base[i], scratch_pte);
}
+static void bxt_vtd_gtt_wa(struct drm_i915_private *dev_priv)
+{
+ /*
+ * Make sure the internal GAM fifo has been cleared of all GTT
+ * Writes before exiting stop_machine(). This guarantees that
+ * any Aperture accesses waiting to start in another process
+ * cannot backup behind the GTT Writes causing a hang.
+ * The register can be any arbitrary GAM register.
+ */
+ POSTING_READ(GFX_FLSH_CNTL_GEN6);
+}
+
+struct insert_page {
+ struct i915_address_space *vm;
+ dma_addr_t addr;
+ u64 offset;
+ enum i915_cache_level level;
+};
+
+static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
+{
+ struct insert_page *arg = _arg;
+
+ struct drm_i915_private *dev_priv = arg->vm->i915;
+
+ gen8_ggtt_insert_page(arg->vm, arg->addr,
+ arg->offset, arg->level, 0);
+
+ bxt_vtd_gtt_wa(dev_priv);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_insert_page__BKL(struct i915_address_space *vm,
+ dma_addr_t addr,
+ u64 offset,
+ enum i915_cache_level level,
+ u32 unused)
+{
+ struct insert_page arg = { vm, addr, offset, level };
+
+ stop_machine(bxt_vtd_ggtt_insert_page__cb, &arg, NULL);
+}
+
+
+struct insert_entries {
+ struct i915_address_space *vm;
+ struct sg_table *st;
+ u64 start;
+ enum i915_cache_level level;
+};
+
+static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
+{
+ struct insert_entries *arg = _arg;
+
+ struct drm_i915_private *dev_priv = arg->vm->i915;
+
+ gen8_ggtt_insert_entries(arg->vm, arg->st,
+ arg->start, arg->level, 0);
+
+ bxt_vtd_gtt_wa(dev_priv);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
+ struct sg_table *st,
+ u64 start,
+ enum i915_cache_level level,
+ u32 unused)
+{
+ struct insert_entries arg = { vm, st, start, level };
+
+ stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
+}
+
+struct clear_range {
+ struct i915_address_space *vm;
+ u64 start;
+ u64 length;
+};
+
+static int bxt_vtd_ggtt_clear_range__cb(void *_arg)
+{
+ struct clear_range *arg = _arg;
+
+ struct drm_i915_private *dev_priv = arg->vm->i915;
+
+ gen8_ggtt_clear_range(arg->vm, arg->start, arg->length);
+
+ bxt_vtd_gtt_wa(dev_priv);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_clear_range__BKL(struct i915_address_space *vm,
+ u64 start,
+ u64 length)
+{
+ struct clear_range arg = { vm, start, length };
+ stop_machine(bxt_vtd_ggtt_clear_range__cb, &arg, NULL);
+}
+
static void gen6_ggtt_clear_range(struct i915_address_space *vm,
u64 start, u64 length)
{
@@ -2789,6 +2893,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+ /* Serialize GTT updates with Aperture on BXT if VT-d is on. */
+ if (intel_gtt_update_needs_vtd_wa(dev_priv)) {
+ ggtt->base.insert_entries = bxt_vtd_ggtt_insert_entries__BKL;
+ ggtt->base.insert_page = bxt_vtd_ggtt_insert_page__BKL;
+ if (!USES_FULL_PPGTT(dev_priv) ||
+ intel_scanout_needs_vtd_wa(dev_priv)) {
+ ggtt->base.clear_range = bxt_vtd_ggtt_clear_range__BKL;
+ }
+ }
+
ggtt->invalidate = gen6_ggtt_invalidate;
return ggtt_probe_common(ggtt, size);
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Serialize GTT/Aperture accesses on BXT
2017-05-24 15:54 [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT Jon Bloomfield
@ 2017-05-24 16:11 ` Patchwork
2017-05-25 10:54 ` [PATCH] " Tvrtko Ursulin
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-05-24 16:11 UTC (permalink / raw)
To: Bloomfield, Jon; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Serialize GTT/Aperture accesses on BXT
URL : https://patchwork.freedesktop.org/series/24882/
State : success
== Summary ==
Series 24882v1 drm/i915: Serialize GTT/Aperture accesses on BXT
https://patchwork.freedesktop.org/api/1.0/series/24882/revisions/1/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:443s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:445s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:579s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:518s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:489s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:481s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:418s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:412s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:424s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:502s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:463s
fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:466s
fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:567s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:464s
fi-skl-6700hq total:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:439s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:467s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:501s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:438s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:534s
fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:408s
7808a0f3330f5bdf11b5b6880af8407ad6200989 drm-tip: 2017y-05m-24d-11h-04m-21s UTC integration manifest
2799429 drm/i915: Serialize GTT/Aperture accesses on BXT
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4802/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT
2017-05-24 15:54 [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT Jon Bloomfield
2017-05-24 16:11 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-05-25 10:54 ` Tvrtko Ursulin
2017-05-25 11:48 ` Chris Wilson
2017-05-25 11:16 ` [CI] " Chris Wilson
2017-05-25 11:35 ` ✓ Fi.CI.BAT: success for drm/i915: Serialize GTT/Aperture accesses on BXT (rev2) Patchwork
3 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2017-05-25 10:54 UTC (permalink / raw)
To: Jon Bloomfield, intel-gfx; +Cc: Daniel Vetter
On 24/05/2017 16:54, Jon Bloomfield wrote:
> BXT has a H/W issue with IOMMU which can lead to system hangs when
> Aperture accesses are queued within the GAM behind GTT Accesses.
>
> This patch avoids the condition by wrapping all GTT updates in stop_machine
> and using a flushing read prior to restarting the machine.
>
> The stop_machine guarantees no new Aperture accesses can begin while
> the PTE writes are being emmitted. The flushing read ensures that
> any following Aperture accesses cannot begin until the PTE writes
> have been cleared out of the GAM's fifo.
>
> Only FOLLOWING Aperture accesses need to be separated from in flight
> PTE updates. PTE Writes may follow tightly behind already in flight
> Aperture accesses, so no flushing read is required at the start of
> a PTE update sequence.
>
> This issue was reproduced by running
> igt/gem_readwrite and
> igt/gem_render_copy
> simultaneously from different processes, each in a tight loop,
> with INTEL_IOMMU enabled.
>
> This patch was originally published as:
> drm/i915: Serialize GTT Updates on BXT
>
> v2: Move bxt/iommu detection into static function
> Remove #ifdef CONFIG_INTEL_IOMMU protection
> Make function names more reflective of purpose
> Move flushing read into static function
>
> Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
> Cc: John Harrison <john.C.Harrison@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 9 +++
> drivers/gpu/drm/i915/i915_gem_gtt.c | 114 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 123 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 17883a8..b2783b2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3006,6 +3006,15 @@ static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
> return false;
> }
>
> +static inline bool intel_gtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
> +{
> +#ifdef CONFIG_INTEL_IOMMU
> + if (IS_BROXTON(dev_priv) && intel_iommu_gfx_mapped)
> + return true;
> +#endif
> + return false;
> +}
> +
> int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
> int enable_ppgtt);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7c769d7..ab91178 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2191,6 +2191,110 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
> gen8_set_pte(>t_base[i], scratch_pte);
> }
>
> +static void bxt_vtd_gtt_wa(struct drm_i915_private *dev_priv)
> +{
> + /*
> + * Make sure the internal GAM fifo has been cleared of all GTT
> + * Writes before exiting stop_machine(). This guarantees that
> + * any Aperture accesses waiting to start in another process
> + * cannot backup behind the GTT Writes causing a hang.
> + * The register can be any arbitrary GAM register.
> + */
> + POSTING_READ(GFX_FLSH_CNTL_GEN6);
> +}
> +
> +struct insert_page {
> + struct i915_address_space *vm;
> + dma_addr_t addr;
> + u64 offset;
> + enum i915_cache_level level;
> +};
> +
> +static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
> +{
> + struct insert_page *arg = _arg;
> +
I don't remember seeing blank lines in declaration blocks anywhere else
so I'd remove this (and below).
But FWIW, in essence the patch looks good to me. I was even surprised
that GCC understands not to complain about defined but unused functions
when IOMMU is compiled out. One learns something new every day! (Until
one forgets it again!)
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Regards,
Tvrtko
> + struct drm_i915_private *dev_priv = arg->vm->i915;
> +
> + gen8_ggtt_insert_page(arg->vm, arg->addr,
> + arg->offset, arg->level, 0);
> +
> + bxt_vtd_gtt_wa(dev_priv);
> +
> + return 0;
> +}
> +
> +static void bxt_vtd_ggtt_insert_page__BKL(struct i915_address_space *vm,
> + dma_addr_t addr,
> + u64 offset,
> + enum i915_cache_level level,
> + u32 unused)
> +{
> + struct insert_page arg = { vm, addr, offset, level };
> +
> + stop_machine(bxt_vtd_ggtt_insert_page__cb, &arg, NULL);
> +}
> +
> +
> +struct insert_entries {
> + struct i915_address_space *vm;
> + struct sg_table *st;
> + u64 start;
> + enum i915_cache_level level;
> +};
> +
> +static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
> +{
> + struct insert_entries *arg = _arg;
> +
> + struct drm_i915_private *dev_priv = arg->vm->i915;
> +
> + gen8_ggtt_insert_entries(arg->vm, arg->st,
> + arg->start, arg->level, 0);
> +
> + bxt_vtd_gtt_wa(dev_priv);
> +
> + return 0;
> +}
> +
> +static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
> + struct sg_table *st,
> + u64 start,
> + enum i915_cache_level level,
> + u32 unused)
> +{
> + struct insert_entries arg = { vm, st, start, level };
> +
> + stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
> +}
> +
> +struct clear_range {
> + struct i915_address_space *vm;
> + u64 start;
> + u64 length;
> +};
> +
> +static int bxt_vtd_ggtt_clear_range__cb(void *_arg)
> +{
> + struct clear_range *arg = _arg;
> +
> + struct drm_i915_private *dev_priv = arg->vm->i915;
> +
> + gen8_ggtt_clear_range(arg->vm, arg->start, arg->length);
> +
> + bxt_vtd_gtt_wa(dev_priv);
> +
> + return 0;
> +}
> +
> +static void bxt_vtd_ggtt_clear_range__BKL(struct i915_address_space *vm,
> + u64 start,
> + u64 length)
> +{
> + struct clear_range arg = { vm, start, length };
> + stop_machine(bxt_vtd_ggtt_clear_range__cb, &arg, NULL);
> +}
> +
> static void gen6_ggtt_clear_range(struct i915_address_space *vm,
> u64 start, u64 length)
> {
> @@ -2789,6 +2893,16 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
>
> ggtt->base.insert_entries = gen8_ggtt_insert_entries;
>
> + /* Serialize GTT updates with Aperture on BXT if VT-d is on. */
> + if (intel_gtt_update_needs_vtd_wa(dev_priv)) {
> + ggtt->base.insert_entries = bxt_vtd_ggtt_insert_entries__BKL;
> + ggtt->base.insert_page = bxt_vtd_ggtt_insert_page__BKL;
> + if (!USES_FULL_PPGTT(dev_priv) ||
> + intel_scanout_needs_vtd_wa(dev_priv)) {
> + ggtt->base.clear_range = bxt_vtd_ggtt_clear_range__BKL;
> + }
> + }
> +
> ggtt->invalidate = gen6_ggtt_invalidate;
>
> return ggtt_probe_common(ggtt, size);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* [CI] drm/i915: Serialize GTT/Aperture accesses on BXT
2017-05-24 15:54 [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT Jon Bloomfield
2017-05-24 16:11 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-05-25 10:54 ` [PATCH] " Tvrtko Ursulin
@ 2017-05-25 11:16 ` Chris Wilson
2017-05-25 11:35 ` ✓ Fi.CI.BAT: success for drm/i915: Serialize GTT/Aperture accesses on BXT (rev2) Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-05-25 11:16 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
From: Jon Bloomfield <jon.bloomfield@intel.com>
BXT has a H/W issue with IOMMU which can lead to system hangs when
Aperture accesses are queued within the GAM behind GTT Accesses.
This patch avoids the condition by wrapping all GTT updates in stop_machine
and using a flushing read prior to restarting the machine.
The stop_machine guarantees no new Aperture accesses can begin while
the PTE writes are being emmitted. The flushing read ensures that
any following Aperture accesses cannot begin until the PTE writes
have been cleared out of the GAM's fifo.
Only FOLLOWING Aperture accesses need to be separated from in flight
PTE updates. PTE Writes may follow tightly behind already in flight
Aperture accesses, so no flushing read is required at the start of
a PTE update sequence.
This issue was reproduced by running
igt/gem_readwrite and
igt/gem_render_copy
simultaneously from different processes, each in a tight loop,
with INTEL_IOMMU enabled.
This patch was originally published as:
drm/i915: Serialize GTT Updates on BXT
v2: Move bxt/iommu detection into static function
Remove #ifdef CONFIG_INTEL_IOMMU protection
Make function names more reflective of purpose
Move flushing read into static function
v3: Tidy up for checkpatch.pl
Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
Cc: John Harrison <john.C.Harrison@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1495641251-30022-1-git-send-email-jon.bloomfield@intel.com
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_drv.h | 10 ++++
drivers/gpu/drm/i915/i915_gem_gtt.c | 103 ++++++++++++++++++++++++++++++++++++
2 files changed, 113 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 17883a84b8c1..a5a01b683d48 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3006,6 +3006,16 @@ static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
return false;
}
+static inline bool
+intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *dev_priv)
+{
+#ifdef CONFIG_INTEL_IOMMU
+ if (IS_BROXTON(dev_priv) && intel_iommu_gfx_mapped)
+ return true;
+#endif
+ return false;
+}
+
int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
int enable_ppgtt);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index b18ed511ffe5..cb58c61d15ef 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2189,6 +2189,101 @@ static void gen8_ggtt_clear_range(struct i915_address_space *vm,
gen8_set_pte(>t_base[i], scratch_pte);
}
+static void bxt_vtd_ggtt_wa(struct i915_address_space *vm)
+{
+ struct drm_i915_private *dev_priv = vm->i915;
+
+ /*
+ * Make sure the internal GAM fifo has been cleared of all GTT
+ * Writes before exiting stop_machine(). This guarantees that
+ * any aperture accesses waiting to start in another process
+ * cannot back up behind the GTT writes causing a hang.
+ * The register can be any arbitrary GAM register.
+ */
+ POSTING_READ(GFX_FLSH_CNTL_GEN6);
+}
+
+struct insert_page {
+ struct i915_address_space *vm;
+ dma_addr_t addr;
+ u64 offset;
+ enum i915_cache_level level;
+};
+
+static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
+{
+ struct insert_page *arg = _arg;
+
+ gen8_ggtt_insert_page(arg->vm, arg->addr, arg->offset, arg->level, 0);
+ bxt_vtd_ggtt_wa(arg->vm);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_insert_page__BKL(struct i915_address_space *vm,
+ dma_addr_t addr,
+ u64 offset,
+ enum i915_cache_level level,
+ u32 unused)
+{
+ struct insert_page arg = { vm, addr, offset, level };
+
+ stop_machine(bxt_vtd_ggtt_insert_page__cb, &arg, NULL);
+}
+
+struct insert_entries {
+ struct i915_address_space *vm;
+ struct sg_table *st;
+ u64 start;
+ enum i915_cache_level level;
+};
+
+static int bxt_vtd_ggtt_insert_entries__cb(void *_arg)
+{
+ struct insert_entries *arg = _arg;
+
+ gen8_ggtt_insert_entries(arg->vm, arg->st, arg->start, arg->level, 0);
+ bxt_vtd_ggtt_wa(arg->vm);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_insert_entries__BKL(struct i915_address_space *vm,
+ struct sg_table *st,
+ u64 start,
+ enum i915_cache_level level,
+ u32 unused)
+{
+ struct insert_entries arg = { vm, st, start, level };
+
+ stop_machine(bxt_vtd_ggtt_insert_entries__cb, &arg, NULL);
+}
+
+struct clear_range {
+ struct i915_address_space *vm;
+ u64 start;
+ u64 length;
+};
+
+static int bxt_vtd_ggtt_clear_range__cb(void *_arg)
+{
+ struct clear_range *arg = _arg;
+
+ gen8_ggtt_clear_range(arg->vm, arg->start, arg->length);
+ bxt_vtd_ggtt_wa(arg->vm);
+
+ return 0;
+}
+
+static void bxt_vtd_ggtt_clear_range__BKL(struct i915_address_space *vm,
+ u64 start,
+ u64 length)
+{
+ struct clear_range arg = { vm, start, length };
+
+ stop_machine(bxt_vtd_ggtt_clear_range__cb, &arg, NULL);
+}
+
static void gen6_ggtt_clear_range(struct i915_address_space *vm,
u64 start, u64 length)
{
@@ -2787,6 +2882,14 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
ggtt->base.insert_entries = gen8_ggtt_insert_entries;
+ /* Serialize GTT updates with aperture access on BXT if VT-d is on. */
+ if (intel_ggtt_update_needs_vtd_wa(dev_priv)) {
+ ggtt->base.insert_entries = bxt_vtd_ggtt_insert_entries__BKL;
+ ggtt->base.insert_page = bxt_vtd_ggtt_insert_page__BKL;
+ if (ggtt->base.clear_range != nop_clear_range)
+ ggtt->base.clear_range = bxt_vtd_ggtt_clear_range__BKL;
+ }
+
ggtt->invalidate = gen6_ggtt_invalidate;
return ggtt_probe_common(ggtt, size);
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Serialize GTT/Aperture accesses on BXT (rev2)
2017-05-24 15:54 [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT Jon Bloomfield
` (2 preceding siblings ...)
2017-05-25 11:16 ` [CI] " Chris Wilson
@ 2017-05-25 11:35 ` Patchwork
3 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2017-05-25 11:35 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Serialize GTT/Aperture accesses on BXT (rev2)
URL : https://patchwork.freedesktop.org/series/24882/
State : success
== Summary ==
Series 24882v2 drm/i915: Serialize GTT/Aperture accesses on BXT
https://patchwork.freedesktop.org/api/1.0/series/24882/revisions/2/mbox/
Test gem_exec_flush:
Subgroup basic-batch-kernel-default-uc:
pass -> FAIL (fi-snb-2600) fdo#100007
fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time:441s
fi-bdw-gvtdvm total:278 pass:256 dwarn:8 dfail:0 fail:0 skip:14 time:433s
fi-bsw-n3050 total:278 pass:242 dwarn:0 dfail:0 fail:0 skip:36 time:588s
fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time:506s
fi-byt-j1900 total:278 pass:254 dwarn:0 dfail:0 fail:0 skip:24 time:499s
fi-byt-n2820 total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:488s
fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:418s
fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time:410s
fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time:420s
fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:494s
fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time:468s
fi-kbl-7500u total:278 pass:255 dwarn:5 dfail:0 fail:0 skip:18 time:464s
fi-kbl-7560u total:278 pass:263 dwarn:5 dfail:0 fail:0 skip:10 time:564s
fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:453s
fi-skl-6700hq total:278 pass:239 dwarn:0 dfail:1 fail:17 skip:21 time:438s
fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time:462s
fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time:495s
fi-skl-gvtdvm total:278 pass:265 dwarn:0 dfail:0 fail:0 skip:13 time:441s
fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time:533s
fi-snb-2600 total:278 pass:248 dwarn:0 dfail:0 fail:1 skip:29 time:403s
af161db6dba99f58096174c0456f211aab3976ad drm-tip: 2017y-05m-25d-01h-32m-39s UTC integration manifest
9fae59a drm/i915: Serialize GTT/Aperture accesses on BXT
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4806/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT
2017-05-25 10:54 ` [PATCH] " Tvrtko Ursulin
@ 2017-05-25 11:48 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2017-05-25 11:48 UTC (permalink / raw)
To: Tvrtko Ursulin; +Cc: Daniel Vetter, intel-gfx
On Thu, May 25, 2017 at 11:54:03AM +0100, Tvrtko Ursulin wrote:
>
> On 24/05/2017 16:54, Jon Bloomfield wrote:
> >BXT has a H/W issue with IOMMU which can lead to system hangs when
> >Aperture accesses are queued within the GAM behind GTT Accesses.
> >
> >This patch avoids the condition by wrapping all GTT updates in stop_machine
> >and using a flushing read prior to restarting the machine.
> >
> >The stop_machine guarantees no new Aperture accesses can begin while
> >the PTE writes are being emmitted. The flushing read ensures that
> >any following Aperture accesses cannot begin until the PTE writes
> >have been cleared out of the GAM's fifo.
> >
> >Only FOLLOWING Aperture accesses need to be separated from in flight
> >PTE updates. PTE Writes may follow tightly behind already in flight
> >Aperture accesses, so no flushing read is required at the start of
> >a PTE update sequence.
> >
> >This issue was reproduced by running
> > igt/gem_readwrite and
> > igt/gem_render_copy
> >simultaneously from different processes, each in a tight loop,
> >with INTEL_IOMMU enabled.
> >
> >This patch was originally published as:
> > drm/i915: Serialize GTT Updates on BXT
> >
> >v2: Move bxt/iommu detection into static function
> > Remove #ifdef CONFIG_INTEL_IOMMU protection
> > Make function names more reflective of purpose
> > Move flushing read into static function
> >
> >Signed-off-by: Jon Bloomfield <jon.bloomfield@intel.com>
> >Cc: John Harrison <john.C.Harrison@intel.com>
> >Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >Cc: Daniel Vetter <daniel.vetter@intel.com>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >+static int bxt_vtd_ggtt_insert_page__cb(void *_arg)
> >+{
> >+ struct insert_page *arg = _arg;
> >+
>
> I don't remember seeing blank lines in declaration blocks anywhere
> else so I'd remove this (and below).
>
> But FWIW, in essence the patch looks good to me. I was even
> surprised that GCC understands not to complain about defined but
> unused functions when IOMMU is compiled out. One learns something
> new every day! (Until one forgets it again!)
>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Tidied up the whitespace to appease checkpatch, and pushed.
Thanks for the fix and review,
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-05-25 11:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-24 15:54 [PATCH] drm/i915: Serialize GTT/Aperture accesses on BXT Jon Bloomfield
2017-05-24 16:11 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-05-25 10:54 ` [PATCH] " Tvrtko Ursulin
2017-05-25 11:48 ` Chris Wilson
2017-05-25 11:16 ` [CI] " Chris Wilson
2017-05-25 11:35 ` ✓ Fi.CI.BAT: success for drm/i915: Serialize GTT/Aperture accesses on BXT (rev2) Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.