<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Sat, Dec 16, 2017 at 3:07 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On December 16, 2017 14:35:29 Nanley Chery <<a href="mailto:nanleychery@gmail.com" target="_blank">nanleychery@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> On Wed, Dec 13, 2017 at 05:52:03PM -0800, Jason Ekstrand wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> This reverts commit ee57b15ec764736e2d5360beaef9fb2045ed0f68. Cc: "17.3" <<a href="mailto:mesa-stable@lists.freedesktop.org" target="_blank">mesa-stable@lists.freedesktop.org</a>> --- 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(mt->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(mt->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; </blockquote> 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). </blockquote> </div></div> 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.</blockquote><div> </div><div>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?</div><div> </div><div>--Jason </div></div></div></div>