Discussion:
[Intel-gfx] [CI 1/2] drm/i915/selftests: verify_gt_engine_wa() needs rpm wakeref
Chris Wilson
2018-12-06 18:07:12 UTC
Permalink
The mmio readback for verify_gt_engine_wa() also needs a runtime-pm
wakeref, so effectively do the entirety of both engine workarounds
tests. As such simplify the rpm behaviour here by acquiring the wakeref
for the whole of each subtest. It would be still useful to later verify
the registers retain their magic values across rpm suspend/resume.

Signed-off-by: Chris Wilson <***@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <***@intel.com>
Reviewed-by: Tvrtko Ursulin <***@intel.com>
---
.../gpu/drm/i915/selftests/intel_workarounds.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/selftests/intel_workarounds.c b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
index 67017d5175b8..c2b3cd8fcc34 100644
--- a/drivers/gpu/drm/i915/selftests/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/selftests/intel_workarounds.c
@@ -333,7 +333,6 @@ static bool verify_gt_engine_wa(struct drm_i915_private *i915, const char *str)
bool ok = true;

ok &= intel_gt_verify_workarounds(i915, str);
-
for_each_engine(engine, i915, id)
ok &= intel_engine_verify_workarounds(engine, str);

@@ -353,19 +352,19 @@ live_gpu_reset_gt_engine_workarounds(void *arg)
pr_info("Verifying after GPU reset...\n");

igt_global_reset_lock(i915);
+ intel_runtime_pm_get(i915);

ok = verify_gt_engine_wa(i915, "before reset");
if (!ok)
goto out;

- intel_runtime_pm_get(i915);
set_bit(I915_RESET_HANDOFF, &error->flags);
i915_reset(i915, ALL_ENGINES, "live_workarounds");
- intel_runtime_pm_put(i915);

ok = verify_gt_engine_wa(i915, "after reset");

out:
+ intel_runtime_pm_put(i915);
igt_global_reset_unlock(i915);

return ok ? 0 : -ESRCH;
@@ -390,6 +389,7 @@ live_engine_reset_gt_engine_workarounds(void *arg)
return PTR_ERR(ctx);

igt_global_reset_lock(i915);
+ intel_runtime_pm_get(i915);

for_each_engine(engine, i915, id) {
bool ok;
@@ -402,9 +402,7 @@ live_engine_reset_gt_engine_workarounds(void *arg)
goto err;
}

- intel_runtime_pm_get(i915);
i915_reset_engine(engine, "live_workarounds");
- intel_runtime_pm_put(i915);

ok = verify_gt_engine_wa(i915, "after idle reset");
if (!ok) {
@@ -416,13 +414,10 @@ live_engine_reset_gt_engine_workarounds(void *arg)
if (ret)
goto err;

- intel_runtime_pm_get(i915);
-
rq = igt_spinner_create_request(&spin, ctx, engine, MI_NOOP);
if (IS_ERR(rq)) {
ret = PTR_ERR(rq);
igt_spinner_fini(&spin);
- intel_runtime_pm_put(i915);
goto err;
}

@@ -431,15 +426,12 @@ live_engine_reset_gt_engine_workarounds(void *arg)
if (!igt_wait_for_spinner(&spin, rq)) {
pr_err("Spinner failed to start\n");
igt_spinner_fini(&spin);
- intel_runtime_pm_put(i915);
ret = -ETIMEDOUT;
goto err;
}

i915_reset_engine(engine, "live_workarounds");

- intel_runtime_pm_put(i915);
-
igt_spinner_end(&spin);
igt_spinner_fini(&spin);

@@ -451,6 +443,7 @@ live_engine_reset_gt_engine_workarounds(void *arg)
}

err:
+ intel_runtime_pm_put(i915);
igt_global_reset_unlock(i915);
kernel_context_close(ctx);
--
2.20.0.rc2
Chris Wilson
2018-12-06 18:07:13 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.

v2: INSTPM_FORCE_ORDERING is already part of gen8_ctx_workarounds, and
as confirmed by the mmio verification is a part of the context image!
v3: MI_MODE is already part of gen_ctx_workarounds...

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

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index d7fa301b5ec7..dc8981be22cf 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1628,6 +1628,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);

@@ -1644,43 +1645,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)
{
@@ -2280,10 +2244,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 6bcac78a9c36..d02f7e6c0051 100644
--- a/drivers/gpu/drm/i915/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/intel_workarounds.c
@@ -1231,6 +1231,7 @@ static void rcs_engine_wa_init(struct intel_engine_cs *engine)
GEN8_L3SQCREG4,
GEN8_LQSC_FLUSH_COHERENT_LINES);
}
+
}

static void xcs_engine_wa_init(struct intel_engine_cs *engine)
--
2.20.0.rc2
Patchwork
2018-12-06 18:45:22 UTC
Permalink
== Series Details ==

Series: series starting with [CI,1/2] drm/i915/selftests: verify_gt_engine_wa() needs rpm wakeref
URL : https://patchwork.freedesktop.org/series/53687/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5277 -> Patchwork_11039
====================================================

Summary
-------

**SUCCESS**

No regressions found.

External URL: https://patchwork.freedesktop.org/api/1.0/series/53687/revisions/1/mbox/

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

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

### IGT changes ###

#### Warnings ####

* ***@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
- {fi-kbl-7567u}: SKIP -> PASS +33


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

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

### IGT changes ###

#### Issues hit ####

* ***@amdgpu/***@cs-compute:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#108094]

* ***@amdgpu/***@amd-to-i915:
- fi-kbl-8809g: NOTRUN -> FAIL [fdo#107341]

* ***@gem_exec_suspend@basic-s3:
- fi-blb-e6850: PASS -> INCOMPLETE [fdo#107718]


#### Possible fixes ####

* ***@amdgpu/***@userptr:
- fi-kbl-8809g: DMESG-WARN -> PASS

* ***@gem_ctx_create@basic-files:
- fi-bsw-kefka: FAIL [fdo#108656] -> PASS

* ***@gem_mmap_gtt@basic-small-copy:
- fi-glk-dsi: INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

* ***@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
- fi-cfl-8109u: INCOMPLETE [fdo#106070] / [fdo#108126] -> PASS


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

[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#106070]: https://bugs.freedesktop.org/show_bug.cgi?id=106070
[fdo#107341]: https://bugs.freedesktop.org/show_bug.cgi?id=107341
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
[fdo#108126]: https://bugs.freedesktop.org/show_bug.cgi?id=108126
[fdo#108656]: https://bugs.freedesktop.org/show_bug.cgi?id=108656
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (48 -> 41)
------------------------------

Missing (7): fi-ilk-m540 fi-hsw-4200u fi-skl-guc fi-byt-squawks fi-bsw-cyan fi-gdg-551 fi-pnv-d510


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

* Linux: CI_DRM_5277 -> Patchwork_11039

CI_DRM_5277: 13e7a0e59420cbdafca1cedbe2d9c136180aa689 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_4743: edb2db2cf2b6665d7ba3fa9117263302f6307a4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_11039: 6fe6cf6ba62eff3e69185819a6a8ae6953901136 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

6fe6cf6ba62e drm/i915/execlists: Move RCS mmio workaround to new common wa_list
422c46d9ff54 drm/i915/selftests: verify_gt_engine_wa() needs rpm wakeref

== Logs ==

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