[02/12] panfrost: Combine has_afbc/tiled in layout enum

Submitted by Alyssa Rosenzweig on March 10, 2019, 6:50 a.m.

Details

Message ID 20190310065026.3994-3-alyssa@rosenzweig.io
State New
Headers show
Series "Refactors related to BO layouts" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Alyssa Rosenzweig March 10, 2019, 6:50 a.m.
AFBC, tiled, and linear BO layouts are mutually exclusive; they should
be coupled via a single enum rather than ad hoc checks of booleans.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
---
 src/gallium/drivers/panfrost/pan_context.c  | 33 ++++++++++++++------
 src/gallium/drivers/panfrost/pan_resource.c | 34 ++++++++++++++++-----
 src/gallium/drivers/panfrost/pan_resource.h | 20 +++++++++---
 3 files changed, 64 insertions(+), 23 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 4c41969fd05..630c41fbf1e 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -119,7 +119,7 @@  panfrost_enable_afbc(struct panfrost_context *ctx, struct panfrost_resource *rsr
                                (rsrc->bo->afbc_metadata_size + main_size + 4095) / 4096,
                                true, 0, 0, 0);
 
-        rsrc->bo->has_afbc = true;
+        rsrc->bo->layout = PAN_AFBC;
 
         /* Compressed textured reads use a tagged pointer to the metadata */
 
@@ -153,7 +153,7 @@  panfrost_set_fragment_afbc(struct panfrost_context *ctx)
                 struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.cbufs[cb]->texture;
 
                 /* Non-AFBC is the default */
-                if (!rsrc->bo->has_afbc)
+                if (rsrc->bo->layout != PAN_AFBC)
                         continue;
 
                 if (require_sfbd) {
@@ -179,7 +179,7 @@  panfrost_set_fragment_afbc(struct panfrost_context *ctx)
         if (ctx->pipe_framebuffer.zsbuf) {
                 struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture;
 
-                if (rsrc->bo->has_afbc) {
+                if (rsrc->bo->layout == PAN_AFBC) {
                         if (require_sfbd) {
                                 fprintf(stderr, "Depth AFBC not supported on SFBD\n");
                                 assert(0);
@@ -2244,6 +2244,23 @@  panfrost_create_sampler_view(
 
         enum mali_format format = panfrost_find_format(desc);
 
+        unsigned usage2_layout = 0x10;
+
+        switch (prsrc->bo->layout) {
+                case PAN_AFBC:
+                        usage2_layout |= 0xc;
+                        break;
+                case PAN_TILED:
+                        usage2_layout |= 0x1;
+                        break;
+                case PAN_LINEAR:
+                        usage2_layout |= 0x2;
+                        break;
+                default:
+                        assert(0);
+                        break;
+        }
+
         struct mali_texture_descriptor texture_descriptor = {
                 .width = MALI_POSITIVE(texture->width0),
                 .height = MALI_POSITIVE(texture->height0),
@@ -2257,11 +2274,7 @@  panfrost_create_sampler_view(
                         .usage1 = 0x0,
                         .is_not_cubemap = 1,
 
-                        /* 0x11 - regular texture 2d, uncompressed tiled */
-                        /* 0x12 - regular texture 2d, uncompressed linear */
-                        /* 0x1c - AFBC compressed (internally tiled, probably) texture 2D */
-
-                        .usage2 = prsrc->bo->has_afbc ? 0x1c : (prsrc->bo->tiled ? 0x11 : 0x12),
+                        .usage2 = usage2_layout
                 },
 
                 .swizzle = panfrost_translate_swizzle_4(user_swizzle)
@@ -2353,7 +2366,7 @@  panfrost_set_framebuffer_state(struct pipe_context *pctx,
                 struct panfrost_resource *tex = ((struct panfrost_resource *) ctx->pipe_framebuffer.cbufs[i]->texture);
                 bool is_scanout = panfrost_is_scanout(ctx);
 
-                if (!is_scanout && !tex->bo->has_afbc) {
+                if (!is_scanout && tex->bo->layout != PAN_AFBC) {
                         /* The blob is aggressive about enabling AFBC. As such,
                          * it's pretty much necessary to use it here, since we
                          * have no traces of non-compressed FBO. */
@@ -2387,7 +2400,7 @@  panfrost_set_framebuffer_state(struct pipe_context *pctx,
 
                                 struct panfrost_resource *tex = ((struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture);
 
-                                if (!tex->bo->has_afbc && !panfrost_is_scanout(ctx))
+                                if (tex->bo->layout != PAN_AFBC && !panfrost_is_scanout(ctx))
                                         panfrost_enable_afbc(ctx, tex, true);
                         }
                 }
diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
index 7dfeb773d8b..0cebbdb6e51 100644
--- a/src/gallium/drivers/panfrost/pan_resource.c
+++ b/src/gallium/drivers/panfrost/pan_resource.c
@@ -195,10 +195,28 @@  panfrost_create_bo(struct panfrost_screen *screen, const struct pipe_resource *t
         if (template->height0) sz *= template->height0;
         if (template->depth0) sz *= template->depth0;
 
+        /* Based on the usage, figure out what storing will be used. There are
+         * various tradeoffs:
+         *
+         * Linear: the basic format, bad for memory bandwidth, bad for cache
+         * use. Zero-copy, though. Renderable.
+         *
+         * Tiled: Not compressed, but cache-optimized. Expensive to write into
+         * (due to software tiling), but cheap to sample from. Ideal for most
+         * textures. 
+         *
+         * AFBC: Compressed and renderable (so always desirable for non-scanout
+         * rendertargets). Cheap to sample from. The format is black box, so we
+         * can't read/write from software.
+         */
+
         /* Tiling textures is almost always faster, unless we only use it once */
-        bo->tiled = (template->usage != PIPE_USAGE_STREAM) && (template->bind & PIPE_BIND_SAMPLER_VIEW);
+        bool should_tile = (template->usage != PIPE_USAGE_STREAM) && (template->bind & PIPE_BIND_SAMPLER_VIEW);
+
+        /* Set the layout appropriately */
+        bo->layout = should_tile ? PAN_TILED : PAN_LINEAR;
 
-        if (bo->tiled) {
+        if (bo->layout == PAN_TILED) {
                 /* For tiled, we don't map directly, so just malloc any old buffer */
 
                 for (int l = 0; l < (template->last_level + 1); ++l) {
@@ -291,7 +309,7 @@  panfrost_destroy_bo(struct panfrost_screen *screen, struct panfrost_bo *pbo)
                 }
         }
 
-        if (bo->tiled) {
+        if (bo->layout == PAN_TILED) {
                 /* Tiled has a malloc'd CPU, so just plain ol' free needed */
 
                 for (int l = 0; l < MAX_MIP_LEVELS; ++l) {
@@ -299,7 +317,7 @@  panfrost_destroy_bo(struct panfrost_screen *screen, struct panfrost_bo *pbo)
                 }
         }
 
-        if (bo->has_afbc) {
+        if (bo->layout == PAN_AFBC) {
                 /* TODO */
                 printf("--leaking afbc (%d bytes)--\n", bo->afbc_metadata_size);
         }
@@ -338,8 +356,8 @@  panfrost_map_bo(struct panfrost_context *ctx, struct pipe_transfer *transfer)
         /* If non-zero level, it's a mipmapped resource and needs to be treated as such */
         bo->is_mipmap |= transfer->level;
 
-        if (transfer->usage & PIPE_TRANSFER_MAP_DIRECTLY && bo->tiled) {
-                /* We cannot directly map tiled textures */
+        if (transfer->usage & PIPE_TRANSFER_MAP_DIRECTLY && bo->layout != PAN_LINEAR) {
+                /* We can only directly map linear resources */
                 return NULL;
         }
 
@@ -444,9 +462,9 @@  panfrost_unmap_bo(struct panfrost_context *ctx,
                         struct panfrost_resource *prsrc = (struct panfrost_resource *) transfer->resource;
 
                         /* Gallium thinks writeback happens here; instead, this is our cue to tile */
-                        if (bo->has_afbc) {
+                        if (bo->layout == PAN_AFBC) {
                                 printf("Warning: writes to afbc surface can't possibly work out well for you...\n");
-                        } else if (bo->tiled) {
+                        } else if (bo->layout == PAN_TILED) {
                                 struct pipe_context *gallium = (struct pipe_context *) ctx;
                                 struct panfrost_screen *screen = pan_screen(gallium->screen);
                                 panfrost_tile_texture(screen, prsrc, transfer->level);
diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h
index b0b5cc15a0c..7e50e4190c5 100644
--- a/src/gallium/drivers/panfrost/pan_resource.h
+++ b/src/gallium/drivers/panfrost/pan_resource.h
@@ -31,6 +31,14 @@ 
 #include "pan_allocate.h"
 #include "drm-uapi/drm.h"
 
+/* Describes the memory layout of a BO */
+
+enum panfrost_memory_layout {
+        PAN_LINEAR,
+        PAN_TILED,
+        PAN_AFBC
+};
+
 struct panfrost_bo {
         /* Address to the BO in question */
 
@@ -51,21 +59,23 @@  struct panfrost_bo {
         /* Number of bytes of the imported allocation */
         size_t imported_size;
 
-        /* Set for tiled, clear for linear. */
-        bool tiled;
+        /* Internal layout (tiled?) */
+        enum panfrost_memory_layout layout;
 
         /* Is something other than level 0 ever written? */
         bool is_mipmap;
 
         /* If AFBC is enabled for this resource, we lug around an AFBC
          * metadata buffer as well. The actual AFBC resource is also in
-         * afbc_slab (only defined for AFBC) at position afbc_main_offset */
+         * afbc_slab (only defined for AFBC) at position afbc_main_offset
+         */
 
-        bool has_afbc;
         struct panfrost_memory afbc_slab;
         int afbc_metadata_size;
 
-        /* Similarly for TE */
+        /* If transaciton elimination is enabled, we have a dedicated
+         * buffer for that as well. */
+
         bool has_checksum;
         struct panfrost_memory checksum_slab;
         int checksum_stride;

Comments

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> AFBC, tiled, and linear BO layouts are mutually exclusive; they should
> be coupled via a single enum rather than ad hoc checks of booleans.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

> ---
>  src/gallium/drivers/panfrost/pan_context.c  | 33 ++++++++++++++------
>  src/gallium/drivers/panfrost/pan_resource.c | 34 ++++++++++++++++-----
>  src/gallium/drivers/panfrost/pan_resource.h | 20 +++++++++---
>  3 files changed, 64 insertions(+), 23 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 4c41969fd05..630c41fbf1e 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -119,7 +119,7 @@ panfrost_enable_afbc(struct panfrost_context *ctx, struct panfrost_resource *rsr
>                                 (rsrc->bo->afbc_metadata_size + main_size + 4095) / 4096,
>                                 true, 0, 0, 0);
>
> -        rsrc->bo->has_afbc = true;
> +        rsrc->bo->layout = PAN_AFBC;
>
>          /* Compressed textured reads use a tagged pointer to the metadata */
>
> @@ -153,7 +153,7 @@ panfrost_set_fragment_afbc(struct panfrost_context *ctx)
>                  struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.cbufs[cb]->texture;
>
>                  /* Non-AFBC is the default */
> -                if (!rsrc->bo->has_afbc)
> +                if (rsrc->bo->layout != PAN_AFBC)
>                          continue;
>
>                  if (require_sfbd) {
> @@ -179,7 +179,7 @@ panfrost_set_fragment_afbc(struct panfrost_context *ctx)
>          if (ctx->pipe_framebuffer.zsbuf) {
>                  struct panfrost_resource *rsrc = (struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture;
>
> -                if (rsrc->bo->has_afbc) {
> +                if (rsrc->bo->layout == PAN_AFBC) {
>                          if (require_sfbd) {
>                                  fprintf(stderr, "Depth AFBC not supported on SFBD\n");
>                                  assert(0);
> @@ -2244,6 +2244,23 @@ panfrost_create_sampler_view(
>
>          enum mali_format format = panfrost_find_format(desc);
>
> +        unsigned usage2_layout = 0x10;
> +
> +        switch (prsrc->bo->layout) {
> +                case PAN_AFBC:
> +                        usage2_layout |= 0xc;
> +                        break;
> +                case PAN_TILED:
> +                        usage2_layout |= 0x1;
> +                        break;
> +                case PAN_LINEAR:
> +                        usage2_layout |= 0x2;
> +                        break;
> +                default:
> +                        assert(0);
> +                        break;
> +        }
> +
>          struct mali_texture_descriptor texture_descriptor = {
>                  .width = MALI_POSITIVE(texture->width0),
>                  .height = MALI_POSITIVE(texture->height0),
> @@ -2257,11 +2274,7 @@ panfrost_create_sampler_view(
>                          .usage1 = 0x0,
>                          .is_not_cubemap = 1,
>
> -                        /* 0x11 - regular texture 2d, uncompressed tiled */
> -                        /* 0x12 - regular texture 2d, uncompressed linear */
> -                        /* 0x1c - AFBC compressed (internally tiled, probably) texture 2D */
> -
> -                        .usage2 = prsrc->bo->has_afbc ? 0x1c : (prsrc->bo->tiled ? 0x11 : 0x12),
> +                        .usage2 = usage2_layout
>                  },
>
>                  .swizzle = panfrost_translate_swizzle_4(user_swizzle)
> @@ -2353,7 +2366,7 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
>                  struct panfrost_resource *tex = ((struct panfrost_resource *) ctx->pipe_framebuffer.cbufs[i]->texture);
>                  bool is_scanout = panfrost_is_scanout(ctx);
>
> -                if (!is_scanout && !tex->bo->has_afbc) {
> +                if (!is_scanout && tex->bo->layout != PAN_AFBC) {
>                          /* The blob is aggressive about enabling AFBC. As such,
>                           * it's pretty much necessary to use it here, since we
>                           * have no traces of non-compressed FBO. */
> @@ -2387,7 +2400,7 @@ panfrost_set_framebuffer_state(struct pipe_context *pctx,
>
>                                  struct panfrost_resource *tex = ((struct panfrost_resource *) ctx->pipe_framebuffer.zsbuf->texture);
>
> -                                if (!tex->bo->has_afbc && !panfrost_is_scanout(ctx))
> +                                if (tex->bo->layout != PAN_AFBC && !panfrost_is_scanout(ctx))
>                                          panfrost_enable_afbc(ctx, tex, true);
>                          }
>                  }
> diff --git a/src/gallium/drivers/panfrost/pan_resource.c b/src/gallium/drivers/panfrost/pan_resource.c
> index 7dfeb773d8b..0cebbdb6e51 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.c
> +++ b/src/gallium/drivers/panfrost/pan_resource.c
> @@ -195,10 +195,28 @@ panfrost_create_bo(struct panfrost_screen *screen, const struct pipe_resource *t
>          if (template->height0) sz *= template->height0;
>          if (template->depth0) sz *= template->depth0;
>
> +        /* Based on the usage, figure out what storing will be used. There are
> +         * various tradeoffs:
> +         *
> +         * Linear: the basic format, bad for memory bandwidth, bad for cache
> +         * use. Zero-copy, though. Renderable.
> +         *
> +         * Tiled: Not compressed, but cache-optimized. Expensive to write into
> +         * (due to software tiling), but cheap to sample from. Ideal for most
> +         * textures.
> +         *
> +         * AFBC: Compressed and renderable (so always desirable for non-scanout
> +         * rendertargets). Cheap to sample from. The format is black box, so we
> +         * can't read/write from software.
> +         */
> +
>          /* Tiling textures is almost always faster, unless we only use it once */
> -        bo->tiled = (template->usage != PIPE_USAGE_STREAM) && (template->bind & PIPE_BIND_SAMPLER_VIEW);
> +        bool should_tile = (template->usage != PIPE_USAGE_STREAM) && (template->bind & PIPE_BIND_SAMPLER_VIEW);
> +
> +        /* Set the layout appropriately */
> +        bo->layout = should_tile ? PAN_TILED : PAN_LINEAR;
>
> -        if (bo->tiled) {
> +        if (bo->layout == PAN_TILED) {
>                  /* For tiled, we don't map directly, so just malloc any old buffer */
>
>                  for (int l = 0; l < (template->last_level + 1); ++l) {
> @@ -291,7 +309,7 @@ panfrost_destroy_bo(struct panfrost_screen *screen, struct panfrost_bo *pbo)
>                  }
>          }
>
> -        if (bo->tiled) {
> +        if (bo->layout == PAN_TILED) {
>                  /* Tiled has a malloc'd CPU, so just plain ol' free needed */
>
>                  for (int l = 0; l < MAX_MIP_LEVELS; ++l) {
> @@ -299,7 +317,7 @@ panfrost_destroy_bo(struct panfrost_screen *screen, struct panfrost_bo *pbo)
>                  }
>          }
>
> -        if (bo->has_afbc) {
> +        if (bo->layout == PAN_AFBC) {
>                  /* TODO */
>                  printf("--leaking afbc (%d bytes)--\n", bo->afbc_metadata_size);
>          }
> @@ -338,8 +356,8 @@ panfrost_map_bo(struct panfrost_context *ctx, struct pipe_transfer *transfer)
>          /* If non-zero level, it's a mipmapped resource and needs to be treated as such */
>          bo->is_mipmap |= transfer->level;
>
> -        if (transfer->usage & PIPE_TRANSFER_MAP_DIRECTLY && bo->tiled) {
> -                /* We cannot directly map tiled textures */
> +        if (transfer->usage & PIPE_TRANSFER_MAP_DIRECTLY && bo->layout != PAN_LINEAR) {
> +                /* We can only directly map linear resources */
>                  return NULL;
>          }
>
> @@ -444,9 +462,9 @@ panfrost_unmap_bo(struct panfrost_context *ctx,
>                          struct panfrost_resource *prsrc = (struct panfrost_resource *) transfer->resource;
>
>                          /* Gallium thinks writeback happens here; instead, this is our cue to tile */
> -                        if (bo->has_afbc) {
> +                        if (bo->layout == PAN_AFBC) {
>                                  printf("Warning: writes to afbc surface can't possibly work out well for you...\n");
> -                        } else if (bo->tiled) {
> +                        } else if (bo->layout == PAN_TILED) {
>                                  struct pipe_context *gallium = (struct pipe_context *) ctx;
>                                  struct panfrost_screen *screen = pan_screen(gallium->screen);
>                                  panfrost_tile_texture(screen, prsrc, transfer->level);
> diff --git a/src/gallium/drivers/panfrost/pan_resource.h b/src/gallium/drivers/panfrost/pan_resource.h
> index b0b5cc15a0c..7e50e4190c5 100644
> --- a/src/gallium/drivers/panfrost/pan_resource.h
> +++ b/src/gallium/drivers/panfrost/pan_resource.h
> @@ -31,6 +31,14 @@
>  #include "pan_allocate.h"
>  #include "drm-uapi/drm.h"
>
> +/* Describes the memory layout of a BO */
> +
> +enum panfrost_memory_layout {
> +        PAN_LINEAR,
> +        PAN_TILED,
> +        PAN_AFBC
> +};
> +
>  struct panfrost_bo {
>          /* Address to the BO in question */
>
> @@ -51,21 +59,23 @@ struct panfrost_bo {
>          /* Number of bytes of the imported allocation */
>          size_t imported_size;
>
> -        /* Set for tiled, clear for linear. */
> -        bool tiled;
> +        /* Internal layout (tiled?) */
> +        enum panfrost_memory_layout layout;
>
>          /* Is something other than level 0 ever written? */
>          bool is_mipmap;
>
>          /* If AFBC is enabled for this resource, we lug around an AFBC
>           * metadata buffer as well. The actual AFBC resource is also in
> -         * afbc_slab (only defined for AFBC) at position afbc_main_offset */
> +         * afbc_slab (only defined for AFBC) at position afbc_main_offset
> +         */
>
> -        bool has_afbc;
>          struct panfrost_memory afbc_slab;
>          int afbc_metadata_size;
>
> -        /* Similarly for TE */
> +        /* If transaciton elimination is enabled, we have a dedicated
> +         * buffer for that as well. */
> +
>          bool has_checksum;
>          struct panfrost_memory checksum_slab;
>          int checksum_stride;
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev