On 26/08/16 19:47, Chris Wilson wrote:
On Fri, Aug 26, 2016 at 07:25:57PM +0100, Dave Gordon wrote:
'relative_constants_mode' has always been tracked per-device, but this
has actually been wrong ever since hardware contexts were introduced, as
the INSTPM register is saved (and automatically restored) as part of the
render ring context. The software per-device value could therefore get
out of sync with the hardware per-context value.
Rather than track this correctly (per-context in LRC modes, per-device
in legacy ringbuffer mode), a simpler solution is just to give up trying
to track it at all, and always prefix each render batch with the single
instruction needed to explicitly set it to the required mode for the
current batch.
Agreed. However always emitting it wasteful for those who never touch
it, i.e. everybody. Rather than track the last value, just track if it
ever changed from default then reassign it before every batch?
But where to track whether it changed? If tracked globally, that will
mean everyone pays if any one process ever submits a batch with a
non-default value. If not global, then we still have the discrepancy
between LRC modes (h/w value saved per-context) vs ringbuffer (maybe
saved if device supports "hardware contexts", not saved otherwise).
Perhaps instead we should decide that between batches it shall always be
left at the default value, so any batch that wants any other value must
set it at the beginning and reset it when finished?
.Dave.
[PS: the reason I spotted this is because the tracking was broken with
pre-emption enabled: you can no longer assume that the mode at the start
of each batch is the same mode that the preceding batch used.]
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 99d50c0a710c..394c9cd3ddd3 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1578,7 +1578,7 @@ execbuf_submit(struct i915_execbuffer_params *params,
intel_ring_emit(ring, instp_mask << 16 | instp_mode);
intel_ring_advance(ring);
- dev_priv->relative_constants_mode = instp_mode;
+ dev_priv->relative_constants_mode = -1;
}
if (args->flags & I915_EXEC_GEN7_SOL_RESET) {
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
