<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Sep 6, 2016 at 8:16 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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]<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > Signed-off-by: Topi Pohjolainen <[2]<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> <div><div class="h5">> --- > 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; </div></div>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. </blockquote><div> </div><div>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. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > + > + 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 </div></div>> [3]<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > [4]<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > References > > 1. mailto:<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a> > 2. mailto:<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a> > 3. mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > 4. <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>