[Intel-gfx] [PATCH] drm/i915: Defer final intel_wakeref_put to process context
Chris Wilson
chris at chris-wilson.co.uk
Mon Aug 5 10:42:16 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>
---
Just rearranging the struct for fun.
---
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 | 58 +++++++++++++++--------
drivers/gpu/drm/i915/intel_wakeref.h | 45 ++++++++++--------
6 files changed, 98 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 = >->i915->runtime_pm;
-
- intel_wakeref_get(rpm, >->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 = >->i915->runtime_pm;
-
- intel_wakeref_put(rpm, >->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(>->wakeref);
+ intel_wakeref_init(>->wakeref, >->i915->runtime_pm, &wf_ops);
+
BLOCKING_INIT_NOTIFIER_HEAD(>->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(>->wakeref);
+}
static inline bool intel_gt_pm_get_if_awake(struct intel_gt *gt)
{
return intel_wakeref_get_if_active(>->wakeref);
}
+static inline void intel_gt_pm_put(struct intel_gt *gt)
+{
+ intel_wakeref_put(>->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..432081e320fe 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,47 @@ 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;
+ INTEL_WAKEREF_BUG_ON(work_pending(&wf->work));
+
+ if (in_interrupt()) {
+ 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..d63e8bc6e38f 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,39 @@
#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 {
atomic_t count;
struct mutex mutex;
+
intel_wakeref_t wakeref;
+
+ struct intel_runtime_pm *rpm;
+ const struct intel_wakeref_ops *ops;
+
+ struct work_struct work;
};
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 +72,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 +111,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