[Intel-gfx] [PATCH v4] drm/i915: Execlists small cleanups and micro-optimisations

[Tvrtko Ursulin]

On 29/02/16 11:48, Chris Wilson wrote:
> On Mon, Feb 29, 2016 at 11:40:37AM +0000, Tvrtko Ursulin wrote:
>>
>>
>> On 29/02/16 11:13, Chris Wilson wrote:
>>> On Mon, Feb 29, 2016 at 11:01:49AM +0000, Tvrtko Ursulin wrote:
>>>>
>>>> On 29/02/16 10:53, Chris Wilson wrote:
>>>>> On Mon, Feb 29, 2016 at 10:45:34AM +0000, Tvrtko Ursulin wrote:
>>>>>> This ok?
>>>>>>
>>>>>> """
>>>>>> One unexplained result is with "gem_latency -n 0" (dispatching
>>>>>> empty batches) which shows 5% more throughput, 8% less CPU time,
>>>>>> 25% better producer and consumer latencies, but 15% higher
>>>>>> dispatch latency which looks like a possible measuring artifact.
>>>>>> """
>>>>>
>>>>> I doubt it is a measuring artefact since throughput = 1/(dispatch +
>>>>> latency + test overhead), and the dispatch latency here is larger than
>>>>> the wakeup latency and so has greater impact on throughput in this
>>>>> scenario.
>>>>
>>>> I don't follow you, if dispatch latency has larger effect on
>>>> throughput how to explain the increase and still better throughput?
>>>>
>>>> I see in gem_latency this block:
>>>>
>>>> 	measure_latency(p, &p->latency);
>>>> 	igt_stats_push(&p->dispatch, *p->last_timestamp - start);
>>>>
>>>> measure_latency waits for the batch to complete and then dispatch
>>>> latency uses p->last_timestamp which is something written by the GPU
>>>> and not a CPU view of the latency ?
>>>
>>> Exactly, measurements are entirely made from the running engine clock
>>> (which is ~80ns clock, and should be verified during init). The register
>>> is read before dispatch, inside the batch and then at wakeup, but the
>>> information is presented as dispatch = batch - start and
>>> wakeup = end - batch, so to get the duration (end - start) we need
>>> to add the two together. Throughput will also include some overhead from
>>> the test iteration (that will mainly be scheduler interference).
>>>
>>> My comment about dispatch having greater effect, is in terms of
>>> its higher absolute value (so the relative % means a larger change wrt
>>> throughput).
>>
>> Change to this then?
>>
>> """
>>      One unexplained result is with "gem_latency -n 0" (dispatching
>>      empty batches) which shows 5% more throughput, 8% less CPU time,
>>      25% better producer and consumer latencies, but 15% higher
>>      dispatch latency which looks like an amplified effect of test
>>      overhead.
>> """
>
> No. Dispatch latency is important and this attempts to pass the change
> off a test effect when to the best of my knowledge it is a valid external
> observation of the system.

I just don't understand how can it be valid when we have executed more 
empty batches than before in a unit of time.

Because even dispatch + wake up latency is worse, but throughput is 
still better.

Sounds impossible to me so it must be the effect of using two different 
time sources. CPU side to measure throughput and GPU side to measure 
dispatch latency.I don't know, could you suggest a paragraph to add to 
the commit message so we can close on this?

Regards,

Tvrtko