Morning,
After playing UT2004 for 10 or so minutes, and then quickly checking
some other
apps known to worn, I see no regressions with either patch.
I'll be putting it through some more rigorous testing as the day
progresses, will
report back if I find anything.
Also, out of interest, what triggered the lockup you saw?
-Ben.
Nicolai Haehnle wrote:
>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.. */
>
>
|