There's a patch on your branch I didn't see on mesa-dev.
Subject: i965: Be a bit more conservative about certain resolves
It has my r-b.
I have comments on this patch...
On Fri 26 May 2017, Jason Ekstrand wrote:
> We have two different bits of resolve code for render targets: one in
> brw_draw where it's always been and one in brw_context to deal with sRGB
> on gen9. Let's pull them together.
> ---
> src/mesa/drivers/dri/i965/brw_context.c | 47
> ++++++++++++++++++++-------------
> src/mesa/drivers/dri/i965/brw_draw.c | 34 ------------------------
> 2 files changed, 29 insertions(+), 52 deletions(-)
> + /* For layered rendering non-compressed fast cleared buffers need to be
> + * resolved. Surface state can carry only one fast color clear value
> + * while each layer may have its own fast clear color value. For
> + * compressed buffers color value is available in the color buffer.
> + */
> + if (irb->layer_count > 1 &&
> + !(irb->mt->aux_disable & INTEL_AUX_DISABLE_CCS) &&
> + !intel_miptree_is_lossless_compressed(brw, mt)) {
This condition smells bad. It smells like a shot in the dark. It smells
like a haphazard guess. "We haven't permanently disabled CCS for this
miptree. And it lacks CCS_E. So, well, it probably has CCS_D, I guess.".
I would much rather see the condition with something more certain.
Something like:
if (irb->layer_count > 1 &&
intel_miptree_has_css_d_in_layer_range(brw, mt, irb->mt_level,
irb->mt_layer, irb->layer_count))
Anway, this patch is a good cleanup, and functional changes like I'm
requesting don't belong in a refactoring patch like this one.
Reviewed-by: Chad Versace <chadvers...@chromium.org>
> + assert(brw->gen >= 8);
> +
> + intel_miptree_resolve_color(brw, mt, irb->mt_level, 1,
> + irb->mt_layer, irb->layer_count, 0);
> + }
> }
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
