| Message ID | 20180730121409.22689-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915: Limit C-states when waiting for the active request | expand |
Quoting Chris Wilson (2018-07-30 13:14:09) > If we are waiting for the currently executing request, we have a good > idea that it will be completed in the very near future and so want to > cap the CPU_DMA_LATENCY to ensure that we wake up the client quickly. > > v2: Not allowed to block in kmalloc after setting TASK_INTERRUPTIBLE. > > Testcase: igt/gem_sync/store-default > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Eero Tamminen <eero.t.tamminen@intel.com> > Cc: Francisco Jerez <currojerez@riseup.net> > --- > drivers/gpu/drm/i915/i915_request.c | 29 +++++++++++++++++++++++++++++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c > index 5c2c93cbab12..de907150664e 100644 > --- a/drivers/gpu/drm/i915/i915_request.c > +++ b/drivers/gpu/drm/i915/i915_request.c > @@ -1258,6 +1258,19 @@ static bool __i915_wait_request_check_and_reset(struct i915_request *request) > return true; > } > > +struct pm_qos { > + struct pm_qos_request req; > + struct work_struct work; > +}; > + > +static void pm_qos_del(struct work_struct *work) > +{ > + struct pm_qos *pm_qos = container_of(work, typeof(*pm_qos), work); > + > + pm_qos_remove_request(&pm_qos->req); > + kfree(pm_qos); > +} > + > /** > * i915_request_wait - wait until execution of request has finished > * @rq: the request to wait upon > @@ -1286,6 +1299,7 @@ long i915_request_wait(struct i915_request *rq, > wait_queue_head_t *errq = &rq->i915->gpu_error.wait_queue; > DEFINE_WAIT_FUNC(reset, default_wake_function); > DEFINE_WAIT_FUNC(exec, default_wake_function); > + struct pm_qos *pm_qos = NULL; > struct intel_wait wait; > > might_sleep(); > @@ -1363,6 +1377,19 @@ long i915_request_wait(struct i915_request *rq, > break; > } > > + if (!pm_qos && > + i915_seqno_passed(intel_engine_get_seqno(rq->engine), > + wait.seqno - 1)) { > + pm_qos = kzalloc(sizeof(*pm_qos), > + GFP_NOWAIT | __GFP_NOWARN); > + if (pm_qos) { > + pm_qos_add_request(&pm_qos->req, > + PM_QOS_CPU_DMA_LATENCY, > + 50); Ho hum, it should also then complain about the blocking notifier here. Let's see how observant it is... -Chris
Quoting Chris Wilson (2018-07-30 13:25:03) > Quoting Chris Wilson (2018-07-30 13:14:09) > > + if (!pm_qos && > > + i915_seqno_passed(intel_engine_get_seqno(rq->engine), > > + wait.seqno - 1)) { > > + pm_qos = kzalloc(sizeof(*pm_qos), > > + GFP_NOWAIT | __GFP_NOWARN); > > + if (pm_qos) { > > + pm_qos_add_request(&pm_qos->req, > > + PM_QOS_CPU_DMA_LATENCY, > > + 50); > > Ho hum, it should also then complain about the blocking notifier > here. Let's see how observant it is... Though that was kind of the intention as we could afford to lose a bit of time before sleeping, so I don't think the blocking notifier itself is a big deal... It would be a bit of a nuisance to avoid, the easiest might be to reuse the work. -Chris
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c index 5c2c93cbab12..de907150664e 100644 --- a/drivers/gpu/drm/i915/i915_request.c +++ b/drivers/gpu/drm/i915/i915_request.c @@ -1258,6 +1258,19 @@ static bool __i915_wait_request_check_and_reset(struct i915_request *request) return true; } +struct pm_qos { + struct pm_qos_request req; + struct work_struct work; +}; + +static void pm_qos_del(struct work_struct *work) +{ + struct pm_qos *pm_qos = container_of(work, typeof(*pm_qos), work); + + pm_qos_remove_request(&pm_qos->req); + kfree(pm_qos); +} + /** * i915_request_wait - wait until execution of request has finished * @rq: the request to wait upon @@ -1286,6 +1299,7 @@ long i915_request_wait(struct i915_request *rq, wait_queue_head_t *errq = &rq->i915->gpu_error.wait_queue; DEFINE_WAIT_FUNC(reset, default_wake_function); DEFINE_WAIT_FUNC(exec, default_wake_function); + struct pm_qos *pm_qos = NULL; struct intel_wait wait; might_sleep(); @@ -1363,6 +1377,19 @@ long i915_request_wait(struct i915_request *rq, break; } + if (!pm_qos && + i915_seqno_passed(intel_engine_get_seqno(rq->engine), + wait.seqno - 1)) { + pm_qos = kzalloc(sizeof(*pm_qos), + GFP_NOWAIT | __GFP_NOWARN); + if (pm_qos) { + pm_qos_add_request(&pm_qos->req, + PM_QOS_CPU_DMA_LATENCY, + 50); + INIT_WORK(&pm_qos->work, pm_qos_del); + } + } + timeout = io_schedule_timeout(timeout); if (intel_wait_complete(&wait) && @@ -1412,6 +1439,8 @@ long i915_request_wait(struct i915_request *rq, if (flags & I915_WAIT_LOCKED) remove_wait_queue(errq, &reset); remove_wait_queue(&rq->execute, &exec); + if (pm_qos) + schedule_work(&pm_qos->work); trace_i915_request_wait_end(rq); return timeout;
If we are waiting for the currently executing request, we have a good idea that it will be completed in the very near future and so want to cap the CPU_DMA_LATENCY to ensure that we wake up the client quickly. v2: Not allowed to block in kmalloc after setting TASK_INTERRUPTIBLE. Testcase: igt/gem_sync/store-default Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> Cc: Eero Tamminen <eero.t.tamminen@intel.com> Cc: Francisco Jerez <currojerez@riseup.net> --- drivers/gpu/drm/i915/i915_request.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)