[Intel-gfx] [PATCH 1/2] drm/i915: Defer final intel_wakeref_put to process context

Chris Wilson chris at chris-wilson.co.uk
Mon Aug 5 09:43:32 UTC 2019


As we need to acquire a mutex to serialise the final
intel_wakeref_put, we need to ensure that we are in process context at
that time. However, we want to allow operation on the intel_wakeref from
inside timer and other hardirq context, which means that need to defer
that final put to a workqueue.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111295
Fixes: 18398904ca9e ("drm/i915: Only recover active engines")
Fixes: 51fbd8de87dc ("drm/i915/pmu: Atomically acquire the gt_pm wakeref")
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c | 17 +++----
 drivers/gpu/drm/i915/gt/intel_engine_pm.h | 18 ++++---
 drivers/gpu/drm/i915/gt/intel_gt_pm.c     | 24 ++++------
 drivers/gpu/drm/i915/gt/intel_gt_pm.h     | 11 ++++-
 drivers/gpu/drm/i915/intel_wakeref.c      | 57 +++++++++++++++--------
 drivers/gpu/drm/i915/intel_wakeref.h      | 44 +++++++++--------
 6 files changed, 96 insertions(+), 75 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 0336204988d6..e56283a05b07 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -37,11 +37,6 @@ static int __engine_unpark(struct intel_wakeref *wf)
 	return 0;
 }
 
-void intel_engine_pm_get(struct intel_engine_cs *engine)
-{
-	intel_wakeref_get(&engine->i915->runtime_pm, &engine->wakeref, __engine_unpark);
-}
-
 void intel_engine_park(struct intel_engine_cs *engine)
 {
 	/*
@@ -136,12 +131,14 @@ static int __engine_park(struct intel_wakeref *wf)
 	return 0;
 }
 
-void intel_engine_pm_put(struct intel_engine_cs *engine)
-{
-	intel_wakeref_put(&engine->i915->runtime_pm, &engine->wakeref, __engine_park);
-}
+static const struct intel_wakeref_ops wf_ops = {
+	.get = __engine_unpark,
+	.put = __engine_park,
+};
 
 void intel_engine_init__pm(struct intel_engine_cs *engine)
 {
-	intel_wakeref_init(&engine->wakeref);
+	struct intel_runtime_pm *rpm = &engine->i915->runtime_pm;
+
+	intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
 }
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
index 015ac72d7ad0..d3d48216f4a6 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
@@ -10,23 +10,27 @@
 #include "intel_engine_types.h"
 #include "intel_wakeref.h"
 
-struct drm_i915_private;
-
-void intel_engine_pm_get(struct intel_engine_cs *engine);
-void intel_engine_pm_put(struct intel_engine_cs *engine);
-
 static inline bool
 intel_engine_pm_is_awake(const struct intel_engine_cs *engine)
 {
 	return intel_wakeref_is_active(&engine->wakeref);
 }
 
-static inline bool
-intel_engine_pm_get_if_awake(struct intel_engine_cs *engine)
+static inline void intel_engine_pm_get(struct intel_engine_cs *engine)
+{
+	intel_wakeref_get(&engine->wakeref);
+}
+
+static inline bool intel_engine_pm_get_if_awake(struct intel_engine_cs *engine)
 {
 	return intel_wakeref_get_if_active(&engine->wakeref);
 }
 
+static inline void intel_engine_pm_put(struct intel_engine_cs *engine)
+{
+	intel_wakeref_put(&engine->wakeref);
+}
+
 void intel_engine_park(struct intel_engine_cs *engine);
 
 void intel_engine_init__pm(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
index 6c8970271a7f..e74a6ea841a1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c
@@ -17,7 +17,7 @@ static void pm_notify(struct drm_i915_private *i915, int state)
 	blocking_notifier_call_chain(&i915->gt.pm_notifications, state, i915);
 }
 
-static int intel_gt_unpark(struct intel_wakeref *wf)
+static int __gt_unpark(struct intel_wakeref *wf)
 {
 	struct intel_gt *gt = container_of(wf, typeof(*gt), wakeref);
 	struct drm_i915_private *i915 = gt->i915;
@@ -53,14 +53,7 @@ static int intel_gt_unpark(struct intel_wakeref *wf)
 	return 0;
 }
 
-void intel_gt_pm_get(struct intel_gt *gt)
-{
-	struct intel_runtime_pm *rpm = &gt->i915->runtime_pm;
-
-	intel_wakeref_get(rpm, &gt->wakeref, intel_gt_unpark);
-}
-
-static int intel_gt_park(struct intel_wakeref *wf)
+static int __gt_park(struct intel_wakeref *wf)
 {
 	struct drm_i915_private *i915 =
 		container_of(wf, typeof(*i915), gt.wakeref);
@@ -80,16 +73,15 @@ static int intel_gt_park(struct intel_wakeref *wf)
 	return 0;
 }
 
-void intel_gt_pm_put(struct intel_gt *gt)
-{
-	struct intel_runtime_pm *rpm = &gt->i915->runtime_pm;
-
-	intel_wakeref_put(rpm, &gt->wakeref, intel_gt_park);
-}
+static const struct intel_wakeref_ops wf_ops = {
+	.get = __gt_unpark,
+	.put = __gt_park,
+};
 
 void intel_gt_pm_init_early(struct intel_gt *gt)
 {
-	intel_wakeref_init(&gt->wakeref);
+	intel_wakeref_init(&gt->wakeref, &gt->i915->runtime_pm, &wf_ops);
+
 	BLOCKING_INIT_NOTIFIER_HEAD(&gt->pm_notifications);
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.h b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
index e8a18d4b27c9..5e0cd3044eb6 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
@@ -17,14 +17,21 @@ enum {
 	INTEL_GT_PARK,
 };
 
-void intel_gt_pm_get(struct intel_gt *gt);
-void intel_gt_pm_put(struct intel_gt *gt);
+static inline void intel_gt_pm_get(struct intel_gt *gt)
+{
+	intel_wakeref_get(&gt->wakeref);
+}
 
 static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt)
 {
 	return intel_wakeref_get_if_active(&gt->wakeref);
 }
 
+static inline void intel_gt_pm_put(struct intel_gt *gt)
+{
+	intel_wakeref_put(&gt->wakeref);
+}
+
 void intel_gt_pm_init_early(struct intel_gt *gt);
 
 void intel_gt_sanitize(struct intel_gt *gt, bool force);
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c b/drivers/gpu/drm/i915/intel_wakeref.c
index 06bd8b215cc2..1ec973f67042 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -7,22 +7,20 @@
 #include "intel_runtime_pm.h"
 #include "intel_wakeref.h"
 
-static void rpm_get(struct intel_runtime_pm *rpm, struct intel_wakeref *wf)
+static void rpm_get(struct intel_wakeref *wf)
 {
-	wf->wakeref = intel_runtime_pm_get(rpm);
+	wf->wakeref = intel_runtime_pm_get(wf->rpm);
 }
 
-static void rpm_put(struct intel_runtime_pm *rpm, struct intel_wakeref *wf)
+static void rpm_put(struct intel_wakeref *wf)
 {
 	intel_wakeref_t wakeref = fetch_and_zero(&wf->wakeref);
 
-	intel_runtime_pm_put(rpm, wakeref);
+	intel_runtime_pm_put(wf->rpm, wakeref);
 	INTEL_WAKEREF_BUG_ON(!wakeref);
 }
 
-int __intel_wakeref_get_first(struct intel_runtime_pm *rpm,
-			      struct intel_wakeref *wf,
-			      int (*fn)(struct intel_wakeref *wf))
+int __intel_wakeref_get_first(struct intel_wakeref *wf)
 {
 	/*
 	 * Treat get/put as different subclasses, as we may need to run
@@ -34,11 +32,11 @@ int __intel_wakeref_get_first(struct intel_runtime_pm *rpm,
 	if (!atomic_read(&wf->count)) {
 		int err;
 
-		rpm_get(rpm, wf);
+		rpm_get(wf);
 
-		err = fn(wf);
+		err = wf->ops->get(wf);
 		if (unlikely(err)) {
-			rpm_put(rpm, wf);
+			rpm_put(wf);
 			mutex_unlock(&wf->mutex);
 			return err;
 		}
@@ -52,27 +50,46 @@ int __intel_wakeref_get_first(struct intel_runtime_pm *rpm,
 	return 0;
 }
 
-int __intel_wakeref_put_last(struct intel_runtime_pm *rpm,
-			     struct intel_wakeref *wf,
-			     int (*fn)(struct intel_wakeref *wf))
+void __intel_wakeref_put_last(struct intel_wakeref *wf)
 {
-	int err;
+	if (in_interrupt()) {
+		INTEL_WAKEREF_BUG_ON(work_pending(&wf->work));
+		schedule_work(&wf->work);
+		return;
+	}
 
-	err = fn(wf);
-	if (likely(!err))
-		rpm_put(rpm, wf);
+	if (!atomic_dec_and_mutex_lock(&wf->count, &wf->mutex))
+		return;
+
+	if (likely(!wf->ops->put(wf)))
+		rpm_put(wf);
 	else
-		atomic_inc(&wf->count);
+		/* ops->put() must schedule its own release on deferral */
+		atomic_set_release(&wf->count, 1);
+
 	mutex_unlock(&wf->mutex);
+}
+
+static void __intel_wakeref_put_work(struct work_struct *wrk)
+{
+	struct intel_wakeref *wf = container_of(wrk, typeof(*wf), work);
 
-	return err;
+	intel_wakeref_put(wf);
 }
 
-void __intel_wakeref_init(struct intel_wakeref *wf, struct lock_class_key *key)
+void __intel_wakeref_init(struct intel_wakeref *wf,
+			  struct intel_runtime_pm *rpm,
+			  const struct intel_wakeref_ops *ops,
+			  struct lock_class_key *key)
 {
+	wf->rpm = rpm;
+	wf->ops = ops;
+
 	__mutex_init(&wf->mutex, "wakeref", key);
 	atomic_set(&wf->count, 0);
 	wf->wakeref = 0;
+
+	INIT_WORK(&wf->work, __intel_wakeref_put_work);
 }
 
 static void wakeref_auto_timeout(struct timer_list *t)
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h b/drivers/gpu/drm/i915/intel_wakeref.h
index 1d6f5986e4e5..7052ad76bb8e 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -12,6 +12,7 @@
 #include <linux/refcount.h>
 #include <linux/stackdepot.h>
 #include <linux/timer.h>
+#include <linux/workqueue.h>
 
 #if IS_ENABLED(CONFIG_DRM_I915_DEBUG)
 #define INTEL_WAKEREF_BUG_ON(expr) BUG_ON(expr)
@@ -20,29 +21,38 @@
 #endif
 
 struct intel_runtime_pm;
+struct intel_wakeref;
 
 typedef depot_stack_handle_t intel_wakeref_t;
 
+struct intel_wakeref_ops {
+	int (*get)(struct intel_wakeref *wf);
+	int (*put)(struct intel_wakeref *wf);
+};
+
 struct intel_wakeref {
+	struct intel_runtime_pm *rpm;
+
 	atomic_t count;
 	struct mutex mutex;
 	intel_wakeref_t wakeref;
+	struct work_struct work;
+
+	const struct intel_wakeref_ops *ops;
 };
 
 void __intel_wakeref_init(struct intel_wakeref *wf,
+			  struct intel_runtime_pm *rpm,
+			  const struct intel_wakeref_ops *ops,
 			  struct lock_class_key *key);
-#define intel_wakeref_init(wf) do {					\
+#define intel_wakeref_init(wf, rpm, ops) do {				\
 	static struct lock_class_key __key;				\
 									\
-	__intel_wakeref_init((wf), &__key);				\
+	__intel_wakeref_init((wf), (rpm), (ops), &__key);		\
 } while (0)
 
-int __intel_wakeref_get_first(struct intel_runtime_pm *rpm,
-			      struct intel_wakeref *wf,
-			      int (*fn)(struct intel_wakeref *wf));
-int __intel_wakeref_put_last(struct intel_runtime_pm *rpm,
-			     struct intel_wakeref *wf,
-			     int (*fn)(struct intel_wakeref *wf));
+int __intel_wakeref_get_first(struct intel_wakeref *wf);
+void __intel_wakeref_put_last(struct intel_wakeref *wf);
 
 /**
  * intel_wakeref_get: Acquire the wakeref
@@ -61,12 +71,10 @@ int __intel_wakeref_put_last(struct intel_runtime_pm *rpm,
  * code otherwise.
  */
 static inline int
-intel_wakeref_get(struct intel_runtime_pm *rpm,
-		  struct intel_wakeref *wf,
-		  int (*fn)(struct intel_wakeref *wf))
+intel_wakeref_get(struct intel_wakeref *wf)
 {
 	if (unlikely(!atomic_inc_not_zero(&wf->count)))
-		return __intel_wakeref_get_first(rpm, wf, fn);
+		return __intel_wakeref_get_first(wf);
 
 	return 0;
 }
@@ -102,16 +110,12 @@ intel_wakeref_get_if_active(struct intel_wakeref *wf)
  * Returns: 0 if the wakeref was released successfully, or a negative error
  * code otherwise.
  */
-static inline int
-intel_wakeref_put(struct intel_runtime_pm *rpm,
-		  struct intel_wakeref *wf,
-		  int (*fn)(struct intel_wakeref *wf))
+static inline void
+intel_wakeref_put(struct intel_wakeref *wf)
 {
 	INTEL_WAKEREF_BUG_ON(atomic_read(&wf->count) <= 0);
-	if (atomic_dec_and_mutex_lock(&wf->count, &wf->mutex))
-		return __intel_wakeref_put_last(rpm, wf, fn);
-
-	return 0;
+	if (unlikely(!atomic_add_unless(&wf->count, -1, 1)))
+		__intel_wakeref_put_last(wf);
 }
 
 /**
-- 
2.23.0.rc1



More information about the Intel-gfx mailing list