[Mesa-dev] [v5 05/11] i965: Deferred allocation of mcs for lossless compressed

[Ben Widawsky]

On Mon, Apr 25, 2016 at 08:10:02PM +0300, Topi Pohjolainen wrote:
> Until not mcs was associated to single sampled buffers only for
         ^ now

> fast clear purposes and it was therefore the responsibility of the
> clear logic to allocate the aux buffer when needed. Now that normal
> 3D render or blorp blit may render with mcs enabled also, they need
> to prepare the mcs just as well.
> 
> v2: Do not enable for scanout buffers
> 
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp  |  2 ++
>  src/mesa/drivers/dri/i965/brw_draw.c          | 17 ++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 34 +++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  4 ++++
>  4 files changed, 57 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> index 7556d6a..30206f2 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> @@ -82,6 +82,8 @@ brw_blorp_blit_miptrees(struct brw_context *brw,
>     intel_miptree_slice_resolve_depth(brw, src_mt, src_level, src_layer);
>     intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level, dst_layer);
>  
> +   intel_miptree_prepare_mcs(brw, dst_mt);
> +
>     DBG("%s from %dx %s mt %p %d %d (%f,%f) (%f,%f)"
>         "to %dx %s mt %p %d %d (%f,%f) (%f,%f) (flip %d,%d)\n",
>         __func__,
> diff --git a/src/mesa/drivers/dri/i965/brw_draw.c b/src/mesa/drivers/dri/i965/brw_draw.c
> index afa8a4e..4fa0f4e 100644
> --- a/src/mesa/drivers/dri/i965/brw_draw.c
> +++ b/src/mesa/drivers/dri/i965/brw_draw.c
> @@ -371,6 +371,22 @@ brw_postdraw_set_buffers_need_resolve(struct brw_context *brw)
>     }
>  }
>  
> +static void
> +brw_predraw_set_aux_buffers(struct brw_context *brw)
> +{
> +   struct gl_context *ctx = &brw->ctx;
> +   struct gl_framebuffer *fb = ctx->DrawBuffer;
> +
> +   for (unsigned i = 0; i < fb->_NumColorDrawBuffers; i++) {
> +      struct intel_renderbuffer *irb =
> +         intel_renderbuffer(fb->_ColorDrawBuffers[i]);
> +
> +      if (irb) {
> +         intel_miptree_prepare_mcs(brw, irb->mt);
> +      }
> +   }
> +}
> +

I wonder if it makes sense to put the gen check in this function to avoid
unnecessary pointer derefs.

>  /* May fail if out of video memory for texture or vbo upload, or on
>   * fallback conditions.
>   */
> @@ -416,6 +432,7 @@ brw_try_draw_prims(struct gl_context *ctx,
>        _mesa_fls(ctx->VertexProgram._Current->Base.SamplersUsed);
>  
>     intel_prepare_render(brw);
> +   brw_predraw_set_aux_buffers(brw);
>  
>     /* This workaround has to happen outside of brw_upload_render_state()
>      * because it may flush the batchbuffer for a blit, affecting the state
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index cdf2fbd..758b9dd 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -1624,6 +1624,40 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
>     return mt->mcs_mt;
>  }
>  
> +void
> +intel_miptree_prepare_mcs(struct brw_context *brw,
> +                          struct intel_mipmap_tree *mt)
> +{
> +   if (mt->mcs_mt)
> +      return;
> +

I'm in favor of easily readable early gen checks, but up to you:
if (brw->gen < 9)
   return;

> +   /* Consider if lossless compression is supported but the needed
> +    * auxiliary buffer doesn't exist yet.
> +    */
> +   if (brw->gen >= 9 &&
> +       intel_tiling_supports_non_msrt_mcs(brw, mt->tiling) &&
> +       intel_miptree_supports_non_msrt_fast_clear(brw, mt) &&
> +       intel_miptree_supports_lossless_compressed(mt->format)) {
> +
> +      /* Clients are not currently capable of consuming compressed
> +       * single-sampled buffers.
> +       */
> +      if (mt->is_scanout)
> +         return;
> +
> +      /* Failing to allocate the auxiliary buffer means running out of
> +       * memory. The pointer to the aux miptree is left NULL which should
> +       * signal non-compressed behavior.
                   ^ uncompressed
> +       */
> +      if (!intel_miptree_alloc_non_msrt_mcs(brw, mt)) {
> +         _mesa_warning(NULL,
> +                       "Failed to allocated aux buffer for lossless"
> +                       " compressed %p %u:%u %s\n",
> +                       mt, mt->logical_width0, mt->logical_height0,
> +                       _mesa_get_format_name(mt->format));
> +      }
> +   }
> +}
>  
>  /**
>   * Helper for intel_miptree_alloc_hiz() that sets
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index bb06522..e0d543b 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -693,6 +693,10 @@ bool
>  intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
>                                   struct intel_mipmap_tree *mt);
>  
> +void
> +intel_miptree_prepare_mcs(struct brw_context *brw,
> +                          struct intel_mipmap_tree *mt);
> +
>  enum {
>     MIPTREE_LAYOUT_ACCELERATED_UPLOAD       = 1 << 0,
>     MIPTREE_LAYOUT_FORCE_ALL_SLICE_AT_LOD   = 1 << 1,

lgtm:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>