Quoting Sebastian Andrzej Siewior (2019-09-26 16:13:08) > On 2019-09-26 15:25:38 [+0100], 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. > > cycletest is the name of the tool I don't know about, right? It is not > cyclictest with a typo? It's a tool like cyclictest that measures the timer latency in one thread while flooding the system with GPU work, filling the page cache, grabbing as many THP as possible etc. (It's actually called gem_syslatency.) > I don't know how many interrupts you have in a system but if this is the > only one for the card then you could remove _irqsave() from locking if > the lock is never observed in IRQ context (but then you have irqwork > which requires this irqsave). True, pushing the irq into a thread should allow us to re-evaluate a lot of our spin_lock_irq. > > 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); > > I think you should add IRQF_ONESHOT. Otherwise a LEVEL interrupt will > keep interrupting the CPU and you never manage to switch to the thread. The interrupts only keep coming if we feed the GPU, and we only feed the GPU if we service the interrupt. That should provide a natural quiescence :) -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
