On Sat, Dec 16, 2017 at 3:07 PM, Jason Ekstrand <ja...@jlekstrand.net>
wrote:
> On December 16, 2017 14:35:29 Nanley Chery <nanleych...@gmail.com> wrote:
>
> On Wed, Dec 13, 2017 at 05:52:03PM -0800, Jason Ekstrand wrote:
>>
>>> This reverts commit ee57b15ec764736e2d5360beaef9fb2045ed0f68.
>>>
>>> Cc: "17.3" <mesa-sta...@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?
--Jason
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
