From 050b934a24072df862a1fdad424c8f33379e8e26 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Sat, 4 May 2019 21:57:01 +0000 Subject: [PATCH] panfrost: Refactor blend descriptors This commit does a fairly large cleanup of blend descriptors, although there should not be any functional changes. In particular, we split apart the Midgard and Bifrost blend descriptors, since they are radically different. From there, we can identify that the Midgard descriptor as previously written was really two render targets' descriptors stuck together. From this observation, we split the Midgard descriptor into what a single RT actually needs. This enables us to correctly dump blending configuration for MRT samples on Midgard. It also allows the Midgard and Bifrost blend code to peacefully coexist, with runtime selection rather than a #ifdef. So, as a bonus, this will help the future Bifrost effort, eliminating one major source of compile-time architectural divergence. Signed-off-by: Alyssa Rosenzweig --- .../drivers/panfrost/include/panfrost-job.h | 56 ++++--- src/gallium/drivers/panfrost/pan_context.c | 31 ++-- .../drivers/panfrost/pandecode/decode.c | 155 +++++++++--------- 3 files changed, 122 insertions(+), 120 deletions(-) diff --git a/src/gallium/drivers/panfrost/include/panfrost-job.h b/src/gallium/drivers/panfrost/include/panfrost-job.h index c2d922678b8..71ac054f7c3 100644 --- a/src/gallium/drivers/panfrost/include/panfrost-job.h +++ b/src/gallium/drivers/panfrost/include/panfrost-job.h @@ -415,25 +415,37 @@ enum mali_format { #define MALI_READS_ZS (1 << 12) #define MALI_READS_TILEBUFFER (1 << 16) -struct mali_blend_meta { -#ifndef BIFROST - /* Base value of 0x200. +/* The raw Midgard blend payload can either be an equation or a shader + * address, depending on the context */ + +union midgard_blend { + mali_ptr shader; + struct mali_blend_equation equation; +}; + +/* On MRT Midgard systems (using an MFBD), each render target gets its own + * blend descriptor */ + +struct midgard_blend_rt { + /* Flags base value of 0x200 to enable the render target. * OR with 0x1 for blending (anything other than REPLACE). - * OR with 0x2 for programmable blending + * OR with 0x2 for programmable blending with 0-2 registers + * OR with 0x3 for programmable blending with 2+ registers */ - u64 unk1; + u64 flags; + union midgard_blend blend; +} __attribute__((packed)); - union { - struct mali_blend_equation blend_equation_1; - mali_ptr blend_shader; - }; +/* On Bifrost systems (all MRT), each render target gets one of these + * descriptors */ + +struct bifrost_blend_rt { + /* This is likely an analogue of the flags on + * midgard_blend_rt */ - u64 zero2; - struct mali_blend_equation blend_equation_2; -#else u32 unk1; // = 0x200 - struct mali_blend_equation blend_equation; + struct mali_blend_equation equation; /* * - 0x19 normally * - 0x3 when this slot is unused (everything else is 0 except the index) @@ -479,11 +491,13 @@ struct mali_blend_meta { * in the same pool as the original shader. The kernel will * make sure this allocation is aligned to 2^24 bytes. */ - u32 blend_shader; + u32 shader; }; -#endif } __attribute__((packed)); +/* Descriptor for the shader. Following this is at least one, up to four blend + * descriptors for each active render target */ + struct mali_shader_meta { mali_ptr shader; u16 texture_count; @@ -584,17 +598,7 @@ struct mali_shader_meta { * MALI_HAS_BLEND_SHADER to decide how to interpret. */ - union { - mali_ptr blend_shader; - struct mali_blend_equation blend_equation; - }; - - /* There can be up to 4 blend_meta's. None of them are required for - * vertex shaders or the non-MRT case for Midgard (so the blob doesn't - * allocate any space). - */ - struct mali_blend_meta blend_meta[]; - + union midgard_blend blend; } __attribute__((packed)); /* This only concerns hardware jobs */ diff --git a/src/gallium/drivers/panfrost/pan_context.c b/src/gallium/drivers/panfrost/pan_context.c index 17b5b75db92..cab7c89ac8b 100644 --- a/src/gallium/drivers/panfrost/pan_context.c +++ b/src/gallium/drivers/panfrost/pan_context.c @@ -1000,7 +1000,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool with_vertex_data) * maybe both are read...?) */ if (ctx->blend->has_blend_shader) { - ctx->fragment_shader_core.blend_shader = ctx->blend->blend_shader; + ctx->fragment_shader_core.blend.shader = ctx->blend->blend_shader; } if (ctx->require_sfbd) { @@ -1010,7 +1010,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool with_vertex_data) * modes (so we're able to read back the destination buffer) */ if (!ctx->blend->has_blend_shader) { - memcpy(&ctx->fragment_shader_core.blend_equation, &ctx->blend->equation, sizeof(ctx->blend->equation)); + ctx->fragment_shader_core.blend.equation = ctx->blend->equation; } if (!no_blending) { @@ -1018,7 +1018,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool with_vertex_data) } } - size_t size = sizeof(struct mali_shader_meta) + sizeof(struct mali_blend_meta); + size_t size = sizeof(struct mali_shader_meta) + sizeof(struct midgard_blend_rt); struct panfrost_transfer transfer = panfrost_allocate_transient(ctx, size); memcpy(transfer.cpu, &ctx->fragment_shader_core, sizeof(struct mali_shader_meta)); @@ -1027,7 +1027,7 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool with_vertex_data) if (!ctx->require_sfbd) { /* Additional blend descriptor tacked on for jobs using MFBD */ - unsigned blend_count = 0; + unsigned blend_count = 0x200; if (ctx->blend->has_blend_shader) { /* For a blend shader, the bottom nibble corresponds to @@ -1045,25 +1045,20 @@ panfrost_emit_for_draw(struct panfrost_context *ctx, bool with_vertex_data) blend_count |= 0x1; } - /* Second blend equation is always a simple replace */ + struct midgard_blend_rt rts[4]; - uint64_t replace_magic = 0xf0122122; - struct mali_blend_equation replace_mode; - memcpy(&replace_mode, &replace_magic, sizeof(replace_mode)); + /* TODO: MRT */ - struct mali_blend_meta blend_meta[] = { - { - .unk1 = 0x200 | blend_count, - .blend_equation_1 = ctx->blend->equation, - .blend_equation_2 = replace_mode - }, - }; + for (unsigned i = 0; i < 1; ++i) { + rts[i].flags = blend_count; - if (ctx->blend->has_blend_shader) { - blend_meta[0].blend_shader = ctx->blend->blend_shader; + if (ctx->blend->has_blend_shader) + rts[i].blend.shader = ctx->blend->blend_shader; + else + rts[i].blend.equation = ctx->blend->equation; } - memcpy(transfer.cpu + sizeof(struct mali_shader_meta), blend_meta, sizeof(blend_meta)); + memcpy(transfer.cpu + sizeof(struct mali_shader_meta), rts, sizeof(rts[0]) * 1); } } diff --git a/src/gallium/drivers/panfrost/pandecode/decode.c b/src/gallium/drivers/panfrost/pandecode/decode.c index 9936249e524..9d9b9b31bcd 100644 --- a/src/gallium/drivers/panfrost/pandecode/decode.c +++ b/src/gallium/drivers/panfrost/pandecode/decode.c @@ -857,12 +857,12 @@ pandecode_replay_stencil(const char *name, const struct mali_stencil_test *stenc } static void -pandecode_replay_blend_equation(const struct mali_blend_equation *blend, const char *suffix) +pandecode_replay_blend_equation(const struct mali_blend_equation *blend) { if (blend->zero1) pandecode_msg("Blend zero tripped: %X\n", blend->zero1); - pandecode_log(".blend_equation%s = {\n", suffix); + pandecode_log(".equation = {\n"); pandecode_indent++; pandecode_prop("rgb_mode = 0x%X", blend->rgb_mode); @@ -876,6 +876,69 @@ pandecode_replay_blend_equation(const struct mali_blend_equation *blend, const c pandecode_log("},\n"); } +static mali_ptr +pandecode_bifrost_blend(void *descs, int job_no, int rt_no) +{ + struct bifrost_blend_rt *b = + ((struct bifrost_blend_rt *) descs) + rt_no; + + pandecode_log("struct bifrost_blend_rt blend_rt_%d_%d = {\n", job_no, rt_no); + pandecode_indent++; + + pandecode_prop("unk1 = 0x%" PRIx32, b->unk1); + /* TODO figure out blend shader enable bit */ + pandecode_replay_blend_equation(&b->equation); + pandecode_prop("unk2 = 0x%" PRIx16, b->unk2); + pandecode_prop("index = 0x%" PRIx16, b->index); + pandecode_prop("shader = 0x%" PRIx32, b->shader); + + pandecode_indent--; + pandecode_log("},\n"); + + return 0; +} + +static mali_ptr +pandecode_midgard_blend(union midgard_blend *blend, bool is_shader) +{ + pandecode_log(".blend = {\n"); + pandecode_indent++; + + if (is_shader) { + pandecode_replay_shader_address("shader", blend->shader); + } else { + pandecode_replay_blend_equation(&blend->equation); + } + + pandecode_indent--; + pandecode_log("},\n"); + + /* Return blend shader to disassemble if present */ + return is_shader ? (blend->shader & ~0xF) : 0; +} + +static mali_ptr +pandecode_midgard_blend_mrt(void *descs, int job_no, int rt_no) +{ + struct midgard_blend_rt *b = + ((struct midgard_blend_rt *) descs) + rt_no; + + /* Flags determine presence of blend shader */ + bool is_shader = (b->flags & 0xF) >= 0x2; + + pandecode_log("struct midgard_blend_rt blend_rt_%d_%d = {\n", job_no, rt_no); + pandecode_indent++; + + pandecode_prop("flags = 0x%" PRIx64, b->flags); + + mali_ptr shader = pandecode_midgard_blend(&b->blend, is_shader); + + pandecode_indent--; + pandecode_log("};\n"); + + return shader; +} + static int pandecode_replay_attribute_meta(int job_no, int count, const struct mali_vertex_tiler_postfix *v, bool varying, char *suffix) { @@ -1248,95 +1311,35 @@ pandecode_replay_vertex_tiler_postfix_pre(const struct mali_vertex_tiler_postfix pandecode_prop("unknown2_8 = 0x%" PRIx32, s->unknown2_8); - bool blend_shader = false; - if (!is_bifrost) { - if (s->unknown2_3 & MALI_HAS_BLEND_SHADER) { - blend_shader = true; - pandecode_replay_shader_address("blend_shader", s->blend_shader); - } else { - pandecode_replay_blend_equation(&s->blend_equation, ""); - } + /* TODO: Blend shaders routing/disasm */ + + pandecode_midgard_blend(&s->blend, false); } pandecode_indent--; pandecode_log("};\n"); - /* MRT blend fields are used whenever MFBD is used */ + /* MRT blend fields are used whenever MFBD is used, with + * per-RT descriptors */ if (job_type == JOB_TYPE_TILER) { - pandecode_log("struct mali_blend_meta blend_meta_%d[] = {\n", - job_no); - pandecode_indent++; - - int i; - - for (i = 0; i < 4; i++) { - const struct mali_blend_meta *b = &s->blend_meta[i]; - pandecode_log("{\n"); - pandecode_indent++; - -#ifndef BIFROST - pandecode_prop("unk1 = 0x%" PRIx64, b->unk1); - - /* Depending on unk1, we determine if there's a - * blend shader */ - - if ((b->unk1 & 0xF) >= 0x2) { - blend_shader = true; - pandecode_replay_shader_address("blend_shader", b->blend_shader); - } else { - pandecode_replay_blend_equation(&b->blend_equation_1, "_1"); - } - - /* This is always an equation, I think. If - * there's a shader, it just defaults to - * REPLACE (0x122) */ - pandecode_replay_blend_equation(&b->blend_equation_2, "_2"); - - if (b->zero2) { - pandecode_msg("blend zero tripped\n"); - pandecode_prop("zero2 = 0x%x", b->zero2); - } + void* blend_base = (void *) (s + 1); -#else + for (unsigned i = 0; i < 4; i++) { + mali_ptr shader = 0; - pandecode_prop("unk1 = 0x%" PRIx32, b->unk1); - /* TODO figure out blend shader enable bit */ - pandecode_replay_blend_equation(&b->blend_equation); - pandecode_prop("unk2 = 0x%" PRIx16, b->unk2); - pandecode_prop("index = 0x%" PRIx16, b->index); - pandecode_prop("unk3 = 0x%" PRIx32, b->unk3); -#endif - - pandecode_indent--; - pandecode_log("},\n"); - -#ifdef BIFROST - if (b->unk2 == 3) - break; -#else - /* TODO: What's this supposed to be? */ - if (b->unk1 & 0x200) - break; -#endif + if (is_bifrost) + shader = pandecode_bifrost_blend(blend_base, job_no, i); + else + shader = pandecode_midgard_blend_mrt(blend_base, job_no, i); + if (shader) + pandecode_shader_disassemble(shader, job_no, job_type, false); } - - pandecode_indent--; - pandecode_log("};\n"); - - /* This needs to be uploaded right after the - * shader_meta since it's technically part of the same - * (variable-size) structure. - */ } pandecode_shader_disassemble(shader_ptr, job_no, job_type, is_bifrost); - - if (!is_bifrost && blend_shader) - pandecode_shader_disassemble(s->blend_shader & ~0xF, job_no, job_type, false); - } else pandecode_msg("\n"); -- 2.30.2