panfrost: MALI_DEPTH_TEST is actually MALI_DEPTH_WRITEMASK
[mesa.git] / src / panfrost / pandecode / decode.c
index 148be10fdc47bd5a95397825821d8e3c00bc2c82..a310582ebe853526930a8957f0dc7c1ff1623645 100644 (file)
@@ -146,9 +146,9 @@ pandecode_validate_buffer(mali_ptr addr, size_t sz)
         unsigned total = offset + sz;
 
         if (total > bo->length) {
-                pandecode_msg("XXX: buffer overrun."
-                                "Chunk of size %d at offset %d in buffer of size %d. "
-                                "Overrun by %d bytes.",
+                pandecode_msg("XXX: buffer overrun. "
+                                "Chunk of size %zu at offset %d in buffer of size %zu. "
+                                "Overrun by %zu bytes. \n",
                                 sz, offset, bo->length, total - bo->length);
                 return;
         }
@@ -223,7 +223,7 @@ static const struct pandecode_flag_info u3_flag_info[] = {
         FLAG_INFO(HAS_MSAA),
         FLAG_INFO(CAN_DISCARD),
         FLAG_INFO(HAS_BLEND_SHADER),
-        FLAG_INFO(DEPTH_TEST),
+        FLAG_INFO(DEPTH_WRITEMASK),
         {}
 };
 
@@ -267,7 +267,6 @@ static const struct pandecode_flag_info mfbd_extra_flag_info[] = {
 #define FLAG_INFO(flag) { MALI_##flag, "MALI_" #flag }
 static const struct pandecode_flag_info shader_midgard1_flag_info [] = {
         FLAG_INFO(EARLY_Z),
-        FLAG_INFO(HELPER_INVOCATIONS),
         FLAG_INFO(READS_TILEBUFFER),
         FLAG_INFO(READS_ZS),
         {}
@@ -413,24 +412,42 @@ pandecode_stencil_op(enum mali_stencil_op op)
 
 #undef DEFINE_CASE
 
-#define DEFINE_CASE(name) case MALI_ATTR_ ## name: return "MALI_ATTR_" #name
-static char *pandecode_attr_mode(enum mali_attr_mode mode)
+static char *pandecode_attr_mode_short(enum mali_attr_mode mode)
 {
         switch(mode) {
-                DEFINE_CASE(UNUSED);
-                DEFINE_CASE(LINEAR);
-                DEFINE_CASE(POT_DIVIDE);
-                DEFINE_CASE(MODULO);
-                DEFINE_CASE(NPOT_DIVIDE);
-                DEFINE_CASE(IMAGE);
-                DEFINE_CASE(INTERNAL);
+                /* TODO: Combine to just "instanced" once this can be done
+                 * unambiguously in all known cases */
+        case MALI_ATTR_POT_DIVIDE:
+                return "instanced_pot";
+        case MALI_ATTR_MODULO:
+                return "instanced_mod";
+        case MALI_ATTR_NPOT_DIVIDE:
+                return "instanced_npot";
+        case MALI_ATTR_IMAGE:
+                return "image";
+        case MALI_ATTR_INTERNAL:
+                return "internal";
         default:
                 pandecode_msg("XXX: invalid attribute mode %X\n", mode);
                 return "";
         }
 }
 
-#undef DEFINE_CASE
+static const char *
+pandecode_special_varying(uint64_t v)
+{
+        switch(v) {
+        case MALI_VARYING_FRAG_COORD:
+                return "gl_FragCoord";
+        case MALI_VARYING_FRONT_FACING:
+                return "gl_FrontFacing";
+        case MALI_VARYING_POINT_COORD:
+                return "gl_PointCoord";
+        default:
+                pandecode_msg("XXX: invalid special varying %" PRIx64 "\n", v);
+                return "";
+        }
+}
 
 #define DEFINE_CASE(name) case MALI_WRAP_## name: return "MALI_WRAP_" #name
 static char *
@@ -554,7 +571,7 @@ pandecode_midgard_tiler_descriptor(
                 /* When tiling is enabled, the heap should be a tight fit */
                 unsigned heap_offset = t->heap_start - heap->gpu_va;
                 if ((heap_offset + heap_size) != heap->length) {
-                        pandecode_msg("XXX: heap size %d (expected %d)\n",
+                        pandecode_msg("XXX: heap size %u (expected %zu)\n",
                                         heap_size, heap->length - heap_offset);
                 }
 
@@ -606,12 +623,27 @@ pandecode_midgard_tiler_descriptor(
         pandecode_log("}\n");
 }
 
-static void
+/* Information about the framebuffer passed back for
+ * additional analysis */
+
+struct pandecode_fbd {
+        unsigned width;
+        unsigned height;
+        unsigned rt_count;
+        bool has_extra;
+};
+
+static struct pandecode_fbd
 pandecode_sfbd(uint64_t gpu_va, int job_no, bool is_fragment)
 {
         struct pandecode_mapped_memory *mem = pandecode_find_mapped_gpu_mem_containing(gpu_va);
         const struct mali_single_framebuffer *PANDECODE_PTR_VAR(s, mem, (mali_ptr) gpu_va);
 
+        struct pandecode_fbd info = {
+                .has_extra = false,
+                .rt_count = 1
+        };
+
         pandecode_log("struct mali_single_framebuffer framebuffer_%"PRIx64"_%d = {\n", gpu_va, job_no);
         pandecode_indent++;
 
@@ -622,8 +654,11 @@ pandecode_sfbd(uint64_t gpu_va, int job_no, bool is_fragment)
         pandecode_log_decoded_flags(fb_fmt_flag_info, s->format);
         pandecode_log_cont(",\n");
 
-        pandecode_prop("width = MALI_POSITIVE(%" PRId16 ")", s->width + 1);
-        pandecode_prop("height = MALI_POSITIVE(%" PRId16 ")", s->height + 1);
+        info.width = s->width + 1;
+        info.height = s->height + 1;
+
+        pandecode_prop("width = MALI_POSITIVE(%" PRId16 ")", info.width);
+        pandecode_prop("height = MALI_POSITIVE(%" PRId16 ")", info.height);
 
         MEMORY_PROP(s, framebuffer);
         pandecode_prop("stride = %d", s->stride);
@@ -688,6 +723,8 @@ pandecode_sfbd(uint64_t gpu_va, int job_no, bool is_fragment)
                 printf("%X, ", s->zero6[i]);
 
         printf("},\n");
+
+        return info;
 }
 
 static void
@@ -946,12 +983,14 @@ pandecode_render_target(uint64_t gpu_va, unsigned job_no, const struct bifrost_f
         pandecode_log("};\n");
 }
 
-static unsigned
+static struct pandecode_fbd
 pandecode_mfbd_bfr(uint64_t gpu_va, int job_no, bool is_fragment)
 {
         struct pandecode_mapped_memory *mem = pandecode_find_mapped_gpu_mem_containing(gpu_va);
         const struct bifrost_framebuffer *PANDECODE_PTR_VAR(fb, mem, (mali_ptr) gpu_va);
 
+        struct pandecode_fbd info;
         if (fb->sample_locations) {
                 /* The blob stores all possible sample locations in a single buffer
                  * allocated on startup, and just switches the pointer when switching
@@ -991,6 +1030,10 @@ pandecode_mfbd_bfr(uint64_t gpu_va, int job_no, bool is_fragment)
          * now */
         MEMORY_PROP(fb, unknown1);
 
+        info.width = fb->width1 + 1;
+        info.height = fb->height1 + 1;
+        info.rt_count = fb->rt_count_1 + 1;
+
         pandecode_prop("width1 = MALI_POSITIVE(%d)", fb->width1 + 1);
         pandecode_prop("height1 = MALI_POSITIVE(%d)", fb->height1 + 1);
         pandecode_prop("width2 = MALI_POSITIVE(%d)", fb->width2 + 1);
@@ -1031,7 +1074,9 @@ pandecode_mfbd_bfr(uint64_t gpu_va, int job_no, bool is_fragment)
 
         gpu_va += sizeof(struct bifrost_framebuffer);
 
-        if ((fb->mfbd_flags & MALI_MFBD_EXTRA) && is_fragment) {
+        info.has_extra = (fb->mfbd_flags & MALI_MFBD_EXTRA) && is_fragment;
+
+        if (info.has_extra) {
                 mem = pandecode_find_mapped_gpu_mem_containing(gpu_va);
                 const struct bifrost_fb_extra *PANDECODE_PTR_VAR(fbx, mem, (mali_ptr) gpu_va);
 
@@ -1115,8 +1160,7 @@ pandecode_mfbd_bfr(uint64_t gpu_va, int job_no, bool is_fragment)
         if (is_fragment)
                 pandecode_render_target(gpu_va, job_no, fb);
 
-        /* Passback the render target count */
-        return MALI_NEGATIVE(fb->rt_count_1);
+        return info;
 }
 
 /* Just add a comment decoding the shift/odd fields forming the padded vertices
@@ -1218,38 +1262,83 @@ pandecode_magic_divisor(uint32_t magic, unsigned shift, unsigned orig_divisor, u
 static void
 pandecode_attributes(const struct pandecode_mapped_memory *mem,
                             mali_ptr addr, int job_no, char *suffix,
-                            int count, bool varying)
+                            int count, bool varying, enum mali_job_type job_type)
 {
-        char *prefix = varying ? "varyings" : "attributes";
+        char *prefix = varying ? "varying" : "attribute";
+        assert(addr);
 
-        if (!addr) {
-                pandecode_msg("no %s\n", prefix);
+        if (!count) {
+                pandecode_msg("warn: No %s records\n", prefix);
                 return;
         }
 
         union mali_attr *attr = pandecode_fetch_gpu_mem(mem, addr, sizeof(union mali_attr) * count);
 
-        pandecode_log("union mali_attr %s_%d[] = {\n", prefix, job_no);
-        pandecode_indent++;
-
         for (int i = 0; i < count; ++i) {
-                pandecode_log("{\n");
-                pandecode_indent++;
+                /* First, check for special records */
+                if (attr[i].elements < MALI_VARYING_SPECIAL) {
+                        /* Special records are always varyings */
+
+                        if (!varying)
+                                pandecode_msg("XXX: Special varying in attribute field\n");
+
+                        if (job_type != JOB_TYPE_TILER)
+                                pandecode_msg("XXX: Special varying in non-FS\n");
+
+                        /* We're special, so all fields should be zero */
+                        unsigned zero = attr[i].stride | attr[i].size;
+                        zero |= attr[i].shift | attr[i].extra_flags;
+
+                        if (zero)
+                                pandecode_msg("XXX: Special varying has non-zero fields\n");
+                        else {
+                                /* Print the special varying name */
+                                pandecode_log("varying_%d = %s;", i, pandecode_special_varying(attr[i].elements));
+                                continue;
+                        }
+                }
 
                 enum mali_attr_mode mode = attr[i].elements & 7;
 
                 if (mode == MALI_ATTR_UNUSED)
                         pandecode_msg("XXX: unused attribute record\n");
 
+                /* For non-linear records, we need to print the type of record */
+                if (mode != MALI_ATTR_LINEAR)
+                        pandecode_log_cont("%s ", pandecode_attr_mode_short(mode));
+
+                /* Print the name to link with attr_meta */
+                pandecode_log_cont("%s_%d", prefix, i);
+
+                /* Print the stride and size */
+                pandecode_log_cont("<%u>[%u]", attr[i].stride, attr[i].size);
+
+                /* TODO: Sanity check the quotient itself. It must be equal to
+                 * (or be greater than, if the driver added padding) the padded
+                 * vertex count. */
+
+                /* Finally, print the pointer */
                 mali_ptr raw_elements = attr[i].elements & ~7;
                 char *a = pointer_as_memory_reference(raw_elements);
-                pandecode_prop("elements = (%s) | %s", a, pandecode_attr_mode(mode));
+                pandecode_log_cont(" = (%s);\n", a);
                 free(a);
 
-                pandecode_prop("shift = %d", attr[i].shift);
-                pandecode_prop("extra_flags = %d", attr[i].extra_flags);
-                pandecode_prop("stride = 0x%" PRIx32, attr[i].stride);
-                pandecode_prop("size = 0x%" PRIx32, attr[i].size);
+                /* Check the pointer */
+                pandecode_validate_buffer(raw_elements, attr[i].size);
+
+                /* shift/extra_flags exist only for instanced */
+                if (attr[i].shift | attr[i].extra_flags) {
+                        /* These are set to random values by the blob for
+                         * varyings, most likely a symptom of uninitialized
+                         * memory where the hardware masked the bug. As such we
+                         * put this at a warning, not an error. */
+
+                        if (mode == MALI_ATTR_LINEAR)
+                                pandecode_msg("warn: instancing fields set for linear\n");
+
+                        pandecode_prop("shift = %d", attr[i].shift);
+                        pandecode_prop("extra_flags = %d", attr[i].extra_flags);
+                }
 
                 /* Decode further where possible */
 
@@ -1259,9 +1348,6 @@ pandecode_attributes(const struct pandecode_mapped_memory *mem,
                                 attr[i].extra_flags);
                 }
 
-                pandecode_indent--;
-                pandecode_log("}, \n");
-
                 if (mode == MALI_ATTR_NPOT_DIVIDE) {
                         i++;
                         pandecode_log("{\n");
@@ -1278,8 +1364,7 @@ pandecode_attributes(const struct pandecode_mapped_memory *mem,
 
         }
 
-        pandecode_indent--;
-        pandecode_log("};\n");
+        pandecode_log("\n");
 }
 
 static mali_ptr
@@ -1295,21 +1380,14 @@ pandecode_shader_address(const char *name, mali_ptr ptr)
         return shader_ptr;
 }
 
-static bool
-all_zero(unsigned *buffer, unsigned count)
-{
-        for (unsigned i = 0; i < count; ++i) {
-                if (buffer[i])
-                        return false;
-        }
-
-        return true;
-}
-
 static void
 pandecode_stencil(const char *name, const struct mali_stencil_test *stencil)
 {
-        if (all_zero((unsigned *) stencil, sizeof(stencil) / sizeof(unsigned)))
+        unsigned any_nonzero =
+                stencil->ref | stencil->mask | stencil->func |
+                stencil->sfail | stencil->dpfail | stencil->dppass;
+
+        if (any_nonzero == 0)
                 return;
 
         const char *func = pandecode_func(stencil->func);
@@ -1391,7 +1469,8 @@ pandecode_bifrost_blend(void *descs, int job_no, int rt_no)
 static mali_ptr
 pandecode_midgard_blend(union midgard_blend *blend, bool is_shader)
 {
-        if (all_zero((unsigned *) blend, sizeof(blend) / sizeof(unsigned)))
+        /* constant/equation is in a union */
+        if (!blend->shader)
                 return 0;
 
         pandecode_log(".blend = {\n");
@@ -1447,11 +1526,8 @@ pandecode_attribute_meta(int job_no, int count, const struct mali_vertex_tiler_p
         unsigned max_index = 0;
         snprintf(base, sizeof(base), "%s_meta", prefix);
 
-        pandecode_log("struct mali_attr_meta %s_%d%s[] = {\n", base, job_no, suffix);
-        pandecode_indent++;
-
         struct mali_attr_meta *attr_meta;
-        mali_ptr p = varying ? (v->varying_meta & ~0xF) : v->attribute_meta;
+        mali_ptr p = varying ? v->varying_meta : v->attribute_meta;
 
         struct pandecode_mapped_memory *attr_mem = pandecode_find_mapped_gpu_mem_containing(p);
 
@@ -1504,7 +1580,6 @@ pandecode_attribute_meta(int job_no, int count, const struct mali_vertex_tiler_p
                         pandecode_prop("unknown3 = 0x%" PRIx64, (u64) attr_meta->unknown3);
                 }
 
-                pandecode_make_indent();
                 pandecode_format_short(attr_meta->format, false);
                 pandecode_log_cont(" %s_%u", prefix, attr_meta->index);
 
@@ -1516,36 +1591,11 @@ pandecode_attribute_meta(int job_no, int count, const struct mali_vertex_tiler_p
                 pandecode_log_cont(";\n");
         }
 
-        pandecode_indent--;
-        pandecode_log("};\n");
+        pandecode_log("\n");
 
         return count ? (max_index + 1) : 0;
 }
 
-static void
-pandecode_indices(uintptr_t pindices, uint32_t index_count, int job_no)
-{
-        struct pandecode_mapped_memory *imem = pandecode_find_mapped_gpu_mem_containing(pindices);
-
-        if (imem) {
-                /* Indices are literally just a u32 array :) */
-
-                uint32_t *PANDECODE_PTR_VAR(indices, imem, pindices);
-
-                pandecode_log("uint32_t indices_%d[] = {\n", job_no);
-                pandecode_indent++;
-
-                for (unsigned i = 0; i < (index_count + 1); i += 3)
-                        pandecode_log("%d, %d, %d,\n",
-                                      indices[i],
-                                      indices[i + 1],
-                                      indices[i + 2]);
-
-                pandecode_indent--;
-                pandecode_log("};\n");
-        }
-}
-
 /* return bits [lo, hi) of word */
 static u32
 bits(u32 word, u32 lo, u32 hi)
@@ -1596,7 +1646,7 @@ pandecode_vertex_tiler_prefix(struct mali_vertex_tiler_prefix *p, int job_no, bo
 
         if (!canonical) {
                 pandecode_msg("XXX: non-canonical workgroups packing\n");
-                pandecode_msg("expected: %X, %d, %d, %d, %d, %d\n",
+                pandecode_msg("expected: %X, %d, %d, %d, %d, %d, %d\n",
                                 ref.invocation_count,
                                 ref.size_y_shift,
                                 ref.size_z_shift,
@@ -1633,6 +1683,30 @@ pandecode_vertex_tiler_prefix(struct mali_vertex_tiler_prefix *p, int job_no, bo
         if (p->index_count)
                 pandecode_prop("index_count = MALI_POSITIVE(%" PRId32 ")", p->index_count + 1);
 
+
+        unsigned index_raw_size = (p->unknown_draw & MALI_DRAW_INDEXED_SIZE);
+        index_raw_size >>= MALI_DRAW_INDEXED_SHIFT;
+
+        /* Validate an index buffer is present if we need one. TODO: verify
+         * relationship between invocation_count and index_count */
+
+        if (p->indices) {
+                unsigned count = p->index_count;
+
+                /* Grab the size */
+                unsigned size = (index_raw_size == 0x3) ? 4 : index_raw_size;
+
+                /* Ensure we got a size, and if so, validate the index buffer
+                 * is large enough to hold a full set of indices of the given
+                 * size */
+
+                if (!index_raw_size)
+                        pandecode_msg("XXX: index size missing\n");
+                else
+                        pandecode_validate_buffer(p->indices, count * size);
+        } else if (index_raw_size)
+                pandecode_msg("XXX: unexpected index size %u\n", index_raw_size);
+
         if (p->offset_bias_correction)
                 pandecode_prop("offset_bias_correction = %d", p->offset_bias_correction);
 
@@ -1649,29 +1723,28 @@ pandecode_uniform_buffers(mali_ptr pubufs, int ubufs_count, int job_no)
         struct pandecode_mapped_memory *umem = pandecode_find_mapped_gpu_mem_containing(pubufs);
         struct mali_uniform_buffer_meta *PANDECODE_PTR_VAR(ubufs, umem, pubufs);
 
-        pandecode_log("struct mali_uniform_buffer_meta uniform_buffers_%"PRIx64"_%d[] = {\n",
-                      pubufs, job_no);
-        pandecode_indent++;
-
         for (int i = 0; i < ubufs_count; i++) {
-                pandecode_log("{\n");
-                pandecode_indent++;
-
                 unsigned size = (ubufs[i].size + 1) * 16;
                 mali_ptr addr = ubufs[i].ptr << 2;
 
                 pandecode_validate_buffer(addr, size);
 
                 char *ptr = pointer_as_memory_reference(ubufs[i].ptr << 2);
-                pandecode_prop("size = %u", size);
-                pandecode_prop("ptr = (%s) >> 2", ptr);
-                pandecode_indent--;
-                pandecode_log("},\n");
+                pandecode_log("ubuf_%d[%u] = %s;\n", i, size, ptr);
                 free(ptr);
         }
 
-        pandecode_indent--;
-        pandecode_log("};\n");
+        pandecode_log("\n");
+}
+
+static void
+pandecode_uniforms(mali_ptr uniforms, unsigned uniform_count)
+{
+        pandecode_validate_buffer(uniforms, uniform_count * 16);
+
+        char *ptr = pointer_as_memory_reference(uniforms);
+        pandecode_log("vec4 uniforms[%u] = %s;\n", uniform_count, ptr);
+        free(ptr);
 }
 
 static void
@@ -1699,9 +1772,9 @@ pandecode_scratchpad(uintptr_t pscratchpad, int job_no, char *suffix)
 
 static unsigned shader_id = 0;
 
-static void
+static struct midgard_disasm_stats
 pandecode_shader_disassemble(mali_ptr shader_ptr, int shader_no, int type,
-                             bool is_bifrost, unsigned nr_regs)
+                             bool is_bifrost)
 {
         struct pandecode_mapped_memory *mem = pandecode_find_mapped_gpu_mem_containing(shader_ptr);
         uint8_t *PANDECODE_PTR_VAR(code, mem, shader_ptr);
@@ -1714,19 +1787,45 @@ pandecode_shader_disassemble(mali_ptr shader_ptr, int shader_no, int type,
 
         printf("\n\n");
 
-        char prefix[512];
-
-        snprintf(prefix, sizeof(prefix) - 1, "shader%d - %s shader: ",
-                        shader_id++,
-                        (type == JOB_TYPE_TILER) ? "FRAGMENT" : "VERTEX");
+        struct midgard_disasm_stats stats;
 
         if (is_bifrost) {
                 disassemble_bifrost(code, sz, false);
+
+                /* TODO: Extend stats to Bifrost */
+                stats.texture_count = -128;
+                stats.sampler_count = -128;
+                stats.attribute_count = -128;
+                stats.varying_count = -128;
+                stats.uniform_count = -128;
+                stats.uniform_buffer_count = -128;
+                stats.work_count = -128;
+
+                stats.instruction_count = 0;
+                stats.bundle_count = 0;
+                stats.quadword_count = 0;
+                stats.helper_invocations = false;
         } else {
-                disassemble_midgard(code, sz, true, nr_regs, prefix);
+                stats = disassemble_midgard(code, sz);
         }
 
-        printf("\n\n");
+        /* Print shader-db stats */
+
+        unsigned nr_threads =
+                (stats.work_count <= 4) ? 4 :
+                (stats.work_count <= 8) ? 2 :
+                1;
+
+        printf("shader%d - %s shader: "
+                "%u inst, %u bundles, %u quadwords, "
+                "%u registers, %u threads, 0 loops\n\n\n",
+                shader_id++,
+                (type == JOB_TYPE_TILER) ? "FRAGMENT" : "VERTEX",
+                stats.instruction_count, stats.bundle_count, stats.quadword_count,
+                stats.work_count, nr_threads);
+
+
+        return stats;
 }
 
 static void
@@ -1882,39 +1981,91 @@ pandecode_texture(mali_ptr u,
         pandecode_log("};\n");
 }
 
+/* For shader properties like texture_count, we have a claimed property in the shader_meta, and the actual Truth from static analysis (this may just be an upper limit). We validate accordingly */
+
+static void
+pandecode_shader_prop(const char *name, unsigned claim, signed truth, bool fuzzy)
+{
+        /* Nothing to do */
+        if (claim == truth)
+                return;
+
+        if (fuzzy)
+                assert(truth >= 0);
+
+        if ((truth >= 0) && !fuzzy) {
+                pandecode_msg("%s: expected %s = %d, claimed %u\n",
+                                (truth < claim) ? "warn" : "XXX",
+                                name, truth, claim);
+        } else if ((claim > -truth) && !fuzzy) {
+                pandecode_msg("XXX: expected %s <= %u, claimed %u\n",
+                                name, -truth, claim);
+        } else if (fuzzy && (claim < truth))
+                pandecode_msg("XXX: expected %s >= %u, claimed %u\n",
+                                name, truth, claim);
+
+        pandecode_log(".%s = %" PRId16, name, claim);
+
+        if (fuzzy)
+                pandecode_log_cont(" /* %u used */", truth);
+
+        pandecode_log_cont(",\n");
+}
+
 static void
 pandecode_vertex_tiler_postfix_pre(
                 const struct mali_vertex_tiler_postfix *p,
                 int job_no, enum mali_job_type job_type,
                 char *suffix, bool is_bifrost)
 {
-        mali_ptr shader_meta_ptr = (u64) (uintptr_t) (p->_shader_upper << 4);
         struct pandecode_mapped_memory *attr_mem;
 
-        unsigned rt_count = 1;
-
         /* On Bifrost, since the tiler heap (for tiler jobs) and the scratchpad
          * are the only things actually needed from the FBD, vertex/tiler jobs
          * no longer reference the FBD -- instead, this field points to some
          * info about the scratchpad.
          */
+
+        struct pandecode_fbd fbd_info = {
+                /* Default for Bifrost */
+                .rt_count = 1
+        };
+
         if (is_bifrost)
                 pandecode_scratchpad(p->framebuffer & ~FBD_TYPE, job_no, suffix);
         else if (p->framebuffer & MALI_MFBD)
-                rt_count = pandecode_mfbd_bfr((u64) ((uintptr_t) p->framebuffer) & FBD_MASK, job_no, false);
+                fbd_info = pandecode_mfbd_bfr((u64) ((uintptr_t) p->framebuffer) & FBD_MASK, job_no, false);
         else if (job_type == JOB_TYPE_COMPUTE)
                 pandecode_compute_fbd((u64) (uintptr_t) p->framebuffer, job_no);
         else
-                pandecode_sfbd((u64) (uintptr_t) p->framebuffer, job_no, false);
+                fbd_info = pandecode_sfbd((u64) (uintptr_t) p->framebuffer, job_no, false);
 
         int varying_count = 0, attribute_count = 0, uniform_count = 0, uniform_buffer_count = 0;
         int texture_count = 0, sampler_count = 0;
 
-        if (shader_meta_ptr) {
-                struct pandecode_mapped_memory *smem = pandecode_find_mapped_gpu_mem_containing(shader_meta_ptr);
-                struct mali_shader_meta *PANDECODE_PTR_VAR(s, smem, shader_meta_ptr);
+        if (p->shader) {
+                struct pandecode_mapped_memory *smem = pandecode_find_mapped_gpu_mem_containing(p->shader);
+                struct mali_shader_meta *PANDECODE_PTR_VAR(s, smem, p->shader);
+
+                /* Disassemble ahead-of-time to get stats. Initialize with
+                 * stats for the missing-shader case so we get validation
+                 * there, too */
 
-                pandecode_log("struct mali_shader_meta shader_meta_%"PRIx64"_%d%s = {\n", shader_meta_ptr, job_no, suffix);
+                struct midgard_disasm_stats info = {
+                        .texture_count = 0,
+                        .sampler_count = 0,
+                        .attribute_count = 0,
+                        .varying_count = 0,
+                        .work_count = 1,
+
+                        .uniform_count = -128,
+                        .uniform_buffer_count = 0
+                };
+
+                if (s->shader & ~0xF)
+                        info = pandecode_shader_disassemble(s->shader & ~0xF, job_no, job_type, is_bifrost);
+
+                pandecode_log("struct mali_shader_meta shader_meta_%"PRIx64"_%d%s = {\n", p->shader, job_no, suffix);
                 pandecode_indent++;
 
                 /* Save for dumps */
@@ -1927,45 +2078,45 @@ pandecode_vertex_tiler_postfix_pre(
                         uniform_count = s->bifrost2.uniform_count;
                         uniform_buffer_count = s->bifrost1.uniform_buffer_count;
                 } else {
-                        uniform_count = s->midgard1.uniform_buffer_count;
+                        uniform_count = s->midgard1.uniform_count;
                         uniform_buffer_count = s->midgard1.uniform_buffer_count;
                 }
 
-                mali_ptr shader_ptr = pandecode_shader_address("shader", s->shader);
+                pandecode_shader_address("shader", s->shader);
 
-                pandecode_prop("texture_count = %" PRId16, s->texture_count);
-                pandecode_prop("sampler_count = %" PRId16, s->sampler_count);
-                pandecode_prop("attribute_count = %" PRId16, s->attribute_count);
-                pandecode_prop("varying_count = %" PRId16, s->varying_count);
+                pandecode_shader_prop("texture_count", s->texture_count, info.texture_count, false);
+                pandecode_shader_prop("sampler_count", s->sampler_count, info.sampler_count, false);
+                pandecode_shader_prop("attribute_count", s->attribute_count, info.attribute_count, false);
+                pandecode_shader_prop("varying_count", s->varying_count, info.varying_count, false);
+                pandecode_shader_prop("uniform_buffer_count",
+                                uniform_buffer_count,
+                                info.uniform_buffer_count, true);
 
-                unsigned nr_registers = 0;
-
-                if (is_bifrost) {
-                        pandecode_log(".bifrost1 = {\n");
-                        pandecode_indent++;
+                if (!is_bifrost) {
+                        pandecode_shader_prop("uniform_count",
+                                        uniform_count,
+                                        info.uniform_count, false);
 
-                        pandecode_prop("uniform_buffer_count = %" PRId32, s->bifrost1.uniform_buffer_count);
-                        pandecode_prop("unk1 = 0x%" PRIx32, s->bifrost1.unk1);
+                        pandecode_shader_prop("work_count",
+                                        s->midgard1.work_count, info.work_count, false);
+                }
 
-                        pandecode_indent--;
-                        pandecode_log("},\n");
+                if (is_bifrost) {
+                        pandecode_prop("bifrost1.unk1 = 0x%" PRIx32, s->bifrost1.unk1);
                 } else {
-                        pandecode_log(".midgard1 = {\n");
-                        pandecode_indent++;
+                        bool helpers = s->midgard1.flags & MALI_HELPER_INVOCATIONS;
+                        s->midgard1.flags &= ~MALI_HELPER_INVOCATIONS;
 
-                        pandecode_prop("uniform_count = %" PRId16, s->midgard1.uniform_count);
-                        pandecode_prop("uniform_buffer_count = %" PRId16, s->midgard1.uniform_buffer_count);
-                        pandecode_prop("work_count = %" PRId16, s->midgard1.work_count);
-                        nr_registers = s->midgard1.work_count;
+                        if (helpers != info.helper_invocations) {
+                                pandecode_msg("XXX: expected helpers %u but got %u\n",
+                                                info.helper_invocations, helpers);
+                        }
 
-                        pandecode_log(".flags = ");
+                        pandecode_log(".midgard1.flags = ");
                         pandecode_log_decoded_flags(shader_midgard1_flag_info, s->midgard1.flags);
                         pandecode_log_cont(",\n");
 
-                        pandecode_prop("unknown2 = 0x%" PRIx32, s->midgard1.unknown2);
-
-                        pandecode_indent--;
-                        pandecode_log("},\n");
+                        pandecode_prop("midgard1.unknown2 = 0x%" PRIx32, s->midgard1.unknown2);
                 }
 
                 if (s->depth_units || s->depth_factor) {
@@ -1990,7 +2141,7 @@ pandecode_vertex_tiler_postfix_pre(
 
                         /* We're not quite sure what these flags mean without the depth test, if anything */
 
-                        if (unknown2_3 & (MALI_DEPTH_TEST | MALI_DEPTH_FUNC_MASK)) {
+                        if (unknown2_3 & (MALI_DEPTH_WRITEMASK | MALI_DEPTH_FUNC_MASK)) {
                                 const char *func = pandecode_func(MALI_GET_DEPTH_FUNC(unknown2_3));
                                 unknown2_3 &= ~MALI_DEPTH_FUNC_MASK;
 
@@ -2052,7 +2203,7 @@ pandecode_vertex_tiler_postfix_pre(
                 if (job_type == JOB_TYPE_TILER) {
                         void* blend_base = (void *) (s + 1);
 
-                        for (unsigned i = 0; i < rt_count; i++) {
+                        for (unsigned i = 0; i < fbd_info.rt_count; i++) {
                                 mali_ptr shader = 0;
 
                                 if (is_bifrost)
@@ -2060,15 +2211,31 @@ pandecode_vertex_tiler_postfix_pre(
                                 else
                                         shader = pandecode_midgard_blend_mrt(blend_base, job_no, i);
 
-                                if (shader & ~0xF)
-                                        pandecode_shader_disassemble(shader, job_no, job_type, false, 0);
+                                if (shader & ~0xF) {
+                                        struct midgard_disasm_stats stats =
+                                                pandecode_shader_disassemble(shader, job_no, job_type, false);
+
+                                        bool has_texture = (stats.texture_count > 0);
+                                        bool has_sampler = (stats.sampler_count > 0);
+                                        bool has_attribute = (stats.attribute_count > 0);
+                                        bool has_varying = (stats.varying_count > 0);
+                                        bool has_uniform = (stats.uniform_count > 0);
+                                        bool has_ubo = (stats.uniform_buffer_count > 0);
+
+                                        if (has_texture || has_sampler)
+                                                pandecode_msg("XXX: blend shader accessing textures\n");
+
+                                        if (has_attribute || has_varying)
+                                                pandecode_msg("XXX: blend shader accessing interstage\n");
+
+                                        if (has_uniform || has_ubo)
+                                                pandecode_msg("XXX: blend shader accessing uniforms\n");
+                                }
+
                         }
                 }
-
-                if (shader_ptr & ~0xF)
-                   pandecode_shader_disassemble(shader_ptr, job_no, job_type, is_bifrost, nr_registers);
         } else
-                pandecode_msg("<no shader>\n");
+                pandecode_msg("XXX: missing shader descriptor\n");
 
         if (p->viewport) {
                 struct pandecode_mapped_memory *fmem = pandecode_find_mapped_gpu_mem_containing(p->viewport);
@@ -2096,11 +2263,14 @@ pandecode_vertex_tiler_postfix_pre(
                 pandecode_log("};\n");
         }
 
-        if (p->attribute_meta) {
-                unsigned max_attr_index = pandecode_attribute_meta(job_no, attribute_count, p, false, suffix);
+        unsigned max_attr_index = 0;
+
+        if (p->attribute_meta)
+                max_attr_index = pandecode_attribute_meta(job_no, attribute_count, p, false, suffix);
 
+        if (p->attributes) {
                 attr_mem = pandecode_find_mapped_gpu_mem_containing(p->attributes);
-                pandecode_attributes(attr_mem, p->attributes, job_no, suffix, max_attr_index, false);
+                pandecode_attributes(attr_mem, p->attributes, job_no, suffix, max_attr_index, false, job_type);
         }
 
         /* Varyings are encoded like attributes but not actually sent; we just
@@ -2117,14 +2287,14 @@ pandecode_vertex_tiler_postfix_pre(
                 /* Number of descriptors depends on whether there are
                  * non-internal varyings */
 
-                pandecode_attributes(attr_mem, p->varyings, job_no, suffix, varying_count, true);
+                pandecode_attributes(attr_mem, p->varyings, job_no, suffix, varying_count, true, job_type);
         }
 
         if (p->uniform_buffers) {
                 if (uniform_buffer_count)
                         pandecode_uniform_buffers(p->uniform_buffers, uniform_buffer_count, job_no);
                 else
-                        pandecode_msg("XXX: UBOs specified but not referenced\n");
+                        pandecode_msg("warn: UBOs specified but not referenced\n");
         } else if (uniform_buffer_count)
                 pandecode_msg("XXX: UBOs referenced but not specified\n");
 
@@ -2133,11 +2303,11 @@ pandecode_vertex_tiler_postfix_pre(
 
         if (p->uniforms) {
                 if (uniform_count)
-                        pandecode_validate_buffer(p->uniforms, uniform_count * 16);
+                        pandecode_uniforms(p->uniforms, uniform_count);
                 else
-                        pandecode_msg("XXX: Uniforms specified but not referenced");
+                        pandecode_msg("warn: Uniforms specified but not referenced\n");
         } else if (uniform_count)
-                pandecode_msg("XXX: UBOs referenced but not specified\n");
+                pandecode_msg("XXX: Uniforms referenced but not specified\n");
 
         if (p->texture_trampoline) {
                 struct pandecode_mapped_memory *mmem = pandecode_find_mapped_gpu_mem_containing(p->texture_trampoline);
@@ -2218,7 +2388,10 @@ pandecode_vertex_tiler_postfix_pre(
 static void
 pandecode_vertex_tiler_postfix(const struct mali_vertex_tiler_postfix *p, int job_no, bool is_bifrost)
 {
-        if (!(p->position_varying || p->occlusion_counter || p->flags))
+        if (p->shader & 0xF)
+                pandecode_msg("warn: shader tagged %X\n", p->shader & 0xF);
+
+        if (!(p->position_varying || p->occlusion_counter))
                 return;
 
         pandecode_log(".postfix = {\n");
@@ -2227,9 +2400,6 @@ pandecode_vertex_tiler_postfix(const struct mali_vertex_tiler_postfix *p, int jo
         MEMORY_PROP(p, position_varying);
         MEMORY_PROP(p, occlusion_counter);
 
-        if (p->flags)
-                pandecode_prop("flags = %d", p->flags);
-
         pandecode_indent--;
         pandecode_log("},\n");
 }
@@ -2420,8 +2590,6 @@ pandecode_tiler_job_bfr(const struct mali_job_descriptor_header *h,
         struct bifrost_payload_tiler *PANDECODE_PTR_VAR(t, mem, payload);
 
         pandecode_vertex_tiler_postfix_pre(&t->postfix, job_no, h->job_type, "", true);
-
-        pandecode_indices(t->prefix.indices, t->prefix.index_count, job_no);
         pandecode_tiler_meta(t->tiler.tiler_meta, job_no);
 
         pandecode_log("struct bifrost_payload_tiler payload_%d = {\n", job_no);
@@ -2450,8 +2618,6 @@ pandecode_vertex_or_tiler_job_mdg(const struct mali_job_descriptor_header *h,
 
         pandecode_vertex_tiler_postfix_pre(&v->postfix, job_no, h->job_type, "", false);
 
-        pandecode_indices(v->prefix.indices, v->prefix.index_count, job_no);
-
         pandecode_log("struct midgard_payload_vertex_tiler payload_%d = {\n", job_no);
         pandecode_indent++;
 
@@ -2498,67 +2664,87 @@ pandecode_fragment_job(const struct pandecode_mapped_memory *mem,
 {
         const struct mali_payload_fragment *PANDECODE_PTR_VAR(s, mem, payload);
 
-        bool fbd_dumped = false;
-
-        if (!is_bifrost && (s->framebuffer & FBD_TYPE) == MALI_SFBD) {
-                /* Only SFBDs are understood, not MFBDs. We're speculating,
-                 * based on the versioning, kernel code, etc, that the
-                 * difference is between Single FrameBuffer Descriptor and
-                 * Multiple FrmaeBuffer Descriptor; the change apparently lines
-                 * up with multi-framebuffer support being added (T7xx onwards,
-                 * including Gxx). In any event, there's some field shuffling
-                 * that we haven't looked into yet. */
-
-                pandecode_sfbd(s->framebuffer & FBD_MASK, job_no, true);
-                fbd_dumped = true;
-        } else if ((s->framebuffer & FBD_TYPE) == MALI_MFBD) {
-                /* We don't know if Bifrost supports SFBD's at all, since the
-                 * driver never uses them. And the format is different from
-                 * Midgard anyways, due to the tiler heap and scratchpad being
-                 * moved out into separate structures, so it's not clear what a
-                 * Bifrost SFBD would even look like without getting an actual
-                 * trace, which appears impossible.
-                 */
+        bool is_mfbd = (s->framebuffer & FBD_TYPE) == MALI_MFBD;
+
+        /* Bifrost theoretically may retain support for SFBD on compute jobs,
+         * but for graphics workloads with a FRAGMENT payload, use MFBD */
 
-                pandecode_mfbd_bfr(s->framebuffer & FBD_MASK, job_no, true);
-                fbd_dumped = true;
+        if (!is_mfbd && is_bifrost)
+                pandecode_msg("XXX: Bifrost fragment must use MFBD\n");
+
+        struct pandecode_fbd info;
+
+        if (is_mfbd)
+                info = pandecode_mfbd_bfr(s->framebuffer & FBD_MASK, job_no, true);
+        else
+                info = pandecode_sfbd(s->framebuffer & FBD_MASK, job_no, true);
+
+        /* Compute the tag for the tagged pointer. This contains the type of
+         * FBD (MFBD/SFBD), and in the case of an MFBD, information about which
+         * additional structures follow the MFBD header (an extra payload or
+         * not, as well as a count of render targets) */
+
+        unsigned expected_tag = is_mfbd ? MALI_MFBD : MALI_SFBD;
+
+        if (is_mfbd) {
+                if (info.has_extra)
+                        expected_tag |= MALI_MFBD_TAG_EXTRA;
+
+                expected_tag |= (MALI_POSITIVE(info.rt_count) << 2);
         }
 
-        uintptr_t p = (uintptr_t) s->framebuffer & FBD_MASK;
-        pandecode_log("struct mali_payload_fragment payload_%"PRIx64"_%d = {\n", payload, job_no);
-        pandecode_indent++;
+        if ((s->min_tile_coord | s->max_tile_coord) & ~(MALI_X_COORD_MASK | MALI_Y_COORD_MASK)) {
+                pandecode_msg("XXX: unexpected tile coordinate bits\n");
+                pandecode_prop("min_tile_coord = 0x%X\n", s->min_tile_coord);
+                pandecode_prop("max_tile_coord = 0x%X\n", s->min_tile_coord);
+        }
 
-        /* See the comments by the macro definitions for mathematical context
-         * on why this is so weird */
+        /* Extract tile coordinates */
 
-        if (MALI_TILE_COORD_FLAGS(s->max_tile_coord) || MALI_TILE_COORD_FLAGS(s->min_tile_coord))
-                pandecode_msg("Tile coordinate flag missed, replay wrong\n");
+        unsigned min_x = MALI_TILE_COORD_X(s->min_tile_coord) << MALI_TILE_SHIFT;
+        unsigned min_y = MALI_TILE_COORD_Y(s->min_tile_coord) << MALI_TILE_SHIFT;
 
-        pandecode_prop("min_tile_coord = MALI_COORDINATE_TO_TILE_MIN(%d, %d)",
-                       MALI_TILE_COORD_X(s->min_tile_coord) << MALI_TILE_SHIFT,
-                       MALI_TILE_COORD_Y(s->min_tile_coord) << MALI_TILE_SHIFT);
+        unsigned max_x = (MALI_TILE_COORD_X(s->max_tile_coord) + 1) << MALI_TILE_SHIFT;
+        unsigned max_y = (MALI_TILE_COORD_Y(s->max_tile_coord) + 1) << MALI_TILE_SHIFT;
 
-        pandecode_prop("max_tile_coord = MALI_COORDINATE_TO_TILE_MAX(%d, %d)",
-                       (MALI_TILE_COORD_X(s->max_tile_coord) + 1) << MALI_TILE_SHIFT,
-                       (MALI_TILE_COORD_Y(s->max_tile_coord) + 1) << MALI_TILE_SHIFT);
+        /* For the max, we also want the floored (rather than ceiled) version for checking */
 
-        /* If the FBD was just decoded, we can refer to it by pointer. If not,
-         * we have to fallback on offsets. */
+        unsigned max_x_f = (MALI_TILE_COORD_X(s->max_tile_coord)) << MALI_TILE_SHIFT;
+        unsigned max_y_f = (MALI_TILE_COORD_Y(s->max_tile_coord)) << MALI_TILE_SHIFT;
 
-        const char *fbd_type = s->framebuffer & MALI_MFBD ? "MALI_MFBD" : "MALI_SFBD";
+        /* Validate the coordinates are well-ordered */
 
-        /* TODO: Decode */
-        unsigned extra_flags = (s->framebuffer & ~FBD_MASK) & ~MALI_MFBD;
+        if (min_x == max_x)
+                pandecode_msg("XXX: empty X coordinates (%u = %u)\n", min_x, max_x);
+        else if (min_x > max_x)
+                pandecode_msg("XXX: misordered X coordinates (%u > %u)\n", min_x, max_x);
 
-        if (fbd_dumped)
-                pandecode_prop("framebuffer = framebuffer_%d_p | %s | 0x%X", job_no,
-                                fbd_type, extra_flags);
-        else
-                pandecode_prop("framebuffer = %s | %s | 0x%X", pointer_as_memory_reference(p),
-                                fbd_type, extra_flags);
+        if (min_y == max_y)
+                pandecode_msg("XXX: empty X coordinates (%u = %u)\n", min_x, max_x);
+        else if (min_y > max_y)
+                pandecode_msg("XXX: misordered X coordinates (%u > %u)\n", min_x, max_x);
 
-        pandecode_indent--;
-        pandecode_log("};\n");
+        /* Validate the coordinates fit inside the framebuffer. We use floor,
+         * rather than ceil, for the max coordinates, since the tile
+         * coordinates for something like an 800x600 framebuffer will actually
+         * resolve to 800x608, which would otherwise trigger a Y-overflow */
+
+        if ((min_x > info.width) || (max_x_f > info.width))
+                pandecode_msg("XXX: tile coordinates overflow in X direction\n");
+
+        if ((min_y > info.height) || (max_y_f > info.height))
+                pandecode_msg("XXX: tile coordinates overflow in Y direction\n");
+
+        /* After validation, we print */
+
+        pandecode_log("fragment (%u, %u) ... (%u, %u)\n\n", min_x, min_y, max_x, max_y);
+
+        /* The FBD is a tagged pointer */
+
+        unsigned tag = (s->framebuffer & ~FBD_MASK);
+
+        if (tag != expected_tag)
+                pandecode_msg("XXX: expected FBD tag %X but got %X\n", expected_tag, tag);
 
         return sizeof(*s);
 }
@@ -2591,8 +2777,7 @@ pandecode_jc(mali_ptr jc_gpu_va, bool bifrost)
                              h->job_type != JOB_TYPE_FRAGMENT ? 4 : 0;
                 mali_ptr payload_ptr = jc_gpu_va + sizeof(*h) - offset;
 
-                payload = pandecode_fetch_gpu_mem(mem, payload_ptr,
-                                                  MALI_PAYLOAD_SIZE);
+                payload = pandecode_fetch_gpu_mem(mem, payload_ptr, 256);
 
                 int job_no = job_descriptor_number++;