On 08/05/2018 12:05, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-05-08 11:56:37)
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.
It's not about ACTIVE_USER. This is a hook to indicate a change in
context state being executed on the GPU. It's to be hooked into by the
cpufreq/pstate code, gpufreq code, etc. Later realisation is that they
need to be notified for all context changes here.
Ok so nothing as such relating to making tasklets hardirq safe.
Is the execlists_user_begin still the right name then?
Any future use for execlists_set_active_once or we could drop it
(replacing with execlists_set_active)?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
