[Mesa-dev] [v2 05/19] i965: Add helper for detecting lossless compression

[Ben Widawsky]

On Sat, Feb 13, 2016 at 09:36:01AM +0200, Pohjolainen, Topi wrote:
> On Thu, Feb 11, 2016 at 01:48:18PM -0800, Ben Widawsky wrote:
> > On Thu, Feb 11, 2016 at 08:33:58PM +0200, Topi Pohjolainen wrote:
> > > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > > ---
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 26 ++++++++++++++++++++++++++
> > >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  4 ++++
> > >  2 files changed, 30 insertions(+)
> > > 
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > index 5f739d9..31de1ff 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > > @@ -266,6 +266,32 @@ intel_miptree_supports_non_msrt_fast_clear(struct brw_context *brw,
> > >        return true;
> > >  }
> > >  
> > > +/* On Gen9 support for color buffer compression was extended to single
> > > + * sampled surfaces. This is a helper considering both auxiliary buffer
> > > + * type and number of samples telling if the given miptree represents
> > > + * the new single sampled case - also called lossless compression.
> > > + */
> > > +bool
> > > +intel_miptree_is_lossless_compressed(const struct brw_context *brw,
> > > +                                     const struct intel_mipmap_tree *mt)
> > > +{
> > > +   /* Only available from Gen9 onwards. */
> > > +   if (brw->gen < 9)
> > > +      return false;
> > > +
> > > +   /* Compression always requires auxiliary buffer. */
> > > +   if (!mt->mcs_mt)
> > > +      return false;
> > > +
> > > +   /* Single sample compression is represented re-using msaa compression
> > > +    * layout type: "Compressed Multisampled Surfaces".
> > > +    */
> > > +   if (mt->msaa_layout != INTEL_MSAA_LAYOUT_CMS)
> > > +      return false;
> > > +
> > > +   /* And finally distinguish between msaa and single sample case. */
> > > +   return mt->num_samples <= 1;
> > > +}
> > >  
> > >  /**
> > >   * Determine depth format corresponding to a depth+stencil format,
> > > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > > index 64f73ea..13d4d7e 100644
> > > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > > @@ -667,6 +667,10 @@ intel_get_non_msrt_mcs_alignment(struct intel_mipmap_tree *mt,
> > >                                   unsigned *width_px, unsigned *height);
> > >  
> > >  bool
> > > +intel_miptree_is_lossless_compressed(const struct brw_context *brw,
> > > +                                     const struct intel_mipmap_tree *mt);
> > > +
> > > +bool
> > >  intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
> > >                                   struct intel_mipmap_tree *mt);
> > >  
> > 
> > So I take it this was one of the big changes from my feedback earlier. Do you
> > prefer this result, or what? I can live with the old thing if you want it.
> 
> I think I like this better, it avoids the CSS/CCS confusion. And in principle,
> both MCS and CCS_E are about compression, and the fact that hardware
> implements them separately only needs to be reflected in surface setup and
> color resolve logic. No need to go further.

I'm glad you like it. I hope it didn't cost too much time, and more importantly,
I hope we don't decide the other way was better at some later date :-)

Reviewed-by: Ben Widawsky <benjamin.widawsky at intel.com>

-- 
Ben Widawsky, Intel Open Source Technology Center