panfrost: Implement gl_FragCoord correctly

Rather than passing through the transformed gl_Position, we can use the
hardware-level varying for this, which will correctly handle
gl_FragCoord.w

Signed-off-by: Alyssa Rosenzweig <alyssa.rosenzweig@collabora.com>

5 files changed, 32 insertions, 25 deletions

diff --git a/src/gallium/drivers/panfrost/ci/expected-failures.txt b/src/gallium/drivers/panfrost/ci/expected-failures.txt
index a15c3f9c6705..b0fc872a3009 100644
--- a/src/gallium/drivers/panfrost/ci/expected-failures.txt
+++ b/src/gallium/drivers/panfrost/ci/expected-failures.txt
@@ -876,6 +876,5 @@ dEQP-GLES2.functional.polygon_offset.default_factor_1_slope Fail
 dEQP-GLES2.functional.polygon_offset.default_render_with_units Fail
 dEQP-GLES2.functional.polygon_offset.fixed16_factor_1_slope Fail
 dEQP-GLES2.functional.polygon_offset.fixed16_render_with_units Fail
-dEQP-GLES2.functional.shaders.builtin_variable.fragcoord_w Fail
 dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_fragment Fail
 dEQP-GLES2.functional.shaders.scoping.valid.local_variable_hides_function_parameter_vertex Fail
diff --git a/src/gallium/drivers/panfrost/pan_assemble.c b/src/gallium/drivers/panfrost/pan_assemble.c
index 47f6c1e5312d..b57cd5ef6ad2 100644
--- a/src/gallium/drivers/panfrost/pan_assemble.c
+++ b/src/gallium/drivers/panfrost/pan_assemble.c
@@ -145,7 +145,10 @@ panfrost_shader_compile(
                 /* Check for special cases, otherwise assume general varying */
 
                 if (location == VARYING_SLOT_POS) {
-                        v.format = MALI_VARYING_POS;
+                        if (stage == MESA_SHADER_FRAGMENT)
+                                state->reads_frag_coord = true;
+                        else
+                                v.format = MALI_VARYING_POS;
                 } else if (location == VARYING_SLOT_PSIZ) {
                         v.format = MALI_R16F;
                         v.swizzle = default_vec1_swizzle;
diff --git a/src/gallium/drivers/panfrost/pan_context.h b/src/gallium/drivers/panfrost/pan_context.h
index 8efd97e779b6..4c1580b33931 100644
--- a/src/gallium/drivers/panfrost/pan_context.h
+++ b/src/gallium/drivers/panfrost/pan_context.h
@@ -231,6 +231,7 @@ struct panfrost_shader_state {
         bool writes_point_size;
         bool reads_point_coord;
         bool reads_face;
+        bool reads_frag_coord;
 
         struct mali_attr_meta varyings[PIPE_MAX_ATTRIBS];
         gl_varying_slot varyings_loc[PIPE_MAX_ATTRIBS];
diff --git a/src/gallium/drivers/panfrost/pan_varyings.c b/src/gallium/drivers/panfrost/pan_varyings.c
index 0ac3b9d88545..113043b699dc 100644
--- a/src/gallium/drivers/panfrost/pan_varyings.c
+++ b/src/gallium/drivers/panfrost/pan_varyings.c
@@ -73,19 +73,6 @@ panfrost_emit_streamout(
         slot->elements = addr;
 }
 
-static void
-panfrost_emit_point_coord(union mali_attr *slot)
-{
-        slot->elements = MALI_VARYING_POINT_COORD | MALI_ATTR_LINEAR;
-        slot->stride = slot->size = slot->shift = slot->extra_flags = 0;
-}
-
-static void
-panfrost_emit_front_face(union mali_attr *slot)
-{
-        slot->elements = MALI_VARYING_FRONT_FACING | MALI_ATTR_INTERNAL;
-}
-
 /* Given a shader and buffer indices, link varying metadata together */
 
 static bool
@@ -250,6 +237,7 @@ panfrost_emit_varying_descriptor(
         memcpy(trans.cpu + vs_size, fs->varyings, fs_size);
 
         union mali_attr varyings[PIPE_MAX_ATTRIBS];
+        memset(varyings, 0, sizeof(varyings));
 
         /* Figure out how many streamout buffers could be bound */
         unsigned so_count = ctx->streamout.num_targets;
@@ -269,6 +257,7 @@ panfrost_emit_varying_descriptor(
         signed gl_PointSize = vs->writes_point_size ? (idx++) : -1;
         signed gl_PointCoord = reads_point_coord ? (idx++) : -1;
         signed gl_FrontFacing = fs->reads_face ? (idx++) : -1;
+        signed gl_FragCoord = fs->reads_frag_coord ? (idx++) : -1;
 
         /* Emit the stream out buffers */
 
@@ -305,20 +294,25 @@ panfrost_emit_varying_descriptor(
                                                2, vertex_count);
 
         if (reads_point_coord)
-                panfrost_emit_point_coord(&varyings[gl_PointCoord]);
+                varyings[gl_PointCoord].elements = MALI_VARYING_POINT_COORD;
 
         if (fs->reads_face)
-                panfrost_emit_front_face(&varyings[gl_FrontFacing]);
+                varyings[gl_FrontFacing].elements = MALI_VARYING_FRONT_FACING;
+
+        if (fs->reads_frag_coord)
+                varyings[gl_FragCoord].elements = MALI_VARYING_FRAG_COORD;
 
         /* Let's go ahead and link varying meta to the buffer in question, now
-         * that that information is available */
+         * that that information is available. VARYING_SLOT_POS is mapped to
+         * gl_FragCoord for fragment shaders but gl_Positionf or vertex shaders
+         * */
 
         panfrost_emit_varying_meta(trans.cpu, vs,
                 general, gl_Position, gl_PointSize,
                 gl_PointCoord, gl_FrontFacing);
 
         panfrost_emit_varying_meta(trans.cpu + vs_size, fs,
-                general, gl_Position, gl_PointSize,
+                general, gl_FragCoord, gl_PointSize,
                 gl_PointCoord, gl_FrontFacing);
 
         /* Replace streamout */
@@ -376,6 +370,9 @@ panfrost_emit_varying_descriptor(
 
         /* Fix up unaligned addresses */
         for (unsigned i = 0; i < so_count; ++i) {
+                if (varyings[i].elements < MALI_VARYING_SPECIAL)
+                        continue;
+
                 unsigned align = (varyings[i].elements & 63);
 
                 /* While we're at it, the SO buffers are linear */
diff --git a/src/panfrost/include/panfrost-job.h b/src/panfrost/include/panfrost-job.h
index b19bfacf1295..6f8a757e2c69 100644
--- a/src/panfrost/include/panfrost-job.h
+++ b/src/panfrost/include/panfrost-job.h
@@ -804,9 +804,9 @@ struct mali_payload_set_value {
  * let shift=extra_flags=0. Stride is set to the image format's bytes-per-pixel
  * (*NOT the row stride*). Size is set to the size of the image itself.
  *
- * Special internal varyings (including gl_FrontFacing) are handled vai
- * MALI_ATTR_INTERNAL, which has all fields set to zero and uses a special
- * elements pseudo-pointer.
+ * Special internal varyings (including gl_FrontFacing) could be seen as
+ * IMAGE/INTERNAL as well as LINEAR, setting all fields set to zero and using a
+ * special elements pseudo-pointer.
  */
 
 enum mali_attr_mode {
@@ -819,16 +819,23 @@ enum mali_attr_mode {
         MALI_ATTR_INTERNAL = 6
 };
 
-/* Pseudo-address for gl_FrontFacing */
+/* Pseudo-address for gl_FrontFacing, used with INTERNAL. Same addres is used
+ * for gl_FragCoord with IMAGE, needing a coordinate flip. Who knows. */
 
-#define MALI_VARYING_FRONT_FACING (0x20)
+#define MALI_VARYING_FRAG_COORD (0x25)
+#define MALI_VARYING_FRONT_FACING (0x26)
 
 /* This magic "pseudo-address" is used as `elements` to implement
  * gl_PointCoord. When read from a fragment shader, it generates a point
  * coordinate per the OpenGL ES 2.0 specification. Flipped coordinate spaces
  * require an affine transformation in the shader. */
 
-#define MALI_VARYING_POINT_COORD (0x60)
+#define MALI_VARYING_POINT_COORD (0x61)
+
+/* Used for comparison to check if an address is special. Mostly a guess, but
+ * it doesn't really matter. */
+
+#define MALI_VARYING_SPECIAL (0x100)
 
 union mali_attr {
 	/* This is used for actual attributes. */