Discussion:
[Intel-gfx] [PATCH] drm/i915/execlists: Move RCS mmio workaround to new common wa_list
Chris Wilson
2018-12-06 12:56:07 UTC
Permalink
We can move the remaining RCS workarounds applied to only gen8 to the
engine->wa_list, and then reduce all engine->init_hw callbacks to common
code. The benefit of using the new wa_list is that we verify that the
registers are indeed restored and keep their magic values.

Signed-off-by: Chris Wilson <***@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <***@intel.com>
---
drivers/gpu/drm/i915/intel_lrc.c | 42 +-----------------------
drivers/gpu/drm/i915/intel_workarounds.c | 14 ++++++++
2 files changed, 15 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0b64bc38f66f..31f378cf5525 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1671,6 +1671,7 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
static int gen8_init_common_ring(struct intel_engine_cs *engine)
{
intel_engine_apply_workarounds(engine);
+ intel_engine_apply_whitelist(engine);

intel_mocs_init_engine(engine);

@@ -1687,43 +1688,6 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
return 0;
}

-static int gen8_init_render_ring(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
- int ret;
-
- ret = gen8_init_common_ring(engine);
- if (ret)
- return ret;
-
- intel_engine_apply_whitelist(engine);
-
- /* We need to disable the AsyncFlip performance optimisations in order
- * to use MI_WAIT_FOR_EVENT within the CS. It should already be
- * programmed to '1' on all products.
- *
- * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv,bdw,chv
- */
- I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
-
- I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
-
- return 0;
-}
-
-static int gen9_init_render_ring(struct intel_engine_cs *engine)
-{
- int ret;
-
- ret = gen8_init_common_ring(engine);
- if (ret)
- return ret;
-
- intel_engine_apply_whitelist(engine);
-
- return 0;
-}
-
static struct i915_request *
execlists_reset_prepare(struct intel_engine_cs *engine)
{
@@ -2286,10 +2250,6 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
engine->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;

/* Override some for render ring. */
- if (INTEL_GEN(dev_priv) >= 9)
- engine->init_hw = gen9_init_render_ring;
- else
- engine->init_hw = gen8_init_render_ring;
engine->init_context = gen8_init_rcs_context;
engine->emit_flush = gen8_emit_flush_render;
engine->emit_breadcrumb = gen8_emit_breadcrumb_rcs;
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index a4ab78bdcbbf..50229eb1eb08 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1229,6 +1229,20 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine)
GEN8_L3SQCREG4,
GEN8_LQSC_FLUSH_COHERENT_LINES);
}
+
+ if (IS_GEN8(i915)) {
+ /*
+ * We need to disable the AsyncFlip performance optimisations
+ * in order to use MI_WAIT_FOR_EVENT within the CS. It should
+ * already be programmed to '1' on all products.
+ *
+ * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv,bdw,chv
+ */
+ wa_masked_en(wal, MI_MODE, ASYNC_FLIP_PERF_DISABLE);
+
+ /* XXX Is not INSTPM per-context + non-priv for gen6+? */
+ wa_masked_en(wal, INSTPM, INSTPM_FORCE_ORDERING);
+ }
}

static void xcs_engine_wa_init(struct intel_engine_cs *engine)
--
2.20.0.rc2
Tvrtko Ursulin
2018-12-06 13:02:12 UTC
Permalink
Post by Chris Wilson
We can move the remaining RCS workarounds applied to only gen8 to the
engine->wa_list, and then reduce all engine->init_hw callbacks to common
code. The benefit of using the new wa_list is that we verify that the
registers are indeed restored and keep their magic values.
---
drivers/gpu/drm/i915/intel_lrc.c | 42 +-----------------------
drivers/gpu/drm/i915/intel_workarounds.c | 14 ++++++++
2 files changed, 15 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 0b64bc38f66f..31f378cf5525 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1671,6 +1671,7 @@ static bool unexpected_starting_state(struct intel_engine_cs *engine)
static int gen8_init_common_ring(struct intel_engine_cs *engine)
{
intel_engine_apply_workarounds(engine);
+ intel_engine_apply_whitelist(engine);
intel_mocs_init_engine(engine);
@@ -1687,43 +1688,6 @@ static int gen8_init_common_ring(struct intel_engine_cs *engine)
return 0;
}
-static int gen8_init_render_ring(struct intel_engine_cs *engine)
-{
- struct drm_i915_private *dev_priv = engine->i915;
- int ret;
-
- ret = gen8_init_common_ring(engine);
- if (ret)
- return ret;
-
- intel_engine_apply_whitelist(engine);
-
- /* We need to disable the AsyncFlip performance optimisations in order
- * to use MI_WAIT_FOR_EVENT within the CS. It should already be
- * programmed to '1' on all products.
- *
- * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv,bdw,chv
- */
- I915_WRITE(MI_MODE, _MASKED_BIT_ENABLE(ASYNC_FLIP_PERF_DISABLE));
-
- I915_WRITE(INSTPM, _MASKED_BIT_ENABLE(INSTPM_FORCE_ORDERING));
-
- return 0;
-}
-
-static int gen9_init_render_ring(struct intel_engine_cs *engine)
-{
- int ret;
-
- ret = gen8_init_common_ring(engine);
- if (ret)
- return ret;
-
- intel_engine_apply_whitelist(engine);
-
- return 0;
-}
-
static struct i915_request *
execlists_reset_prepare(struct intel_engine_cs *engine)
{
@@ -2286,10 +2250,6 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
engine->irq_keep_mask |= GT_RENDER_L3_PARITY_ERROR_INTERRUPT;
/* Override some for render ring. */
- if (INTEL_GEN(dev_priv) >= 9)
- engine->init_hw = gen9_init_render_ring;
- else
- engine->init_hw = gen8_init_render_ring;
engine->init_context = gen8_init_rcs_context;
engine->emit_flush = gen8_emit_flush_render;
engine->emit_breadcrumb = gen8_emit_breadcrumb_rcs;
diff --git a/drivers/gpu/drm/i915/intel_workarounds.c b/drivers/gpu/drm/i915/intel_workarounds.c
index a4ab78bdcbbf..50229eb1eb08 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1229,6 +1229,20 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine)
GEN8_L3SQCREG4,
GEN8_LQSC_FLUSH_COHERENT_LINES);
}
+
+ if (IS_GEN8(i915)) {
+ /*
+ * We need to disable the AsyncFlip performance optimisations
+ * in order to use MI_WAIT_FOR_EVENT within the CS. It should
+ * already be programmed to '1' on all products.
+ *
+ * WaDisableAsyncFlipPerfMode:snb,ivb,hsw,vlv,bdw,chv
+ */
+ wa_masked_en(wal, MI_MODE, ASYNC_FLIP_PERF_DISABLE);
+
+ /* XXX Is not INSTPM per-context + non-priv for gen6+? */
+ wa_masked_en(wal, INSTPM, INSTPM_FORCE_ORDERING);
+ }
}
static void xcs_engine_wa_init(struct intel_engine_cs *engine)
Simples!

Reviewed-by: Tvrtko Ursulin <***@intel.com>

Regards,

Tvrtko
Patchwork
2018-12-06 14:01:12 UTC
Permalink
== Series Details ==

Series: drm/i915/execlists: Move RCS mmio workaround to new common wa_list
URL : https://patchwork.freedesktop.org/series/53656/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5275 -> Patchwork_11034
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with Patchwork_11034 absolutely need to be
verified manually.

If you think the reported changes have nothing to do with the changes
introduced in Patchwork_11034, 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/53656/revisions/1/mbox/

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

* ***@i915_selftest@live_workarounds:
- fi-bsw-n3050: PASS -> DMESG-FAIL
- fi-bsw-kefka: PASS -> DMESG-FAIL
- fi-bdw-5557u: PASS -> DMESG-FAIL
- fi-bdw-gvtdvm: PASS -> DMESG-FAIL

* {***@runner@aborted}:
- fi-bdw-gvtdvm: NOTRUN -> FAIL
- fi-bsw-n3050: NOTRUN -> FAIL
- fi-bsw-kefka: NOTRUN -> FAIL
- fi-bdw-5557u: NOTRUN -> FAIL


Known issues
------------

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

### IGT changes ###

#### Issues hit ####

* ***@gem_exec_suspend@basic-s4-devices:
- fi-ivb-3520m: PASS -> FAIL [fdo#108880]


{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).

[fdo#108880]: https://bugs.freedesktop.org/show_bug.cgi?id=108880


Participating hosts (48 -> 43)
------------------------------

Missing (5): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-glk-j4005


Build changes
-------------

* Linux: CI_DRM_5275 -> Patchwork_11034

CI_DRM_5275: 95dba5a17f86524f31cdc13af7d3238351db9a9c @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11034: 01e3692245d777c44640736d9f257d88cb2095d2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

01e3692245d7 drm/i915/execlists: Move RCS mmio workaround to new common wa_list

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_11034/
Loading...