On Tue, Sep 6, 2016 at 8:16 AM, Pohjolainen, Topi <
topi.pohjolai...@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.pohjolai...@gmail.com> wrote:
> >
> > Signed-off-by: Topi Pohjolainen <[2]topi.pohjolai...@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@lists.freedesktop.org
> > [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > References
> >
> > 1. mailto:topi.pohjolai...@gmail.com
> > 2. mailto:topi.pohjolai...@intel.com
> > 3. mailto:mesa-dev@lists.freedesktop.org
> > 4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
