From 3641dfe4367e37b3bbe125c9b18044a07e35e502 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 2 May 2022 09:19:41 -0400 Subject: [PATCH] panfrost: Flip point coords in hardware On Bifrost, this is very easy: there's an RSD bit to Y-flip gl_PointCoord. It should map perfectly to the Gallium bit. With this change, we no longer use lower_pntc_ytransform on Bifrost, saving a bit of ALU when reading point coordinates. On Valhall, this is quite hard: the bit is in the framebuffer descriptor now! That means it can't be changed in a batch. This is expected to be ok: on GLES and VK, the origin is controlled only by the framebuffer orientation. It's a bigger problem on big GL, where GL_POINT_SPRITE_COORD_ORIGIN can be set freely. To cope, a tri-state data structure is used for the state tracking. This has a failure case on Valhall: every draw toggling the coord origin. However, the intention of the ORIGIN state bit is smoothing over coordinate system differences; it should never /actually/ change once set. Until we see an app doing something so stupid, I don't think we should worry about. We need all the Valhall tri-state infrastructure for handling provoking vertices on big GL anyway. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_cmdstream.c | 34 ++++++++++--- src/gallium/drivers/panfrost/pan_job.c | 1 + src/gallium/drivers/panfrost/pan_job.h | 51 ++++++++++++++++++++ src/panfrost/bifrost/bifrost_compile.h | 1 - src/panfrost/lib/pan_cs.c | 4 ++ src/panfrost/lib/pan_cs.h | 3 ++ 6 files changed, 85 insertions(+), 9 deletions(-) diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c index 1823144db6f..82e57afd027 100644 --- a/src/gallium/drivers/panfrost/pan_cmdstream.c +++ b/src/gallium/drivers/panfrost/pan_cmdstream.c @@ -573,6 +573,13 @@ panfrost_prepare_fs_state(struct panfrost_context *ctx, cfg.multisample_misc.evaluate_per_sample = true; cfg.preload.fragment.sample_mask_id = true; } + + /* Flip gl_PointCoord (and point sprites) depending on API + * setting on framebuffer orientation. We do not use + * lower_wpos_pntc on Bifrost. + */ + cfg.properties.point_sprite_coord_origin_max_y = + (rast->sprite_coord_mode == PIPE_SPRITE_COORD_LOWER_LEFT); #endif cfg.stencil_mask_misc.alpha_to_coverage = alpha_to_coverage; @@ -3780,6 +3787,21 @@ panfrost_indirect_draw(struct panfrost_batch *batch, } #endif +static bool +panfrost_compatible_batch_state(struct panfrost_batch *batch) +{ + /* Only applies on Valhall */ + if (PAN_ARCH < 9) + return true; + + struct panfrost_context *ctx = batch->ctx; + struct pipe_rasterizer_state *rast = &ctx->rasterizer->base; + + bool coord = (rast->sprite_coord_mode == PIPE_SPRITE_COORD_LOWER_LEFT); + + return pan_tristate_set(&batch->sprite_coord_origin, coord); +} + static void panfrost_draw_vbo(struct pipe_context *pipe, const struct pipe_draw_info *info, @@ -3810,6 +3832,9 @@ panfrost_draw_vbo(struct pipe_context *pipe, if (unlikely(batch->scoreboard.job_index > 10000)) batch = panfrost_get_fresh_batch_for_fbo(ctx, "Too many draws"); + if (unlikely(!panfrost_compatible_batch_state(batch))) + batch = panfrost_get_fresh_batch_for_fbo(ctx, "State change"); + /* panfrost_batch_skip_rasterization reads * batch->scissor_culls_everything, which is set by * panfrost_emit_viewport, so call that first. @@ -4371,14 +4396,7 @@ prepare_shader(struct panfrost_shader_state *state, pan_pack(out, RENDERER_STATE, cfg) { pan_shader_prepare_rsd(&state->info, state->bin.gpu, &cfg); -#if PAN_ARCH >= 6 - /* Match the mesa/st convention. If this needs to be flipped, - * nir_lower_pntc_ytransform will do so. - */ - if (state->info.stage == MESA_SHADER_FRAGMENT) - cfg.properties.point_sprite_coord_origin_max_y = true; -#endif - } + } #else assert(upload); diff --git a/src/gallium/drivers/panfrost/pan_job.c b/src/gallium/drivers/panfrost/pan_job.c index 25d634baf8b..6e051fd892d 100644 --- a/src/gallium/drivers/panfrost/pan_job.c +++ b/src/gallium/drivers/panfrost/pan_job.c @@ -435,6 +435,7 @@ panfrost_batch_to_fb_info(const struct panfrost_batch *batch, fb->extent.maxy = batch->maxy - 1; fb->nr_samples = util_framebuffer_get_num_samples(&batch->key); fb->rt_count = batch->key.nr_cbufs; + fb->sprite_coord_origin = pan_tristate_get(batch->sprite_coord_origin); static const unsigned char id_swz[] = { PIPE_SWIZZLE_X, PIPE_SWIZZLE_Y, PIPE_SWIZZLE_Z, PIPE_SWIZZLE_W, diff --git a/src/gallium/drivers/panfrost/pan_job.h b/src/gallium/drivers/panfrost/pan_job.h index a905ec028f6..94635b3cc90 100644 --- a/src/gallium/drivers/panfrost/pan_job.h +++ b/src/gallium/drivers/panfrost/pan_job.h @@ -33,6 +33,52 @@ #include "pan_resource.h" #include "pan_scoreboard.h" +/* Simple tri-state data structure. In the default "don't care" state, the value + * may be set to true or false. However, once the value is set, it must not be + * changed. Declared inside of a struct to prevent casting to bool, which is an + * error. The getter needs to be used instead. + */ +struct pan_tristate { + enum { + PAN_TRISTATE_DONTCARE, + PAN_TRISTATE_FALSE, + PAN_TRISTATE_TRUE, + } v; +}; + +/* + * Try to set a tristate value to a desired boolean value. Returns whether the + * operation is successful. + */ +static bool +pan_tristate_set(struct pan_tristate *state, bool value) +{ + switch (state->v) { + case PAN_TRISTATE_DONTCARE: + state->v = value ? PAN_TRISTATE_TRUE : PAN_TRISTATE_FALSE; + return true; + + case PAN_TRISTATE_FALSE: + return (value == false); + + case PAN_TRISTATE_TRUE: + return (value == true); + + default: + unreachable("Invalid tristate value"); + } +} + +/* + * Read the boolean value of a tristate. Return value undefined in the don't + * care state. + */ +static bool +pan_tristate_get(struct pan_tristate state) +{ + return (state.v == PAN_TRISTATE_TRUE); +} + /* A panfrost_batch corresponds to a bound FBO we're rendering to, * collecting over multiple draws. */ @@ -141,6 +187,11 @@ struct panfrost_batch { */ mali_ptr images[PIPE_SHADER_TYPES]; + /* On Valhall, these are properties of the batch. On Bifrost, they are + * per draw. + */ + struct pan_tristate sprite_coord_origin; + /* Referenced resources */ struct set *resources; }; diff --git a/src/panfrost/bifrost/bifrost_compile.h b/src/panfrost/bifrost/bifrost_compile.h index 7a8d24c03f5..746a32ce7d0 100644 --- a/src/panfrost/bifrost/bifrost_compile.h +++ b/src/panfrost/bifrost/bifrost_compile.h @@ -48,7 +48,6 @@ static const nir_shader_compiler_options bifrost_nir_options = { .lower_fdph = true, .lower_fsqrt = true, - .lower_wpos_pntc = true, .lower_fsign = true, .lower_bitfield_insert_to_shifts = true, diff --git a/src/panfrost/lib/pan_cs.c b/src/panfrost/lib/pan_cs.c index e0f451c3f9e..4ca908dbcbb 100644 --- a/src/panfrost/lib/pan_cs.c +++ b/src/panfrost/lib/pan_cs.c @@ -690,6 +690,10 @@ GENX(pan_emit_fbd)(const struct panfrost_device *dev, *valid |= full; } + +#if PAN_ARCH >= 9 + cfg.point_sprite_coord_origin_max_y = fb->sprite_coord_origin; +#endif } #if PAN_ARCH >= 6 diff --git a/src/panfrost/lib/pan_cs.h b/src/panfrost/lib/pan_cs.h index 27d030efdf2..6e134d76556 100644 --- a/src/panfrost/lib/pan_cs.h +++ b/src/panfrost/lib/pan_cs.h @@ -117,6 +117,9 @@ struct pan_fb_info { union { struct pan_fb_bifrost_info bifrost; }; + + /* Only used on Valhall */ + bool sprite_coord_origin; }; static inline unsigned