From 626f825bcc91a3068e2e1c68e7467b42826c51ea Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 17 Aug 2004 20:10:29 +0000 Subject: [PATCH] Revert the move of lost_context setting to UNLOCK_HARDWARE that was done in the last commit. I've been convinced by keithw that it's sufficient, and put a note in the code about it. Close another race for state in the Clear functions. I made the situation worse in my last commit, but this should fix things. Might be a slight performance hit, which could be regained by splitting the R*_FIREVERTICES calls in r*Clear up so that the EmitState doesn't happen in a separate new cmdbuf. --- src/mesa/drivers/dri/r200/r200_ioctl.c | 25 ++++++++++++++++++--- src/mesa/drivers/dri/r200/r200_lock.h | 11 --------- src/mesa/drivers/dri/radeon/radeon_ioctl.c | 26 +++++++++++++++++++--- src/mesa/drivers/dri/radeon/radeon_lock.h | 11 --------- 4 files changed, 45 insertions(+), 28 deletions(-) diff --git a/src/mesa/drivers/dri/r200/r200_ioctl.c b/src/mesa/drivers/dri/r200/r200_ioctl.c index 5d084baf3e0..fb462e08509 100644 --- a/src/mesa/drivers/dri/r200/r200_ioctl.c +++ b/src/mesa/drivers/dri/r200/r200_ioctl.c @@ -132,6 +132,19 @@ int r200FlushCmdBufLocked( r200ContextPtr rmesa, const char * caller ) rmesa->store.statenr = 0; rmesa->store.cmd_used = 0; rmesa->dma.nr_released_bufs = 0; + /* Set lost_context so that the first state emit on the new buffer is a full + * one. This is because the context might get lost while preparing the next + * buffer, and when we lock and find out, we don't have the information to + * recreate the state. This function should always be called before the new + * buffer is begun, so it's sufficient to just set lost_context here. + * + * The alternative to this would be to copy out the state on unlock + * (approximately) and if we did lose the context, dispatch a cmdbuf to reset + * the state to that old copy before continuing with the accumulated command + * buffer. + */ + rmesa->lost_context = 1; + return ret; } @@ -563,9 +576,6 @@ static void r200Clear( GLcontext *ctx, GLbitfield mask, GLboolean all, return; } - /* Need to cope with lostcontext here as kernel relies on - * some residual state: - */ R200_FIREVERTICES( rmesa ); if ( mask & DD_FRONT_LEFT_BIT ) { @@ -603,6 +613,13 @@ static void r200Clear( GLcontext *ctx, GLbitfield mask, GLboolean all, cx += dPriv->x; cy = dPriv->y + dPriv->h - cy - ch; + /* We have to emit state along with the clear, since the kernel relies on + * some of it. The EmitState that was above R200_FIREVERTICES was an + * attempt to do that, except that another context may come in and cause us + * to lose our context while we're unlocked. + */ + r200EmitState( rmesa ); + LOCK_HARDWARE( rmesa ); /* Throttle the number of clear ioctls we do. @@ -635,6 +652,8 @@ static void r200Clear( GLcontext *ctx, GLbitfield mask, GLboolean all, } } + /* Send current state to the hardware */ + r200FlushCmdBufLocked( rmesa, __FUNCTION__ ); for ( i = 0 ; i < dPriv->numClipRects ; ) { GLint nr = MIN2( i + RADEON_NR_SAREA_CLIPRECTS, dPriv->numClipRects ); diff --git a/src/mesa/drivers/dri/r200/r200_lock.h b/src/mesa/drivers/dri/r200/r200_lock.h index c913bd5f629..587e4fe5ccd 100644 --- a/src/mesa/drivers/dri/r200/r200_lock.h +++ b/src/mesa/drivers/dri/r200/r200_lock.h @@ -98,23 +98,12 @@ extern int prevLockLine; DEBUG_LOCK(); \ } while (0) -/* Unlock the hardware. We must assume that state has been lost when we unlock, - * because when we next grab the lock (to emit an accumulated cmdbuf), we don't - * have the information to recreate the context state as of the last unlock in - * in the case that we did lose the context state. - * - * The alternative to this would be to copy out the state on unlock - * (approximately) and if we did lose the context, dispatch a cmdbuf to reset - * the state to that old copy before continuing with the accumulated command - * buffer. - */ #define UNLOCK_HARDWARE( rmesa ) \ do { \ DRM_UNLOCK( rmesa->dri.fd, \ rmesa->dri.hwLock, \ rmesa->dri.hwContext ); \ DEBUG_RESET(); \ - rmesa->lost_context = GL_TRUE; \ } while (0) #endif diff --git a/src/mesa/drivers/dri/radeon/radeon_ioctl.c b/src/mesa/drivers/dri/radeon/radeon_ioctl.c index 3cb7dca2150..999176e0e9c 100644 --- a/src/mesa/drivers/dri/radeon/radeon_ioctl.c +++ b/src/mesa/drivers/dri/radeon/radeon_ioctl.c @@ -544,6 +544,19 @@ static int radeonFlushCmdBufLocked( radeonContextPtr rmesa, rmesa->store.statenr = 0; rmesa->store.cmd_used = 0; rmesa->dma.nr_released_bufs = 0; + /* Set lost_context so that the first state emit on the new buffer is a full + * one. This is because the context might get lost while preparing the next + * buffer, and when we lock and find out, we don't have the information to + * recreate the state. This function should always be called before the new + * buffer is begun, so it's sufficient to just set lost_context here. + * + * The alternative to this would be to copy out the state on unlock + * (approximately) and if we did lose the context, dispatch a cmdbuf to reset + * the state to that old copy before continuing with the accumulated command + * buffer. + */ + rmesa->lost_context = 1; + return ret; } @@ -977,9 +990,6 @@ static void radeonClear( GLcontext *ctx, GLbitfield mask, GLboolean all, __FUNCTION__, all, cx, cy, cw, ch ); } - /* Need to cope with lostcontext here as kernel relies on - * some residual state: - */ RADEON_FIREVERTICES( rmesa ); if ( mask & DD_FRONT_LEFT_BIT ) { @@ -1018,6 +1028,13 @@ static void radeonClear( GLcontext *ctx, GLbitfield mask, GLboolean all, cx += dPriv->x; cy = dPriv->y + dPriv->h - cy - ch; + /* We have to emit state along with the clear, since the kernel relies on + * some of it. The EmitState that was above RADEON_FIREVERTICES was an + * attempt to do that, except that another context may come in and cause us + * to lose our context while we're unlocked. + */ + radeonEmitState( rmesa ); + LOCK_HARDWARE( rmesa ); /* Throttle the number of clear ioctls we do. @@ -1059,6 +1076,9 @@ static void radeonClear( GLcontext *ctx, GLbitfield mask, GLboolean all, } } + /* Send current state to the hardware */ + radeonFlushCmdBufLocked( rmesa, __FUNCTION__ ); + for ( i = 0 ; i < dPriv->numClipRects ; ) { GLint nr = MIN2( i + RADEON_NR_SAREA_CLIPRECTS, dPriv->numClipRects ); drm_clip_rect_t *box = dPriv->pClipRects; diff --git a/src/mesa/drivers/dri/radeon/radeon_lock.h b/src/mesa/drivers/dri/radeon/radeon_lock.h index e18a642088a..c2e0c3706b8 100644 --- a/src/mesa/drivers/dri/radeon/radeon_lock.h +++ b/src/mesa/drivers/dri/radeon/radeon_lock.h @@ -99,23 +99,12 @@ extern int prevLockLine; DEBUG_LOCK(); \ } while (0) -/* Unlock the hardware. We must assume that state has been lost when we unlock, - * because when we next grab the lock (to emit an accumulated cmdbuf), we don't - * have the information to recreate the context state as of the last unlock in - * in the case that we did lose the context state. - * - * The alternative to this would be to copy out the state on unlock - * (approximately) and if we did lose the context, dispatch a cmdbuf to reset - * the state to that old copy before continuing with the accumulated command - * buffer. - */ #define UNLOCK_HARDWARE( rmesa ) \ do { \ DRM_UNLOCK( rmesa->dri.fd, \ rmesa->dri.hwLock, \ rmesa->dri.hwContext ); \ DEBUG_RESET(); \ - rmesa->lost_context = GL_TRUE; \ } while (0) #endif -- 2.30.2