[Intel-gfx] [PATCH] drm/i915: Workaround incoherence with fence updates on Valleyview

[Chris Wilson]

In commit 25ff1195f8a0b3724541ae7bbe331b4296de9c06
Author: Chris Wilson <chris at chris-wilson.co.uk>
Date:   Thu Apr 4 21:31:03 2013 +0100

    drm/i915: Workaround incoherence between fences and LLC across multiple CPUs

we introduced an empirical workaround for memory corruption when using
fences from multiple CPUs. At the time, we did not have any results for
Valleyview, so the presumption was that it was limited to recent
generations using LLC. Now we have evidence that Valleyview also suffers
incoherence and requires a similar but different workaround. For
Valleyview, the wbinvd instruction is insufficient and we require the
serialising register write per-CPU. Conversely, that serialising
register write is not enough for SNB/IVB/HSW. To compromise and keep the
code relatively clean, employ both serialisation techniques in the same
workaround.

Reported-by: Jon Bloomfield <jon.bloomfield at intel.com>
Tested-by: Jon Bloomfield <jon.bloomfield at intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=62191
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c |   35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 38c8f11..0c6f0bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2711,18 +2711,33 @@ static inline int fence_number(struct drm_i915_private *dev_priv,
 	return fence - dev_priv->fence_regs;
 }
 
+struct write_fence {
+	struct drm_device *dev;
+	struct drm_i915_gem_object *obj;
+	int fence;
+};
+
 static void i915_gem_write_fence__ipi(void *data)
 {
+	struct write_fence *args = data;
+
+	/* Required for SNB+ with LLC */
 	wbinvd();
+
+	/* Required for VLV */
+	i915_gem_write_fence(args->dev, args->fence, args->obj);
 }
 
 static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
 					 struct drm_i915_fence_reg *fence,
 					 bool enable)
 {
-	struct drm_device *dev = obj->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int fence_reg = fence_number(dev_priv, fence);
+	struct drm_i915_private *dev_priv = obj->base.dev->dev_private;
+	struct write_fence args = {
+		.dev = obj->base.dev,
+		.fence = fence_number(dev_priv, fence),
+		.obj = enable ? obj : NULL,
+	};
 
 	/* In order to fully serialize access to the fenced region and
 	 * the update to the fence register we need to take extreme
@@ -2733,13 +2748,19 @@ static void i915_gem_object_update_fence(struct drm_i915_gem_object *obj,
 	 * SNB+ we need to take a step further and emit an explicit wbinvd()
 	 * on each processor in order to manually flush all memory
 	 * transactions before updating the fence register.
+	 *
+	 * However, Valleyview complicates matter. There the wbinvd is
+	 * insufficient and unlike SNB/IVB requires the serialising
+	 * register write. (Note that that register write by itself is
+	 * conversely not sufficient for SNB+.) To compromise, we do both.
 	 */
-	if (HAS_LLC(obj->base.dev))
-		on_each_cpu(i915_gem_write_fence__ipi, NULL, 1);
-	i915_gem_write_fence(dev, fence_reg, enable ? obj : NULL);
+	if (INTEL_INFO(args.dev)->gen >= 6)
+		on_each_cpu(i915_gem_write_fence__ipi, &args, 1);
+	else
+		i915_gem_write_fence(args.dev, args.fence, args.obj);
 
 	if (enable) {
-		obj->fence_reg = fence_reg;
+		obj->fence_reg = args.fence;
 		fence->obj = obj;
 		list_move_tail(&fence->lru_list, &dev_priv->mm.fence_list);
 	} else {
-- 
1.7.10.4