From 279f5a00c9a9b39f4f6e9813e6d4da8c181d34c8 Mon Sep 17 00:00:00 2001
From: Chris Wilson <chris@chris-wilson.co.uk>
Date: Thu, 5 Oct 2017 20:10:05 +0100
Subject: [PATCH] drm/i915/execlists: Add a comment for the extra MI_ARB_ENABLE
Mime-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8bit
Git-commit: 279f5a00c9a9b39f4f6e9813e6d4da8c181d34c8
Patch-mainline: v4.15-rc1
References: FATE#322643 bsc#1055900
Michel Thierry noticed that we were applying WaDisableCtxRestoreArbitration
even to gen9, which does not require the w/a. The rationale is that we
need to enable MI arbitration for execlists to work, and to be safe we
do that before every batch (in addition to every context switch into the
batch). Since this is not clear from the single line comment suggesting
the MI_ARB_ENABLE is solely for the w/a, add a little more detail.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20171005191005.13462-1-chris@chris-wilson.co.uk
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
Acked-by: Takashi Iwai <tiwai@suse.de>
---
drivers/gpu/drm/i915/intel_lrc.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1618,7 +1618,23 @@ static int gen8_emit_bb_start(struct drm
if (IS_ERR(cs))
return PTR_ERR(cs);
- /* WaDisableCtxRestoreArbitration:bdw,chv */
+ /*
+ * WaDisableCtxRestoreArbitration:bdw,chv
+ *
+ * We don't need to perform MI_ARB_ENABLE as often as we do (in
+ * particular all the gen that do not need the w/a at all!), if we
+ * took care to make sure that on every switch into this context
+ * (both ordinary and for preemption) that arbitrartion was enabled
+ * we would be fine. However, there doesn't seem to be a downside to
+ * being paranoid and making sure it is set before each batch and
+ * every context-switch.
+ *
+ * Note that if we fail to enable arbitration before the request
+ * is complete, then we do not see the context-switch interrupt and
+ * the engine hangs (with RING_HEAD == RING_TAIL).
+ *
+ * That satisfies both the GPGPU w/a and our heavy-handed paranoia.
+ */
*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
/* FIXME(BDW): Address space and security selectors. */