[Intel-gfx] [PATCH 3/3] drm/i915/icl: Add gen11 specific render breadcrumbs

[Chris Wilson]

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 at chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at 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 at chris-wilson.co.uk>
-Chris