[Mesa-dev] [PATCH 11/12] panfrost: Identify fragment_extra flags

[Tomeu Vizoso]

One more brick in the wall :)

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

On Sun, 10 Mar 2019 at 07:50, Alyssa Rosenzweig <alyssa at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev