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>
diff --git a/drivers/gpu/drm/nouveau/nouveau_dma.c b/drivers/gpu/drm/nouveau/nouveau_dma.c
index 7afbe8b..50d9e67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dma.c
@@ -126,47 +126,52 @@
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;
}
- *get = (val - chan->pushbuf_base) >> 2;
- return true;
+ if ((++*timeout & 0xff) == 0) {
+ 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
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
@@ -177,7 +182,7 @@
* 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) {
@@ -203,6 +208,19 @@
* 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