[Mesa-dev] [v2 3/6] i965: Track non-compressible sampling of renderbuffers

Jason Ekstrand jason at jlekstrand.net
Tue Sep 6 15:24:11 UTC 2016


On Tue, Sep 6, 2016 at 8:16 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Tue, Sep 06, 2016 at 07:54:16AM -0700, Jason Ekstrand wrote:
> >    On Tue, Sep 6, 2016 at 12:28 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_context.c          | 16
> >      ++++++++++++++++
> >       src/mesa/drivers/dri/i965/brw_context.h          | 10 ++++++++++
> >       src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 12 ++++++++++--
> >       3 files changed, 36 insertions(+), 2 deletions(-)
> >      diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> >      b/src/mesa/drivers/dri/i965/brw_context.c
> >      index b880b4f..c5c6fdd 100644
> >      --- a/src/mesa/drivers/dri/i965/brw_context.c
> >      +++ b/src/mesa/drivers/dri/i965/brw_context.c
> >      @@ -197,6 +197,22 @@ intel_texture_view_requires_resolve(struct
> >      brw_context *brw,
> >                     _mesa_get_format_name(intel_tex->_Format),
> >                     _mesa_get_format_name(intel_tex->mt->format));
> >      +   const struct gl_framebuffer *fb = brw->ctx.DrawBuffer;
> >      +   for (unsigned i = 0; i < fb->_NumColorDrawBuffers; i++) {
> >      +      const struct intel_renderbuffer *irb =
> >      +         intel_renderbuffer(fb->_ColorDrawBuffers[i]);
> >      +
> >      +      /* In case the same surface is also used for rendering one
> >      needs to
> >      +       * disable the compression.
> >      +       */
> >      +      brw->draw_aux_buffer_disabled[i] = intel_tex->mt->bo ==
> >      irb->mt->bo;
>
> This loop goes thru all render surfaces and explicitly sets the flag. In
> other words all flags are reset before uploading state - no need for
> separate memset.
>

Ugh... then it is busted if you have multiple render targets *or* multiple
textures where the one being rendered to isn't the last one.  Only the
render buffer for the last bound texture will get the
buffer_aux_buffer_disabled[] bit set.  We really need a reset and
set-of-disable-needed model.


> >      +
> >      +      if (brw->draw_aux_buffer_disabled[i]) {
> >      +         perf_debug("Sampling renderbuffer with non-compressible
> >      format - "
> >      +                    "turning off compression");
> >      +      }
> >      +   }
> >      +
> >          return true;
> >       }
> >      diff --git a/src/mesa/drivers/dri/i965/brw_context.h
> >      b/src/mesa/drivers/dri/i965/brw_context.h
> >      index 12ac8af..074d554 100644
> >      --- a/src/mesa/drivers/dri/i965/brw_context.h
> >      +++ b/src/mesa/drivers/dri/i965/brw_context.h
> >      @@ -1333,6 +1333,16 @@ struct brw_context
> >          struct brw_fast_clear_state *fast_clear_state;
> >      +   /* Array of flags telling if auxiliary buffer is disabled for
> >      corresponding
> >      +    * renderbuffer. If draw_aux_buffer_disabled[i] is set then use
> >      of
> >      +    * auxiliary buffer for gl_framebuffer::_ColorDrawBuffers[i] is
> >      +    * disabled.
> >      +    * This is needed in case the same underlying buffer is also
> >      configured
> >      +    * to be sampled but with a format that the sampling engine
> >      can't treat
> >      +    * compressed or fast cleared.
> >      +    */
> >      +   bool draw_aux_buffer_disabled[MAX_DRAW_BUFFERS];
> >
> >    I like the way you handled this.  It's nice and clean.  However, I
> >    don't see where you memset draw_aux_buffer_disabled to 0 to reset it.
> >    Did that just go missing?
> >
> >      +
> >          __DRIcontext *driContext;
> >          struct intel_screen *intelScreen;
> >       };
> >      diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >      b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >      index 073919e..af102a9 100644
> >      --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >      +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> >      @@ -56,6 +56,7 @@
> >       enum {
> >          INTEL_RENDERBUFFER_LAYERED = 1 << 0,
> >      +   INTEL_RENDERBUFFER_AUX_DISABLED = 1 << 1,
> >       };
> >       struct surface_state_info {
> >      @@ -194,6 +195,10 @@ brw_update_renderbuffer_surface(struct
> >      brw_context *brw,
> >          struct intel_renderbuffer *irb = intel_renderbuffer(rb);
> >          struct intel_mipmap_tree *mt = irb->mt;
> >      +   if (brw->gen < 9) {
> >      +      assert(!(flags & INTEL_RENDERBUFFER_AUX_DISABLED));
> >      +   }
> >      +
> >          assert(brw_render_target_supported(brw, rb));
> >          intel_miptree_used_for_rendering(mt);
> >      @@ -885,6 +890,7 @@ gen4_update_renderbuffer_surface(struct
> >      brw_context *brw,
> >          /* BRW_NEW_FS_PROG_DATA */
> >          assert(!(flags & INTEL_RENDERBUFFER_LAYERED));
> >      +   assert(!(flags & INTEL_RENDERBUFFER_AUX_DISABLED));
> >          if (rb->TexImage && !brw->has_surface_tile_offset) {
> >             intel_renderbuffer_get_tile_offsets(irb, &tile_x, &tile_y);
> >      @@ -987,8 +993,10 @@ brw_update_renderbuffer_surfaces(struct
> >      brw_context *brw,
> >          if (fb->_NumColorDrawBuffers >= 1) {
> >             for (i = 0; i < fb->_NumColorDrawBuffers; i++) {
> >                const uint32_t surf_index = render_target_start + i;
> >      -         const int flags =
> >      -            _mesa_geometric_layers(fb) > 0 ?
> >      INTEL_RENDERBUFFER_LAYERED : 0;
> >      +         const int flags = (_mesa_geometric_layers(fb) > 0 ?
> >      +                              INTEL_RENDERBUFFER_LAYERED : 0) |
> >      +                           (brw->draw_aux_buffer_disabled[i] ?
> >      +                              INTEL_RENDERBUFFER_AUX_DISABLED :
> 0);
> >               if (intel_renderbuffer(fb->_ColorDrawBuffers[i])) {
> >                   surf_offset[surf_index] =
> >      --
> >      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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160906/3c5914f0/attachment.html>


More information about the mesa-dev mailing list