Re: [PATCH] drm/i915/execlists: Explain why we skip an ELSP update if port[1] is active

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Chris Wilson (2017-09-29 11:45:57)
>> We coalesce onto an active port[0], but not onto an active port[1]
>> despite it being the same mechanism. So explain the complications that
>> make skipping the first active port and coalescing onto the second
>> active port intractable (at least while keeping some resemblance of
>> sanity).
>> 
>> Suggested-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_lrc.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index bc3fc4cd039e..cc5574ff56c8 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -579,6 +579,26 @@ static void execlists_dequeue(struct intel_engine_cs *engine)
>>                         execlists->preempt = true;
>>                         goto unlock;
>>                 } else {
>> +                       /*
>> +                        * In theory, we could coalesce more requests onto
>> +                        * the second port (the first port is active, with
>> +                        * no preemptions pending). However, that means we
>> +                        * then have to deal with the possible lite-restore
>> +                        * of the second port (as we submit the ELSP, there
>> +                        * may be a context-switch) but also we may complete
>> +                        * the resubmission before the context-switch. Ergo,
>> +                        * coalescing onto the second port will cause a
>> +                        * preemption event, but we cannot predict whether
>> +                        * that will affect port[0] or port[1].
>> +                        *
>> +                        * If the second port is already active, we can wait
>> +                        * until the next context-switch before contemplating
>> +                        * new requests. The GPU will be busy and we should be
>> +                        * able to resubmit the new ELSP before it idles,
>> +                        * avoiding pipeline bubbles (momentary pauses where
>> +                        * the driver is unable to keep up the supply of new
>> +                        * work).
>> +                        */
>>                         if (port_count(&port[1]))
>>                                 goto unlock;
>
> I think I'll squash this into the "Preemption!" patch if we're happy
> with the explanation?

Yup please do.
-Mika

> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx