On Wed, Nov 23, 2016 at 10:13 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:
> 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+.
>
Ok. That makes sense. I just hadn't read far enough yet.
> >
> > +
> > + (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
