[Mesa-dev] [v2 04/17] i965: Provide slice details to color resolver

Pohjolainen, Topi topi.pohjolainen at gmail.com
Wed Nov 23 18:13:15 UTC 2016


On Wed, Nov 23, 2016 at 09:26:29AM -0800, Jason Ekstrand wrote:
>    General comment: Does it make sense to squash this with the previous
>    patch?  I'm fine either way.
>    On Wed, Nov 23, 2016 at 1:16 AM, Topi Pohjolainen
>    <[1]topi.pohjolainen at gmail.com> wrote:
> 
>      Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
>      ---
>       src/mesa/drivers/dri/i965/brw_blorp.c         | 15 +++++++++------
>       src/mesa/drivers/dri/i965/brw_blorp.h         |  3 ++-
>       src/mesa/drivers/dri/i965/brw_context.c       | 14 +++++++++-----
>       src/mesa/drivers/dri/i965/intel_blit.c        |  4 ++--
>       src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 27
>      +++++++++++++++++++++++++--
>       src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  1 +
>       6 files changed, 48 insertions(+), 16 deletions(-)
>      diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
>      b/src/mesa/drivers/dri/i965/brw_blorp.c
>      index 9a849f5..99df21e 100644
>      --- a/src/mesa/drivers/dri/i965/brw_blorp.c
>      +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
>      @@ -213,7 +213,10 @@ blorp_surf_for_miptree(struct brw_context *brw,
>                if (safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E))
>                   flags |= INTEL_MIPTREE_IGNORE_CCS_E;
>      -         intel_miptree_resolve_color(brw, mt, flags);
>      +         for (unsigned i = 0; i < num_layers; i++) {
>      +            intel_miptree_resolve_color(brw, mt,
>      +                                        *level, start_layer + i,
>      flags);
>      +         }
>                assert(mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_
>      RESOLVED);
>                surf->aux_usage = ISL_AUX_USAGE_NONE;
>      @@ -942,19 +945,19 @@ brw_blorp_clear_color(struct brw_context *brw,
>      struct gl_framebuffer *fb,
>       }
>       void
>      -brw_blorp_resolve_color(struct brw_context *brw, struct
>      intel_mipmap_tree *mt)
>      +brw_blorp_resolve_color(struct brw_context *brw, struct
>      intel_mipmap_tree *mt,
>      +                        unsigned level, unsigned layer)
>       {
>      -   DBG("%s to mt %p\n", __FUNCTION__, mt);
>      +   DBG("%s to mt %p level %u layer %u\n", __FUNCTION__, mt, level,
>      layer);
>          const mesa_format format = _mesa_get_srgb_format_linear(
>      mt->format);
>          struct isl_surf isl_tmp[2];
>          struct blorp_surf surf;
>      -   unsigned level = 0;
>          blorp_surf_for_miptree(brw, &surf, mt, true,
>                                 (1 << ISL_AUX_USAGE_CCS_E) |
>                                 (1 << ISL_AUX_USAGE_CCS_D),
>      -                          &level, 0 /* start_layer */, 1 /*
>      num_layers */,
>      +                          &level, layer, 1 /* num_layers */,
>                                 isl_tmp);
>          enum blorp_fast_clear_op resolve_op;
>      @@ -971,7 +974,7 @@ brw_blorp_resolve_color(struct brw_context *brw,
>      struct intel_mipmap_tree *mt)
>          struct blorp_batch batch;
>          blorp_batch_init(&brw->blorp, &batch, brw, 0);
>      -   blorp_ccs_resolve(&batch, &surf, 0 /* level */, 0 /* layer */,
>      +   blorp_ccs_resolve(&batch, &surf, level, layer,
>                            brw_blorp_to_isl_format(brw, format, true),
>                            resolve_op);
>          blorp_batch_finish(&batch);
>      diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
>      b/src/mesa/drivers/dri/i965/brw_blorp.h
>      index abf3956..277b00e 100644
>      --- a/src/mesa/drivers/dri/i965/brw_blorp.h
>      +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
>      @@ -64,7 +64,8 @@ brw_blorp_clear_color(struct brw_context *brw,
>      struct gl_framebuffer *fb,
>       void
>       brw_blorp_resolve_color(struct brw_context *brw,
>      -                        struct intel_mipmap_tree *mt);
>      +                        struct intel_mipmap_tree *mt,
>      +                        unsigned level, unsigned layer);
> 
>    Would it be better to make this start_layer and num_layers and do the
>    looping in blorp?

Sounds good to me.

> 
>       void
>       intel_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree
>      *mt,
>      diff --git a/src/mesa/drivers/dri/i965/brw_context.c
>      b/src/mesa/drivers/dri/i965/brw_context.c
>      index 3f88f7f..b0e762b 100644
>      --- a/src/mesa/drivers/dri/i965/brw_context.c
>      +++ b/src/mesa/drivers/dri/i965/brw_context.c
>      @@ -316,8 +316,9 @@ intel_update_state(struct gl_context * ctx,
>      GLuint new_state)
>                   intel_renderbuffer(fb->_ColorDrawBuffers[i]);
>                if (irb &&
>      -             intel_miptree_resolve_color(brw, irb->mt,
>      -
>      INTEL_MIPTREE_IGNORE_CCS_E))
>      +             intel_miptree_resolve_color(
>      +                brw, irb->mt, irb->mt_level, irb->mt_layer,
>      +                INTEL_MIPTREE_IGNORE_CCS_E))
> 
>    Do you need to loop here?

Let me check, I think you might be right.

> 
>                   brw_render_cache_set_check_flush(brw, irb->mt->bo);
>             }
>          }
>      @@ -1349,10 +1350,13 @@ intel_resolve_for_dri2_flush(struct
>      brw_context *brw,
>             rb = intel_get_renderbuffer(fb, buffers[i]);
>             if (rb == NULL || rb->mt == NULL)
>                continue;
>      -      if (rb->mt->num_samples <= 1)
>      -         intel_miptree_resolve_color(brw, rb->mt, 0);
>      -      else
>      +      if (rb->mt->num_samples <= 1) {
>      +         assert(rb->mt_layer == 0 && rb->mt_level == 0 &&
>      +                rb->layer_count == 1);
>      +         intel_miptree_resolve_color(brw, rb->mt, 0, 0, 0);
>      +      } else {
>                intel_renderbuffer_downsample(brw, rb);
>      +      }
>          }
>       }
>      diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
>      b/src/mesa/drivers/dri/i965/intel_blit.c
>      index 7e97fbc..a4e1216 100644
>      --- a/src/mesa/drivers/dri/i965/intel_blit.c
>      +++ b/src/mesa/drivers/dri/i965/intel_blit.c
>      @@ -294,8 +294,8 @@ intel_miptree_blit(struct brw_context *brw,
>           */
>          intel_miptree_slice_resolve_depth(brw, src_mt, src_level,
>      src_slice);
>          intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
>      dst_slice);
>      -   intel_miptree_resolve_color(brw, src_mt, 0);
>      -   intel_miptree_resolve_color(brw, dst_mt, 0);
>      +   intel_miptree_resolve_color(brw, src_mt, src_level, src_slice,
>      0);
>      +   intel_miptree_resolve_color(brw, dst_mt, dst_level, dst_slice,
>      0);
>          if (src_flip)
>             src_y = minify(src_mt->physical_height0, src_level -
>      src_mt->first_level) - src_y - height;
>      diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>      b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>      index 328c770..8482afe 100644
>      --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>      +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
>      @@ -2203,12 +2203,35 @@ intel_miptree_all_slices_
>      resolve_depth(struct brw_context *brw,
> 
>      BLORP_HIZ_OP_DEPTH_RESOLVE);
>       }
>      +static void
>      +intel_miptree_check_color_resolve(const struct intel_mipmap_tree
>      *mt,
>      +                                  unsigned level, unsigned layer)
>      +{
>      +   if (!mt->mcs_buf)
>      +      return;
>      +
>      +   /* Fast color clear is not supported for mipmapped surfaces. */
>      +   assert(level == 0 && mt->first_level == 0 && mt->last_level ==
>      0);
>      +
>      +   /* Compression of arrayed msaa surfaces is supported. */
>      +   if (mt->num_samples > 1)
>      +      return;
>      +
>      +   /* Fast color clear is not supported for non-msaa arrays. */
>      +   assert(layer == 0 && mt->logical_depth0 == 1);
> 
>    Is this a hardware limitation or a software limitation?  If it's a
>    hardware limitation, I'd like a PRM citation or something to explain
>    why.  This seems to me like something that should totally be possible.
>    If there are any layered rendering tests in the Vulkan CTS (I think
>    there are but I'm not sure), then it's definitely possible.  I just
>    kicked off a Jenkins run to find out.

This is just stating the current state of affairs in the driver just as the
assert a few lines earlier for mip-mapped. Patch "i965/gen8: Relax asserts
prohibiting arrayed/mipmapped fast clears" in the end lifts the assert for
gen8+.

> 
>      +
>      +   (void)level;
>      +   (void)layer;
>      +}
>       bool
>       intel_miptree_resolve_color(struct brw_context *brw,
>                                   struct intel_mipmap_tree *mt,
>      +                            unsigned level, unsigned layer,
>                                   int flags)
>       {
>      +   intel_miptree_check_color_resolve(mt, level, layer);
>      +
>          /* From gen9 onwards there is new compression scheme for single
>      sampled
>           * surfaces called "lossless compressed". These don't need to be
>      always
>           * resolved.
>      @@ -2227,7 +2250,7 @@ intel_miptree_resolve_color(struct brw_context
>      *brw,
>             /* Fast color clear resolves only make sense for non-MSAA
>      buffers. */
>             if (mt->msaa_layout == INTEL_MSAA_LAYOUT_NONE ||
>                 intel_miptree_is_lossless_compressed(brw, mt)) {
>      -         brw_blorp_resolve_color(brw, mt);
>      +         brw_blorp_resolve_color(brw, mt, level, layer);
>                return true;
>             } else {
>                return false;
>      @@ -2242,7 +2265,7 @@ intel_miptree_all_slices_resolve_color(struct
>      brw_context *brw,
>                                              struct intel_mipmap_tree
>      *mt,
>                                              int flags)
>       {
>      -   intel_miptree_resolve_color(brw, mt, flags);
>      +   intel_miptree_resolve_color(brw, mt, 0, 0, flags);
>       }
>       /**
>      diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>      b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>      index 80cc876..95d9dad 100644
>      --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>      +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
>      @@ -991,6 +991,7 @@ intel_miptree_used_for_rendering(const struct
>      brw_context *brw,
>       bool
>       intel_miptree_resolve_color(struct brw_context *brw,
>                                   struct intel_mipmap_tree *mt,
>      +                            unsigned level, unsigned layer,
>                                   int flags);
>       void
>      --
>      2.5.5
>      _______________________________________________
>      mesa-dev mailing list
>      [3]mesa-dev at lists.freedesktop.org
>      [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> 
> References
> 
>    1. mailto:topi.pohjolainen at gmail.com
>    2. mailto:topi.pohjolainen at intel.com
>    3. mailto:mesa-dev at lists.freedesktop.org
>    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list