Re: [PATCH 5/8] drm/i915: Remove obsolete ringbuffer emission for gen8+

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

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

> Since removing the module parameter to force selection of ringbuffer
> emission for gen8, the code is defunct. Remove it.
>
> To put the difference into perspective, a couple of microbenchmarks
> (bdw i7-5557u, 20170324):
>                                         ring          execlists
> exec continuous nops on all rings:   1.491us            2.223us
> exec sequential nops on each ring:  12.508us           53.682us
> single nop + sync:                   9.272us           30.291us
>
> vblank_mode=0 glxgears:            ~11000fps           ~9000fps
>
> Since the earlier submission, gen8 ringbuffer submission has fallen
> further and further behind in features. So while ringbuffer may hold the
> throughput crown, in terms of interactive latency, execlists is much
> better. Alas, we have no convenient metrics for such, other than
> demonstrating things we can do with execlists but can not using
> legacy ringbuffer submission.
>
> We have made a few improvements to lowlevel execlists throughput,
> and ringbuffer currently panics on boot! (bdw i7-5557u, 20171026):
>
>                                         ring          execlists
> exec continuous nops on all rings:       n/a            1.921us
> exec sequential nops on each ring:       n/a           44.621us
> single nop + sync:                       n/a           21.953us
>
> vblank_mode=0 glxgears:                  n/a          ~18500fps
>
> References: https://bugs.freedesktop.org/show_bug.cgi?id=87725
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Once-upon-a-time-Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

lgtm but found one thing more we can throw out:

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 45835d83dfbf..873439abc415 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -37,8 +37,6 @@
  * Resource Streamer, is 66944 bytes, which rounds to 17 pages.
  */
 #define HSW_CXT_TOTAL_SIZE             (17 * PAGE_SIZE)
-/* Same as Haswell, but 72064 bytes now. */
-#define GEN8_CXT_TOTAL_SIZE            (18 * PAGE_SIZE)
 
 #define GEN8_LR_CONTEXT_RENDER_SIZE    (20 * PAGE_SIZE)
 #define GEN9_LR_CONTEXT_RENDER_SIZE    (22 * PAGE_SIZE)
@@ -164,9 +162,7 @@ __intel_engine_context_size(struct drm_i915_private *dev_priv, u8 class)
                case 9:
                        return GEN9_LR_CONTEXT_RENDER_SIZE;
                case 8:
-                       return i915_modparams.enable_execlists ?
-                              GEN8_LR_CONTEXT_RENDER_SIZE :
-                              GEN8_CXT_TOTAL_SIZE;
+                       return GEN8_LR_CONTEXT_RENDER_SIZE;
                case 7:

With that included,

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx