[PATCH 1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[PATCH 1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Chris Wilson]

Currently, we cancel the extra wakeref we have for !runtime-pm devices
inside power_wells_fini_hw. However, this is not strictly paired with
the acquisition of that wakeref in runtime_pm_enable (as the fini_hw may
be called on errors paths before we even call runtime_pm_enable). Make
the symmetry more explicit and include a check that we do release all of
our rpm wakerefs.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.c         |  8 ++++++--
 drivers/gpu/drm/i915/intel_drv.h        |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 24 +++++++++++++++---------
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 9dce55182c3a..62ef105a241d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1281,6 +1281,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	 */
 	if (INTEL_INFO(dev_priv)->num_pipes)
 		drm_kms_helper_poll_init(dev);
+
+	intel_runtime_pm_enable(dev_priv);
 }
 
 /**
@@ -1289,6 +1291,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 {
+	intel_runtime_pm_disable(dev_priv);
+
 	intel_fbdev_unregister(dev_priv);
 	intel_audio_deinit(dev_priv);
 
@@ -1408,8 +1412,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915_driver_register(dev_priv);
 
-	intel_runtime_pm_enable(dev_priv);
-
 	intel_init_ipc(dev_priv);
 
 	intel_runtime_pm_put(dev_priv);
@@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev)
 	i915_driver_cleanup_mmio(dev_priv);
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+
+	WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count));
 }
 
 static void i915_driver_release(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0601abb8c71f..dc6c0cec9b36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1956,6 +1956,7 @@ void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
 void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
 void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
 const char *
 intel_display_power_domain_str(enum intel_display_power_domain domain);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e209edbc561d..b78c3b48aa62 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3793,8 +3793,6 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
  */
 void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
 {
-	struct device *kdev = &dev_priv->drm.pdev->dev;
-
 	/*
 	 * The i915.ko module is still not prepared to be loaded when
 	 * the power well is not enabled, so just enable it in case
@@ -3809,13 +3807,6 @@ void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
 	/* Remove the refcount we took to keep power well support disabled. */
 	if (!i915_modparams.disable_power_well)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-
-	/*
-	 * Remove the refcount we took in intel_runtime_pm_enable() in case
-	 * the platform doesn't support runtime PM.
-	 */
-	if (!HAS_RUNTIME_PM(dev_priv))
-		pm_runtime_put(kdev);
 }
 
 /**
@@ -4074,3 +4065,18 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
 	 */
 	pm_runtime_put_autosuspend(kdev);
 }
+
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	struct device *kdev = &pdev->dev;
+
+	pm_runtime_dont_use_autosuspend(kdev);
+
+	/*
+	 * Remove the refcount we took in intel_runtime_pm_enable() in case
+	 * the platform doesn't support runtime PM.
+	 */
+	if (!HAS_RUNTIME_PM(dev_priv))
+		pm_runtime_put(kdev);
+}
-- 
2.18.0

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

[PATCH 2/2] drm/i915: Track all held rpm wakerefs

[Chris Wilson]

Everytime we take a wakeref, record the stack trace of where it was
taken; clearing the set if we ever drop back to no owners. For debugging
a rpm leak, we can look at all the current wakerefs and check if they
have a matching rpm_put.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/Kconfig.debug      |  13 +++
 drivers/gpu/drm/i915/i915_drv.c         |   3 +-
 drivers/gpu/drm/i915/i915_drv.h         |   7 ++
 drivers/gpu/drm/i915/intel_drv.h        |   1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 143 +++++++++++++++++++++++-
 5 files changed, 165 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index 459f8f88a34c..ed572a454e01 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -39,6 +39,19 @@ config DRM_I915_DEBUG
 
           If in doubt, say "N".
 
+config DRM_I915_DEBUG_RPM
+        bool "Insert extra checks into the runtime pm internals"
+        depends on DRM_I915
+        default n
+        select STACKDEPOT
+        help
+          Enable extra sanity checks (including BUGs) along the runtime pm
+          paths that may slow the system down and if hit hang the machine.
+
+          Recommended for driver developers only.
+
+          If in doubt, say "N".
+
 config DRM_I915_DEBUG_GEM
         bool "Insert extra checks into the GEM internals"
         default n
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 62ef105a241d..63992fe45dbf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1477,7 +1477,7 @@ void i915_driver_unload(struct drm_device *dev)
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
 
-	WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count));
+	intel_runtime_pm_cleanup(dev_priv);
 }
 
 static void i915_driver_release(struct drm_device *dev)
@@ -2603,6 +2603,7 @@ static int intel_runtime_suspend(struct device *kdev)
 
 	DRM_DEBUG_KMS("Suspending device\n");
 
+	intel_runtime_pm_cleanup(dev_priv);
 	disable_rpm_wakeref_asserts(dev_priv);
 
 	/*
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b10a30b7d96..c8d5b896b155 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -45,6 +45,7 @@
 #include <linux/pm_qos.h>
 #include <linux/reservation.h>
 #include <linux/shmem_fs.h>
+#include <linux/stackdepot.h>
 
 #include <drm/drmP.h>
 #include <drm/intel-gtt.h>
@@ -1278,6 +1279,12 @@ struct i915_runtime_pm {
 	atomic_t wakeref_count;
 	bool suspended;
 	bool irqs_enabled;
+
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RPM)
+	spinlock_t debug_lock;
+	depot_stack_handle_t *debug_owners;
+	unsigned long debug_count;
+#endif
 };
 
 enum intel_pipe_crc_source {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dc6c0cec9b36..968c9074f1a8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1957,6 +1957,7 @@ void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
 void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_cleanup(struct drm_i915_private *dev_priv);
 const char *
 intel_display_power_domain_str(enum intel_display_power_domain domain);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b78c3b48aa62..7f555a0ad2ee 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,6 +49,130 @@
  * present for a given platform.
  */
 
+#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_RPM)
+
+#include <linux/sort.h>
+
+#define STACKDEPTH 12
+
+static void track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+	unsigned long entries[STACKDEPTH];
+	struct stack_trace trace = {
+		.entries = entries,
+		.max_entries = ARRAY_SIZE(entries),
+		.skip = 2
+	};
+	unsigned long flags;
+	depot_stack_handle_t stack, *stacks;
+
+	if (!HAS_RUNTIME_PM(i915))
+		return;
+
+	save_stack_trace(&trace);
+	if (trace.nr_entries &&
+	    trace.entries[trace.nr_entries - 1] == ULONG_MAX)
+		trace.nr_entries--;
+
+	stack = depot_save_stack(&trace, GFP_NOWAIT | __GFP_NOWARN);
+	if (!stack)
+		return;
+
+	spin_lock_irqsave(&i915->runtime_pm.debug_lock, flags);
+	stacks = krealloc(i915->runtime_pm.debug_owners,
+			  (i915->runtime_pm.debug_count + 1) * sizeof(*stacks),
+			  GFP_NOWAIT | __GFP_NOWARN);
+	if (stacks) {
+		stacks[i915->runtime_pm.debug_count++] = stack;
+		i915->runtime_pm.debug_owners = stacks;
+	}
+	spin_unlock_irqrestore(&i915->runtime_pm.debug_lock, flags);
+}
+
+static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+	depot_stack_handle_t *stacks;
+	unsigned long flags;
+
+	spin_lock_irqsave(&i915->runtime_pm.debug_lock, flags);
+	stacks = fetch_and_zero(&i915->runtime_pm.debug_owners);
+	i915->runtime_pm.debug_count = 0;
+	spin_unlock_irqrestore(&i915->runtime_pm.debug_lock, flags);
+
+	kfree(stacks);
+}
+
+static int cmphandle(const void *_a, const void *_b)
+{
+        const depot_stack_handle_t * const a = _a, * const b = _b;
+
+        if (*a < *b)
+                return -1;
+        else if (*a > *b)
+                return 1;
+        else
+                return 0;
+}
+
+static void show_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+	unsigned long entries[STACKDEPTH];
+	depot_stack_handle_t *stacks;
+	unsigned long flags, count, i;
+	char *buf;
+
+	spin_lock_irqsave(&i915->runtime_pm.debug_lock, flags);
+	stacks = fetch_and_zero(&i915->runtime_pm.debug_owners);
+	count = fetch_and_zero(&i915->runtime_pm.debug_count);
+	spin_unlock_irqrestore(&i915->runtime_pm.debug_lock, flags);
+	if (!count)
+		return;
+
+	DRM_DEBUG_DRIVER("leaked %lu wakerefs\n", count);
+
+	buf = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!buf)
+		goto out_stacks;
+
+	sort(stacks, count, sizeof(*stacks), cmphandle, NULL);
+
+	for (i = 0; i < count; i++) {
+		struct stack_trace trace = {
+			.entries = entries,
+			.max_entries = ARRAY_SIZE(entries),
+		};
+		depot_stack_handle_t stack = stacks[i];
+		unsigned long rep;
+
+		rep = 1;
+		while (i + 1 < count && stacks[i + 1] == stack)
+			rep++, i++;
+		depot_fetch_stack(stack, &trace);
+		snprint_stack_trace(buf, PAGE_SIZE, &trace, 0);
+		DRM_DEBUG_DRIVER("wakeref x%lu at\n%s", rep, buf);
+	}
+
+	kfree(buf);
+out_stacks:
+	kfree(stacks);
+}
+
+#else
+
+static void track_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+}
+
+static void untrack_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+}
+
+static void show_intel_runtime_pm_wakeref(struct drm_i915_private *i915)
+{
+}
+
+#endif
+
 bool intel_display_power_well_is_enabled(struct drm_i915_private *dev_priv,
 					 enum i915_power_well_id power_well_id);
 
@@ -3939,6 +4063,8 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
 
 	atomic_inc(&dev_priv->runtime_pm.wakeref_count);
 	assert_rpm_wakelock_held(dev_priv);
+
+	track_intel_runtime_pm_wakeref(dev_priv);
 }
 
 /**
@@ -3973,6 +4099,8 @@ bool intel_runtime_pm_get_if_in_use(struct drm_i915_private *dev_priv)
 	atomic_inc(&dev_priv->runtime_pm.wakeref_count);
 	assert_rpm_wakelock_held(dev_priv);
 
+	track_intel_runtime_pm_wakeref(dev_priv);
+
 	return true;
 }
 
@@ -4002,6 +4130,8 @@ void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv)
 	pm_runtime_get_noresume(kdev);
 
 	atomic_inc(&dev_priv->runtime_pm.wakeref_count);
+
+	track_intel_runtime_pm_wakeref(dev_priv);
 }
 
 /**
@@ -4018,7 +4148,8 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
 	struct device *kdev = &pdev->dev;
 
 	assert_rpm_wakelock_held(dev_priv);
-	atomic_dec(&dev_priv->runtime_pm.wakeref_count);
+	if (!atomic_dec_and_test(&dev_priv->runtime_pm.wakeref_count))
+		untrack_intel_runtime_pm_wakeref(dev_priv);
 
 	pm_runtime_mark_last_busy(kdev);
 	pm_runtime_put_autosuspend(kdev);
@@ -4080,3 +4211,13 @@ void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
 	if (!HAS_RUNTIME_PM(dev_priv))
 		pm_runtime_put(kdev);
 }
+
+void intel_runtime_pm_cleanup(struct drm_i915_private *dev_priv)
+{
+	if (WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count))) {
+		show_intel_runtime_pm_wakeref(dev_priv);
+		atomic_set(&dev_priv->runtime_pm.wakeref_count, 0);
+	}
+
+	untrack_intel_runtime_pm_wakeref(dev_priv);
+}
-- 
2.18.0

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
URL   : https://patchwork.freedesktop.org/series/47934/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1787a18142d6 drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
5d4072eab971 drm/i915: Track all held rpm wakerefs
-:76: CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment
#76: FILE: drivers/gpu/drm/i915/i915_drv.h:1284:
+	spinlock_t debug_lock;

-:158: ERROR:CODE_INDENT: code indent should use tabs where possible
#158: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:107:
+        const depot_stack_handle_t * const a = _a, * const b = _b;$

-:158: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#158: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:107:
+        const depot_stack_handle_t * const a = _a, * const b = _b;$

-:160: ERROR:CODE_INDENT: code indent should use tabs where possible
#160: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:109:
+        if (*a < *b)$

-:160: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#160: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:109:
+        if (*a < *b)$

-:161: ERROR:CODE_INDENT: code indent should use tabs where possible
#161: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:110:
+                return -1;$

-:161: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#161: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:110:
+                return -1;$

-:162: ERROR:CODE_INDENT: code indent should use tabs where possible
#162: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:111:
+        else if (*a > *b)$

-:162: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#162: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:111:
+        else if (*a > *b)$

-:163: ERROR:CODE_INDENT: code indent should use tabs where possible
#163: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:112:
+                return 1;$

-:163: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#163: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:112:
+                return 1;$

-:164: ERROR:CODE_INDENT: code indent should use tabs where possible
#164: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:113:
+        else$

-:164: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#164: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:113:
+        else$

-:165: ERROR:CODE_INDENT: code indent should use tabs where possible
#165: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:114:
+                return 0;$

-:165: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#165: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:114:
+                return 0;$

total: 7 errors, 7 warnings, 1 checks, 236 lines checked

_______________________________________________
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/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
URL   : https://patchwork.freedesktop.org/series/47934/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
Okay!

Commit: drm/i915: Track all held rpm wakerefs
+
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3675:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3682:16: warning: expression using sizeof(void)
+Error in reading or end of file.

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

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
URL   : https://patchwork.freedesktop.org/series/47934/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4636 -> Patchwork_9902 =

== Summary - FAILURE ==

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_module_reload@basic-no-display:
      {fi-skl-iommu}:     PASS -> DMESG-WARN +3
      fi-elk-e7500:       PASS -> DMESG-WARN +16

    igt@drv_module_reload@basic-reload:
      fi-skl-guc:         PASS -> DMESG-WARN +3
      fi-kbl-x1275:       PASS -> DMESG-WARN +16

    igt@drv_module_reload@basic-reload-inject:
      fi-skl-6260u:       PASS -> DMESG-WARN +3
      fi-kbl-7560u:       NOTRUN -> DMESG-WARN +16
      fi-skl-6770hq:      PASS -> DMESG-WARN +3

    igt@drv_selftest@live_contexts:
      fi-cfl-s3:          PASS -> DMESG-WARN +16

    igt@drv_selftest@live_dmabuf:
      fi-byt-n2820:       PASS -> DMESG-WARN +16
      fi-hsw-4770r:       PASS -> DMESG-WARN +16

    igt@drv_selftest@live_evict:
      fi-cnl-psr:         PASS -> DMESG-WARN +16
      {fi-bsw-kefka}:     PASS -> DMESG-WARN +16

    igt@drv_selftest@live_gtt:
      {fi-byt-clapper}:   PASS -> DMESG-WARN +16
      {fi-kbl-8809g}:     PASS -> DMESG-WARN +16
      fi-kbl-guc:         PASS -> DMESG-WARN +15
      fi-gdg-551:         PASS -> DMESG-WARN +16
      fi-kbl-7500u:       PASS -> DMESG-WARN +16

    igt@drv_selftest@live_guc:
      fi-hsw-peppy:       PASS -> DMESG-WARN +16
      {fi-bdw-samus}:     PASS -> DMESG-WARN +16

    igt@drv_selftest@live_hangcheck:
      fi-snb-2520m:       PASS -> DMESG-WARN +16
      fi-skl-6700hq:      PASS -> DMESG-WARN +3
      fi-skl-6700k2:      PASS -> DMESG-WARN +3

    igt@drv_selftest@live_hugepages:
      fi-glk-dsi:         PASS -> DMESG-WARN +16
      {fi-cfl-8109u}:     PASS -> DMESG-WARN +16

    igt@drv_selftest@live_objects:
      fi-bwr-2160:        PASS -> DMESG-WARN +16
      fi-bdw-5557u:       PASS -> DMESG-WARN +16
      fi-snb-2600:        PASS -> DMESG-WARN +16
      fi-hsw-4770:        PASS -> DMESG-WARN +16
      fi-bxt-dsi:         PASS -> DMESG-WARN +16

    igt@drv_selftest@live_requests:
      fi-whl-u:           PASS -> DMESG-WARN +15
      fi-skl-gvtdvm:      PASS -> DMESG-WARN +16
      fi-ivb-3520m:       PASS -> DMESG-WARN +16
      fi-bxt-j4205:       PASS -> DMESG-WARN +16
      fi-cfl-guc:         PASS -> DMESG-WARN +15

    igt@drv_selftest@live_sanitycheck:
      fi-bdw-gvtdvm:      PASS -> DMESG-WARN +16
      fi-ilk-650:         PASS -> DMESG-WARN +16
      fi-bsw-n3050:       PASS -> DMESG-WARN +16
      fi-kbl-7567u:       PASS -> DMESG-WARN +16
      fi-glk-j4005:       PASS -> DMESG-WARN +16
      fi-ivb-3770:        PASS -> DMESG-WARN +16

    igt@drv_selftest@live_uncore:
      fi-pnv-d510:        PASS -> DMESG-WARN +16

    igt@drv_selftest@live_workarounds:
      fi-cfl-8700k:       PASS -> DMESG-WARN +16
      fi-kbl-r:           PASS -> DMESG-WARN +16
      fi-byt-j1900:       PASS -> DMESG-WARN +16
      fi-blb-e6850:       PASS -> DMESG-WARN +16

    igt@gem_exec_suspend@basic-s3:
      {fi-kbl-soraka}:    NOTRUN -> INCOMPLETE

    igt@pm_rpm@basic-pci-d3-state:
      fi-skl-6600u:       PASS -> DMESG-WARN +4

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_objects:
      fi-skl-6770hq:      PASS -> DMESG-WARN (fdo#107175) +12
      fi-skl-6700k2:      PASS -> DMESG-WARN (fdo#107175) +12
      fi-skl-6260u:       PASS -> DMESG-WARN (fdo#107175) +12

    igt@drv_selftest@live_requests:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#107175) +12
      fi-skl-guc:         PASS -> DMESG-WARN (fdo#107175) +11

    igt@drv_selftest@live_sanitycheck:
      {fi-skl-iommu}:     PASS -> DMESG-WARN (fdo#107175) +12

    igt@drv_selftest@live_uncore:
      fi-skl-6600u:       PASS -> DMESG-WARN (fdo#107175) +12

    igt@drv_selftest@live_workarounds:
      fi-whl-u:           PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_frontbuffer_tracking@basic:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#103167)

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-WARN (fdo#107372) -> DMESG-FAIL (fdo#107372)

    
  {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#107175 https://bugs.freedesktop.org/show_bug.cgi?id=107175
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (50 -> 48) ==

  Additional (2): fi-kbl-soraka fi-kbl-7560u 
  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4636 -> Patchwork_9902

  CI_DRM_4636: 084bb2fb549650b6da80976c9bc594779ce342b4 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4590: e6ddaca7a8ea9d3d27f0ecaa36b357cc02e2df3b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9902: 5d4072eab971a88dcd1291918bfb44279e17d8df @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5d4072eab971 drm/i915: Track all held rpm wakerefs
1787a18142d6 drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

== Logs ==

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

[PATCH 1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Imre Deak]

From: Chris Wilson <chris@chris-wilson.co.uk>

Currently, we cancel the extra wakeref we have for !runtime-pm devices
inside power_wells_fini_hw. However, this is not strictly paired with
the acquisition of that wakeref in runtime_pm_enable (as the fini_hw may
be called on errors paths before we even call runtime_pm_enable). Make
the symmetry more explicit and include a check that we do release all of
our rpm wakerefs.

v2: Fixup transfer of ownership back to core whilst keeping our wakeref
count balanced.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         | 27 ++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h        |  1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 43 ++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 41111f2a9c39..021304e252eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1281,6 +1281,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
 	 */
 	if (INTEL_INFO(dev_priv)->num_pipes)
 		drm_kms_helper_poll_init(dev);
+
+	intel_runtime_pm_enable(dev_priv);
 }
 
 /**
@@ -1289,6 +1291,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv)
  */
 static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 {
+	intel_runtime_pm_disable(dev_priv);
+
 	intel_fbdev_unregister(dev_priv);
 	intel_audio_deinit(dev_priv);
 
@@ -1366,16 +1370,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 		goto out_fini;
 
 	pci_set_drvdata(pdev, &dev_priv->drm);
-	/*
-	 * Disable the system suspend direct complete optimization, which can
-	 * leave the device suspended skipping the driver's suspend handlers
-	 * if the device was already runtime suspended. This is needed due to
-	 * the difference in our runtime and system suspend sequence and
-	 * becaue the HDA driver may require us to enable the audio power
-	 * domain during system suspend.
-	 */
-	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
-
 	ret = i915_driver_init_early(dev_priv, ent);
 	if (ret < 0)
 		goto out_pci_disable;
@@ -1408,8 +1402,6 @@ int i915_driver_load(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	i915_driver_register(dev_priv);
 
-	intel_runtime_pm_enable(dev_priv);
-
 	intel_init_ipc(dev_priv);
 
 	intel_runtime_pm_put(dev_priv);
@@ -1441,13 +1433,13 @@ void i915_driver_unload(struct drm_device *dev)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
-	i915_driver_unregister(dev_priv);
-
-	if (i915_gem_suspend(dev_priv))
-		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
 
+	i915_driver_unregister(dev_priv);
+
+	if (i915_gem_suspend(dev_priv))
+		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
+
 	drm_atomic_helper_shutdown(dev);
 
 	intel_gvt_cleanup(dev_priv);
@@ -1474,6 +1466,7 @@ void i915_driver_unload(struct drm_device *dev)
 	i915_driver_cleanup_mmio(dev_priv);
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
+	WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count));
 }
 
 static void i915_driver_release(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7b984aefce98..3f012716f2dc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1959,6 +1959,7 @@ void intel_power_domains_verify_state(struct drm_i915_private *dev_priv);
 void bxt_display_core_init(struct drm_i915_private *dev_priv, bool resume);
 void bxt_display_core_uninit(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv);
 const char *
 intel_display_power_domain_str(enum intel_display_power_domain domain);
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index e209edbc561d..c0983f0e46ac 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3793,29 +3793,19 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
  */
 void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
 {
-	struct device *kdev = &dev_priv->drm.pdev->dev;
-
 	/*
 	 * The i915.ko module is still not prepared to be loaded when
 	 * the power well is not enabled, so just enable it in case
 	 * we're going to unload/reload.
-	 * The following also reacquires the RPM reference the core passed
-	 * to the driver during loading, which is dropped in
-	 * intel_runtime_pm_enable(). We have to hand back the control of the
-	 * device to the core with this reference held.
 	 */
-	intel_display_set_init_power(dev_priv, true);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
+
+	/* Keep the power well enabled, but cancel its rpm wakeref. */
+	intel_runtime_pm_put(dev_priv);
 
 	/* Remove the refcount we took to keep power well support disabled. */
 	if (!i915_modparams.disable_power_well)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_INIT);
-
-	/*
-	 * Remove the refcount we took in intel_runtime_pm_enable() in case
-	 * the platform doesn't support runtime PM.
-	 */
-	if (!HAS_RUNTIME_PM(dev_priv))
-		pm_runtime_put(kdev);
 }
 
 /**
@@ -4048,6 +4038,16 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 	struct device *kdev = &pdev->dev;
 
+	/*
+	 * Disable the system suspend direct complete optimization, which can
+	 * leave the device suspended skipping the driver's suspend handlers
+	 * if the device was already runtime suspended. This is needed due to
+	 * the difference in our runtime and system suspend sequence and
+	 * becaue the HDA driver may require us to enable the audio power
+	 * domain during system suspend.
+	 */
+	dev_pm_set_driver_flags(kdev, DPM_FLAG_NEVER_SKIP);
+
 	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
 	pm_runtime_mark_last_busy(kdev);
 
@@ -4074,3 +4074,18 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
 	 */
 	pm_runtime_put_autosuspend(kdev);
 }
+
+void intel_runtime_pm_disable(struct drm_i915_private *dev_priv)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	struct device *kdev = &pdev->dev;
+
+	/* Transfer rpm ownership back to core */
+	WARN(pm_runtime_get_sync(&dev_priv->drm.pdev->dev) < 0,
+	     "Failed to pass rpm ownership back to core\n");
+
+	pm_runtime_dont_use_autosuspend(kdev);
+
+	if (!HAS_RUNTIME_PM(dev_priv))
+		pm_runtime_put(kdev);
+}
-- 
2.13.2

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

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
URL   : https://patchwork.freedesktop.org/series/48315/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
62108989ff60 drm/i915: Introduce intel_runtime_pm_disable to pair intel_runtime_pm_enable
2e47b9b2aba3 drm/i915: Refactor intel_display_set_init_power() logic
-:436: CHECK:COMPARISON_TO_NULL: Comparison to NULL could be written "dev_priv->csr.dmc_payload"
#436: FILE: drivers/gpu/drm/i915/intel_runtime_pm.c:3848:
+	    dev_priv->csr.dmc_payload != NULL)

total: 0 errors, 0 warnings, 1 checks, 402 lines checked

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