[Intel-gfx] [PATCH 2/3] drm/i915: Do not short-circuit tasklets during reset

Michel Thierry michel.thierry at intel.com
Fri Jul 13 20:44:00 UTC 2018


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 at chris-wilson.co.uk>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala at 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
> 


More information about the Intel-gfx mailing list