[11/12] panfrost: Identify fragment_extra flags

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

Details

Message ID 20190310065026.3994-12-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.
The fragment_extra structure contains additional fields extending the
MRT framebuffer descriptor, snuck in between the main framebuffer
descriptor and the render targets. Its fields include those related to
transaction elimination and depth/stencil buffers. This patch identifies
the flags field (previously just "unk" with some magic values) as well
as identifying some (but not all) flags set by the driver.

The process of identifying flags brought a bug to light where
transaction elimination (checksumming) could not be enabled unless AFBC
was in-use. This issue is now resolved.

Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
---
 .../drivers/panfrost/include/panfrost-job.h    |  8 +++++++-
 src/gallium/drivers/panfrost/pan_context.c     | 16 ++++++++--------
 .../drivers/panfrost/pandecode/decode.c        | 18 +++++++++++++-----
 3 files changed, 28 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/gallium/drivers/panfrost/include/panfrost-job.h b/src/gallium/drivers/panfrost/include/panfrost-job.h
index 3c5ed2bc802..dccd8410ae9 100644
--- a/src/gallium/drivers/panfrost/include/panfrost-job.h
+++ b/src/gallium/drivers/panfrost/include/panfrost-job.h
@@ -1419,12 +1419,18 @@  struct bifrost_render_target {
  * - TODO: Anything else?
  */
 
+/* Flags */
+#define MALI_EXTRA_PRESENT      (0x400)
+#define MALI_EXTRA_AFBC         (0x20)
+#define MALI_EXTRA_AFBC_ZS      (0x10)
+#define MALI_EXTRA_ZS           (0x4)
+
 struct bifrost_fb_extra {
         mali_ptr checksum;
         /* Each tile has an 8 byte checksum, so the stride is "width in tiles * 8" */
         u32 checksum_stride;
 
-        u32 unk;
+        u32 flags;
 
         union {
                 /* Note: AFBC is only allowed for 24/8 combined depth/stencil. */
diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
index 419c1a4eb6f..cdced27f101 100644
--- a/src/gallium/drivers/panfrost/pan_context.c
+++ b/src/gallium/drivers/panfrost/pan_context.c
@@ -263,12 +263,12 @@  panfrost_set_fragment_target_zsbuf(
                 ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
                 ctx->fragment_extra.ds_afbc.padding = 0x1000;
 
-                /* There's a general 0x400 in all versions of this field seen.
-                 * ORed with 0x5 for depth/stencil. ORed 0x10 for AFBC encoded
-                 * depth stencil -- er, no. It's unclear where the remaining 0x20 bit
-                 * is from, checksumming maybe? */
-
-                ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
+                ctx->fragment_extra.flags =
+                        MALI_EXTRA_PRESENT |
+                        MALI_EXTRA_AFBC |
+                        MALI_EXTRA_AFBC_ZS |
+                        MALI_EXTRA_ZS |
+                        0x1; /* unknown */
 
                 ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;
         } else if (rsrc->bo->layout == PAN_LINEAR) {
@@ -278,7 +278,7 @@  panfrost_set_fragment_target_zsbuf(
                 /* Setup combined 24/8 depth/stencil */
                 ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
 
-                ctx->fragment_extra.unk = 0x404;
+                ctx->fragment_extra.flags = MALI_EXTRA_PRESENT | MALI_EXTRA_ZS;
                 ctx->fragment_extra.ds_linear.depth = rsrc->bo->gpu[0];
                 ctx->fragment_extra.ds_linear.depth_stride = stride;
 
@@ -1027,7 +1027,7 @@  panfrost_fragment_job(struct panfrost_context *ctx)
                         int stride = util_format_get_stride(rsrc->base.format, rsrc->base.width0);
 
                         ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
-                        ctx->fragment_extra.unk |= 0x420;
+                        ctx->fragment_extra.flags |= MALI_EXTRA_PRESENT;
                         ctx->fragment_extra.checksum_stride = rsrc->bo->checksum_stride;
                         ctx->fragment_extra.checksum = rsrc->bo->gpu[0] + stride * rsrc->base.height0;
                 }
diff --git a/src/gallium/drivers/panfrost/pandecode/decode.c b/src/gallium/drivers/panfrost/pandecode/decode.c
index ea635bbe981..e6932744939 100644
--- a/src/gallium/drivers/panfrost/pandecode/decode.c
+++ b/src/gallium/drivers/panfrost/pandecode/decode.c
@@ -209,6 +209,15 @@  static const struct pandecode_flag_info mfbd_fmt_flag_info[] = {
 };
 #undef FLAG_INFO
 
+#define FLAG_INFO(flag) { MALI_EXTRA_##flag, "MALI_EXTRA_" #flag }
+static const struct pandecode_flag_info mfbd_extra_flag_info[] = {
+        FLAG_INFO(PRESENT),
+        FLAG_INFO(AFBC),
+        FLAG_INFO(ZS),
+        {}
+};
+#undef FLAG_INFO
+
 extern char *replace_fragment;
 extern char *replace_vertex;
 
@@ -604,12 +613,11 @@  pandecode_replay_mfbd_bfr(uint64_t gpu_va, int job_no)
                 if (fbx->checksum_stride)
                         pandecode_prop("checksum_stride = %d", fbx->checksum_stride);
 
-                pandecode_prop("unk = 0x%x", fbx->unk);
+                pandecode_log(".flags = ");
+                pandecode_log_decoded_flags(mfbd_extra_flag_info, fbx->flags);
+                pandecode_log_cont(",\n");
 
-                /* TODO figure out if this is actually the right way to
-                 * determine whether AFBC is enabled
-                 */
-                if (fbx->unk & 0x10) {
+                if (fbx->flags & MALI_EXTRA_AFBC_ZS) {
                         pandecode_log(".ds_afbc = {\n");
                         pandecode_indent++;
 

Comments

One more brick in the wall :)

Reviewed-by: Tomeu Vizoso <tomeu.vizoso@collabora.com>

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa@rosenzweig.io> wrote:
>
> The fragment_extra structure contains additional fields extending the
> MRT framebuffer descriptor, snuck in between the main framebuffer
> descriptor and the render targets. Its fields include those related to
> transaction elimination and depth/stencil buffers. This patch identifies
> the flags field (previously just "unk" with some magic values) as well
> as identifying some (but not all) flags set by the driver.
>
> The process of identifying flags brought a bug to light where
> transaction elimination (checksumming) could not be enabled unless AFBC
> was in-use. This issue is now resolved.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> ---
>  .../drivers/panfrost/include/panfrost-job.h    |  8 +++++++-
>  src/gallium/drivers/panfrost/pan_context.c     | 16 ++++++++--------
>  .../drivers/panfrost/pandecode/decode.c        | 18 +++++++++++++-----
>  3 files changed, 28 insertions(+), 14 deletions(-)
>
> diff --git a/src/gallium/drivers/panfrost/include/panfrost-job.h b/src/gallium/drivers/panfrost/include/panfrost-job.h
> index 3c5ed2bc802..dccd8410ae9 100644
> --- a/src/gallium/drivers/panfrost/include/panfrost-job.h
> +++ b/src/gallium/drivers/panfrost/include/panfrost-job.h
> @@ -1419,12 +1419,18 @@ struct bifrost_render_target {
>   * - TODO: Anything else?
>   */
>
> +/* Flags */
> +#define MALI_EXTRA_PRESENT      (0x400)
> +#define MALI_EXTRA_AFBC         (0x20)
> +#define MALI_EXTRA_AFBC_ZS      (0x10)
> +#define MALI_EXTRA_ZS           (0x4)
> +
>  struct bifrost_fb_extra {
>          mali_ptr checksum;
>          /* Each tile has an 8 byte checksum, so the stride is "width in tiles * 8" */
>          u32 checksum_stride;
>
> -        u32 unk;
> +        u32 flags;
>
>          union {
>                  /* Note: AFBC is only allowed for 24/8 combined depth/stencil. */
> diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c
> index 419c1a4eb6f..cdced27f101 100644
> --- a/src/gallium/drivers/panfrost/pan_context.c
> +++ b/src/gallium/drivers/panfrost/pan_context.c
> @@ -263,12 +263,12 @@ panfrost_set_fragment_target_zsbuf(
>                  ctx->fragment_extra.ds_afbc.zero1 = 0x10009;
>                  ctx->fragment_extra.ds_afbc.padding = 0x1000;
>
> -                /* There's a general 0x400 in all versions of this field seen.
> -                 * ORed with 0x5 for depth/stencil. ORed 0x10 for AFBC encoded
> -                 * depth stencil -- er, no. It's unclear where the remaining 0x20 bit
> -                 * is from, checksumming maybe? */
> -
> -                ctx->fragment_extra.unk = 0x400 | 0x20 | 0x10 | 0x5;
> +                ctx->fragment_extra.flags =
> +                        MALI_EXTRA_PRESENT |
> +                        MALI_EXTRA_AFBC |
> +                        MALI_EXTRA_AFBC_ZS |
> +                        MALI_EXTRA_ZS |
> +                        0x1; /* unknown */
>
>                  ctx->fragment_mfbd.unk3 |= MALI_MFBD_DEPTH_WRITE;
>          } else if (rsrc->bo->layout == PAN_LINEAR) {
> @@ -278,7 +278,7 @@ panfrost_set_fragment_target_zsbuf(
>                  /* Setup combined 24/8 depth/stencil */
>                  ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
>
> -                ctx->fragment_extra.unk = 0x404;
> +                ctx->fragment_extra.flags = MALI_EXTRA_PRESENT | MALI_EXTRA_ZS;
>                  ctx->fragment_extra.ds_linear.depth = rsrc->bo->gpu[0];
>                  ctx->fragment_extra.ds_linear.depth_stride = stride;
>
> @@ -1027,7 +1027,7 @@ panfrost_fragment_job(struct panfrost_context *ctx)
>                          int stride = util_format_get_stride(rsrc->base.format, rsrc->base.width0);
>
>                          ctx->fragment_mfbd.unk3 |= MALI_MFBD_EXTRA;
> -                        ctx->fragment_extra.unk |= 0x420;
> +                        ctx->fragment_extra.flags |= MALI_EXTRA_PRESENT;
>                          ctx->fragment_extra.checksum_stride = rsrc->bo->checksum_stride;
>                          ctx->fragment_extra.checksum = rsrc->bo->gpu[0] + stride * rsrc->base.height0;
>                  }
> diff --git a/src/gallium/drivers/panfrost/pandecode/decode.c b/src/gallium/drivers/panfrost/pandecode/decode.c
> index ea635bbe981..e6932744939 100644
> --- a/src/gallium/drivers/panfrost/pandecode/decode.c
> +++ b/src/gallium/drivers/panfrost/pandecode/decode.c
> @@ -209,6 +209,15 @@ static const struct pandecode_flag_info mfbd_fmt_flag_info[] = {
>  };
>  #undef FLAG_INFO
>
> +#define FLAG_INFO(flag) { MALI_EXTRA_##flag, "MALI_EXTRA_" #flag }
> +static const struct pandecode_flag_info mfbd_extra_flag_info[] = {
> +        FLAG_INFO(PRESENT),
> +        FLAG_INFO(AFBC),
> +        FLAG_INFO(ZS),
> +        {}
> +};
> +#undef FLAG_INFO
> +
>  extern char *replace_fragment;
>  extern char *replace_vertex;
>
> @@ -604,12 +613,11 @@ pandecode_replay_mfbd_bfr(uint64_t gpu_va, int job_no)
>                  if (fbx->checksum_stride)
>                          pandecode_prop("checksum_stride = %d", fbx->checksum_stride);
>
> -                pandecode_prop("unk = 0x%x", fbx->unk);
> +                pandecode_log(".flags = ");
> +                pandecode_log_decoded_flags(mfbd_extra_flag_info, fbx->flags);
> +                pandecode_log_cont(",\n");
>
> -                /* TODO figure out if this is actually the right way to
> -                 * determine whether AFBC is enabled
> -                 */
> -                if (fbx->unk & 0x10) {
> +                if (fbx->flags & MALI_EXTRA_AFBC_ZS) {
>                          pandecode_log(".ds_afbc = {\n");
>                          pandecode_indent++;
>
> --
> 2.20.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev