On Wed, Nov 23, 2016 at 09:26:29AM -0800, Jason Ekstrand wrote:
> General comment: Does it make sense to squash this with the previous
> patch? I'm fine either way.
> On Wed, Nov 23, 2016 at 1:16 AM, Topi Pohjolainen
> <[1]topi.pohjolai...@gmail.com> wrote:
>
> Signed-off-by: Topi Pohjolainen <[2]topi.pohjolai...@intel.com>
> ---
> src/mesa/drivers/dri/i965/brw_blorp.c | 15 +++++++++------
> src/mesa/drivers/dri/i965/brw_blorp.h | 3 ++-
> src/mesa/drivers/dri/i965/brw_context.c | 14 +++++++++-----
> src/mesa/drivers/dri/i965/intel_blit.c | 4 ++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 27
> +++++++++++++++++++++++++--
> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 1 +
> 6 files changed, 48 insertions(+), 16 deletions(-)
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c
> b/src/mesa/drivers/dri/i965/brw_blorp.c
> index 9a849f5..99df21e 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.c
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c
> @@ -213,7 +213,10 @@ blorp_surf_for_miptree(struct brw_context *brw,
> if (safe_aux_usage & (1 << ISL_AUX_USAGE_CCS_E))
> flags |= INTEL_MIPTREE_IGNORE_CCS_E;
> - intel_miptree_resolve_color(brw, mt, flags);
> + for (unsigned i = 0; i < num_layers; i++) {
> + intel_miptree_resolve_color(brw, mt,
> + *level, start_layer + i,
> flags);
> + }
> assert(mt->fast_clear_state == INTEL_FAST_CLEAR_STATE_
> RESOLVED);
> surf->aux_usage = ISL_AUX_USAGE_NONE;
> @@ -942,19 +945,19 @@ brw_blorp_clear_color(struct brw_context *brw,
> struct gl_framebuffer *fb,
> }
> void
> -brw_blorp_resolve_color(struct brw_context *brw, struct
> intel_mipmap_tree *mt)
> +brw_blorp_resolve_color(struct brw_context *brw, struct
> intel_mipmap_tree *mt,
> + unsigned level, unsigned layer)
> {
> - DBG("%s to mt %p\n", __FUNCTION__, mt);
> + DBG("%s to mt %p level %u layer %u\n", __FUNCTION__, mt, level,
> layer);
> const mesa_format format = _mesa_get_srgb_format_linear(
> mt->format);
> struct isl_surf isl_tmp[2];
> struct blorp_surf surf;
> - unsigned level = 0;
> blorp_surf_for_miptree(brw, &surf, mt, true,
> (1 << ISL_AUX_USAGE_CCS_E) |
> (1 << ISL_AUX_USAGE_CCS_D),
> - &level, 0 /* start_layer */, 1 /*
> num_layers */,
> + &level, layer, 1 /* num_layers */,
> isl_tmp);
> enum blorp_fast_clear_op resolve_op;
> @@ -971,7 +974,7 @@ brw_blorp_resolve_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt)
> struct blorp_batch batch;
> blorp_batch_init(&brw->blorp, &batch, brw, 0);
> - blorp_ccs_resolve(&batch, &surf, 0 /* level */, 0 /* layer */,
> + blorp_ccs_resolve(&batch, &surf, level, layer,
> brw_blorp_to_isl_format(brw, format, true),
> resolve_op);
> blorp_batch_finish(&batch);
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.h
> b/src/mesa/drivers/dri/i965/brw_blorp.h
> index abf3956..277b00e 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp.h
> +++ b/src/mesa/drivers/dri/i965/brw_blorp.h
> @@ -64,7 +64,8 @@ brw_blorp_clear_color(struct brw_context *brw,
> struct gl_framebuffer *fb,
> void
> brw_blorp_resolve_color(struct brw_context *brw,
> - struct intel_mipmap_tree *mt);
> + struct intel_mipmap_tree *mt,
> + unsigned level, unsigned layer);
>
> Would it be better to make this start_layer and num_layers and do the
> looping in blorp?
Sounds good to me.
>
> void
> intel_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree
> *mt,
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c
> b/src/mesa/drivers/dri/i965/brw_context.c
> index 3f88f7f..b0e762b 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -316,8 +316,9 @@ intel_update_state(struct gl_context * ctx,
> GLuint new_state)
> intel_renderbuffer(fb->_ColorDrawBuffers[i]);
> if (irb &&
> - intel_miptree_resolve_color(brw, irb->mt,
> -
> INTEL_MIPTREE_IGNORE_CCS_E))
> + intel_miptree_resolve_color(
> + brw, irb->mt, irb->mt_level, irb->mt_layer,
> + INTEL_MIPTREE_IGNORE_CCS_E))
>
> Do you need to loop here?
Let me check, I think you might be right.
>
> brw_render_cache_set_check_flush(brw, irb->mt->bo);
> }
> }
> @@ -1349,10 +1350,13 @@ intel_resolve_for_dri2_flush(struct
> brw_context *brw,
> rb = intel_get_renderbuffer(fb, buffers[i]);
> if (rb == NULL || rb->mt == NULL)
> continue;
> - if (rb->mt->num_samples <= 1)
> - intel_miptree_resolve_color(brw, rb->mt, 0);
> - else
> + if (rb->mt->num_samples <= 1) {
> + assert(rb->mt_layer == 0 && rb->mt_level == 0 &&
> + rb->layer_count == 1);
> + intel_miptree_resolve_color(brw, rb->mt, 0, 0, 0);
> + } else {
> intel_renderbuffer_downsample(brw, rb);
> + }
> }
> }
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c
> b/src/mesa/drivers/dri/i965/intel_blit.c
> index 7e97fbc..a4e1216 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -294,8 +294,8 @@ intel_miptree_blit(struct brw_context *brw,
> */
> intel_miptree_slice_resolve_depth(brw, src_mt, src_level,
> src_slice);
> intel_miptree_slice_resolve_depth(brw, dst_mt, dst_level,
> dst_slice);
> - intel_miptree_resolve_color(brw, src_mt, 0);
> - intel_miptree_resolve_color(brw, dst_mt, 0);
> + intel_miptree_resolve_color(brw, src_mt, src_level, src_slice,
> 0);
> + intel_miptree_resolve_color(brw, dst_mt, dst_level, dst_slice,
> 0);
> if (src_flip)
> src_y = minify(src_mt->physical_height0, src_level -
> src_mt->first_level) - src_y - height;
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> index 328c770..8482afe 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> @@ -2203,12 +2203,35 @@ intel_miptree_all_slices_
> resolve_depth(struct brw_context *brw,
>
> BLORP_HIZ_OP_DEPTH_RESOLVE);
> }
> +static void
> +intel_miptree_check_color_resolve(const struct intel_mipmap_tree
> *mt,
> + unsigned level, unsigned layer)
> +{
> + if (!mt->mcs_buf)
> + return;
> +
> + /* Fast color clear is not supported for mipmapped surfaces. */
> + assert(level == 0 && mt->first_level == 0 && mt->last_level ==
> 0);
> +
> + /* Compression of arrayed msaa surfaces is supported. */
> + if (mt->num_samples > 1)
> + return;
> +
> + /* Fast color clear is not supported for non-msaa arrays. */
> + assert(layer == 0 && mt->logical_depth0 == 1);
>
> Is this a hardware limitation or a software limitation? If it's a
> hardware limitation, I'd like a PRM citation or something to explain
> why. This seems to me like something that should totally be possible.
> If there are any layered rendering tests in the Vulkan CTS (I think
> there are but I'm not sure), then it's definitely possible. I just
> kicked off a Jenkins run to find out.
This is just stating the current state of affairs in the driver just as the
assert a few lines earlier for mip-mapped. Patch "i965/gen8: Relax asserts
prohibiting arrayed/mipmapped fast clears" in the end lifts the assert for
gen8+.
>
> +
> + (void)level;
> + (void)layer;
> +}
> bool
> intel_miptree_resolve_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> + unsigned level, unsigned layer,
> int flags)
> {
> + intel_miptree_check_color_resolve(mt, level, layer);
> +
> /* From gen9 onwards there is new compression scheme for single
> sampled
> * surfaces called "lossless compressed". These don't need to be
> always
> * resolved.
> @@ -2227,7 +2250,7 @@ intel_miptree_resolve_color(struct brw_context
> *brw,
> /* Fast color clear resolves only make sense for non-MSAA
> buffers. */
> if (mt->msaa_layout == INTEL_MSAA_LAYOUT_NONE ||
> intel_miptree_is_lossless_compressed(brw, mt)) {
> - brw_blorp_resolve_color(brw, mt);
> + brw_blorp_resolve_color(brw, mt, level, layer);
> return true;
> } else {
> return false;
> @@ -2242,7 +2265,7 @@ intel_miptree_all_slices_resolve_color(struct
> brw_context *brw,
> struct intel_mipmap_tree
> *mt,
> int flags)
> {
> - intel_miptree_resolve_color(brw, mt, flags);
> + intel_miptree_resolve_color(brw, mt, 0, 0, flags);
> }
> /**
> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> index 80cc876..95d9dad 100644
> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> @@ -991,6 +991,7 @@ intel_miptree_used_for_rendering(const struct
> brw_context *brw,
> bool
> intel_miptree_resolve_color(struct brw_context *brw,
> struct intel_mipmap_tree *mt,
> + unsigned level, unsigned layer,
> int flags);
> void
> --
> 2.5.5
> _______________________________________________
> mesa-dev mailing list
> [3]mesa-dev@lists.freedesktop.org
> [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> References
>
> 1. mailto:topi.pohjolai...@gmail.com
> 2. mailto:topi.pohjolai...@intel.com
> 3. mailto:mesa-dev@lists.freedesktop.org
> 4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
