diff mbox series

[7/7] drm/i915/execlists: Apply a full mb before execution for Braswell

Message ID 20181201095231.324-7-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show
Series [1/7] drm/i915: Complete the fences as they are cancelled due to wedging | expand

Commit Message

Chris Wilson Dec. 1, 2018, 9:52 a.m. UTC
Braswell is really picky about having our writes posted to memory before
we execute or else the GPU may see stale values. A wmb() is insufficient
as it only ensures the writes are visible to other cores, we need a full
mb() to ensure the writes are in memory and visible to the GPU.

The most frequent failure in flushing before execution is that we see
stale PTE values and execute the wrong pages.

References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/i915/intel_lrc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Chris Wilson Dec. 1, 2018, 6:32 p.m. UTC | #1
Quoting Chris Wilson (2018-12-01 09:52:31)
> Braswell is really picky about having our writes posted to memory before
> we execute or else the GPU may see stale values. A wmb() is insufficient
> as it only ensures the writes are visible to other cores, we need a full
> mb() to ensure the writes are in memory and visible to the GPU.
> 
> The most frequent failure in flushing before execution is that we see
> stale PTE values and execute the wrong pages.
> 
> References: 987abd5c62f9 ("drm/i915/execlists: Force write serialisation into context image vs execution")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 0a690c557113..cad1228f6503 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -379,8 +379,12 @@ static u64 execlists_update_context(struct i915_request *rq)
>          * may not be visible to the HW prior to the completion of the UC
>          * register write and that we may begin execution from the context
>          * before its image is complete leading to invalid PD chasing.
> +        *
> +        * Furthermore, Braswell wants a full mb to be sure that the writes
> +        * are coherent in memory (visible to the GPU) prior to execution,
> +        * and not just visible to other CPUs (as is the result of wmb).
>          */
> -       wmb();
> +       mb();

Scratch that. Just took ages to hit this time. It seemed so promising!
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0a690c557113..cad1228f6503 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -379,8 +379,12 @@  static u64 execlists_update_context(struct i915_request *rq)
 	 * may not be visible to the HW prior to the completion of the UC
 	 * register write and that we may begin execution from the context
 	 * before its image is complete leading to invalid PD chasing.
+	 *
+	 * Furthermore, Braswell wants a full mb to be sure that the writes
+	 * are coherent in memory (visible to the GPU) prior to execution,
+	 * and not just visible to other CPUs (as is the result of wmb).
 	 */
-	wmb();
+	mb();
 	return ce->lrc_desc;
 }