RFC drm/i915: Mark runtime_pm as a special class of lock

Commit Message

Chris Wilson July 12, 2018, 8:20 a.m. UTC

Signed-off-by: Chris Wilson <[e-mail]>
---
 drivers/gpu/drm/i915/i915_drv.c         | 5 +++++
 drivers/gpu/drm/i915/i915_drv.h         | 1 +
 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 +++
 3 files changed, 9 insertions(+)

Comments

Chris Wilson July 12, 2018, 8:33 a.m. UTC | #1

Quoting Chris Wilson (2018-07-12 09:20:37)

It needs to be equally on runtime_pm_get.
-Chris

Daniel Vetter July 12, 2018, 8:37 a.m. UTC | #2

On Thu, Jul 12, 2018 at 09:33:32AM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2018-07-12 09:20:37)
> 
> It needs to be equally on runtime_pm_get.

Just wanted to complain about "what if we know there's someone holding an
rpm ref already?", but then remembered that this is what we have the
_if_in_use and _noresume variants for.

So a might_lock in runtime_pm_get sounds perfect.
-Daniel

Chris Wilson July 12, 2018, 10:14 a.m. UTC | #3

Quoting Patchwork (2018-07-12 11:12:25)
> == Series Details ==
> 
> Series: RFC drm/i915: Mark runtime_pm as a special class of lock (rev2)
> URL   : https://patchwork.freedesktop.org/series/46366/
> State : success
> 
> == Summary ==
> 
> = CI Bug Log - changes from CI_DRM_4475 -> Patchwork_9626 =
> 
> == Summary - SUCCESS ==
> 
>   No regressions found.

Hmm. I feel like I missed something! Perhaps just wait until we get the
shard results...
-Chris

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3eba3d1ab5b8..2e6d3259f6d0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -2603,6 +2603,7 @@  static int intel_runtime_suspend(struct device *kdev)
 	DRM_DEBUG_KMS("Suspending device\n");
 
 	disable_rpm_wakeref_asserts(dev_priv);
+	lock_map_acquire(&dev_priv->runtime_pm.lock);
 
 	/*
 	 * We are safe here against re-faults, since the fault handler takes
@@ -2637,11 +2638,13 @@  static int intel_runtime_suspend(struct device *kdev)
 		i915_gem_init_swizzling(dev_priv);
 		i915_gem_restore_fences(dev_priv);
 
+		lock_map_release(&dev_priv->runtime_pm.lock);
 		enable_rpm_wakeref_asserts(dev_priv);
 
 		return ret;
 	}
 
+	lock_map_release(&dev_priv->runtime_pm.lock);
 	enable_rpm_wakeref_asserts(dev_priv);
 	WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count));
 
@@ -2696,6 +2699,7 @@  static int intel_runtime_resume(struct device *kdev)
 
 	WARN_ON_ONCE(atomic_read(&dev_priv->runtime_pm.wakeref_count));
 	disable_rpm_wakeref_asserts(dev_priv);
+	lock_map_acquire(&dev_priv->runtime_pm.lock);
 
 	intel_opregion_notify_adapter(dev_priv, PCI_D0);
 	dev_priv->runtime_pm.suspended = false;
@@ -2737,6 +2741,7 @@  static int intel_runtime_resume(struct device *kdev)
 
 	intel_enable_ipc(dev_priv);
 
+	lock_map_release(&dev_priv->runtime_pm.lock);
 	enable_rpm_wakeref_asserts(dev_priv);
 
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 01dd29837233..be50a0e6d8c9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1251,6 +1251,7 @@  struct skl_wm_params {
  * For more, read the Documentation/power/runtime_pm.txt.
  */
 struct i915_runtime_pm {
+	struct lockdep_map lock;
 	atomic_t wakeref_count;
 	bool suspended;
 	bool irqs_enabled;
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6b5aa3b074ec..4ed35cdb378a 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -3796,9 +3796,12 @@  void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
  */
 void intel_runtime_pm_enable(struct drm_i915_private *dev_priv)
 {
+	static struct lock_class_key lock_key;
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 	struct device *kdev = &pdev->dev;
 
+	lockdep_init_map(&dev_priv->runtime_pm.lock,
+			 "i915->runtime_pm", &lock_key, 0);
 	pm_runtime_set_autosuspend_delay(kdev, 10000); /* 10s */
 	pm_runtime_mark_last_busy(kdev);