Chris Wilson <ch...@chris-wilson.co.uk> writes:
> On finishing the reset, the intention is to restart the GPU before we
> relinquish the forcewake taken to handle the reset - the goal being the
> GPU reloads a context before it is allowed to sleep. For this purpose,
> we used tasklet_flush() which although it accomplished the goal of
> restarting the GPU, carried with it a sting in its tail: it cleared the
> TASKLET_STATE_SCHED bit. This meant that if another CPU queued a new
> request to this engine, we would clear the flag and later attempt to
> requeue the tasklet on the local CPU, breaking the per-cpu softirq
> lists.
>
> Remove the dangerous tasklet_kill() and just run the tasklet func
> directly as we know it is safe to do so (the tasklets are internally
> locked to allow mixed usage from direct submission).
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107710
> Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
> Cc: Mika Kuoppala <mika.kuopp...@intel.com>
> Cc: Michel Thierry <michel.thie...@intel.com>
> Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.h | 6 ------
> drivers/gpu/drm/i915/intel_lrc.c | 17 ++++++-----------
> 2 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.h b/drivers/gpu/drm/i915/i915_gem.h
> index e46592956872..599c4f6eb1ea 100644
> --- a/drivers/gpu/drm/i915/i915_gem.h
> +++ b/drivers/gpu/drm/i915/i915_gem.h
> @@ -82,12 +82,6 @@ static inline void __tasklet_disable_sync_once(struct
> tasklet_struct *t)
> tasklet_unlock_wait(t);
> }
>
> -static inline void __tasklet_enable_sync_once(struct tasklet_struct *t)
> -{
> - if (atomic_dec_return(&t->count) == 0)
> - tasklet_kill(t);
> -}
> -
> static inline bool __tasklet_is_enabled(const struct tasklet_struct *t)
> {
> return !atomic_read(&t->count);
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> b/drivers/gpu/drm/i915/intel_lrc.c
> index 36050f085071..f8ceb9c99dd6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1962,21 +1962,16 @@ static void execlists_reset_finish(struct
> intel_engine_cs *engine)
> {
> struct intel_engine_execlists * const execlists = &engine->execlists;
>
> - /* After a GPU reset, we may have requests to replay */
> - if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
> - tasklet_schedule(&execlists->tasklet);
> -
> /*
> - * Flush the tasklet while we still have the forcewake to be sure
> - * that it is not allowed to sleep before we restart and reload a
> - * context.
> + * After a GPU reset, we may have requests to replay. Do so now while
> + * we still have the forcewake to be sure that the GPU is not allowed
> + * to sleep before we restart and reload a context.
> *
> - * As before (with execlists_reset_prepare) we rely on the caller
> - * serialising multiple attempts to reset so that we know that we
> - * are the only one manipulating tasklet state.
> */
> - __tasklet_enable_sync_once(&execlists->tasklet);
> + if (!RB_EMPTY_ROOT(&execlists->queue.rb_root))
> + execlists->tasklet.func(execlists->tasklet.data);
The enabling right after should be enough of a hint that
we can do this.
Reviewed-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
>
> + tasklet_enable(&execlists->tasklet);
> GEM_TRACE("%s: depth->%d\n", engine->name,
> atomic_read(&execlists->tasklet.count));
> }
> --
> 2.18.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
