On Mon, Feb 15, 2016 at 12:55:28PM -0800, Ben Widawsky wrote: > On Thu, Feb 11, 2016 at 08:34:07PM +0200, Topi Pohjolainen wrote: > > v2 (Ben): Use combination of msaa_layout and number of samples > > instead of introducing explicit type for lossless > > compression (intel_miptree_is_lossless_compressed()). > > > > Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > --- > > src/mesa/drivers/dri/i965/brw_defines.h | 1 + > > src/mesa/drivers/dri/i965/gen8_surface_state.c | 6 ++++++ > > 2 files changed, 7 insertions(+) > > > > diff --git a/src/mesa/drivers/dri/i965/brw_defines.h > > b/src/mesa/drivers/dri/i965/brw_defines.h > > index b1fa559..f903335 100644 > > --- a/src/mesa/drivers/dri/i965/brw_defines.h > > +++ b/src/mesa/drivers/dri/i965/brw_defines.h > > @@ -656,6 +656,7 @@ > > #define GEN8_SURFACE_AUX_MODE_MCS 1 > > #define GEN8_SURFACE_AUX_MODE_APPEND 2 > > #define GEN8_SURFACE_AUX_MODE_HIZ 3 > > +#define GEN9_SURFACE_AUX_MODE_CCS_E 5 > > > > /* Surface state DW7 */ > > #define GEN9_SURFACE_RT_COMPRESSION_SHIFT 30 > > diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c > > b/src/mesa/drivers/dri/i965/gen8_surface_state.c > > index 0a52815..e1a37d8 100644 > > --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c > > +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c > > @@ -216,6 +216,9 @@ gen8_get_aux_mode(const struct brw_context *brw, > > if (brw->gen >= 9 || mt->num_samples == 1) > > assert(mt->halign == 16); > > > > + if (intel_miptree_is_lossless_compressed(brw, mt)) > > + return GEN9_SURFACE_AUX_MODE_CCS_E; > > + > > return GEN8_SURFACE_AUX_MODE_MCS; > > } > > > > @@ -484,6 +487,9 @@ gen8_update_renderbuffer_surface(struct brw_context > > *brw, > > struct intel_mipmap_tree *aux_mt = mt->mcs_mt; > > const uint32_t aux_mode = gen8_get_aux_mode(brw, mt, surf_type); > > > > + if (aux_mode == GEN9_SURFACE_AUX_MODE_CCS_E) > > + mt->fast_clear_state = INTEL_FAST_CLEAR_STATE_UNRESOLVED; > > + > > I am somewhat undecided about whether or not this should be here. On the one > hand, this is a gen specific thing (you rendered to a losslessy compressed > buffer, which is only a gen9+ thing). On the other hand, we handle all similar > stuff in the common meta code, and modifying fast_clear_state here seems a bit > unclean. > > I'm not opposed to doing this as long as you've considered my potential > objection.
I wasn't that happy putting this here either. Main render target loop in brw_wm_surface_state.c::brw_update_renderbuffer_surfaces() is even more gen-agnostic, and therefore I didn't want to put this there either. Now that you brought this up, I think the correct place would be brw_postdraw_set_buffers_need_resolve() called in the end of brw_try_draw_prims(). What do you think? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev