[PATCH v2 0/9] GuC fixes

[PATCH v2 0/9] GuC fixes

[Michal Wajdeczko]

Misc GuC fixes for upcoming 32.0.3

v2: modified reset selftests

Michal Wajdeczko (9):
  drm/i915/selftests: Move some reset testcases to separate file
  drm/i915/selftests: Split igt_atomic_reset testcase
  drm/i915/selftests: Use prepare/finish during atomic reset test
  drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded
  drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish
  drm/i915/uc: Use GuC firmware status helper
  drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
  drm/i915/uc: Stop talking with GuC when resetting
  drm/i915/uc: Skip reset preparation if GuC is already dead

 drivers/gpu/drm/i915/gt/intel_reset.c         |   4 +
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 159 +++---------------
 drivers/gpu/drm/i915/gt/selftest_reset.c      | 119 +++++++++++++
 drivers/gpu/drm/i915/intel_guc.h              |  10 +-
 drivers/gpu/drm/i915/intel_guc_ct.h           |   5 +
 drivers/gpu/drm/i915/intel_guc_submission.c   |   2 +-
 drivers/gpu/drm/i915/intel_uc.c               |  44 +++--
 .../drm/i915/selftests/i915_live_selftests.h  |   1 +
 drivers/gpu/drm/i915/selftests/igt_atomic.h   |  56 ++++++
 drivers/gpu/drm/i915/selftests/igt_reset.c    |   8 +
 drivers/gpu/drm/i915/selftests/igt_reset.h    |   1 +
 11 files changed, 249 insertions(+), 160 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_reset.c
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_atomic.h

-- 
2.19.2

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

[PATCH v2 1/9] drm/i915/selftests: Move some reset testcases to separate file

[Michal Wajdeczko]

igt_global_reset and igt_wedged_reset testcases are first candidates.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_reset.c         |  4 +
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c  | 50 ------------
 drivers/gpu/drm/i915/gt/selftest_reset.c      | 76 +++++++++++++++++++
 .../drm/i915/selftests/i915_live_selftests.h  |  1 +
 4 files changed, 81 insertions(+), 50 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_reset.c

diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
index 464369bc55ad..8c60f7550f9c 100644
--- a/drivers/gpu/drm/i915/gt/intel_reset.c
+++ b/drivers/gpu/drm/i915/gt/intel_reset.c
@@ -1394,3 +1394,7 @@ void __i915_fini_wedge(struct i915_wedge_me *w)
 	destroy_delayed_work_on_stack(&w->work);
 	w->i915 = NULL;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_reset.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index dab3d30c9c73..a6e7f84bbbe9 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -365,54 +365,6 @@ static int igt_hang_sanitycheck(void *arg)
 	return err;
 }
 
-static int igt_global_reset(void *arg)
-{
-	struct drm_i915_private *i915 = arg;
-	unsigned int reset_count;
-	int err = 0;
-
-	/* Check that we can issue a global GPU reset */
-
-	igt_global_reset_lock(i915);
-
-	reset_count = i915_reset_count(&i915->gpu_error);
-
-	i915_reset(i915, ALL_ENGINES, NULL);
-
-	if (i915_reset_count(&i915->gpu_error) == reset_count) {
-		pr_err("No GPU reset recorded!\n");
-		err = -EINVAL;
-	}
-
-	igt_global_reset_unlock(i915);
-
-	if (i915_reset_failed(i915))
-		err = -EIO;
-
-	return err;
-}
-
-static int igt_wedged_reset(void *arg)
-{
-	struct drm_i915_private *i915 = arg;
-	intel_wakeref_t wakeref;
-
-	/* Check that we can recover a wedged device with a GPU reset */
-
-	igt_global_reset_lock(i915);
-	wakeref = intel_runtime_pm_get(i915);
-
-	i915_gem_set_wedged(i915);
-
-	GEM_BUG_ON(!i915_reset_failed(i915));
-	i915_reset(i915, ALL_ENGINES, NULL);
-
-	intel_runtime_pm_put(i915, wakeref);
-	igt_global_reset_unlock(i915);
-
-	return i915_reset_failed(i915) ? -EIO : 0;
-}
-
 static bool wait_for_idle(struct intel_engine_cs *engine)
 {
 	return wait_for(intel_engine_is_idle(engine), IGT_IDLE_TIMEOUT) == 0;
@@ -1846,8 +1798,6 @@ static int igt_atomic_reset(void *arg)
 int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
-		SUBTEST(igt_global_reset), /* attempt to recover GPU first */
-		SUBTEST(igt_wedged_reset),
 		SUBTEST(igt_hang_sanitycheck),
 		SUBTEST(igt_reset_nop),
 		SUBTEST(igt_reset_nop_engine),
diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
new file mode 100644
index 000000000000..ef90101bb87a
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "selftests/igt_reset.h"
+
+static int igt_global_reset(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	unsigned int reset_count;
+	int err = 0;
+
+	/* Check that we can issue a global GPU reset */
+
+	igt_global_reset_lock(i915);
+
+	reset_count = i915_reset_count(&i915->gpu_error);
+
+	i915_reset(i915, ALL_ENGINES, NULL);
+
+	if (i915_reset_count(&i915->gpu_error) == reset_count) {
+		pr_err("No GPU reset recorded!\n");
+		err = -EINVAL;
+	}
+
+	igt_global_reset_unlock(i915);
+
+	if (i915_reset_failed(i915))
+		err = -EIO;
+
+	return err;
+}
+
+static int igt_wedged_reset(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	intel_wakeref_t wakeref;
+
+	/* Check that we can recover a wedged device with a GPU reset */
+
+	igt_global_reset_lock(i915);
+	wakeref = intel_runtime_pm_get(i915);
+
+	i915_gem_set_wedged(i915);
+
+	GEM_BUG_ON(!i915_reset_failed(i915));
+	i915_reset(i915, ALL_ENGINES, NULL);
+
+	intel_runtime_pm_put(i915, wakeref);
+	igt_global_reset_unlock(i915);
+
+	return i915_reset_failed(i915) ? -EIO : 0;
+}
+
+int intel_reset_live_selftests(struct drm_i915_private *i915)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_global_reset), /* attempt to recover GPU first */
+		SUBTEST(igt_wedged_reset),
+	};
+	intel_wakeref_t wakeref;
+	int err = 0;
+
+	if (!intel_has_gpu_reset(i915))
+		return 0;
+
+	if (i915_terminally_wedged(i915))
+		return -EIO; /* we're long past hope of a successful reset */
+
+	with_intel_runtime_pm(i915, wakeref)
+		err = i915_subtests(tests, i915);
+
+	return err;
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index a54f590788a4..a953125b14c4 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -24,6 +24,7 @@ selftest(gem, i915_gem_live_selftests)
 selftest(evict, i915_gem_evict_live_selftests)
 selftest(hugepages, i915_gem_huge_page_live_selftests)
 selftest(contexts, i915_gem_context_live_selftests)
+selftest(reset, intel_reset_live_selftests)
 selftest(hangcheck, intel_hangcheck_live_selftests)
 selftest(execlists, intel_execlists_live_selftests)
 selftest(guc, intel_guc_live_selftest)
-- 
2.19.2

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

[PATCH v2 2/9] drm/i915/selftests: Split igt_atomic_reset testcase

[Michal Wajdeczko]

Split igt_atomic_reset selftests into separate full & engines parts,
so we can move former to the dedicated reset selftests file.

While here change engines test to loop first over atomic phases and
then loop over available engines.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 109 ++++---------------
 drivers/gpu/drm/i915/gt/selftest_reset.c     |  41 +++++++
 drivers/gpu/drm/i915/selftests/igt_atomic.h  |  56 ++++++++++
 drivers/gpu/drm/i915/selftests/igt_reset.c   |   8 ++
 drivers/gpu/drm/i915/selftests/igt_reset.h   |   1 +
 5 files changed, 125 insertions(+), 90 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/selftests/igt_atomic.h

diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
index a6e7f84bbbe9..48a51739b926 100644
--- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
+++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c
@@ -32,6 +32,7 @@
 #include "selftests/igt_gem_utils.h"
 #include "selftests/igt_reset.h"
 #include "selftests/igt_wedge_me.h"
+#include "selftests/igt_atomic.h"
 
 #include "selftests/mock_context.h"
 #include "selftests/mock_drm.h"
@@ -1603,44 +1604,8 @@ static int igt_handle_error(void *arg)
 	return err;
 }
 
-static void __preempt_begin(void)
-{
-	preempt_disable();
-}
-
-static void __preempt_end(void)
-{
-	preempt_enable();
-}
-
-static void __softirq_begin(void)
-{
-	local_bh_disable();
-}
-
-static void __softirq_end(void)
-{
-	local_bh_enable();
-}
-
-static void __hardirq_begin(void)
-{
-	local_irq_disable();
-}
-
-static void __hardirq_end(void)
-{
-	local_irq_enable();
-}
-
-struct atomic_section {
-	const char *name;
-	void (*critical_section_begin)(void);
-	void (*critical_section_end)(void);
-};
-
 static int __igt_atomic_reset_engine(struct intel_engine_cs *engine,
-				     const struct atomic_section *p,
+				     const struct igt_atomic_section *p,
 				     const char *mode)
 {
 	struct tasklet_struct * const t = &engine->execlists.tasklet;
@@ -1665,7 +1630,7 @@ static int __igt_atomic_reset_engine(struct intel_engine_cs *engine,
 }
 
 static int igt_atomic_reset_engine(struct intel_engine_cs *engine,
-				   const struct atomic_section *p)
+				   const struct igt_atomic_section *p)
 {
 	struct drm_i915_private *i915 = engine->i915;
 	struct i915_request *rq;
@@ -1716,79 +1681,43 @@ static int igt_atomic_reset_engine(struct intel_engine_cs *engine,
 	return err;
 }
 
-static void force_reset(struct drm_i915_private *i915)
+static int igt_reset_engines_atomic(void *arg)
 {
-	i915_gem_set_wedged(i915);
-	i915_reset(i915, 0, NULL);
-}
-
-static int igt_atomic_reset(void *arg)
-{
-	static const struct atomic_section phases[] = {
-		{ "preempt", __preempt_begin, __preempt_end },
-		{ "softirq", __softirq_begin, __softirq_end },
-		{ "hardirq", __hardirq_begin, __hardirq_end },
-		{ }
-	};
 	struct drm_i915_private *i915 = arg;
-	intel_wakeref_t wakeref;
+	const typeof(*igt_atomic_phases) *p;
 	int err = 0;
 
-	/* Check that the resets are usable from atomic context */
+	/* Check that the engines resets are usable from atomic context */
+
+	if (!intel_has_reset_engine(i915))
+		return 0;
+
+	if (USES_GUC_SUBMISSION(i915))
+		return 0;
 
 	igt_global_reset_lock(i915);
 	mutex_lock(&i915->drm.struct_mutex);
-	wakeref = intel_runtime_pm_get(i915);
 
 	/* Flush any requests before we get started and check basics */
-	force_reset(i915);
-	if (i915_reset_failed(i915))
+	if (!igt_force_reset(i915))
 		goto unlock;
 
-	if (intel_has_gpu_reset(i915)) {
-		const typeof(*phases) *p;
-
-		for (p = phases; p->name; p++) {
-			GEM_TRACE("intel_gpu_reset under %s\n", p->name);
-
-			p->critical_section_begin();
-			err = intel_gpu_reset(i915, ALL_ENGINES);
-			p->critical_section_end();
-
-			if (err) {
-				pr_err("intel_gpu_reset failed under %s\n",
-				       p->name);
-				goto out;
-			}
-		}
-
-		force_reset(i915);
-	}
-
-	if (USES_GUC_SUBMISSION(i915))
-		goto unlock;
-
-	if (intel_has_reset_engine(i915)) {
+	for (p = igt_atomic_phases; p->name; p++) {
 		struct intel_engine_cs *engine;
 		enum intel_engine_id id;
 
 		for_each_engine(engine, i915, id) {
-			const typeof(*phases) *p;
-
-			for (p = phases; p->name; p++) {
-				err = igt_atomic_reset_engine(engine, p);
-				if (err)
-					goto out;
-			}
+			err = igt_atomic_reset_engine(engine, p);
+			if (err)
+				goto out;
 		}
 	}
 
 out:
 	/* As we poke around the guts, do a full reset before continuing. */
-	force_reset(i915);
+	igt_force_reset(i915);
 
 unlock:
-	intel_runtime_pm_put(i915, wakeref);
 	mutex_unlock(&i915->drm.struct_mutex);
 	igt_global_reset_unlock(i915);
 
@@ -1804,13 +1733,13 @@ int intel_hangcheck_live_selftests(struct drm_i915_private *i915)
 		SUBTEST(igt_reset_idle_engine),
 		SUBTEST(igt_reset_active_engine),
 		SUBTEST(igt_reset_engines),
+		SUBTEST(igt_reset_engines_atomic),
 		SUBTEST(igt_reset_queue),
 		SUBTEST(igt_reset_wait),
 		SUBTEST(igt_reset_evict_ggtt),
 		SUBTEST(igt_reset_evict_ppgtt),
 		SUBTEST(igt_reset_evict_fence),
 		SUBTEST(igt_handle_error),
-		SUBTEST(igt_atomic_reset),
 	};
 	intel_wakeref_t wakeref;
 	bool saved_hangcheck;
diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index ef90101bb87a..0f8ccd8845ab 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -5,6 +5,7 @@
 
 #include "i915_selftest.h"
 #include "selftests/igt_reset.h"
+#include "selftests/igt_atomic.h"
 
 static int igt_global_reset(void *arg)
 {
@@ -54,11 +55,51 @@ static int igt_wedged_reset(void *arg)
 	return i915_reset_failed(i915) ? -EIO : 0;
 }
 
+static int igt_atomic_reset(void *arg)
+{
+	struct drm_i915_private *i915 = arg;
+	const typeof(*igt_atomic_phases) *p;
+	int err = 0;
+
+	/* Check that the resets are usable from atomic context */
+
+	igt_global_reset_lock(i915);
+	mutex_lock(&i915->drm.struct_mutex);
+
+	/* Flush any requests before we get started and check basics */
+	if (!igt_force_reset(i915))
+		goto unlock;
+
+	for (p = igt_atomic_phases; p->name; p++) {
+		GEM_TRACE("intel_gpu_reset under %s\n", p->name);
+
+		p->critical_section_begin();
+		err = intel_gpu_reset(i915, ALL_ENGINES);
+		p->critical_section_end();
+
+		if (err) {
+			pr_err("intel_gpu_reset failed under %s\n",
+				p->name);
+			break;
+		}
+	}
+
+	/* As we poke around the guts, do a full reset before continuing. */
+	igt_force_reset(i915);
+
+unlock:
+	mutex_unlock(&i915->drm.struct_mutex);
+	igt_global_reset_unlock(i915);
+
+	return err;
+}
+
 int intel_reset_live_selftests(struct drm_i915_private *i915)
 {
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_global_reset), /* attempt to recover GPU first */
 		SUBTEST(igt_wedged_reset),
+		SUBTEST(igt_atomic_reset),
 	};
 	intel_wakeref_t wakeref;
 	int err = 0;
diff --git a/drivers/gpu/drm/i915/selftests/igt_atomic.h b/drivers/gpu/drm/i915/selftests/igt_atomic.h
new file mode 100644
index 000000000000..93ec89f487ec
--- /dev/null
+++ b/drivers/gpu/drm/i915/selftests/igt_atomic.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2018 Intel Corporation
+ */
+
+#ifndef IGT_ATOMIC_H
+#define IGT_ATOMIC_H
+
+#include <linux/preempt.h>
+#include <linux/bottom_half.h>
+#include <linux/irqflags.h>
+
+static void __preempt_begin(void)
+{
+	preempt_disable();
+}
+
+static void __preempt_end(void)
+{
+	preempt_enable();
+}
+
+static void __softirq_begin(void)
+{
+	local_bh_disable();
+}
+
+static void __softirq_end(void)
+{
+	local_bh_enable();
+}
+
+static void __hardirq_begin(void)
+{
+	local_irq_disable();
+}
+
+static void __hardirq_end(void)
+{
+	local_irq_enable();
+}
+
+struct igt_atomic_section {
+	const char *name;
+	void (*critical_section_begin)(void);
+	void (*critical_section_end)(void);
+};
+
+static const struct igt_atomic_section igt_atomic_phases[] = {
+	{ "preempt", __preempt_begin, __preempt_end },
+	{ "softirq", __softirq_begin, __softirq_end },
+	{ "hardirq", __hardirq_begin, __hardirq_end },
+	{ }
+};
+
+#endif /* IGT_ATOMIC_H */
diff --git a/drivers/gpu/drm/i915/selftests/igt_reset.c b/drivers/gpu/drm/i915/selftests/igt_reset.c
index 4f31b137c428..587df6fd4ffe 100644
--- a/drivers/gpu/drm/i915/selftests/igt_reset.c
+++ b/drivers/gpu/drm/i915/selftests/igt_reset.c
@@ -43,3 +43,11 @@ void igt_global_reset_unlock(struct drm_i915_private *i915)
 	clear_bit(I915_RESET_BACKOFF, &i915->gpu_error.flags);
 	wake_up_all(&i915->gpu_error.reset_queue);
 }
+
+bool igt_force_reset(struct drm_i915_private *i915)
+{
+	i915_gem_set_wedged(i915);
+	i915_reset(i915, 0, NULL);
+
+	return !i915_reset_failed(i915);
+}
diff --git a/drivers/gpu/drm/i915/selftests/igt_reset.h b/drivers/gpu/drm/i915/selftests/igt_reset.h
index 5f0234d045d5..363bd853e50f 100644
--- a/drivers/gpu/drm/i915/selftests/igt_reset.h
+++ b/drivers/gpu/drm/i915/selftests/igt_reset.h
@@ -11,5 +11,6 @@
 
 void igt_global_reset_lock(struct drm_i915_private *i915);
 void igt_global_reset_unlock(struct drm_i915_private *i915);
+bool igt_force_reset(struct drm_i915_private *i915);
 
 #endif
-- 
2.19.2

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

[PATCH v2 3/9] drm/i915/selftests: Use prepare/finish during atomic reset test

[Michal Wajdeczko]

We were testing full GPU reset in atomic context without correctly
wrapping it by prepare/finish steps. This could confuse our GuC
reset handling code.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/selftest_reset.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
index 0f8ccd8845ab..93860d9856ae 100644
--- a/drivers/gpu/drm/i915/gt/selftest_reset.c
+++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
@@ -74,7 +74,9 @@ static int igt_atomic_reset(void *arg)
 		GEM_TRACE("intel_gpu_reset under %s\n", p->name);
 
 		p->critical_section_begin();
+		reset_prepare(i915);
 		err = intel_gpu_reset(i915, ALL_ENGINES);
+		reset_finish(i915);
 		p->critical_section_end();
 
 		if (err) {
-- 
2.19.2

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

[PATCH v2 4/9] drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded

[Michal Wajdeczko]

This function just check our software flag, while 'is_alive'
may suggest that we are checking runtime firmware status.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.h            | 10 +++++-----
 drivers/gpu/drm/i915/intel_guc_submission.c |  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 2494e84831a2..d4b015ab8a36 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -96,11 +96,6 @@ struct intel_guc {
 	void (*notify)(struct intel_guc *guc);
 };
 
-static inline bool intel_guc_is_alive(struct intel_guc *guc)
-{
-	return intel_uc_fw_is_loaded(&guc->fw);
-}
-
 static
 inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
@@ -176,6 +171,11 @@ u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
 int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
 void intel_guc_release_ggtt_top(struct intel_guc *guc);
 
+static inline bool intel_guc_is_loaded(struct intel_guc *guc)
+{
+	return intel_uc_fw_is_loaded(&guc->fw);
+}
+
 static inline int intel_guc_sanitize(struct intel_guc *guc)
 {
 	intel_uc_fw_sanitize(&guc->fw);
diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index ea0e3734d37c..987ff586d7f9 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -1199,7 +1199,7 @@ static void __guc_client_disable(struct intel_guc_client *client)
 	 * the case, instead of trying (in vain) to communicate with it, let's
 	 * just cleanup the doorbell HW and our internal state.
 	 */
-	if (intel_guc_is_alive(client->guc))
+	if (intel_guc_is_loaded(client->guc))
 		destroy_doorbell(client);
 	else
 		__fini_doorbell(client);
-- 
2.19.2

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

[PATCH v2 5/9] drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish

[Michal Wajdeczko]

Explicitly sanitize GuC/HuC on load failure and when we finish
using them to make sure our fw state tracking is always correct.

While around, use new helper in uc_reset_prepare.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1ee70df51627..415f4058ce2a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -337,14 +337,11 @@ void intel_uc_fini(struct drm_i915_private *i915)
 	intel_guc_fini(guc);
 }
 
-void intel_uc_sanitize(struct drm_i915_private *i915)
+static void __uc_sanitize(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
 	struct intel_huc *huc = &i915->huc;
 
-	if (!USES_GUC(i915))
-		return;
-
 	GEM_BUG_ON(!HAS_GUC(i915));
 
 	intel_huc_sanitize(huc);
@@ -353,6 +350,14 @@ void intel_uc_sanitize(struct drm_i915_private *i915)
 	__intel_uc_reset_hw(i915);
 }
 
+void intel_uc_sanitize(struct drm_i915_private *i915)
+{
+	if (!USES_GUC(i915))
+		return;
+
+	__uc_sanitize(i915);
+}
+
 int intel_uc_init_hw(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
@@ -438,6 +443,8 @@ int intel_uc_init_hw(struct drm_i915_private *i915)
 err_log_capture:
 	guc_capture_load_err_log(guc);
 err_out:
+	__uc_sanitize(i915);
+
 	/*
 	 * Note that there is no fallback as either user explicitly asked for
 	 * the GuC or driver default option was to run with the GuC enabled.
@@ -462,6 +469,7 @@ void intel_uc_fini_hw(struct drm_i915_private *i915)
 		intel_guc_submission_disable(guc);
 
 	guc_disable_communication(guc);
+	__uc_sanitize(i915);
 }
 
 /**
@@ -478,7 +486,7 @@ void intel_uc_reset_prepare(struct drm_i915_private *i915)
 		return;
 
 	guc_disable_communication(guc);
-	intel_uc_sanitize(i915);
+	__uc_sanitize(i915);
 }
 
 void intel_uc_runtime_suspend(struct drm_i915_private *i915)
-- 
2.19.2

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

[PATCH v2 6/9] drm/i915/uc: Use GuC firmware status helper

[Michal Wajdeczko]

We already have helper function for checking GuC firmware
load status. Replace existing open-coded checks.

v2: drop redundant USES_GUC check

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uc.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 415f4058ce2a..1a945f61cf69 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -494,7 +494,7 @@ void intel_uc_runtime_suspend(struct drm_i915_private *i915)
 	struct intel_guc *guc = &i915->guc;
 	int err;
 
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_guc_is_loaded(guc))
 		return;
 
 	err = intel_guc_suspend(guc);
@@ -509,7 +509,7 @@ void intel_uc_suspend(struct drm_i915_private *i915)
 	struct intel_guc *guc = &i915->guc;
 	intel_wakeref_t wakeref;
 
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_guc_is_loaded(guc))
 		return;
 
 	with_intel_runtime_pm(i915, wakeref)
@@ -521,10 +521,7 @@ int intel_uc_resume(struct drm_i915_private *i915)
 	struct intel_guc *guc = &i915->guc;
 	int err;
 
-	if (!USES_GUC(i915))
-		return 0;
-
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+	if (!intel_guc_is_loaded(guc))
 		return 0;
 
 	guc_enable_communication(guc);
-- 
2.19.2

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

[PATCH v2 7/9] drm/i915/uc: Skip GuC HW unwinding if GuC is already dead

[Michal Wajdeczko]

We should not attempt to unwind GuC hardware/firmware setup
if we already have sanitized GuC.

v2: replace USES_GUC with guc_is_loaded

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 1a945f61cf69..3d81a512e5c8 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -460,7 +460,7 @@ void intel_uc_fini_hw(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
 
-	if (!USES_GUC(i915))
+	if (!intel_guc_is_loaded(guc))
 		return;
 
 	GEM_BUG_ON(!HAS_GUC(i915));
-- 
2.19.2

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

[PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Michal Wajdeczko]

Knowing that GuC will be reset soon, we may stop all communication
immediately without doing graceful cleanup as it is not needed.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.h |  5 +++++
 drivers/gpu/drm/i915/intel_uc.c     | 13 ++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index f5e7f0663304..41ba593a4df7 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
 int intel_guc_ct_enable(struct intel_guc_ct *ct);
 void intel_guc_ct_disable(struct intel_guc_ct *ct);
 
+static inline void intel_guc_ct_stop(struct intel_guc_ct *ct)
+{
+	ct->host_channel.enabled = false;
+}
+
 #endif /* _INTEL_GUC_CT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3d81a512e5c8..f17cb3dad90b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -224,6 +224,17 @@ static int guc_enable_communication(struct intel_guc *guc)
 	return 0;
 }
 
+static void guc_stop_communication(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+
+	if (HAS_GUC_CT(i915))
+		intel_guc_ct_stop(&guc->ct);
+
+	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
+}
+
 static void guc_disable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_i915(guc);
@@ -485,7 +496,7 @@ void intel_uc_reset_prepare(struct drm_i915_private *i915)
 	if (!USES_GUC(i915))
 		return;
 
-	guc_disable_communication(guc);
+	guc_stop_communication(guc);
 	__uc_sanitize(i915);
 }
 
-- 
2.19.2

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

[PATCH v2 9/9] drm/i915/uc: Skip reset preparation if GuC is already dead

[Michal Wajdeczko]

We may skip reset preparation steps if GuC is already sanitized.

v2: replace USES_GUC with guc_is_loaded

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index f17cb3dad90b..63fc12cbc25d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -493,7 +493,7 @@ void intel_uc_reset_prepare(struct drm_i915_private *i915)
 {
 	struct intel_guc *guc = &i915->guc;
 
-	if (!USES_GUC(i915))
+	if (!intel_guc_is_loaded(guc))
 		return;
 
 	guc_stop_communication(guc);
-- 
2.19.2

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

Re: [PATCH v2 1/9] drm/i915/selftests: Move some reset testcases to separate file

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:31:55)
> igt_global_reset and igt_wedged_reset testcases are first candidates.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

> diff --git a/drivers/gpu/drm/i915/gt/selftest_reset.c b/drivers/gpu/drm/i915/gt/selftest_reset.c
> new file mode 100644
> index 000000000000..ef90101bb87a
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_reset.c
> @@ -0,0 +1,76 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2018 Intel Corporation
> + */
> +
> +#include "i915_selftest.h"
> +#include "selftests/igt_reset.h"

Oh, I missed that helper.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/9] drm/i915/selftests: Split igt_atomic_reset testcase

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:31:56)
> Split igt_atomic_reset selftests into separate full & engines parts,
> so we can move former to the dedicated reset selftests file.
> 
> While here change engines test to loop first over atomic phases and
> then loop over available engines.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Ok, but reminds me we need to move all the per-engine resets over from
selftest_hangcheck.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 3/9] drm/i915/selftests: Use prepare/finish during atomic reset test

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:31:57)
> We were testing full GPU reset in atomic context without correctly
> wrapping it by prepare/finish steps. This could confuse our GuC
> reset handling code.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/9] drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:31:58)
> This function just check our software flag, while 'is_alive'
> may suggest that we are checking runtime firmware status.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.h            | 10 +++++-----
>  drivers/gpu/drm/i915/intel_guc_submission.c |  2 +-
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 2494e84831a2..d4b015ab8a36 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -96,11 +96,6 @@ struct intel_guc {
>         void (*notify)(struct intel_guc *guc);
>  };
>  
> -static inline bool intel_guc_is_alive(struct intel_guc *guc)
> -{
> -       return intel_uc_fw_is_loaded(&guc->fw);
> -}
> -
>  static
>  inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
>  {
> @@ -176,6 +171,11 @@ u32 intel_guc_reserved_gtt_size(struct intel_guc *guc);
>  int intel_guc_reserve_ggtt_top(struct intel_guc *guc);
>  void intel_guc_release_ggtt_top(struct intel_guc *guc);
>  
> +static inline bool intel_guc_is_loaded(struct intel_guc *guc)
> +{
> +       return intel_uc_fw_is_loaded(&guc->fw);
> +}

Aye, a better reflection.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/9] drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:31:59)
> Explicitly sanitize GuC/HuC on load failure and when we finish
> using them to make sure our fw state tracking is always correct.
> 
> While around, use new helper in uc_reset_prepare.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 6/9] drm/i915/uc: Use GuC firmware status helper

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:32:00)
> We already have helper function for checking GuC firmware
> load status. Replace existing open-coded checks.
> 
> v2: drop redundant USES_GUC check
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 7/9] drm/i915/uc: Skip GuC HW unwinding if GuC is already dead

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:32:01)
> We should not attempt to unwind GuC hardware/firmware setup
> if we already have sanitized GuC.
> 
> v2: replace USES_GUC with guc_is_loaded
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 9/9] drm/i915/uc: Skip reset preparation if GuC is already dead

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:32:03)
> We may skip reset preparation steps if GuC is already sanitized.
> 
> v2: replace USES_GUC with guc_is_loaded
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-22 20:32:02)
> Knowing that GuC will be reset soon, we may stop all communication
> immediately without doing graceful cleanup as it is not needed.

The difference between stop and disable is that it avoids the
serialisation, right? Is this patch still required -- do we still
need the complication in two similar but subtly different paths?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for GuC fixes (rev2)

[Patchwork]

== Series Details ==

Series: GuC fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/60795/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
54ff1dca4c58 drm/i915/selftests: Move some reset testcases to separate file
-:95: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#95: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 152 lines checked
010025dfb682 drm/i915/selftests: Split igt_atomic_reset testcase
-:233: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#233: FILE: drivers/gpu/drm/i915/gt/selftest_reset.c:82:
+			pr_err("intel_gpu_reset failed under %s\n",
+				p->name);

-:258: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#258: 
new file mode 100644

total: 0 errors, 1 warnings, 1 checks, 299 lines checked
a24312534312 drm/i915/selftests: Use prepare/finish during atomic reset test
346a854fbd49 drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded
6a0e54a07d39 drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish
599cfb7e0a02 drm/i915/uc: Use GuC firmware status helper
cea15b131008 drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
191f064b4da3 drm/i915/uc: Stop talking with GuC when resetting
df2d1b431445 drm/i915/uc: Skip reset preparation if GuC is already dead

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

✗ Fi.CI.BAT: failure for GuC fixes (rev2)

[Patchwork]

== Series Details ==

Series: GuC fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/60795/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6123 -> Patchwork_13075
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13075 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13075, 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_13075/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-apl-guc:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-apl-guc/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-apl-guc/igt@i915_module_load@reload.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-apl-guc:         [FAIL][3] ([fdo#110622]) -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-apl-guc/igt@runner@aborted.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-apl-guc/igt@runner@aborted.html

  
New tests
---------

  New tests have been introduced between CI_DRM_6123 and Patchwork_13075:

### New IGT tests (1) ###

  * igt@i915_selftest@live_reset:
    - Statuses : 43 pass(s)
    - Exec time: [0.31, 1.29] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       [PASS][5] -> [DMESG-WARN][6] ([fdo#108965])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html

  * igt@i915_selftest@live_contexts:
    - fi-skl-gvtdvm:      [PASS][7] -> [DMESG-FAIL][8] ([fdo#110235])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][9] ([fdo#107718]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@gem_render_linear_blits@basic:
    - {fi-icl-u3}:        [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12] +2 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-icl-u3/igt@gem_render_linear_blits@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-icl-u3/igt@gem_render_linear_blits@basic.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][13] ([fdo#108511]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [DMESG-FAIL][15] ([fdo#110235]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

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

  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622


Participating hosts (53 -> 45)
------------------------------

  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-cfl-8109u fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6123 -> Patchwork_13075

  CI_DRM_6123: d37dd1f4dfcc7a8814fd27f8bdfa97ea5c0a9bd3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5005: adf9f435a795d692e30cd6eafe26eddf4993c8ff @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13075: df2d1b4314459cf88eddc80b6d13a54446cfbccf @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

df2d1b431445 drm/i915/uc: Skip reset preparation if GuC is already dead
191f064b4da3 drm/i915/uc: Stop talking with GuC when resetting
cea15b131008 drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
599cfb7e0a02 drm/i915/uc: Use GuC firmware status helper
6a0e54a07d39 drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish
346a854fbd49 drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded
a24312534312 drm/i915/selftests: Use prepare/finish during atomic reset test
010025dfb682 drm/i915/selftests: Split igt_atomic_reset testcase
54ff1dca4c58 drm/i915/selftests: Move some reset testcases to separate file

== Logs ==

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

Re: ✗ Fi.CI.BAT: failure for GuC fixes (rev2)

[Michal Wajdeczko]

On Wed, 22 May 2019 22:53:11 +0200, Patchwork  
<patchwork@emeril.freedesktop.org> wrote:

> == Series Details ==
>
> Series: GuC fixes (rev2)
> URL   : https://patchwork.freedesktop.org/series/60795/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_6123 -> Patchwork_13075
> ====================================================
>
> Summary
> -------
>
>   **FAILURE**
>
>   Serious unknown changes coming with Patchwork_13075 absolutely need to  
> be
>   verified manually.
>  If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_13075, 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_13075/
>
> Possible new issues
> -------------------
>
>   Here are the unknown changes that may have been introduced in  
> Patchwork_13075:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
>   * igt@i915_module_load@reload:
>     - fi-apl-guc:         [PASS][1] -> [DMESG-WARN][2]
>    [1]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-apl-guc/igt@i915_module_load@reload.html
>    [2]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-apl-guc/igt@i915_module_load@reload.html

these are doorbells warnings at unload/load when GuC submission is enabled:

<7> [309.389262] [drm:doorbell_ok [i915]] Doorbell 0 has unexpected state:  
valid=yes
<7> [309.389683] [drm:doorbell_ok [i915]] Doorbell 128 has unexpected  
state: valid=yes
<4> [309.390061] ------------[ cut here ]------------
<4> [309.390067] WARN_ON(!guc_verify_doorbells(guc))
<4> [309.390360] Call Trace:
<4> [309.390445]  intel_uc_fini+0x46/0x140 [i915]
<4> [309.390520]  i915_gem_fini+0x70/0x1a0 [i915]
<4> [309.390585]  i915_driver_unload+0xdd/0x130 [i915]
<4> [309.390651]  i915_pci_remove+0x19/0x30 [i915]

...

<7> [310.812673] [drm:doorbell_ok [i915]] Doorbell 0 has unexpected state:  
valid=yes
<7> [310.813014] [drm:doorbell_ok [i915]] Doorbell 128 has unexpected  
state: valid=yes
<4> [310.813290] ------------[ cut here ]------------
<4> [310.813295] WARN_ON(!guc_verify_doorbells(guc))
<4> [310.813646] Call Trace:
<4> [310.813755]  intel_uc_init+0xc8/0x1f0 [i915]
<4> [310.813856]  i915_gem_init+0x49b/0xa90 [i915]
<4> [310.813945]  i915_driver_load+0xdb8/0x18b0 [i915]

and can be fixed by patch [a], but since we are going to disable GuC
submission soon [b] maybe we don't care to fix that, as it works [c].

also note that newer GuC takes care of the doorbells maintenance, so
above warnings will be simply removed.

[a] https://patchwork.freedesktop.org/patch/305573/?series=60735&rev=3
[b] https://patchwork.freedesktop.org/patch/305562/?series=60735&rev=3
[c] https://patchwork.freedesktop.org/series/60925/


>
> #### Warnings ####
>
>   * igt@runner@aborted:
>     - fi-apl-guc:         [FAIL][3] ([fdo#110622]) -> [FAIL][4]
>    [3]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-apl-guc/igt@runner@aborted.html
>    [4]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-apl-guc/igt@runner@aborted.html
>
> New tests
> ---------
>
>   New tests have been introduced between CI_DRM_6123 and Patchwork_13075:
>
> ### New IGT tests (1) ###
>
>   * igt@i915_selftest@live_reset:
>     - Statuses : 43 pass(s)
>     - Exec time: [0.31, 1.29] s
>
>
> Known issues
> ------------
>
>   Here are the changes found in Patchwork_13075 that come from known  
> issues:
>
> ### IGT changes ###
>
> #### Issues hit ####
>
>   * igt@amdgpu/amd_basic@userptr:
>     - fi-kbl-8809g:       [PASS][5] -> [DMESG-WARN][6] ([fdo#108965])
>    [5]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
>    [6]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-kbl-8809g/igt@amdgpu/amd_basic@userptr.html
>
>   * igt@i915_selftest@live_contexts:
>     - fi-skl-gvtdvm:      [PASS][7] -> [DMESG-FAIL][8] ([fdo#110235])
>    [7]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
>    [8]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-skl-gvtdvm/igt@i915_selftest@live_contexts.html
>
> #### Possible fixes ####
>
>   * igt@gem_exec_suspend@basic-s3:
>     - fi-blb-e6850:       [INCOMPLETE][9] ([fdo#107718]) -> [PASS][10]
>    [9]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
>    [10]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
>
>   * igt@gem_render_linear_blits@basic:
>     - {fi-icl-u3}:        [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]  
> +2 similar issues
>    [11]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-icl-u3/igt@gem_render_linear_blits@basic.html
>    [12]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-icl-u3/igt@gem_render_linear_blits@basic.html
>
>   * igt@i915_pm_rpm@module-reload:
>     - fi-skl-6770hq:      [FAIL][13] ([fdo#108511]) -> [PASS][14]
>    [13]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>    [14]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
>
>   * igt@i915_selftest@live_contexts:
>     - fi-bdw-gvtdvm:      [DMESG-FAIL][15] ([fdo#110235]) -> [PASS][16]
>    [15]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
>    [16]:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
>
>  {name}: This element is suppressed. This means it is ignored when  
> computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
>
>   [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
>   [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
>   [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
>   [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
>   [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
>   [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
>   [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
>   [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622
>
>
> Participating hosts (53 -> 45)
> ------------------------------
>
>   Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks  
> fi-bsw-cyan fi-cfl-8109u fi-byt-clapper fi-bdw-samus
>
>
> Build changes
> -------------
>
>   * Linux: CI_DRM_6123 -> Patchwork_13075
>
>   CI_DRM_6123: d37dd1f4dfcc7a8814fd27f8bdfa97ea5c0a9bd3 @  
> git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_5005: adf9f435a795d692e30cd6eafe26eddf4993c8ff @  
> git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_13075: df2d1b4314459cf88eddc80b6d13a54446cfbccf @  
> git://anongit.freedesktop.org/gfx-ci/linux
>
>
> == Linux commits ==
>
> df2d1b431445 drm/i915/uc: Skip reset preparation if GuC is already dead
> 191f064b4da3 drm/i915/uc: Stop talking with GuC when resetting
> cea15b131008 drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
> 599cfb7e0a02 drm/i915/uc: Use GuC firmware status helper
> 6a0e54a07d39 drm/i915/uc: Explicitly sanitize GuC/HuC on failure and  
> finish
> 346a854fbd49 drm/i915/guc: Rename intel_guc_is_alive to  
> intel_guc_is_loaded
> a24312534312 drm/i915/selftests: Use prepare/finish during atomic reset  
> test
> 010025dfb682 drm/i915/selftests: Split igt_atomic_reset testcase
> 54ff1dca4c58 drm/i915/selftests: Move some reset testcases to separate  
> file
>
> == Logs ==
>
> For more details see:  
> https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Michal Wajdeczko]

On Wed, 22 May 2019 22:06:53 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2019-05-22 20:32:02)
>> Knowing that GuC will be reset soon, we may stop all communication
>> immediately without doing graceful cleanup as it is not needed.
>
> The difference between stop and disable is that it avoids the
> serialisation, right? Is this patch still required -- do we still
> need the complication in two similar but subtly different paths?

This patch helps us capture any unwanted/unexpected attempts to talk
with GuC after we decided to reset it. We need 'disable' part as current
and upcoming firmware still expects us to do graceful cleanup. This may
be changed in the future releases. And we can use 'disable' as this will
break atomic_reset tests.

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

Re: [PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-23 16:53:27)
> On Wed, 22 May 2019 22:06:53 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2019-05-22 20:32:02)
> >> Knowing that GuC will be reset soon, we may stop all communication
> >> immediately without doing graceful cleanup as it is not needed.
> >
> > The difference between stop and disable is that it avoids the
> > serialisation, right? Is this patch still required -- do we still
> > need the complication in two similar but subtly different paths?
> 
> This patch helps us capture any unwanted/unexpected attempts to talk
> with GuC after we decided to reset it. We need 'disable' part as current
> and upcoming firmware still expects us to do graceful cleanup. This may
> be changed in the future releases. And we can use 'disable' as this will
> break atomic_reset tests.

Please promote that to the commitmsg :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Michal Wajdeczko]

Knowing that GuC will be reset soon, we may stop all communication
immediately without doing graceful cleanup as it is not needed.

This patch will also help us capture any unwanted/unexpected attempts
to talk with GuC after we decided to reset it. And we need to keep
'disable' part as current and upcoming firmware still expect graceful
cleanup.

v2: update commit msg

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.h |  5 +++++
 drivers/gpu/drm/i915/intel_uc.c     | 13 ++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index f5e7f0663304..41ba593a4df7 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -96,4 +96,9 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct);
 int intel_guc_ct_enable(struct intel_guc_ct *ct);
 void intel_guc_ct_disable(struct intel_guc_ct *ct);
 
+static inline void intel_guc_ct_stop(struct intel_guc_ct *ct)
+{
+	ct->host_channel.enabled = false;
+}
+
 #endif /* _INTEL_GUC_CT_H_ */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 3d81a512e5c8..f17cb3dad90b 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -224,6 +224,17 @@ static int guc_enable_communication(struct intel_guc *guc)
 	return 0;
 }
 
+static void guc_stop_communication(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+
+	if (HAS_GUC_CT(i915))
+		intel_guc_ct_stop(&guc->ct);
+
+	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
+}
+
 static void guc_disable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *i915 = guc_to_i915(guc);
@@ -485,7 +496,7 @@ void intel_uc_reset_prepare(struct drm_i915_private *i915)
 	if (!USES_GUC(i915))
 		return;
 
-	guc_disable_communication(guc);
+	guc_stop_communication(guc);
 	__uc_sanitize(i915);
 }
 
-- 
2.19.2

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

✗ Fi.CI.CHECKPATCH: warning for GuC fixes (rev3)

[Patchwork]

== Series Details ==

Series: GuC fixes (rev3)
URL   : https://patchwork.freedesktop.org/series/60795/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9a0b5c696229 drm/i915/selftests: Move some reset testcases to separate file
-:95: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#95: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 152 lines checked
1926469a0fcf drm/i915/selftests: Split igt_atomic_reset testcase
-:233: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#233: FILE: drivers/gpu/drm/i915/gt/selftest_reset.c:82:
+			pr_err("intel_gpu_reset failed under %s\n",
+				p->name);

-:258: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#258: 
new file mode 100644

total: 0 errors, 1 warnings, 1 checks, 299 lines checked
32781025d4a6 drm/i915/selftests: Use prepare/finish during atomic reset test
f822eaa25eab drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded
1ae3bc32d6de drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish
84c28a98d6e7 drm/i915/uc: Use GuC firmware status helper
b178f2c6201a drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
8da418b5e516 drm/i915/uc: Stop talking with GuC when resetting
0b45f1494628 drm/i915/uc: Skip reset preparation if GuC is already dead

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

✗ Fi.CI.BAT: failure for GuC fixes (rev3)

[Patchwork]

== Series Details ==

Series: GuC fixes (rev3)
URL   : https://patchwork.freedesktop.org/series/60795/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6133 -> Patchwork_13082
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13082 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13082, 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_13082/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@reload:
    - fi-apl-guc:         [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-apl-guc/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-apl-guc/igt@i915_module_load@reload.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-apl-guc:         [FAIL][3] ([fdo#110622]) -> [FAIL][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-apl-guc/igt@runner@aborted.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-apl-guc/igt@runner@aborted.html

  
New tests
---------

  New tests have been introduced between CI_DRM_6133 and Patchwork_13082:

### New IGT tests (1) ###

  * igt@i915_selftest@live_reset:
    - Statuses : 44 pass(s)
    - Exec time: [0.35, 1.31] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-skl-6600u:       [PASS][5] -> [FAIL][6] ([fdo#107707])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-skl-6600u/igt@i915_pm_rpm@basic-pci-d3-state.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-skl-6600u/igt@i915_pm_rpm@basic-pci-d3-state.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       [INCOMPLETE][7] ([fdo#107718]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-blb-e6850/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_module_load@reload-no-display:
    - {fi-icl-u3}:        [DMESG-WARN][9] ([fdo#110718]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-icl-u3/igt@i915_module_load@reload-no-display.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-icl-u3/igt@i915_module_load@reload-no-display.html

  * igt@i915_selftest@live_coherency:
    - fi-pnv-d510:        [INCOMPLETE][11] ([fdo#110740]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-pnv-d510/igt@i915_selftest@live_coherency.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-pnv-d510/igt@i915_selftest@live_coherency.html

  * igt@kms_frontbuffer_tracking@basic:
    - {fi-icl-u2}:        [FAIL][13] ([fdo#103167]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [DMESG-WARN][15] ([fdo#106387]) -> [PASS][16] +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6133/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13082/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107707]: https://bugs.freedesktop.org/show_bug.cgi?id=107707
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622
  [fdo#110718]: https://bugs.freedesktop.org/show_bug.cgi?id=110718
  [fdo#110740]: https://bugs.freedesktop.org/show_bug.cgi?id=110740


Participating hosts (52 -> 45)
------------------------------

  Additional (1): fi-byt-j1900 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6133 -> Patchwork_13082

  CI_DRM_6133: c22847d8bc09118895483b277cbe4bf4f82ac444 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5011: 7f120c5f1bff2727d50f3c392d81c0f6878b61d6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13082: 0b45f149462880126e70daa79790f4b4bf84a2a5 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0b45f1494628 drm/i915/uc: Skip reset preparation if GuC is already dead
8da418b5e516 drm/i915/uc: Stop talking with GuC when resetting
b178f2c6201a drm/i915/uc: Skip GuC HW unwinding if GuC is already dead
84c28a98d6e7 drm/i915/uc: Use GuC firmware status helper
1ae3bc32d6de drm/i915/uc: Explicitly sanitize GuC/HuC on failure and finish
f822eaa25eab drm/i915/guc: Rename intel_guc_is_alive to intel_guc_is_loaded
32781025d4a6 drm/i915/selftests: Use prepare/finish during atomic reset test
1926469a0fcf drm/i915/selftests: Split igt_atomic_reset testcase
9a0b5c696229 drm/i915/selftests: Move some reset testcases to separate file

== Logs ==

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

Re: [PATCH v2 8/9] drm/i915/uc: Stop talking with GuC when resetting

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-23 18:25:55)
> Knowing that GuC will be reset soon, we may stop all communication
> immediately without doing graceful cleanup as it is not needed.
> 
> This patch will also help us capture any unwanted/unexpected attempts
> to talk with GuC after we decided to reset it. And we need to keep
> 'disable' part as current and upcoming firmware still expect graceful
> cleanup.
> 
> v2: update commit msg
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

I'm a bit more reassured now that there is some purpose to this :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✗ Fi.CI.BAT: failure for GuC fixes (rev2)

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-23 15:46:58)
> On Wed, 22 May 2019 22:53:11 +0200, Patchwork  
> <patchwork@emeril.freedesktop.org> wrote:
> 
> > == Series Details ==
> >
> > Series: GuC fixes (rev2)
> > URL   : https://patchwork.freedesktop.org/series/60795/
> > State : failure
> >
> > == Summary ==
> >
> > CI Bug Log - changes from CI_DRM_6123 -> Patchwork_13075
> > ====================================================
> >
> > Summary
> > -------
> >
> >   **FAILURE**
> >
> >   Serious unknown changes coming with Patchwork_13075 absolutely need to  
> > be
> >   verified manually.
> >  If you think the reported changes have nothing to do with the changes
> >   introduced in Patchwork_13075, 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_13075/
> >
> > Possible new issues
> > -------------------
> >
> >   Here are the unknown changes that may have been introduced in  
> > Patchwork_13075:
> >
> > ### IGT changes ###
> >
> > #### Possible regressions ####
> >
> >   * igt@i915_module_load@reload:
> >     - fi-apl-guc:         [PASS][1] -> [DMESG-WARN][2]
> >    [1]:  
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6123/fi-apl-guc/igt@i915_module_load@reload.html
> >    [2]:  
> > https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13075/fi-apl-guc/igt@i915_module_load@reload.html
> 
> these are doorbells warnings at unload/load when GuC submission is enabled:
> 
> <7> [309.389262] [drm:doorbell_ok [i915]] Doorbell 0 has unexpected state:  
> valid=yes
> <7> [309.389683] [drm:doorbell_ok [i915]] Doorbell 128 has unexpected  
> state: valid=yes
> <4> [309.390061] ------------[ cut here ]------------
> <4> [309.390067] WARN_ON(!guc_verify_doorbells(guc))
> <4> [309.390360] Call Trace:
> <4> [309.390445]  intel_uc_fini+0x46/0x140 [i915]
> <4> [309.390520]  i915_gem_fini+0x70/0x1a0 [i915]
> <4> [309.390585]  i915_driver_unload+0xdd/0x130 [i915]
> <4> [309.390651]  i915_pci_remove+0x19/0x30 [i915]
> 
> ...
> 
> <7> [310.812673] [drm:doorbell_ok [i915]] Doorbell 0 has unexpected state:  
> valid=yes
> <7> [310.813014] [drm:doorbell_ok [i915]] Doorbell 128 has unexpected  
> state: valid=yes
> <4> [310.813290] ------------[ cut here ]------------
> <4> [310.813295] WARN_ON(!guc_verify_doorbells(guc))
> <4> [310.813646] Call Trace:
> <4> [310.813755]  intel_uc_init+0xc8/0x1f0 [i915]
> <4> [310.813856]  i915_gem_init+0x49b/0xa90 [i915]
> <4> [310.813945]  i915_driver_load+0xdb8/0x18b0 [i915]
> 
> and can be fixed by patch [a], but since we are going to disable GuC
> submission soon [b] maybe we don't care to fix that, as it works [c].

Hmm, it's a bit of nuisance as it means CI stops running tests and we
miss the selftests for guc/apl.

I don't think that's a huge issue as we have your gen11 enabling patch
to land in the very near future which as you say makes this a non-issue.
Till then we will just have to upset Martin!

Thanks for the patches, pushed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx