Re: [PATCH v2 3/7] drm/i915/execlists: Make submission tasklet hardirq safe

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 08/05/2018 11:24, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2018-05-08 11:10:41)
>> On 07/05/2018 14:57, Chris Wilson wrote:
>>> @@ -744,13 +748,25 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>>>        /* We must always keep the beast fed if we have work piled up */
>>>        GEM_BUG_ON(execlists->first && !port_isset(execlists->port));
>>>    
>>> -unlock:
>>> -     spin_unlock_irq(&engine->timeline.lock);
>>> -
>>> -     if (submit) {
>>> +     /* Re-evaluate the executing context setup after each preemptive kick */
>>> +     if (last)
>>>                execlists_user_begin(execlists, execlists->port);
>>
>> Last can be non-null and submit false, so this is not equivalent.
>>
>> By the looks of it makes no difference since it is OK to set the
>> execlists user active bit multiple times. Even though the helper is
>> called execlists_set_active_once. But the return value is not looked at.
>>
>> Still, why not keep doing this when submit is true?
> 
> It's a subtle difference, in that we want the context reevaluated every
> time we kick the queue as we may have changed state that we want to
> reload, and not just ELSP. Sometimes we need inheritance of more than
> just priority...

What do you mean by context re-evaluated?

ACTIVE_USER is set from first to last request, no? I don't understand 
what would change if you would set it multiple times while it is already 
set.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx