On 9/26/2019 7:25 AM, Chris Wilson wrote:
> Moving our primary irq handler to a RT thread incurs an extra 1us delay
> in process interrupts. This is most notice in waking up client threads,
> where it adds about 20% of extra latency. It also imposes a delay in
> feeding the GPU, an extra 1us before signaling secondary engines and
> extra latency in resubmitting work to keep the GPU busy. The latter case
> is insignificant as the latency hidden by the active GPU, and
> preempt-to-busy ensures that no extra latency is incurred for
> preemption.
>
> The benefit is that we reduced the impact on the rest of the system, the
> cycletest shows a reduction from 5us mean latency to 2us, with the
> maximum observed latency (in a 2 minute window) reduced by over 160us.
Hi Chris,
I'm curious to understand this a little better.
If only benefit of this patch is improving overall system performance, then
can you say why i915 interrupt handling doesn't fit into what I thought was
the common usage of threaded interrupts...
Usually with request_threaded_irq(), I thought typically both handlers
are specified and so you'd only fallback to the threaded handler when the
interrupt context handler is overwhelmed?
Like so:
while (HW events need some action) {
.... do something ...
if (too overwhelmed) /* ie. reduce load on system */
return IRQ_WAKE_THREAD;
}
return IRQ_HANDLED;
Likely you considered something like above.
I'm just looking to understand why using only threaded interrupt handler is best in this case.
(Also maybe discuss this a little bit further in commit message...?)
Sorry if this was perhaps discussed in response to Tvrtko's question.
I didn't quite follow the fast/slow separation mentioned in that thread.
Thanks,
-Brian
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Cc: Clark Williams <williams@xxxxxxxxxx>
> Cc: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
> Note this should need the fixes in
> 20190926105644.16703-2-bigeasy@xxxxxxxxxxxxx, by itself this should be a
> test vehicle to exercise that patch!
> ---
> drivers/gpu/drm/i915/i915_irq.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bc83f094065a..f3df7714a3f3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -4491,8 +4491,8 @@ int intel_irq_install(struct drm_i915_private *dev_priv)
>
> intel_irq_reset(dev_priv);
>
> - ret = request_irq(irq, intel_irq_handler(dev_priv),
> - IRQF_SHARED, DRIVER_NAME, dev_priv);
> + ret = request_threaded_irq(irq, NULL, intel_irq_handler(dev_priv),
> + IRQF_SHARED, DRIVER_NAME, dev_priv);
> if (ret < 0) {
> dev_priv->drm.irq_enabled = false;
> return ret;
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
