[Intel-gfx] [PATCH 1/6] drm/i915: Remove redundant trailing request flush

[Tvrtko Ursulin]

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 at intel.com>

Regards,

Tvrtko