drm/nouveau: fix a race condition in nouveau_dma_wait()

Can be triggered easily on certain cards (NV46 and NV50 of mine) by
running "dmesg", the DRM's channel will lockup.

Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
This commit is contained in:
Ben Skeggs 2010-01-15 12:08:57 +10:00
parent 12f735b79f
commit ba59953d28
1 changed files with 47 additions and 29 deletions

View File

@ -126,47 +126,52 @@ OUT_RINGp(struct nouveau_channel *chan, const void *data, unsigned nr_dwords)
chan->dma.cur += nr_dwords; chan->dma.cur += nr_dwords;
} }
static inline bool /* Fetch and adjust GPU GET pointer
READ_GET(struct nouveau_channel *chan, uint32_t *get) *
* Returns:
* value >= 0, the adjusted GET pointer
* -EINVAL if GET pointer currently outside main push buffer
* -EBUSY if timeout exceeded
*/
static inline int
READ_GET(struct nouveau_channel *chan, uint32_t *prev_get, uint32_t *timeout)
{ {
uint32_t val; uint32_t val;
val = nvchan_rd32(chan, chan->user_get); val = nvchan_rd32(chan, chan->user_get);
if (val < chan->pushbuf_base ||
val > chan->pushbuf_base + (chan->dma.max << 2)) { /* reset counter as long as GET is still advancing, this is
/* meaningless to dma_wait() except to know whether the * to avoid misdetecting a GPU lockup if the GPU happens to
* GPU has stalled or not * just be processing an operation that takes a long time
*/ */
*get = val; if (val != *prev_get) {
return false; *prev_get = val;
*timeout = 0;
} }
*get = (val - chan->pushbuf_base) >> 2; if ((++*timeout & 0xff) == 0) {
return true; DRM_UDELAY(1);
if (*timeout > 100000)
return -EBUSY;
}
if (val < chan->pushbuf_base ||
val > chan->pushbuf_base + (chan->dma.max << 2))
return -EINVAL;
return (val - chan->pushbuf_base) >> 2;
} }
int int
nouveau_dma_wait(struct nouveau_channel *chan, int size) nouveau_dma_wait(struct nouveau_channel *chan, int size)
{ {
uint32_t get, prev_get = 0, cnt = 0; uint32_t prev_get = 0, cnt = 0;
bool get_valid; int get;
while (chan->dma.free < size) { while (chan->dma.free < size) {
/* reset counter as long as GET is still advancing, this is get = READ_GET(chan, &prev_get, &cnt);
* to avoid misdetecting a GPU lockup if the GPU happens to if (unlikely(get == -EBUSY))
* just be processing an operation that takes a long time return -EBUSY;
*/
get_valid = READ_GET(chan, &get);
if (get != prev_get) {
prev_get = get;
cnt = 0;
}
if ((++cnt & 0xff) == 0) {
DRM_UDELAY(1);
if (cnt > 100000)
return -EBUSY;
}
/* loop until we have a usable GET pointer. the value /* loop until we have a usable GET pointer. the value
* we read from the GPU may be outside the main ring if * we read from the GPU may be outside the main ring if
@ -177,7 +182,7 @@ nouveau_dma_wait(struct nouveau_channel *chan, int size)
* from the SKIPS area, so the code below doesn't have to deal * from the SKIPS area, so the code below doesn't have to deal
* with some fun corner cases. * with some fun corner cases.
*/ */
if (!get_valid || get < NOUVEAU_DMA_SKIPS) if (unlikely(get == -EINVAL) || get < NOUVEAU_DMA_SKIPS)
continue; continue;
if (get <= chan->dma.cur) { if (get <= chan->dma.cur) {
@ -203,6 +208,19 @@ nouveau_dma_wait(struct nouveau_channel *chan, int size)
* after processing the currently pending commands. * after processing the currently pending commands.
*/ */
OUT_RING(chan, chan->pushbuf_base | 0x20000000); OUT_RING(chan, chan->pushbuf_base | 0x20000000);
/* wait for GET to depart from the skips area.
* prevents writing GET==PUT and causing a race
* condition that causes us to think the GPU is
* idle when it's not.
*/
do {
get = READ_GET(chan, &prev_get, &cnt);
if (unlikely(get == -EBUSY))
return -EBUSY;
if (unlikely(get == -EINVAL))
continue;
} while (get <= NOUVEAU_DMA_SKIPS);
WRITE_PUT(NOUVEAU_DMA_SKIPS); WRITE_PUT(NOUVEAU_DMA_SKIPS);
/* we're now submitting commands at the start of /* we're now submitting commands at the start of