[Mesa-dev] [PATCH 3/3] Revert "i965: Disable regular fast-clears (CCS_D) on gen9+"

Nanley Chery nanleychery at gmail.com
Mon Dec 18 16:46:26 UTC 2017


On Sun, Dec 17, 2017 at 07:34:47PM -0800, Jason Ekstrand wrote:
> On Sat, Dec 16, 2017 at 3:07 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> 
> > On December 16, 2017 14:35:29 Nanley Chery <nanleychery at gmail.com> wrote:
> >
> > On Wed, Dec 13, 2017 at 05:52:03PM -0800, Jason Ekstrand wrote:
> >>
> >>> This reverts commit ee57b15ec764736e2d5360beaef9fb2045ed0f68.
> >>>
> >>> Cc: "17.3" <mesa-stable at lists.freedesktop.org>
> >>> ---
> >>>  src/mesa/drivers/dri/i965/brw_meta_util.c     | 10 -----
> >>>  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 57
> >>> ++++++++++++---------------
> >>>  2 files changed, 25 insertions(+), 42 deletions(-)
> >>>
> >>> diff --git a/src/mesa/drivers/dri/i965/brw_meta_util.c
> >>> b/src/mesa/drivers/dri/i965/brw_meta_util.c
> >>> index 54dc6a5..b311815 100644
> >>> --- a/src/mesa/drivers/dri/i965/brw_meta_util.c
> >>> +++ b/src/mesa/drivers/dri/i965/brw_meta_util.c
> >>> @@ -293,17 +293,7 @@ brw_is_color_fast_clear_compatible(struct
> >>> brw_context *brw,
> >>>         brw->mesa_to_isl_render_format[mt->format])
> >>>        return false;
> >>>
> >>> -   /* Gen9 doesn't support fast clear on single-sampled SRGB buffers.
> >>> When
> >>> -    * GL_FRAMEBUFFER_SRGB is enabled any color renderbuffers will be
> >>> -    * resolved in intel_update_state. In that case it's pointless to do
> >>> a
> >>> -    * fast clear because it's very likely to be immediately resolved.
> >>> -    */
> >>>     const bool srgb_rb = _mesa_get_srgb_format_linear(mt->format) !=
> >>> mt->format;
> >>> -   if (devinfo->gen >= 9 &&
> >>> -       mt->surf.samples == 1 &&
> >>> -       ctx->Color.sRGBEnabled && srgb_rb)
> >>> -      return false;
> >>> -
> >>>    /* Gen10 doesn't automatically decode the clear color of sRGB
> >>> buffers. Since
> >>>     * we currently don't perform this decode in software, avoid a
> >>> fast-clear
> >>>     * altogether. TODO: Do this in software.
> >>> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >>> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >>> index c1a4ce1..b87d356 100644
> >>> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >>> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >>> @@ -207,13 +207,7 @@ intel_miptree_supports_ccs(struct brw_context *brw,
> >>>     if (!brw->mesa_format_supports_render[mt->format])
> >>>        return false;
> >>>
> >>> -   if (devinfo->gen >= 9) {
> >>> -      mesa_format linear_format = _mesa_get_srgb_format_linear(m
> >>> t->format);
> >>> -      const enum isl_format isl_format =
> >>> -         brw_isl_format_for_mesa_format(linear_format);
> >>> -      return isl_format_supports_ccs_e(&brw->screen->devinfo,
> >>> isl_format);
> >>> -   } else
> >>> -      return true;
> >>> +   return true;
> >>>  }
> >>>
> >>>  static bool
> >>> @@ -256,7 +250,7 @@ intel_miptree_supports_hiz(const struct brw_context
> >>> *brw,
> >>>   * our HW tends to support more linear formats than sRGB ones, we use
> >>> this
> >>>   * format variant for check for CCS_E compatibility.
> >>>   */
> >>> -MAYBE_UNUSED static bool
> >>> +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)
> >>> @@ -290,12 +284,13 @@ intel_miptree_supports_ccs_e(struct brw_context
> >>> *brw,
> >>>     if (!intel_miptree_supports_ccs(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.
> >>> +   /* Many window system buffers are sRGB even if they are never
> >>> rendered as
> >>> +    * sRGB.  For those, we want CCS_E for when sRGBEncode is false.
> >>> When the
> >>> +    * surface is used as sRGB, we fall back to CCS_D.
> >>>      */
> >>> -   return _mesa_get_srgb_format_linear(mt->format) == mt->format;
> >>> +   mesa_format linear_format = _mesa_get_srgb_format_linear(m
> >>> t->format);
> >>> +   enum isl_format isl_format = brw_isl_format_for_mesa_format
> >>> (linear_format);
> >>> +   return isl_format_supports_ccs_e(&brw->screen->devinfo, isl_format);
> >>>  }
> >>>
> >>>  /**
> >>> @@ -2686,29 +2681,27 @@ intel_miptree_render_aux_usage(struct
> >>> brw_context *brw,
> >>>        return ISL_AUX_USAGE_MCS;
> >>>
> >>>     case ISL_AUX_USAGE_CCS_D:
> >>> -      /* If FRAMEBUFFER_SRGB is used on Gen9+ then we need to resolve
> >>> any of
> >>> -       * the single-sampled color renderbuffers because the CCS buffer
> >>> isn't
> >>> -       * supported for SRGB formats. This only matters if
> >>> FRAMEBUFFER_SRGB is
> >>> -       * enabled because otherwise the surface state will be programmed
> >>> with
> >>> -       * the linear equivalent format anyway.
> >>> -       */
> >>> -      if (isl_format_is_srgb(render_format) &&
> >>> -          _mesa_get_srgb_format_linear(mt->format) != mt->format) {
> >>> -         return ISL_AUX_USAGE_NONE;
> >>> -      } else if (!mt->mcs_buf) {
> >>> -         return ISL_AUX_USAGE_NONE;
> >>> -      } else {
> >>> -         return ISL_AUX_USAGE_CCS_D;
> >>> -      }
> >>> +      return mt->mcs_buf ? ISL_AUX_USAGE_CCS_D : ISL_AUX_USAGE_NONE;
> >>>
> >>>     case ISL_AUX_USAGE_CCS_E: {
> >>> -      /* Lossless compression is not supported for SRGB formats, it
> >>> -       * should be impossible to get here with such surfaces.
> >>> +      /* If the format supports CCS_E and is compatible with the
> >>> miptree,
> >>> +       * then we can use it.
> >>>         */
> >>> -      assert(!isl_format_is_srgb(render_format) ||
> >>> -             _mesa_get_srgb_format_linear(mt->format) == mt->format);
> >>> +      if (format_ccs_e_compat_with_miptree(&brw->screen->devinfo,
> >>> +                                           mt, render_format))
> >>> +         return ISL_AUX_USAGE_CCS_E;
> >>> +
> >>> +      /* Otherwise, we have to fall back to CCS_D */
> >>> +
> >>> +      /* gen9 hardware technically supports non-0/1 clear colors with
> >>> sRGB
> >>> +       * formats.  However, there are issues with blending where it
> >>> doesn't
> >>> +       * properly apply the sRGB curve to the clear color when blending.
> >>> +       */
> >>> +      if (blend_enabled && isl_format_is_srgb(render_format) &&
> >>> +          !isl_color_value_is_zero_one(mt->fast_clear_color,
> >>> render_format))
> >>> +         return ISL_AUX_USAGE_NONE;
> >>>
> >>>
> >> We also need the blend workaround for the CCS_D case. Otherwise, we'll
> >> start failing piglit's arb_framebuffer_srgb-fast-clear-blend when
> >> DEBUG_NO_RBC is set (MD5-287).
> >>
> >
> > Initially, I waz very confused as to why that's in the CCS_E case at all.
> > However, as of this patch, mt->aux_usage is always set to CCS_D for any
> > 8888 surfaces.  That said, it would probably be better to have it in both
> > cases.
> 
> 
> Sorry, I misread and didn't see the bit about DEBUG_NO_RGC.  Given that it
> requires a debug flag, do you mind if I do that as a follow-on?
> 

That's fine with me.

-Nanley

> --Jason


More information about the mesa-dev mailing list