[PATCH 1/4] drm/i915: Switch to kernel context before idling at runtime

[PATCH 1/4] drm/i915: Switch to kernel context before idling at runtime

[Chris Wilson]

We can reduce our exposure to random neutrinos by resting on the kernel
context having flushed out the user contexts to system memory and
beyond. The corollary is that we then we require two passes through the
idle handler to go to sleep, which on a truly idle system involves an
extra pass through the slow and irregular retire work handler.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  9 +++++--
 drivers/gpu/drm/i915/i915_gem.c         | 36 +++++++++++++++++++++----
 drivers/gpu/drm/i915/i915_gem_context.c |  2 +-
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a8e7761cdc7d..15e86d34a81c 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4226,8 +4226,13 @@ i915_drop_caches_set(void *data, u64 val)
 		i915_gem_shrink_all(dev_priv);
 	fs_reclaim_release(GFP_KERNEL);
 
-	if (val & DROP_IDLE)
-		drain_delayed_work(&dev_priv->gt.idle_work);
+	if (val & DROP_IDLE) {
+		do {
+			if (READ_ONCE(dev_priv->gt.active_requests))
+				flush_delayed_work(&dev_priv->gt.retire_work);
+			drain_delayed_work(&dev_priv->gt.idle_work);
+		} while (READ_ONCE(dev_priv->gt.awake));
+	}
 
 	if (val & DROP_FREED)
 		i915_gem_drain_freed_objects(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 530d6d0109b4..173d1e4ad963 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -139,6 +139,8 @@ int i915_mutex_lock_interruptible(struct drm_device *dev)
 
 static u32 __i915_gem_park(struct drm_i915_private *i915)
 {
+	GEM_TRACE("\n");
+
 	lockdep_assert_held(&i915->drm.struct_mutex);
 	GEM_BUG_ON(i915->gt.active_requests);
 	GEM_BUG_ON(!list_empty(&i915->gt.active_rings));
@@ -181,6 +183,8 @@ static u32 __i915_gem_park(struct drm_i915_private *i915)
 
 void i915_gem_park(struct drm_i915_private *i915)
 {
+	GEM_TRACE("\n");
+
 	lockdep_assert_held(&i915->drm.struct_mutex);
 	GEM_BUG_ON(i915->gt.active_requests);
 
@@ -193,6 +197,8 @@ void i915_gem_park(struct drm_i915_private *i915)
 
 void i915_gem_unpark(struct drm_i915_private *i915)
 {
+	GEM_TRACE("\n");
+
 	lockdep_assert_held(&i915->drm.struct_mutex);
 	GEM_BUG_ON(!i915->gt.active_requests);
 
@@ -3503,6 +3509,24 @@ i915_gem_idle_work_handler(struct work_struct *work)
 	if (!READ_ONCE(dev_priv->gt.awake))
 		return;
 
+	if (READ_ONCE(dev_priv->gt.active_requests))
+		return;
+
+	/*
+	 * Flush out the last user context, leaving only the pinned
+	 * kernel context resident. When we are idling on the kernel_context,
+	 * no more new requests (with a context switch) are emitted and we
+	 * can finally rest. A consequence is that the idle work handler is
+	 * always called at least twice before idling (and if the system is
+	 * idle that implies a round trip through the retire worker).
+	 */
+	mutex_lock(&dev_priv->drm.struct_mutex);
+	i915_gem_switch_to_kernel_context(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+
+	GEM_TRACE("active_requests=%d (after switch-to-kernel-context)\n",
+		  READ_ONCE(dev_priv->gt.active_requests));
+
 	/*
 	 * Wait for last execlists context complete, but bail out in case a
 	 * new request is submitted. As we don't trust the hardware, we
@@ -4913,11 +4937,9 @@ static void assert_kernel_context_is_current(struct drm_i915_private *i915)
 
 void i915_gem_sanitize(struct drm_i915_private *i915)
 {
-	if (i915_terminally_wedged(&i915->gpu_error)) {
-		mutex_lock(&i915->drm.struct_mutex);
+	mutex_lock(&i915->drm.struct_mutex);
+	if (i915_terminally_wedged(&i915->gpu_error))
 		i915_gem_unset_wedged(i915);
-		mutex_unlock(&i915->drm.struct_mutex);
-	}
 
 	/*
 	 * If we inherit context state from the BIOS or earlier occupants
@@ -4929,6 +4951,9 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	 */
 	if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915))
 		WARN_ON(intel_gpu_reset(i915, ALL_ENGINES));
+
+	i915_gem_contexts_lost(i915);
+	mutex_unlock(&i915->drm.struct_mutex);
 }
 
 int i915_gem_suspend(struct drm_i915_private *dev_priv)
@@ -4964,7 +4989,6 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 		assert_kernel_context_is_current(dev_priv);
 	}
-	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
 	intel_uc_suspend(dev_priv);
@@ -5017,6 +5041,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 void i915_gem_resume(struct drm_i915_private *i915)
 {
+	GEM_TRACE("\n");
+
 	WARN_ON(i915->gt.awake);
 
 	mutex_lock(&i915->drm.struct_mutex);
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 45393f6e0208..81f086397d10 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -649,7 +649,7 @@ int i915_gem_switch_to_kernel_context(struct drm_i915_private *i915)
 	struct intel_engine_cs *engine;
 	enum intel_engine_id id;
 
-	GEM_TRACE("\n");
+	GEM_TRACE("awake?=%s\n", yesno(i915->gt.awake));
 
 	lockdep_assert_held(&i915->drm.struct_mutex);
 	GEM_BUG_ON(!i915->kernel_context);
-- 
2.17.0

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

[PATCH 2/4] drm/i915: "Race-to-idle" after switching to the kernel context

[Chris Wilson]

During suspend we want to flush out all active contexts and their
rendering. To do so we queue a request from the kernel's context, once
we know that request is done, we know the GPU is completely idle. To
speed up that switch bump the GPU clocks.

Switching to the kernel context prior to idling is also used to enforce
a barrier before changing OA properties, and when evicting active
rendering from the global GTT. All cases where we do want to
race-to-idle.

v2: Limit the boosting to only the switch before suspend.
v3: Limit it to the wait-for-idle on suspend.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Tested-by: David Weinehall <david.weinehall@linux.intel.com> #v1
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c     | 27 +++++++++++++++++++++++++--
 drivers/gpu/drm/i915/i915_request.h |  1 +
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 173d1e4ad963..b312ac006d24 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3708,7 +3708,29 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
 
 static int wait_for_timeline(struct i915_timeline *tl, unsigned int flags)
 {
-	return i915_gem_active_wait(&tl->last_request, flags);
+	struct i915_request *rq;
+	long ret;
+
+	rq = i915_gem_active_get_unlocked(&tl->last_request);
+	if (!rq)
+		return 0;
+
+	/*
+	 * "Race-to-idle".
+	 *
+	 * Switching to the kernel context is often used a synchronous
+	 * step prior to idling, e.g. in suspend for flushing all
+	 * current operations to memory before sleeping. These we
+	 * want to complete as quickly as possible to avoid prolonged
+	 * stalls, so allow the gpu to boost to maximum clocks.
+	 */
+	if (flags & I915_WAIT_FOR_IDLE_BOOST)
+		gen6_rps_boost(rq, NULL);
+
+	ret = i915_request_wait(rq, flags, MAX_SCHEDULE_TIMEOUT);
+	i915_request_put(rq);
+
+	return ret < 0 ? ret : 0;
 }
 
 static int wait_for_engines(struct drm_i915_private *i915)
@@ -4983,7 +5005,8 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 
 		ret = i915_gem_wait_for_idle(dev_priv,
 					     I915_WAIT_INTERRUPTIBLE |
-					     I915_WAIT_LOCKED);
+					     I915_WAIT_LOCKED |
+					     I915_WAIT_FOR_IDLE_BOOST);
 		if (ret && ret != -EIO)
 			goto err_unlock;
 
diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h
index 1bbbb7a9fa03..491ff81d0fea 100644
--- a/drivers/gpu/drm/i915/i915_request.h
+++ b/drivers/gpu/drm/i915/i915_request.h
@@ -267,6 +267,7 @@ long i915_request_wait(struct i915_request *rq,
 #define I915_WAIT_INTERRUPTIBLE	BIT(0)
 #define I915_WAIT_LOCKED	BIT(1) /* struct_mutex held, handle GPU reset */
 #define I915_WAIT_ALL		BIT(2) /* used by i915_gem_object_wait() */
+#define I915_WAIT_FOR_IDLE_BOOST BIT(3)
 
 static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine);
 
-- 
2.17.0

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

[PATCH 3/4] drm/i915: After reset on sanitization, reset the engine backends

[Chris Wilson]

As we reset the GPU on suspend/resume, we also do need to reset the
engine state tracking so call into the engine backends. This is
especially important so that we can also sanitize the state tracking
across resume.

References: https://bugs.freedesktop.org/show_bug.cgi?id=106702
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c  | 24 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c |  8 +++++---
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b312ac006d24..9b8fa1866cc9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4959,7 +4959,22 @@ static void assert_kernel_context_is_current(struct drm_i915_private *i915)
 
 void i915_gem_sanitize(struct drm_i915_private *i915)
 {
+	struct intel_engine_cs *engine;
+	enum intel_engine_id id;
+
+	GEM_TRACE("\n");
+
 	mutex_lock(&i915->drm.struct_mutex);
+
+	intel_runtime_pm_get(i915);
+	intel_uncore_forcewake_get(i915, FORCEWAKE_ALL);
+
+	/*
+	 * As we have just resumed the machine and woken the device up from
+	 * deep PCI sleep (presumably D3_cold), assume the HW has been reset
+	 * back to defaults, recovering from whatever wedged state we left it
+	 * in and so worth trying to use the device once more.
+	 */
 	if (i915_terminally_wedged(&i915->gpu_error))
 		i915_gem_unset_wedged(i915);
 
@@ -4974,6 +4989,15 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	if (INTEL_GEN(i915) >= 5 && intel_has_gpu_reset(i915))
 		WARN_ON(intel_gpu_reset(i915, ALL_ENGINES));
 
+	/* Reset the submission backend after resume as well as the GPU reset */
+	for_each_engine(engine, i915, id) {
+		if (engine->reset.reset)
+			engine->reset.reset(engine, NULL);
+	}
+
+	intel_uncore_forcewake_put(i915, FORCEWAKE_ALL);
+	intel_runtime_pm_put(i915);
+
 	i915_gem_contexts_lost(i915);
 	mutex_unlock(&i915->drm.struct_mutex);
 }
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8a6058b4102f..517e92c6a70b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1787,9 +1787,6 @@ static void enable_execlists(struct intel_engine_cs *engine)
 	I915_WRITE(RING_HWS_PGA(engine->mmio_base),
 		   engine->status_page.ggtt_offset);
 	POSTING_READ(RING_HWS_PGA(engine->mmio_base));
-
-	/* Following the reset, we need to reload the CSB read/write pointers */
-	engine->execlists.csb_head = -1;
 }
 
 static bool unexpected_starting_state(struct intel_engine_cs *engine)
@@ -1961,6 +1958,9 @@ static void execlists_reset(struct intel_engine_cs *engine,
 	__unwind_incomplete_requests(engine);
 	spin_unlock(&engine->timeline.lock);
 
+	/* Following the reset, we need to reload the CSB read/write pointers */
+	engine->execlists.csb_head = -1;
+
 	local_irq_restore(flags);
 
 	/*
@@ -2460,6 +2460,8 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 			upper_32_bits(ce->lrc_desc);
 	}
 
+	engine->execlists.csb_head = -1;
+
 	return 0;
 
 error:
-- 
2.17.0

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

[PATCH 4/4] drm/i915: Only sanitize GEM from late suspend

[Chris Wilson]

During testing we encounter a conflict between SUSPEND_TEST_DEVICES and
disabling reset (gem_eio/suspend). This results in the device continuing
on without being reset, but since it has gone through HW sanitization to
account for the suspend/resume cycle, we have to assume the device has
been reset to its defaults. A simple way around this is to skip the
sanitize phase for SUSPEND_TEST_DEVICES by moving it to suspend-late.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c |  6 +++++-
 drivers/gpu/drm/i915/i915_drv.h |  1 +
 drivers/gpu/drm/i915/i915_gem.c | 22 +++++++++++++---------
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8f002ae22e62..0d9b8cc0436d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -636,6 +636,8 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
 
 static void i915_gem_fini(struct drm_i915_private *dev_priv)
 {
+	i915_gem_suspend_late(dev_priv);
+
 	/* Flush any outstanding unpin_work. */
 	i915_gem_drain_workqueue(dev_priv);
 
@@ -1611,7 +1613,6 @@ static int i915_drm_suspend(struct drm_device *dev)
 	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
 	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
 
-	intel_uncore_suspend(dev_priv);
 	intel_opregion_unregister(dev_priv);
 
 	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
@@ -1633,7 +1634,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
 
 	disable_rpm_wakeref_asserts(dev_priv);
 
+	i915_gem_suspend_late(dev_priv);
+
 	intel_display_set_init_power(dev_priv, false);
+	intel_uncore_suspend(dev_priv);
 
 	/*
 	 * In case of firmware assisted context save/restore don't manually
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 32a65de7222d..38157df6ff5c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3169,6 +3169,7 @@ void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
 int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
+void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
 int i915_gem_fault(struct vm_fault *vmf);
 int i915_gem_object_wait(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 9b8fa1866cc9..f5c4ef052001 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5055,6 +5055,17 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	if (WARN_ON(!intel_engines_are_idle(dev_priv)))
 		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
 
+	intel_runtime_pm_put(dev_priv);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	intel_runtime_pm_put(dev_priv);
+	return ret;
+}
+
+void i915_gem_suspend_late(struct drm_i915_private *i915)
+{
 	/*
 	 * Neither the BIOS, ourselves or any other kernel
 	 * expects the system to be in execlists mode on startup,
@@ -5074,16 +5085,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machines is a good idea, we don't - just in case it leaves the
 	 * machine in an unusable condition.
 	 */
-	intel_uc_sanitize(dev_priv);
-	i915_gem_sanitize(dev_priv);
 
-	intel_runtime_pm_put(dev_priv);
-	return 0;
-
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
-	intel_runtime_pm_put(dev_priv);
-	return ret;
+	intel_uc_sanitize(i915);
+	i915_gem_sanitize(i915);
 }
 
 void i915_gem_resume(struct drm_i915_private *i915)
-- 
2.17.0

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

✗ Fi.CI.SPARSE: warning for series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime
URL   : https://patchwork.freedesktop.org/series/44004/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Switch to kernel context before idling at runtime
Okay!

Commit: drm/i915: "Race-to-idle" after switching to the kernel context
Okay!

Commit: drm/i915: After reset on sanitization, reset the engine backends
Okay!

Commit: drm/i915: Only sanitize GEM from late suspend
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3664:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3665:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime
URL   : https://patchwork.freedesktop.org/series/44004/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4262 -> Patchwork_9154 =

== Summary - SUCCESS ==

  No regressions found.

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

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_flip@basic-flip-vs-wf_vblank:
      fi-glk-j4005:       FAIL -> PASS

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       DMESG-WARN (fdo#106607) -> PASS

    igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
      fi-glk-j4005:       FAIL (fdo#103481) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-cnl-y3:          DMESG-WARN (fdo#104951) -> PASS
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103481 https://bugs.freedesktop.org/show_bug.cgi?id=103481
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951
  fdo#106607 https://bugs.freedesktop.org/show_bug.cgi?id=106607


== Participating hosts (45 -> 39) ==

  Missing    (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-cfl-u2 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4262 -> Patchwork_9154

  CI_DRM_4262: 867e60e3df29db24c84b4b1613f0a1366769aa20 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4501: 6796a604bab6df9c84af149e799902360afdd157 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9154: 36a5fe4833fa6ba0dec81f2d8909689f5fb86d2b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

36a5fe4833fa drm/i915: Only sanitize GEM from late suspend
6cb6754cc4e5 drm/i915: After reset on sanitization, reset the engine backends
d7121e35592c drm/i915: "Race-to-idle" after switching to the kernel context
ba2022d8ffd4 drm/i915: Switch to kernel context before idling at runtime

== Logs ==

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

✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime
URL   : https://patchwork.freedesktop.org/series/44004/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4262_full -> Patchwork_9154_full =

== Summary - WARNING ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-blt:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS +1

    igt@kms_atomic@plane_primary_legacy:
      shard-snb:          SKIP -> PASS +1

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_gtt:
      shard-glk:          PASS -> INCOMPLETE (fdo#103359, k.org#198133)

    igt@gem_eio@suspend:
      shard-snb:          PASS -> INCOMPLETE (fdo#105411) +1

    igt@gem_exec_big:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540)

    igt@kms_cursor_legacy@2x-nonblocking-modeset-vs-cursor-atomic:
      shard-glk:          PASS -> FAIL (fdo#105454, fdo#106509)

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          PASS -> FAIL (fdo#105363)

    igt@kms_flip@flip-vs-expired-vblank:
      shard-glk:          PASS -> FAIL (fdo#105363, fdo#102887)

    igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
      shard-snb:          SKIP -> FAIL (fdo#104724, fdo#103167)

    
    ==== Possible fixes ====

    igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
      shard-glk:          FAIL (fdo#105703) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-hsw:          FAIL (fdo#100368) -> PASS +1

    igt@kms_flip@modeset-vs-vblank-race:
      shard-glk:          FAIL (fdo#103060) -> PASS

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-hsw:          FAIL (fdo#103060) -> PASS

    igt@kms_flip_tiling@flip-to-x-tiled:
      shard-glk:          FAIL (fdo#103822, fdo#104724) -> PASS +1

    igt@kms_flip_tiling@flip-x-tiled:
      shard-glk:          FAIL (fdo#104724) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105454 https://bugs.freedesktop.org/show_bug.cgi?id=105454
  fdo#105703 https://bugs.freedesktop.org/show_bug.cgi?id=105703
  fdo#106509 https://bugs.freedesktop.org/show_bug.cgi?id=106509
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4262 -> Patchwork_9154

  CI_DRM_4262: 867e60e3df29db24c84b4b1613f0a1366769aa20 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4501: 6796a604bab6df9c84af149e799902360afdd157 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9154: 36a5fe4833fa6ba0dec81f2d8909689f5fb86d2b @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

Re: [PATCH 4/4] drm/i915: Only sanitize GEM from late suspend

[Tvrtko Ursulin]

On 31/05/2018 09:22, Chris Wilson wrote:
> During testing we encounter a conflict between SUSPEND_TEST_DEVICES and
> disabling reset (gem_eio/suspend). This results in the device continuing
> on without being reset, but since it has gone through HW sanitization to
> account for the suspend/resume cycle, we have to assume the device has
> been reset to its defaults. A simple way around this is to skip the
> sanitize phase for SUSPEND_TEST_DEVICES by moving it to suspend-late.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/i915_drv.c |  6 +++++-
>   drivers/gpu/drm/i915/i915_drv.h |  1 +
>   drivers/gpu/drm/i915/i915_gem.c | 22 +++++++++++++---------
>   3 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8f002ae22e62..0d9b8cc0436d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -636,6 +636,8 @@ static const struct vga_switcheroo_client_ops i915_switcheroo_ops = {
>   
>   static void i915_gem_fini(struct drm_i915_private *dev_priv)
>   {
> +	i915_gem_suspend_late(dev_priv);
> +
>   	/* Flush any outstanding unpin_work. */
>   	i915_gem_drain_workqueue(dev_priv);
>   
> @@ -1611,7 +1613,6 @@ static int i915_drm_suspend(struct drm_device *dev)
>   	opregion_target_state = suspend_to_idle(dev_priv) ? PCI_D1 : PCI_D3cold;
>   	intel_opregion_notify_adapter(dev_priv, opregion_target_state);
>   
> -	intel_uncore_suspend(dev_priv);
>   	intel_opregion_unregister(dev_priv);
>   
>   	intel_fbdev_set_suspend(dev, FBINFO_STATE_SUSPENDED, true);
> @@ -1633,7 +1634,10 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation)
>   
>   	disable_rpm_wakeref_asserts(dev_priv);
>   
> +	i915_gem_suspend_late(dev_priv);
> +
>   	intel_display_set_init_power(dev_priv, false);
> +	intel_uncore_suspend(dev_priv);
>   
>   	/*
>   	 * In case of firmware assisted context save/restore don't manually
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 32a65de7222d..38157df6ff5c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3169,6 +3169,7 @@ void i915_gem_cleanup_engines(struct drm_i915_private *dev_priv);
>   int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
>   			   unsigned int flags);
>   int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
> +void i915_gem_suspend_late(struct drm_i915_private *dev_priv);
>   void i915_gem_resume(struct drm_i915_private *dev_priv);
>   int i915_gem_fault(struct vm_fault *vmf);
>   int i915_gem_object_wait(struct drm_i915_gem_object *obj,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 9b8fa1866cc9..f5c4ef052001 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5055,6 +5055,17 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>   	if (WARN_ON(!intel_engines_are_idle(dev_priv)))
>   		i915_gem_set_wedged(dev_priv); /* no hope, discard everything */
>   
> +	intel_runtime_pm_put(dev_priv);
> +	return 0;
> +
> +err_unlock:
> +	mutex_unlock(&dev->struct_mutex);
> +	intel_runtime_pm_put(dev_priv);
> +	return ret;
> +}
> +
> +void i915_gem_suspend_late(struct drm_i915_private *i915)
> +{
>   	/*
>   	 * Neither the BIOS, ourselves or any other kernel
>   	 * expects the system to be in execlists mode on startup,
> @@ -5074,16 +5085,9 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
>   	 * machines is a good idea, we don't - just in case it leaves the
>   	 * machine in an unusable condition.
>   	 */
> -	intel_uc_sanitize(dev_priv);
> -	i915_gem_sanitize(dev_priv);
>   
> -	intel_runtime_pm_put(dev_priv);
> -	return 0;
> -
> -err_unlock:
> -	mutex_unlock(&dev->struct_mutex);
> -	intel_runtime_pm_put(dev_priv);
> -	return ret;
> +	intel_uc_sanitize(i915);
> +	i915_gem_sanitize(i915);
>   }
>   
>   void i915_gem_resume(struct drm_i915_private *i915)
> 

It's hackery for PM core not suspending devices fully in device test 
mode, but I have no better ideas, nor can I figure out why PM core does 
it like that:

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

Regards,

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

Re: ✓ Fi.CI.IGT: success for series starting with [1/4] drm/i915: Switch to kernel context before idling at runtime

[Chris Wilson]

Quoting Patchwork (2018-05-31 11:04:13)
>     igt@gem_eio@suspend:
>       shard-snb:          PASS -> INCOMPLETE (fdo#105411) +1

I was concerned by this since gem_eio targets the code being changed. In
particular, I was concerned that this was bringing back the snb gem_eio
nightmare (where we would take out the machine). However, running
locally and looking at the previous CI results doesn't show that this is
a persistent problem, so I think this failure was a fluke.

That isn't to say that there isn't something weird happening on snb, but
I don't think it's any weirder than normal.

Thank you all for the reviews, let's just hope we don't have to stare
at suspend again for a few weeks^W days.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx