Quoting Summers, Stuart (2019-10-08 15:52:15)
> On Tue, 2019-10-08 at 11:56 +0100, Chris Wilson wrote:
> > A common bane of ours is arbitrary delays in ksoftirqd processing our
> > submission tasklet. Give the submission tasklet a kick before we wait
> > to
> > avoid those delays eating into a tight timeout.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine.h | 3 +-
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 33 +++++++++++++----
> > ----
> > drivers/gpu/drm/i915/gt/intel_gt_requests.c | 12 ++++++++
> > 3 files changed, 34 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine.h
> > b/drivers/gpu/drm/i915/gt/intel_engine.h
> > index c9e8c8ccbd47..d624752f2a92 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine.h
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine.h
> > @@ -407,8 +407,9 @@ static inline void __intel_engine_reset(struct
> > intel_engine_cs *engine,
> > engine->serial++; /* contexts lost */
> > }
> >
> > -bool intel_engine_is_idle(struct intel_engine_cs *engine);
> > bool intel_engines_are_idle(struct intel_gt *gt);
> > +bool intel_engine_is_idle(struct intel_engine_cs *engine);
> > +void intel_engine_flush_submission(struct intel_engine_cs *engine);
> >
> > void intel_engines_reset_default_submission(struct intel_gt *gt);
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 6220b7151bb9..7e2aa7a6bef0 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -1040,6 +1040,25 @@ static bool ring_is_idle(struct
> > intel_engine_cs *engine)
> > return idle;
> > }
> >
> > +void intel_engine_flush_submission(struct intel_engine_cs *engine)
> > +{
> > + struct tasklet_struct *t = &engine->execlists.tasklet;
> > +
> > + if (__tasklet_is_scheduled(t)) {
> > + local_bh_disable();
> > + if (tasklet_trylock(t)) {
> > + /* Must wait for any GPU reset in progress. */
> > + if (__tasklet_is_enabled(t))
> > + t->func(t->data);
> > + tasklet_unlock(t);
> > + }
> > + local_bh_enable();
> > + }
> > +
> > + /* Otherwise flush the tasklet if it was running on another cpu
> > */
> > + tasklet_unlock_wait(t);
> > +}
> > +
> > /**
> > * intel_engine_is_idle() - Report if the engine has finished
> > process all work
> > * @engine: the intel_engine_cs
> > @@ -1058,21 +1077,9 @@ bool intel_engine_is_idle(struct
> > intel_engine_cs *engine)
> >
> > /* Waiting to drain ELSP? */
> > if (execlists_active(&engine->execlists)) {
> > - struct tasklet_struct *t = &engine->execlists.tasklet;
> > -
> > synchronize_hardirq(engine->i915->drm.pdev->irq);
> >
> > - local_bh_disable();
> > - if (tasklet_trylock(t)) {
> > - /* Must wait for any GPU reset in progress. */
> > - if (__tasklet_is_enabled(t))
> > - t->func(t->data);
> > - tasklet_unlock(t);
> > - }
> > - local_bh_enable();
> > -
> > - /* Otherwise flush the tasklet if it was on another cpu
> > */
> > - tasklet_unlock_wait(t);
> > + intel_engine_flush_submission(engine);
> >
> > if (execlists_active(&engine->execlists))
> > return false;
> > diff --git a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> > b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> > index ca606b79fd5e..cbb4069b11e1 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_gt_requests.c
> > @@ -4,6 +4,7 @@
> > * Copyright © 2019 Intel Corporation
> > */
> >
> > +#include "i915_drv.h" /* for_each_engine() */
> > #include "i915_request.h"
> > #include "intel_gt.h"
> > #include "intel_gt_pm.h"
> > @@ -19,6 +20,15 @@ static void retire_requests(struct intel_timeline
> > *tl)
> > break;
> > }
> >
> > +static void flush_submission(struct intel_gt *gt)
> > +{
> > + struct intel_engine_cs *engine;
> > + enum intel_engine_id id;
> > +
> > + for_each_engine(engine, gt->i915, id)
> > + intel_engine_flush_submission(engine);
> > +}
> > +
> > long intel_gt_retire_requests_timeout(struct intel_gt *gt, long
> > timeout)
> > {
> > struct intel_gt_timelines *timelines = >->timelines;
> > @@ -32,6 +42,8 @@ long intel_gt_retire_requests_timeout(struct
> > intel_gt *gt, long timeout)
> > if (unlikely(timeout < 0))
> > timeout = -timeout, interruptible = false;
> >
> > + flush_submission(gt); /* kick the ksoftirqd tasklets */
>
> Won't this add a performance hit if we are doing this across all
> engines? Is there a way we can isolate this a bit more?
One of my earlier thoughts was to use request->engine just before the
wait, but that depends on actually hitting the wait and so is a bit
weaker than always flushing.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
