Commit e3ed90b8 authored by Chris Wilson's avatar Chris Wilson
Browse files

drm/i915/gt: Drain the breadcrumbs just once 



Matthew Brost pointed out that the while-loop on a shared breadcrumb was
inherently fraught with danger as it competed with the other users of
the breadcrumbs. However, in order to completely drain the re-arming irq
worker, the while-loop is a necessity, despite my optimism that we could
force cancellation with a couple of irq_work invocations.

Given that we can't merely drop the while-loop, use an activity counter on
the breadcrumbs to detect when we are parking the breadcrumbs for the
last time.

Based on a patch by Matthew Brost.

Reported-by: default avatarMatthew Brost <matthew.brost@intel.com>
Suggested-by: default avatarMatthew Brost <matthew.brost@intel.com>
Fixes: 9d5612ca ("drm/i915/gt: Defer enabling the breadcrumb interrupt to after submission")
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: default avatarMatthew Brost <matthew.brost@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20201217091524.10258-1-chris@chris-wilson.co.uk
parent 460d02ba

drivers/gpu/drm/i915/gt/intel_breadcrumbs.c

+6 −4
Original line number Diff line number Diff line
@@ -332,17 +332,19 @@ void intel_breadcrumbs_reset(struct intel_breadcrumbs *b) 
	spin_unlock_irqrestore(&b->irq_lock, flags);

}



void intel_breadcrumbs_park(struct intel_breadcrumbs *b)

void __intel_breadcrumbs_park(struct intel_breadcrumbs *b)

{

	/* Kick the work once more to drain the signalers */

	if (!READ_ONCE(b->irq_armed))

		return;



	/* Kick the work once more to drain the signalers, and disarm the irq */

	irq_work_sync(&b->irq_work);

	while (unlikely(READ_ONCE(b->irq_armed))) {

	while (READ_ONCE(b->irq_armed) && !atomic_read(&b->active)) {

		local_irq_disable();

		signal_irq_work(&b->irq_work);

		local_irq_enable();

		cond_resched();

	}

	GEM_BUG_ON(!list_empty(&b->signalers));

}



void intel_breadcrumbs_free(struct intel_breadcrumbs *b)

drivers/gpu/drm/i915/gt/intel_breadcrumbs.h

+12 −1
Original line number Diff line number Diff line
@@ -19,7 +19,18 @@ intel_breadcrumbs_create(struct intel_engine_cs *irq_engine); 
void intel_breadcrumbs_free(struct intel_breadcrumbs *b);



void intel_breadcrumbs_reset(struct intel_breadcrumbs *b);

void intel_breadcrumbs_park(struct intel_breadcrumbs *b);

void __intel_breadcrumbs_park(struct intel_breadcrumbs *b);



static inline void intel_breadcrumbs_unpark(struct intel_breadcrumbs *b)

{

	atomic_inc(&b->active);

}



static inline void intel_breadcrumbs_park(struct intel_breadcrumbs *b)

{

	if (atomic_dec_and_test(&b->active))

		__intel_breadcrumbs_park(b);

}



static inline void

intel_engine_signal_breadcrumbs(struct intel_engine_cs *engine)

drivers/gpu/drm/i915/gt/intel_breadcrumbs_types.h

+4 −2
Original line number Diff line number Diff line
@@ -29,8 +29,7 @@ 
 * the overhead of waking that client is much preferred.

 */

struct intel_breadcrumbs {

	/* Not all breadcrumbs are attached to physical HW */

	struct intel_engine_cs *irq_engine;

	atomic_t active;



	spinlock_t signalers_lock; /* protects the list of signalers */

	struct list_head signalers;
@@ -40,6 +39,9 @@ struct intel_breadcrumbs { 
	struct irq_work irq_work; /* for use from inside irq_lock */

	unsigned int irq_enabled;

	bool irq_armed;



	/* Not all breadcrumbs are attached to physical HW */

	struct intel_engine_cs *irq_engine;

};



#endif /* __INTEL_BREADCRUMBS_TYPES__ */

drivers/gpu/drm/i915/gt/intel_engine_pm.c

+1 −0
Original line number Diff line number Diff line
@@ -65,6 +65,7 @@ static int __engine_unpark(struct intel_wakeref *wf) 
	if (engine->unpark)

		engine->unpark(engine);



	intel_breadcrumbs_unpark(engine->breadcrumbs);

	intel_engine_unpark_heartbeat(engine);

	return 0;

}