On Wed, May 31, 2017 at 2:08 AM, Pohjolainen, Topi <
topi.pohjolai...@gmail.com> wrote:
> 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);
>
> 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.
>
Yeah... I'm also not sure how I feel about the proliferation of enums...
> >
> > 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);
>
> 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).
>
I've done one better. I split all those changes out into their own patch:
https://cgit.freedesktop.org/~jekstrand/mesa/commit/?h=wip/i965-resolve-rework-v3&id=eeee565a0c8d02816d55669e405b16334e11567b
> >
> > /* 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
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > 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
