<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jun 14, 2017 at 12:55 PM, 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"><div class="HOEnZb"><div class="h5">On Wed, Jun 14, 2017 at 10:18:18AM +0300, Pohjolainen, Topi wrote: > On Tue, Jun 13, 2017 at 04:20:02PM -0700, Jason Ekstrand wrote: > > On Tue, Jun 13, 2017 at 4:14 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > wrote: > > > > > On Tue, Jun 13, 2017 at 7:53 AM, Topi Pohjolainen < > > > <a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a>> wrote: > > > > > >> Signed-off-by: Topi Pohjolainen <<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a>> > > >> --- > > >> src/mesa/drivers/dri/i965/brw_blorp.c | 9 +++-- > > >> src/mesa/drivers/dri/i965/gen6_depth_state.c | 12 +++---- > > >> src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 50 > > >> ++++++++++++++------------- > > >> src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 7 ++-- > > >> 4 files changed, 39 insertions(+), 39 deletions(-) > > >> > > >> diff --git a/src/mesa/drivers/dri/i965/brw_blorp.c > > >> b/src/mesa/drivers/dri/i965/brw_blorp.c > > >> index 4bc53b76b5..b722454703 100644 > > >> --- a/src/mesa/drivers/dri/i965/brw_blorp.c > > >> +++ b/src/mesa/drivers/dri/i965/brw_blorp.c > > >> @@ -165,8 +165,13 @@ blorp_surf_for_miptree(struct brw_context *brw, > > >> > > >> surf->aux_usage = intel_miptree_get_aux_isl_usage(brw, mt); > > >> > > >> - struct isl_surf *aux_surf = &tmp_surfs[1]; > > >> - intel_miptree_get_aux_isl_surf(brw, mt, surf->aux_usage, aux_surf); > > >> + struct isl_surf *aux_surf; > > >> + if (brw->gen == 6 && mt->hiz_buf) { > > >> + aux_surf = &mt->hiz_buf->aux_base.surf; > > >> + } else { > > >> + aux_surf = &tmp_surfs[1]; > > >> + intel_miptree_get_aux_isl_surf(brw, mt, surf->aux_usage, > > >> aux_surf); > > >> > > > > > > This is a bit awkward. Maybe just make intel_miptree_get_aux_isl_surf > > > return the surf from hiz_buf on gen6? Not that it matters much since I > > > have a feeling this is all going away in the future. > > I'd like to keep intel_miptree_get_aux_isl_surf() unchanged, I'm throwing it > out later and it is clearer when I don't need to move anything back from it. > > > > > > > > > >> + } > > >> > > >> if (wants_resolve) { > > >> bool supports_aux = surf->aux_usage != ISL_AUX_USAGE_NONE && > > >> diff --git a/src/mesa/drivers/dri/i965/gen6_depth_state.c > > >> b/src/mesa/drivers/dri/i965/gen6_depth_state.c > > >> index 0d8785db65..0f5e4d3201 100644 > > >> --- a/src/mesa/drivers/dri/i965/gen6_depth_state.c > > >> +++ b/src/mesa/drivers/dri/i965/gen6_depth_state.c > > >> @@ -165,18 +165,14 @@ gen6_emit_depth_stencil_hiz(struct brw_context > > >> *brw, > > >> /* Emit hiz buffer. */ > > >> if (hiz) { > > >> assert(depth_mt); > > >> - struct intel_mipmap_tree *hiz_mt = depth_mt->hiz_buf->mt; > > >> > > >> - assert(hiz_mt->array_layout == GEN6_HIZ_STENCIL); > > >> - > > >> - const uint32_t offset = intel_miptree_get_aligned_offset( > > >> - hiz_mt, > > >> - hiz_mt->level[lod].level_x, > > >> - hiz_mt->level[lod].level_y); > > >> + uint32_t offset; > > >> + isl_surf_get_image_offset_B_tile_sa(&depth_mt->hiz_buf->aux > > >> _base.surf, > > >> + lod, 0, 0, &offset, NULL, > > >> NULL); > > >> > > >> BEGIN_BATCH(3); > > >> OUT_BATCH((_3DSTATE_HIER_DEPTH_BUFFER << 16) | (3 - 2)); > > >> - OUT_BATCH(depth_mt->hiz_buf->aux_base.pitch - 1); > > >> + OUT_BATCH(depth_mt->hiz_buf->aux_base.surf.row_pitch - 1); > > >> OUT_RELOC(depth_mt->hiz_buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a>, > > >> I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER, > > >> offset); > > >> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > >> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > >> index d87dbfaacd..4dbf853eee 100644 > > >> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > >> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > > >> @@ -1058,10 +1058,7 @@ intel_miptree_hiz_buffer_free(struct > > >> intel_miptree_hiz_buffer *hiz_buf) > > >> if (hiz_buf == NULL) > > >> return; > > >> > > >> - if (hiz_buf->mt) > > >> - intel_miptree_release(&hiz_buf->mt); > > >> - else > > >> - brw_bo_unreference(hiz_buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a>); > > >> + brw_bo_unreference(hiz_buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a>); > > >> > > >> free(hiz_buf); > > >> } > > >> @@ -2007,34 +2004,39 @@ intel_hiz_miptree_buf_create(struct brw_context > > >> *brw, > > >> struct intel_mipmap_tree *mt) > > >> { > > >> struct intel_miptree_hiz_buffer *buf = calloc(sizeof(*buf), 1); > > >> - uint32_t layout_flags = MIPTREE_LAYOUT_ACCELERATED_UPLOAD; > > >> + if (!buf) > > >> + return NULL; > > >> > > >> - if (brw->gen == 6) > > >> - layout_flags |= MIPTREE_LAYOUT_GEN6_HIZ_STENCIL; > > >> + struct isl_surf_init_info init_info = { > > >> + .dim = get_isl_surf_dim(mt->target), > > >> + .format = ISL_FORMAT_HIZ, > > >> + .width = mt->logical_width0, > > >> + .height = mt->logical_height0, > > >> + .depth = mt->target == GL_TEXTURE_3D ? mt->logical_depth0 : 1, > > >> + .levels = mt->last_level - mt->first_level + 1, > > >> + .array_len = mt->target == GL_TEXTURE_3D ? 1 : mt->logical_depth0, > > >> + .samples = MAX2(mt->num_samples, 1), > > >> + .usage = ISL_SURF_USAGE_HIZ_BIT, > > >> + .tiling_flags = ISL_TILING_HIZ_BIT > > >> > > > > > Is there some reason why you can't just use miptree_get_isl_surf() and > > isl_surf_get_hiz_surf()? That seems like the more straightforward path. > > This is a good call as well. This was needed when I had to create layout > for level zero only and tweak it later on. Now that you gave me proper support > in ISL, this is not needed anymore. </div></div>I would need to create temporary main surface though in order to use isl_surf_get_hiz_surf(). At this point I don't have it yet. Would you be okay changing this after the series that changes depth to use isl? <div class="HOEnZb"><div class="h5"></div></div></blockquote><div> </div><div>I'd prefer the temp but I guess I won't insist too hard. I would just feel better if all HiZ surface creation were going to isl_surf_get_hiz_surf(). </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > > > > > > > > + }; > > >> > > >> - if (!buf) > > >> + if (!isl_surf_init_s(&brw->isl_dev, &buf->aux_base.surf, > > >> &init_info)) { > > >> + free(buf); > > >> return NULL; > > >> + } > > >> > > >> - layout_flags |= MIPTREE_LAYOUT_TILING_ANY; > > >> - buf->mt = intel_miptree_create(brw, > > >> - mt->target, > > >> - mt->format, > > >> - mt->first_level, > > >> - mt->last_level, > > >> - mt->logical_width0, > > >> - mt->logical_height0, > > >> - mt->logical_depth0, > > >> - mt->num_samples, > > >> - layout_flags); > > >> - if (!buf->mt) { > > >> + struct isl_surf *surf = &buf->aux_base.surf; > > >> + unsigned pitch = surf->row_pitch; > > >> + buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a> = brw_bo_alloc_tiled(brw->bufmgr, "hiz", > > >> surf->row_pitch, > > >> + surf->size / surf->row_pitch, 1, > > >> + I915_TILING_Y, &pitch, > > >> + BO_ALLOC_FOR_RENDER); > > >> > > > > > I just sent 3 patches to the list which make this less painful. :-) > > > > > > > + if (!buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a>) { > > >> free(buf); > > >> return NULL; > > >> } > > >> > > >> - buf-><a href="http://aux_base.bo" rel="noreferrer" target="_blank">aux_base.bo</a> = buf->mt->bo; > > >> - buf->aux_base.size = buf->mt->total_height * buf->mt->pitch; > > >> - buf->aux_base.pitch = buf->mt->pitch; > > >> - buf->aux_base.qpitch = buf->mt->qpitch * 2; > > >> + assert(pitch == surf->row_pitch); > > >> > > >> return buf; > > >> } > > >> diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > >> b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > >> index 37d6bb3813..9f23088d7b 100644 > > >> --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > >> +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > > >> @@ -307,6 +307,8 @@ enum intel_aux_disable { > > >> */ > > >> struct intel_miptree_aux_buffer > > >> { > > >> + struct isl_surf surf; > > >> + > > >> /** > > >> * Buffer object containing the pixel data. > > >> * > > >> @@ -360,11 +362,6 @@ struct intel_miptree_aux_buffer > > >> struct intel_miptree_hiz_buffer > > >> { > > >> struct intel_miptree_aux_buffer aux_base; > > >> - > > >> - /** > > >> - * Hiz miptree. Used only by Gen6. > > >> - */ > > >> - struct intel_mipmap_tree *mt; > > >> }; > > >> > > >> struct intel_mipmap_tree > > >> -- > > >> 2.11.0 > > >> > > >> _______________________________________________ > > >> 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>