Include request for reset-rework branch v4

Include request for reset-rework branch v4

[Christian König]

Hi Dave,

there still seems to be the need for some further discussion about the SA code,
so I again split that out of the patchset and tested the result a bit.

Most of the stuff still works fine without those offending changes, so to avoid
mailing around unrelated and already reviewed patches, I request the include
the following 17 patches into drm-next.

If you prefer to merge they are also available from
git://people.freedesktop.org/~deathsimple/linux branch reset-rework.

Cheers,
Christian.

[PATCH 01/17] drm/radeon: make radeon_gpu_is_lockup a per ring function

[Christian König]

Different rings have different criteria to test
if they are stuck.

v2: rebased on current drm-next

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h       |    4 +-
 drivers/gpu/drm/radeon/radeon_asic.c  |   44 ++++++++++++++++++--------------
 drivers/gpu/drm/radeon/radeon_fence.c |    2 +-
 3 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 610acee..4026a4c0 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1162,7 +1162,6 @@ struct radeon_asic {
 	int (*resume)(struct radeon_device *rdev);
 	int (*suspend)(struct radeon_device *rdev);
 	void (*vga_set_state)(struct radeon_device *rdev, bool state);
-	bool (*gpu_is_lockup)(struct radeon_device *rdev, struct radeon_ring *cp);
 	int (*asic_reset)(struct radeon_device *rdev);
 	/* ioctl hw specific callback. Some hw might want to perform special
 	 * operation on specific ioctl. For instance on wait idle some hw
@@ -1191,6 +1190,7 @@ struct radeon_asic {
 		void (*ring_start)(struct radeon_device *rdev, struct radeon_ring *cp);
 		int (*ring_test)(struct radeon_device *rdev, struct radeon_ring *cp);
 		int (*ib_test)(struct radeon_device *rdev, struct radeon_ring *cp);
+		bool (*is_lockup)(struct radeon_device *rdev, struct radeon_ring *cp);
 	} ring[RADEON_NUM_RINGS];
 	/* irqs */
 	struct {
@@ -1740,7 +1740,6 @@ void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
 #define radeon_suspend(rdev) (rdev)->asic->suspend((rdev))
 #define radeon_cs_parse(rdev, r, p) (rdev)->asic->ring[(r)].cs_parse((p))
 #define radeon_vga_set_state(rdev, state) (rdev)->asic->vga_set_state((rdev), (state))
-#define radeon_gpu_is_lockup(rdev, cp) (rdev)->asic->gpu_is_lockup((rdev), (cp))
 #define radeon_asic_reset(rdev) (rdev)->asic->asic_reset((rdev))
 #define radeon_gart_tlb_flush(rdev) (rdev)->asic->gart.tlb_flush((rdev))
 #define radeon_gart_set_page(rdev, i, p) (rdev)->asic->gart.set_page((rdev), (i), (p))
@@ -1749,6 +1748,7 @@ void radeon_ring_write(struct radeon_ring *ring, uint32_t v);
 #define radeon_ib_test(rdev, r, cp) (rdev)->asic->ring[(r)].ib_test((rdev), (cp))
 #define radeon_ring_ib_execute(rdev, r, ib) (rdev)->asic->ring[(r)].ib_execute((rdev), (ib))
 #define radeon_ring_ib_parse(rdev, r, ib) (rdev)->asic->ring[(r)].ib_parse((rdev), (ib))
+#define radeon_ring_is_lockup(rdev, r, cp) (rdev)->asic->ring[(r)].is_lockup((rdev), (cp))
 #define radeon_irq_set(rdev) (rdev)->asic->irq.set((rdev))
 #define radeon_irq_process(rdev) (rdev)->asic->irq.process((rdev))
 #define radeon_get_vblank_counter(rdev, crtc) (rdev)->asic->display.get_vblank_counter((rdev), (crtc))
diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
index be4dc2f..958b9ea 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.c
+++ b/drivers/gpu/drm/radeon/radeon_asic.c
@@ -134,7 +134,6 @@ static struct radeon_asic r100_asic = {
 	.suspend = &r100_suspend,
 	.resume = &r100_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r100_gpu_is_lockup,
 	.asic_reset = &r100_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -152,6 +151,7 @@ static struct radeon_asic r100_asic = {
 			.ring_start = &r100_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -208,7 +208,6 @@ static struct radeon_asic r200_asic = {
 	.suspend = &r100_suspend,
 	.resume = &r100_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r100_gpu_is_lockup,
 	.asic_reset = &r100_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -226,6 +225,7 @@ static struct radeon_asic r200_asic = {
 			.ring_start = &r100_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -282,7 +282,6 @@ static struct radeon_asic r300_asic = {
 	.suspend = &r300_suspend,
 	.resume = &r300_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &r300_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -300,6 +299,7 @@ static struct radeon_asic r300_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -356,7 +356,6 @@ static struct radeon_asic r300_asic_pcie = {
 	.suspend = &r300_suspend,
 	.resume = &r300_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &r300_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -374,6 +373,7 @@ static struct radeon_asic r300_asic_pcie = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -430,7 +430,6 @@ static struct radeon_asic r420_asic = {
 	.suspend = &r420_suspend,
 	.resume = &r420_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &r300_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -448,6 +447,7 @@ static struct radeon_asic r420_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -504,7 +504,6 @@ static struct radeon_asic rs400_asic = {
 	.suspend = &rs400_suspend,
 	.resume = &rs400_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &r300_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -522,6 +521,7 @@ static struct radeon_asic rs400_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -578,7 +578,6 @@ static struct radeon_asic rs600_asic = {
 	.suspend = &rs600_suspend,
 	.resume = &rs600_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &rs600_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -596,6 +595,7 @@ static struct radeon_asic rs600_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -652,7 +652,6 @@ static struct radeon_asic rs690_asic = {
 	.suspend = &rs690_suspend,
 	.resume = &rs690_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &rs600_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -670,6 +669,7 @@ static struct radeon_asic rs690_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -726,7 +726,6 @@ static struct radeon_asic rv515_asic = {
 	.suspend = &rv515_suspend,
 	.resume = &rv515_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &rs600_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -744,6 +743,7 @@ static struct radeon_asic rv515_asic = {
 			.ring_start = &rv515_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -800,7 +800,6 @@ static struct radeon_asic r520_asic = {
 	.suspend = &rv515_suspend,
 	.resume = &r520_resume,
 	.vga_set_state = &r100_vga_set_state,
-	.gpu_is_lockup = &r300_gpu_is_lockup,
 	.asic_reset = &rs600_asic_reset,
 	.ioctl_wait_idle = NULL,
 	.gui_idle = &r100_gui_idle,
@@ -818,6 +817,7 @@ static struct radeon_asic r520_asic = {
 			.ring_start = &rv515_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
+			.is_lockup = &r300_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -874,7 +874,6 @@ static struct radeon_asic r600_asic = {
 	.suspend = &r600_suspend,
 	.resume = &r600_resume,
 	.vga_set_state = &r600_vga_set_state,
-	.gpu_is_lockup = &r600_gpu_is_lockup,
 	.asic_reset = &r600_asic_reset,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
 	.gui_idle = &r600_gui_idle,
@@ -891,6 +890,7 @@ static struct radeon_asic r600_asic = {
 			.cs_parse = &r600_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &r600_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -946,7 +946,6 @@ static struct radeon_asic rs780_asic = {
 	.fini = &r600_fini,
 	.suspend = &r600_suspend,
 	.resume = &r600_resume,
-	.gpu_is_lockup = &r600_gpu_is_lockup,
 	.vga_set_state = &r600_vga_set_state,
 	.asic_reset = &r600_asic_reset,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -964,6 +963,7 @@ static struct radeon_asic rs780_asic = {
 			.cs_parse = &r600_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &r600_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1020,7 +1020,6 @@ static struct radeon_asic rv770_asic = {
 	.suspend = &rv770_suspend,
 	.resume = &rv770_resume,
 	.asic_reset = &r600_asic_reset,
-	.gpu_is_lockup = &r600_gpu_is_lockup,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
 	.gui_idle = &r600_gui_idle,
@@ -1037,6 +1036,7 @@ static struct radeon_asic rv770_asic = {
 			.cs_parse = &r600_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &r600_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1092,7 +1092,6 @@ static struct radeon_asic evergreen_asic = {
 	.fini = &evergreen_fini,
 	.suspend = &evergreen_suspend,
 	.resume = &evergreen_resume,
-	.gpu_is_lockup = &evergreen_gpu_is_lockup,
 	.asic_reset = &evergreen_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1110,6 +1109,7 @@ static struct radeon_asic evergreen_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1165,7 +1165,6 @@ static struct radeon_asic sumo_asic = {
 	.fini = &evergreen_fini,
 	.suspend = &evergreen_suspend,
 	.resume = &evergreen_resume,
-	.gpu_is_lockup = &evergreen_gpu_is_lockup,
 	.asic_reset = &evergreen_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1183,6 +1182,7 @@ static struct radeon_asic sumo_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		},
 	},
 	.irq = {
@@ -1238,7 +1238,6 @@ static struct radeon_asic btc_asic = {
 	.fini = &evergreen_fini,
 	.suspend = &evergreen_suspend,
 	.resume = &evergreen_resume,
-	.gpu_is_lockup = &evergreen_gpu_is_lockup,
 	.asic_reset = &evergreen_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1256,6 +1255,7 @@ static struct radeon_asic btc_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1321,7 +1321,6 @@ static struct radeon_asic cayman_asic = {
 	.fini = &cayman_fini,
 	.suspend = &cayman_suspend,
 	.resume = &cayman_resume,
-	.gpu_is_lockup = &cayman_gpu_is_lockup,
 	.asic_reset = &cayman_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1340,6 +1339,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP1_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1349,6 +1349,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP2_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1358,6 +1359,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1413,7 +1415,6 @@ static struct radeon_asic trinity_asic = {
 	.fini = &cayman_fini,
 	.suspend = &cayman_suspend,
 	.resume = &cayman_resume,
-	.gpu_is_lockup = &cayman_gpu_is_lockup,
 	.asic_reset = &cayman_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1432,6 +1433,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP1_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1441,6 +1443,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP2_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1450,6 +1453,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &cayman_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1515,7 +1519,6 @@ static struct radeon_asic si_asic = {
 	.fini = &si_fini,
 	.suspend = &si_suspend,
 	.resume = &si_resume,
-	.gpu_is_lockup = &si_gpu_is_lockup,
 	.asic_reset = &si_asic_reset,
 	.vga_set_state = &r600_vga_set_state,
 	.ioctl_wait_idle = r600_ioctl_wait_idle,
@@ -1534,6 +1537,7 @@ static struct radeon_asic si_asic = {
 			.cs_parse = NULL,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &si_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP1_INDEX] = {
 			.ib_execute = &si_ring_ib_execute,
@@ -1543,6 +1547,7 @@ static struct radeon_asic si_asic = {
 			.cs_parse = NULL,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &si_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP2_INDEX] = {
 			.ib_execute = &si_ring_ib_execute,
@@ -1552,6 +1557,7 @@ static struct radeon_asic si_asic = {
 			.cs_parse = NULL,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
+			.is_lockup = &si_gpu_is_lockup,
 		}
 	},
 	.irq = {
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 4bd36a3..66b2229 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -259,7 +259,7 @@ retry:
 		 * if we experiencing a lockup the value doesn't change
 		 */
 		if (seq == rdev->fence_drv[fence->ring].last_seq &&
-		    radeon_gpu_is_lockup(rdev, &rdev->ring[fence->ring])) {
+		    radeon_ring_is_lockup(rdev, fence->ring, &rdev->ring[fence->ring])) {
 			/* good news we believe it's a lockup */
 			printk(KERN_WARNING "GPU lockup (waiting for 0x%08X last fence id 0x%08X)\n",
 			     fence->seq, seq);
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/17] drm/radeon: replace gpu_lockup with ring->ready flag

[Christian König]

It makes no sense at all to have more than one flag.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/r100.c          |    1 -
 drivers/gpu/drm/radeon/r300.c          |    1 -
 drivers/gpu/drm/radeon/radeon.h        |    1 -
 drivers/gpu/drm/radeon/radeon_device.c |    1 -
 drivers/gpu/drm/radeon/radeon_fence.c  |   36 +++++++++++--------------------
 drivers/gpu/drm/radeon/rs600.c         |    1 -
 6 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index cb11418..a0b44a5 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2300,7 +2300,6 @@ int r100_asic_reset(struct radeon_device *rdev)
 	if (G_000E40_SE_BUSY(status) || G_000E40_RE_BUSY(status) ||
 		G_000E40_TAM_BUSY(status) || G_000E40_PB_BUSY(status)) {
 		dev_err(rdev->dev, "failed to reset GPU\n");
-		rdev->gpu_lockup = true;
 		ret = -1;
 	} else
 		dev_info(rdev->dev, "GPU reset succeed\n");
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index fa14383..a63f432 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -449,7 +449,6 @@ int r300_asic_reset(struct radeon_device *rdev)
 	/* Check if GPU is idle */
 	if (G_000E40_GA_BUSY(status) || G_000E40_VAP_BUSY(status)) {
 		dev_err(rdev->dev, "failed to reset GPU\n");
-		rdev->gpu_lockup = true;
 		ret = -1;
 	} else
 		dev_info(rdev->dev, "GPU reset succeed\n");
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 4026a4c0..c76724b 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1547,7 +1547,6 @@ struct radeon_device {
 	struct radeon_mutex		cs_mutex;
 	struct radeon_wb		wb;
 	struct radeon_dummy_page	dummy_page;
-	bool				gpu_lockup;
 	bool				shutdown;
 	bool				suspend;
 	bool				need_dma32;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 0fb4f89..dedb398 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -714,7 +714,6 @@ int radeon_device_init(struct radeon_device *rdev,
 	rdev->is_atom_bios = false;
 	rdev->usec_timeout = RADEON_MAX_USEC_TIMEOUT;
 	rdev->mc.gtt_size = radeon_gart_size * 1024 * 1024;
-	rdev->gpu_lockup = false;
 	rdev->accel_working = false;
 
 	DRM_INFO("initializing kernel modesetting (%s 0x%04X:0x%04X 0x%04X:0x%04X).\n",
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 66b2229..36c411f 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -71,14 +71,7 @@ int radeon_fence_emit(struct radeon_device *rdev, struct radeon_fence *fence)
 		return 0;
 	}
 	fence->seq = atomic_add_return(1, &rdev->fence_drv[fence->ring].seq);
-	if (!rdev->ring[fence->ring].ready)
-		/* FIXME: cp is not running assume everythings is done right
-		 * away
-		 */
-		radeon_fence_write(rdev, fence->seq, fence->ring);
-	else
-		radeon_fence_ring_emit(rdev, fence->ring, fence);
-
+	radeon_fence_ring_emit(rdev, fence->ring, fence);
 	trace_radeon_fence_emit(rdev->ddev, fence->seq);
 	fence->emitted = true;
 	list_move_tail(&fence->list, &rdev->fence_drv[fence->ring].emitted);
@@ -191,9 +184,6 @@ bool radeon_fence_signaled(struct radeon_fence *fence)
 	if (!fence)
 		return true;
 
-	if (fence->rdev->gpu_lockup)
-		return true;
-
 	write_lock_irqsave(&fence->rdev->fence_lock, irq_flags);
 	signaled = fence->signaled;
 	/* if we are shuting down report all fence as signaled */
@@ -260,18 +250,16 @@ retry:
 		 */
 		if (seq == rdev->fence_drv[fence->ring].last_seq &&
 		    radeon_ring_is_lockup(rdev, fence->ring, &rdev->ring[fence->ring])) {
+
 			/* good news we believe it's a lockup */
 			printk(KERN_WARNING "GPU lockup (waiting for 0x%08X last fence id 0x%08X)\n",
 			     fence->seq, seq);
-			/* FIXME: what should we do ? marking everyone
-			 * as signaled for now
-			 */
-			rdev->gpu_lockup = true;
+
+			/* mark the ring as not ready any more */
+			rdev->ring[fence->ring].ready = false;
 			r = radeon_gpu_reset(rdev);
 			if (r)
 				return r;
-			radeon_fence_write(rdev, fence->seq, fence->ring);
-			rdev->gpu_lockup = false;
 		}
 		timeout = RADEON_FENCE_JIFFIES_TIMEOUT;
 		write_lock_irqsave(&rdev->fence_lock, irq_flags);
@@ -289,10 +277,11 @@ int radeon_fence_wait_next(struct radeon_device *rdev, int ring)
 	struct radeon_fence *fence;
 	int r;
 
-	if (rdev->gpu_lockup) {
-		return 0;
-	}
 	write_lock_irqsave(&rdev->fence_lock, irq_flags);
+	if (!rdev->ring[ring].ready) {
+		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+		return -EBUSY;
+	}
 	if (list_empty(&rdev->fence_drv[ring].emitted)) {
 		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
 		return 0;
@@ -312,10 +301,11 @@ int radeon_fence_wait_last(struct radeon_device *rdev, int ring)
 	struct radeon_fence *fence;
 	int r;
 
-	if (rdev->gpu_lockup) {
-		return 0;
-	}
 	write_lock_irqsave(&rdev->fence_lock, irq_flags);
+	if (!rdev->ring[ring].ready) {
+		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+		return -EBUSY;
+	}
 	if (list_empty(&rdev->fence_drv[ring].emitted)) {
 		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
 		return 0;
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 10706c6..7fb3b1f 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -396,7 +396,6 @@ int rs600_asic_reset(struct radeon_device *rdev)
 	/* Check if GPU is idle */
 	if (G_000E40_GA_BUSY(status) || G_000E40_VAP_BUSY(status)) {
 		dev_err(rdev->dev, "failed to reset GPU\n");
-		rdev->gpu_lockup = true;
 		ret = -1;
 	} else
 		dev_info(rdev->dev, "GPU reset succeed\n");
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/17] drm/radeon: register ring debugfs handlers on init

[Christian König]

Just register the debugfs files on init instead of
checking the chipset type multiple times.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ring.c |   31 +++++++++++++++++++------------
 1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index cc33b3d..b6eb1d2 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -34,7 +34,7 @@
 #include "atom.h"
 
 int radeon_debugfs_ib_init(struct radeon_device *rdev);
-int radeon_debugfs_ring_init(struct radeon_device *rdev);
+int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring);
 
 u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx)
 {
@@ -237,9 +237,6 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
 	if (radeon_debugfs_ib_init(rdev)) {
 		DRM_ERROR("Failed to register debugfs file for IB !\n");
 	}
-	if (radeon_debugfs_ring_init(rdev)) {
-		DRM_ERROR("Failed to register debugfs file for rings !\n");
-	}
 	radeon_mutex_unlock(&rdev->ib_pool.mutex);
 	return 0;
 }
@@ -411,6 +408,9 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsig
 	}
 	ring->ptr_mask = (ring->ring_size / 4) - 1;
 	ring->ring_free_dw = ring->ring_size / 4;
+	if (radeon_debugfs_ring_init(rdev, ring)) {
+		DRM_ERROR("Failed to register debugfs file for rings !\n");
+	}
 	return 0;
 }
 
@@ -501,17 +501,24 @@ static char radeon_debugfs_ib_names[RADEON_IB_POOL_SIZE][32];
 static unsigned radeon_debugfs_ib_idx[RADEON_IB_POOL_SIZE];
 #endif
 
-int radeon_debugfs_ring_init(struct radeon_device *rdev)
+int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring)
 {
 #if defined(CONFIG_DEBUG_FS)
-	if (rdev->family >= CHIP_CAYMAN)
-		return radeon_debugfs_add_files(rdev, radeon_debugfs_ring_info_list,
-						ARRAY_SIZE(radeon_debugfs_ring_info_list));
-	else
-		return radeon_debugfs_add_files(rdev, radeon_debugfs_ring_info_list, 1);
-#else
-	return 0;
+	unsigned i;
+	for (i = 0; i < ARRAY_SIZE(radeon_debugfs_ring_info_list); ++i) {
+		struct drm_info_list *info = &radeon_debugfs_ring_info_list[i];
+		int ridx = *(int*)radeon_debugfs_ring_info_list[i].data;
+		unsigned r;
+
+		if (&rdev->ring[ridx] != ring)
+			continue;
+
+		r = radeon_debugfs_add_files(rdev, info, 1);
+		if (r)
+			return r;
+	}
 #endif
+	return 0;
 }
 
 int radeon_debugfs_ib_init(struct radeon_device *rdev)
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/17] drm/radeon: use central function for IB testing

[Christian König]

Removing all the different error messages and
having just one standard behaviour over all
chipset generations.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c   |    7 ++-----
 drivers/gpu/drm/radeon/ni.c          |    7 ++-----
 drivers/gpu/drm/radeon/r100.c        |    7 ++-----
 drivers/gpu/drm/radeon/r300.c        |    7 ++-----
 drivers/gpu/drm/radeon/r420.c        |    7 ++-----
 drivers/gpu/drm/radeon/r520.c        |    8 +++-----
 drivers/gpu/drm/radeon/r600.c        |    7 ++-----
 drivers/gpu/drm/radeon/radeon.h      |    1 +
 drivers/gpu/drm/radeon/radeon_ring.c |   30 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/radeon/rs400.c       |    7 ++-----
 drivers/gpu/drm/radeon/rs600.c       |    7 ++-----
 drivers/gpu/drm/radeon/rs690.c       |    7 ++-----
 drivers/gpu/drm/radeon/rv515.c       |    8 +++-----
 drivers/gpu/drm/radeon/rv770.c       |    7 ++-----
 14 files changed, 57 insertions(+), 60 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index eed7ace..8b7a01b 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3376,12 +3376,9 @@ static int evergreen_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	r = r600_audio_init(rdev);
 	if (r) {
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index a48ca53..0146428 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1601,12 +1601,9 @@ static int cayman_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	r = radeon_vm_manager_start(rdev);
 	if (r)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index a0b44a5..825f117 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -3968,12 +3968,9 @@ static int r100_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index a63f432..26e0db8 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -1417,12 +1417,9 @@ static int r300_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/r420.c b/drivers/gpu/drm/radeon/r420.c
index f3fcaac..99137be 100644
--- a/drivers/gpu/drm/radeon/r420.c
+++ b/drivers/gpu/drm/radeon/r420.c
@@ -279,12 +279,9 @@ static int r420_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/r520.c b/drivers/gpu/drm/radeon/r520.c
index ebcc15b..b5cf837 100644
--- a/drivers/gpu/drm/radeon/r520.c
+++ b/drivers/gpu/drm/radeon/r520.c
@@ -207,12 +207,10 @@ static int r520_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 222245d..6070f90 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2494,12 +2494,9 @@ int r600_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		DRM_ERROR("radeon: failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index c76724b..65855af 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -802,6 +802,7 @@ int radeon_ib_pool_init(struct radeon_device *rdev);
 void radeon_ib_pool_fini(struct radeon_device *rdev);
 int radeon_ib_pool_start(struct radeon_device *rdev);
 int radeon_ib_pool_suspend(struct radeon_device *rdev);
+int radeon_ib_ring_tests(struct radeon_device *rdev);
 /* Ring access between begin & end cannot sleep */
 int radeon_ring_index(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_free_size(struct radeon_device *rdev, struct radeon_ring *cp);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index b6eb1d2..1b020ef 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -267,6 +267,36 @@ int radeon_ib_pool_suspend(struct radeon_device *rdev)
 	return radeon_sa_bo_manager_suspend(rdev, &rdev->ib_pool.sa_manager);
 }
 
+int radeon_ib_ring_tests(struct radeon_device *rdev)
+{
+	unsigned i;
+	int r;
+
+	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
+		struct radeon_ring *ring = &rdev->ring[i];
+
+		if (!ring->ready)
+			continue;
+
+		r = radeon_ib_test(rdev, i, ring);
+		if (r) {
+			ring->ready = false;
+
+			if (i == RADEON_RING_TYPE_GFX_INDEX) {
+				/* oh, oh, that's really bad */
+				DRM_ERROR("radeon: failed testing IB on GFX ring (%d).\n", r);
+		                rdev->accel_working = false;
+				return r;
+
+			} else {
+				/* still not good, but we can live with it */
+				DRM_ERROR("radeon: failed testing IB on ring %d (%d).\n", i, r);
+			}
+		}
+	}
+	return 0;
+}
+
 /*
  * Ring.
  */
diff --git a/drivers/gpu/drm/radeon/rs400.c b/drivers/gpu/drm/radeon/rs400.c
index 4cf381b..a464eb5 100644
--- a/drivers/gpu/drm/radeon/rs400.c
+++ b/drivers/gpu/drm/radeon/rs400.c
@@ -430,12 +430,9 @@ static int rs400_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/rs600.c b/drivers/gpu/drm/radeon/rs600.c
index 7fb3b1f..25f9eef 100644
--- a/drivers/gpu/drm/radeon/rs600.c
+++ b/drivers/gpu/drm/radeon/rs600.c
@@ -918,12 +918,9 @@ static int rs600_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/rs690.c b/drivers/gpu/drm/radeon/rs690.c
index f2c3b9d..3277dde 100644
--- a/drivers/gpu/drm/radeon/rs690.c
+++ b/drivers/gpu/drm/radeon/rs690.c
@@ -647,12 +647,9 @@ static int rs690_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/radeon/rv515.c b/drivers/gpu/drm/radeon/rv515.c
index d8d78fe..7f08ced 100644
--- a/drivers/gpu/drm/radeon/rv515.c
+++ b/drivers/gpu/drm/radeon/rv515.c
@@ -412,12 +412,10 @@ static int rv515_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "failed testing IB (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index c62ae4b..cacec0e 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -1114,12 +1114,9 @@ static int rv770_startup(struct radeon_device *rdev)
 	if (r)
 		return r;
 
-	r = radeon_ib_test(rdev, RADEON_RING_TYPE_GFX_INDEX, &rdev->ring[RADEON_RING_TYPE_GFX_INDEX]);
-	if (r) {
-		dev_err(rdev->dev, "IB test failed (%d).\n", r);
-		rdev->accel_working = false;
+	r = radeon_ib_ring_tests(rdev);
+	if (r)
 		return r;
-	}
 
 	return 0;
 }
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/17] drm/radeon: rework gpu lockup detection and processing

[Christian König]

Previusly multiple rings could trigger multiple GPU
resets at the same time.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h       |    3 +-
 drivers/gpu/drm/radeon/radeon_fence.c |  146 +++++++++++++++++----------------
 2 files changed, 75 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 65855af..35db5bd 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -255,8 +255,7 @@ struct radeon_fence_driver {
 	volatile uint32_t		*cpu_addr;
 	atomic_t			seq;
 	uint32_t			last_seq;
-	unsigned long			last_jiffies;
-	unsigned long			last_timeout;
+	unsigned long			last_activity;
 	wait_queue_head_t		queue;
 	struct list_head		created;
 	struct list_head		emitted;
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 36c411f..1a9765a 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -74,6 +74,10 @@ int radeon_fence_emit(struct radeon_device *rdev, struct radeon_fence *fence)
 	radeon_fence_ring_emit(rdev, fence->ring, fence);
 	trace_radeon_fence_emit(rdev->ddev, fence->seq);
 	fence->emitted = true;
+	/* are we the first fence on a previusly idle ring? */
+	if (list_empty(&rdev->fence_drv[fence->ring].emitted)) {
+		rdev->fence_drv[fence->ring].last_activity = jiffies;
+	}
 	list_move_tail(&fence->list, &rdev->fence_drv[fence->ring].emitted);
 	write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
 	return 0;
@@ -85,34 +89,14 @@ static bool radeon_fence_poll_locked(struct radeon_device *rdev, int ring)
 	struct list_head *i, *n;
 	uint32_t seq;
 	bool wake = false;
-	unsigned long cjiffies;
 
 	seq = radeon_fence_read(rdev, ring);
-	if (seq != rdev->fence_drv[ring].last_seq) {
-		rdev->fence_drv[ring].last_seq = seq;
-		rdev->fence_drv[ring].last_jiffies = jiffies;
-		rdev->fence_drv[ring].last_timeout = RADEON_FENCE_JIFFIES_TIMEOUT;
-	} else {
-		cjiffies = jiffies;
-		if (time_after(cjiffies, rdev->fence_drv[ring].last_jiffies)) {
-			cjiffies -= rdev->fence_drv[ring].last_jiffies;
-			if (time_after(rdev->fence_drv[ring].last_timeout, cjiffies)) {
-				/* update the timeout */
-				rdev->fence_drv[ring].last_timeout -= cjiffies;
-			} else {
-				/* the 500ms timeout is elapsed we should test
-				 * for GPU lockup
-				 */
-				rdev->fence_drv[ring].last_timeout = 1;
-			}
-		} else {
-			/* wrap around update last jiffies, we will just wait
-			 * a little longer
-			 */
-			rdev->fence_drv[ring].last_jiffies = cjiffies;
-		}
+	if (seq == rdev->fence_drv[ring].last_seq)
 		return false;
-	}
+
+	rdev->fence_drv[ring].last_seq = seq;
+	rdev->fence_drv[ring].last_activity = jiffies;
+
 	n = NULL;
 	list_for_each(i, &rdev->fence_drv[ring].emitted) {
 		fence = list_entry(i, struct radeon_fence, list);
@@ -207,66 +191,84 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr)
 	struct radeon_device *rdev;
 	unsigned long irq_flags, timeout;
 	u32 seq;
-	int r;
+	int i, r;
+	bool signaled;
 
 	if (fence == NULL) {
 		WARN(1, "Querying an invalid fence : %p !\n", fence);
-		return 0;
+		return -EINVAL;
 	}
+
 	rdev = fence->rdev;
-	if (radeon_fence_signaled(fence)) {
-		return 0;
-	}
-	timeout = rdev->fence_drv[fence->ring].last_timeout;
-retry:
-	/* save current sequence used to check for GPU lockup */
-	seq = rdev->fence_drv[fence->ring].last_seq;
-	trace_radeon_fence_wait_begin(rdev->ddev, seq);
-	if (intr) {
+	signaled = radeon_fence_signaled(fence);
+	while (!signaled) {
+		read_lock_irqsave(&rdev->fence_lock, irq_flags);
+		timeout = jiffies - RADEON_FENCE_JIFFIES_TIMEOUT;
+		if (time_after(rdev->fence_drv[fence->ring].last_activity, timeout)) {
+			/* the normal case, timeout is somewhere before last_activity */
+			timeout = rdev->fence_drv[fence->ring].last_activity - timeout;
+		} else {
+			/* either jiffies wrapped around, or no fence was signaled in the last 500ms
+			 * anyway we will just wait for the minimum amount and then check for a lockup */
+			timeout = 1;
+		}
+		/* save current sequence value used to check for GPU lockups */
+		seq = rdev->fence_drv[fence->ring].last_seq;
+		read_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+
+		trace_radeon_fence_wait_begin(rdev->ddev, seq);
 		radeon_irq_kms_sw_irq_get(rdev, fence->ring);
-		r = wait_event_interruptible_timeout(rdev->fence_drv[fence->ring].queue,
-				radeon_fence_signaled(fence), timeout);
+		if (intr) {
+			r = wait_event_interruptible_timeout(
+				rdev->fence_drv[fence->ring].queue,
+				(signaled = radeon_fence_signaled(fence)), timeout);
+		} else {
+			r = wait_event_timeout(
+				rdev->fence_drv[fence->ring].queue,
+				(signaled = radeon_fence_signaled(fence)), timeout);
+		}
 		radeon_irq_kms_sw_irq_put(rdev, fence->ring);
 		if (unlikely(r < 0)) {
 			return r;
 		}
-	} else {
-		radeon_irq_kms_sw_irq_get(rdev, fence->ring);
-		r = wait_event_timeout(rdev->fence_drv[fence->ring].queue,
-			 radeon_fence_signaled(fence), timeout);
-		radeon_irq_kms_sw_irq_put(rdev, fence->ring);
-	}
-	trace_radeon_fence_wait_end(rdev->ddev, seq);
-	if (unlikely(!radeon_fence_signaled(fence))) {
-		/* we were interrupted for some reason and fence isn't
-		 * isn't signaled yet, resume wait
-		 */
-		if (r) {
-			timeout = r;
-			goto retry;
-		}
-		/* don't protect read access to rdev->fence_drv[t].last_seq
-		 * if we experiencing a lockup the value doesn't change
-		 */
-		if (seq == rdev->fence_drv[fence->ring].last_seq &&
-		    radeon_ring_is_lockup(rdev, fence->ring, &rdev->ring[fence->ring])) {
-
-			/* good news we believe it's a lockup */
-			printk(KERN_WARNING "GPU lockup (waiting for 0x%08X last fence id 0x%08X)\n",
-			     fence->seq, seq);
-
-			/* mark the ring as not ready any more */
-			rdev->ring[fence->ring].ready = false;
-			r = radeon_gpu_reset(rdev);
-			if (r)
-				return r;
+		trace_radeon_fence_wait_end(rdev->ddev, seq);
+
+		if (unlikely(!signaled)) {
+			/* we were interrupted for some reason and fence
+			 * isn't signaled yet, resume waiting */
+			if (r) {
+				continue;
+			}
+
+			write_lock_irqsave(&rdev->fence_lock, irq_flags);
+			/* check if sequence value has changed since last_activity */
+			if (seq != rdev->fence_drv[fence->ring].last_seq) {
+				write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+				continue;
+			}
+
+			/* change sequence value on all rings, so nobody else things there is a lockup */
+			for (i = 0; i < RADEON_NUM_RINGS; ++i)
+				rdev->fence_drv[i].last_seq -= 0x10000;
+			write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+
+			if (radeon_ring_is_lockup(rdev, fence->ring, &rdev->ring[fence->ring])) {
+
+				/* good news we believe it's a lockup */
+				printk(KERN_WARNING "GPU lockup (waiting for 0x%08X last fence id 0x%08X)\n",
+				     fence->seq, seq);
+
+				/* mark the ring as not ready any more */
+				rdev->ring[fence->ring].ready = false;
+				r = radeon_gpu_reset(rdev);
+				if (r)
+					return r;
+
+				write_lock_irqsave(&rdev->fence_lock, irq_flags);
+				rdev->fence_drv[fence->ring].last_activity = jiffies;
+				write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+			}
 		}
-		timeout = RADEON_FENCE_JIFFIES_TIMEOUT;
-		write_lock_irqsave(&rdev->fence_lock, irq_flags);
-		rdev->fence_drv[fence->ring].last_timeout = RADEON_FENCE_JIFFIES_TIMEOUT;
-		rdev->fence_drv[fence->ring].last_jiffies = jiffies;
-		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
-		goto retry;
 	}
 	return 0;
 }
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/17] drm/radeon: fix a bug in the SA code

[Christian König]

Aligning offset can make it bigger than tmp->offset
leading to an overrun bug in the following subtraction.

v2: Against initial suspicions this can't happen in mainline,
    so no need to push it into stable.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_sa.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 4cce47e..8fbfe69 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -150,7 +150,7 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
 	offset = 0;
 	list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
 		/* room before this object ? */
-		if ((tmp->offset - offset) >= size) {
+		if (offset < tmp->offset && (tmp->offset - offset) >= size) {
 			head = tmp->list.prev;
 			goto out;
 		}
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/17] drm/radeon: return -ENOENT in fence_wait_next v2

[Christian König]

We should signal the caller that we haven't waited at all.

v2: only change fence_wait_next not fence_wait_last.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_fence.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 1a9765a..2fbbc34 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -286,7 +286,7 @@ int radeon_fence_wait_next(struct radeon_device *rdev, int ring)
 	}
 	if (list_empty(&rdev->fence_drv[ring].emitted)) {
 		write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
-		return 0;
+		return -ENOENT;
 	}
 	fence = list_entry(rdev->fence_drv[ring].emitted.next,
 			   struct radeon_fence, list);
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/17] drm/radeon: rename fence_wait_last to fence_wait_empty

[Christian König]

As discussed with Michel that name better
describes the behavior of this function.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h        |    2 +-
 drivers/gpu/drm/radeon/radeon_device.c |    2 +-
 drivers/gpu/drm/radeon/radeon_fence.c  |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 35db5bd..d73bba8 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -285,7 +285,7 @@ void radeon_fence_process(struct radeon_device *rdev, int ring);
 bool radeon_fence_signaled(struct radeon_fence *fence);
 int radeon_fence_wait(struct radeon_fence *fence, bool interruptible);
 int radeon_fence_wait_next(struct radeon_device *rdev, int ring);
-int radeon_fence_wait_last(struct radeon_device *rdev, int ring);
+int radeon_fence_wait_empty(struct radeon_device *rdev, int ring);
 struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence);
 void radeon_fence_unref(struct radeon_fence **fence);
 int radeon_fence_count_emitted(struct radeon_device *rdev, int ring);
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index dedb398..89be94b 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -915,7 +915,7 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state)
 	radeon_bo_evict_vram(rdev);
 	/* wait for gpu to finish processing current batch */
 	for (i = 0; i < RADEON_NUM_RINGS; i++)
-		radeon_fence_wait_last(rdev, i);
+		radeon_fence_wait_empty(rdev, i);
 
 	radeon_save_bios_scratch_regs(rdev);
 
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 2fbbc34..2d13843 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -297,7 +297,7 @@ int radeon_fence_wait_next(struct radeon_device *rdev, int ring)
 	return r;
 }
 
-int radeon_fence_wait_last(struct radeon_device *rdev, int ring)
+int radeon_fence_wait_empty(struct radeon_device *rdev, int ring)
 {
 	unsigned long irq_flags;
 	struct radeon_fence *fence;
@@ -442,7 +442,7 @@ void radeon_fence_driver_fini(struct radeon_device *rdev)
 	for (ring = 0; ring < RADEON_NUM_RINGS; ring++) {
 		if (!rdev->fence_drv[ring].initialized)
 			continue;
-		radeon_fence_wait_last(rdev, ring);
+		radeon_fence_wait_empty(rdev, ring);
 		wake_up_all(&rdev->fence_drv[ring].queue);
 		write_lock_irqsave(&rdev->fence_lock, irq_flags);
 		radeon_scratch_free(rdev, rdev->fence_drv[ring].scratch_reg);
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/17] drm/radeon: don't keep list of created fences.

[Christian König]

It's never used and so practically superfluous.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h       |    1 -
 drivers/gpu/drm/radeon/radeon_fence.c |    7 -------
 2 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index d73bba8..794182a 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -257,7 +257,6 @@ struct radeon_fence_driver {
 	uint32_t			last_seq;
 	unsigned long			last_activity;
 	wait_queue_head_t		queue;
-	struct list_head		created;
 	struct list_head		emitted;
 	struct list_head		signaled;
 	bool				initialized;
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 2d13843..aadd73a 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -139,8 +139,6 @@ int radeon_fence_create(struct radeon_device *rdev,
 			struct radeon_fence **fence,
 			int ring)
 {
-	unsigned long irq_flags;
-
 	*fence = kmalloc(sizeof(struct radeon_fence), GFP_KERNEL);
 	if ((*fence) == NULL) {
 		return -ENOMEM;
@@ -153,10 +151,6 @@ int radeon_fence_create(struct radeon_device *rdev,
 	(*fence)->ring = ring;
 	(*fence)->semaphore = NULL;
 	INIT_LIST_HEAD(&(*fence)->list);
-
-	write_lock_irqsave(&rdev->fence_lock, irq_flags);
-	list_add_tail(&(*fence)->list, &rdev->fence_drv[ring].created);
-	write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
 	return 0;
 }
 
@@ -411,7 +405,6 @@ static void radeon_fence_driver_init_ring(struct radeon_device *rdev, int ring)
 	rdev->fence_drv[ring].cpu_addr = NULL;
 	rdev->fence_drv[ring].gpu_addr = 0;
 	atomic_set(&rdev->fence_drv[ring].seq, 0);
-	INIT_LIST_HEAD(&rdev->fence_drv[ring].created);
 	INIT_LIST_HEAD(&rdev->fence_drv[ring].emitted);
 	INIT_LIST_HEAD(&rdev->fence_drv[ring].signaled);
 	init_waitqueue_head(&rdev->fence_drv[ring].queue);
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/17] drm/radeon: fix a bug with the ring syncing code

[Christian König]

Rings need to lock in order, otherwise
the ring subsystem can deadlock.

v2: fix error handling and number of locked doublewords.
v3: stop creating unneeded semaphores.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h           |    4 ++
 drivers/gpu/drm/radeon/radeon_cs.c        |   35 ++++++------------
 drivers/gpu/drm/radeon/radeon_semaphore.c |   56 +++++++++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_ttm.c       |   46 ++++++++++-------------
 4 files changed, 92 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 794182a..f7372c4 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -460,6 +460,10 @@ void radeon_semaphore_emit_signal(struct radeon_device *rdev, int ring,
 				  struct radeon_semaphore *semaphore);
 void radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring,
 				struct radeon_semaphore *semaphore);
+int radeon_semaphore_sync_rings(struct radeon_device *rdev,
+				struct radeon_semaphore *semaphore,
+				bool sync_to[RADEON_NUM_RINGS],
+				int dst_ring);
 void radeon_semaphore_free(struct radeon_device *rdev,
 			   struct radeon_semaphore *semaphore);
 
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index e7b0b5d..24fb001 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -118,6 +118,7 @@ static int radeon_cs_get_ring(struct radeon_cs_parser *p, u32 ring, s32 priority
 static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
 {
 	bool sync_to_ring[RADEON_NUM_RINGS] = { };
+	bool need_sync = false;
 	int i, r;
 
 	for (i = 0; i < p->nrelocs; i++) {
@@ -126,36 +127,24 @@ static int radeon_cs_sync_rings(struct radeon_cs_parser *p)
 
 		if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
 			struct radeon_fence *fence = p->relocs[i].robj->tbo.sync_obj;
-			if (!radeon_fence_signaled(fence)) {
+			if (fence->ring != p->ring && !radeon_fence_signaled(fence)) {
 				sync_to_ring[fence->ring] = true;
+				need_sync = true;
 			}
 		}
 	}
 
-	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
-		/* no need to sync to our own or unused rings */
-		if (i == p->ring || !sync_to_ring[i] || !p->rdev->ring[i].ready)
-			continue;
-
-		if (!p->ib->fence->semaphore) {
-			r = radeon_semaphore_create(p->rdev, &p->ib->fence->semaphore);
-			if (r)
-				return r;
-		}
-
-		r = radeon_ring_lock(p->rdev, &p->rdev->ring[i], 3);
-		if (r)
-			return r;
-		radeon_semaphore_emit_signal(p->rdev, i, p->ib->fence->semaphore);
-		radeon_ring_unlock_commit(p->rdev, &p->rdev->ring[i]);
+	if (!need_sync) {
+		return 0;
+	}
 
-		r = radeon_ring_lock(p->rdev, &p->rdev->ring[p->ring], 3);
-		if (r)
-			return r;
-		radeon_semaphore_emit_wait(p->rdev, p->ring, p->ib->fence->semaphore);
-		radeon_ring_unlock_commit(p->rdev, &p->rdev->ring[p->ring]);
+	r = radeon_semaphore_create(p->rdev, &p->ib->fence->semaphore);
+	if (r) {
+		return r;
 	}
-	return 0;
+
+	return radeon_semaphore_sync_rings(p->rdev, p->ib->fence->semaphore,
+					   sync_to_ring, p->ring);
 }
 
 int radeon_cs_parser_init(struct radeon_cs_parser *p, void *data)
diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c
index 61dd4e3..930a08a 100644
--- a/drivers/gpu/drm/radeon/radeon_semaphore.c
+++ b/drivers/gpu/drm/radeon/radeon_semaphore.c
@@ -149,6 +149,62 @@ void radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring,
 	radeon_semaphore_ring_emit(rdev, ring, &rdev->ring[ring], semaphore, true);
 }
 
+int radeon_semaphore_sync_rings(struct radeon_device *rdev,
+				struct radeon_semaphore *semaphore,
+				bool sync_to[RADEON_NUM_RINGS],
+				int dst_ring)
+{
+	int i, r;
+
+	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
+		unsigned num_ops = i == dst_ring ? RADEON_NUM_RINGS : 1;
+
+		/* don't lock unused rings */
+		if (!sync_to[i] && i != dst_ring)
+			continue;
+
+		/* prevent GPU deadlocks */
+		if (!rdev->ring[i].ready) {
+			dev_err(rdev->dev, "Trying to sync to a disabled ring!");
+			r = -EINVAL;
+			goto error;
+		}
+
+                r = radeon_ring_lock(rdev, &rdev->ring[i], num_ops * 8);
+                if (r)
+			goto error;
+	}
+
+	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
+		/* no need to sync to our own or unused rings */
+		if (!sync_to[i] || i == dst_ring)
+                        continue;
+
+		radeon_semaphore_emit_signal(rdev, i, semaphore);
+		radeon_semaphore_emit_wait(rdev, dst_ring, semaphore);
+	}
+
+	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
+
+		/* don't unlock unused rings */
+		if (!sync_to[i] && i != dst_ring)
+			continue;
+
+		radeon_ring_unlock_commit(rdev, &rdev->ring[i]);
+	}
+
+	return 0;
+
+error:
+	/* unlock all locks taken so far */
+	for (--i; i >= 0; --i) {
+		if (sync_to[i] || i == dst_ring) {
+			radeon_ring_unlock_undo(rdev, &rdev->ring[i]);
+		}
+	}
+	return r;
+}
+
 void radeon_semaphore_free(struct radeon_device *rdev,
 			   struct radeon_semaphore *semaphore)
 {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index f493c64..5e3d54d 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -222,8 +222,8 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
 {
 	struct radeon_device *rdev;
 	uint64_t old_start, new_start;
-	struct radeon_fence *fence;
-	int r, i;
+	struct radeon_fence *fence, *old_fence;
+	int r;
 
 	rdev = radeon_get_rdev(bo->bdev);
 	r = radeon_fence_create(rdev, &fence, radeon_copy_ring_index(rdev));
@@ -242,6 +242,7 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
 		break;
 	default:
 		DRM_ERROR("Unknown placement %d\n", old_mem->mem_type);
+		radeon_fence_unref(&fence);
 		return -EINVAL;
 	}
 	switch (new_mem->mem_type) {
@@ -253,42 +254,35 @@ static int radeon_move_blit(struct ttm_buffer_object *bo,
 		break;
 	default:
 		DRM_ERROR("Unknown placement %d\n", old_mem->mem_type);
+		radeon_fence_unref(&fence);
 		return -EINVAL;
 	}
 	if (!rdev->ring[radeon_copy_ring_index(rdev)].ready) {
 		DRM_ERROR("Trying to move memory with ring turned off.\n");
+		radeon_fence_unref(&fence);
 		return -EINVAL;
 	}
 
 	BUILD_BUG_ON((PAGE_SIZE % RADEON_GPU_PAGE_SIZE) != 0);
 
 	/* sync other rings */
-	if (rdev->family >= CHIP_R600) {
-		for (i = 0; i < RADEON_NUM_RINGS; ++i) {
-			/* no need to sync to our own or unused rings */
-			if (i == radeon_copy_ring_index(rdev) || !rdev->ring[i].ready)
-				continue;
-
-			if (!fence->semaphore) {
-				r = radeon_semaphore_create(rdev, &fence->semaphore);
-				/* FIXME: handle semaphore error */
-				if (r)
-					continue;
-			}
+	old_fence = bo->sync_obj;
+	if (old_fence && old_fence->ring != fence->ring
+	    && !radeon_fence_signaled(old_fence)) {
+		bool sync_to_ring[RADEON_NUM_RINGS] = { };
+		sync_to_ring[old_fence->ring] = true;
+
+		r = radeon_semaphore_create(rdev, &fence->semaphore);
+		if (r) {
+			radeon_fence_unref(&fence);
+			return r;
+		}
 
-			r = radeon_ring_lock(rdev, &rdev->ring[i], 3);
-			/* FIXME: handle ring lock error */
-			if (r)
-				continue;
-			radeon_semaphore_emit_signal(rdev, i, fence->semaphore);
-			radeon_ring_unlock_commit(rdev, &rdev->ring[i]);
-
-			r = radeon_ring_lock(rdev, &rdev->ring[radeon_copy_ring_index(rdev)], 3);
-			/* FIXME: handle ring lock error */
-			if (r)
-				continue;
-			radeon_semaphore_emit_wait(rdev, radeon_copy_ring_index(rdev), fence->semaphore);
-			radeon_ring_unlock_commit(rdev, &rdev->ring[radeon_copy_ring_index(rdev)]);
+		r = radeon_semaphore_sync_rings(rdev, fence->semaphore,
+						sync_to_ring, fence->ring);
+		if (r) {
+			radeon_fence_unref(&fence);
+			return r;
 		}
 	}
 
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/17] drm/radeon: rework recursive gpu reset handling

[Christian König]

Instead of all this humpy pumpy with recursive
mutex (which also fixes only halve of the problem)
move the actual gpu reset out of the fence code,
return -EDEADLK and then reset the gpu in the
calling ioctl function.

v2: Split removal of radeon_mutex into separate patch.
    Return -EAGAIN if reset is successful.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_cs.c     |   13 +++++++++++++
 drivers/gpu/drm/radeon/radeon_device.c |    5 -----
 drivers/gpu/drm/radeon/radeon_fence.c  |   10 +++-------
 drivers/gpu/drm/radeon/radeon_gem.c    |   16 ++++++++++++++++
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 24fb001..02eee4b 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -496,6 +496,16 @@ out:
 	return r;
 }
 
+static int radeon_cs_handle_lockup(struct radeon_device *rdev, int r)
+{
+	if (r == -EDEADLK) {
+		r = radeon_gpu_reset(rdev);
+		if (!r)
+			r = -EAGAIN;
+	}
+	return r;
+}
+
 int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 {
 	struct radeon_device *rdev = dev->dev_private;
@@ -517,6 +527,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	if (r) {
 		DRM_ERROR("Failed to initialize parser !\n");
 		radeon_cs_parser_fini(&parser, r);
+		r = radeon_cs_handle_lockup(rdev, r);
 		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
@@ -525,6 +536,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("Failed to parse relocation %d!\n", r);
 		radeon_cs_parser_fini(&parser, r);
+		r = radeon_cs_handle_lockup(rdev, r);
 		radeon_mutex_unlock(&rdev->cs_mutex);
 		return r;
 	}
@@ -538,6 +550,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 	}
 out:
 	radeon_cs_parser_fini(&parser, r);
+	r = radeon_cs_handle_lockup(rdev, r);
 	radeon_mutex_unlock(&rdev->cs_mutex);
 	return r;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 89be94b..d18f0c4 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -986,9 +986,6 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 	int r;
 	int resched;
 
-	/* Prevent CS ioctl from interfering */
-	radeon_mutex_lock(&rdev->cs_mutex);
-
 	radeon_save_bios_scratch_regs(rdev);
 	/* block TTM */
 	resched = ttm_bo_lock_delayed_workqueue(&rdev->mman.bdev);
@@ -1003,8 +1000,6 @@ int radeon_gpu_reset(struct radeon_device *rdev)
 		ttm_bo_unlock_delayed_workqueue(&rdev->mman.bdev, resched);
 	}
 
-	radeon_mutex_unlock(&rdev->cs_mutex);
-
 	if (r) {
 		/* bad news, how to tell it to userspace ? */
 		dev_info(rdev->dev, "GPU reset failed\n");
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index aadd73a..5bb78bf 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -244,6 +244,8 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr)
 			/* change sequence value on all rings, so nobody else things there is a lockup */
 			for (i = 0; i < RADEON_NUM_RINGS; ++i)
 				rdev->fence_drv[i].last_seq -= 0x10000;
+
+			rdev->fence_drv[fence->ring].last_activity = jiffies;
 			write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
 
 			if (radeon_ring_is_lockup(rdev, fence->ring, &rdev->ring[fence->ring])) {
@@ -254,13 +256,7 @@ int radeon_fence_wait(struct radeon_fence *fence, bool intr)
 
 				/* mark the ring as not ready any more */
 				rdev->ring[fence->ring].ready = false;
-				r = radeon_gpu_reset(rdev);
-				if (r)
-					return r;
-
-				write_lock_irqsave(&rdev->fence_lock, irq_flags);
-				rdev->fence_drv[fence->ring].last_activity = jiffies;
-				write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+				return -EDEADLK;
 			}
 		}
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index c7008b5..e15cb1f 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -154,6 +154,17 @@ void radeon_gem_object_close(struct drm_gem_object *obj,
 	radeon_bo_unreserve(rbo);
 }
 
+static int radeon_gem_handle_lockup(struct radeon_device *rdev, int r)
+{
+	if (r == -EDEADLK) {
+		radeon_mutex_lock(&rdev->cs_mutex);
+		r = radeon_gpu_reset(rdev);
+		if (!r)
+			r = -EAGAIN;
+		radeon_mutex_unlock(&rdev->cs_mutex);
+	}
+	return r;
+}
 
 /*
  * GEM ioctls.
@@ -210,12 +221,14 @@ int radeon_gem_create_ioctl(struct drm_device *dev, void *data,
 					args->initial_domain, false,
 					false, &gobj);
 	if (r) {
+		r = radeon_gem_handle_lockup(rdev, r);
 		return r;
 	}
 	r = drm_gem_handle_create(filp, gobj, &handle);
 	/* drop reference from allocate - handle holds it now */
 	drm_gem_object_unreference_unlocked(gobj);
 	if (r) {
+		r = radeon_gem_handle_lockup(rdev, r);
 		return r;
 	}
 	args->handle = handle;
@@ -245,6 +258,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	r = radeon_gem_set_domain(gobj, args->read_domains, args->write_domain);
 
 	drm_gem_object_unreference_unlocked(gobj);
+	r = radeon_gem_handle_lockup(robj->rdev, r);
 	return r;
 }
 
@@ -301,6 +315,7 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
 		break;
 	}
 	drm_gem_object_unreference_unlocked(gobj);
+	r = radeon_gem_handle_lockup(robj->rdev, r);
 	return r;
 }
 
@@ -322,6 +337,7 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 	if (robj->rdev->asic->ioctl_wait_idle)
 		robj->rdev->asic->ioctl_wait_idle(robj->rdev, robj);
 	drm_gem_object_unreference_unlocked(gobj);
+	r = radeon_gem_handle_lockup(robj->rdev, r);
 	return r;
 }
 
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/17] drm/radeon: move lockup detection code into radeon_ring.c

[Christian König]

It isn't chipset specific, so it makes no sense
to have that inside r100.c.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c   |    5 +--
 drivers/gpu/drm/radeon/ni.c          |    5 +--
 drivers/gpu/drm/radeon/r100.c        |   57 +--------------------------------
 drivers/gpu/drm/radeon/r300.c        |    4 +-
 drivers/gpu/drm/radeon/r600.c        |   10 +-----
 drivers/gpu/drm/radeon/radeon.h      |   16 ++-------
 drivers/gpu/drm/radeon/radeon_asic.h |    5 ---
 drivers/gpu/drm/radeon/radeon_ring.c |   53 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/radeon/si.c          |    5 +--
 9 files changed, 69 insertions(+), 91 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 8b7a01b..66f05bb 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -2424,7 +2424,6 @@ bool evergreen_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status_se0, grbm_status_se1;
-	struct r100_gpu_lockup *lockup = &rdev->config.evergreen.lockup;
 	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
@@ -2432,7 +2431,7 @@ bool evergreen_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 	grbm_status_se0 = RREG32(GRBM_STATUS_SE0);
 	grbm_status_se1 = RREG32(GRBM_STATUS_SE1);
 	if (!(grbm_status & GUI_ACTIVE)) {
-		r100_gpu_lockup_update(lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -2444,7 +2443,7 @@ bool evergreen_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 		radeon_ring_unlock_commit(rdev, ring);
 	}
 	ring->rptr = RREG32(CP_RB_RPTR);
-	return r100_gpu_cp_is_lockup(rdev, lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 static int evergreen_gpu_soft_reset(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 0146428..8404b2a 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1397,7 +1397,6 @@ bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status_se0, grbm_status_se1;
-	struct r100_gpu_lockup *lockup = &rdev->config.cayman.lockup;
 	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
@@ -1405,7 +1404,7 @@ bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	grbm_status_se0 = RREG32(GRBM_STATUS_SE0);
 	grbm_status_se1 = RREG32(GRBM_STATUS_SE1);
 	if (!(grbm_status & GUI_ACTIVE)) {
-		r100_gpu_lockup_update(lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -1418,7 +1417,7 @@ bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	}
 	/* XXX deal with CP0,1,2 */
 	ring->rptr = RREG32(ring->rptr_reg);
-	return r100_gpu_cp_is_lockup(rdev, lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 static int cayman_gpu_soft_reset(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index 825f117..ccf5e3b 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2159,59 +2159,6 @@ int r100_mc_wait_for_idle(struct radeon_device *rdev)
 	return -1;
 }
 
-void r100_gpu_lockup_update(struct r100_gpu_lockup *lockup, struct radeon_ring *ring)
-{
-	lockup->last_cp_rptr = ring->rptr;
-	lockup->last_jiffies = jiffies;
-}
-
-/**
- * r100_gpu_cp_is_lockup() - check if CP is lockup by recording information
- * @rdev:	radeon device structure
- * @lockup:	r100_gpu_lockup structure holding CP lockup tracking informations
- * @cp:		radeon_cp structure holding CP information
- *
- * We don't need to initialize the lockup tracking information as we will either
- * have CP rptr to a different value of jiffies wrap around which will force
- * initialization of the lockup tracking informations.
- *
- * A possible false positivie is if we get call after while and last_cp_rptr ==
- * the current CP rptr, even if it's unlikely it might happen. To avoid this
- * if the elapsed time since last call is bigger than 2 second than we return
- * false and update the tracking information. Due to this the caller must call
- * r100_gpu_cp_is_lockup several time in less than 2sec for lockup to be reported
- * the fencing code should be cautious about that.
- *
- * Caller should write to the ring to force CP to do something so we don't get
- * false positive when CP is just gived nothing to do.
- *
- **/
-bool r100_gpu_cp_is_lockup(struct radeon_device *rdev, struct r100_gpu_lockup *lockup, struct radeon_ring *ring)
-{
-	unsigned long cjiffies, elapsed;
-
-	cjiffies = jiffies;
-	if (!time_after(cjiffies, lockup->last_jiffies)) {
-		/* likely a wrap around */
-		lockup->last_cp_rptr = ring->rptr;
-		lockup->last_jiffies = jiffies;
-		return false;
-	}
-	if (ring->rptr != lockup->last_cp_rptr) {
-		/* CP is still working no lockup */
-		lockup->last_cp_rptr = ring->rptr;
-		lockup->last_jiffies = jiffies;
-		return false;
-	}
-	elapsed = jiffies_to_msecs(cjiffies - lockup->last_jiffies);
-	if (elapsed >= 10000) {
-		dev_err(rdev->dev, "GPU lockup CP stall for more than %lumsec\n", elapsed);
-		return true;
-	}
-	/* give a chance to the GPU ... */
-	return false;
-}
-
 bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 {
 	u32 rbbm_status;
@@ -2219,7 +2166,7 @@ bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 
 	rbbm_status = RREG32(R_000E40_RBBM_STATUS);
 	if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
-		r100_gpu_lockup_update(&rdev->config.r100.lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -2231,7 +2178,7 @@ bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		radeon_ring_unlock_commit(rdev, ring);
 	}
 	ring->rptr = RREG32(ring->rptr_reg);
-	return r100_gpu_cp_is_lockup(rdev, &rdev->config.r100.lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 void r100_bm_disable(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 26e0db8..e207664 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -384,7 +384,7 @@ bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 
 	rbbm_status = RREG32(R_000E40_RBBM_STATUS);
 	if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
-		r100_gpu_lockup_update(&rdev->config.r300.lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -396,7 +396,7 @@ bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		radeon_ring_unlock_commit(rdev, ring);
 	}
 	ring->rptr = RREG32(RADEON_CP_RB_RPTR);
-	return r100_gpu_cp_is_lockup(rdev, &rdev->config.r300.lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 int r300_asic_reset(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 6070f90..45d52cc 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -1350,19 +1350,13 @@ bool r600_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status2;
-	struct r100_gpu_lockup *lockup;
 	int r;
 
-	if (rdev->family >= CHIP_RV770)
-		lockup = &rdev->config.rv770.lockup;
-	else
-		lockup = &rdev->config.r600.lockup;
-
 	srbm_status = RREG32(R_000E50_SRBM_STATUS);
 	grbm_status = RREG32(R_008010_GRBM_STATUS);
 	grbm_status2 = RREG32(R_008014_GRBM_STATUS2);
 	if (!G_008010_GUI_ACTIVE(grbm_status)) {
-		r100_gpu_lockup_update(lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -1374,7 +1368,7 @@ bool r600_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		radeon_ring_unlock_commit(rdev, ring);
 	}
 	ring->rptr = RREG32(ring->rptr_reg);
-	return r100_gpu_cp_is_lockup(rdev, lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 int r600_asic_reset(struct radeon_device *rdev)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index f7372c4..b8d4d94 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -670,6 +670,8 @@ struct radeon_ring {
 	unsigned		ring_size;
 	unsigned		ring_free_dw;
 	int			count_dw;
+	unsigned long		last_activity;
+	unsigned		last_rptr;
 	uint64_t		gpu_addr;
 	uint32_t		align_mask;
 	uint32_t		ptr_mask;
@@ -814,6 +816,8 @@ void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_unlock_commit(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *cp);
 int radeon_ring_test(struct radeon_device *rdev, struct radeon_ring *cp);
+void radeon_ring_lockup_update(struct radeon_ring *ring);
+bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring);
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *cp, unsigned ring_size,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
 		     u32 ptr_reg_shift, u32 ptr_reg_mask, u32 nop);
@@ -1272,16 +1276,10 @@ struct radeon_asic {
 /*
  * Asic structures
  */
-struct r100_gpu_lockup {
-	unsigned long	last_jiffies;
-	u32		last_cp_rptr;
-};
-
 struct r100_asic {
 	const unsigned		*reg_safe_bm;
 	unsigned		reg_safe_bm_size;
 	u32			hdp_cntl;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct r300_asic {
@@ -1289,7 +1287,6 @@ struct r300_asic {
 	unsigned		reg_safe_bm_size;
 	u32			resync_scratch;
 	u32			hdp_cntl;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct r600_asic {
@@ -1311,7 +1308,6 @@ struct r600_asic {
 	unsigned		tiling_group_size;
 	unsigned		tile_config;
 	unsigned		backend_map;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct rv770_asic {
@@ -1337,7 +1333,6 @@ struct rv770_asic {
 	unsigned		tiling_group_size;
 	unsigned		tile_config;
 	unsigned		backend_map;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct evergreen_asic {
@@ -1364,7 +1359,6 @@ struct evergreen_asic {
 	unsigned tiling_group_size;
 	unsigned tile_config;
 	unsigned backend_map;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct cayman_asic {
@@ -1403,7 +1397,6 @@ struct cayman_asic {
 	unsigned multi_gpu_tile_size;
 
 	unsigned tile_config;
-	struct r100_gpu_lockup	lockup;
 };
 
 struct si_asic {
@@ -1434,7 +1427,6 @@ struct si_asic {
 	unsigned multi_gpu_tile_size;
 
 	unsigned tile_config;
-	struct r100_gpu_lockup	lockup;
 };
 
 union radeon_asic_config {
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index b135bec..b0941f9 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -103,11 +103,6 @@ int r100_pci_gart_enable(struct radeon_device *rdev);
 void r100_pci_gart_disable(struct radeon_device *rdev);
 int r100_debugfs_mc_info_init(struct radeon_device *rdev);
 int r100_gui_wait_for_idle(struct radeon_device *rdev);
-void r100_gpu_lockup_update(struct r100_gpu_lockup *lockup,
-			    struct radeon_ring *cp);
-bool r100_gpu_cp_is_lockup(struct radeon_device *rdev,
-			   struct r100_gpu_lockup *lockup,
-			   struct radeon_ring *cp);
 void r100_ib_fini(struct radeon_device *rdev);
 int r100_ib_test(struct radeon_device *rdev, struct radeon_ring *ring);
 void r100_irq_disable(struct radeon_device *rdev);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 1b020ef..203ec1f 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -396,6 +396,59 @@ void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *rin
 	mutex_unlock(&ring->mutex);
 }
 
+void radeon_ring_lockup_update(struct radeon_ring *ring)
+{
+	ring->last_rptr = ring->rptr;
+	ring->last_activity = jiffies;
+}
+
+/**
+ * radeon_ring_test_lockup() - check if ring is lockedup by recording information
+ * @rdev:       radeon device structure
+ * @ring:       radeon_ring structure holding ring information
+ *
+ * We don't need to initialize the lockup tracking information as we will either
+ * have CP rptr to a different value of jiffies wrap around which will force
+ * initialization of the lockup tracking informations.
+ *
+ * A possible false positivie is if we get call after while and last_cp_rptr ==
+ * the current CP rptr, even if it's unlikely it might happen. To avoid this
+ * if the elapsed time since last call is bigger than 2 second than we return
+ * false and update the tracking information. Due to this the caller must call
+ * radeon_ring_test_lockup several time in less than 2sec for lockup to be reported
+ * the fencing code should be cautious about that.
+ *
+ * Caller should write to the ring to force CP to do something so we don't get
+ * false positive when CP is just gived nothing to do.
+ *
+ **/
+bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
+{
+	unsigned long cjiffies, elapsed;
+	uint32_t rptr;
+
+	cjiffies = jiffies;
+	if (!time_after(cjiffies, ring->last_activity)) {
+		/* likely a wrap around */
+		radeon_ring_lockup_update(ring);
+		return false;
+	}
+	rptr = RREG32(ring->rptr_reg);
+	ring->rptr = (rptr & ring->ptr_reg_mask) >> ring->ptr_reg_shift;
+	if (ring->rptr != ring->last_rptr) {
+		/* CP is still working no lockup */
+		radeon_ring_lockup_update(ring);
+		return false;
+	}
+	elapsed = jiffies_to_msecs(cjiffies - ring->last_activity);
+	if (elapsed >= 10000) {
+		dev_err(rdev->dev, "GPU lockup CP stall for more than %lumsec\n", elapsed);
+		return true;
+	}
+	/* give a chance to the GPU ... */
+	return false;
+}
+
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsigned ring_size,
 		     unsigned rptr_offs, unsigned rptr_reg, unsigned wptr_reg,
 		     u32 ptr_reg_shift, u32 ptr_reg_mask, u32 nop)
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 14919e1..c2422f5 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -2217,7 +2217,6 @@ bool si_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status, grbm_status2;
 	u32 grbm_status_se0, grbm_status_se1;
-	struct r100_gpu_lockup *lockup = &rdev->config.si.lockup;
 	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
@@ -2226,7 +2225,7 @@ bool si_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	grbm_status_se0 = RREG32(GRBM_STATUS_SE0);
 	grbm_status_se1 = RREG32(GRBM_STATUS_SE1);
 	if (!(grbm_status & GUI_ACTIVE)) {
-		r100_gpu_lockup_update(lockup, ring);
+		radeon_ring_lockup_update(ring);
 		return false;
 	}
 	/* force CP activities */
@@ -2239,7 +2238,7 @@ bool si_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	}
 	/* XXX deal with CP0,1,2 */
 	ring->rptr = RREG32(ring->rptr_reg);
-	return r100_gpu_cp_is_lockup(rdev, lockup, ring);
+	return radeon_ring_test_lockup(rdev, ring);
 }
 
 static int si_gpu_soft_reset(struct radeon_device *rdev)
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/17] drm/radeon: make lockup timeout a module param

[Christian König]

Don't hard code the 10 seconds timeout. Compute jobs
can run much longer.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon.h      |    1 +
 drivers/gpu/drm/radeon/radeon_drv.c  |    4 ++++
 drivers/gpu/drm/radeon/radeon_ring.c |    2 +-
 3 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index b8d4d94..0784e4d 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -94,6 +94,7 @@ extern int radeon_disp_priority;
 extern int radeon_hw_i2c;
 extern int radeon_pcie_gen2;
 extern int radeon_msi;
+extern int radeon_lockup_timeout;
 
 /*
  * Copy from radeon_drv.h so we don't have to include both and have conflicting
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index ef7bb3f..e62e56a 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -128,6 +128,7 @@ int radeon_disp_priority = 0;
 int radeon_hw_i2c = 0;
 int radeon_pcie_gen2 = 0;
 int radeon_msi = -1;
+int radeon_lockup_timeout = 10000;
 
 MODULE_PARM_DESC(no_wb, "Disable AGP writeback for scratch registers");
 module_param_named(no_wb, radeon_no_wb, int, 0444);
@@ -177,6 +178,9 @@ module_param_named(pcie_gen2, radeon_pcie_gen2, int, 0444);
 MODULE_PARM_DESC(msi, "MSI support (1 = enable, 0 = disable, -1 = auto)");
 module_param_named(msi, radeon_msi, int, 0444);
 
+MODULE_PARM_DESC(lockup_timeout, "GPU lockup timeout in ms (defaul 10000 = 10 seconds, 0 = disable)");
+module_param_named(lockup_timeout, radeon_lockup_timeout, int, 0444);
+
 static int radeon_suspend(struct drm_device *dev, pm_message_t state)
 {
 	drm_radeon_private_t *dev_priv = dev->dev_private;
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 203ec1f..08e1578 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -441,7 +441,7 @@ bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 		return false;
 	}
 	elapsed = jiffies_to_msecs(cjiffies - ring->last_activity);
-	if (elapsed >= 10000) {
+	if (radeon_lockup_timeout && elapsed >= radeon_lockup_timeout) {
 		dev_err(rdev->dev, "GPU lockup CP stall for more than %lumsec\n", elapsed);
 		return true;
 	}
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/17] drm/radeon: unlock the ring mutex while waiting for the next fence

[Christian König]

Fixing just another deadlock problem with gpu reset tests.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/radeon_ring.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 08e1578..407d90a 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -346,7 +346,9 @@ int radeon_ring_alloc(struct radeon_device *rdev, struct radeon_ring *ring, unsi
 		if (ndw < ring->ring_free_dw) {
 			break;
 		}
+		mutex_unlock(&ring->mutex);
 		r = radeon_fence_wait_next(rdev, radeon_ring_index(rdev, ring));
+		mutex_lock(&ring->mutex);
 		if (r)
 			return r;
 	}
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/17] drm/radeon: make forcing ring activity a common function

[Christian König]

Nothing chipset or ring specific with it,
so also move it to radon_ring.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/evergreen.c   |   10 +---------
 drivers/gpu/drm/radeon/ni.c          |   11 +----------
 drivers/gpu/drm/radeon/r100.c        |   10 +---------
 drivers/gpu/drm/radeon/r300.c        |   10 +---------
 drivers/gpu/drm/radeon/r600.c        |   10 +---------
 drivers/gpu/drm/radeon/radeon.h      |    1 +
 drivers/gpu/drm/radeon/radeon_ring.c |   16 ++++++++++++++++
 drivers/gpu/drm/radeon/si.c          |   11 +----------
 8 files changed, 23 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 66f05bb..ecc29bc 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -2424,7 +2424,6 @@ bool evergreen_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status_se0, grbm_status_se1;
-	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
 	grbm_status = RREG32(GRBM_STATUS);
@@ -2435,14 +2434,7 @@ bool evergreen_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *rin
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	ring->rptr = RREG32(CP_RB_RPTR);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 8404b2a..1184635 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1397,7 +1397,6 @@ bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status_se0, grbm_status_se1;
-	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
 	grbm_status = RREG32(GRBM_STATUS);
@@ -1408,15 +1407,7 @@ bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	/* XXX deal with CP0,1,2 */
-	ring->rptr = RREG32(ring->rptr_reg);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index ccf5e3b..42d60ab 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -2162,7 +2162,6 @@ int r100_mc_wait_for_idle(struct radeon_device *rdev)
 bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 {
 	u32 rbbm_status;
-	int r;
 
 	rbbm_status = RREG32(R_000E40_RBBM_STATUS);
 	if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
@@ -2170,14 +2169,7 @@ bool r100_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	ring->rptr = RREG32(ring->rptr_reg);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index e207664..04ec269 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -380,7 +380,6 @@ void r300_gpu_init(struct radeon_device *rdev)
 bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 {
 	u32 rbbm_status;
-	int r;
 
 	rbbm_status = RREG32(R_000E40_RBBM_STATUS);
 	if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
@@ -388,14 +387,7 @@ bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	ring->rptr = RREG32(RADEON_CP_RB_RPTR);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 45d52cc..87a2333 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -1350,7 +1350,6 @@ bool r600_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status;
 	u32 grbm_status2;
-	int r;
 
 	srbm_status = RREG32(R_000E50_SRBM_STATUS);
 	grbm_status = RREG32(R_008010_GRBM_STATUS);
@@ -1360,14 +1359,7 @@ bool r600_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	ring->rptr = RREG32(ring->rptr_reg);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 0784e4d..82ffa6a 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -817,6 +817,7 @@ void radeon_ring_commit(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_unlock_commit(struct radeon_device *rdev, struct radeon_ring *cp);
 void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *cp);
 int radeon_ring_test(struct radeon_device *rdev, struct radeon_ring *cp);
+void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *ring);
 void radeon_ring_lockup_update(struct radeon_ring *ring);
 bool radeon_ring_test_lockup(struct radeon_device *rdev, struct radeon_ring *ring);
 int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *cp, unsigned ring_size,
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 407d90a..2eb4c6e 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -398,6 +398,22 @@ void radeon_ring_unlock_undo(struct radeon_device *rdev, struct radeon_ring *rin
 	mutex_unlock(&ring->mutex);
 }
 
+void radeon_ring_force_activity(struct radeon_device *rdev, struct radeon_ring *ring)
+{
+	int r;
+
+	mutex_lock(&ring->mutex);
+	radeon_ring_free_size(rdev, ring);
+	if (ring->rptr == ring->wptr) {
+		r = radeon_ring_alloc(rdev, ring, 1);
+		if (!r) {
+			radeon_ring_write(ring, ring->nop);
+			radeon_ring_commit(rdev, ring);
+		}
+	}
+	mutex_unlock(&ring->mutex);
+}
+
 void radeon_ring_lockup_update(struct radeon_ring *ring)
 {
 	ring->last_rptr = ring->rptr;
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index c2422f5..0bad5ff 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -2217,7 +2217,6 @@ bool si_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 	u32 srbm_status;
 	u32 grbm_status, grbm_status2;
 	u32 grbm_status_se0, grbm_status_se1;
-	int r;
 
 	srbm_status = RREG32(SRBM_STATUS);
 	grbm_status = RREG32(GRBM_STATUS);
@@ -2229,15 +2228,7 @@ bool si_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
 		return false;
 	}
 	/* force CP activities */
-	r = radeon_ring_lock(rdev, ring, 2);
-	if (!r) {
-		/* PACKET2 NOP */
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_write(ring, 0x80000000);
-		radeon_ring_unlock_commit(rdev, ring);
-	}
-	/* XXX deal with CP0,1,2 */
-	ring->rptr = RREG32(ring->rptr_reg);
+	radeon_ring_force_activity(rdev, ring);
 	return radeon_ring_test_lockup(rdev, ring);
 }
 
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/17] drm/radeon: remove r300_gpu_is_lockup

[Christian König]

Since it is now identical to r100_gpu_is_lockup.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/r300.c        |   14 --------------
 drivers/gpu/drm/radeon/radeon_asic.c |   16 ++++++++--------
 drivers/gpu/drm/radeon/radeon_asic.h |    1 -
 3 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 04ec269..6419a59 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -377,20 +377,6 @@ void r300_gpu_init(struct radeon_device *rdev)
 		 rdev->num_gb_pipes, rdev->num_z_pipes);
 }
 
-bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
-{
-	u32 rbbm_status;
-
-	rbbm_status = RREG32(R_000E40_RBBM_STATUS);
-	if (!G_000E40_GUI_ACTIVE(rbbm_status)) {
-		radeon_ring_lockup_update(ring);
-		return false;
-	}
-	/* force CP activities */
-	radeon_ring_force_activity(rdev, ring);
-	return radeon_ring_test_lockup(rdev, ring);
-}
-
 int r300_asic_reset(struct radeon_device *rdev)
 {
 	struct r100_mc_save save;
diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
index 958b9ea..5e5694e 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.c
+++ b/drivers/gpu/drm/radeon/radeon_asic.c
@@ -299,7 +299,7 @@ static struct radeon_asic r300_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -373,7 +373,7 @@ static struct radeon_asic r300_asic_pcie = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -447,7 +447,7 @@ static struct radeon_asic r420_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -521,7 +521,7 @@ static struct radeon_asic rs400_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -595,7 +595,7 @@ static struct radeon_asic rs600_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -669,7 +669,7 @@ static struct radeon_asic rs690_asic = {
 			.ring_start = &r300_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -743,7 +743,7 @@ static struct radeon_asic rv515_asic = {
 			.ring_start = &rv515_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -817,7 +817,7 @@ static struct radeon_asic r520_asic = {
 			.ring_start = &rv515_ring_start,
 			.ring_test = &r100_ring_test,
 			.ib_test = &r100_ib_test,
-			.is_lockup = &r300_gpu_is_lockup,
+			.is_lockup = &r100_gpu_is_lockup,
 		}
 	},
 	.irq = {
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index b0941f9..1e128e0 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -154,7 +154,6 @@ extern int r300_init(struct radeon_device *rdev);
 extern void r300_fini(struct radeon_device *rdev);
 extern int r300_suspend(struct radeon_device *rdev);
 extern int r300_resume(struct radeon_device *rdev);
-extern bool r300_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *cp);
 extern int r300_asic_reset(struct radeon_device *rdev);
 extern void r300_ring_start(struct radeon_device *rdev, struct radeon_ring *ring);
 extern void r300_fence_ring_emit(struct radeon_device *rdev,
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/17] drm/radeon: remove cayman_gpu_is_lockup

[Christian König]

Since it is now identical to evergreen_gpu_is_lockup.

Signed-off-by: Christian König <deathsimple@vodafone.de>
Reviewed-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/ni.c          |   19 -------------------
 drivers/gpu/drm/radeon/radeon_asic.c |   12 ++++++------
 drivers/gpu/drm/radeon/radeon_asic.h |    1 -
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 1184635..9cd2657 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1392,25 +1392,6 @@ int cayman_cp_resume(struct radeon_device *rdev)
 	return 0;
 }
 
-bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *ring)
-{
-	u32 srbm_status;
-	u32 grbm_status;
-	u32 grbm_status_se0, grbm_status_se1;
-
-	srbm_status = RREG32(SRBM_STATUS);
-	grbm_status = RREG32(GRBM_STATUS);
-	grbm_status_se0 = RREG32(GRBM_STATUS_SE0);
-	grbm_status_se1 = RREG32(GRBM_STATUS_SE1);
-	if (!(grbm_status & GUI_ACTIVE)) {
-		radeon_ring_lockup_update(ring);
-		return false;
-	}
-	/* force CP activities */
-	radeon_ring_force_activity(rdev, ring);
-	return radeon_ring_test_lockup(rdev, ring);
-}
-
 static int cayman_gpu_soft_reset(struct radeon_device *rdev)
 {
 	struct evergreen_mc_save save;
diff --git a/drivers/gpu/drm/radeon/radeon_asic.c b/drivers/gpu/drm/radeon/radeon_asic.c
index 5e5694e..f533df5 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.c
+++ b/drivers/gpu/drm/radeon/radeon_asic.c
@@ -1339,7 +1339,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP1_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1349,7 +1349,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP2_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1359,7 +1359,7 @@ static struct radeon_asic cayman_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		}
 	},
 	.irq = {
@@ -1433,7 +1433,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP1_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1443,7 +1443,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		},
 		[CAYMAN_RING_TYPE_CP2_INDEX] = {
 			.ib_execute = &cayman_ring_ib_execute,
@@ -1453,7 +1453,7 @@ static struct radeon_asic trinity_asic = {
 			.cs_parse = &evergreen_cs_parse,
 			.ring_test = &r600_ring_test,
 			.ib_test = &r600_ib_test,
-			.is_lockup = &cayman_gpu_is_lockup,
+			.is_lockup = &evergreen_gpu_is_lockup,
 		}
 	},
 	.irq = {
diff --git a/drivers/gpu/drm/radeon/radeon_asic.h b/drivers/gpu/drm/radeon/radeon_asic.h
index 1e128e0..7830931 100644
--- a/drivers/gpu/drm/radeon/radeon_asic.h
+++ b/drivers/gpu/drm/radeon/radeon_asic.h
@@ -437,7 +437,6 @@ int cayman_init(struct radeon_device *rdev);
 void cayman_fini(struct radeon_device *rdev);
 int cayman_suspend(struct radeon_device *rdev);
 int cayman_resume(struct radeon_device *rdev);
-bool cayman_gpu_is_lockup(struct radeon_device *rdev, struct radeon_ring *cp);
 int cayman_asic_reset(struct radeon_device *rdev);
 void cayman_ring_ib_execute(struct radeon_device *rdev, struct radeon_ib *ib);
 int cayman_vm_init(struct radeon_device *rdev);
-- 
1.7.5.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Include request for reset-rework branch v4

[Jerome Glisse]

On Wed, May 2, 2012 at 9:11 AM, Christian König <deathsimple@vodafone.de> wrote:
> Hi Dave,
>
> there still seems to be the need for some further discussion about the SA code,
> so I again split that out of the patchset and tested the result a bit.
>
> Most of the stuff still works fine without those offending changes, so to avoid
> mailing around unrelated and already reviewed patches, I request the include
> the following 17 patches into drm-next.
>
> If you prefer to merge they are also available from
> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>
> Cheers,
> Christian.
>

I am ok with this 17 patchset, i just sent 3 patch on top of those 17 that
bring back some other of the previous cleanup.

So for the 17
Reviewed-by: Jerome Glisse <jglisse@redhat.com>

Cheers,
Jerome

Re: Include request for reset-rework branch v4

[Christian König]

On 02.05.2012 18:01, Jerome Glisse wrote:
> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>  wrote:
>> Hi Dave,
>>
>> there still seems to be the need for some further discussion about the SA code,
>> so I again split that out of the patchset and tested the result a bit.
>>
>> Most of the stuff still works fine without those offending changes, so to avoid
>> mailing around unrelated and already reviewed patches, I request the include
>> the following 17 patches into drm-next.
>>
>> If you prefer to merge they are also available from
>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>
>> Cheers,
>> Christian.
>>
> I am ok with this 17 patchset, i just sent 3 patch on top of those 17 that
> bring back some other of the previous cleanup.
At least for now those three are NAK, cause I just realized we need to 
put those on top of a more sophisticated fence implementation.

Your idea of not using a list, but 64 bit sequences instead actually 
sounds quite nifty to me. Going to hack something together in the next 
couple of hours.

Christian.

Re: Include request for reset-rework branch v4

[Dave Airlie]

> 
> there still seems to be the need for some further discussion about the SA code,
> so I again split that out of the patchset and tested the result a bit.
> 
> Most of the stuff still works fine without those offending changes, so to avoid
> mailing around unrelated and already reviewed patches, I request the include
> the following 17 patches into drm-next.

Okay I've merged these 17 patches into drm-next.

Thanks,
Dave,

Re: Include request for reset-rework branch v4

[Jerome Glisse]

On Thu, May 3, 2012 at 4:19 AM, Christian König <deathsimple@vodafone.de> wrote:
> On 02.05.2012 18:01, Jerome Glisse wrote:
>>
>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>  wrote:
>>>
>>> Hi Dave,
>>>
>>> there still seems to be the need for some further discussion about the SA
>>> code,
>>> so I again split that out of the patchset and tested the result a bit.
>>>
>>> Most of the stuff still works fine without those offending changes, so to
>>> avoid
>>> mailing around unrelated and already reviewed patches, I request the
>>> include
>>> the following 17 patches into drm-next.
>>>
>>> If you prefer to merge they are also available from
>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>
>>> Cheers,
>>> Christian.
>>>
>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17 that
>> bring back some other of the previous cleanup.
>
> At least for now those three are NAK, cause I just realized we need to put
> those on top of a more sophisticated fence implementation.
>
> Your idea of not using a list, but 64 bit sequences instead actually sounds
> quite nifty to me. Going to hack something together in the next couple of
> hours.
>
> Christian.

Btw you said that you are having issue when using multiple ring, it
comes to my attention that you never sync with the GFX ring (unless
asked by userspace) that's wrong, before scheduling on another ring
than GFX index you need to emit semaphore to make the ring wait for
the last emited fence on the GFX ring because of ttm. What might
happen is that ttm scheduled bo move on the GFX ring and that the ring
you work on start using those bo at there soon to be GPU offset while
the bo data is not there yet.

Cheers,
Jerome

Re: Include request for reset-rework branch v4

[Christian König]

On 03.05.2012 18:32, Jerome Glisse wrote:
> On Thu, May 3, 2012 at 4:19 AM, Christian König<deathsimple@vodafone.de>  wrote:
>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>>   wrote:
>>>> Hi Dave,
>>>>
>>>> there still seems to be the need for some further discussion about the SA
>>>> code,
>>>> so I again split that out of the patchset and tested the result a bit.
>>>>
>>>> Most of the stuff still works fine without those offending changes, so to
>>>> avoid
>>>> mailing around unrelated and already reviewed patches, I request the
>>>> include
>>>> the following 17 patches into drm-next.
>>>>
>>>> If you prefer to merge they are also available from
>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>
>>>> Cheers,
>>>> Christian.
>>>>
>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17 that
>>> bring back some other of the previous cleanup.
>> At least for now those three are NAK, cause I just realized we need to put
>> those on top of a more sophisticated fence implementation.
>>
>> Your idea of not using a list, but 64 bit sequences instead actually sounds
>> quite nifty to me. Going to hack something together in the next couple of
>> hours.
>>
>> Christian.
> Btw you said that you are having issue when using multiple ring, it
> comes to my attention that you never sync with the GFX ring (unless
> asked by userspace) that's wrong, before scheduling on another ring
> than GFX index you need to emit semaphore to make the ring wait for
> the last emited fence on the GFX ring because of ttm. What might
> happen is that ttm scheduled bo move on the GFX ring and that the ring
> you work on start using those bo at there soon to be GPU offset while
> the bo data is not there yet.
Yeah, already stumbled over that but IIRC Alex already solved that issue.

We set the sync_obj to the fence of the move operation, so when there is 
a move operation in progress we sync to it correctly (at least I hope so).

Christian.

Re: Include request for reset-rework branch v4

[Jerome Glisse]

On Thu, May 3, 2012 at 12:39 PM, Christian König
<deathsimple@vodafone.de> wrote:
> On 03.05.2012 18:32, Jerome Glisse wrote:
>>
>> On Thu, May 3, 2012 at 4:19 AM, Christian König<deathsimple@vodafone.de>
>>  wrote:
>>>
>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>
>>>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>>>  wrote:
>>>>>
>>>>> Hi Dave,
>>>>>
>>>>> there still seems to be the need for some further discussion about the
>>>>> SA
>>>>> code,
>>>>> so I again split that out of the patchset and tested the result a bit.
>>>>>
>>>>> Most of the stuff still works fine without those offending changes, so
>>>>> to
>>>>> avoid
>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>> include
>>>>> the following 17 patches into drm-next.
>>>>>
>>>>> If you prefer to merge they are also available from
>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>
>>>>> Cheers,
>>>>> Christian.
>>>>>
>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>> that
>>>> bring back some other of the previous cleanup.
>>>
>>> At least for now those three are NAK, cause I just realized we need to
>>> put
>>> those on top of a more sophisticated fence implementation.
>>>
>>> Your idea of not using a list, but 64 bit sequences instead actually
>>> sounds
>>> quite nifty to me. Going to hack something together in the next couple of
>>> hours.
>>>
>>> Christian.
>>
>> Btw you said that you are having issue when using multiple ring, it
>> comes to my attention that you never sync with the GFX ring (unless
>> asked by userspace) that's wrong, before scheduling on another ring
>> than GFX index you need to emit semaphore to make the ring wait for
>> the last emited fence on the GFX ring because of ttm. What might
>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>> you work on start using those bo at there soon to be GPU offset while
>> the bo data is not there yet.
>
> Yeah, already stumbled over that but IIRC Alex already solved that issue.
>
> We set the sync_obj to the fence of the move operation, so when there is a
> move operation in progress we sync to it correctly (at least I hope so).
>
> Christian.
>

Looking at code doesn't seems ok, yes you use the fence from the move
operation but you only emit wait if userspace ask for it, that's
wrong.

if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {

in radeon_cs.c

Cheers,
Jerome

Re: Include request for reset-rework branch v4

[Alex Deucher]

2012/5/3 Jerome Glisse <j.glisse@gmail.com>:
> On Thu, May 3, 2012 at 12:39 PM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>
>>> On Thu, May 3, 2012 at 4:19 AM, Christian König<deathsimple@vodafone.de>
>>>  wrote:
>>>>
>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>
>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>>>>  wrote:
>>>>>>
>>>>>> Hi Dave,
>>>>>>
>>>>>> there still seems to be the need for some further discussion about the
>>>>>> SA
>>>>>> code,
>>>>>> so I again split that out of the patchset and tested the result a bit.
>>>>>>
>>>>>> Most of the stuff still works fine without those offending changes, so
>>>>>> to
>>>>>> avoid
>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>> include
>>>>>> the following 17 patches into drm-next.
>>>>>>
>>>>>> If you prefer to merge they are also available from
>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>
>>>>>> Cheers,
>>>>>> Christian.
>>>>>>
>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>> that
>>>>> bring back some other of the previous cleanup.
>>>>
>>>> At least for now those three are NAK, cause I just realized we need to
>>>> put
>>>> those on top of a more sophisticated fence implementation.
>>>>
>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>> sounds
>>>> quite nifty to me. Going to hack something together in the next couple of
>>>> hours.
>>>>
>>>> Christian.
>>>
>>> Btw you said that you are having issue when using multiple ring, it
>>> comes to my attention that you never sync with the GFX ring (unless
>>> asked by userspace) that's wrong, before scheduling on another ring
>>> than GFX index you need to emit semaphore to make the ring wait for
>>> the last emited fence on the GFX ring because of ttm. What might
>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>> you work on start using those bo at there soon to be GPU offset while
>>> the bo data is not there yet.
>>
>> Yeah, already stumbled over that but IIRC Alex already solved that issue.
>>
>> We set the sync_obj to the fence of the move operation, so when there is a
>> move operation in progress we sync to it correctly (at least I hope so).
>>
>> Christian.
>>
>
> Looking at code doesn't seems ok, yes you use the fence from the move
> operation but you only emit wait if userspace ask for it, that's
> wrong.
>
> if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
>

The default is to sync all the rings.  We only skip the sync if the
_DONT_ sync flag is set.

Alex

> in radeon_cs.c
>
> Cheers,
> Jerome

Re: Include request for reset-rework branch v4

[Jerome Glisse]

On Thu, May 3, 2012 at 1:20 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> 2012/5/3 Jerome Glisse <j.glisse@gmail.com>:
>> On Thu, May 3, 2012 at 12:39 PM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>>
>>>> On Thu, May 3, 2012 at 4:19 AM, Christian König<deathsimple@vodafone.de>
>>>>  wrote:
>>>>>
>>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>>
>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>>>>>  wrote:
>>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> there still seems to be the need for some further discussion about the
>>>>>>> SA
>>>>>>> code,
>>>>>>> so I again split that out of the patchset and tested the result a bit.
>>>>>>>
>>>>>>> Most of the stuff still works fine without those offending changes, so
>>>>>>> to
>>>>>>> avoid
>>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>>> include
>>>>>>> the following 17 patches into drm-next.
>>>>>>>
>>>>>>> If you prefer to merge they are also available from
>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Christian.
>>>>>>>
>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>>> that
>>>>>> bring back some other of the previous cleanup.
>>>>>
>>>>> At least for now those three are NAK, cause I just realized we need to
>>>>> put
>>>>> those on top of a more sophisticated fence implementation.
>>>>>
>>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>>> sounds
>>>>> quite nifty to me. Going to hack something together in the next couple of
>>>>> hours.
>>>>>
>>>>> Christian.
>>>>
>>>> Btw you said that you are having issue when using multiple ring, it
>>>> comes to my attention that you never sync with the GFX ring (unless
>>>> asked by userspace) that's wrong, before scheduling on another ring
>>>> than GFX index you need to emit semaphore to make the ring wait for
>>>> the last emited fence on the GFX ring because of ttm. What might
>>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>>> you work on start using those bo at there soon to be GPU offset while
>>>> the bo data is not there yet.
>>>
>>> Yeah, already stumbled over that but IIRC Alex already solved that issue.
>>>
>>> We set the sync_obj to the fence of the move operation, so when there is a
>>> move operation in progress we sync to it correctly (at least I hope so).
>>>
>>> Christian.
>>>
>>
>> Looking at code doesn't seems ok, yes you use the fence from the move
>> operation but you only emit wait if userspace ask for it, that's
>> wrong.
>>
>> if (!(p->relocs[i].flags & RADEON_RELOC_DONT_SYNC)) {
>>
>
> The default is to sync all the rings.  We only skip the sync if the
> _DONT_ sync flag is set.
>
> Alex
>

That's my point exactly, so if a bo move happen and userspace set dont
sync flag then the other ring might start doing work before bo data is
in place. This test should be removed or at least make the sync to the
bo move fence unconditional and use this flag only for sync with other
ring relative to current job you are scheduling.

Cheers,
Jerome

Re: Include request for reset-rework branch v4

[Christian König]

On 03.05.2012 19:20, Alex Deucher wrote:
> 2012/5/3 Jerome Glisse<j.glisse@gmail.com>:
>> On Thu, May 3, 2012 at 12:39 PM, Christian König
>> <deathsimple@vodafone.de>  wrote:
>>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>> On Thu, May 3, 2012 at 4:19 AM, Christian König<deathsimple@vodafone.de>
>>>>   wrote:
>>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian König<deathsimple@vodafone.de>
>>>>>>   wrote:
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> there still seems to be the need for some further discussion about the
>>>>>>> SA
>>>>>>> code,
>>>>>>> so I again split that out of the patchset and tested the result a bit.
>>>>>>>
>>>>>>> Most of the stuff still works fine without those offending changes, so
>>>>>>> to
>>>>>>> avoid
>>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>>> include
>>>>>>> the following 17 patches into drm-next.
>>>>>>>
>>>>>>> If you prefer to merge they are also available from
>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>>
>>>>>>> Cheers,
>>>>>>> Christian.
>>>>>>>
>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>>> that
>>>>>> bring back some other of the previous cleanup.
>>>>> At least for now those three are NAK, cause I just realized we need to
>>>>> put
>>>>> those on top of a more sophisticated fence implementation.
>>>>>
>>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>>> sounds
>>>>> quite nifty to me. Going to hack something together in the next couple of
>>>>> hours.
>>>>>
>>>>> Christian.
>>>> Btw you said that you are having issue when using multiple ring, it
>>>> comes to my attention that you never sync with the GFX ring (unless
>>>> asked by userspace) that's wrong, before scheduling on another ring
>>>> than GFX index you need to emit semaphore to make the ring wait for
>>>> the last emited fence on the GFX ring because of ttm. What might
>>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>>> you work on start using those bo at there soon to be GPU offset while
>>>> the bo data is not there yet.
>>> Yeah, already stumbled over that but IIRC Alex already solved that issue..
>>>
>>> We set the sync_obj to the fence of the move operation, so when there is a
>>> move operation in progress we sync to it correctly (at least I hope so).
>>>
>>> Christian.
>>>
>> Looking at code doesn't seems ok, yes you use the fence from the move
>> operation but you only emit wait if userspace ask for it, that's
>> wrong.
>>
>> if (!(p->relocs[i].flags&  RADEON_RELOC_DONT_SYNC)) {
>>
> The default is to sync all the rings.  We only skip the sync if the
> _DONT_ sync flag is set.
Yeah, but the _DONT_ sync flag is just an optimization, and we only want 
to not sync to things userspace has submitted.

  E.g. userspace tells us that two jobs can happily run on the same bo 
even if they are both writing to it....

So Jerome is completely right, userspace doesn't expect that TTM is 
jumping in between and moving the bo around, that is indeed a bug and 
another thing for the todo list.

Christian.

Re: Include request for reset-rework branch v4

[Jerome Glisse]

On Thu, May 3, 2012 at 1:28 PM, Christian König <deathsimple@vodafone.de> wrote:
> On 03.05.2012 19:20, Alex Deucher wrote:
>>
>> 2012/5/3 Jerome Glisse<j.glisse@gmail.com>:
>>>
>>> On Thu, May 3, 2012 at 12:39 PM, Christian König
>>> <deathsimple@vodafone.de>  wrote:
>>>>
>>>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>>>
>>>>> On Thu, May 3, 2012 at 4:19 AM, Christian
>>>>> König<deathsimple@vodafone.de>
>>>>>  wrote:
>>>>>>
>>>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>>>
>>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian
>>>>>>> König<deathsimple@vodafone.de>
>>>>>>>  wrote:
>>>>>>>>
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> there still seems to be the need for some further discussion about
>>>>>>>> the
>>>>>>>> SA
>>>>>>>> code,
>>>>>>>> so I again split that out of the patchset and tested the result a
>>>>>>>> bit.
>>>>>>>>
>>>>>>>> Most of the stuff still works fine without those offending changes,
>>>>>>>> so
>>>>>>>> to
>>>>>>>> avoid
>>>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>>>> include
>>>>>>>> the following 17 patches into drm-next.
>>>>>>>>
>>>>>>>> If you prefer to merge they are also available from
>>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Christian.
>>>>>>>>
>>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>>>> that
>>>>>>> bring back some other of the previous cleanup.
>>>>>>
>>>>>> At least for now those three are NAK, cause I just realized we need to
>>>>>> put
>>>>>> those on top of a more sophisticated fence implementation.
>>>>>>
>>>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>>>> sounds
>>>>>> quite nifty to me. Going to hack something together in the next couple
>>>>>> of
>>>>>> hours.
>>>>>>
>>>>>> Christian.
>>>>>
>>>>> Btw you said that you are having issue when using multiple ring, it
>>>>> comes to my attention that you never sync with the GFX ring (unless
>>>>> asked by userspace) that's wrong, before scheduling on another ring
>>>>> than GFX index you need to emit semaphore to make the ring wait for
>>>>> the last emited fence on the GFX ring because of ttm. What might
>>>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>>>> you work on start using those bo at there soon to be GPU offset while
>>>>> the bo data is not there yet.
>>>>
>>>> Yeah, already stumbled over that but IIRC Alex already solved that
>>>> issue..
>>>>
>>>>
>>>> We set the sync_obj to the fence of the move operation, so when there is
>>>> a
>>>> move operation in progress we sync to it correctly (at least I hope so).
>>>>
>>>> Christian.
>>>>
>>> Looking at code doesn't seems ok, yes you use the fence from the move
>>> operation but you only emit wait if userspace ask for it, that's
>>> wrong.
>>>
>>> if (!(p->relocs[i].flags&  RADEON_RELOC_DONT_SYNC)) {
>>>
>> The default is to sync all the rings.  We only skip the sync if the
>> _DONT_ sync flag is set.
>
> Yeah, but the _DONT_ sync flag is just an optimization, and we only want to
> not sync to things userspace has submitted.
>
>  E.g. userspace tells us that two jobs can happily run on the same bo even
> if they are both writing to it....
>
> So Jerome is completely right, userspace doesn't expect that TTM is jumping
> in between and moving the bo around, that is indeed a bug and another thing
> for the todo list.
>
> Christian.

Well until userspace can tell kernel on which fence it wants to wait i
believe this flags become obsolete and can be remove, i am pretty no
release userspace code ever used that flags.

Cheers,
Jerome

Re: Include request for reset-rework branch v4

[Alex Deucher]

2012/5/3 Christian König <deathsimple@vodafone.de>:
> On 03.05.2012 19:20, Alex Deucher wrote:
>>
>> 2012/5/3 Jerome Glisse<j.glisse@gmail.com>:
>>>
>>> On Thu, May 3, 2012 at 12:39 PM, Christian König
>>> <deathsimple@vodafone.de>  wrote:
>>>>
>>>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>>>
>>>>> On Thu, May 3, 2012 at 4:19 AM, Christian
>>>>> König<deathsimple@vodafone.de>
>>>>>  wrote:
>>>>>>
>>>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>>>
>>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian
>>>>>>> König<deathsimple@vodafone.de>
>>>>>>>  wrote:
>>>>>>>>
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> there still seems to be the need for some further discussion about
>>>>>>>> the
>>>>>>>> SA
>>>>>>>> code,
>>>>>>>> so I again split that out of the patchset and tested the result a
>>>>>>>> bit.
>>>>>>>>
>>>>>>>> Most of the stuff still works fine without those offending changes,
>>>>>>>> so
>>>>>>>> to
>>>>>>>> avoid
>>>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>>>> include
>>>>>>>> the following 17 patches into drm-next.
>>>>>>>>
>>>>>>>> If you prefer to merge they are also available from
>>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> Christian.
>>>>>>>>
>>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>>>> that
>>>>>>> bring back some other of the previous cleanup.
>>>>>>
>>>>>> At least for now those three are NAK, cause I just realized we need to
>>>>>> put
>>>>>> those on top of a more sophisticated fence implementation.
>>>>>>
>>>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>>>> sounds
>>>>>> quite nifty to me. Going to hack something together in the next couple
>>>>>> of
>>>>>> hours.
>>>>>>
>>>>>> Christian.
>>>>>
>>>>> Btw you said that you are having issue when using multiple ring, it
>>>>> comes to my attention that you never sync with the GFX ring (unless
>>>>> asked by userspace) that's wrong, before scheduling on another ring
>>>>> than GFX index you need to emit semaphore to make the ring wait for
>>>>> the last emited fence on the GFX ring because of ttm. What might
>>>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>>>> you work on start using those bo at there soon to be GPU offset while
>>>>> the bo data is not there yet.
>>>>
>>>> Yeah, already stumbled over that but IIRC Alex already solved that
>>>> issue..
>>>>
>>>>
>>>> We set the sync_obj to the fence of the move operation, so when there is
>>>> a
>>>> move operation in progress we sync to it correctly (at least I hope so).
>>>>
>>>> Christian.
>>>>
>>> Looking at code doesn't seems ok, yes you use the fence from the move
>>> operation but you only emit wait if userspace ask for it, that's
>>> wrong.
>>>
>>> if (!(p->relocs[i].flags&  RADEON_RELOC_DONT_SYNC)) {
>>>
>> The default is to sync all the rings.  We only skip the sync if the
>> _DONT_ sync flag is set.
>
> Yeah, but the _DONT_ sync flag is just an optimization, and we only want to
> not sync to things userspace has submitted.
>
>  E.g. userspace tells us that two jobs can happily run on the same bo even
> if they are both writing to it....
>
> So Jerome is completely right, userspace doesn't expect that TTM is jumping
> in between and moving the bo around, that is indeed a bug and another thing
> for the todo list.

Ah, yes, that makes sense.  I misinterpreted the original comment.

Alex

Re: Include request for reset-rework branch v4

[Christian König]

On 03.05.2012 19:34, Jerome Glisse wrote:
> On Thu, May 3, 2012 at 1:28 PM, Christian König<deathsimple@vodafone.de>  wrote:
>> On 03.05.2012 19:20, Alex Deucher wrote:
>>> 2012/5/3 Jerome Glisse<j.glisse@gmail.com>:
>>>> On Thu, May 3, 2012 at 12:39 PM, Christian König
>>>> <deathsimple@vodafone.de>    wrote:
>>>>> On 03.05.2012 18:32, Jerome Glisse wrote:
>>>>>> On Thu, May 3, 2012 at 4:19 AM, Christian
>>>>>> König<deathsimple@vodafone.de>
>>>>>>   wrote:
>>>>>>> On 02.05.2012 18:01, Jerome Glisse wrote:
>>>>>>>> On Wed, May 2, 2012 at 9:11 AM, Christian
>>>>>>>> König<deathsimple@vodafone.de>
>>>>>>>>   wrote:
>>>>>>>>> Hi Dave,
>>>>>>>>>
>>>>>>>>> there still seems to be the need for some further discussion about
>>>>>>>>> the
>>>>>>>>> SA
>>>>>>>>> code,
>>>>>>>>> so I again split that out of the patchset and tested the result a
>>>>>>>>> bit.
>>>>>>>>>
>>>>>>>>> Most of the stuff still works fine without those offending changes,
>>>>>>>>> so
>>>>>>>>> to
>>>>>>>>> avoid
>>>>>>>>> mailing around unrelated and already reviewed patches, I request the
>>>>>>>>> include
>>>>>>>>> the following 17 patches into drm-next.
>>>>>>>>>
>>>>>>>>> If you prefer to merge they are also available from
>>>>>>>>> git://people.freedesktop.org/~deathsimple/linux branch reset-rework.
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Christian.
>>>>>>>>>
>>>>>>>> I am ok with this 17 patchset, i just sent 3 patch on top of those 17
>>>>>>>> that
>>>>>>>> bring back some other of the previous cleanup.
>>>>>>> At least for now those three are NAK, cause I just realized we need to
>>>>>>> put
>>>>>>> those on top of a more sophisticated fence implementation.
>>>>>>>
>>>>>>> Your idea of not using a list, but 64 bit sequences instead actually
>>>>>>> sounds
>>>>>>> quite nifty to me. Going to hack something together in the next couple
>>>>>>> of
>>>>>>> hours.
>>>>>>>
>>>>>>> Christian.
>>>>>> Btw you said that you are having issue when using multiple ring, it
>>>>>> comes to my attention that you never sync with the GFX ring (unless
>>>>>> asked by userspace) that's wrong, before scheduling on another ring
>>>>>> than GFX index you need to emit semaphore to make the ring wait for
>>>>>> the last emited fence on the GFX ring because of ttm. What might
>>>>>> happen is that ttm scheduled bo move on the GFX ring and that the ring
>>>>>> you work on start using those bo at there soon to be GPU offset while
>>>>>> the bo data is not there yet.
>>>>> Yeah, already stumbled over that but IIRC Alex already solved that
>>>>> issue..
>>>>>
>>>>>
>>>>> We set the sync_obj to the fence of the move operation, so when there is
>>>>> a
>>>>> move operation in progress we sync to it correctly (at least I hope so).
>>>>>
>>>>> Christian.
>>>>>
>>>> Looking at code doesn't seems ok, yes you use the fence from the move
>>>> operation but you only emit wait if userspace ask for it, that's
>>>> wrong.
>>>>
>>>> if (!(p->relocs[i].flags&    RADEON_RELOC_DONT_SYNC)) {
>>>>
>>> The default is to sync all the rings.  We only skip the sync if the
>>> _DONT_ sync flag is set.
>> Yeah, but the _DONT_ sync flag is just an optimization, and we only want to
>> not sync to things userspace has submitted.
>>
>>   E.g. userspace tells us that two jobs can happily run on the same bo even
>> if they are both writing to it....
>>
>> So Jerome is completely right, userspace doesn't expect that TTM is jumping
>> in between and moving the bo around, that is indeed a bug and another thing
>> for the todo list.
>>
>> Christian.
> Well until userspace can tell kernel on which fence it wants to wait i
> believe this flags become obsolete and can be remove, i am pretty no
> release userspace code ever used that flags.

Agree, AFAIK it currently isn't used anyway.

Christian.