[Intel-gfx] [PATCH 2/3] drm/i915/gt: create workqueue dedicated to wake references
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri May 12 09:04:17 UTC 2023
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 <penguin-kernel at I-love.SAKURA.ne.jp>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Jani Nikula <jani.nikula at intel.com>
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Luca Coelho <luciano.coelho at intel.com>
> ---
> 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);
More information about the Intel-gfx
mailing list