| Message ID | 20180326115044.2505-3-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-03-26 12:50:35) > When cancelling the requests and clearing out the ports following a > successful preemption completion, also clear the active flag. I had > assumed that all preemptions would be followed by an immediate dequeue > (preserving the active user flag), but under rare circumstances we may > be triggering a preemption for the second port only for it to have > completed before the preemotion kicks in; leaving execlists->active set > even though the system is now idle. > > We can clear the flag inside the common execlists_cancel_port_requests() > as the other users also expect the semantics of active being cleared. > > Fixes: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Michał Winiarski <michal.winiarski@intel.com> > Cc: Michel Thierry <michel.thierry@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> From the earlier posting, Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> Mika, any chance you want to complete the hat check and review the first patch as well? :) -Chris
Quoting Chris Wilson (2018-03-27 11:00:32) > Quoting Chris Wilson (2018-03-26 12:50:35) > > When cancelling the requests and clearing out the ports following a > > successful preemption completion, also clear the active flag. I had > > assumed that all preemptions would be followed by an immediate dequeue > > (preserving the active user flag), but under rare circumstances we may > > be triggering a preemption for the second port only for it to have > > completed before the preemotion kicks in; leaving execlists->active set > > even though the system is now idle. > > > > We can clear the flag inside the common execlists_cancel_port_requests() > > as the other users also expect the semantics of active being cleared. > > > > Fixes: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Michał Winiarski <michal.winiarski@intel.com> > > Cc: Michel Thierry <michel.thierry@intel.com> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > From the earlier posting, > Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> > > Mika, any chance you want to complete the hat check and review the first > patch as well? :) s/hat check/hat trick/ -Chris
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 104b69e0494f..c302952ab476 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -577,6 +577,8 @@ static void execlists_dequeue(struct intel_engine_cs *engine) * know the next preemption status we see corresponds * to this ELSP update. */ + GEM_BUG_ON(!execlists_is_active(execlists, + EXECLISTS_ACTIVE_USER)); GEM_BUG_ON(!port_count(&port[0])); if (port_count(&port[0]) > 1) goto unlock; @@ -738,6 +740,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists) memset(port, 0, sizeof(*port)); port++; } + + execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); } static void clear_gtiir(struct intel_engine_cs *engine) @@ -1042,6 +1046,11 @@ static void execlists_submission_tasklet(unsigned long data) if (fw) intel_uncore_forcewake_put(dev_priv, execlists->fw_domains); + + /* If the engine is now idle, so should be the flag; and vice versa. */ + GEM_BUG_ON(execlists_is_active(&engine->execlists, + EXECLISTS_ACTIVE_USER) == + !port_isset(engine->execlists.port)); } static void queue_request(struct intel_engine_cs *engine,
When cancelling the requests and clearing out the ports following a successful preemption completion, also clear the active flag. I had assumed that all preemptions would be followed by an immediate dequeue (preserving the active user flag), but under rare circumstances we may be triggering a preemption for the second port only for it to have completed before the preemotion kicks in; leaving execlists->active set even though the system is now idle. We can clear the flag inside the common execlists_cancel_port_requests() as the other users also expect the semantics of active being cleared. Fixes: f6322eddaff7 ("drm/i915/preemption: Allow preemption between submission ports") Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Michał Winiarski <michal.winiarski@intel.com> Cc: Michel Thierry <michel.thierry@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 9 +++++++++ 1 file changed, 9 insertions(+)