Skip to content

Commit

Permalink
drm/nouveau: fix a race condition in nouveau_dma_wait()
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Ben Skeggs committed Jan 17, 2010
1 parent 12f735b commit ba59953
Showing 1 changed file with 47 additions and 29 deletions.
76 changes: 47 additions & 29 deletions drivers/gpu/drm/nouveau/nouveau_dma.c
Original file line number Diff line number Diff line change
Expand Up @@ -126,47 +126,52 @@ OUT_RINGp(struct nouveau_channel *chan, const void *data, unsigned nr_dwords)
chan->dma.cur += nr_dwords;
}

static inline bool
READ_GET(struct nouveau_channel *chan, uint32_t *get)
/* Fetch and adjust GPU GET pointer
*
* 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;

val = nvchan_rd32(chan, chan->user_get);
if (val < chan->pushbuf_base ||
val > chan->pushbuf_base + (chan->dma.max << 2)) {
/* meaningless to dma_wait() except to know whether the
* GPU has stalled or not
*/
*get = val;
return false;

/* reset counter as long as GET is still advancing, this is
* to avoid misdetecting a GPU lockup if the GPU happens to
* just be processing an operation that takes a long time
*/
if (val != *prev_get) {
*prev_get = val;
*timeout = 0;
}

if ((++*timeout & 0xff) == 0) {
DRM_UDELAY(1);
if (*timeout > 100000)
return -EBUSY;
}

*get = (val - chan->pushbuf_base) >> 2;
return true;
if (val < chan->pushbuf_base ||
val > chan->pushbuf_base + (chan->dma.max << 2))
return -EINVAL;

return (val - chan->pushbuf_base) >> 2;
}

int
nouveau_dma_wait(struct nouveau_channel *chan, int size)
{
uint32_t get, prev_get = 0, cnt = 0;
bool get_valid;
uint32_t prev_get = 0, cnt = 0;
int get;

while (chan->dma.free < size) {
/* reset counter as long as GET is still advancing, this is
* to avoid misdetecting a GPU lockup if the GPU happens to
* just be processing an operation that takes a long time
*/
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;
}
get = READ_GET(chan, &prev_get, &cnt);
if (unlikely(get == -EBUSY))
return -EBUSY;

/* loop until we have a usable GET pointer. the value
* we read from the GPU may be outside the main ring if
Expand All @@ -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
* with some fun corner cases.
*/
if (!get_valid || get < NOUVEAU_DMA_SKIPS)
if (unlikely(get == -EINVAL) || get < NOUVEAU_DMA_SKIPS)
continue;

if (get <= chan->dma.cur) {
Expand All @@ -203,6 +208,19 @@ nouveau_dma_wait(struct nouveau_channel *chan, int size)
* after processing the currently pending commands.
*/
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);

/* we're now submitting commands at the start of
Expand Down

0 comments on commit ba59953

Please sign in to comment.