On 2018-10-23 11:13, Joonas Lahtinen wrote:
Quoting Lis, Tomasz (2018-10-19 19:00:15)
On 2018-10-16 12:53, Joonas Lahtinen wrote:
Quoting Tomasz Lis (2018-10-15 20:29:18)
The patch adds support of preempt-to-idle requesting by setting a proper
bit within Execlist Control Register, and receiving preemption result from
Context Status Buffer.
Preemption in previous gens required a special batch buffer to be executed,
so the Command Streamer never preempted to idle directly. In Icelake it is
possible, as there is a hardware mechanism to inform the kernel about
status of the preemption request.
This patch does not cover using the new preemption mechanism when GuC is
active.
v2: Added needs_preempt_context() change so that it is not created when
preempt-to-idle is supported. (Chris)
Updated setting HWACK flag so that it is cleared after
preempt-to-dle. (Chris, Daniele)
Updated to use I915_ENGINE_HAS_PREEMPTION flag. (Chris)
v3: Fixed needs_preempt_context() change. (Chris)
Merged preemption trigger functions to one. (Chris)
Fixed conyext state tonot assume COMPLETED_MASK after preemption,
since idle-to-idle case will not have it set.
v4: Simplified needs_preempt_context() change. (Daniele)
Removed clearing HWACK flag in idle-to-idle preempt. (Daniele)
v5: Renamed inject_preempt_context(). (Daniele)
Removed duplicated GEM_BUG_ON() on HWACK (Daniele)
Bspec: 18922
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
This R-b was on v4, and should be indicated with # v4 comment.
The commit message doesn't say much about why preempting to idle is
beneficial? The pre-Gen11 codepath needs to be maintained anyway.
Regards, Joonas
The benefit is one less context switch - there is no "preempt context".
Yes.
But that still doesn't quite explain what material benefits there are? :)
Is there some actual workloads/microbenchmarks that get an improvement?
This alters the behavior between different platforms for a very delicate
feature, probably resulting in slightly different bugs. So there should
be some more reasoning than just because we can.
Regards, Joonas
Less context switching does imply perf improvement, though it would
require measurement - it might be hardly detectable. We may even lose
performance - without measurements, we don't know. So not a strong argument.
One more benefit I could think of is - GuC path will use
preempt-to-idle, so this would make execlists use the same path as GuC.
But that's not a strong argument as well.
I must agree - there doesn't seem to be any strong enough reason to go
with this change.
We might consider it after we have performance data.
-Tomasz
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
