| Message ID | 20180626115015.8521-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 26/06/2018 12:50, Chris Wilson wrote: > In the following patch, we will process the CSB events under the > timeline.lock and not serialised by the tasklet. This also means that we > will need to protect access to common variables such as > execlists->csb_head with the timeline.lock during reset. > > v2: Move sync_irq to avoid deadlocks between taking timeline.lock from > our interrupt handler. > v3: Kill off the synchronize_hardirq as it raises more questions than > answered; now we use the timeline.lock entirely for CSB serialisation > between the irq and elsewhere, we don't need to be so heavy handed with > flushing > v4: Treat request cancellation (wedging after failed reset) similarly > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 16 ++++------------ > 1 file changed, 4 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 009db92b67d7..4b31e8f42aeb 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -871,7 +871,6 @@ static void reset_irq(struct intel_engine_cs *engine) > { > /* Mark all CS interrupts as complete */ > smp_store_mb(engine->execlists.active, 0); > - synchronize_hardirq(engine->i915->drm.irq); > > clear_gtiir(engine); > > @@ -908,14 +907,12 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) > * submission's irq state, we also wish to remind ourselves that > * it is irq state.) > */ > - local_irq_save(flags); > + spin_lock_irqsave(&engine->timeline.lock, flags); > > /* Cancel the requests on the HW and clear the ELSP tracker. */ > execlists_cancel_port_requests(execlists); > reset_irq(engine); > > - spin_lock(&engine->timeline.lock); > - > /* Mark all executing requests as skipped. */ > list_for_each_entry(rq, &engine->timeline.requests, link) { > GEM_BUG_ON(!rq->global_seqno); > @@ -949,9 +946,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) > execlists->first = NULL; > GEM_BUG_ON(port_isset(execlists->port)); > > - spin_unlock(&engine->timeline.lock); > - > - local_irq_restore(flags); > + spin_unlock_irqrestore(&engine->timeline.lock, flags); > } > > static void process_csb(struct intel_engine_cs *engine) > @@ -1969,8 +1964,7 @@ static void execlists_reset(struct intel_engine_cs *engine, > engine->name, request ? request->global_seqno : 0, > intel_engine_get_seqno(engine)); > > - /* See execlists_cancel_requests() for the irq/spinlock split. */ > - local_irq_save(flags); > + spin_lock_irqsave(&engine->timeline.lock, flags); > > /* > * Catch up with any missed context-switch interrupts. > @@ -1985,14 +1979,12 @@ static void execlists_reset(struct intel_engine_cs *engine, > reset_irq(engine); > > /* Push back any incomplete requests for replay after the reset. */ > - spin_lock(&engine->timeline.lock); > __unwind_incomplete_requests(engine); > - spin_unlock(&engine->timeline.lock); > > /* Following the reset, we need to reload the CSB read/write pointers */ > engine->execlists.csb_head = GEN8_CSB_ENTRIES - 1; > > - local_irq_restore(flags); > + spin_unlock_irqrestore(&engine->timeline.lock, flags); > > /* > * If the request was innocent, we leave the request in the ELSP > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 009db92b67d7..4b31e8f42aeb 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -871,7 +871,6 @@ static void reset_irq(struct intel_engine_cs *engine) { /* Mark all CS interrupts as complete */ smp_store_mb(engine->execlists.active, 0); - synchronize_hardirq(engine->i915->drm.irq); clear_gtiir(engine); @@ -908,14 +907,12 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) * submission's irq state, we also wish to remind ourselves that * it is irq state.) */ - local_irq_save(flags); + spin_lock_irqsave(&engine->timeline.lock, flags); /* Cancel the requests on the HW and clear the ELSP tracker. */ execlists_cancel_port_requests(execlists); reset_irq(engine); - spin_lock(&engine->timeline.lock); - /* Mark all executing requests as skipped. */ list_for_each_entry(rq, &engine->timeline.requests, link) { GEM_BUG_ON(!rq->global_seqno); @@ -949,9 +946,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) execlists->first = NULL; GEM_BUG_ON(port_isset(execlists->port)); - spin_unlock(&engine->timeline.lock); - - local_irq_restore(flags); + spin_unlock_irqrestore(&engine->timeline.lock, flags); } static void process_csb(struct intel_engine_cs *engine) @@ -1969,8 +1964,7 @@ static void execlists_reset(struct intel_engine_cs *engine, engine->name, request ? request->global_seqno : 0, intel_engine_get_seqno(engine)); - /* See execlists_cancel_requests() for the irq/spinlock split. */ - local_irq_save(flags); + spin_lock_irqsave(&engine->timeline.lock, flags); /* * Catch up with any missed context-switch interrupts. @@ -1985,14 +1979,12 @@ static void execlists_reset(struct intel_engine_cs *engine, reset_irq(engine); /* Push back any incomplete requests for replay after the reset. */ - spin_lock(&engine->timeline.lock); __unwind_incomplete_requests(engine); - spin_unlock(&engine->timeline.lock); /* Following the reset, we need to reload the CSB read/write pointers */ engine->execlists.csb_head = GEN8_CSB_ENTRIES - 1; - local_irq_restore(flags); + spin_unlock_irqrestore(&engine->timeline.lock, flags); /* * If the request was innocent, we leave the request in the ELSP
In the following patch, we will process the CSB events under the timeline.lock and not serialised by the tasklet. This also means that we will need to protect access to common variables such as execlists->csb_head with the timeline.lock during reset. v2: Move sync_irq to avoid deadlocks between taking timeline.lock from our interrupt handler. v3: Kill off the synchronize_hardirq as it raises more questions than answered; now we use the timeline.lock entirely for CSB serialisation between the irq and elsewhere, we don't need to be so heavy handed with flushing v4: Treat request cancellation (wedging after failed reset) similarly Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)