<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, May 31, 2017 at 2:08 AM, Pohjolainen, Topi <<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5">On Fri, May 26, 2017 at 04:30:20PM -0700, Jason Ekstrand wrote: > --- > src/mesa/drivers/dri/i965/brw_context.c | 2 +- > src/mesa/drivers/dri/i965/intel_blit.c | 14 ++++---------- > src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 12 +++++------- > src/mesa/drivers/dri/i965/intel_mipmap_tree.h | 18 ++++++++++++++++++ > src/mesa/drivers/dri/i965/intel_pixel_bitmap.c | 2 +- > src/mesa/drivers/dri/i965/intel_pixel_read.c | 2 +- > src/mesa/drivers/dri/i965/intel_tex_image.c | 7 ++++--- > src/mesa/drivers/dri/i965/intel_tex_subimage.c | 7 ++++--- > 8 files changed, 38 insertions(+), 26 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c > index 7840217..593b325 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -1390,7 +1390,7 @@ intel_resolve_for_dri2_flush(struct brw_context *brw, > 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, 1, 0, 1, 0); > + intel_miptree_prepare_access(brw, rb->mt, 0, 1, 0, 1, false, false); > } 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 4d68fcf..5fa43b7 100644 > --- a/src/mesa/drivers/dri/i965/intel_blit.c > +++ b/src/mesa/drivers/dri/i965/intel_blit.c > @@ -325,11 +325,8 @@ intel_miptree_blit(struct brw_context *brw, > /* The blitter has no idea about HiZ or fast color clears, so we need to > * resolve the miptrees before we do anything. > */ > - 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, src_level, 1, src_slice, 1, 0); > - intel_miptree_resolve_color(brw, dst_mt, dst_level, 1, dst_slice, 1, 0); > - intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level, dst_slice); > + intel_miptree_access_raw(brw, src_mt, src_level, src_slice, false); > + intel_miptree_access_raw(brw, dst_mt, dst_level, dst_slice, true); > > if (src_flip) > src_y = minify(src_mt->physical_height0, src_level - src_mt->first_level) - src_y - height; > @@ -384,11 +381,8 @@ intel_miptree_copy(struct brw_context *brw, > /* The blitter has no idea about HiZ or fast color clears, so we need to > * resolve the miptrees before we do anything. > */ > - 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, src_level, 1, src_slice, 1, 0); > - intel_miptree_resolve_color(brw, dst_mt, dst_level, 1, dst_slice, 1, 0); > - intel_miptree_slice_set_needs_hiz_resolve(dst_mt, dst_level, dst_slice); > + intel_miptree_access_raw(brw, src_mt, src_level, src_slice, false); > + intel_miptree_access_raw(brw, dst_mt, dst_level, dst_slice, true); > > uint32_t src_image_x, src_image_y; > intel_miptree_get_image_offset(src_mt, src_level, src_slice, > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > index 0659e75..195d5e6 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c > @@ -2390,8 +2390,10 @@ intel_miptree_make_shareable(struct brw_context *brw, > */ > assert(mt->msaa_layout == INTEL_MSAA_LAYOUT_NONE || mt->num_samples <= 1); > > + intel_miptree_prepare_access(brw, mt, 0, INTEL_REMAINING_LEVELS, > + 0, INTEL_REMAINING_LAYERS, false, false); > + > if (mt->mcs_buf) { > - intel_miptree_all_slices_resolve_color(brw, mt, 0); > mt->aux_disable |= (INTEL_AUX_DISABLE_CCS | INTEL_AUX_DISABLE_MCS); > brw_bo_unreference(mt->mcs_buf->bo); > free(mt->mcs_buf); > @@ -2406,7 +2408,6 @@ intel_miptree_make_shareable(struct brw_context *brw, > > if (mt->hiz_buf) { > mt->aux_disable |= INTEL_AUX_DISABLE_HIZ; > - intel_miptree_all_slices_resolve_depth(brw, mt); > intel_miptree_hiz_buffer_free(mt->hiz_buf); > mt->hiz_buf = NULL; > > @@ -3179,11 +3180,8 @@ intel_miptree_map(struct brw_context *brw, > return; > } > > - intel_miptree_resolve_color(brw, mt, level, 1, slice, 1, 0); > - intel_miptree_slice_resolve_depth(brw, mt, level, slice); > - if (map->mode & GL_MAP_WRITE_BIT) { > - intel_miptree_slice_set_needs_hiz_resolve(mt, level, slice); > - } > + intel_miptree_access_raw(brw, mt, level, slice, > + map->mode & GL_MAP_WRITE_BIT); </div></div>This is the only case where the given boolean is clear what it means without knowing the function signature by heart. I was about to comment in the previous patch that using "uint32_t flags" and properly named enum values instead of booleans document the call sites better. <div><div class="gmail-h5"></div></div></blockquote><div> </div><div>Yeah... I'm also not sure how I feel about the proliferation of enums... </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5"> > > if (mt->format == MESA_FORMAT_S_UINT8) { > intel_miptree_map_s8(brw, mt, map, level, slice); > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > index e74fca6..aadb8f5 100644 > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h > @@ -992,6 +992,24 @@ intel_miptree_set_aux_state(struct brw_context *brw, > uint32_t start_layer, uint32_t num_layers, > enum isl_aux_state aux_state); > > +/** > + * Prepare a miptree for raw access > + * > + * This helper prepares the miptree for access that knows nothing about any > + * sort of compression whatsoever. This is useful when mapping the surface or > + * using it with the blitter. > + */ > +static inline void > +intel_miptree_access_raw(struct brw_context *brw, > + struct intel_mipmap_tree *mt, > + uint32_t level, uint32_t layer, > + bool write) > +{ > + intel_miptree_prepare_access(brw, mt, level, 1, layer, 1, false, false); > + if (write) > + intel_miptree_finish_write(brw, mt, level, layer, 1, false); > +} > + > void > intel_miptree_make_shareable(struct brw_context *brw, > struct intel_mipmap_tree *mt); > diff --git a/src/mesa/drivers/dri/i965/intel_pixel_bitmap.c b/src/mesa/drivers/dri/i965/intel_pixel_bitmap.c > index 4522d28..af94a1c 100644 > --- a/src/mesa/drivers/dri/i965/intel_pixel_bitmap.c > +++ b/src/mesa/drivers/dri/i965/intel_pixel_bitmap.c > @@ -256,7 +256,7 @@ do_blit_bitmap( struct gl_context *ctx, > /* The blitter has no idea about fast color clears, so we need to resolve > * the miptree before we do anything. > */ > - intel_miptree_all_slices_resolve_color(brw, irb->mt, 0); > + intel_miptree_access_raw(brw, irb->mt, irb->mt_level, irb->mt_layer, true); </div></div>It would be nice to have a word or two in the commit message about now resolving only the needed slice instead of all (here and the in the cases further down). <div><div class="gmail-h5"></div></div></blockquote><div> </div><div>I've done one better. I split all those changes out into their own patch: <a href="https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-resolve-rework-v3&id=eeee565a0c8d02816d55669e405b16334e11567b">https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-resolve-rework-v3&id=eeee565a0c8d02816d55669e405b16334e11567b</a> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="gmail-h5"> > > /* Chop it all into chunks that can be digested by hardware: */ > for (py = 0; py < height; py += DY) { > diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c > index 8793c3e..e583f1d 100644 > --- a/src/mesa/drivers/dri/i965/intel_pixel_read.c > +++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c > @@ -138,7 +138,7 @@ intel_readpixels_tiled_memcpy(struct gl_context * ctx, > /* Since we are going to read raw data to the miptree, we need to resolve > * any pending fast color clears before we start. > */ > - intel_miptree_all_slices_resolve_color(brw, irb->mt, 0); > + intel_miptree_access_raw(brw, irb->mt, irb->mt_level, irb->mt_layer, false); > > bo = irb->mt->bo; > > diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c > index 7208d8e..2c3b6f2 100644 > --- a/src/mesa/drivers/dri/i965/intel_tex_image.c > +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c > @@ -520,10 +520,13 @@ intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx, > return false; > } > > + int level = texImage->Level + texImage->TexObject->MinLevel; > + > /* Since we are going to write raw data to the miptree, we need to resolve > * any pending fast color clears before we start. > */ > - intel_miptree_all_slices_resolve_color(brw, image->mt, 0); > + assert(image->mt->logical_depth0 == 1); > + intel_miptree_access_raw(brw, image->mt, level, 0, true); > > bo = image->mt->bo; > > @@ -548,8 +551,6 @@ intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx, > packing->Alignment, packing->RowLength, packing->SkipPixels, > packing->SkipRows); > > - int level = texImage->Level + texImage->TexObject->MinLevel; > - > /* Adjust x and y offset based on miplevel */ > xoffset += image->mt->level[level].level_x; > yoffset += image->mt->level[level].level_y; > diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c b/src/mesa/drivers/dri/i965/intel_tex_subimage.c > index 9126222..29d8a3f 100644 > --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c > +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c > @@ -136,10 +136,13 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx, > return false; > } > > + int level = texImage->Level + texImage->TexObject->MinLevel; > + > /* Since we are going to write raw data to the miptree, we need to resolve > * any pending fast color clears before we start. > */ > - intel_miptree_all_slices_resolve_color(brw, image->mt, 0); > + assert(image->mt->logical_depth0 == 1); > + intel_miptree_access_raw(brw, image->mt, level, 0, true); > > bo = image->mt->bo; > > @@ -168,8 +171,6 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx, > packing->Alignment, packing->RowLength, packing->SkipPixels, > packing->SkipRows, for_glTexImage); > > - int level = texImage->Level + texImage->TexObject->MinLevel; > - > /* Adjust x and y offset based on miplevel */ > xoffset += image->mt->level[level].level_x; > yoffset += image->mt->level[level].level_y; > -- > 2.5.0.400.gff86faf > </div></div>> _______________________________________________ > 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> </blockquote></div> </div></div>