| Message ID | 20170920143705.11277-4-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Sep 20, 2017 at 05:37:00PM +0300, Mika Kuoppala wrote: > On reset and wedged path, we want to release the requests > that are tied to ports and then mark the ports to be unset. > Introduce a function for this. > > v2: rebase > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index a4ece4c4f291..ffb9c900328b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine) > execlists_submit_ports(engine); > } > > +static void execlist_cancel_port_requests(struct intel_engine_execlist *el) > +{ > + unsigned int i; > + > + for (i = 0; i < ARRAY_SIZE(el->port); i++) > + i915_gem_request_put(port_request(&el->port[i])); > + > + memset(el->port, 0, sizeof(el->port)); > +} > + > static void execlists_cancel_requests(struct intel_engine_cs *engine) > { > struct intel_engine_execlist * const el = &engine->execlist; > @@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) > struct drm_i915_gem_request *rq, *rn; > struct rb_node *rb; > unsigned long flags; > - unsigned long n; > > spin_lock_irqsave(&engine->timeline->lock, flags); > > /* Cancel the requests on the HW and clear the ELSP tracker. */ > - for (n = 0; n < ARRAY_SIZE(el->port); n++) > - i915_gem_request_put(port_request(&port[n])); We could also drop the local variable for port. It's only used in GEM_BUG_ON(port_isset(&port[0])). Do we even need this assert when we're starting to treat ports in a more ring-like fashion? Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> -Michał > - memset(el->port, 0, sizeof(el->port)); > + execlist_cancel_port_requests(el); > > /* Mark all executing requests as skipped. */ > list_for_each_entry(rq, &engine->timeline->requests, link) { > @@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs *engine, > struct drm_i915_gem_request *request) > { > struct intel_engine_execlist * const el = &engine->execlist; > - struct execlist_port *port = el->port; > struct drm_i915_gem_request *rq, *rn; > struct intel_context *ce; > unsigned long flags; > - unsigned int n; > > spin_lock_irqsave(&engine->timeline->lock, flags); > > @@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs *engine, > * guessing the missed context-switch events by looking at what > * requests were completed. > */ > - for (n = 0; n < ARRAY_SIZE(el->port); n++) > - i915_gem_request_put(port_request(&port[n])); > - memset(el->port, 0, sizeof(el->port)); > + execlist_cancel_port_requests(el); > > /* Push back any incomplete requests for replay after the reset. */ > list_for_each_entry_safe_reverse(rq, rn, > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2017-09-20 15:37:00) > On reset and wedged path, we want to release the requests > that are tied to ports and then mark the ports to be unset. > Introduce a function for this. > > v2: rebase > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
Michał Winiarski <michal.winiarski@intel.com> writes: > On Wed, Sep 20, 2017 at 05:37:00PM +0300, Mika Kuoppala wrote: >> On reset and wedged path, we want to release the requests >> that are tied to ports and then mark the ports to be unset. >> Introduce a function for this. >> >> v2: rebase >> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> >> --- >> drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++--------- >> 1 file changed, 12 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c >> index a4ece4c4f291..ffb9c900328b 100644 >> --- a/drivers/gpu/drm/i915/intel_lrc.c >> +++ b/drivers/gpu/drm/i915/intel_lrc.c >> @@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine) >> execlists_submit_ports(engine); >> } >> >> +static void execlist_cancel_port_requests(struct intel_engine_execlist *el) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < ARRAY_SIZE(el->port); i++) >> + i915_gem_request_put(port_request(&el->port[i])); >> + >> + memset(el->port, 0, sizeof(el->port)); >> +} >> + >> static void execlists_cancel_requests(struct intel_engine_cs *engine) >> { >> struct intel_engine_execlist * const el = &engine->execlist; >> @@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) >> struct drm_i915_gem_request *rq, *rn; >> struct rb_node *rb; >> unsigned long flags; >> - unsigned long n; >> >> spin_lock_irqsave(&engine->timeline->lock, flags); >> >> /* Cancel the requests on the HW and clear the ELSP tracker. */ >> - for (n = 0; n < ARRAY_SIZE(el->port); n++) >> - i915_gem_request_put(port_request(&port[n])); > > We could also drop the local variable for port. Dropped. > It's only used in GEM_BUG_ON(port_isset(&port[0])). > Do we even need this assert when we're starting to treat ports in a more > ring-like fashion? > The memset is, still, so close there in this version that it indeed begs the question. But it is there to ensure that we really did the port parts properly. -Mika > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> > > -Michał > >> - memset(el->port, 0, sizeof(el->port)); >> + execlist_cancel_port_requests(el); >> >> /* Mark all executing requests as skipped. */ >> list_for_each_entry(rq, &engine->timeline->requests, link) { >> @@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs *engine, >> struct drm_i915_gem_request *request) >> { >> struct intel_engine_execlist * const el = &engine->execlist; >> - struct execlist_port *port = el->port; >> struct drm_i915_gem_request *rq, *rn; >> struct intel_context *ce; >> unsigned long flags; >> - unsigned int n; >> >> spin_lock_irqsave(&engine->timeline->lock, flags); >> >> @@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs *engine, >> * guessing the missed context-switch events by looking at what >> * requests were completed. >> */ >> - for (n = 0; n < ARRAY_SIZE(el->port); n++) >> - i915_gem_request_put(port_request(&port[n])); >> - memset(el->port, 0, sizeof(el->port)); >> + execlist_cancel_port_requests(el); >> >> /* Push back any incomplete requests for replay after the reset. */ >> list_for_each_entry_safe_reverse(rq, rn, >> -- >> 2.11.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a4ece4c4f291..ffb9c900328b 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -568,6 +568,16 @@ static void execlists_dequeue(struct intel_engine_cs *engine) execlists_submit_ports(engine); } +static void execlist_cancel_port_requests(struct intel_engine_execlist *el) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(el->port); i++) + i915_gem_request_put(port_request(&el->port[i])); + + memset(el->port, 0, sizeof(el->port)); +} + static void execlists_cancel_requests(struct intel_engine_cs *engine) { struct intel_engine_execlist * const el = &engine->execlist; @@ -575,14 +585,11 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine) struct drm_i915_gem_request *rq, *rn; struct rb_node *rb; unsigned long flags; - unsigned long n; spin_lock_irqsave(&engine->timeline->lock, flags); /* Cancel the requests on the HW and clear the ELSP tracker. */ - for (n = 0; n < ARRAY_SIZE(el->port); n++) - i915_gem_request_put(port_request(&port[n])); - memset(el->port, 0, sizeof(el->port)); + execlist_cancel_port_requests(el); /* Mark all executing requests as skipped. */ list_for_each_entry(rq, &engine->timeline->requests, link) { @@ -1372,11 +1379,9 @@ static void reset_common_ring(struct intel_engine_cs *engine, struct drm_i915_gem_request *request) { struct intel_engine_execlist * const el = &engine->execlist; - struct execlist_port *port = el->port; struct drm_i915_gem_request *rq, *rn; struct intel_context *ce; unsigned long flags; - unsigned int n; spin_lock_irqsave(&engine->timeline->lock, flags); @@ -1389,9 +1394,7 @@ static void reset_common_ring(struct intel_engine_cs *engine, * guessing the missed context-switch events by looking at what * requests were completed. */ - for (n = 0; n < ARRAY_SIZE(el->port); n++) - i915_gem_request_put(port_request(&port[n])); - memset(el->port, 0, sizeof(el->port)); + execlist_cancel_port_requests(el); /* Push back any incomplete requests for replay after the reset. */ list_for_each_entry_safe_reverse(rq, rn,
On reset and wedged path, we want to release the requests that are tied to ports and then mark the ports to be unset. Introduce a function for this. v2: rebase Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_lrc.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)