On 31/12/2018 10:32, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2018-12-31 10:25:41)
On 28/12/2018 17:16, Chris Wilson wrote:
@@ -616,9 +612,13 @@ i915_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx)
* i915_request_add() call can't fail. Note that the reserve may need
* to be redone if the request is not actually submitted straight
* away, e.g. because a GPU scheduler has deferred it.
+ *
+ * Note that due to how we add reserved_space to intel_ring_begin()
+ * we need to double our request to ensure that if we need to wrap
+ * around inside i915_request_add() there is sufficient space at
+ * the beginning of the ring as well.
Is there a benefit of keeping this intel_ring_begin behaviour? I mean,
could we just drop the special casing in there and always wrap the whole
space from the beginning if either part does not fit? That would allow
this part to pass in the true reserved space size I think.
I was tempted to rewrite intel_ring_begin() to do the doubling itself it
it chose to wrap now that we know we only emit one packet. But I chose
the path of least resistance. The effect is miniscule as we only reserve
the extra space, but don't actually emit any extra MI_NOOPs. The effect
is that we may wait for a few bytes more than we actually require -- and
if the ring is full enough that we wait for this request, in all
likelihood we would be waiting again for the next.
It's just awfully ugly that upper layer adds a hack to account for how
lower layer handles things when both are totally ours. But okay, lets
mark intel_ring_begin for a rewrite when things settle.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
