Hello,
On Thu, 7 Feb 2019 15:46:29 -0800
Nanley Chery <nanleych...@gmail.com> wrote:
> > - !(mode & BRW_MAP_DIRECT_BIT)) {
> > + !(mode & BRW_MAP_DIRECT_BIT) &&
> > + !(intel_miptree_needs_fake_etc(brw, mt))) {
> > intel_miptree_map_etc(brw, mt, map, level, slice);
>
> Out of curiosity, is there any reason you wait until patch 5 to delete
> this case?
No, I just removed this lines together with the unreached map/unmap_etc
functions. I will move the change to this patch.
>
> > } else if (mt->stencil_mt && !(mode & BRW_MAP_DIRECT_BIT)) {
> > intel_miptree_map_depthstencil(brw, mt, map, level, slice);
> > @@ -3816,6 +3839,7 @@ intel_miptree_unmap(struct brw_context *brw,
> > unsigned int slice)
> > {
> > struct intel_miptree_map *map =
> > mt->level[level].slice[slice].map;
> > + int level_w, level_h;
> >
> > assert(mt->surf.samples == 1);
> >
> > @@ -3825,10 +3849,20 @@ intel_miptree_unmap(struct brw_context *brw,
> > DBG("%s: mt %p (%s) level %d slice %d\n", __func__,
> > mt, _mesa_get_format_name(mt->format), level, slice);
> >
> > + level_w = minify(mt->surf.phys_level0_sa.width,
> > + level - mt->first_level);
> > + level_h = minify(mt->surf.phys_level0_sa.height,
> > + level - mt->first_level);
> > +
> > if (map->unmap)
> > map->unmap(brw, mt, map, level, slice);
> >
> > intel_miptree_release_map(mt, level, slice);
> > +
> > + if (intel_miptree_has_etc_shadow(brw, mt) &&
> > mt->shadow_needs_update) {
> > + intel_miptree_update_etc_shadow(brw, mt, level, slice,
> > level_w,
> > + level_h);
> > + }
>
> With the next patch applied, the change in this function becomes
> unnecessary. Is there any reason you're leaving it around?
Right, if we force the update before the rendering, we don't need to
copy the data during the unmap. I will remove it, sorry I dismissed
it in the previous email.
> > }
> >
> > enum isl_surf_dim
> > @@ -3943,3 +3977,81 @@ intel_miptree_get_clear_color(const struct
> > gen_device_info *devinfo, return mt->fast_clear_color;
> > }
> > }
> > +
> > +static void
> > +intel_miptree_update_etc_shadow(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt,
> > + unsigned int level,
> > + unsigned int slice,
> > + int level_w,
> > + int level_h)
> > +{
> > + struct intel_mipmap_tree *smt;
> > + ptrdiff_t etc_stride, shadow_stride;
> > + GLbitfield etc_mode, shadow_mode;
> > + void *mptr, *sptr;
> > +
> > + assert(intel_miptree_has_etc_shadow(brw, mt));
> > + if (!mt->shadow_needs_update)
> > + return;
> > +
> > + mt->shadow_needs_update = false;
> > + smt = mt->shadow_mt;
> > +
> > + etc_mode = GL_MAP_READ_BIT;
> > + shadow_mode = GL_MAP_WRITE_BIT | GL_MAP_INVALIDATE_RANGE_BIT;
> > +
> > + intel_miptree_map(brw, mt, level, slice, 0, 0, level_w, level_h,
> > + etc_mode, &mptr, &etc_stride);
> > + intel_miptree_map(brw, smt, level, slice, 0, 0, level_w,
> > level_h,
> > + shadow_mode, &sptr, &shadow_stride);
> > +
> > + if (mt->format == MESA_FORMAT_ETC1_RGB8) {
> > + _mesa_etc1_unpack_rgba8888(sptr, shadow_stride, mptr,
> > etc_stride,
> > + level_w, level_h);
> > + } else {
> > + /* destination and source images must have the same swizzle
> > */
> > + bool is_bgra = (smt->format == MESA_FORMAT_B8G8R8A8_SRGB);
> > + _mesa_unpack_etc2_format(sptr, shadow_stride, mptr,
> > etc_stride,
> > + level_w, level_h, mt->format,
> > is_bgra);
> > + }
> > +
> > + intel_miptree_unmap(brw, mt, level, slice);
> > + intel_miptree_unmap(brw, smt, level, slice);
> > +}
> > +
> > +void
> > +intel_miptree_update_etc_shadow_levels(struct brw_context *brw,
> > + struct intel_mipmap_tree
> > *mt) +{
> > + struct intel_mipmap_tree *smt;
> > + int num_slices;
> > + int level_w, level_h;
> > +
> > + assert(mt);
> > + assert(mt->surf.size_B > 0);
> > +
> > + assert(intel_miptree_has_etc_shadow(brw, mt));
> > +
> > + smt = mt->shadow_mt;
> > +
> > + level_w = smt->surf.logical_level0_px.width;
> > + level_h = smt->surf.logical_level0_px.height;
> > +
> > + num_slices = smt->surf.logical_level0_px.array_len;
> > +
> > + for (int level = smt->first_level; level <= smt->last_level;
> > level++)
> > + {
> > + for (unsigned int slice = 0; slice < num_slices; slice++) {
> > + intel_miptree_update_etc_shadow(brw, mt, level, slice,
> > level_w,
> > + level_h);
> > + }
> > +
> > + level_w = minify(mt->surf.logical_level0_px.width,
> > + level - mt->first_level);
> > + level_h = minify(mt->surf.logical_level0_px.height,
> > + level - mt->first_level);
>
> This function goes from accessing the smt to the mt for the
> logical_level0_px field. It does work, but it is a little surprising.
I will fix that too, it might have worked because the trees have the
same logical_level0_px w, h, but yes the correct thing is to use the
smt here :)
>
> > + }
> > +
> > + mt->shadow_needs_update = false;
> > +}
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h index
> > 1a7507023a1..752aeaaf9b7 100644 ---
> > a/src/mesa/drivers/dri/i965/intel_mipmap_tree.h +++
> > b/src/mesa/drivers/dri/i965/intel_mipmap_tree.h @@ -301,6 +301,8 @@
> > struct intel_mipmap_tree *
> > * This miptree may be used for:
> > * - Stencil texturing (pre-BDW) as required by
> > GL_ARB_stencil_texturing.
> > + * - To store the decompressed ETC/EAC data in case we emulate
> > the ETC
> > + * compression on Gen 7 or earlier GPUs.
> > */
> > struct intel_mipmap_tree *shadow_mt;
> > bool shadow_needs_update;
> > @@ -730,6 +732,28 @@ isl_memcpy_type
> > intel_miptree_get_memcpy_type(mesa_format tiledFormat, GLenum
> > format, GLenum type, uint32_t *cpp);
> >
> > +static inline bool
> > +intel_miptree_needs_fake_etc(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt)
> > +{
> > + const struct gen_device_info *devinfo = &brw->screen->devinfo;
> > + bool is_etc = _mesa_is_format_etc2(mt->format) ||
> > + (mt->format == MESA_FORMAT_ETC1_RGB8);
> > +
> > + return devinfo->gen < 8 && !devinfo->is_baytrail && is_etc;
> > +}
> > +
> > +static inline bool
> > +intel_miptree_has_etc_shadow(struct brw_context *brw,
> > + struct intel_mipmap_tree *mt)
> > +{
> > + return intel_miptree_needs_fake_etc(brw, mt) && mt->shadow_mt;
>
> This function does the same work as the various
> mt->etc_format != MESA_FORMAT_NONE checks we have in i965, but I
> prefer the way you've implemented it here.
Ok, thanks a lot for the review, I'm going to fix the patches and send
the new version.
Regards,
Eleni
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
