Mesa (master): i965/drm: Make brw_bo_alloc_tiled take tiling by value, not pointer.

Kenneth Graunke kwg at kemper.freedesktop.org
Wed Apr 12 04:11:54 UTC 2017


Module: Mesa
Branch: master
Commit: 193601311cb4cb6a7d8f915b09e2b30ff0af0fa7
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=193601311cb4cb6a7d8f915b09e2b30ff0af0fa7

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Mon Apr 10 23:31:20 2017 -0700

i965/drm: Make brw_bo_alloc_tiled take tiling by value, not pointer.

For some reason we passed tiling by pointer, through several layers,
even though the functions only read the initial value, and never
actually change it.  We even had a do-while loop that executed until
the tiling mode matched - except it always did, so it only ran once.
We then had bogus error handling in case it changed the tiling mode
to something nonsensical...which it never did.

Drop all this nonsense.

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

---

 src/mesa/drivers/dri/i965/brw_bufmgr.c        | 70 ++++++++++++---------------
 src/mesa/drivers/dri/i965/brw_bufmgr.h        |  2 +-
 src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 24 +++------
 src/mesa/drivers/dri/i965/intel_screen.c      |  7 ++-
 4 files changed, 41 insertions(+), 62 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index 1c5ad2467a..5243078eda 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -138,10 +138,9 @@ hash_find_bo(struct hash_table *ht, unsigned int key)
 }
 
 static unsigned long
-bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size,
-             uint32_t *tiling_mode)
+bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size, uint32_t tiling)
 {
-   if (*tiling_mode == I915_TILING_NONE)
+   if (tiling == I915_TILING_NONE)
       return size;
 
    /* 965+ just need multiples of page size for tiling */
@@ -154,18 +153,17 @@ bo_tile_size(struct brw_bufmgr *bufmgr, unsigned long size,
  * change.
  */
 static unsigned long
-bo_tile_pitch(struct brw_bufmgr *bufmgr,
-              unsigned long pitch, uint32_t *tiling_mode)
+bo_tile_pitch(struct brw_bufmgr *bufmgr, unsigned long pitch, uint32_t tiling)
 {
    unsigned long tile_width;
 
    /* If untiled, then just align it so that we can do rendering
     * to it with the 3D engine.
     */
-   if (*tiling_mode == I915_TILING_NONE)
+   if (tiling == I915_TILING_NONE)
       return ALIGN(pitch, 64);
 
-   if (*tiling_mode == I915_TILING_X)
+   if (tiling == I915_TILING_X)
       tile_width = 512;
    else
       tile_width = 128;
@@ -378,42 +376,36 @@ brw_bo_alloc(struct brw_bufmgr *bufmgr,
 
 struct brw_bo *
 brw_bo_alloc_tiled(struct brw_bufmgr *bufmgr, const char *name,
-                   int x, int y, int cpp, uint32_t *tiling_mode,
+                   int x, int y, int cpp, uint32_t tiling,
                    unsigned long *pitch, unsigned long flags)
 {
    unsigned long size, stride;
-   uint32_t tiling;
+   unsigned long aligned_y, height_alignment;
 
-   do {
-      unsigned long aligned_y, height_alignment;
-
-      tiling = *tiling_mode;
-
-      /* If we're tiled, our allocations are in 8 or 32-row blocks,
-       * so failure to align our height means that we won't allocate
-       * enough pages.
-       *
-       * If we're untiled, we still have to align to 2 rows high
-       * because the data port accesses 2x2 blocks even if the
-       * bottom row isn't to be rendered, so failure to align means
-       * we could walk off the end of the GTT and fault.  This is
-       * documented on 965, and may be the case on older chipsets
-       * too so we try to be careful.
-       */
-      aligned_y = y;
-      height_alignment = 2;
-
-      if (tiling == I915_TILING_X)
-         height_alignment = 8;
-      else if (tiling == I915_TILING_Y)
-         height_alignment = 32;
-      aligned_y = ALIGN(y, height_alignment);
-
-      stride = x * cpp;
-      stride = bo_tile_pitch(bufmgr, stride, tiling_mode);
-      size = stride * aligned_y;
-      size = bo_tile_size(bufmgr, size, tiling_mode);
-   } while (*tiling_mode != tiling);
+   /* If we're tiled, our allocations are in 8 or 32-row blocks,
+    * so failure to align our height means that we won't allocate
+    * enough pages.
+    *
+    * If we're untiled, we still have to align to 2 rows high
+    * because the data port accesses 2x2 blocks even if the
+    * bottom row isn't to be rendered, so failure to align means
+    * we could walk off the end of the GTT and fault.  This is
+    * documented on 965, and may be the case on older chipsets
+    * too so we try to be careful.
+    */
+   aligned_y = y;
+   height_alignment = 2;
+
+   if (tiling == I915_TILING_X)
+      height_alignment = 8;
+   else if (tiling == I915_TILING_Y)
+      height_alignment = 32;
+   aligned_y = ALIGN(y, height_alignment);
+
+   stride = x * cpp;
+   stride = bo_tile_pitch(bufmgr, stride, tiling);
+   size = stride * aligned_y;
+   size = bo_tile_size(bufmgr, size, tiling);
    *pitch = stride;
 
    if (tiling == I915_TILING_NONE)
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.h b/src/mesa/drivers/dri/i965/brw_bufmgr.h
index b27178b6fe..aa3d40bb95 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.h
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.h
@@ -160,7 +160,7 @@ struct brw_bo *brw_bo_alloc(struct brw_bufmgr *bufmgr, const char *name,
 struct brw_bo *brw_bo_alloc_tiled(struct brw_bufmgr *bufmgr,
                                   const char *name,
                                   int x, int y, int cpp,
-                                  uint32_t *tiling_mode,
+                                  uint32_t tiling_mode,
                                   unsigned long *pitch,
                                   unsigned long flags);
 
diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
index 81e2d6e1e9..137bb075f6 100644
--- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
+++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
@@ -619,12 +619,12 @@ miptree_create(struct brw_context *brw,
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   ALIGN(mt->total_width, 64),
                                   ALIGN(mt->total_height, 64),
-                                  mt->cpp, &mt->tiling, &pitch,
+                                  mt->cpp, mt->tiling, &pitch,
                                   alloc_flags);
    } else {
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   mt->total_width, mt->total_height,
-                                  mt->cpp, &mt->tiling, &pitch,
+                                  mt->cpp, mt->tiling, &pitch,
                                   alloc_flags);
    }
 
@@ -668,7 +668,7 @@ intel_miptree_create(struct brw_context *brw,
       brw_bo_unreference(mt->bo);
       mt->bo = brw_bo_alloc_tiled(brw->bufmgr, "miptree",
                                   mt->total_width, mt->total_height, mt->cpp,
-                                  &mt->tiling, &pitch, alloc_flags);
+                                  mt->tiling, &pitch, alloc_flags);
       mt->pitch = pitch;
    }
 
@@ -1544,7 +1544,6 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
     */
    const uint32_t alloc_flags =
       is_lossless_compressed ? 0 : BO_ALLOC_FOR_RENDER;
-   uint32_t tiling = I915_TILING_Y;
    unsigned long pitch;
 
    /* ISL has stricter set of alignment rules then the drm allocator.
@@ -1553,10 +1552,9 @@ intel_miptree_alloc_non_msrt_mcs(struct brw_context *brw,
     */
    buf->bo = brw_bo_alloc_tiled(brw->bufmgr, "ccs-miptree",
                                 buf->pitch, buf->size / buf->pitch,
-                                1, &tiling, &pitch, alloc_flags);
+                                1, I915_TILING_Y, &pitch, alloc_flags);
    if (buf->bo) {
       assert(pitch == buf->pitch);
-      assert(tiling == I915_TILING_Y);
    } else {
       free(buf);
       return false;
@@ -1687,18 +1685,13 @@ intel_gen7_hiz_buf_create(struct brw_context *brw,
    }
 
    unsigned long pitch;
-   uint32_t tiling = I915_TILING_Y;
    buf->aux_base.bo = brw_bo_alloc_tiled(brw->bufmgr, "hiz",
                                          hz_width, hz_height, 1,
-                                         &tiling, &pitch,
+                                         I915_TILING_Y, &pitch,
                                          BO_ALLOC_FOR_RENDER);
    if (!buf->aux_base.bo) {
       free(buf);
       return NULL;
-   } else if (tiling != I915_TILING_Y) {
-      brw_bo_unreference(buf->aux_base.bo);
-      free(buf);
-      return NULL;
    }
 
    buf->aux_base.size = hz_width * hz_height;
@@ -1784,18 +1777,13 @@ intel_gen8_hiz_buf_create(struct brw_context *brw,
    }
 
    unsigned long pitch;
-   uint32_t tiling = I915_TILING_Y;
    buf->aux_base.bo = brw_bo_alloc_tiled(brw->bufmgr, "hiz",
                                          hz_width, hz_height, 1,
-                                         &tiling, &pitch,
+                                         I915_TILING_Y, &pitch,
                                          BO_ALLOC_FOR_RENDER);
    if (!buf->aux_base.bo) {
       free(buf);
       return NULL;
-   } else if (tiling != I915_TILING_Y) {
-      brw_bo_unreference(buf->aux_base.bo);
-      free(buf);
-      return NULL;
    }
 
    buf->aux_base.size = hz_width * hz_height;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 76e82b03b2..f1e9d660d9 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -614,7 +614,7 @@ intel_create_image_common(__DRIscreen *dri_screen,
 
    cpp = _mesa_get_format_bytes(image->format);
    image->bo = brw_bo_alloc_tiled(screen->bufmgr, "image",
-                                  width, height, cpp, &tiling,
+                                  width, height, cpp, tiling,
                                   &pitch, 0);
    if (image->bo == NULL) {
       free(image);
@@ -1298,7 +1298,7 @@ intel_detect_swizzling(struct intel_screen *screen)
    uint32_t swizzle_mode = 0;
 
    buffer = brw_bo_alloc_tiled(screen->bufmgr, "swizzle test",
-                               64, 64, 4, &tiling, &aligned_pitch, flags);
+                               64, 64, 4, tiling, &aligned_pitch, flags);
    if (buffer == NULL)
       return false;
 
@@ -2097,7 +2097,6 @@ intelAllocateBuffer(__DRIscreen *dri_screen,
    /* The front and back buffers are color buffers, which are X tiled. GEN9+
     * supports Y tiled and compressed buffers, but there is no way to plumb that
     * through to here. */
-   uint32_t tiling = I915_TILING_X;
    unsigned long pitch;
    int cpp = format / 8;
    intelBuffer->bo = brw_bo_alloc_tiled(screen->bufmgr,
@@ -2105,7 +2104,7 @@ intelAllocateBuffer(__DRIscreen *dri_screen,
                                         width,
                                         height,
                                         cpp,
-                                        &tiling, &pitch,
+                                        I915_TILING_X, &pitch,
                                         BO_ALLOC_FOR_RENDER);
 
    if (intelBuffer->bo == NULL) {




More information about the mesa-commit mailing list