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

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

[Chris Wilson]

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;
 
-- 
2.14.2

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

✗ Fi.CI.BAT: failure for drm/i915/execlists: Explain why we skip an ELSP update if port[1] is active

[Patchwork]

== Series Details ==

Series: drm/i915/execlists: Explain why we skip an ELSP update if port[1] is active
URL   : https://patchwork.freedesktop.org/series/31162/
State : failure

== Summary ==

Series 31162 revision 1 was fully merged or fully failed: no git log

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

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

[Chris Wilson]

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?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Mika Kuoppala]

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