[PATCH 1/9] drm/i915: Defer final intel_wakeref_put to process context
Chris Wilson
chris at chris-wilson.co.uk
Mon Aug 5 13:00:43 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.
v2: Add a selftest to ensure we exercise the code while lockdep watches.
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_cs.c | 1 +
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 21 ++---
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 | 17 +++-
drivers/gpu/drm/i915/gt/selftest_engine.c | 28 +++++++
drivers/gpu/drm/i915/gt/selftest_engine.h | 14 ++++
drivers/gpu/drm/i915/gt/selftest_engine_pm.c | 78 +++++++++++++++++++
drivers/gpu/drm/i915/intel_wakeref.c | 58 +++++++++-----
drivers/gpu/drm/i915/intel_wakeref.h | 45 ++++++-----
.../drm/i915/selftests/i915_live_selftests.h | 5 +-
11 files changed, 232 insertions(+), 77 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/selftest_engine.c
create mode 100644 drivers/gpu/drm/i915/gt/selftest_engine.h
create mode 100644 drivers/gpu/drm/i915/gt/selftest_engine_pm.c
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index a91f15717cc1..414b0dbb3a20 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1672,5 +1672,6 @@ intel_engine_find_active_request(struct intel_engine_cs *engine)
}
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_engine.c"
#include "selftest_engine_cs.c"
#endif
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index 0336204988d6..2ba231f210e7 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,18 @@ 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);
}
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_engine_pm.c"
+#endif
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..087b6cdad39c 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.h
@@ -17,14 +17,27 @@ enum {
INTEL_GT_PARK,
};
-void intel_gt_pm_get(struct intel_gt *gt);
-void intel_gt_pm_put(struct intel_gt *gt);
+static inline bool
+intel_gt_pm_is_awake(const struct intel_gt *gt)
+{
+ return intel_wakeref_is_active(>->wakeref);
+}
+
+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/gt/selftest_engine.c b/drivers/gpu/drm/i915/gt/selftest_engine.c
new file mode 100644
index 000000000000..f65b118e261d
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_engine.c
@@ -0,0 +1,28 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "selftest_engine.h"
+
+int intel_engine_live_selftests(struct drm_i915_private *i915)
+{
+ static int (* const tests[])(struct intel_gt *) = {
+ live_engine_pm_selftests,
+ NULL,
+ };
+ struct intel_gt *gt = &i915->gt;
+ typeof(*tests) *fn;
+
+ for (fn = tests; *fn; fn++) {
+ int err;
+
+ err = (*fn)(gt);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine.h b/drivers/gpu/drm/i915/gt/selftest_engine.h
new file mode 100644
index 000000000000..ab32d09ec5a1
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_engine.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2019 Intel Corporation
+ */
+
+#ifndef SELFTEST_ENGINE_H
+#define SELFTEST_ENGINE_H
+
+struct intel_gt;
+
+int live_engine_pm_selftests(struct intel_gt *gt);
+
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_engine_pm.c b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
new file mode 100644
index 000000000000..d94aaabb3719
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_engine_pm.c
@@ -0,0 +1,78 @@
+/*
+ * SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright © 2018 Intel Corporation
+ */
+
+#include "i915_selftest.h"
+#include "selftest_engine.h"
+#include "selftests/igt_atomic.h"
+
+static int live_engine_pm(void *arg)
+{
+ struct intel_gt *gt = arg;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
+ /*
+ * Check we can call intel_engine_pm_put from any context. No
+ * failures are reported directly, but if we mess up lockdep should
+ * tell us.
+ */
+ GEM_BUG_ON(intel_gt_pm_is_awake(gt));
+ for_each_engine(engine, gt->i915, id) {
+ const typeof(*igt_atomic_phases) *p;
+
+ for (p = igt_atomic_phases; p->name; p++) {
+ /*
+ * Acquisition is always synchronous, except if we
+ * know that the engine is already awale, in which
+ * we should use intel_engine_pm_get_if_awake() to
+ * atomically grab the wakeref.
+ *
+ * In practice,
+ * intel_engine_pm_get();
+ * intel_engine_pm_put();
+ * occurs in one thread, while simultaneously
+ * intel_engine_pm_get_if_awake();
+ * intel_engine_pm_put();
+ * occurs in atomic context in another.
+ */
+ GEM_BUG_ON(intel_engine_pm_is_awake(engine));
+ intel_engine_pm_get(engine);
+
+ p->critical_section_begin();
+ if (!intel_engine_pm_get_if_awake(engine))
+ pr_err("intel_engine_pm_get_if_awake(%s) failed under %s\n",
+ engine->name, p->name);
+ else
+ intel_engine_pm_put(engine);
+ intel_engine_pm_put(engine);
+ p->critical_section_end();
+
+ flush_work(&engine->wakeref.work);
+ if (intel_engine_pm_is_awake(engine)) {
+ pr_err("%s is still awake after flushing pm\n",
+ engine->name);
+ return -EINVAL;
+ }
+
+ flush_work(>->wakeref.work);
+ if (intel_gt_pm_is_awake(gt)) {
+ pr_err("GT is still awake after flushing pm\n");
+ return -EINVAL;
+ }
+ }
+ }
+
+ return 0;
+}
+
+int live_engine_pm_selftests(struct intel_gt *gt)
+{
+ static const struct i915_subtest tests[] = {
+ SUBTEST(live_engine_pm),
+ };
+
+ return intel_gt_live_subtests(tests, gt);
+}
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);
}
/**
diff --git a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
index a841d3f9bedc..1ccf0f731ac0 100644
--- a/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_live_selftests.h
@@ -12,10 +12,11 @@
selftest(sanitycheck, i915_live_sanitycheck) /* keep first (igt selfcheck) */
selftest(uncore, intel_uncore_live_selftests)
selftest(workarounds, intel_workarounds_live_selftests)
-selftest(timelines, intel_timeline_live_selftests)
+selftest(gt_engines, intel_engine_live_selftests)
+selftest(gt_timelines, intel_timeline_live_selftests)
+selftest(gt_contexts, intel_context_live_selftests)
selftest(requests, i915_request_live_selftests)
selftest(active, i915_active_live_selftests)
-selftest(gt_contexts, intel_context_live_selftests)
selftest(objects, i915_gem_object_live_selftests)
selftest(mman, i915_gem_mman_live_selftests)
selftest(dmabuf, i915_gem_dmabuf_live_selftests)
--
2.23.0.rc1
More information about the Intel-gfx-trybot
mailing list