Hi everybody, I once again tripped upon an R300 lockup (possibly the same one that everybody's been talking about) and spent the last one and half days chasing it down.
It turns out that writing the vertex buffer age to scratch registers (the ones that are written back to main memory) during a 3D sequence is a bad idea. Apparently, this confuses the memory controller so much that some part of the engine locks up hard. The attached patch.out-of-loop-dispatch-age fixes this, at least for me. The attached patch.remove-userspace-pacifiers removes additional unnecessary emission of the pacifier sequence from the userspace driver. Userspace isn't supposed to emit this sequence anyway. Could everybody please test whether a) the first patch really does fix the lockups people are seeing and b) whether combining both the first and the second patch causes any regressions. If everything's fine with these patches, I'm going to commit them in a few days or so. cu, Nicolai
Index: drm/shared-core/r300_cmdbuf.c
===================================================================
RCS file: /cvsroot/r300/r300_driver/drm/shared-core/r300_cmdbuf.c,v
retrieving revision 1.22
diff -u -3 -p -r1.22 r300_cmdbuf.c
--- drm/shared-core/r300_cmdbuf.c 28 May 2005 05:18:42 -0000 1.22
+++ drm/shared-core/r300_cmdbuf.c 28 May 2005 20:56:59 -0000
@@ -487,21 +487,19 @@ static __inline__ void r300_pacify(drm_r
}
+/**
+ * Called by r300_do_cp_cmdbuf to update the internal buffer age and state.
+ * The actual age emit is done by r300_do_cp_cmdbuf, which is why you must
+ * be careful about how this function is called.
+ */
static void r300_discard_buffer(drm_device_t * dev, drm_buf_t * buf)
{
- drm_radeon_private_t *dev_priv = dev->dev_private;
- drm_radeon_buf_priv_t *buf_priv = buf->dev_private;
- RING_LOCALS;
-
- buf_priv->age = ++dev_priv->sarea_priv->last_dispatch;
-
- /* Emit the vertex buffer age */
- BEGIN_RING(2);
- RADEON_DISPATCH_AGE(buf_priv->age);
- ADVANCE_RING();
+ drm_radeon_private_t *dev_priv = dev->dev_private;
+ drm_radeon_buf_priv_t *buf_priv = buf->dev_private;
- buf->pending = 1;
- buf->used = 0;
+ buf_priv->age = dev_priv->sarea_priv->last_dispatch+1;
+ buf->pending = 1;
+ buf->used = 0;
}
@@ -518,6 +516,7 @@ int r300_do_cp_cmdbuf(drm_device_t* dev,
drm_radeon_private_t *dev_priv = dev->dev_private;
drm_device_dma_t *dma = dev->dma;
drm_buf_t *buf = NULL;
+ int emit_dispatch_age = 0;
int ret = 0;
DRM_DEBUG("\n");
@@ -608,14 +607,15 @@ int r300_do_cp_cmdbuf(drm_device_t* dev,
goto cleanup;
}
- r300_discard_buffer(dev, buf);
+ emit_dispatch_age = 1;
+ r300_discard_buffer(dev, buf);
break;
case R300_CMD_WAIT:
/* simple enough, we can do it here */
DRM_DEBUG("R300_CMD_WAIT\n");
if(header.wait.flags==0)break; /* nothing to do */
-
+
{
RING_LOCALS;
@@ -639,6 +639,24 @@ int r300_do_cp_cmdbuf(drm_device_t* dev,
cleanup:
r300_pacify(dev_priv);
+
+ /* We emit the vertex buffer age here, outside the pacifier "brackets"
+ * for two reasons:
+ * (1) This may coalesce multiple age emissions into a single one and
+ * (2) more importantly, some chips lock up hard when scratch registers
+ * are written inside the pacifier bracket.
+ */
+ if (emit_dispatch_age) {
+ RING_LOCALS;
+
+ dev_priv->sarea_priv->last_dispatch++;
+
+ /* Emit the vertex buffer age */
+ BEGIN_RING(2);
+ RADEON_DISPATCH_AGE(dev_priv->sarea_priv->last_dispatch);
+ ADVANCE_RING();
+ }
+
COMMIT_RING();
return ret;
Index: r300/r300_render.c
===================================================================
RCS file: /cvsroot/r300/r300_driver/r300/r300_render.c,v
retrieving revision 1.87
diff -u -3 -p -r1.87 r300_render.c
--- r300/r300_render.c 19 May 2005 00:03:52 -0000 1.87
+++ r300/r300_render.c 28 May 2005 20:49:43 -0000
@@ -489,23 +489,18 @@ static GLboolean r300_run_vb_render(GLco
struct vertex_buffer *VB = &tnl->vb;
int i, j;
LOCAL_VARS
-
+
if (RADEON_DEBUG & DEBUG_PRIMS)
fprintf(stderr, "%s\n", __FUNCTION__);
-
+
r300ReleaseArrays(ctx);
r300EmitArrays(ctx, GL_FALSE);
// LOCK_HARDWARE(&(rmesa->radeon));
- reg_start(R300_RB3D_DSTCACHE_CTLSTAT,0);
- e32(0x0000000a);
-
- reg_start(0x4f18,0);
- e32(0x00000003);
r300EmitState(rmesa);
-
+
if(hw_tcl_on) /* FIXME */
r300FlushCmdBuf(rmesa, __FUNCTION__);
@@ -515,16 +510,10 @@ static GLboolean r300_run_vb_render(GLco
GLuint prim = VB->Primitive[i].mode;
GLuint start = VB->Primitive[i].start;
GLuint length = VB->Primitive[i].count;
-
+
r300_render_vb_primitive(rmesa, ctx, start, start + length, prim);
}
- reg_start(R300_RB3D_DSTCACHE_CTLSTAT,0);
- e32(0x0000000a);
-
- reg_start(0x4f18,0);
- e32(0x00000003);
-
// end_3d(PASS_PREFIX_VOID);
/* Flush state - we are done drawing.. */
pgpo4sI9yBTrX.pgp
Description: PGP signature
