On Thu, May 05, 2016 at 10:59:16AM -0700, Ben Widawsky wrote: > On Thu, May 05, 2016 at 10:51:32AM -0700, Ben Widawsky wrote: > > On Thu, Apr 21, 2016 at 02:58:57PM +0300, Topi Pohjolainen wrote: > > > Signed-off-by: Topi Pohjolainen <topi.pohjolai...@intel.com> > > > --- > > > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 ++++++- > > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > > index ae08300..b68575f 100644 > > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > > @@ -556,8 +556,13 @@ intel_miptree_create_layout(struct brw_context *brw, > > > } else if (brw->gen >= 9 && num_samples > 1) { > > > layout_flags |= MIPTREE_LAYOUT_FORCE_HALIGN16; > > > } else { > > > + const bool is_lossless_compressed_aux = > > > + brw->gen >= 9 && num_samples == 1 && > > > + mt->format == MESA_FORMAT_R_UINT32; > > > + > > > /* For now, nothing else has this requirement */ > > > - assert((layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0); > > > + assert(is_lossless_compressed_aux || > > > + (layout_flags & MIPTREE_LAYOUT_FORCE_HALIGN16) == 0); > > > } > > > > > > brw_miptree_layout(brw, mt, layout_flags); > > > > I'd just drop the else condition entirely instead. The else case assertion > > also > > becomes invalid when we want to fast clear multi LOD and arrayed surfaces. > > I'm > > slightly worried that the assertion is somewhat frail and not future proof. > > > > Also the format check makes me wonder if you want to add some assertion or > > condition to intel_miptree_is_lossless_compressed? ie. > > > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > index 17c87a2..467a60b 100644 > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > @@ -296,6 +296,9 @@ intel_miptree_is_lossless_compressed(const struct > > brw_context *brw, > > if (mt->msaa_layout != INTEL_MSAA_LAYOUT_CMS) > > return false; > > > > + if (mt->format == MESA_FORMAT_R_UINT32) > > + return false; > > + > > /* And finally distinguish between msaa and single sample case. */ > > return mt->num_samples <= 1; > > } > > > > Either way: > > Reviewed-by: Ben Widawsky <b...@bwidawsk.net> > > Hmm. On second thought, doesn't this still violate what the spec says? > "When Auxiliary Surface Mode is set to AUX_CCS_D or AUX_CCS_E, HALIGN 16 must > be > used."
I'm not sure if I understood this question correctly. The rational for the patch is that intel_miptree_alloc_non_msrt_mcs() calls miptree creation for the aux the buffer with MIPTREE_LAYOUT_FORCE_HALIGN16. This ends up into the branch addressed above. Format in question in MESA_FORMAT_R_UINT32 for which neither of the first two conditions apply: if (intel_miptree_supports_non_msrt_fast_clear(brw, mt)) { ... } else if (brw->gen >= 9 && num_samples > 1) { ... } else { and hence it drops to the default where the assertion hits unless it is relaxed. _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev