On 11/05/2023 09:20, Luca Coelho wrote:
Add a work queue in the intel_wakeref structure to be used exclusively
by the wake reference mechanism. This is needed in order to avoid
using the system workqueue and relying on flush_scheduled_work().
Cc: Tetsuo Handa <[email protected]>
Cc: Tvrtko Ursulin <[email protected]>
Cc: Jani Nikula <[email protected]>
Cc: Ville Syrjälä <[email protected]>
Signed-off-by: Luca Coelho <[email protected]>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 7 ++++++-
drivers/gpu/drm/i915/gt/intel_engine_pm.c | 15 ++++++++++++--
drivers/gpu/drm/i915/gt/intel_engine_pm.h | 3 ++-
drivers/gpu/drm/i915/gt/mock_engine.c | 8 +++++++-
drivers/gpu/drm/i915/intel_wakeref.c | 21 ++++++++++++++-----
drivers/gpu/drm/i915/intel_wakeref.h | 25 +++++++++++++++--------
6 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 0aff5bb13c53..6505bfa70cd0 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1290,7 +1290,11 @@ static int engine_setup_common(struct intel_engine_cs
*engine)
goto err_cmd_parser;
intel_engine_init_execlists(engine);
- intel_engine_init__pm(engine);
+
+ err = intel_engine_init__pm(engine);
+ if (err)
+ goto err_cmd_parser;
+
intel_engine_init_retire(engine);
/* Use the whole device by default */
@@ -1525,6 +1529,7 @@ void intel_engine_cleanup_common(struct intel_engine_cs
*engine)
{
GEM_BUG_ON(!list_empty(&engine->sched_engine->requests));
+ intel_engine_destroy__pm(engine);
i915_sched_engine_put(engine->sched_engine);
intel_breadcrumbs_put(engine->breadcrumbs);
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index ee531a5c142c..859b62cf660f 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -294,14 +294,25 @@ static const struct intel_wakeref_ops wf_ops = {
.put = __engine_park,
};
-void intel_engine_init__pm(struct intel_engine_cs *engine)
+int intel_engine_init__pm(struct intel_engine_cs *engine)
{
struct intel_runtime_pm *rpm = engine->uncore->rpm;
+ int err;
+
+ err = intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
+ if (err)
+ return err;
- intel_wakeref_init(&engine->wakeref, rpm, &wf_ops);
intel_engine_init_heartbeat(engine);
intel_gsc_idle_msg_enable(engine);
+
+ return 0;
+}
+
+void intel_engine_destroy__pm(struct intel_engine_cs *engine)
+{
+ intel_wakeref_destroy(&engine->wakeref);
}
/**
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
index d68675925b79..e8568f7d10c6 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.h
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.h
@@ -104,7 +104,8 @@ intel_engine_create_kernel_request(struct intel_engine_cs
*engine)
return rq;
}
-void intel_engine_init__pm(struct intel_engine_cs *engine);
+int intel_engine_init__pm(struct intel_engine_cs *engine);
+void intel_engine_destroy__pm(struct intel_engine_cs *engine);
void intel_engine_reset_pinned_contexts(struct intel_engine_cs *engine);
diff --git a/drivers/gpu/drm/i915/gt/mock_engine.c b/drivers/gpu/drm/i915/gt/mock_engine.c
index c0637bf799a3..0a3c702c21e2 100644
--- a/drivers/gpu/drm/i915/gt/mock_engine.c
+++ b/drivers/gpu/drm/i915/gt/mock_engine.c
@@ -336,6 +336,7 @@ static void mock_engine_release(struct intel_engine_cs
*engine)
intel_context_put(engine->kernel_context);
intel_engine_fini_retire(engine);
+ intel_engine_destroy__pm(engine);
}
struct intel_engine_cs *mock_engine(struct drm_i915_private *i915,
@@ -393,6 +394,7 @@ struct intel_engine_cs *mock_engine(struct drm_i915_private
*i915,
int mock_engine_init(struct intel_engine_cs *engine)
{
struct intel_context *ce;
+ int err;
INIT_LIST_HEAD(&engine->pinned_contexts_list);
@@ -402,7 +404,11 @@ int mock_engine_init(struct intel_engine_cs *engine)
engine->sched_engine->private_data = engine;
intel_engine_init_execlists(engine);
- intel_engine_init__pm(engine);
+
+ err = intel_engine_init__pm(engine);
+ if (err)
+ return err;
+
intel_engine_init_retire(engine);
engine->breadcrumbs = intel_breadcrumbs_create(NULL);
diff --git a/drivers/gpu/drm/i915/intel_wakeref.c
b/drivers/gpu/drm/i915/intel_wakeref.c
index dfd87d082218..6bae609e1312 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.c
+++ b/drivers/gpu/drm/i915/intel_wakeref.c
@@ -74,7 +74,7 @@ void __intel_wakeref_put_last(struct intel_wakeref *wf,
unsigned long flags)
/* Assume we are not in process context and so cannot sleep. */
if (flags & INTEL_WAKEREF_PUT_ASYNC || !mutex_trylock(&wf->mutex)) {
- mod_delayed_work(system_wq, &wf->work,
+ mod_delayed_work(wf->wq, &wf->work,
FIELD_GET(INTEL_WAKEREF_PUT_DELAY, flags));
return;
}
@@ -93,10 +93,10 @@ static void __intel_wakeref_put_work(struct work_struct
*wrk)
____intel_wakeref_put_last(wf);
}
-void __intel_wakeref_init(struct intel_wakeref *wf,
- struct intel_runtime_pm *rpm,
- const struct intel_wakeref_ops *ops,
- struct intel_wakeref_lockclass *key)
+int __intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops,
+ struct intel_wakeref_lockclass *key)
{
wf->rpm = rpm;
wf->ops = ops;
@@ -105,9 +105,20 @@ void __intel_wakeref_init(struct intel_wakeref *wf,
atomic_set(&wf->count, 0);
wf->wakeref = 0;
+ wf->wq = alloc_workqueue("i1915-wakeref", 0, 0);
Typo here - I wanted to ask however - why does this particular wq
"deserves" to be dedicated and can't just use one of the
drm_i915_private ones?
Regards,
Tvrtko
+ if (wf->wq == NULL)
+ return -ENOMEM;
+
INIT_DELAYED_WORK(&wf->work, __intel_wakeref_put_work);
lockdep_init_map(&wf->work.work.lockdep_map,
"wakeref.work", &key->work, 0);
+
+ return 0;
+}
+
+void intel_wakeref_destroy(struct intel_wakeref *wf)
+{
+ destroy_workqueue(wf->wq);
}
int intel_wakeref_wait_for_idle(struct intel_wakeref *wf)
diff --git a/drivers/gpu/drm/i915/intel_wakeref.h
b/drivers/gpu/drm/i915/intel_wakeref.h
index 0b6b4852ab23..86f99c044444 100644
--- a/drivers/gpu/drm/i915/intel_wakeref.h
+++ b/drivers/gpu/drm/i915/intel_wakeref.h
@@ -42,6 +42,7 @@ struct intel_wakeref {
struct intel_runtime_pm *rpm;
const struct intel_wakeref_ops *ops;
+ struct workqueue_struct *wq;
struct delayed_work work;
};
@@ -50,15 +51,21 @@ struct intel_wakeref_lockclass {
struct lock_class_key work;
};
-void __intel_wakeref_init(struct intel_wakeref *wf,
- struct intel_runtime_pm *rpm,
- const struct intel_wakeref_ops *ops,
- struct intel_wakeref_lockclass *key);
-#define intel_wakeref_init(wf, rpm, ops) do { \
- static struct intel_wakeref_lockclass __key; \
- \
- __intel_wakeref_init((wf), (rpm), (ops), &__key); \
-} while (0)
+int __intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops,
+ struct intel_wakeref_lockclass *key);
+static inline int
+intel_wakeref_init(struct intel_wakeref *wf,
+ struct intel_runtime_pm *rpm,
+ const struct intel_wakeref_ops *ops)
+{
+ static struct intel_wakeref_lockclass __key;
+
+ return __intel_wakeref_init((wf), (rpm), (ops), &__key);
+}
+
+void intel_wakeref_destroy(struct intel_wakeref *wf);
int __intel_wakeref_get_first(struct intel_wakeref *wf);
void __intel_wakeref_put_last(struct intel_wakeref *wf, unsigned long flags);
