From 0ef527928c6acc63ce88a8df023b64d2a5c4468c Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Mon, 1 Jun 2020 20:44:19 -0400 Subject: [PATCH] panfrost: Fix gl_PointSize out of GL_POINTS In this case, vs->writes_point_size is true as the VS writes gl_PointSize, but panfrost_writes_points_size() is false as we are not drawing points so the hardware doesn't process it. Thus the varying descriptor is emitted but elements is never written. When the VS runs, it will attempt to write to elements, a NULL pointer. The behaviour is architecture-independent. On Midgard, the write silently fails, hence why this bug was never noticed before. On Bifrost, this raises an MMU fault. The fix is to set the format to VARYING_DISCARD to ignore the write. Noticed on Neverball. Signed-off-by: Alyssa Rosenzweig Part-of: --- src/gallium/drivers/panfrost/pan_cmdstream.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/gallium/drivers/panfrost/pan_cmdstream.c b/src/gallium/drivers/panfrost/pan_cmdstream.c index a40a1505817..608d1799f97 100644 --- a/src/gallium/drivers/panfrost/pan_cmdstream.c +++ b/src/gallium/drivers/panfrost/pan_cmdstream.c @@ -1900,6 +1900,15 @@ panfrost_emit_varying_descriptor(struct panfrost_batch *batch, for (unsigned i = 0; i < vs->varying_count; i++) { gl_varying_slot loc = vs->varyings_loc[i]; + /* If we write gl_PointSize from the vertex shader but don't + * consume it, no memory will be allocated for it, so if we + * attempted to write anyway we would dereference a NULL + * pointer on the GPU. Midgard seems fine with this; Bifrost + * faults. */ + + if (loc == VARYING_SLOT_PSIZ && !panfrost_writes_point_size(ctx)) + ovs[i].format = MALI_VARYING_DISCARD; + bool captured = ((vs->so_mask & (1ll << loc)) ? true : false); if (!captured) continue; -- 2.30.2