[Mesa-dev] [v4 04/11] i965: Add helper for lossless compression support

[Pohjolainen, Topi]

On Thu, May 05, 2016 at 11:17:41AM -0700, Ben Widawsky wrote:
> On Thu, Apr 21, 2016 at 02:58:59PM +0300, Topi Pohjolainen wrote:
> > v2: Check explicitly against base type of GL_FLOAT instead of
> >     using _mesa_is_format_integer_color(). Otherwise we miss
> >     GL_UNSIGNED_NORMALIZED.
> > 
> > Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 22 ++++++++++++++++++++++
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.h |  3 +++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index b68575f..78717df 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -294,6 +294,28 @@ intel_miptree_is_lossless_compressed(const struct brw_context *brw,
> >     return mt->num_samples <= 1;
> >  }
> >  
> > +bool
> > +intel_miptree_supports_lossless_compressed(mesa_format format)
> > +{
> > +   /* For now compression is only enabled for integer formats even though
> > +    * there exist supported floating point formats also. This is a heuristic
> > +    * decision based on current public benchmarks. In none of the cases these
> > +    * formats provided any improvement but a few cases were seen to regress.
> > +    * Hence these are left to to be enabled in the future when they are known
> > +    * to improve things.
> > +    */
> > +   if (_mesa_get_format_datatype(format) == GL_FLOAT)
> > +      return false;
> > +
> > +   /* In principle, fast clear mechanism and lossless compression go hand in
> > +    * hand. However, fast clear can be also used to clear srgb surfaces by
> > +    * using equivalent linear format. This trick, however, can't be extended
> > +    * to be used with lossless compression and therefore a check is needed to
> > +    * see if the format really is linear.
> > +    */
> > +   return _mesa_get_srgb_format_linear(format) == format;
> > +}
> > +
> 
> Picking nits here, but I don't like this function because it doesn't do what it
> purports to do in it's name. It's really an ancillary function to MCS support.
> How about?

I like it, changed locally and rebased the rest of the series (two patches)
to use it.

> 
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 20c8a7f..17a6f7e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -301,7 +301,9 @@ intel_miptree_is_lossless_compressed(const struct brw_context *brw,
>  }
>  
>  bool
> -intel_miptree_supports_lossless_compressed(mesa_format format)
> +intel_miptree_supports_lossless_compressed(struct brw_context *brw,
> +                                           const struct intel_mipmap_tree *mt,
> +                                           mesa_format format)
>  {
>     /* For now compression is only enabled for integer formats even though
>      * there exist supported floating point formats also. This is a heuristic
> @@ -313,11 +315,14 @@ intel_miptree_supports_lossless_compressed(mesa_format format)
>     if (_mesa_get_format_datatype(format) == GL_FLOAT)
>        return false;
>  
> -   /* In principle, fast clear mechanism and lossless compression go hand in
> -    * hand. However, fast clear can be also used to clear srgb surfaces by
> -    * using equivalent linear format. This trick, however, can't be extended
> -    * to be used with lossless compression and therefore a check is needed to
> -    * see if the format really is linear.
> +   /* Fast clear mechanism and lossless compression go hand in hand. */
> +   if (!intel_miptree_supports_non_msrt_fast_clear(brw, mt))
> +      return false;
> +
> +   /* Fast clear can be also used to clear srgb surfaces by using equivalent
> +    * linear format. This trick, however, can't be extended to be used with
> +    * lossless compression and therefore a check is needed to see if the format
> +    * really is linear.
>      */
>     return _mesa_get_srgb_format_linear(format) == format;
>  }
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index d4f8563..c859f6e 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -680,7 +680,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct brw_context *brw,
>                                             const struct intel_mipmap_tree *mt);
>  
>  bool
> -intel_miptree_supports_lossless_compressed(mesa_format format);
> +intel_miptree_supports_lossless_compressed(struct brw_context *brw,
> +                                           const struct intel_mipmap_tree *mt,
> +                                           mesa_format format);
>  
>  bool
>  intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
> 
> >  /**
> >   * Determine depth format corresponding to a depth+stencil format,
> >   * for separate stencil.
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > index 7862152..6c143c3 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > @@ -679,6 +679,9 @@ intel_miptree_supports_non_msrt_fast_clear(struct brw_context *brw,
> >                                             const struct intel_mipmap_tree *mt);
> >  
> >  bool
> > +intel_miptree_supports_lossless_compressed(mesa_format format);
> > +
> > +bool
> >  intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
> >                                   struct intel_mipmap_tree *mt);
> >