diff mbox series

[3/3] drm/i915/icl: Add gen11 specific render breadcrumbs

Message ID 20190815083055.14132-3-mika.kuoppala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] drm/i915/icl: Implement gen11 flush including tile cache | expand

Commit Message

Mika Kuoppala Aug. 15, 2019, 8:30 a.m. UTC 
Flush according to what gen11 expects when writing
breadcrumbs. As only the seqnowrite + flush differs
between engine and gens, enclose the footer to
helper.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 88 ++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 26 deletions(-)

Comments

Chris Wilson Aug. 15, 2019, 9:08 a.m. UTC | #1 
Quoting Mika Kuoppala (2019-08-15 09:30:55)
> Flush according to what gen11 expects when writing
> breadcrumbs. As only the seqnowrite + flush differs
> between engine and gens, enclose the footer to
> helper.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 88 ++++++++++++++++++++---------
>  1 file changed, 62 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 9018afb4e9ef..de84febdec43 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2740,12 +2740,10 @@ static u32 *emit_preempt_busywait(struct i915_request *request, u32 *cs)
>         return cs;
>  }
>  
> -static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
> +static __always_inline u32*
> +gen8_emit_fini_breadcrumb_footer(struct i915_request *request,
> +                                u32 *cs)
>  {
> -       cs = gen8_emit_ggtt_write(cs,
> -                                 request->fence.seqno,
> -                                 request->timeline->hwsp_offset,
> -                                 0);
>         *cs++ = MI_USER_INTERRUPT;
>  
>         *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
> @@ -2758,29 +2756,55 @@ static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
>         return gen8_emit_wa_tail(request, cs);
>  }
>  
> +static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
> +{
> +       cs = gen8_emit_ggtt_write(cs,
> +                                 request->fence.seqno,
> +                                 request->timeline->hwsp_offset,
> +                                 0);
> +
> +       return gen8_emit_fini_breadcrumb_footer(request, cs);
> +}
> +
>  static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
>  {
> +       const u32 pipectl_first =
> +               PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +               PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +               PIPE_CONTROL_DC_FLUSH_ENABLE;

We weren't overflowing, it didn't look too bad to have the constants
inline.

> +
>         /* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
> +       const u32 pipectl_second =
> +               PIPE_CONTROL_FLUSH_ENABLE |
> +               PIPE_CONTROL_CS_STALL;
> +
>         cs = gen8_emit_ggtt_write_rcs(cs,
>                                       request->fence.seqno,
>                                       request->timeline->hwsp_offset,
> -                                     PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> -                                     PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> -                                     PIPE_CONTROL_DC_FLUSH_ENABLE);
> -       cs = gen8_emit_pipe_control(cs,
> -                                   PIPE_CONTROL_FLUSH_ENABLE |
> -                                   PIPE_CONTROL_CS_STALL,
> -                                   0);
> -       *cs++ = MI_USER_INTERRUPT;
> +                                     pipectl_first);
>  
> -       *cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
> -       if (intel_engine_has_semaphores(request->engine))
> -               cs = emit_preempt_busywait(request, cs);
> +       cs = gen8_emit_pipe_control(cs, pipectl_second, 0);
>  
> -       request->tail = intel_ring_offset(request, cs);
> -       assert_ring_tail_valid(request->ring, request->tail);
> +       return gen8_emit_fini_breadcrumb_footer(request, cs);
> +}
>  
> -       return gen8_emit_wa_tail(request, cs);
> +static u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *request,
> +                                          u32 *cs)
> +{
> +       const u32 pipectl =
> +               PIPE_CONTROL_CS_STALL |
> +               PIPE_CONTROL_TILE_CACHE_FLUSH |
> +               PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
> +               PIPE_CONTROL_DEPTH_CACHE_FLUSH |
> +               PIPE_CONTROL_DC_FLUSH_ENABLE |
> +               PIPE_CONTROL_FLUSH_ENABLE;
> +
> +       cs = gen8_emit_ggtt_write_rcs(cs,
> +                                     request->fence.seqno,
> +                                     request->timeline->hwsp_offset,
> +                                     pipectl);
> +
> +       return gen8_emit_fini_breadcrumb_footer(request, cs);
>  }
>  
>  static void execlists_park(struct intel_engine_cs *engine)
> @@ -2876,6 +2900,23 @@ logical_ring_default_irqs(struct intel_engine_cs *engine)
>         engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
>  }
>  
> +static void rcs_submission_override(struct intel_engine_cs *engine)
> +{
> +       switch (INTEL_GEN(engine->i915)) {
> +

Bonus \n!

With the choice of placement for the constants justified one way or the
other (I ask not for pipectl_the_second :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 9018afb4e9ef..de84febdec43 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2740,12 +2740,10 @@  static u32 *emit_preempt_busywait(struct i915_request *request, u32 *cs)
 	return cs;
 }
 
-static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
+static __always_inline u32*
+gen8_emit_fini_breadcrumb_footer(struct i915_request *request,
+				 u32 *cs)
 {
-	cs = gen8_emit_ggtt_write(cs,
-				  request->fence.seqno,
-				  request->timeline->hwsp_offset,
-				  0);
 	*cs++ = MI_USER_INTERRUPT;
 
 	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
@@ -2758,29 +2756,55 @@  static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
 	return gen8_emit_wa_tail(request, cs);
 }
 
+static u32 *gen8_emit_fini_breadcrumb(struct i915_request *request, u32 *cs)
+{
+	cs = gen8_emit_ggtt_write(cs,
+				  request->fence.seqno,
+				  request->timeline->hwsp_offset,
+				  0);
+
+	return gen8_emit_fini_breadcrumb_footer(request, cs);
+}
+
 static u32 *gen8_emit_fini_breadcrumb_rcs(struct i915_request *request, u32 *cs)
 {
+	const u32 pipectl_first =
+		PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
+		PIPE_CONTROL_DEPTH_CACHE_FLUSH |
+		PIPE_CONTROL_DC_FLUSH_ENABLE;
+
 	/* XXX flush+write+CS_STALL all in one upsets gem_concurrent_blt:kbl */
+	const u32 pipectl_second =
+		PIPE_CONTROL_FLUSH_ENABLE |
+		PIPE_CONTROL_CS_STALL;
+
 	cs = gen8_emit_ggtt_write_rcs(cs,
 				      request->fence.seqno,
 				      request->timeline->hwsp_offset,
-				      PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
-				      PIPE_CONTROL_DEPTH_CACHE_FLUSH |
-				      PIPE_CONTROL_DC_FLUSH_ENABLE);
-	cs = gen8_emit_pipe_control(cs,
-				    PIPE_CONTROL_FLUSH_ENABLE |
-				    PIPE_CONTROL_CS_STALL,
-				    0);
-	*cs++ = MI_USER_INTERRUPT;
+				      pipectl_first);
 
-	*cs++ = MI_ARB_ON_OFF | MI_ARB_ENABLE;
-	if (intel_engine_has_semaphores(request->engine))
-		cs = emit_preempt_busywait(request, cs);
+	cs = gen8_emit_pipe_control(cs, pipectl_second, 0);
 
-	request->tail = intel_ring_offset(request, cs);
-	assert_ring_tail_valid(request->ring, request->tail);
+	return gen8_emit_fini_breadcrumb_footer(request, cs);
+}
 
-	return gen8_emit_wa_tail(request, cs);
+static u32 *gen11_emit_fini_breadcrumb_rcs(struct i915_request *request,
+					   u32 *cs)
+{
+	const u32 pipectl =
+		PIPE_CONTROL_CS_STALL |
+		PIPE_CONTROL_TILE_CACHE_FLUSH |
+		PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH |
+		PIPE_CONTROL_DEPTH_CACHE_FLUSH |
+		PIPE_CONTROL_DC_FLUSH_ENABLE |
+		PIPE_CONTROL_FLUSH_ENABLE;
+
+	cs = gen8_emit_ggtt_write_rcs(cs,
+				      request->fence.seqno,
+				      request->timeline->hwsp_offset,
+				      pipectl);
+
+	return gen8_emit_fini_breadcrumb_footer(request, cs);
 }
 
 static void execlists_park(struct intel_engine_cs *engine)
@@ -2876,6 +2900,23 @@  logical_ring_default_irqs(struct intel_engine_cs *engine)
 	engine->irq_keep_mask = GT_CONTEXT_SWITCH_INTERRUPT << shift;
 }
 
+static void rcs_submission_override(struct intel_engine_cs *engine)
+{
+	switch (INTEL_GEN(engine->i915)) {
+
+	case 12:
+	case 11:
+		engine->emit_flush = gen11_emit_flush_render;
+		engine->emit_fini_breadcrumb = gen11_emit_fini_breadcrumb_rcs;
+		break;
+
+	default:
+		engine->emit_flush = gen8_emit_flush_render;
+		engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_rcs;
+		break;
+	}
+}
+
 int intel_execlists_submission_setup(struct intel_engine_cs *engine)
 {
 	tasklet_init(&engine->execlists.tasklet,
@@ -2885,13 +2926,8 @@  int intel_execlists_submission_setup(struct intel_engine_cs *engine)
 	logical_ring_default_vfuncs(engine);
 	logical_ring_default_irqs(engine);
 
-	if (engine->class == RENDER_CLASS) {
-		if (INTEL_GEN(engine->i915) >= 11)
-			engine->emit_flush = gen11_emit_flush_render;
-		else
-			engine->emit_flush = gen8_emit_flush_render;
-		engine->emit_fini_breadcrumb = gen8_emit_fini_breadcrumb_rcs;
-	}
+	if (engine->class == RENDER_CLASS)
+		rcs_submission_override(engine);
 
 	return 0;
 }