Re: [PATCH v2] drm/i915/gen11: Preempt-to-idle support in execlists.

[Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>]

<snip>

>>   
>> @@ -1010,7 +1033,15 @@ static void execlists_submission_tasklet(unsigned long data)
>>                                        GEN8_CTX_STATUS_PREEMPTED))
>>                                  execlists_set_active(execlists,
>>                                                       EXECLISTS_ACTIVE_HWACK);
>> -                       if (status & GEN8_CTX_STATUS_ACTIVE_IDLE)
>> +
>> +                       /*
>> +                        * Check if switched to idle or preempted to idle.
>> +                        * The STATUS_IDLE_ACTIVE flag is really used to mark
>> +                        * preemtion from idle to idle, this is not a mistake.
>> +                        */
>> +                       if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) ||
>> +                           ((status & GEN8_CTX_STATUS_IDLE_ACTIVE) &&
>> +                            (status & GEN11_CTX_STATUS_PREEMPT_IDLE)))
>>                                  execlists_clear_active(execlists,
>>                                                         EXECLISTS_ACTIVE_HWACK);
> 
> But still pointless, no?
> 

Just to understand, is it pointless because we have a preemption in 
flight and we're thus going to call execlists_dequeue below, which will 
eventually clear the flag in execlists_submit_ports? Or do we just don't 
care if this gets cleared here because we always clear it before a write 
to the elsp and we're only interested in it being clear between the 
write and the subsequent csb event?

Also, now that I think about it, with the current flow it doesn't look 
like we would clear EXECLISTS_ACTIVE_PREEMPT if a preempt-to-idle 
happens on idle HW, so we still need a condition for that even if we 
drop the one for EXECLISTS_ACTIVE_HWACK.

Daniele

>> @@ -1020,8 +1051,13 @@ static void execlists_submission_tasklet(unsigned long data)
>>                          /* We should never get a COMPLETED | IDLE_ACTIVE! */
>>                          GEM_BUG_ON(status & GEN8_CTX_STATUS_IDLE_ACTIVE);
>>   
>> -                       if (status & GEN8_CTX_STATUS_COMPLETE &&
>> -                           buf[2*head + 1] == execlists->preempt_complete_status) {
>> +                       /*
>> +                        * Check if preempted to real idle, either directly or
>> +                        * the preemptive context already finished executing
>> +                        */
>> +                       if ((status & GEN11_CTX_STATUS_PREEMPT_IDLE) ||
>> +                           (status & GEN8_CTX_STATUS_COMPLETE &&
>> +                           buf[2*head + 1] == execlists->preempt_complete_status)) {
>>                                  GEM_TRACE("%s preempt-idle\n", engine->name);
>>   
>>                                  execlists_cancel_port_requests(execlists);
>> @@ -2157,7 +2193,8 @@ static void execlists_set_default_submission(struct intel_engine_cs *engine)
>>          engine->unpark = NULL;
>>   
>>          engine->flags |= I915_ENGINE_SUPPORTS_STATS;
>> -       if (engine->i915->preempt_context)
>> +       if (engine->i915->preempt_context ||
>> +           HAS_HW_PREEMPT_TO_IDLE(engine->i915))
>>                  engine->flags |= I915_ENGINE_HAS_PREEMPTION;
> 
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx