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@xxxxxxxxxxxxxxxxxx> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx> --- 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
