[Mesa-dev] [PATCH 2/2] i965: Check CCS_E compatibility for texture view rendering

[Nanley Chery]

On Fri, Oct 27, 2017 at 12:52:30PM -0700, Jason Ekstrand wrote:
> On Fri, Oct 27, 2017 at 12:24 PM, Nanley Chery <nanleychery at gmail.com>
> wrote:
> 
> > Only use CCS_E to render to a texture that is CCS_E-compatible with the
> > original texture's miptree (linear) format. This prevents render
> > operations from writing data that can't be decoded with the original
> > miptree format.
> >
> > On Gen10, with the new CCS_E-enabled formats handled, this enables the
> > driver to pass the arb_texture_view-rendering-formats piglit test.
> >
> > Cc: <mesa-stable at lists.freedesktop.org>
> > Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 28
> > +++++++++++++++++++++++++--
> >  1 file changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index a850f4d17b..59c57c227b 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -241,6 +241,27 @@ intel_miptree_supports_hiz(const struct brw_context
> > *brw,
> >     }
> >  }
> >
> > +/**
> > + * Return true if the format that will be used to access the miptree is
> > + * CCS_E-compatible with the miptree's linear/non-sRGB format.
> > + *
> > + * Why use the linear format? Well, although the miptree may be specified
> > with
> > + * an sRGB format, the usage of that color space/format can be toggled.
> > Since
> > + * our HW tends to support more linear formats than sRGB ones, we use this
> > + * format variant for check for CCS_E compatibility.
> > + */
> > +static bool
> > +format_ccs_e_compat_with_miptree(const struct gen_device_info *devinfo,
> > +                                 const struct intel_mipmap_tree *mt,
> > +                                 enum isl_format access_format)
> > +{
> > +   assert(mt->aux_usage == ISL_AUX_USAGE_CCS_E);
> > +
> > +   mesa_format linear_format = _mesa_get_srgb_format_linear(mt->format);
> > +   enum isl_format isl_format = brw_isl_format_for_mesa_
> > format(linear_format);
> > +   return isl_formats_are_ccs_e_compatible(devinfo, isl_format,
> > access_format);
> > +}
> > +
> >  static bool
> >  intel_miptree_supports_ccs_e(struct brw_context *brw,
> >                               const struct intel_mipmap_tree *mt)
> > @@ -2662,8 +2683,11 @@ intel_miptree_render_aux_usage(struct brw_context
> > *brw,
> >        return mt->mcs_buf ? ISL_AUX_USAGE_CCS_D : ISL_AUX_USAGE_NONE;
> >
> >     case ISL_AUX_USAGE_CCS_E: {
> > -      /* If the format supports CCS_E, then we can just use it */
> > -      if (isl_format_supports_ccs_e(&brw->screen->devinfo,
> > render_format))
> > +      /* If the format supports CCS_E and is compatible with the miptree,
> > +       * then we can use it.
> > +       */
> > +      if (format_ccs_e_compat_with_miptree(&brw->screen->devinfo,
> > +                                           mt, render_format))
> >
> 
> You don't need the helper if you just use mt->surf.format.  That's what
> can_texture_with_ccs does.
> 
> 

Isn't using mt->surf.format making the code more restrictive than
necessary? That field may give you the sRGB format of the surface, but
the helper would always give you the linear variant. Therefore the
helper allows for CCS_E to be used in more cases.

> >           return ISL_AUX_USAGE_CCS_E;
> >
> >        /* Otherwise, we have to fall back to CCS_D */
> > --
> > 2.14.3
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >