All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH/TESTING(all hw)/DISCUSSION] FIFO (minor) create and (major) destroy instabilities on nv50+
@ 2010-01-02 15:36 Maarten Maathuis
       [not found] ` <6d4bc9fc1001020736r4b17971ftb5e7c718433df181-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 17+ messages in thread
From: Maarten Maathuis @ 2010-01-02 15:36 UTC (permalink / raw)
  To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

[-- Attachment #1: Type: text/plain, Size: 1371 bytes --]

Many people using nv50+ hardware are aware of gpu lockups when a fifo
closes under certain conditions. Based on a mmio-trace and some trail
and error testing i've come up with a patch that improves the
situation on my NV96.

This patch needs testing on NV50+ hardware and regression testing on
older hardware, since i did change some of the common codepaths. This
is very much a work in progress, and if you have anything to
add/correct, please share it.

I've also attached a 2 test apps, once is bitscan-fail from mwk, use
it like ./bitscan-fail 0x200 to trigger PGRAPH errors. A modified
version only emits NOPs (method 0x100) and represents the no error
situation.

For me, i can run the NOP program in loops of 10000 iterations with no
problems (i've done so several times), the bitscan-fail survives 10000
iterations sometimes, but can also fail after a few thousand. In
comparison, a single run of bitscan-fail could cause a gpu lockup for
me in the past.

Please try the gallium driver, the test apps, suspend to ram. Suspend
to ram isn't 100% reliable yet for me (this was always the case after
strange experiments/hammering/etc), but should not regress. This goes
for older hw as well, whatever worked should still work, but i
wouldn't expect serious improvements there.

As always, feedback is appreciated, especially since this is a touchy subject.

Maarten.

[-- Attachment #2: test_better_context_handling_v4.patch --]
[-- Type: text/x-patch, Size: 9043 bytes --]

diff --git a/drivers/gpu/drm/nouveau/nouveau_channel.c b/drivers/gpu/drm/nouveau/nouveau_channel.c
index 4f378b6..e5a077d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_channel.c
+++ b/drivers/gpu/drm/nouveau/nouveau_channel.c
@@ -204,12 +204,14 @@ nouveau_channel_alloc(struct drm_device *dev, struct nouveau_channel **chan_ret,
 	/* disable the fifo caches */
 	pfifo->reassign(dev, false);
 
+	pgraph->fifo_access(dev, false);
 	/* Create a graphics context for new channel */
 	ret = pgraph->create_context(chan);
 	if (ret) {
 		nouveau_channel_free(chan);
 		return ret;
 	}
+	pgraph->fifo_access(dev, true);
 
 	/* Construct inital RAMFC for new channel */
 	ret = pfifo->create_context(chan);
@@ -284,30 +286,23 @@ nouveau_channel_free(struct nouveau_channel *chan)
 	struct drm_nouveau_private *dev_priv = dev->dev_private;
 	struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;
 	struct nouveau_fifo_engine *pfifo = &dev_priv->engine.fifo;
-	unsigned long flags;
+	struct nouveau_fence *fence = NULL;
 	int ret;
 
 	NV_INFO(dev, "%s: freeing fifo %d\n", __func__, chan->id);
 
 	nouveau_debugfs_channel_fini(chan);
 
-	/* Give outstanding push buffers a chance to complete */
-	spin_lock_irqsave(&chan->fence.lock, flags);
-	nouveau_fence_update(chan);
-	spin_unlock_irqrestore(&chan->fence.lock, flags);
-	if (chan->fence.sequence != chan->fence.sequence_ack) {
-		struct nouveau_fence *fence = NULL;
-
-		ret = nouveau_fence_new(chan, &fence, true);
-		if (ret == 0) {
-			ret = nouveau_fence_wait(fence, NULL, false, false);
-			nouveau_fence_unref((void *)&fence);
-		}
-
-		if (ret)
-			NV_ERROR(dev, "Failed to idle channel %d.\n", chan->id);
+	/* Unconditionally emit a fence, to be sure it's done. */
+	ret = nouveau_fence_new(chan, &fence, true);
+	if (ret == 0) {
+		ret = nouveau_fence_wait(fence, NULL, false, false);
+		nouveau_fence_unref((void *)&fence);
 	}
 
+	if (ret)
+		NV_ERROR(dev, "Failed to idle channel %d.\n", chan->id);
+
 	/* Ensure all outstanding fences are signaled.  They should be if the
 	 * above attempts at idling were OK, but if we failed this'll tell TTM
 	 * we're done with the buffers.
@@ -317,13 +312,6 @@ nouveau_channel_free(struct nouveau_channel *chan)
 	/* Ensure the channel is no longer active on the GPU */
 	pfifo->reassign(dev, false);
 
-	if (pgraph->channel(dev) == chan) {
-		pgraph->fifo_access(dev, false);
-		pgraph->unload_context(dev);
-		pgraph->fifo_access(dev, true);
-	}
-	pgraph->destroy_context(chan);
-
 	if (pfifo->channel_id(dev) == chan->id) {
 		pfifo->disable(dev);
 		pfifo->unload_context(dev);
@@ -331,6 +319,13 @@ nouveau_channel_free(struct nouveau_channel *chan)
 	}
 	pfifo->destroy_context(chan);
 
+	if (pgraph->channel(dev) == chan) {
+		pgraph->fifo_access(dev, false);
+		pgraph->unload_context(dev);
+		pgraph->fifo_access(dev, true);
+	}
+	pgraph->destroy_context(chan);
+
 	pfifo->reassign(dev, true);
 
 	/* Release the channel's resources */
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.c b/drivers/gpu/drm/nouveau/nouveau_drv.c
index 58c7172..bdb9e3b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.c
@@ -185,6 +185,10 @@ nouveau_pci_suspend(struct pci_dev *pdev, pm_message_t pm_state)
 	pfifo->disable(dev);
 	pfifo->unload_context(dev);
 	pgraph->unload_context(dev);
+	/* This may seem odd, but without it nv50 doesn't resume again.
+	 * It's specifically bit 0 from register 0x00400500 that needs to be set.
+	 */
+	pgraph->fifo_access(dev, true);
 
 	NV_INFO(dev, "Suspending GPU objects...\n");
 	ret = nouveau_gpuobj_suspend(dev);
diff --git a/drivers/gpu/drm/nouveau/nouveau_irq.c b/drivers/gpu/drm/nouveau/nouveau_irq.c
index 370c72c..43ac594 100644
--- a/drivers/gpu/drm/nouveau/nouveau_irq.c
+++ b/drivers/gpu/drm/nouveau/nouveau_irq.c
@@ -579,11 +579,14 @@ nv50_pgraph_irq_handler(struct drm_device *dev)
 	}
 
 	if (status & 0x00001000) {
-		nv_wr32(dev, 0x400500, 0x00000000);
+		struct drm_nouveau_private *dev_priv = dev->dev_private;
+		struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;
+
+		pgraph->fifo_access(dev, false);
 		nv_wr32(dev, NV03_PGRAPH_INTR, NV_PGRAPH_INTR_CONTEXT_SWITCH);
 		nv_wr32(dev, NV40_PGRAPH_INTR_EN, nv_rd32(dev,
 			NV40_PGRAPH_INTR_EN) & ~NV_PGRAPH_INTR_CONTEXT_SWITCH);
-		nv_wr32(dev, 0x400500, 0x00010001);
+		pgraph->fifo_access(dev, true);
 
 		nv50_graph_context_switch(dev);
 
diff --git a/drivers/gpu/drm/nouveau/nv50_fifo.c b/drivers/gpu/drm/nouveau/nv50_fifo.c
index b728228..3eea0d5 100644
--- a/drivers/gpu/drm/nouveau/nv50_fifo.c
+++ b/drivers/gpu/drm/nouveau/nv50_fifo.c
@@ -89,18 +89,39 @@ static void
 nv50_fifo_channel_disable(struct drm_device *dev, int channel, bool nt)
 {
 	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;
 	uint32_t inst;
 
 	NV_DEBUG(dev, "ch%d, nt=%d\n", channel, nt);
 
+	if (!nt) {
+		nv50_fifo_init_thingo(dev);
+
+		nv_wr32(dev, 0x2504, nv_rd32(dev, 0x2504) | 1);
+		if (!nv_wait(0x2504, 0x11, 0x11))
+			NV_ERROR(dev, "0x2504 timed out, 0x%08X\n",
+				nv_rd32(dev, 0x2504));
+
+		/* This is PGRAPH, but the blob does it "here".
+		 * I've seen both cases where the blob only reads and where
+		 * it also writes. I can only assume it's related to the pgraph
+		 * channel. It's done in pgraph unload context if it doesn't
+		 * happen here.
+		 */
+		if (pgraph->channel(dev) == dev_priv->fifos[channel]) {
+			inst = nv_rd32(dev, NV50_PGRAPH_CTXCTL_CUR)
+					& NV50_PGRAPH_CTXCTL_CUR_INSTANCE;
+			nv_wr32(dev, NV50_PGRAPH_CTXCTL_CUR, inst);
+		}
+
+		nv_wr32(dev, 0x2504, nv_rd32(dev, 0x2504) & ~1);
+	}
+
 	if (IS_G80)
 		inst = NV50_PFIFO_CTX_TABLE_INSTANCE_MASK_G80;
 	else
 		inst = NV50_PFIFO_CTX_TABLE_INSTANCE_MASK_G84;
 	nv_wr32(dev, NV50_PFIFO_CTX_TABLE(channel), inst);
-
-	if (!nt)
-		nv50_fifo_init_thingo(dev);
 }
 
 static void
diff --git a/drivers/gpu/drm/nouveau/nv50_graph.c b/drivers/gpu/drm/nouveau/nv50_graph.c
index 444a46b..583d2eb 100644
--- a/drivers/gpu/drm/nouveau/nv50_graph.c
+++ b/drivers/gpu/drm/nouveau/nv50_graph.c
@@ -152,10 +152,21 @@ nv50_graph_fifo_access(struct drm_device *dev, bool enabled)
 {
 	const uint32_t mask = 0x00010001;
 
-	if (enabled)
+	if (enabled) {
 		nv_wr32(dev, 0x400500, nv_rd32(dev, 0x400500) | mask);
-	else
+			nv_wr32(dev, 0x100c80, 0x00000001);
+		if (!nv_wait(0x100c80, 0x00000001, 0x00000000)) {
+			NV_ERROR(dev, "timeout: (0x100c80 & 1) == 0 (2)\n");
+			NV_ERROR(dev, "0x100c80 = 0x%08x\n", nv_rd32(dev, 0x100c80));
+		}
+	} else {
+		nv_wr32(dev, 0x100c80, 0x00050001);
+		if (!nv_wait(0x100c80, 0x00000001, 0x00000000)) {
+			NV_ERROR(dev, "timeout: (0x100c80 & 1) == 0 (2)\n");
+			NV_ERROR(dev, "0x100c80 = 0x%08x\n", nv_rd32(dev, 0x100c80));
+		}
 		nv_wr32(dev, 0x400500, nv_rd32(dev, 0x400500) & ~mask);
+	}
 }
 
 struct nouveau_channel *
@@ -275,29 +286,46 @@ nv50_graph_load_context(struct nouveau_channel *chan)
 int
 nv50_graph_unload_context(struct drm_device *dev)
 {
-	uint32_t inst, fifo = nv_rd32(dev, 0x400500);
-
-	inst  = nv_rd32(dev, NV50_PGRAPH_CTXCTL_CUR);
-	if (!(inst & NV50_PGRAPH_CTXCTL_CUR_LOADED))
-		return 0;
-	inst &= NV50_PGRAPH_CTXCTL_CUR_INSTANCE;
-
-	nv_wr32(dev, 0x400500, fifo & ~1);
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;
+	struct nouveau_fifo_engine *pfifo = &dev_priv->engine.fifo;
+	uint32_t tmp, inst;
+
+	inst = nv_rd32(dev, NV50_PGRAPH_CTXCTL_CUR)
+				& NV50_PGRAPH_CTXCTL_CUR_INSTANCE;
+
+	/* The blob clearly puts this in the "middle" of pfifo context unload,
+	 * but do it here in case it didn't happen in pfifo unload context.
+	 */
+	if (pfifo->channel_id(dev) != pgraph->channel(dev)->id)
+		nv_wr32(dev, NV50_PGRAPH_CTXCTL_CUR, inst);
+
+	/* These 3 registers are important for suspend.
+	 * They don't seem to be needed for normal running.
+	 */
 	nv_wr32(dev, 0x400784, inst);
 	nv_wr32(dev, 0x400824, nv_rd32(dev, 0x400824) | 0x20);
 	nv_wr32(dev, 0x400304, nv_rd32(dev, 0x400304) | 0x01);
 	nouveau_wait_for_idle(dev);
-	nv_wr32(dev, 0x400500, fifo);
 
-	nv_wr32(dev, NV50_PGRAPH_CTXCTL_CUR, inst);
+	if ((tmp = nv_rd32(dev, 0x400380)) != 0)
+		NV_ERROR(dev, "0x400380 has nonzero value 0x%08X\n", tmp);
+	if ((tmp = nv_rd32(dev, 0x400384)) != 0)
+		NV_ERROR(dev, "0x400384 has nonzero value 0x%08X\n", tmp);
+	if ((tmp = nv_rd32(dev, 0x400388)) != 0)
+		NV_ERROR(dev, "0x400388 has nonzero value 0x%08X\n", tmp);
+
 	return 0;
 }
 
 void
 nv50_graph_context_switch(struct drm_device *dev)
 {
+	struct drm_nouveau_private *dev_priv = dev->dev_private;
+	struct nouveau_pgraph_engine *pgraph = &dev_priv->engine.graph;
 	uint32_t inst;
 
+	pgraph->fifo_access(dev, false);
 	nv50_graph_unload_context(dev);
 
 	inst  = nv_rd32(dev, NV50_PGRAPH_CTXCTL_NEXT);
@@ -306,6 +334,7 @@ nv50_graph_context_switch(struct drm_device *dev)
 
 	nv_wr32(dev, NV40_PGRAPH_INTR_EN, nv_rd32(dev,
 		NV40_PGRAPH_INTR_EN) | NV_PGRAPH_INTR_CONTEXT_SWITCH);
+	pgraph->fifo_access(dev, true);
 }
 
 static int

[-- Attachment #3: nop.c --]
[-- Type: application/octet-stream, Size: 1237 bytes --]

#include <xf86drm.h>
#include "nouveau_drm.h"
#include "nouveau_device.h"
#include "nouveau_notifier.h"
#include "nouveau_channel.h"
#include "nouveau_pushbuf.h"
#include "nouveau_bo.h"
#include "nouveau_class.h"
#include <stdio.h>
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char **argv) {
	struct nouveau_device *dev = 0;
	int err;
	int i;
	if (err = nouveau_device_open(&dev, 0)) {
		printf ("open: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_channel *chan = 0;
	if (err = nouveau_channel_alloc(dev, 0xdeadbeef, 0xdeadbeef, &chan)) {
		printf ("chan: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_grobj *turing = 0;
	if (err = nouveau_grobj_alloc(chan, 0xdeadd00d, 0x50c0, &turing)) {
		printf ("turing: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_notifier *sync = 0;
	if (err = nouveau_notifier_alloc(chan, 0xbeef0301, 1, &sync)) {
		printf ("sync: %s\n", strerror(-err));
		return 1;
	}

	BEGIN_RING(chan, turing, 0x180, 1);
	OUT_RING(chan, sync->handle);
	BEGIN_RING(chan, turing, 0x60, 1);
	OUT_RING(chan, sync->handle);
	for (i = 0; i < 33; i++) {
		BEGIN_RING(chan, turing, 0x100, 1);
		OUT_RING(chan, 0);
	}
	FIRE_RING(chan);

	return 0;
}

[-- Attachment #4: bitscan-fail.c --]
[-- Type: application/octet-stream, Size: 1265 bytes --]

#include <xf86drm.h>
#include "nouveau_drm.h"
#include "nouveau_device.h"
#include "nouveau_notifier.h"
#include "nouveau_channel.h"
#include "nouveau_pushbuf.h"
#include "nouveau_bo.h"
#include "nouveau_class.h"
#include <stdio.h>
#include <string.h>
#include <stdint.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char **argv) {
	struct nouveau_device *dev = 0;
	int err;
	int i;
	if (err = nouveau_device_open(&dev, 0)) {
		printf ("open: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_channel *chan = 0;
	if (err = nouveau_channel_alloc(dev, 0xdeadbeef, 0xdeadbeef, &chan)) {
		printf ("chan: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_grobj *turing = 0;
	if (err = nouveau_grobj_alloc(chan, 0xdeadd00d, 0x50c0, &turing)) {
		printf ("turing: %s\n", strerror(-err));
		return 1;
	}
	struct nouveau_notifier *sync = 0;
	if (err = nouveau_notifier_alloc(chan, 0xbeef0301, 1, &sync)) {
		printf ("sync: %s\n", strerror(-err));
		return 1;
	}

	BEGIN_RING(chan, turing, 0x180, 1);
	OUT_RING(chan, sync->handle);
	BEGIN_RING(chan, turing, 0x60, 1);
	OUT_RING(chan, sync->handle);
	for (i = 0; i < 33; i++) {
		BEGIN_RING(chan, turing, strtol(argv[1], 0, 0), 1);
		OUT_RING(chan, (i<32?1<<i:0));
	}
	FIRE_RING(chan);

	return 0;
}

[-- Attachment #5: Makefile --]
[-- Type: application/octet-stream, Size: 340 bytes --]

HEADERS=
CFLAGS =`pkg-config --cflags libdrm_nouveau`
LDFLAGS=`pkg-config --libs libdrm_nouveau`

all: bitscan-fail nop

bitscan-fail: bitscan-fail.c $(HEADERS)
	gcc -ggdb3 $(CFLAGS) -o bitscan-fail bitscan-fail.c $(LDFLAGS)

nop: nop.c $(HEADERS)
	gcc -ggdb3 $(CFLAGS) -o nop nop.c $(LDFLAGS)

clean:
	rm -f bitscan-fail

.PHONY: clean run

[-- Attachment #6: Type: text/plain, Size: 181 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
http://lists.freedesktop.org/mailman/listinfo/nouveau

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-01-10 20:58 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-02 15:36 [PATCH/TESTING(all hw)/DISCUSSION] FIFO (minor) create and (major) destroy instabilities on nv50+ Maarten Maathuis
     [not found] ` <6d4bc9fc1001020736r4b17971ftb5e7c718433df181-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-02 15:39   ` Maarten Maathuis
     [not found]     ` <6d4bc9fc1001020739y57ad5e81u19df23fd127350bb-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-03  0:37       ` Johannes Obermayr
     [not found]         ` <201001030137.19767.johannesobermayr-Mmb7MZpHnFY@public.gmane.org>
2010-01-03  2:45           ` Maarten Maathuis
2010-01-04 19:29   ` Maarten Maathuis
     [not found]     ` <6d4bc9fc1001041129t5ac01715oe64f3e827c01340b-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-04 22:39       ` Ben Skeggs
2010-01-04 22:54         ` Maarten Maathuis
     [not found]           ` <6d4bc9fc1001041454w63d62e7fk7dec9aa2922462f8-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-05  3:20             ` Ben Skeggs
2010-01-05  8:41               ` Maarten Maathuis
     [not found]                 ` <6d4bc9fc1001050041w3cefcaacs287d6c1909c182d0-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-05 21:19                   ` Maarten Maathuis
     [not found]                     ` <6d4bc9fc1001051319l27b5a227ua81dabb98d7a6289-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-05 21:21                       ` Maarten Maathuis
2010-01-05 22:55                       ` Maarten Maathuis
     [not found]                         ` <6d4bc9fc1001051455y301526cwaa935e8dd1956231-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-06 17:58                           ` Maarten Maathuis
     [not found]                             ` <6d4bc9fc1001060958q3b7d0d5dka7bcd3843584d6e2-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-06 22:17                               ` Ben Skeggs
2010-01-04 22:42       ` okias
     [not found]         ` <c2673ca61001041442r1cd832cdme5a202b40b173bf0-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-01-10 20:58           ` Marcin Slusarz
2010-01-04 22:58   ` Xavier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.