<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Wed, Nov 23, 2016 at 10:13 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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]<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > Signed-off-by: Topi Pohjolainen <[2]<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> <div><div class="h5">> --- > 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? </div></div>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. <div><div class="h5"> > > 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. </div></div>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+. </blockquote><div> </div><div>Ok. That makes sense. I just hadn't read far enough yet. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > > + > + (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 </div></div>> [3]<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > [4]<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > References > > 1. mailto:<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a> > 2. mailto:<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a> > 3. mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > 4. <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>