| Message ID | 20190815042031.27750-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Move tasklet kicking to __i915_request_queue caller | expand |
Chris Wilson <chris@chris-wilson.co.uk> writes: > Since __i915_request_queue() may be called from hardirq (timer) context, > we cannot use local_bh_disable/enable at the lower level. As we do want > to kick the tasklet to speed up initial submission or preemption for > normal client submission, lift it to the normal process context > callpath. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_request.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 4021334dd2c5..8a2bc1d317e4 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1203,12 +1203,10 @@ void __i915_request_queue(struct i915_request *rq, > * decide whether to preempt the entire chain so that it is ready to > * run at the earliest possible convenience. > */ > - local_bh_disable(); > i915_sw_fence_commit(&rq->semaphore); > if (attr && rq->engine->schedule) > rq->engine->schedule(rq, attr); > i915_sw_fence_commit(&rq->submit); > - local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ Was this here to only optimize the latency from schedule to resubmission? Or is it actually guarding something? -Mika > } > > void i915_request_add(struct i915_request *rq) > @@ -1247,7 +1245,9 @@ void i915_request_add(struct i915_request *rq) > if (list_empty(&rq->sched.signalers_list)) > attr.priority |= I915_PRIORITY_WAIT; > > + local_bh_disable(); > __i915_request_queue(rq, &attr); > + local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ > > /* > * In typical scenarios, we do not expect the previous request on > -- > 2.23.0.rc1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Mika Kuoppala (2019-08-15 12:35:37) > Chris Wilson <chris@chris-wilson.co.uk> writes: > > > Since __i915_request_queue() may be called from hardirq (timer) context, > > we cannot use local_bh_disable/enable at the lower level. As we do want > > to kick the tasklet to speed up initial submission or preemption for > > normal client submission, lift it to the normal process context > > callpath. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_request.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > > index 4021334dd2c5..8a2bc1d317e4 100644 > > --- a/drivers/gpu/drm/i915/i915_request.c > > +++ b/drivers/gpu/drm/i915/i915_request.c > > @@ -1203,12 +1203,10 @@ void __i915_request_queue(struct i915_request *rq, > > * decide whether to preempt the entire chain so that it is ready to > > * run at the earliest possible convenience. > > */ > > - local_bh_disable(); > > i915_sw_fence_commit(&rq->semaphore); > > if (attr && rq->engine->schedule) > > rq->engine->schedule(rq, attr); > > i915_sw_fence_commit(&rq->submit); > > - local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ > > Was this here to only optimize the latency from schedule > to resubmission? It's only for latency minimisation. If we don't do it now, the raise_softirq will not be until the process hits the scheduler. Fence -> backend is all irq-spinlocks. -Chris
Chris Wilson <chris@chris-wilson.co.uk> writes: > Quoting Mika Kuoppala (2019-08-15 12:35:37) >> Chris Wilson <chris@chris-wilson.co.uk> writes: >> >> > Since __i915_request_queue() may be called from hardirq (timer) context, >> > we cannot use local_bh_disable/enable at the lower level. As we do want >> > to kick the tasklet to speed up initial submission or preemption for >> > normal client submission, lift it to the normal process context >> > callpath. >> > >> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> > --- >> > drivers/gpu/drm/i915/i915_request.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c >> > index 4021334dd2c5..8a2bc1d317e4 100644 >> > --- a/drivers/gpu/drm/i915/i915_request.c >> > +++ b/drivers/gpu/drm/i915/i915_request.c >> > @@ -1203,12 +1203,10 @@ void __i915_request_queue(struct i915_request *rq, >> > * decide whether to preempt the entire chain so that it is ready to >> > * run at the earliest possible convenience. >> > */ >> > - local_bh_disable(); >> > i915_sw_fence_commit(&rq->semaphore); >> > if (attr && rq->engine->schedule) >> > rq->engine->schedule(rq, attr); >> > i915_sw_fence_commit(&rq->submit); >> > - local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ >> >> Was this here to only optimize the latency from schedule >> to resubmission? > > It's only for latency minimisation. If we don't do it now, the > raise_softirq will not be until the process hits the scheduler. > > Fence -> backend is all irq-spinlocks. Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 4021334dd2c5..8a2bc1d317e4 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1203,12 +1203,10 @@ void __i915_request_queue(struct i915_request *rq, * decide whether to preempt the entire chain so that it is ready to * run at the earliest possible convenience. */ - local_bh_disable(); i915_sw_fence_commit(&rq->semaphore); if (attr && rq->engine->schedule) rq->engine->schedule(rq, attr); i915_sw_fence_commit(&rq->submit); - local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ } void i915_request_add(struct i915_request *rq) @@ -1247,7 +1245,9 @@ void i915_request_add(struct i915_request *rq) if (list_empty(&rq->sched.signalers_list)) attr.priority |= I915_PRIORITY_WAIT; + local_bh_disable(); __i915_request_queue(rq, &attr); + local_bh_enable(); /* Kick the execlists tasklet if just scheduled */ /* * In typical scenarios, we do not expect the previous request on
Since __i915_request_queue() may be called from hardirq (timer) context, we cannot use local_bh_disable/enable at the lower level. As we do want to kick the tasklet to speed up initial submission or preemption for normal client submission, lift it to the normal process context callpath. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_request.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)