<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 22, 2016 at 4:02 PM, Jordan Justen <<a href="mailto:jordan.l.justen@intel.com" target="_blank">jordan.l.justen@intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 2016-11-22 15:16:45, Jason Ekstrand wrote: > On Thu, Nov 17, 2016 at 3:27 PM, Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> > wrote: > > Signed-off-by: Jordan Justen <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> > Cc: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> > --- > src/mesa/drivers/dri/i965/brw_blorp.c | 83 > ++++++++++++++++++++++++++++------- > 1 file changed, 66 insertions(+), 17 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > b/src/mesa/drivers/dri/i965/brw_blorp.c > index 8aab1a7..d66bf14 100644 > --- a/src/mesa/drivers/dri/i965/brw_blorp.c > +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > @@ -39,6 +39,8 @@ > > #define FILE_DEBUG_FLAG DEBUG_BLORP > > +static const bool isl_hiz_debug_dump = false; > + > static void > brw_blorp_map(const struct blorp_context *blorp, > const struct blorp_address *blorp_addr, > @@ -1005,20 +1007,42 @@ brw_blorp_resolve_color(struct brw_context *brw, > struct intel_mipmap_tree *mt) > } > > static void > -gen6_blorp_hiz_exec(struct brw_context *brw, struct intel_mipmap_tree > *mt, > - unsigned int level, unsigned int layer, enum > blorp_hiz_op op) > +hiz_dump_blorp_surf(const struct blorp_context *blorp, > + const struct blorp_surf *surf, > + const char *opname, const char *when) > > How is this code different from the other blorp surface dumping code? > They seem pretty much the same to me. > </div></div>Yeah, it is pretty similar, but I didn't add a new public interface to blorp for dumping a surface. </blockquote><div> </div><div>You've already added all of the mapping functions. It's not that much further. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Does adding a new 'blorp_dump_surf' function sound like the right idea? </blockquote><div> </div><div>I'd probably call it blorp_surf_dump, but yeah. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I guess that would make it easier to add similar debug features to other blorp ops (like clears). </blockquote><div> </div><div>Yup </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> -Jordan <div class="HOEnZb"><div class="h5"> > > { > - assert(intel_miptree_level_has_hiz(mt, level)); > - > - struct isl_surf isl_tmp[2]; > - struct blorp_surf surf; > - blorp_surf_for_miptree(brw, &surf, mt, true, (1 << > ISL_AUX_USAGE_HIZ), > - &level, layer, 1, isl_tmp); > + void *map, *aux_map; > + unsigned int size, aux_size; > + bool was_mapped, aux_was_mapped; > + char *basename; > + > + blorp->map(blorp, &surf->addr, &map, &size, &was_mapped); > + if (map == NULL) > + return; > + > + if (surf->aux_addr.buffer) { > + blorp->map(blorp, &surf->aux_addr, &aux_map, &aux_size, > &aux_was_mapped); > + if (aux_map == NULL) { > + if (!was_mapped) > + blorp->unmap(blorp, &surf->addr); > + return; > + } > + } else { > + aux_map = NULL; > + aux_size = 0; > + } > > - struct blorp_batch batch; > - blorp_batch_init(&brw->blorp, &batch, brw, 0); > - blorp_gen6_hiz_op(&batch, &surf, level, layer, op); > - blorp_batch_finish(&batch); > + basename = ralloc_asprintf(NULL, "%s-%s", opname, when); > + assert(basename); > + isl_dump_surf(blorp->isl_dev, surf->surf, map, size, > + aux_map ? surf->aux_surf : NULL, aux_map, aux_size, > + basename); > + ralloc_free(basename); > + > + if (!was_mapped) > + blorp->unmap(blorp, &surf->addr); > + if (surf->aux_addr.buffer && !aux_was_mapped) > + blorp->unmap(blorp, &surf->aux_addr); > } > > /** > @@ -1038,25 +1062,50 @@ intel_hiz_exec(struct brw_context *brw, struct > intel_mipmap_tree *mt, > > switch (op) { > case BLORP_HIZ_OP_DEPTH_RESOLVE: > - opname = "depth resolve"; > + opname = "depth-resolve"; > break; > case BLORP_HIZ_OP_HIZ_RESOLVE: > - opname = "hiz ambiguate"; > + opname = "hiz-ambiguate"; > break; > case BLORP_HIZ_OP_DEPTH_CLEAR: > - opname = "depth clear"; > + opname = "depth-clear"; > break; > case BLORP_HIZ_OP_NONE: > - opname = "noop?"; > + opname = "hiz-noop"; > break; > } > > DBG("%s %s to mt %p level %d layer %d\n", > __func__, opname, mt, level, layer); > > + intel_miptree_check_level_layer(mt, level, layer); > + intel_miptree_used_for_rendering(mt); > + > + assert(intel_miptree_level_has_hiz(mt, level)); > + > + struct isl_surf isl_tmp[2]; > + struct blorp_surf surf; > + struct blorp_batch batch; > + const bool need_blorp = brw->gen < 8 || isl_hiz_debug_dump; > + > + if (need_blorp) { > + blorp_surf_for_miptree(brw, &surf, mt, true, (1 << > ISL_AUX_USAGE_HIZ), > + &level, layer, 1, isl_tmp); > + blorp_batch_init(&brw->blorp, &batch, brw, 0); > + } > + > + if (isl_hiz_debug_dump) > + hiz_dump_blorp_surf(batch.blorp, &surf, opname, "before"); > + > if (brw->gen >= 8) { > gen8_hiz_exec(brw, mt, level, layer, op); > } else { > - gen6_blorp_hiz_exec(brw, mt, level, layer, op); > + blorp_gen6_hiz_op(&batch, &surf, level, layer, op); > } > + > + if (isl_hiz_debug_dump) > + hiz_dump_blorp_surf(batch.blorp, &surf, opname, "after"); > + > + if (need_blorp) > + blorp_batch_finish(&batch); > } > -- > 2.10.2 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </div></div></blockquote></div> </div></div>