Quoting Daniele Ceraolo Spurio (2019-08-12 23:10:16)
> We rely on the tasklet to update the GT PM refcount, so we can't disable
> it even if we've processed all the requests for the engine because we
> might have detected the request completion before the interrupt arrived.
>
> Since on all platforms on which we plan to support guc submission we
> don't allow disabling the breadcrumb interrupts, we can further siplify
> the park/unpark flow by removing the interrupt pin/unpin. A BUG_ON has
> been added to catch changes to this flow that would require us to
> restore some kind of pinning.
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/gt/intel_breadcrumbs.c | 22 ----------------
> drivers/gpu/drm/i915/gt/intel_engine.h | 3 ---
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 25 ++++++++-----------
> 3 files changed, 11 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> index ceba1da61967..15bbdd8c7552 100644
> --- a/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_breadcrumbs.c
> @@ -212,28 +212,6 @@ static void signal_irq_work(struct irq_work *work)
> intel_engine_breadcrumbs_irq(engine);
> }
>
> -void intel_engine_pin_breadcrumbs_irq(struct intel_engine_cs *engine)
> -{
> - struct intel_breadcrumbs *b = &engine->breadcrumbs;
> -
> - spin_lock_irq(&b->irq_lock);
> - if (!b->irq_enabled++)
> - irq_enable(engine);
> - GEM_BUG_ON(!b->irq_enabled); /* no overflow! */
> - spin_unlock_irq(&b->irq_lock);
> -}
> -
> -void intel_engine_unpin_breadcrumbs_irq(struct intel_engine_cs *engine)
> -{
> - struct intel_breadcrumbs *b = &engine->breadcrumbs;
> -
> - spin_lock_irq(&b->irq_lock);
> - GEM_BUG_ON(!b->irq_enabled); /* no underflow! */
> - if (!--b->irq_enabled)
> - irq_disable(engine);
> - spin_unlock_irq(&b->irq_lock);
> -}
Could you split this to a second patch? The last draft of the pv-engine
was still using this pin_irq.
> -
> static void __intel_breadcrumbs_arm_irq(struct intel_breadcrumbs *b)
> {
> struct intel_engine_cs *engine =
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h b/drivers/gpu/drm/i915/gt/intel_engine.h
> index e1228b0e577f..bc694adcd9ea 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> @@ -338,9 +338,6 @@ void intel_engine_init_execlists(struct intel_engine_cs *engine);
> void intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
> void intel_engine_fini_breadcrumbs(struct intel_engine_cs *engine);
>
> -void intel_engine_pin_breadcrumbs_irq(struct intel_engine_cs *engine);
> -void intel_engine_unpin_breadcrumbs_irq(struct intel_engine_cs *engine);
> -
> void intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine);
> void intel_engine_disarm_breadcrumbs(struct intel_engine_cs *engine);
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 449ca6357018..deb054eeb37c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -1059,18 +1059,6 @@ static void guc_interrupts_release(struct intel_gt *gt)
> rps->pm_intrmsk_mbz &= ~ARAT_EXPIRED_INTRMSK;
> }
>
> -static void guc_submission_park(struct intel_engine_cs *engine)
> -{
> - intel_engine_unpin_breadcrumbs_irq(engine);
> - engine->flags &= ~I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> -}
> -
> -static void guc_submission_unpark(struct intel_engine_cs *engine)
> -{
> - engine->flags |= I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> - intel_engine_pin_breadcrumbs_irq(engine);
> -}
> -
> static void guc_set_default_submission(struct intel_engine_cs *engine)
> {
> /*
> @@ -1088,8 +1076,8 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
>
> engine->execlists.tasklet.func = guc_submission_tasklet;
>
> - engine->park = guc_submission_park;
> - engine->unpark = guc_submission_unpark;
> + /* do not use execlists park/unpark */
> + engine->park = engine->unpark = NULL;
>
> engine->reset.prepare = guc_reset_prepare;
> engine->reset.reset = guc_reset;
> @@ -1098,6 +1086,15 @@ static void guc_set_default_submission(struct intel_engine_cs *engine)
> engine->cancel_requests = guc_cancel_requests;
>
> engine->flags &= ~I915_ENGINE_SUPPORTS_STATS;
> + engine->flags |= I915_ENGINE_NEEDS_BREADCRUMB_TASKLET;
> +
> + /*
> + * For the breadcrumb irq to work we need the interrupts to stay
> + * enabled. However, on all platforms on which we'll have support for
> + * GuC submission we don't allow disabling the interrupts at runtime, so
> + * we're always safe with the current flow.
> + */
> + GEM_BUG_ON(engine->irq_enable || engine->irq_disable);
After a momentary panic, yes.
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
