| Message ID | 20180713203529.1973-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Chris Wilson (2018-07-13 21:35:28) > Inside intel_engine_is_idle(), we flush the tasklet to ensure that is > being run in a timely fashion (ksoftirqd has taught us to expect the > worst). However, if we are in the middle of reset, the HW may not yet be > ready to execute the submission tasklet and so we must respect the > disable flag. > > Fixes: dd0cf235d81f ("drm/i915: Speed up idle detection by kicking the tasklets") > Testcase: igt/drv_selftest/live_hangcheck > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 220050107c48..fccb95ea1315 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -989,16 +989,17 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) > > /* Waiting to drain ELSP? */ > if (READ_ONCE(engine->execlists.active)) { > - struct intel_engine_execlists *execlists = &engine->execlists; > + struct tasklet_struct *t = &engine->execlists.tasklet; > > local_bh_disable(); > - if (tasklet_trylock(&execlists->tasklet)) { > - execlists->tasklet.func(execlists->tasklet.data); > - tasklet_unlock(&execlists->tasklet); > + if (tasklet_trylock(t)) { > + if (__tasklet_is_enabled(t)) I should leave a clue here. I don't think calling it reset_in_progress() ties in well with the pure tasklet nature here, so /* must wait if a GPU reset is in progress */ -Chris
On 7/13/2018 1:35 PM, Chris Wilson wrote: > Inside intel_engine_is_idle(), we flush the tasklet to ensure that is > being run in a timely fashion (ksoftirqd has taught us to expect the > worst). However, if we are in the middle of reset, the HW may not yet be > ready to execute the submission tasklet and so we must respect the > disable flag. > > Fixes: dd0cf235d81f ("drm/i915: Speed up idle detection by kicking the tasklets") > Testcase: igt/drv_selftest/live_hangcheck > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c > index 220050107c48..fccb95ea1315 100644 > --- a/drivers/gpu/drm/i915/intel_engine_cs.c > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c > @@ -989,16 +989,17 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) > > /* Waiting to drain ELSP? */ > if (READ_ONCE(engine->execlists.active)) { > - struct intel_engine_execlists *execlists = &engine->execlists; > + struct tasklet_struct *t = &engine->execlists.tasklet; > > local_bh_disable(); > - if (tasklet_trylock(&execlists->tasklet)) { > - execlists->tasklet.func(execlists->tasklet.data); > - tasklet_unlock(&execlists->tasklet); > + if (tasklet_trylock(t)) { > + if (__tasklet_is_enabled(t)) I would add a comment that this catches any reset in progress as it isn't as clear as using reset_in_progress (although you explain why in the commit message). Up-to you. Reviewed-by: Michel Thierry <michel.thierry@intel.com> > + t->func(t->data); > + tasklet_unlock(t); > } > local_bh_enable(); > > - if (READ_ONCE(execlists->active)) > + if (READ_ONCE(engine->execlists.active)) > return false; > } > >
On 7/13/2018 1:41 PM, Chris Wilson wrote: > Quoting Chris Wilson (2018-07-13 21:35:28) >> Inside intel_engine_is_idle(), we flush the tasklet to ensure that is >> being run in a timely fashion (ksoftirqd has taught us to expect the >> worst). However, if we are in the middle of reset, the HW may not yet be >> ready to execute the submission tasklet and so we must respect the >> disable flag. >> >> Fixes: dd0cf235d81f ("drm/i915: Speed up idle detection by kicking the tasklets") >> Testcase: igt/drv_selftest/live_hangcheck >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> >> --- >> drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c >> index 220050107c48..fccb95ea1315 100644 >> --- a/drivers/gpu/drm/i915/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c >> @@ -989,16 +989,17 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) >> >> /* Waiting to drain ELSP? */ >> if (READ_ONCE(engine->execlists.active)) { >> - struct intel_engine_execlists *execlists = &engine->execlists; >> + struct tasklet_struct *t = &engine->execlists.tasklet; >> >> local_bh_disable(); >> - if (tasklet_trylock(&execlists->tasklet)) { >> - execlists->tasklet.func(execlists->tasklet.data); >> - tasklet_unlock(&execlists->tasklet); >> + if (tasklet_trylock(t)) { >> + if (__tasklet_is_enabled(t)) > > I should leave a clue here. I don't think calling it reset_in_progress() > ties in well with the pure tasklet nature here, so > > /* must wait if a GPU reset is in progress */ Yes, reset_in_progress doesn't match, the comment is perfectly clear. Thanks > -Chris >
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index 220050107c48..fccb95ea1315 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -989,16 +989,17 @@ bool intel_engine_is_idle(struct intel_engine_cs *engine) /* Waiting to drain ELSP? */ if (READ_ONCE(engine->execlists.active)) { - struct intel_engine_execlists *execlists = &engine->execlists; + struct tasklet_struct *t = &engine->execlists.tasklet; local_bh_disable(); - if (tasklet_trylock(&execlists->tasklet)) { - execlists->tasklet.func(execlists->tasklet.data); - tasklet_unlock(&execlists->tasklet); + if (tasklet_trylock(t)) { + if (__tasklet_is_enabled(t)) + t->func(t->data); + tasklet_unlock(t); } local_bh_enable(); - if (READ_ONCE(execlists->active)) + if (READ_ONCE(engine->execlists.active)) return false; }
Inside intel_engine_is_idle(), we flush the tasklet to ensure that is being run in a timely fashion (ksoftirqd has taught us to expect the worst). However, if we are in the middle of reset, the HW may not yet be ready to execute the submission tasklet and so we must respect the disable flag. Fixes: dd0cf235d81f ("drm/i915: Speed up idle detection by kicking the tasklets") Testcase: igt/drv_selftest/live_hangcheck Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/intel_engine_cs.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)