Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> Sadly lockdep records when the irqs are reenabled and then marks up the
> fake lock as being irq-unsafe. Our hand is forced and so we must mark up
> the entire fake lock critical section as irq-off.
>
Tricky tweaks are tricky. It was not so easy
to lure lockdep to do our bidding :(
> Hopefully this is the last tweak required!
That's the spirit.
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Fixes: d67739268cf0 ("drm/i915/gt: Mark up the nested engine-pm timeline lock as irqsafe")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_pm.c | 28 +++++++++++++++--------
> 1 file changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index a372d4ea9370..65b5ca74b394 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -39,27 +39,32 @@ static int __engine_unpark(struct intel_wakeref *wf)
>
> #if IS_ENABLED(CONFIG_LOCKDEP)
>
> -static inline void __timeline_mark_lock(struct intel_context *ce)
> +static inline unsigned long __timeline_mark_lock(struct intel_context *ce)
> {
> unsigned long flags;
>
> local_irq_save(flags);
> mutex_acquire(&ce->timeline->mutex.dep_map, 2, 0, _THIS_IP_);
> - local_irq_restore(flags);
> +
> + return flags;
> }
>
> -static inline void __timeline_mark_unlock(struct intel_context *ce)
> +static inline void __timeline_mark_unlock(struct intel_context *ce,
> + unsigned long flags)
> {
> mutex_release(&ce->timeline->mutex.dep_map, 0, _THIS_IP_);
> + local_irq_restore(flags);
> }
>
> #else
>
> -static inline void __timeline_mark_lock(struct intel_context *ce)
> +static inline unsigned long __timeline_mark_lock(struct intel_context *ce)
> {
> + return 0;
> }
>
> -static inline void __timeline_mark_unlock(struct intel_context *ce)
> +static inline void __timeline_mark_unlock(struct intel_context *ce,
> + unsigned long flags)
> {
> }
>
> @@ -68,6 +73,8 @@ static inline void __timeline_mark_unlock(struct intel_context *ce)
> static bool switch_to_kernel_context(struct intel_engine_cs *engine)
> {
> struct i915_request *rq;
> + unsigned long flags;
> + bool result = true;
>
> /* Already inside the kernel context, safe to power down. */
> if (engine->wakeref_serial == engine->serial)
> @@ -89,12 +96,12 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
> * retiring the last request, thus all rings should be empty and
> * all timelines idle.
> */
> - __timeline_mark_lock(engine->kernel_context);
> + flags = __timeline_mark_lock(engine->kernel_context);
>
> rq = __i915_request_create(engine->kernel_context, GFP_NOWAIT);
> if (IS_ERR(rq))
> /* Context switch failed, hope for the best! Maybe reset? */
> - return true;
> + goto out_unlock;
>
> intel_timeline_enter(rq->timeline);
>
> @@ -110,9 +117,10 @@ static bool switch_to_kernel_context(struct intel_engine_cs *engine)
> __intel_wakeref_defer_park(&engine->wakeref);
> __i915_request_queue(rq, NULL);
>
> - __timeline_mark_unlock(engine->kernel_context);
> -
> - return false;
> + result = false;
> +out_unlock:
> + __timeline_mark_unlock(engine->kernel_context, flags);
> + return result;
> }
>
> static int __engine_park(struct intel_wakeref *wf)
> --
> 2.23.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
