[Mesa-dev] [PATCH v2 2/5] i965/miptree: Use cpu tiling/detiling when mapping

Nanley Chery nanleychery at gmail.com
Tue Mar 6 03:02:52 UTC 2018


On Mon, Mar 05, 2018 at 02:40:25PM -0800, Nanley Chery wrote:
> On Thu, Jan 25, 2018 at 08:23:45AM -0800, Scott D Phillips wrote:
> > Rename the (un)map_gtt functions to (un)map_map (map by
> > returning a map) and add new functions (un)map_tiled_memcpy that
> > return a shadow buffer populated with the intel_tiled_memcpy
> > functions.
> > 
> > v2: Compute extents properly in the x|y-rounded-down case (Chris Wilson)
> > ---
> >  src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 95 ++++++++++++++++++++++++---
> >  1 file changed, 86 insertions(+), 9 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > index c480eade93..85297cb0c1 100644
> > --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> > @@ -31,6 +31,7 @@
> >  #include "intel_image.h"
> >  #include "intel_mipmap_tree.h"
> >  #include "intel_tex.h"
> > +#include "intel_tiled_memcpy.h"
> >  #include "intel_blit.h"
> >  #include "intel_fbo.h"
> >  
> > @@ -3028,10 +3029,10 @@ intel_miptree_unmap_raw(struct intel_mipmap_tree *mt)
> >  }
> >  
> >  static void
> > -intel_miptree_map_gtt(struct brw_context *brw,
> > -		      struct intel_mipmap_tree *mt,
> > -		      struct intel_miptree_map *map,
> > -		      unsigned int level, unsigned int slice)
> > +intel_miptree_map_map(struct brw_context *brw,
> > +                      struct intel_mipmap_tree *mt,
> > +                      struct intel_miptree_map *map,
> > +                      unsigned int level, unsigned int slice)
> >  {
> >     unsigned int bw, bh;
> >     void *base;
> > @@ -3049,7 +3050,7 @@ intel_miptree_map_gtt(struct brw_context *brw,
> >     y /= bh;
> >     x /= bw;
> >  
> > -   base = intel_miptree_map_raw(brw, mt, map->mode);
> > +   base = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
> >  
> >     if (base == NULL)
> >        map->ptr = NULL;
> > @@ -3075,11 +3076,80 @@ intel_miptree_map_gtt(struct brw_context *brw,
> >  }
> >  
> >  static void
> > -intel_miptree_unmap_gtt(struct intel_mipmap_tree *mt)
> > +intel_miptree_unmap_map(struct intel_mipmap_tree *mt)
> >  {
> >     intel_miptree_unmap_raw(mt);
> >  }
> >  
> > +/* Compute extent parameters for use with tiled_memcpy functions.
> > + * xs are in units of bytes and ys are in units of strides. */
> > +static inline void
> > +tile_extents(struct intel_mipmap_tree *mt, struct intel_miptree_map *map,
> > +             unsigned int level, unsigned int slice, unsigned int *x1,
> > +             unsigned int *x2, unsigned int *y1, unsigned int *y2)
> > +{
> > +   unsigned int block_width, block_height, block_bytes;
> > +   unsigned int x0_el, y0_el;
> > +
> > +   _mesa_get_format_block_size(mt->format, &block_width, &block_height);
> > +   block_bytes = _mesa_get_format_bytes(mt->format);
> > +
> > +   assert(map->x % block_width == 0);
> > +   assert(map->y % block_height == 0);
> > +
> > +   intel_miptree_get_image_offset(mt, level, slice, &x0_el, &y0_el);
> > +   *x1 = (map->x / block_width + x0_el) * block_bytes;
> > +   *y1 = map->y / block_height + y0_el;
> > +   *x2 = (DIV_ROUND_UP(map->x + map->w, block_width) + x0_el) * block_bytes;
> > +   *y2 = DIV_ROUND_UP(map->y + map->h, block_height) + y0_el;
> > +}
> > +
> > +static void
> > +intel_miptree_map_tiled_memcpy(struct brw_context *brw,
> > +                               struct intel_mipmap_tree *mt,
> > +                               struct intel_miptree_map *map,
> > +                               unsigned int level, unsigned int slice)
> > +{
> > +   unsigned int x1, x2, y1, y2;
> > +   tile_extents(mt, map, level, slice, &x1, &x2, &y1, &y2);
> > +   map->stride = _mesa_format_row_stride(mt->format, map->w);
> > +   map->buffer = map->ptr = malloc(map->stride * (y2 - y1));
> > +
> > +   if (!(map->mode & GL_MAP_INVALIDATE_RANGE_BIT)) {
> > +      char *src = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
> > +      src += mt->offset;
> > +
> > +      tiled_to_linear(x1, x2, y1, y2, map->ptr, src, map->stride,
> > +                      mt->surf.row_pitch, brw->has_swizzling, mt->surf.tiling,
> > +                      memcpy);
> > +
> 
> Do we have a mechanism to test if a miptree level and slice are valid?
> It doesn't seem like the mode have the GL_MAP_INVALIDATE_RANGE_BIT set
> in all the cases we care about.
> 
> For example, it looks like we'll do an extra memcpy operation when the
> user specifies an image for the first time (using glTexImage2D for
> example).
> 
> It looks like there are unknown perf implications with this path. How do
> you feel about putting this behind a debug flag?
> 

I just talked about this with Jason and it seems like the lack of
GL_MAP_INVALIDATE_RANGE_BIT being set may be a bug in core mesa. Also
from that discussion, it seems like we want to move away from gtt
mapping in release builds for more than just performance reasons, so
never mind the bit about the debug flag.

-Nanley

> -Nanley
> 
> > +      intel_miptree_unmap_raw(mt);
> > +   }
> > +}
> > +
> > +static void
> > +intel_miptree_unmap_tiled_memcpy(struct brw_context *brw,
> > +                                 struct intel_mipmap_tree *mt,
> > +                                 struct intel_miptree_map *map,
> > +                                 unsigned int level,
> > +                                 unsigned int slice)
> > +{
> > +   if (map->mode & GL_MAP_WRITE_BIT) {
> > +      unsigned int x1, x2, y1, y2;
> > +      tile_extents(mt, map, level, slice, &x1, &x2, &y1, &y2);
> > +
> > +      char *dst = intel_miptree_map_raw(brw, mt, map->mode | MAP_RAW);
> > +      dst += mt->offset;
> > +
> > +      linear_to_tiled(x1, x2, y1, y2, dst, map->ptr, mt->surf.row_pitch,
> > +                      map->stride, brw->has_swizzling, mt->surf.tiling, memcpy);
> > +
> > +      intel_miptree_unmap_raw(mt);
> > +   }
> > +   free(map->buffer);
> > +   map->buffer = map->ptr = NULL;
> > +}
> > +
> >  static void
> >  intel_miptree_map_blit(struct brw_context *brw,
> >  		       struct intel_mipmap_tree *mt,
> > @@ -3637,8 +3707,10 @@ intel_miptree_map(struct brw_context *brw,
> >                (mt->surf.row_pitch % 16 == 0)) {
> >        intel_miptree_map_movntdqa(brw, mt, map, level, slice);
> >  #endif
> > +   } else if (mt->surf.tiling != ISL_TILING_LINEAR) {
> > +      intel_miptree_map_tiled_memcpy(brw, mt, map, level, slice);
> >     } else {
> > -      intel_miptree_map_gtt(brw, mt, map, level, slice);
> > +      intel_miptree_map_map(brw, mt, map, level, slice);
> >     }
> >  
> >     *out_ptr = map->ptr;
> > @@ -3674,11 +3746,16 @@ intel_miptree_unmap(struct brw_context *brw,
> >     } else if (map->linear_mt) {
> >        intel_miptree_unmap_blit(brw, mt, map, level, slice);
> >  #if defined(USE_SSE41)
> > -   } else if (map->buffer && cpu_has_sse4_1) {
> > +   } else if (!(map->mode & GL_MAP_WRITE_BIT) &&
> > +              !mt->compressed && cpu_has_sse4_1 &&
> > +              (mt->surf.row_pitch % 16 == 0) &&
> > +              map->buffer) {
> >        intel_miptree_unmap_movntdqa(brw, mt, map, level, slice);
> >  #endif
> > +   } else if (mt->surf.tiling != ISL_TILING_LINEAR) {
> > +      intel_miptree_unmap_tiled_memcpy(brw, mt, map, level, slice);
> >     } else {
> > -      intel_miptree_unmap_gtt(mt);
> > +      intel_miptree_unmap_map(mt);
> >     }
> >  
> >     intel_miptree_release_map(mt, level, slice);
> > -- 
> > 2.16.1
> > 
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list