[Mesa-dev] [v4 02/11] i965: Relax assertion of halign == 16 for lossless compressed aux

[Ben Widawsky]

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.pohjolainen at 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 <ben at 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."