[PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

[PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

[Chris Wilson]

For those mock tests that may wish to pretend triggering a GPU reset and
processing the cleanup.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
 drivers/gpu/drm/i915/gt/intel_reset.c         | 32 +++++++++++++------
 drivers/gpu/drm/i915/gt/intel_reset.h         |  5 ++-
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 12 +++----
 drivers/gpu/drm/i915/gt/selftest_lrc.c        |  2 +-
 drivers/gpu/drm/i915/gt/selftest_reset.c      |  4 +--
 .../gpu/drm/i915/gt/selftest_workarounds.c    |  8 ++---
 drivers/gpu/drm/i915/i915_getparam.c          |  4 +--
 8 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 8f125f1624bd..7758a3744626 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -4273,7 +4273,7 @@ __intel_display_resume(struct drm_device *dev,
 static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
 {
 	return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
-		intel_has_gpu_reset(dev_priv));
+		intel_has_gpu_reset(&dev_priv->gt));
 }
 
 void intel_prepare_reset(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index d08226f5bea5..76938fa3a1b9 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -542,13 +542,24 @@ static int gen8_reset_engines(struct intel_gt *gt,
 	return ret;
 }
 
+static int mock_reset(struct intel_gt *gt,
+		      intel_engine_mask_t mask,
+		      unsigned int retry)
+{
+	return 0;
+}
+
 typedef int (*reset_func)(struct intel_gt *,
 			  intel_engine_mask_t engine_mask,
 			  unsigned int retry);
 
-static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
+static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
 {
-	if (INTEL_GEN(i915) >= 8)
+	struct drm_i915_private *i915 = gt->i915;
+
+	if (is_mock_gt(gt))
+		return mock_reset;
+	else if (INTEL_GEN(i915) >= 8)
 		return gen8_reset_engines;
 	else if (INTEL_GEN(i915) >= 6)
 		return gen6_reset_engines;
@@ -571,7 +582,7 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
 	int ret = -ETIMEDOUT;
 	int retry;
 
-	reset = intel_get_gpu_reset(gt->i915);
+	reset = intel_get_gpu_reset(gt);
 	if (!reset)
 		return -ENODEV;
 
@@ -591,17 +602,20 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
 	return ret;
 }
 
-bool intel_has_gpu_reset(struct drm_i915_private *i915)
+bool intel_has_gpu_reset(const struct intel_gt *gt)
 {
 	if (!i915_modparams.reset)
 		return NULL;
 
-	return intel_get_gpu_reset(i915);
+	return intel_get_gpu_reset(gt);
 }
 
-bool intel_has_reset_engine(struct drm_i915_private *i915)
+bool intel_has_reset_engine(const struct intel_gt *gt)
 {
-	return INTEL_INFO(i915)->has_reset_engine && i915_modparams.reset >= 2;
+	if (i915_modparams.reset < 2)
+		return false;
+
+	return INTEL_INFO(gt->i915)->has_reset_engine;
 }
 
 int intel_reset_guc(struct intel_gt *gt)
@@ -958,7 +972,7 @@ void intel_gt_reset(struct intel_gt *gt,
 
 	awake = reset_prepare(gt);
 
-	if (!intel_has_gpu_reset(gt->i915)) {
+	if (!intel_has_gpu_reset(gt)) {
 		if (i915_modparams.reset)
 			dev_err(gt->i915->drm.dev, "GPU reset not supported\n");
 		else
@@ -1179,7 +1193,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
 	 * Try engine reset when available. We fall back to full reset if
 	 * single reset fails.
 	 */
-	if (intel_has_reset_engine(gt->i915) && !intel_gt_is_wedged(gt)) {
+	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
 		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
 			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
 			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
index 0b6ff1ee7f06..8e8d5f761166 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.h
+++ b/drivers/gpu/drm/i915/gt/intel_reset.h
@@ -14,7 +14,6 @@
 #include "intel_engine_types.h"
 #include "intel_reset_types.h"
 
-struct drm_i915_private;
 struct i915_request;
 struct intel_engine_cs;
 struct intel_gt;
@@ -80,7 +79,7 @@ static inline bool __intel_reset_failed(const struct intel_reset *reset)
 	return unlikely(test_bit(I915_WEDGED, &reset->flags));
 }
 
-bool intel_has_gpu_reset(struct drm_i915_private *i915);
-bool intel_has_reset_engine(struct drm_i915_private *i915);
+bool intel_has_gpu_reset(const struct intel_gt *gt);
+bool intel_has_reset_engine(const struct intel_gt *gt);
 
 #endif /* I915_RESET_H */
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index a0098fc35921..9c0c8441c22a 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -458,7 +458,7 @@ static int igt_reset_nop_engine(void *arg)
 
 	/* Check that we can engine-reset during non-user portions */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	file = mock_file(gt->i915);
@@ -559,7 +559,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
 
 	/* Check that we can issue an engine reset on an idle engine (no-op) */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	if (active) {
@@ -791,7 +791,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
 	 * with any other engine.
 	 */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	if (flags & TEST_ACTIVE) {
@@ -1547,7 +1547,7 @@ static int igt_handle_error(void *arg)
 
 	/* Check that we can issue a global GPU and engine reset */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	if (!engine || !intel_engine_can_store_dword(engine))
@@ -1689,7 +1689,7 @@ static int igt_reset_engines_atomic(void *arg)
 
 	/* Check that the engines resets are usable from atomic context */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	if (USES_GUC_SUBMISSION(gt->i915))
@@ -1746,7 +1746,7 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
 	bool saved_hangcheck;
 	int err;
 
-	if (!intel_has_gpu_reset(gt->i915))
+	if (!intel_has_gpu_reset(gt))
 		return 0;
 
 	if (intel_gt_is_wedged(gt))
diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
index 22ea2e747064..93f2fcdc49bf 100644
--- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
+++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
@@ -1310,7 +1310,7 @@ static int live_preempt_hang(void *arg)
 	if (!HAS_LOGICAL_RING_PREEMPTION(i915))
 		return 0;
 
-	if (!intel_has_reset_engine(i915))
+	if (!intel_has_reset_engine(&i915->gt))
 		return 0;
 
 	mutex_lock(&i915->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 00a4f60cdfd5..d79482db7fe8 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -112,7 +112,7 @@ static int igt_atomic_engine_reset(void *arg)
 
 	/* Check that the resets are usable from atomic context */
 
-	if (!intel_has_reset_engine(gt->i915))
+	if (!intel_has_reset_engine(gt))
 		return 0;
 
 	if (USES_GUC_SUBMISSION(gt->i915))
@@ -170,7 +170,7 @@ int intel_reset_live_selftests(struct drm_i915_private *i915)
 	};
 	struct intel_gt *gt = &i915->gt;
 
-	if (!intel_has_gpu_reset(gt->i915))
+	if (!intel_has_gpu_reset(gt))
 		return 0;
 
 	if (intel_gt_is_wedged(gt))
diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index 999a98f00494..d40ce0709bff 100644
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -747,7 +747,7 @@ static int live_reset_whitelist(void *arg)
 
 	igt_global_reset_lock(&i915->gt);
 
-	if (intel_has_reset_engine(i915)) {
+	if (intel_has_reset_engine(&i915->gt)) {
 		err = check_whitelist_across_reset(engine,
 						   do_engine_reset,
 						   "engine");
@@ -755,7 +755,7 @@ static int live_reset_whitelist(void *arg)
 			goto out;
 	}
 
-	if (intel_has_gpu_reset(i915)) {
+	if (intel_has_gpu_reset(&i915->gt)) {
 		err = check_whitelist_across_reset(engine,
 						   do_device_reset,
 						   "device");
@@ -1131,7 +1131,7 @@ live_gpu_reset_workarounds(void *arg)
 	struct wa_lists lists;
 	bool ok;
 
-	if (!intel_has_gpu_reset(i915))
+	if (!intel_has_gpu_reset(&i915->gt))
 		return 0;
 
 	ctx = kernel_context(i915);
@@ -1178,7 +1178,7 @@ live_engine_reset_workarounds(void *arg)
 	struct wa_lists lists;
 	int ret = 0;
 
-	if (!intel_has_reset_engine(i915))
+	if (!intel_has_reset_engine(&i915->gt))
 		return 0;
 
 	ctx = kernel_context(i915);
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index 5d9101376a3d..f4b3cbb1adce 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -79,8 +79,8 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
 		break;
 	case I915_PARAM_HAS_GPU_RESET:
 		value = i915_modparams.enable_hangcheck &&
-			intel_has_gpu_reset(i915);
-		if (value && intel_has_reset_engine(i915))
+			intel_has_gpu_reset(&i915->gt);
+		if (value && intel_has_reset_engine(&i915->gt))
 			value = 2;
 		break;
 	case I915_PARAM_HAS_RESOURCE_STREAMER:
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/selftests; Do not try to sanitize mock HW

[Chris Wilson]

If we are mocking the device, skip trying to sanitize the pm HW state.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andi Shyti <andi.shyti@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 42f175d9b98c..29fa1dabbc2e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -137,7 +137,8 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force)
 
 void intel_gt_pm_disable(struct intel_gt *gt)
 {
-	intel_sanitize_gt_powersave(gt->i915);
+	if (!is_mock_gt(gt))
+		intel_sanitize_gt_powersave(gt->i915);
 }
 
 void intel_gt_pm_fini(struct intel_gt *gt)
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/selftests: Provide a mock GPU reset routine

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/selftests: Provide a mock GPU reset routine
URL   : https://patchwork.freedesktop.org/series/67353/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6971 -> Patchwork_14571
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_14571 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_14571, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_execlists:
    - fi-blb-e6850:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-blb-e6850/igt@i915_selftest@live_execlists.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-blb-e6850/igt@i915_selftest@live_execlists.html

  * igt@i915_selftest@live_hangcheck:
    - fi-snb-2520m:       [PASS][3] -> [DMESG-FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-snb-2520m/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-snb-2520m/igt@i915_selftest@live_hangcheck.html
    - fi-ilk-650:         [PASS][5] -> [DMESG-FAIL][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-ilk-650/igt@i915_selftest@live_hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-ilk-650/igt@i915_selftest@live_hangcheck.html
    - fi-elk-e7500:       [PASS][7] -> [DMESG-FAIL][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-elk-e7500/igt@i915_selftest@live_hangcheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-elk-e7500/igt@i915_selftest@live_hangcheck.html
    - fi-blb-e6850:       [PASS][9] -> [DMESG-FAIL][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-blb-e6850/igt@i915_selftest@live_hangcheck.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-blb-e6850/igt@i915_selftest@live_hangcheck.html
    - fi-ivb-3770:        [PASS][11] -> [DMESG-FAIL][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-ivb-3770/igt@i915_selftest@live_hangcheck.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-ivb-3770/igt@i915_selftest@live_hangcheck.html
    - fi-snb-2600:        [PASS][13] -> [DMESG-FAIL][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-snb-2600/igt@i915_selftest@live_hangcheck.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-snb-2600/igt@i915_selftest@live_hangcheck.html

  * igt@runner@aborted:
    - fi-blb-e6850:       NOTRUN -> [FAIL][15]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-blb-e6850/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@prime_vgem@basic-wait-default:
    - fi-icl-u3:          [PASS][16] -> [DMESG-WARN][17] ([fdo#107724]) +4 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-icl-u3/igt@prime_vgem@basic-wait-default.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-icl-u3/igt@prime_vgem@basic-wait-default.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-cml-s}:         [INCOMPLETE][18] ([fdo#110566]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-cml-s/igt@gem_ctx_create@basic-files.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-cml-s/igt@gem_ctx_create@basic-files.html
    - {fi-tgl-u2}:        [INCOMPLETE][20] ([fdo#111735]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-tgl-u2/igt@gem_ctx_create@basic-files.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-tgl-u2/igt@gem_ctx_create@basic-files.html

  * igt@gem_flink_basic@bad-flink:
    - fi-icl-u3:          [DMESG-WARN][22] ([fdo#107724]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-icl-u3/igt@gem_flink_basic@bad-flink.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-icl-u3/igt@gem_flink_basic@bad-flink.html

  * igt@i915_selftest@live_hangcheck:
    - {fi-icl-dsi}:       [DMESG-FAIL][24] ([fdo#111678]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html

  
#### Warnings ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       [FAIL][26] ([fdo#111407]) -> [FAIL][27] ([fdo#111045] / [fdo#111096])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6971/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/fi-kbl-7500u/igt@kms_chamelium@hdmi-hpd-fast.html

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

  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111045]: https://bugs.freedesktop.org/show_bug.cgi?id=111045
  [fdo#111096]: https://bugs.freedesktop.org/show_bug.cgi?id=111096
  [fdo#111407]: https://bugs.freedesktop.org/show_bug.cgi?id=111407
  [fdo#111678]: https://bugs.freedesktop.org/show_bug.cgi?id=111678
  [fdo#111735]: https://bugs.freedesktop.org/show_bug.cgi?id=111735


Participating hosts (50 -> 44)
------------------------------

  Additional (2): fi-bsw-kefka fi-icl-guc 
  Missing    (8): fi-ilk-m540 fi-bdw-5557u fi-hsw-4200u fi-byt-squawks fi-pnv-d510 fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6971 -> Patchwork_14571

  CI-20190529: 20190529
  CI_DRM_6971: b891ecf6856b90013c667c0d8becb7edb2f0c0d1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5206: 5a6c68568def840cd720f18fc66f529a89f84675 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14571: ff301f6023fa694cf136a526016c8bdfa40ee9b9 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ff301f6023fa drm/i915/selftests; Do not try to sanitize mock HW
36f28a7d198d drm/i915/selftests: Provide a mock GPU reset routine

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14571/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

[Andi Shyti]

Hi Chris,

On Fri, Sep 27, 2019 at 08:14:42PM +0100, Chris Wilson wrote:
> For those mock tests that may wish to pretend triggering a GPU reset and
> processing the cleanup.

The patch is OK, per se, but I think it should be split in two
parts:

 - the i915 to gt conversion (that is the biggest part of the
   patch)
 - the mock-reset part (baskically the function)

right?

Andi

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/gt/intel_reset.c         | 32 +++++++++++++------
>  drivers/gpu/drm/i915/gt/intel_reset.h         |  5 ++-
>  drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 12 +++----
>  drivers/gpu/drm/i915/gt/selftest_lrc.c        |  2 +-
>  drivers/gpu/drm/i915/gt/selftest_reset.c      |  4 +--
>  .../gpu/drm/i915/gt/selftest_workarounds.c    |  8 ++---
>  drivers/gpu/drm/i915/i915_getparam.c          |  4 +--
>  8 files changed, 41 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 8f125f1624bd..7758a3744626 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -4273,7 +4273,7 @@ __intel_display_resume(struct drm_device *dev,
>  static bool gpu_reset_clobbers_display(struct drm_i915_private *dev_priv)
>  {
>  	return (INTEL_INFO(dev_priv)->gpu_reset_clobbers_display &&
> -		intel_has_gpu_reset(dev_priv));
> +		intel_has_gpu_reset(&dev_priv->gt));
>  }
>  
>  void intel_prepare_reset(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index d08226f5bea5..76938fa3a1b9 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -542,13 +542,24 @@ static int gen8_reset_engines(struct intel_gt *gt,
>  	return ret;
>  }
>  
> +static int mock_reset(struct intel_gt *gt,
> +		      intel_engine_mask_t mask,
> +		      unsigned int retry)
> +{
> +	return 0;
> +}
> +
>  typedef int (*reset_func)(struct intel_gt *,
>  			  intel_engine_mask_t engine_mask,
>  			  unsigned int retry);
>  
> -static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
> +static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>  {
> -	if (INTEL_GEN(i915) >= 8)
> +	struct drm_i915_private *i915 = gt->i915;
> +
> +	if (is_mock_gt(gt))
> +		return mock_reset;
> +	else if (INTEL_GEN(i915) >= 8)
>  		return gen8_reset_engines;
>  	else if (INTEL_GEN(i915) >= 6)
>  		return gen6_reset_engines;
> @@ -571,7 +582,7 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>  	int ret = -ETIMEDOUT;
>  	int retry;
>  
> -	reset = intel_get_gpu_reset(gt->i915);
> +	reset = intel_get_gpu_reset(gt);
>  	if (!reset)
>  		return -ENODEV;
>  
> @@ -591,17 +602,20 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>  	return ret;
>  }
>  
> -bool intel_has_gpu_reset(struct drm_i915_private *i915)
> +bool intel_has_gpu_reset(const struct intel_gt *gt)
>  {
>  	if (!i915_modparams.reset)
>  		return NULL;
>  
> -	return intel_get_gpu_reset(i915);
> +	return intel_get_gpu_reset(gt);
>  }
>  
> -bool intel_has_reset_engine(struct drm_i915_private *i915)
> +bool intel_has_reset_engine(const struct intel_gt *gt)
>  {
> -	return INTEL_INFO(i915)->has_reset_engine && i915_modparams.reset >= 2;
> +	if (i915_modparams.reset < 2)
> +		return false;
> +
> +	return INTEL_INFO(gt->i915)->has_reset_engine;
>  }
>  
>  int intel_reset_guc(struct intel_gt *gt)
> @@ -958,7 +972,7 @@ void intel_gt_reset(struct intel_gt *gt,
>  
>  	awake = reset_prepare(gt);
>  
> -	if (!intel_has_gpu_reset(gt->i915)) {
> +	if (!intel_has_gpu_reset(gt)) {
>  		if (i915_modparams.reset)
>  			dev_err(gt->i915->drm.dev, "GPU reset not supported\n");
>  		else
> @@ -1179,7 +1193,7 @@ void intel_gt_handle_error(struct intel_gt *gt,
>  	 * Try engine reset when available. We fall back to full reset if
>  	 * single reset fails.
>  	 */
> -	if (intel_has_reset_engine(gt->i915) && !intel_gt_is_wedged(gt)) {
> +	if (intel_has_reset_engine(gt) && !intel_gt_is_wedged(gt)) {
>  		for_each_engine_masked(engine, gt->i915, engine_mask, tmp) {
>  			BUILD_BUG_ON(I915_RESET_MODESET >= I915_RESET_ENGINE);
>  			if (test_and_set_bit(I915_RESET_ENGINE + engine->id,
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.h b/drivers/gpu/drm/i915/gt/intel_reset.h
> index 0b6ff1ee7f06..8e8d5f761166 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.h
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.h
> @@ -14,7 +14,6 @@
>  #include "intel_engine_types.h"
>  #include "intel_reset_types.h"
>  
> -struct drm_i915_private;
>  struct i915_request;
>  struct intel_engine_cs;
>  struct intel_gt;
> @@ -80,7 +79,7 @@ static inline bool __intel_reset_failed(const struct intel_reset *reset)
>  	return unlikely(test_bit(I915_WEDGED, &reset->flags));
>  }
>  
> -bool intel_has_gpu_reset(struct drm_i915_private *i915);
> -bool intel_has_reset_engine(struct drm_i915_private *i915);
> +bool intel_has_gpu_reset(const struct intel_gt *gt);
> +bool intel_has_reset_engine(const struct intel_gt *gt);
>  
>  #endif /* I915_RESET_H */
> diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> index a0098fc35921..9c0c8441c22a 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
> @@ -458,7 +458,7 @@ static int igt_reset_nop_engine(void *arg)
>  
>  	/* Check that we can engine-reset during non-user portions */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	file = mock_file(gt->i915);
> @@ -559,7 +559,7 @@ static int __igt_reset_engine(struct intel_gt *gt, bool active)
>  
>  	/* Check that we can issue an engine reset on an idle engine (no-op) */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	if (active) {
> @@ -791,7 +791,7 @@ static int __igt_reset_engines(struct intel_gt *gt,
>  	 * with any other engine.
>  	 */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	if (flags & TEST_ACTIVE) {
> @@ -1547,7 +1547,7 @@ static int igt_handle_error(void *arg)
>  
>  	/* Check that we can issue a global GPU and engine reset */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	if (!engine || !intel_engine_can_store_dword(engine))
> @@ -1689,7 +1689,7 @@ static int igt_reset_engines_atomic(void *arg)
>  
>  	/* Check that the engines resets are usable from atomic context */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	if (USES_GUC_SUBMISSION(gt->i915))
> @@ -1746,7 +1746,7 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
>  	bool saved_hangcheck;
>  	int err;
>  
> -	if (!intel_has_gpu_reset(gt->i915))
> +	if (!intel_has_gpu_reset(gt))
>  		return 0;
>  
>  	if (intel_gt_is_wedged(gt))
> diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> index 22ea2e747064..93f2fcdc49bf 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c
> @@ -1310,7 +1310,7 @@ static int live_preempt_hang(void *arg)
>  	if (!HAS_LOGICAL_RING_PREEMPTION(i915))
>  		return 0;
>  
> -	if (!intel_has_reset_engine(i915))
> +	if (!intel_has_reset_engine(&i915->gt))
>  		return 0;
>  
>  	mutex_lock(&i915->drm.struct_mutex);
> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> index 00a4f60cdfd5..d79482db7fe8 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_reset.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
> @@ -112,7 +112,7 @@ static int igt_atomic_engine_reset(void *arg)
>  
>  	/* Check that the resets are usable from atomic context */
>  
> -	if (!intel_has_reset_engine(gt->i915))
> +	if (!intel_has_reset_engine(gt))
>  		return 0;
>  
>  	if (USES_GUC_SUBMISSION(gt->i915))
> @@ -170,7 +170,7 @@ int intel_reset_live_selftests(struct drm_i915_private *i915)
>  	};
>  	struct intel_gt *gt = &i915->gt;
>  
> -	if (!intel_has_gpu_reset(gt->i915))
> +	if (!intel_has_gpu_reset(gt))
>  		return 0;
>  
>  	if (intel_gt_is_wedged(gt))
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index 999a98f00494..d40ce0709bff 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -747,7 +747,7 @@ static int live_reset_whitelist(void *arg)
>  
>  	igt_global_reset_lock(&i915->gt);
>  
> -	if (intel_has_reset_engine(i915)) {
> +	if (intel_has_reset_engine(&i915->gt)) {
>  		err = check_whitelist_across_reset(engine,
>  						   do_engine_reset,
>  						   "engine");
> @@ -755,7 +755,7 @@ static int live_reset_whitelist(void *arg)
>  			goto out;
>  	}
>  
> -	if (intel_has_gpu_reset(i915)) {
> +	if (intel_has_gpu_reset(&i915->gt)) {
>  		err = check_whitelist_across_reset(engine,
>  						   do_device_reset,
>  						   "device");
> @@ -1131,7 +1131,7 @@ live_gpu_reset_workarounds(void *arg)
>  	struct wa_lists lists;
>  	bool ok;
>  
> -	if (!intel_has_gpu_reset(i915))
> +	if (!intel_has_gpu_reset(&i915->gt))
>  		return 0;
>  
>  	ctx = kernel_context(i915);
> @@ -1178,7 +1178,7 @@ live_engine_reset_workarounds(void *arg)
>  	struct wa_lists lists;
>  	int ret = 0;
>  
> -	if (!intel_has_reset_engine(i915))
> +	if (!intel_has_reset_engine(&i915->gt))
>  		return 0;
>  
>  	ctx = kernel_context(i915);
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 5d9101376a3d..f4b3cbb1adce 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -79,8 +79,8 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>  		break;
>  	case I915_PARAM_HAS_GPU_RESET:
>  		value = i915_modparams.enable_hangcheck &&
> -			intel_has_gpu_reset(i915);
> -		if (value && intel_has_reset_engine(i915))
> +			intel_has_gpu_reset(&i915->gt);
> +		if (value && intel_has_reset_engine(&i915->gt))
>  			value = 2;
>  		break;
>  	case I915_PARAM_HAS_RESOURCE_STREAMER:
> -- 
> 2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/selftests; Do not try to sanitize mock HW

[Andi Shyti]

Hi Chris,

On Fri, Sep 27, 2019 at 08:14:43PM +0100, Chris Wilson wrote:
> If we are mocking the device, skip trying to sanitize the pm HW state.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andi Shyti <andi.shyti@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_gt_pm.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> index 42f175d9b98c..29fa1dabbc2e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
> @@ -137,7 +137,8 @@ void intel_gt_sanitize(struct intel_gt *gt, bool force)
>  
>  void intel_gt_pm_disable(struct intel_gt *gt)
>  {
> -	intel_sanitize_gt_powersave(gt->i915);
> +	if (!is_mock_gt(gt))
> +		intel_sanitize_gt_powersave(gt->i915);

Cool!

Reviewed-by: Andi Shyti <andi.shyti@intel.com>

Andi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

[Chris Wilson]

Quoting Andi Shyti (2019-09-27 21:41:19)
> Hi Chris,
> 
> On Fri, Sep 27, 2019 at 08:14:42PM +0100, Chris Wilson wrote:
> > For those mock tests that may wish to pretend triggering a GPU reset and
> > processing the cleanup.
> 
> The patch is OK, per se, but I think it should be split in two
> parts:
> 
>  - the i915 to gt conversion (that is the biggest part of the
>    patch)
>  - the mock-reset part (baskically the function)
> 
> right?

But you read it all already...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/selftests: Provide a mock GPU reset routine

[Chris Wilson]

Quoting Chris Wilson (2019-09-27 20:14:42)
> -static reset_func intel_get_gpu_reset(struct drm_i915_private *i915)
> +static reset_func intel_get_gpu_reset(const struct intel_gt *gt)
>  {
> -       if (INTEL_GEN(i915) >= 8)
> +       struct drm_i915_private *i915 = gt->i915;
> +
> +       if (is_mock_gt(gt))

Actually this highlights an issue with using gt->awake == -1 as our
indicator.

Hmm. I wonder...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx