* [PATCH 01/26] drm/radeon: make radeon_gpu_is_lockup a per ring function
2012-04-30 14:50 Include request for reset-rework branch Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 02/26] drm/radeon: replace gpu_lockup with ring->ready flag Christian König
` (26 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 566ca3b..1bcd2b3 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1153,7 +1153,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
@@ -1182,6 +1181,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 {
@@ -1739,7 +1739,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))
@@ -1748,6 +1747,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 02/26] drm/radeon: replace gpu_lockup with ring->ready flag
2012-04-30 14:50 Include request for reset-rework branch Christian König
2012-04-30 14:50 ` [PATCH 01/26] drm/radeon: make radeon_gpu_is_lockup a per ring function Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 03/26] drm/radeon: register ring debugfs handlers on init Christian König
` (25 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 e11df77..4b677fc 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 1bcd2b3..bb08702 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -1538,7 +1538,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 03/26] drm/radeon: register ring debugfs handlers on init
2012-04-30 14:50 Include request for reset-rework branch Christian König
2012-04-30 14:50 ` [PATCH 01/26] drm/radeon: make radeon_gpu_is_lockup a per ring function Christian König
2012-04-30 14:50 ` [PATCH 02/26] drm/radeon: replace gpu_lockup with ring->ready flag Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 04/26] drm/radeon: use central function for IB testing Christian König
` (24 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 04/26] drm/radeon: use central function for IB testing
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (2 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 03/26] drm/radeon: register ring debugfs handlers on init Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 05/26] drm/radeon: rework gpu lockup detection and processing Christian König
` (23 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 4b677fc..62f9dab 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 ba637d9..db9415a 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 bb08702..e237127 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 05/26] drm/radeon: rework gpu lockup detection and processing
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (3 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 04/26] drm/radeon: use central function for IB testing Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 06/26] drm/radeon: fix a bug in the SA code Christian König
` (22 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Previusly multiple rings could trigger multiple GPU
resets at the same time.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 e237127..92682b7 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 06/26] drm/radeon: fix a bug in the SA code
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (4 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 05/26] drm/radeon: rework gpu lockup detection and processing Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 07/26] drm/radeon: add proper locking to the SA v2 Christian König
` (21 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 07/26] drm/radeon: add proper locking to the SA v2
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (5 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 06/26] drm/radeon: fix a bug in the SA code Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 15:58 ` Jerome Glisse
2012-04-30 14:50 ` [PATCH 08/26] drm/radeon: add sub allocator debugfs file Christian König
` (20 subsequent siblings)
27 siblings, 1 reply; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Make the suballocator self containing to locking.
v2: split the bugfix into a seperate patch.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 1 +
drivers/gpu/drm/radeon/radeon_sa.c | 17 +++++++++++------
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 92682b7..99b188a 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -381,6 +381,7 @@ struct radeon_bo_list {
* alignment).
*/
struct radeon_sa_manager {
+ spinlock_t lock;
struct radeon_bo *bo;
struct list_head sa_bo;
unsigned size;
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 8fbfe69..4ce5c51 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -37,6 +37,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
{
int r;
+ spin_lock_init(&sa_manager->lock);
sa_manager->bo = NULL;
sa_manager->size = size;
sa_manager->domain = domain;
@@ -136,18 +137,19 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
struct radeon_sa_bo *tmp;
struct list_head *head;
unsigned offset = 0, wasted = 0;
+ unsigned long flags;
BUG_ON(align > RADEON_GPU_PAGE_SIZE);
BUG_ON(size > sa_manager->size);
+ spin_lock_irqsave(&sa_manager->lock, flags);
/* no one ? */
- head = sa_manager->sa_bo.prev;
if (list_empty(&sa_manager->sa_bo)) {
+ head = &sa_manager->sa_bo;
goto out;
}
/* look for a hole big enough */
- offset = 0;
list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
/* room before this object ? */
if (offset < tmp->offset && (tmp->offset - offset) >= size) {
@@ -157,9 +159,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
offset = tmp->offset + tmp->size;
wasted = offset % align;
if (wasted) {
- wasted = align - wasted;
+ offset += align - wasted;
}
- offset += wasted;
}
/* room at the end ? */
head = sa_manager->sa_bo.prev;
@@ -167,11 +168,11 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
offset = tmp->offset + tmp->size;
wasted = offset % align;
if (wasted) {
- wasted = align - wasted;
+ offset += wasted = align - wasted;
}
- offset += wasted;
if ((sa_manager->size - offset) < size) {
/* failed to find somethings big enough */
+ spin_unlock_irqrestore(&sa_manager->lock, flags);
return -ENOMEM;
}
@@ -180,10 +181,14 @@ out:
sa_bo->offset = offset;
sa_bo->size = size;
list_add(&sa_bo->list, head);
+ spin_unlock_irqrestore(&sa_manager->lock, flags);
return 0;
}
void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
{
+ unsigned long flags;
+ spin_lock_irqsave(&sa_bo->manager->lock, flags);
list_del_init(&sa_bo->list);
+ spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
}
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH 07/26] drm/radeon: add proper locking to the SA v2
2012-04-30 14:50 ` [PATCH 07/26] drm/radeon: add proper locking to the SA v2 Christian König
@ 2012-04-30 15:58 ` Jerome Glisse
2012-05-01 12:31 ` Christian König
0 siblings, 1 reply; 39+ messages in thread
From: Jerome Glisse @ 2012-04-30 15:58 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Mon, Apr 30, 2012 at 10:50 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Make the suballocator self containing to locking.
>
> v2: split the bugfix into a seperate patch.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
I would say NAK but i don't have better solution yet to the issue.
Idea is that cs mutex protect the SA, i am trying hard to avoid adding
new lock, in fact i would like to remove more lock and have some idea
for that. I looked at the whole patch set and the issue is with radeon
semaphore allocation in ttm move blit case, all other case have the cs
mutex taken already. So if no better solution than yeah introduce a
new lock.
Cheers,
Jerome
> ---
> drivers/gpu/drm/radeon/radeon.h | 1 +
> drivers/gpu/drm/radeon/radeon_sa.c | 17 +++++++++++------
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 92682b7..99b188a 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -381,6 +381,7 @@ struct radeon_bo_list {
> * alignment).
> */
> struct radeon_sa_manager {
> + spinlock_t lock;
> struct radeon_bo *bo;
> struct list_head sa_bo;
> unsigned size;
> diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
> index 8fbfe69..4ce5c51 100644
> --- a/drivers/gpu/drm/radeon/radeon_sa.c
> +++ b/drivers/gpu/drm/radeon/radeon_sa.c
> @@ -37,6 +37,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
> {
> int r;
>
> + spin_lock_init(&sa_manager->lock);
> sa_manager->bo = NULL;
> sa_manager->size = size;
> sa_manager->domain = domain;
> @@ -136,18 +137,19 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
> struct radeon_sa_bo *tmp;
> struct list_head *head;
> unsigned offset = 0, wasted = 0;
> + unsigned long flags;
>
> BUG_ON(align > RADEON_GPU_PAGE_SIZE);
> BUG_ON(size > sa_manager->size);
> + spin_lock_irqsave(&sa_manager->lock, flags);
>
> /* no one ? */
> - head = sa_manager->sa_bo.prev;
> if (list_empty(&sa_manager->sa_bo)) {
> + head = &sa_manager->sa_bo;
> goto out;
> }
>
> /* look for a hole big enough */
> - offset = 0;
> list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
> /* room before this object ? */
> if (offset < tmp->offset && (tmp->offset - offset) >= size) {
> @@ -157,9 +159,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
> offset = tmp->offset + tmp->size;
> wasted = offset % align;
> if (wasted) {
> - wasted = align - wasted;
> + offset += align - wasted;
> }
> - offset += wasted;
> }
> /* room at the end ? */
> head = sa_manager->sa_bo.prev;
> @@ -167,11 +168,11 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
> offset = tmp->offset + tmp->size;
> wasted = offset % align;
> if (wasted) {
> - wasted = align - wasted;
> + offset += wasted = align - wasted;
> }
> - offset += wasted;
> if ((sa_manager->size - offset) < size) {
> /* failed to find somethings big enough */
> + spin_unlock_irqrestore(&sa_manager->lock, flags);
> return -ENOMEM;
> }
>
> @@ -180,10 +181,14 @@ out:
> sa_bo->offset = offset;
> sa_bo->size = size;
> list_add(&sa_bo->list, head);
> + spin_unlock_irqrestore(&sa_manager->lock, flags);
> return 0;
> }
>
> void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
> {
> + unsigned long flags;
> + spin_lock_irqsave(&sa_bo->manager->lock, flags);
> list_del_init(&sa_bo->list);
> + spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
> }
> --
> 1.7.5.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 07/26] drm/radeon: add proper locking to the SA v2
2012-04-30 15:58 ` Jerome Glisse
@ 2012-05-01 12:31 ` Christian König
0 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-05-01 12:31 UTC (permalink / raw)
To: Jerome Glisse; +Cc: airlied, dri-devel
On 30.04.2012 17:58, Jerome Glisse wrote:
> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Make the suballocator self containing to locking.
>>
>> v2: split the bugfix into a seperate patch.
>>
>> Signed-off-by: Christian König<deathsimple@vodafone.de>
> I would say NAK but i don't have better solution yet to the issue.
> Idea is that cs mutex protect the SA, i am trying hard to avoid adding
> new lock, in fact i would like to remove more lock and have some idea
> for that. I looked at the whole patch set and the issue is with radeon
> semaphore allocation in ttm move blit case, all other case have the cs
> mutex taken already. So if no better solution than yeah introduce a
> new lock.
Well, my idea was to remove the cs mutex in the long term. But just
reading your other mail, I think I better describe the picture as a
whole there.
Just let me mention that I intended to be this the lock for the
"transaction memory allocator", so it pretty much matches your idea also.
Christian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 08/26] drm/radeon: add sub allocator debugfs file
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (6 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 07/26] drm/radeon: add proper locking to the SA v2 Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3 Christian König
` (19 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Dumping the current allocations.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon_object.h | 5 +++++
drivers/gpu/drm/radeon/radeon_ring.c | 22 ++++++++++++++++++++++
drivers/gpu/drm/radeon/radeon_sa.c | 15 +++++++++++++++
3 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index f9104be..d9b9333 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -161,5 +161,10 @@ extern int radeon_sa_bo_new(struct radeon_device *rdev,
unsigned size, unsigned align);
extern void radeon_sa_bo_free(struct radeon_device *rdev,
struct radeon_sa_bo *sa_bo);
+#if defined(CONFIG_DEBUG_FS)
+extern void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
+ struct seq_file *m);
+#endif
+
#endif
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 1b020ef..1d9bce9 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -529,6 +529,23 @@ static int radeon_debugfs_ib_info(struct seq_file *m, void *data)
static struct drm_info_list radeon_debugfs_ib_list[RADEON_IB_POOL_SIZE];
static char radeon_debugfs_ib_names[RADEON_IB_POOL_SIZE][32];
static unsigned radeon_debugfs_ib_idx[RADEON_IB_POOL_SIZE];
+
+static int radeon_debugfs_sa_info(struct seq_file *m, void *data)
+{
+ struct drm_info_node *node = (struct drm_info_node *) m->private;
+ struct drm_device *dev = node->minor->dev;
+ struct radeon_device *rdev = dev->dev_private;
+
+ radeon_sa_bo_dump_debug_info(&rdev->ib_pool.sa_manager, m);
+
+ return 0;
+
+}
+
+static struct drm_info_list radeon_debugfs_sa_list[] = {
+ {"radeon_sa_info", &radeon_debugfs_sa_info, 0, NULL},
+};
+
#endif
int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring)
@@ -555,6 +572,11 @@ int radeon_debugfs_ib_init(struct radeon_device *rdev)
{
#if defined(CONFIG_DEBUG_FS)
unsigned i;
+ int r;
+
+ r = radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1);
+ if (r)
+ return r;
for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
sprintf(radeon_debugfs_ib_names[i], "radeon_ib_%04u", i);
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 4ce5c51..013a787 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -192,3 +192,18 @@ void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
list_del_init(&sa_bo->list);
spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
}
+
+#if defined(CONFIG_DEBUG_FS)
+void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
+ struct seq_file *m)
+{
+ struct radeon_sa_bo *i;
+ unsigned long flags;
+
+ spin_lock_irqsave(&sa_manager->lock, flags);
+ list_for_each_entry(i, &sa_manager->sa_bo, list) {
+ seq_printf(m, "offset %08d: size %4d\n", i->offset, i->size);
+ }
+ spin_unlock_irqrestore(&sa_manager->lock, flags);
+}
+#endif
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (7 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 08/26] drm/radeon: add sub allocator debugfs file Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 15:45 ` Jerome Glisse
2012-04-30 14:50 ` [PATCH 10/26] drm/radeon: add try_free callback to the SA Christian König
` (18 subsequent siblings)
27 siblings, 1 reply; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
With that in place clients are automatically blocking
until their memory request can be handled.
v2: block only if the memory request can't be satisfied
in the first try, the first version actually lacked
a night of sleep.
v3: make blocking optional, update comments and fix
another bug with biggest hole tracking.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 5 +-
drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.h | 2 +-
drivers/gpu/drm/radeon/radeon_ring.c | 20 ++--
drivers/gpu/drm/radeon/radeon_sa.c | 211 +++++++++++++++++++++++---------
5 files changed, 165 insertions(+), 75 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 99b188a..e71dc67 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -381,17 +381,16 @@ struct radeon_bo_list {
* alignment).
*/
struct radeon_sa_manager {
- spinlock_t lock;
+ wait_queue_head_t queue;
struct radeon_bo *bo;
struct list_head sa_bo;
unsigned size;
+ struct list_head *biggest_hole;
uint64_t gpu_addr;
void *cpu_ptr;
uint32_t domain;
};
-struct radeon_sa_bo;
-
/* sub-allocation buffer */
struct radeon_sa_bo {
struct list_head list;
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index c58a036..7af4ff9 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -395,7 +395,7 @@ int radeon_vm_bind(struct radeon_device *rdev, struct radeon_vm *vm)
retry:
r = radeon_sa_bo_new(rdev, &rdev->vm_manager.sa_manager, &vm->sa_bo,
RADEON_GPU_PAGE_ALIGN(vm->last_pfn * 8),
- RADEON_GPU_PAGE_SIZE);
+ RADEON_GPU_PAGE_SIZE, false);
if (r) {
if (list_empty(&rdev->vm_manager.lru_vm)) {
return r;
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index d9b9333..85f33d9 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -158,7 +158,7 @@ extern int radeon_sa_bo_manager_suspend(struct radeon_device *rdev,
extern int radeon_sa_bo_new(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager,
struct radeon_sa_bo *sa_bo,
- unsigned size, unsigned align);
+ unsigned size, unsigned align, bool block);
extern void radeon_sa_bo_free(struct radeon_device *rdev,
struct radeon_sa_bo *sa_bo);
#if defined(CONFIG_DEBUG_FS)
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 1d9bce9..ccee74f 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -124,7 +124,7 @@ retry:
if (rdev->ib_pool.ibs[idx].fence == NULL) {
r = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager,
&rdev->ib_pool.ibs[idx].sa_bo,
- size, 256);
+ size, 256, false);
if (!r) {
*ib = &rdev->ib_pool.ibs[idx];
(*ib)->ptr = rdev->ib_pool.sa_manager.cpu_ptr;
@@ -205,10 +205,16 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
int radeon_ib_pool_init(struct radeon_device *rdev)
{
- struct radeon_sa_manager tmp;
int i, r;
- r = radeon_sa_bo_manager_init(rdev, &tmp,
+ radeon_mutex_lock(&rdev->ib_pool.mutex);
+ if (rdev->ib_pool.ready) {
+ return 0;
+ }
+ rdev->ib_pool.ready = true;
+ radeon_mutex_unlock(&rdev->ib_pool.mutex);
+
+ r = radeon_sa_bo_manager_init(rdev, &rdev->ib_pool.sa_manager,
RADEON_IB_POOL_SIZE*64*1024,
RADEON_GEM_DOMAIN_GTT);
if (r) {
@@ -216,14 +222,6 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
}
radeon_mutex_lock(&rdev->ib_pool.mutex);
- if (rdev->ib_pool.ready) {
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
- radeon_sa_bo_manager_fini(rdev, &tmp);
- return 0;
- }
-
- rdev->ib_pool.sa_manager = tmp;
- INIT_LIST_HEAD(&rdev->ib_pool.sa_manager.sa_bo);
for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
rdev->ib_pool.ibs[i].fence = NULL;
rdev->ib_pool.ibs[i].idx = i;
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 013a787..92ab7b4 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -26,6 +26,7 @@
/*
* Authors:
* Jerome Glisse <glisse@freedesktop.org>
+ * Christian König <christian.koenig@amd.com>
*/
#include "drmP.h"
#include "drm.h"
@@ -37,9 +38,10 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
{
int r;
- spin_lock_init(&sa_manager->lock);
+ init_waitqueue_head(&sa_manager->queue);
sa_manager->bo = NULL;
sa_manager->size = size;
+ sa_manager->biggest_hole = &sa_manager->sa_bo;
sa_manager->domain = domain;
INIT_LIST_HEAD(&sa_manager->sa_bo);
@@ -58,6 +60,7 @@ void radeon_sa_bo_manager_fini(struct radeon_device *rdev,
{
struct radeon_sa_bo *sa_bo, *tmp;
+ wake_up_all(&sa_manager->queue);
if (!list_empty(&sa_manager->sa_bo)) {
dev_err(rdev->dev, "sa_manager is not empty, clearing anyway\n");
}
@@ -114,96 +117,186 @@ int radeon_sa_bo_manager_suspend(struct radeon_device *rdev,
return r;
}
+static inline unsigned radeon_sa_bo_hole_start(struct radeon_sa_manager *m,
+ struct list_head *entry)
+{
+ struct radeon_sa_bo *sa_bo;
+
+ if (entry == &m->sa_bo)
+ return 0;
+
+ sa_bo = list_entry(entry, struct radeon_sa_bo, list);
+ return sa_bo->offset + sa_bo->size;
+}
+
+static inline unsigned radeon_sa_bo_hole_end(struct radeon_sa_manager *m,
+ struct list_head *entry)
+{
+ if (entry->next == &m->sa_bo)
+ return m->size;
+
+ return list_entry(entry->next, struct radeon_sa_bo, list)->offset;
+}
+
+static inline unsigned radeon_sa_bo_min_free(struct radeon_sa_manager *m,
+ unsigned align)
+{
+ unsigned start, end, wasted;
+ start = radeon_sa_bo_hole_start(m, m->biggest_hole);
+ wasted = start % align;
+ if (wasted)
+ start += align - wasted;
+
+ end = radeon_sa_bo_hole_end(m, m->biggest_hole);
+ return start < end ? end - start : 0;
+}
+
/*
* Principe is simple, we keep a list of sub allocation in offset
* order (first entry has offset == 0, last entry has the highest
* offset).
*
- * When allocating new object we first check if there is room at
- * the end total_size - (last_object_offset + last_object_size) >=
- * alloc_size. If so we allocate new object there.
+ * When allocating new objects we start checking at what's currently
+ * assumed to be the biggest hole, if that's not big enough we continue
+ * searching the list until we find something big enough or reach the
+ * biggest hole again. If the later happen we optionally block for the
+ * biggest hole to increase in size.
*
- * When there is not enough room at the end, we start waiting for
- * each sub object until we reach object_offset+object_size >=
- * alloc_size, this object then become the sub object we return.
- *
- * Alignment can't be bigger than page size
*/
int radeon_sa_bo_new(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager,
struct radeon_sa_bo *sa_bo,
- unsigned size, unsigned align)
+ unsigned size, unsigned align, bool block)
{
- struct radeon_sa_bo *tmp;
- struct list_head *head;
- unsigned offset = 0, wasted = 0;
- unsigned long flags;
+ struct list_head *head, *curr, *hole;
+ unsigned start, currsize, wasted, holesize;
+ int r;
BUG_ON(align > RADEON_GPU_PAGE_SIZE);
BUG_ON(size > sa_manager->size);
- spin_lock_irqsave(&sa_manager->lock, flags);
- /* no one ? */
- if (list_empty(&sa_manager->sa_bo)) {
- head = &sa_manager->sa_bo;
- goto out;
- }
+ spin_lock_irq(&sa_manager->queue.lock);
+
+ do {
+ curr = head = hole = sa_manager->biggest_hole;
+ holesize = radeon_sa_bo_min_free(sa_manager, 1);
+ do {
+ start = radeon_sa_bo_hole_start(sa_manager, curr);
+ currsize = radeon_sa_bo_hole_end(sa_manager, curr) - start;
+
+ wasted = start % align;
+ if (wasted) {
+ wasted = align - wasted;
+ }
- /* look for a hole big enough */
- list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
- /* room before this object ? */
- if (offset < tmp->offset && (tmp->offset - offset) >= size) {
- head = tmp->list.prev;
- goto out;
+ /* room after current big enough ? */
+ if (currsize >= (size + wasted)) {
+ sa_bo->manager = sa_manager;
+ sa_bo->offset = start + wasted;
+ sa_bo->size = size;
+ list_add(&sa_bo->list, curr);
+
+ /* consider the space left after the newly
+ added sa_bo as the biggest hole */
+ currsize -= (size + wasted);
+ if (hole == sa_bo->list.prev || holesize < currsize) {
+ hole = &sa_bo->list;
+ }
+
+ if (sa_manager->biggest_hole != hole) {
+ sa_manager->biggest_hole = hole;
+ wake_up_locked(&sa_manager->queue);
+ }
+ spin_unlock_irq(&sa_manager->queue.lock);
+ return 0;
+ }
+
+ if (holesize < currsize) {
+ hole = curr;
+ holesize = currsize;
+ }
+
+ curr = curr->next;
+ } while (curr != head);
+
+ if (sa_manager->biggest_hole != hole) {
+ sa_manager->biggest_hole = hole;
}
- offset = tmp->offset + tmp->size;
- wasted = offset % align;
- if (wasted) {
- offset += align - wasted;
+
+ if (block) {
+ /* failed to find something big enough, wait
+ for the biggest hole to increase in size */
+ r = wait_event_interruptible_locked_irq(sa_manager->queue,
+ radeon_sa_bo_min_free(sa_manager, align) >= size
+ );
+ if (r) {
+ spin_unlock_irq(&sa_manager->queue.lock);
+ return r;
+ }
}
- }
- /* room at the end ? */
- head = sa_manager->sa_bo.prev;
- tmp = list_entry(head, struct radeon_sa_bo, list);
- offset = tmp->offset + tmp->size;
- wasted = offset % align;
- if (wasted) {
- offset += wasted = align - wasted;
- }
- if ((sa_manager->size - offset) < size) {
- /* failed to find somethings big enough */
- spin_unlock_irqrestore(&sa_manager->lock, flags);
- return -ENOMEM;
- }
+ } while (block);
+ spin_unlock_irq(&sa_manager->queue.lock);
-out:
- sa_bo->manager = sa_manager;
- sa_bo->offset = offset;
- sa_bo->size = size;
- list_add(&sa_bo->list, head);
- spin_unlock_irqrestore(&sa_manager->lock, flags);
- return 0;
+ return -ENOMEM;
}
void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
{
+ struct radeon_sa_manager *sa_manager = sa_bo->manager;
+ unsigned bsize, fsize;
unsigned long flags;
- spin_lock_irqsave(&sa_bo->manager->lock, flags);
+
+ spin_lock_irqsave(&sa_manager->queue.lock, flags);
+ if (&sa_bo->list == sa_manager->biggest_hole ||
+ sa_bo->list.prev == sa_manager->biggest_hole) {
+
+ sa_manager->biggest_hole = sa_bo->list.prev;
+ wake_up_locked(&sa_manager->queue);
+ } else {
+ bsize = radeon_sa_bo_min_free(sa_manager, 1);
+ fsize = radeon_sa_bo_hole_start(sa_manager, sa_bo->list.prev);
+ fsize = radeon_sa_bo_hole_end(sa_manager, &sa_bo->list) - fsize;
+ if (fsize > bsize) {
+ sa_manager->biggest_hole = sa_bo->list.prev;
+ wake_up_locked(&sa_manager->queue);
+ }
+ }
list_del_init(&sa_bo->list);
- spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
+ spin_unlock_irqrestore(&sa_manager->queue.lock, flags);
}
#if defined(CONFIG_DEBUG_FS)
void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
struct seq_file *m)
{
- struct radeon_sa_bo *i;
+ struct list_head *head, *curr;
+ struct radeon_sa_bo *sa_bo;
+ unsigned start, end;
unsigned long flags;
- spin_lock_irqsave(&sa_manager->lock, flags);
- list_for_each_entry(i, &sa_manager->sa_bo, list) {
- seq_printf(m, "offset %08d: size %4d\n", i->offset, i->size);
- }
- spin_unlock_irqrestore(&sa_manager->lock, flags);
+ spin_lock_irqsave(&sa_manager->queue.lock, flags);
+ curr = head = &sa_manager->sa_bo;
+ do {
+ if (curr != &sa_manager->sa_bo) {
+ sa_bo = list_entry(curr, struct radeon_sa_bo, list);
+ seq_printf(m, "reservation %p %08d: size %7d\n",
+ curr, sa_bo->offset, sa_bo->size);
+ }
+
+ start = radeon_sa_bo_hole_start(sa_manager, curr);
+ end = radeon_sa_bo_hole_end(sa_manager, curr);
+ if (start < end) {
+ seq_printf(m, "hole %p %08d: size %7d\n",
+ curr, start, end - start);
+ }
+ curr = curr->next;
+ } while (curr != head);
+
+ start = radeon_sa_bo_hole_start(sa_manager, sa_manager->biggest_hole);
+ end = radeon_sa_bo_hole_end(sa_manager, sa_manager->biggest_hole);
+ seq_printf(m, "\nbiggest hole %p %08d: size %7d\n",
+ sa_manager->biggest_hole, start, end - start);
+
+ spin_unlock_irqrestore(&sa_manager->queue.lock, flags);
}
#endif
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3
2012-04-30 14:50 ` [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3 Christian König
@ 2012-04-30 15:45 ` Jerome Glisse
2012-05-01 12:22 ` Christian König
0 siblings, 1 reply; 39+ messages in thread
From: Jerome Glisse @ 2012-04-30 15:45 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Mon, Apr 30, 2012 at 10:50 AM, Christian König
<deathsimple@vodafone.de> wrote:
> With that in place clients are automatically blocking
> until their memory request can be handled.
>
> v2: block only if the memory request can't be satisfied
> in the first try, the first version actually lacked
> a night of sleep.
>
> v3: make blocking optional, update comments and fix
> another bug with biggest hole tracking.
>
> Signed-off-by: Christian König <deathsimple@vodafone.de>
I would say NAK, the idea of SA allocator is to be like ring buffer, a
ring allocation. All the allocation made through SA are made for
object that life are ring related, ie once allocated the object is
scheduled through one of the ring and once the ring is done consuming
it the object is free. So the idea behind the SA allocator is to make
it simple we keep track of the highest offset used and the lowest one
free, we try to find room above the highest and if we don't have
enough room we try at the begining of the ring (wrap over), idea is
that in most scenario we don't have to wait because SA is big enough
and if we have to wait well it's because GPU is full of things to do
so waiting a bit won't starve the GPU.
That being said current code doesn't exactly reflect that, i can't
remember why. Anyway i would rather make current looks like intented
as i believe it make allocation easy and fast in usual case. There is
also a reason why i intended to have several sa manager. Idea is to
have one SA manager for the VM (because VM might last longer) and one
for all the ring object (semaphore, ib, ...) because usual case is
that all ring sync from time to time and the all progress in //.
Cheers,
Jerome
> ---
> drivers/gpu/drm/radeon/radeon.h | 5 +-
> drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
> drivers/gpu/drm/radeon/radeon_object.h | 2 +-
> drivers/gpu/drm/radeon/radeon_ring.c | 20 ++--
> drivers/gpu/drm/radeon/radeon_sa.c | 211 +++++++++++++++++++++++---------
> 5 files changed, 165 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
> index 99b188a..e71dc67 100644
> --- a/drivers/gpu/drm/radeon/radeon.h
> +++ b/drivers/gpu/drm/radeon/radeon.h
> @@ -381,17 +381,16 @@ struct radeon_bo_list {
> * alignment).
> */
> struct radeon_sa_manager {
> - spinlock_t lock;
> + wait_queue_head_t queue;
> struct radeon_bo *bo;
> struct list_head sa_bo;
> unsigned size;
> + struct list_head *biggest_hole;
> uint64_t gpu_addr;
> void *cpu_ptr;
> uint32_t domain;
> };
>
> -struct radeon_sa_bo;
> -
> /* sub-allocation buffer */
> struct radeon_sa_bo {
> struct list_head list;
> diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
> index c58a036..7af4ff9 100644
> --- a/drivers/gpu/drm/radeon/radeon_gart.c
> +++ b/drivers/gpu/drm/radeon/radeon_gart.c
> @@ -395,7 +395,7 @@ int radeon_vm_bind(struct radeon_device *rdev, struct radeon_vm *vm)
> retry:
> r = radeon_sa_bo_new(rdev, &rdev->vm_manager.sa_manager, &vm->sa_bo,
> RADEON_GPU_PAGE_ALIGN(vm->last_pfn * 8),
> - RADEON_GPU_PAGE_SIZE);
> + RADEON_GPU_PAGE_SIZE, false);
> if (r) {
> if (list_empty(&rdev->vm_manager.lru_vm)) {
> return r;
> diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
> index d9b9333..85f33d9 100644
> --- a/drivers/gpu/drm/radeon/radeon_object.h
> +++ b/drivers/gpu/drm/radeon/radeon_object.h
> @@ -158,7 +158,7 @@ extern int radeon_sa_bo_manager_suspend(struct radeon_device *rdev,
> extern int radeon_sa_bo_new(struct radeon_device *rdev,
> struct radeon_sa_manager *sa_manager,
> struct radeon_sa_bo *sa_bo,
> - unsigned size, unsigned align);
> + unsigned size, unsigned align, bool block);
> extern void radeon_sa_bo_free(struct radeon_device *rdev,
> struct radeon_sa_bo *sa_bo);
> #if defined(CONFIG_DEBUG_FS)
> diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
> index 1d9bce9..ccee74f 100644
> --- a/drivers/gpu/drm/radeon/radeon_ring.c
> +++ b/drivers/gpu/drm/radeon/radeon_ring.c
> @@ -124,7 +124,7 @@ retry:
> if (rdev->ib_pool.ibs[idx].fence == NULL) {
> r = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager,
> &rdev->ib_pool.ibs[idx].sa_bo,
> - size, 256);
> + size, 256, false);
> if (!r) {
> *ib = &rdev->ib_pool.ibs[idx];
> (*ib)->ptr = rdev->ib_pool.sa_manager.cpu_ptr;
> @@ -205,10 +205,16 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
>
> int radeon_ib_pool_init(struct radeon_device *rdev)
> {
> - struct radeon_sa_manager tmp;
> int i, r;
>
> - r = radeon_sa_bo_manager_init(rdev, &tmp,
> + radeon_mutex_lock(&rdev->ib_pool.mutex);
> + if (rdev->ib_pool.ready) {
> + return 0;
> + }
> + rdev->ib_pool.ready = true;
> + radeon_mutex_unlock(&rdev->ib_pool.mutex);
> +
> + r = radeon_sa_bo_manager_init(rdev, &rdev->ib_pool.sa_manager,
> RADEON_IB_POOL_SIZE*64*1024,
> RADEON_GEM_DOMAIN_GTT);
> if (r) {
> @@ -216,14 +222,6 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
> }
>
> radeon_mutex_lock(&rdev->ib_pool.mutex);
> - if (rdev->ib_pool.ready) {
> - radeon_mutex_unlock(&rdev->ib_pool.mutex);
> - radeon_sa_bo_manager_fini(rdev, &tmp);
> - return 0;
> - }
> -
> - rdev->ib_pool.sa_manager = tmp;
> - INIT_LIST_HEAD(&rdev->ib_pool.sa_manager.sa_bo);
> for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
> rdev->ib_pool.ibs[i].fence = NULL;
> rdev->ib_pool.ibs[i].idx = i;
> diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
> index 013a787..92ab7b4 100644
> --- a/drivers/gpu/drm/radeon/radeon_sa.c
> +++ b/drivers/gpu/drm/radeon/radeon_sa.c
> @@ -26,6 +26,7 @@
> /*
> * Authors:
> * Jerome Glisse <glisse@freedesktop.org>
> + * Christian König <christian.koenig@amd.com>
> */
> #include "drmP.h"
> #include "drm.h"
> @@ -37,9 +38,10 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
> {
> int r;
>
> - spin_lock_init(&sa_manager->lock);
> + init_waitqueue_head(&sa_manager->queue);
> sa_manager->bo = NULL;
> sa_manager->size = size;
> + sa_manager->biggest_hole = &sa_manager->sa_bo;
> sa_manager->domain = domain;
> INIT_LIST_HEAD(&sa_manager->sa_bo);
>
> @@ -58,6 +60,7 @@ void radeon_sa_bo_manager_fini(struct radeon_device *rdev,
> {
> struct radeon_sa_bo *sa_bo, *tmp;
>
> + wake_up_all(&sa_manager->queue);
> if (!list_empty(&sa_manager->sa_bo)) {
> dev_err(rdev->dev, "sa_manager is not empty, clearing anyway\n");
> }
> @@ -114,96 +117,186 @@ int radeon_sa_bo_manager_suspend(struct radeon_device *rdev,
> return r;
> }
>
> +static inline unsigned radeon_sa_bo_hole_start(struct radeon_sa_manager *m,
> + struct list_head *entry)
> +{
> + struct radeon_sa_bo *sa_bo;
> +
> + if (entry == &m->sa_bo)
> + return 0;
> +
> + sa_bo = list_entry(entry, struct radeon_sa_bo, list);
> + return sa_bo->offset + sa_bo->size;
> +}
> +
> +static inline unsigned radeon_sa_bo_hole_end(struct radeon_sa_manager *m,
> + struct list_head *entry)
> +{
> + if (entry->next == &m->sa_bo)
> + return m->size;
> +
> + return list_entry(entry->next, struct radeon_sa_bo, list)->offset;
> +}
> +
> +static inline unsigned radeon_sa_bo_min_free(struct radeon_sa_manager *m,
> + unsigned align)
> +{
> + unsigned start, end, wasted;
> + start = radeon_sa_bo_hole_start(m, m->biggest_hole);
> + wasted = start % align;
> + if (wasted)
> + start += align - wasted;
> +
> + end = radeon_sa_bo_hole_end(m, m->biggest_hole);
> + return start < end ? end - start : 0;
> +}
> +
> /*
> * Principe is simple, we keep a list of sub allocation in offset
> * order (first entry has offset == 0, last entry has the highest
> * offset).
> *
> - * When allocating new object we first check if there is room at
> - * the end total_size - (last_object_offset + last_object_size) >=
> - * alloc_size. If so we allocate new object there.
> + * When allocating new objects we start checking at what's currently
> + * assumed to be the biggest hole, if that's not big enough we continue
> + * searching the list until we find something big enough or reach the
> + * biggest hole again. If the later happen we optionally block for the
> + * biggest hole to increase in size.
> *
> - * When there is not enough room at the end, we start waiting for
> - * each sub object until we reach object_offset+object_size >=
> - * alloc_size, this object then become the sub object we return.
> - *
> - * Alignment can't be bigger than page size
> */
> int radeon_sa_bo_new(struct radeon_device *rdev,
> struct radeon_sa_manager *sa_manager,
> struct radeon_sa_bo *sa_bo,
> - unsigned size, unsigned align)
> + unsigned size, unsigned align, bool block)
> {
> - struct radeon_sa_bo *tmp;
> - struct list_head *head;
> - unsigned offset = 0, wasted = 0;
> - unsigned long flags;
> + struct list_head *head, *curr, *hole;
> + unsigned start, currsize, wasted, holesize;
> + int r;
>
> BUG_ON(align > RADEON_GPU_PAGE_SIZE);
> BUG_ON(size > sa_manager->size);
> - spin_lock_irqsave(&sa_manager->lock, flags);
>
> - /* no one ? */
> - if (list_empty(&sa_manager->sa_bo)) {
> - head = &sa_manager->sa_bo;
> - goto out;
> - }
> + spin_lock_irq(&sa_manager->queue.lock);
> +
> + do {
> + curr = head = hole = sa_manager->biggest_hole;
> + holesize = radeon_sa_bo_min_free(sa_manager, 1);
> + do {
> + start = radeon_sa_bo_hole_start(sa_manager, curr);
> + currsize = radeon_sa_bo_hole_end(sa_manager, curr) - start;
> +
> + wasted = start % align;
> + if (wasted) {
> + wasted = align - wasted;
> + }
>
> - /* look for a hole big enough */
> - list_for_each_entry(tmp, &sa_manager->sa_bo, list) {
> - /* room before this object ? */
> - if (offset < tmp->offset && (tmp->offset - offset) >= size) {
> - head = tmp->list.prev;
> - goto out;
> + /* room after current big enough ? */
> + if (currsize >= (size + wasted)) {
> + sa_bo->manager = sa_manager;
> + sa_bo->offset = start + wasted;
> + sa_bo->size = size;
> + list_add(&sa_bo->list, curr);
> +
> + /* consider the space left after the newly
> + added sa_bo as the biggest hole */
> + currsize -= (size + wasted);
> + if (hole == sa_bo->list.prev || holesize < currsize) {
> + hole = &sa_bo->list;
> + }
> +
> + if (sa_manager->biggest_hole != hole) {
> + sa_manager->biggest_hole = hole;
> + wake_up_locked(&sa_manager->queue);
> + }
> + spin_unlock_irq(&sa_manager->queue.lock);
> + return 0;
> + }
> +
> + if (holesize < currsize) {
> + hole = curr;
> + holesize = currsize;
> + }
> +
> + curr = curr->next;
> + } while (curr != head);
> +
> + if (sa_manager->biggest_hole != hole) {
> + sa_manager->biggest_hole = hole;
> }
> - offset = tmp->offset + tmp->size;
> - wasted = offset % align;
> - if (wasted) {
> - offset += align - wasted;
> +
> + if (block) {
> + /* failed to find something big enough, wait
> + for the biggest hole to increase in size */
> + r = wait_event_interruptible_locked_irq(sa_manager->queue,
> + radeon_sa_bo_min_free(sa_manager, align) >= size
> + );
> + if (r) {
> + spin_unlock_irq(&sa_manager->queue.lock);
> + return r;
> + }
> }
> - }
> - /* room at the end ? */
> - head = sa_manager->sa_bo.prev;
> - tmp = list_entry(head, struct radeon_sa_bo, list);
> - offset = tmp->offset + tmp->size;
> - wasted = offset % align;
> - if (wasted) {
> - offset += wasted = align - wasted;
> - }
> - if ((sa_manager->size - offset) < size) {
> - /* failed to find somethings big enough */
> - spin_unlock_irqrestore(&sa_manager->lock, flags);
> - return -ENOMEM;
> - }
> + } while (block);
> + spin_unlock_irq(&sa_manager->queue.lock);
>
> -out:
> - sa_bo->manager = sa_manager;
> - sa_bo->offset = offset;
> - sa_bo->size = size;
> - list_add(&sa_bo->list, head);
> - spin_unlock_irqrestore(&sa_manager->lock, flags);
> - return 0;
> + return -ENOMEM;
> }
>
> void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo)
> {
> + struct radeon_sa_manager *sa_manager = sa_bo->manager;
> + unsigned bsize, fsize;
> unsigned long flags;
> - spin_lock_irqsave(&sa_bo->manager->lock, flags);
> +
> + spin_lock_irqsave(&sa_manager->queue.lock, flags);
> + if (&sa_bo->list == sa_manager->biggest_hole ||
> + sa_bo->list.prev == sa_manager->biggest_hole) {
> +
> + sa_manager->biggest_hole = sa_bo->list.prev;
> + wake_up_locked(&sa_manager->queue);
> + } else {
> + bsize = radeon_sa_bo_min_free(sa_manager, 1);
> + fsize = radeon_sa_bo_hole_start(sa_manager, sa_bo->list.prev);
> + fsize = radeon_sa_bo_hole_end(sa_manager, &sa_bo->list) - fsize;
> + if (fsize > bsize) {
> + sa_manager->biggest_hole = sa_bo->list.prev;
> + wake_up_locked(&sa_manager->queue);
> + }
> + }
> list_del_init(&sa_bo->list);
> - spin_unlock_irqrestore(&sa_bo->manager->lock, flags);
> + spin_unlock_irqrestore(&sa_manager->queue.lock, flags);
> }
>
> #if defined(CONFIG_DEBUG_FS)
> void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,
> struct seq_file *m)
> {
> - struct radeon_sa_bo *i;
> + struct list_head *head, *curr;
> + struct radeon_sa_bo *sa_bo;
> + unsigned start, end;
> unsigned long flags;
>
> - spin_lock_irqsave(&sa_manager->lock, flags);
> - list_for_each_entry(i, &sa_manager->sa_bo, list) {
> - seq_printf(m, "offset %08d: size %4d\n", i->offset, i->size);
> - }
> - spin_unlock_irqrestore(&sa_manager->lock, flags);
> + spin_lock_irqsave(&sa_manager->queue.lock, flags);
> + curr = head = &sa_manager->sa_bo;
> + do {
> + if (curr != &sa_manager->sa_bo) {
> + sa_bo = list_entry(curr, struct radeon_sa_bo, list);
> + seq_printf(m, "reservation %p %08d: size %7d\n",
> + curr, sa_bo->offset, sa_bo->size);
> + }
> +
> + start = radeon_sa_bo_hole_start(sa_manager, curr);
> + end = radeon_sa_bo_hole_end(sa_manager, curr);
> + if (start < end) {
> + seq_printf(m, "hole %p %08d: size %7d\n",
> + curr, start, end - start);
> + }
> + curr = curr->next;
> + } while (curr != head);
> +
> + start = radeon_sa_bo_hole_start(sa_manager, sa_manager->biggest_hole);
> + end = radeon_sa_bo_hole_end(sa_manager, sa_manager->biggest_hole);
> + seq_printf(m, "\nbiggest hole %p %08d: size %7d\n",
> + sa_manager->biggest_hole, start, end - start);
> +
> + spin_unlock_irqrestore(&sa_manager->queue.lock, flags);
> }
> #endif
> --
> 1.7.5.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3
2012-04-30 15:45 ` Jerome Glisse
@ 2012-05-01 12:22 ` Christian König
2012-05-01 15:24 ` Jerome Glisse
0 siblings, 1 reply; 39+ messages in thread
From: Christian König @ 2012-05-01 12:22 UTC (permalink / raw)
To: Jerome Glisse; +Cc: airlied, dri-devel
On 30.04.2012 17:45, Jerome Glisse wrote:
> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> With that in place clients are automatically blocking
>> until their memory request can be handled.
>>
>> v2: block only if the memory request can't be satisfied
>> in the first try, the first version actually lacked
>> a night of sleep.
>>
>> v3: make blocking optional, update comments and fix
>> another bug with biggest hole tracking.
>>
>> Signed-off-by: Christian König<deathsimple@vodafone.de>
> I would say NAK, the idea of SA allocator is to be like ring buffer, a
> ring allocation. All the allocation made through SA are made for
> object that life are ring related, ie once allocated the object is
> scheduled through one of the ring and once the ring is done consuming
> it the object is free. So the idea behind the SA allocator is to make
> it simple we keep track of the highest offset used and the lowest one
> free, we try to find room above the highest and if we don't have
> enough room we try at the begining of the ring (wrap over), idea is
> that in most scenario we don't have to wait because SA is big enough
> and if we have to wait well it's because GPU is full of things to do
> so waiting a bit won't starve the GPU.
Take a closer look, that's exactly what the new code intends to do.
The biggest_hole is tracking the bottom of the ring allocation algorithm:
In (roughly estimated) 97% of all cases it is pointing after the last
allocation, so new allocations can be directly served.
In 1% of the cases it is pointing to a free block at the end of the
buffer that isn't big enough to serve the next allocation, in that case
we just need to make one step to the beginning of the buffer and
(assuming that allocations are ring like) there should be enough space
gathered to serve the allocation.
In 1% of the cases a left over allocation will prevent a new
biggest_hole to gather together at the beginning of the buffer, but that
is actually unproblematic, cause all that needs to be done is stepping
over that unreleased chunk of memory.
And well every algorithm has a worst case and here it happens when there
isn't enough room to server the allocation, in this case we need to do a
full cycle around all allocations and then optionally wait for the
biggest_hole to increase in size.
> That being said current code doesn't exactly reflect that, i can't
> remember why. Anyway i would rather make current looks like intented
> as i believe it make allocation easy and fast in usual case. There is
> also a reason why i intended to have several sa manager. Idea is to
> have one SA manager for the VM (because VM might last longer) and one
> for all the ring object (semaphore, ib, ...) because usual case is
> that all ring sync from time to time and the all progress in //.
Yeah, even with your good intentions the current code just turned out to
be a linear search over all the allocations and that seemed to be not so
efficient. Also making the code look like it should from the comments
description was also part of my intentions.
Anyway, I think it's definitely an improvement, but there obviously is
also still room for new ideas, e.g. we could track how trustworthy the
biggest_hole is and then early abort allocations that can never succeed.
Or we could also keep track of the last allocation and on new
allocations try once to place them directly behind the last allocation
first, that would give the whole allocator a even more ring like fashion.
And by the way: Having two allocators, one for VM and another for the
ring like stuff sounds like the right thing to do, since their
allocations seems to have different live cycles.
Christian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3
2012-05-01 12:22 ` Christian König
@ 2012-05-01 15:24 ` Jerome Glisse
0 siblings, 0 replies; 39+ messages in thread
From: Jerome Glisse @ 2012-05-01 15:24 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Tue, May 1, 2012 at 8:22 AM, Christian König <deathsimple@vodafone.de> wrote:
> On 30.04.2012 17:45, Jerome Glisse wrote:
>>
>> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>>
>>> With that in place clients are automatically blocking
>>> until their memory request can be handled.
>>>
>>> v2: block only if the memory request can't be satisfied
>>> in the first try, the first version actually lacked
>>> a night of sleep.
>>>
>>> v3: make blocking optional, update comments and fix
>>> another bug with biggest hole tracking.
>>>
>>> Signed-off-by: Christian König<deathsimple@vodafone.de>
>>
>> I would say NAK, the idea of SA allocator is to be like ring buffer, a
>> ring allocation. All the allocation made through SA are made for
>> object that life are ring related, ie once allocated the object is
>> scheduled through one of the ring and once the ring is done consuming
>> it the object is free. So the idea behind the SA allocator is to make
>> it simple we keep track of the highest offset used and the lowest one
>> free, we try to find room above the highest and if we don't have
>> enough room we try at the begining of the ring (wrap over), idea is
>> that in most scenario we don't have to wait because SA is big enough
>> and if we have to wait well it's because GPU is full of things to do
>> so waiting a bit won't starve the GPU.
>
> Take a closer look, that's exactly what the new code intends to do.
>
> The biggest_hole is tracking the bottom of the ring allocation algorithm:
>
> In (roughly estimated) 97% of all cases it is pointing after the last
> allocation, so new allocations can be directly served.
>
> In 1% of the cases it is pointing to a free block at the end of the buffer
> that isn't big enough to serve the next allocation, in that case we just
> need to make one step to the beginning of the buffer and (assuming that
> allocations are ring like) there should be enough space gathered to serve
> the allocation.
>
> In 1% of the cases a left over allocation will prevent a new biggest_hole to
> gather together at the beginning of the buffer, but that is actually
> unproblematic, cause all that needs to be done is stepping over that
> unreleased chunk of memory.
>
> And well every algorithm has a worst case and here it happens when there
> isn't enough room to server the allocation, in this case we need to do a
> full cycle around all allocations and then optionally wait for the
> biggest_hole to increase in size.
>
>
>> That being said current code doesn't exactly reflect that, i can't
>> remember why. Anyway i would rather make current looks like intented
>> as i believe it make allocation easy and fast in usual case. There is
>> also a reason why i intended to have several sa manager. Idea is to
>> have one SA manager for the VM (because VM might last longer) and one
>> for all the ring object (semaphore, ib, ...) because usual case is
>> that all ring sync from time to time and the all progress in //.
>
> Yeah, even with your good intentions the current code just turned out to be
> a linear search over all the allocations and that seemed to be not so
> efficient. Also making the code look like it should from the comments
> description was also part of my intentions.
>
> Anyway, I think it's definitely an improvement, but there obviously is also
> still room for new ideas, e.g. we could track how trustworthy the
> biggest_hole is and then early abort allocations that can never succeed. Or
> we could also keep track of the last allocation and on new allocations try
> once to place them directly behind the last allocation first, that would
> give the whole allocator a even more ring like fashion.
>
> And by the way: Having two allocators, one for VM and another for the ring
> like stuff sounds like the right thing to do, since their allocations seems
> to have different live cycles.
>
> Christian.
This one is still NAK, i have read again, i am gonna post a simpler
version shortly.
Cheers,
Jerome
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 10/26] drm/radeon: add try_free callback to the SA
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (8 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 09/26] drm/radeon: add biggest hole tracking and wakequeue to the sa v3 Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 11/26] drm/radeon: use inline functions to calc sa_bo addr Christian König
` (17 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
To prevent deadlocks under extreme conditions.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 1 +
drivers/gpu/drm/radeon/radeon_gart.c | 2 +-
drivers/gpu/drm/radeon/radeon_object.h | 3 ++-
drivers/gpu/drm/radeon/radeon_ring.c | 2 +-
drivers/gpu/drm/radeon/radeon_sa.c | 15 ++++++++++++---
5 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index e71dc67..4f7e941 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -389,6 +389,7 @@ struct radeon_sa_manager {
uint64_t gpu_addr;
void *cpu_ptr;
uint32_t domain;
+ void (*try_free)(struct radeon_device *);
};
/* sub-allocation buffer */
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 7af4ff9..fba3884 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -291,7 +291,7 @@ int radeon_vm_manager_init(struct radeon_device *rdev)
/* mark first vm as always in use, it's the system one */
r = radeon_sa_bo_manager_init(rdev, &rdev->vm_manager.sa_manager,
rdev->vm_manager.max_pfn * 8,
- RADEON_GEM_DOMAIN_VRAM);
+ RADEON_GEM_DOMAIN_VRAM, NULL);
if (r) {
dev_err(rdev->dev, "failed to allocate vm bo (%dKB)\n",
(rdev->vm_manager.max_pfn * 8) >> 10);
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 85f33d9..08505ed 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -148,7 +148,8 @@ extern struct radeon_bo_va *radeon_bo_va(struct radeon_bo *rbo,
*/
extern int radeon_sa_bo_manager_init(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager,
- unsigned size, u32 domain);
+ unsigned size, u32 domain,
+ void (*try_free)(struct radeon_device *rdev));
extern void radeon_sa_bo_manager_fini(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager);
extern int radeon_sa_bo_manager_start(struct radeon_device *rdev,
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index ccee74f..ae28129 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -216,7 +216,7 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
r = radeon_sa_bo_manager_init(rdev, &rdev->ib_pool.sa_manager,
RADEON_IB_POOL_SIZE*64*1024,
- RADEON_GEM_DOMAIN_GTT);
+ RADEON_GEM_DOMAIN_GTT, NULL);
if (r) {
return r;
}
diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
index 92ab7b4..9d3e70a 100644
--- a/drivers/gpu/drm/radeon/radeon_sa.c
+++ b/drivers/gpu/drm/radeon/radeon_sa.c
@@ -34,7 +34,8 @@
int radeon_sa_bo_manager_init(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager,
- unsigned size, u32 domain)
+ unsigned size, u32 domain,
+ void (*try_free)(struct radeon_device *rdev))
{
int r;
@@ -43,6 +44,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev,
sa_manager->size = size;
sa_manager->biggest_hole = &sa_manager->sa_bo;
sa_manager->domain = domain;
+ sa_manager->try_free = try_free;
INIT_LIST_HEAD(&sa_manager->sa_bo);
r = radeon_bo_create(rdev, size, RADEON_GPU_PAGE_SIZE, true,
@@ -224,8 +226,15 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
}
if (block) {
- /* failed to find something big enough, wait
- for the biggest hole to increase in size */
+ /* failed to find something big enough */
+ if (sa_manager->try_free) {
+ /* try to free something */
+ spin_unlock_irq(&sa_manager->queue.lock);
+ sa_manager->try_free(rdev);
+ spin_lock_irq(&sa_manager->queue.lock);
+ }
+
+ /* and wait for the biggest hole to increase in size */
r = wait_event_interruptible_locked_irq(sa_manager->queue,
radeon_sa_bo_min_free(sa_manager, align) >= size
);
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 11/26] drm/radeon: use inline functions to calc sa_bo addr
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (9 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 10/26] drm/radeon: add try_free callback to the SA Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 12/26] drm/radeon: simplify semaphore handling Christian König
` (16 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Instead of hacking the calculation multiple times.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon_gart.c | 6 ++----
drivers/gpu/drm/radeon/radeon_object.h | 11 +++++++++++
drivers/gpu/drm/radeon/radeon_ring.c | 6 ++----
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index fba3884..1766671 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -404,10 +404,8 @@ retry:
radeon_vm_unbind(rdev, vm_evict);
goto retry;
}
- vm->pt = rdev->vm_manager.sa_manager.cpu_ptr;
- vm->pt += (vm->sa_bo.offset >> 3);
- vm->pt_gpu_addr = rdev->vm_manager.sa_manager.gpu_addr;
- vm->pt_gpu_addr += vm->sa_bo.offset;
+ vm->pt = radeon_sa_bo_cpu_addr(&vm->sa_bo);
+ vm->pt_gpu_addr = radeon_sa_bo_gpu_addr(&vm->sa_bo);
memset(vm->pt, 0, RADEON_GPU_PAGE_ALIGN(vm->last_pfn * 8));
retry_id:
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 08505ed..242c76f 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -146,6 +146,17 @@ extern struct radeon_bo_va *radeon_bo_va(struct radeon_bo *rbo,
/*
* sub allocation
*/
+
+static inline uint64_t radeon_sa_bo_gpu_addr(struct radeon_sa_bo *sa_bo)
+{
+ return sa_bo->manager->gpu_addr + sa_bo->offset;
+}
+
+static inline void * radeon_sa_bo_cpu_addr(struct radeon_sa_bo *sa_bo)
+{
+ return sa_bo->manager->cpu_ptr + sa_bo->offset;
+}
+
extern int radeon_sa_bo_manager_init(struct radeon_device *rdev,
struct radeon_sa_manager *sa_manager,
unsigned size, u32 domain,
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index ae28129..56cfb86 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -127,10 +127,8 @@ retry:
size, 256, false);
if (!r) {
*ib = &rdev->ib_pool.ibs[idx];
- (*ib)->ptr = rdev->ib_pool.sa_manager.cpu_ptr;
- (*ib)->ptr += ((*ib)->sa_bo.offset >> 2);
- (*ib)->gpu_addr = rdev->ib_pool.sa_manager.gpu_addr;
- (*ib)->gpu_addr += (*ib)->sa_bo.offset;
+ (*ib)->ptr = radeon_sa_bo_cpu_addr(&(*ib)->sa_bo);
+ (*ib)->gpu_addr = radeon_sa_bo_gpu_addr(&(*ib)->sa_bo);
(*ib)->fence = fence;
(*ib)->vm_id = 0;
(*ib)->is_const_ib = false;
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 12/26] drm/radeon: simplify semaphore handling
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (10 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 11/26] drm/radeon: use inline functions to calc sa_bo addr Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 13/26] drm/radeon: return -ENOENT in fence_wait_next v2 Christian König
` (15 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Directly use the suballocator to get small chunks
of memory. It's equally fast and doesn't crash when
we encounter a GPU reset.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/evergreen.c | 1 -
drivers/gpu/drm/radeon/ni.c | 1 -
drivers/gpu/drm/radeon/r600.c | 1 -
drivers/gpu/drm/radeon/radeon.h | 29 +------
drivers/gpu/drm/radeon/radeon_device.c | 2 -
drivers/gpu/drm/radeon/radeon_semaphore.c | 134 ++++-------------------------
drivers/gpu/drm/radeon/rv770.c | 1 -
drivers/gpu/drm/radeon/si.c | 1 -
8 files changed, 22 insertions(+), 148 deletions(-)
diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c
index 8b7a01b..26848d6 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -3559,7 +3559,6 @@ void evergreen_fini(struct radeon_device *rdev)
evergreen_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
- radeon_semaphore_driver_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_agp_fini(rdev);
radeon_bo_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 0146428..c0b0956 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -1773,7 +1773,6 @@ void cayman_fini(struct radeon_device *rdev)
cayman_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
- radeon_semaphore_driver_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
radeon_atombios_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index db9415a..659a09c 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2672,7 +2672,6 @@ void r600_fini(struct radeon_device *rdev)
r600_vram_scratch_fini(rdev);
radeon_agp_fini(rdev);
radeon_gem_fini(rdev);
- radeon_semaphore_driver_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
radeon_atombios_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 4f7e941..e81ad96 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -428,34 +428,12 @@ int radeon_mode_dumb_destroy(struct drm_file *file_priv,
/*
* Semaphores.
*/
-struct radeon_ring;
-
-#define RADEON_SEMAPHORE_BO_SIZE 256
-
-struct radeon_semaphore_driver {
- rwlock_t lock;
- struct list_head bo;
-};
-
-struct radeon_semaphore_bo;
-
-/* everything here is constant */
struct radeon_semaphore {
- struct list_head list;
- uint64_t gpu_addr;
- uint32_t *cpu_ptr;
- struct radeon_semaphore_bo *bo;
-};
-
-struct radeon_semaphore_bo {
- struct list_head list;
- struct radeon_ib *ib;
- struct list_head free;
- struct radeon_semaphore semaphores[RADEON_SEMAPHORE_BO_SIZE/8];
- unsigned nused;
+ struct radeon_sa_bo sa_bo;
+ signed waiters;
+ uint64_t gpu_addr;
};
-void radeon_semaphore_driver_fini(struct radeon_device *rdev);
int radeon_semaphore_create(struct radeon_device *rdev,
struct radeon_semaphore **semaphore);
void radeon_semaphore_emit_signal(struct radeon_device *rdev, int ring,
@@ -1528,7 +1506,6 @@ struct radeon_device {
struct radeon_mman mman;
rwlock_t fence_lock;
struct radeon_fence_driver fence_drv[RADEON_NUM_RINGS];
- struct radeon_semaphore_driver semaphore_drv;
struct radeon_ring ring[RADEON_NUM_RINGS];
struct radeon_ib_pool ib_pool;
struct radeon_irq irq;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index dedb398..8c49990 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -733,11 +733,9 @@ int radeon_device_init(struct radeon_device *rdev,
mutex_init(&rdev->pm.mutex);
mutex_init(&rdev->vram_mutex);
rwlock_init(&rdev->fence_lock);
- rwlock_init(&rdev->semaphore_drv.lock);
INIT_LIST_HEAD(&rdev->gem.objects);
init_waitqueue_head(&rdev->irq.vblank_queue);
init_waitqueue_head(&rdev->irq.idle_queue);
- INIT_LIST_HEAD(&rdev->semaphore_drv.bo);
/* initialize vm here */
rdev->vm_manager.use_bitmap = 1;
rdev->vm_manager.max_pfn = 1 << 20;
diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c
index 61dd4e3..5a4741a 100644
--- a/drivers/gpu/drm/radeon/radeon_semaphore.c
+++ b/drivers/gpu/drm/radeon/radeon_semaphore.c
@@ -31,108 +31,27 @@
#include "drm.h"
#include "radeon.h"
-static int radeon_semaphore_add_bo(struct radeon_device *rdev)
+int radeon_semaphore_create(struct radeon_device *rdev,
+ struct radeon_semaphore **semaphore)
{
- struct radeon_semaphore_bo *bo;
- unsigned long irq_flags;
- uint64_t gpu_addr;
- uint32_t *cpu_ptr;
- int r, i;
-
+ int r;
- bo = kmalloc(sizeof(struct radeon_semaphore_bo), GFP_KERNEL);
- if (bo == NULL) {
+ *semaphore = kmalloc(sizeof(struct radeon_semaphore), GFP_KERNEL);
+ if (*semaphore == NULL) {
return -ENOMEM;
}
- INIT_LIST_HEAD(&bo->free);
- INIT_LIST_HEAD(&bo->list);
- bo->nused = 0;
- r = radeon_ib_get(rdev, 0, &bo->ib, RADEON_SEMAPHORE_BO_SIZE);
+ r = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager,
+ &(*semaphore)->sa_bo, 8, 8, true);
if (r) {
- dev_err(rdev->dev, "failed to get a bo after 5 retry\n");
- kfree(bo);
+ kfree(*semaphore);
+ *semaphore = NULL;
return r;
}
- gpu_addr = rdev->ib_pool.sa_manager.gpu_addr;
- gpu_addr += bo->ib->sa_bo.offset;
- cpu_ptr = rdev->ib_pool.sa_manager.cpu_ptr;
- cpu_ptr += (bo->ib->sa_bo.offset >> 2);
- for (i = 0; i < (RADEON_SEMAPHORE_BO_SIZE/8); i++) {
- bo->semaphores[i].gpu_addr = gpu_addr;
- bo->semaphores[i].cpu_ptr = cpu_ptr;
- bo->semaphores[i].bo = bo;
- list_add_tail(&bo->semaphores[i].list, &bo->free);
- gpu_addr += 8;
- cpu_ptr += 2;
- }
- write_lock_irqsave(&rdev->semaphore_drv.lock, irq_flags);
- list_add_tail(&bo->list, &rdev->semaphore_drv.bo);
- write_unlock_irqrestore(&rdev->semaphore_drv.lock, irq_flags);
- return 0;
-}
-static void radeon_semaphore_del_bo_locked(struct radeon_device *rdev,
- struct radeon_semaphore_bo *bo)
-{
- radeon_sa_bo_free(rdev, &bo->ib->sa_bo);
- radeon_fence_unref(&bo->ib->fence);
- list_del(&bo->list);
- kfree(bo);
-}
-
-void radeon_semaphore_shrink_locked(struct radeon_device *rdev)
-{
- struct radeon_semaphore_bo *bo, *n;
-
- if (list_empty(&rdev->semaphore_drv.bo)) {
- return;
- }
- /* only shrink if first bo has free semaphore */
- bo = list_first_entry(&rdev->semaphore_drv.bo, struct radeon_semaphore_bo, list);
- if (list_empty(&bo->free)) {
- return;
- }
- list_for_each_entry_safe_continue(bo, n, &rdev->semaphore_drv.bo, list) {
- if (bo->nused)
- continue;
- radeon_semaphore_del_bo_locked(rdev, bo);
- }
-}
-
-int radeon_semaphore_create(struct radeon_device *rdev,
- struct radeon_semaphore **semaphore)
-{
- struct radeon_semaphore_bo *bo;
- unsigned long irq_flags;
- bool do_retry = true;
- int r;
-
-retry:
- *semaphore = NULL;
- write_lock_irqsave(&rdev->semaphore_drv.lock, irq_flags);
- list_for_each_entry(bo, &rdev->semaphore_drv.bo, list) {
- if (list_empty(&bo->free))
- continue;
- *semaphore = list_first_entry(&bo->free, struct radeon_semaphore, list);
- (*semaphore)->cpu_ptr[0] = 0;
- (*semaphore)->cpu_ptr[1] = 0;
- list_del(&(*semaphore)->list);
- bo->nused++;
- break;
- }
- write_unlock_irqrestore(&rdev->semaphore_drv.lock, irq_flags);
-
- if (*semaphore == NULL) {
- if (do_retry) {
- do_retry = false;
- r = radeon_semaphore_add_bo(rdev);
- if (r)
- return r;
- goto retry;
- }
- return -ENOMEM;
- }
+ (*semaphore)->waiters = 0;
+ (*semaphore)->gpu_addr = radeon_sa_bo_gpu_addr(&(*semaphore)->sa_bo);
+ *((uint64_t*)radeon_sa_bo_cpu_addr(&(*semaphore)->sa_bo)) = 0;
return 0;
}
@@ -140,39 +59,24 @@ retry:
void radeon_semaphore_emit_signal(struct radeon_device *rdev, int ring,
struct radeon_semaphore *semaphore)
{
+ --semaphore->waiters;
radeon_semaphore_ring_emit(rdev, ring, &rdev->ring[ring], semaphore, false);
}
void radeon_semaphore_emit_wait(struct radeon_device *rdev, int ring,
struct radeon_semaphore *semaphore)
{
+ ++semaphore->waiters;
radeon_semaphore_ring_emit(rdev, ring, &rdev->ring[ring], semaphore, true);
}
void radeon_semaphore_free(struct radeon_device *rdev,
struct radeon_semaphore *semaphore)
{
- unsigned long irq_flags;
-
- write_lock_irqsave(&rdev->semaphore_drv.lock, irq_flags);
- semaphore->bo->nused--;
- list_add_tail(&semaphore->list, &semaphore->bo->free);
- radeon_semaphore_shrink_locked(rdev);
- write_unlock_irqrestore(&rdev->semaphore_drv.lock, irq_flags);
-}
-
-void radeon_semaphore_driver_fini(struct radeon_device *rdev)
-{
- struct radeon_semaphore_bo *bo, *n;
- unsigned long irq_flags;
-
- write_lock_irqsave(&rdev->semaphore_drv.lock, irq_flags);
- /* we force to free everything */
- list_for_each_entry_safe(bo, n, &rdev->semaphore_drv.bo, list) {
- if (!list_empty(&bo->free)) {
- dev_err(rdev->dev, "still in use semaphore\n");
- }
- radeon_semaphore_del_bo_locked(rdev, bo);
+ if (semaphore->waiters > 0) {
+ dev_err(rdev->dev, "Semaphore %p has more waiters than signalers,"
+ " hardware lockup imminent!\n", semaphore);
}
- write_unlock_irqrestore(&rdev->semaphore_drv.lock, irq_flags);
+ radeon_sa_bo_free(rdev, &semaphore->sa_bo);
+ kfree(semaphore);
}
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index cacec0e..c6ee54e 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -1278,7 +1278,6 @@ void rv770_fini(struct radeon_device *rdev)
rv770_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
- radeon_semaphore_driver_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_agp_fini(rdev);
radeon_bo_fini(rdev);
diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
index 14919e1..7c0110d 100644
--- a/drivers/gpu/drm/radeon/si.c
+++ b/drivers/gpu/drm/radeon/si.c
@@ -4120,7 +4120,6 @@ void si_fini(struct radeon_device *rdev)
si_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
- radeon_semaphore_driver_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
radeon_atombios_fini(rdev);
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 13/26] drm/radeon: return -ENOENT in fence_wait_next v2
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (11 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 12/26] drm/radeon: simplify semaphore handling Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 14/26] drm/radeon: rename fence_wait_last to fence_wait_empty Christian König
` (14 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 14/26] drm/radeon: rename fence_wait_last to fence_wait_empty
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (12 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 13/26] drm/radeon: return -ENOENT in fence_wait_next v2 Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 15/26] drm/radeon: add general purpose fence signaled callback Christian König
` (13 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 e81ad96..ce22b01 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 8c49990..5966b35 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -913,7 +913,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 15/26] drm/radeon: add general purpose fence signaled callback
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (13 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 14/26] drm/radeon: rename fence_wait_last to fence_wait_empty Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 16/26] drm/radeon: don't keep list of created fences Christian König
` (12 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Should be used to free resource that are protected by a fence.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 8 ++++-
drivers/gpu/drm/radeon/radeon_fence.c | 50 +++++++++++++++++++++++++++++---
2 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index ce22b01..74b6595 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -259,7 +259,6 @@ struct radeon_fence_driver {
wait_queue_head_t queue;
struct list_head created;
struct list_head emitted;
- struct list_head signaled;
bool initialized;
};
@@ -274,6 +273,10 @@ struct radeon_fence {
/* RB, DMA, etc. */
int ring;
struct radeon_semaphore *semaphore;
+
+ /* called when fence is signaled */
+ void (*signal_callback)(struct radeon_device *rdev, void *data);
+ void *callback_data;
};
int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring);
@@ -289,6 +292,9 @@ 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);
+bool radeon_fence_set_signal_callback(struct radeon_fence *fence,
+ void (*callback)(struct radeon_device *, void *),
+ void *data);
/*
* Tiling registers
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index 2d13843..c58660a 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -83,7 +83,8 @@ int radeon_fence_emit(struct radeon_device *rdev, struct radeon_fence *fence)
return 0;
}
-static bool radeon_fence_poll_locked(struct radeon_device *rdev, int ring)
+static bool radeon_fence_poll_locked(struct radeon_device *rdev, int ring,
+ struct list_head *signaled)
{
struct radeon_fence *fence;
struct list_head *i, *n;
@@ -110,7 +111,7 @@ static bool radeon_fence_poll_locked(struct radeon_device *rdev, int ring)
i = n;
do {
n = i->prev;
- list_move_tail(i, &rdev->fence_drv[ring].signaled);
+ list_move_tail(i, signaled);
fence = list_entry(i, struct radeon_fence, list);
fence->signaled = true;
i = n;
@@ -120,6 +121,20 @@ static bool radeon_fence_poll_locked(struct radeon_device *rdev, int ring)
return wake;
}
+static void radeon_fence_process_signaled(struct radeon_device *rdev, struct list_head *signaled)
+{
+ struct radeon_fence *fence;
+ struct list_head *i, *n;
+
+ list_for_each_safe(i, n, signaled) {
+ fence = list_entry(i, struct radeon_fence, list);
+ list_del_init(&fence->list);
+ if (fence->signal_callback) {
+ fence->signal_callback(rdev, fence->callback_data);
+ }
+ }
+}
+
static void radeon_fence_destroy(struct kref *kref)
{
unsigned long irq_flags;
@@ -152,6 +167,8 @@ int radeon_fence_create(struct radeon_device *rdev,
(*fence)->seq = 0;
(*fence)->ring = ring;
(*fence)->semaphore = NULL;
+ (*fence)->signal_callback = NULL;
+ (*fence)->callback_data = NULL;
INIT_LIST_HEAD(&(*fence)->list);
write_lock_irqsave(&rdev->fence_lock, irq_flags);
@@ -164,6 +181,7 @@ bool radeon_fence_signaled(struct radeon_fence *fence)
{
unsigned long irq_flags;
bool signaled = false;
+ LIST_HEAD(siglist);
if (!fence)
return true;
@@ -179,10 +197,12 @@ bool radeon_fence_signaled(struct radeon_fence *fence)
signaled = true;
}
if (!signaled) {
- radeon_fence_poll_locked(fence->rdev, fence->ring);
+ radeon_fence_poll_locked(fence->rdev, fence->ring, &siglist);
signaled = fence->signaled;
}
write_unlock_irqrestore(&fence->rdev->fence_lock, irq_flags);
+ radeon_fence_process_signaled(fence->rdev, &siglist);
+
return signaled;
}
@@ -341,10 +361,12 @@ void radeon_fence_process(struct radeon_device *rdev, int ring)
{
unsigned long irq_flags;
bool wake;
+ LIST_HEAD(signaled);
write_lock_irqsave(&rdev->fence_lock, irq_flags);
- wake = radeon_fence_poll_locked(rdev, ring);
+ wake = radeon_fence_poll_locked(rdev, ring, &signaled);
write_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+ radeon_fence_process_signaled(rdev, &signaled);
if (wake) {
wake_up_all(&rdev->fence_drv[ring].queue);
}
@@ -373,6 +395,25 @@ int radeon_fence_count_emitted(struct radeon_device *rdev, int ring)
return not_processed;
}
+bool radeon_fence_set_signal_callback(struct radeon_fence *fence,
+ void (*callback)(struct radeon_device *, void *),
+ void *data)
+{
+ struct radeon_device *rdev = fence->rdev;
+ unsigned long irq_flags;
+ bool isset = false;
+
+ /* a readlock is suficient, cause this should be called only once */
+ read_lock_irqsave(&rdev->fence_lock, irq_flags);
+ if (fence->emitted && !fence->signaled) {
+ fence->signal_callback = callback;
+ fence->callback_data = data;
+ isset = true;
+ }
+ read_unlock_irqrestore(&rdev->fence_lock, irq_flags);
+ return isset;
+}
+
int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring)
{
unsigned long irq_flags;
@@ -413,7 +454,6 @@ static void radeon_fence_driver_init_ring(struct radeon_device *rdev, int ring)
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);
rdev->fence_drv[ring].initialized = false;
}
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 16/26] drm/radeon: don't keep list of created fences.
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (14 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 15/26] drm/radeon: add general purpose fence signaled callback Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 17/26] drm/radeon: rip out the ib pool v2 Christian König
` (11 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
It's never used and so practically superfluous.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 74b6595..9ba28a4 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;
bool initialized;
};
diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index c58660a..326bdb5 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -154,8 +154,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;
@@ -170,10 +168,6 @@ int radeon_fence_create(struct radeon_device *rdev,
(*fence)->signal_callback = NULL;
(*fence)->callback_data = 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;
}
@@ -452,7 +446,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_waitqueue_head(&rdev->fence_drv[ring].queue);
rdev->fence_drv[ring].initialized = false;
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 17/26] drm/radeon: rip out the ib pool v2
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (15 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 16/26] drm/radeon: don't keep list of created fences Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 18/26] drm/radeon: fix a bug with the ring syncing code Christian König
` (10 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
It isn't necessary any more and the suballocator
seems to perform even better.
v2: ignore ERESTARTSYS in error reporting,
split fence changes into seperate patch,
use try_free SA callback to avoid lockups
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 17 +--
drivers/gpu/drm/radeon/radeon_device.c | 1 -
drivers/gpu/drm/radeon/radeon_gart.c | 12 +-
drivers/gpu/drm/radeon/radeon_ring.c | 264 ++++++++++-------------------
drivers/gpu/drm/radeon/radeon_semaphore.c | 2 +-
5 files changed, 101 insertions(+), 195 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 9ba28a4..12d2003 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -618,7 +618,6 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc);
struct radeon_ib {
struct radeon_sa_bo sa_bo;
- unsigned idx;
uint32_t length_dw;
uint64_t gpu_addr;
uint32_t *ptr;
@@ -627,18 +626,6 @@ struct radeon_ib {
bool is_const_ib;
};
-/*
- * locking -
- * mutex protects scheduled_ibs, ready, alloc_bm
- */
-struct radeon_ib_pool {
- struct radeon_mutex mutex;
- struct radeon_sa_manager sa_manager;
- struct radeon_ib ibs[RADEON_IB_POOL_SIZE];
- bool ready;
- unsigned head_id;
-};
-
struct radeon_ring {
struct radeon_bo *ring_obj;
volatile uint32_t *ring;
@@ -779,7 +766,6 @@ struct si_rlc {
int radeon_ib_get(struct radeon_device *rdev, int ring,
struct radeon_ib **ib, unsigned size);
void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib **ib);
-bool radeon_ib_try_free(struct radeon_device *rdev, struct radeon_ib *ib);
int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib);
int radeon_ib_pool_init(struct radeon_device *rdev);
void radeon_ib_pool_fini(struct radeon_device *rdev);
@@ -1512,7 +1498,8 @@ struct radeon_device {
rwlock_t fence_lock;
struct radeon_fence_driver fence_drv[RADEON_NUM_RINGS];
struct radeon_ring ring[RADEON_NUM_RINGS];
- struct radeon_ib_pool ib_pool;
+ bool ib_pool_ready;
+ struct radeon_sa_manager sa_manager;
struct radeon_irq irq;
struct radeon_asic *asic;
struct radeon_gem gem;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 5966b35..853e1cb 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -723,7 +723,6 @@ int radeon_device_init(struct radeon_device *rdev,
/* mutex initialization are all done here so we
* can recall function without having locking issues */
radeon_mutex_init(&rdev->cs_mutex);
- radeon_mutex_init(&rdev->ib_pool.mutex);
for (i = 0; i < RADEON_NUM_RINGS; ++i)
mutex_init(&rdev->ring[i].mutex);
mutex_init(&rdev->dc_hw_i2c_mutex);
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 1766671..8eba1c6 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -432,8 +432,8 @@ retry_id:
rdev->vm_manager.use_bitmap |= 1 << id;
vm->id = id;
list_add_tail(&vm->list, &rdev->vm_manager.lru_vm);
- return radeon_vm_bo_update_pte(rdev, vm, rdev->ib_pool.sa_manager.bo,
- &rdev->ib_pool.sa_manager.bo->tbo.mem);
+ return radeon_vm_bo_update_pte(rdev, vm, rdev->sa_manager.bo,
+ &rdev->sa_manager.bo->tbo.mem);
}
/* object have to be reserved */
@@ -631,7 +631,7 @@ int radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm)
/* map the ib pool buffer at 0 in virtual address space, set
* read only
*/
- r = radeon_vm_bo_add(rdev, vm, rdev->ib_pool.sa_manager.bo, 0,
+ r = radeon_vm_bo_add(rdev, vm, rdev->sa_manager.bo, 0,
RADEON_VM_PAGE_READABLE | RADEON_VM_PAGE_SNOOPED);
return r;
}
@@ -648,12 +648,12 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
radeon_mutex_unlock(&rdev->cs_mutex);
/* remove all bo */
- r = radeon_bo_reserve(rdev->ib_pool.sa_manager.bo, false);
+ r = radeon_bo_reserve(rdev->sa_manager.bo, false);
if (!r) {
- bo_va = radeon_bo_va(rdev->ib_pool.sa_manager.bo, vm);
+ bo_va = radeon_bo_va(rdev->sa_manager.bo, vm);
list_del_init(&bo_va->bo_list);
list_del_init(&bo_va->vm_list);
- radeon_bo_unreserve(rdev->ib_pool.sa_manager.bo);
+ radeon_bo_unreserve(rdev->sa_manager.bo);
kfree(bo_va);
}
if (!list_empty(&vm->va)) {
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 56cfb86..a56e44e 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -24,6 +24,7 @@
* Authors: Dave Airlie
* Alex Deucher
* Jerome Glisse
+ * Christian König
*/
#include <linux/seq_file.h>
#include <linux/slab.h>
@@ -33,8 +34,10 @@
#include "radeon.h"
#include "atom.h"
-int radeon_debugfs_ib_init(struct radeon_device *rdev);
-int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring);
+/*
+ * IB.
+ */
+int radeon_debugfs_sa_init(struct radeon_device *rdev);
u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx)
{
@@ -61,121 +64,71 @@ u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx)
return idx_value;
}
-void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
-{
-#if DRM_DEBUG_CODE
- if (ring->count_dw <= 0) {
- DRM_ERROR("radeon: writting more dword to ring than expected !\n");
- }
-#endif
- ring->ring[ring->wptr++] = v;
- ring->wptr &= ring->ptr_mask;
- ring->count_dw--;
- ring->ring_free_dw--;
-}
-
-/*
- * IB.
- */
-bool radeon_ib_try_free(struct radeon_device *rdev, struct radeon_ib *ib)
-{
- bool done = false;
-
- /* only free ib which have been emited */
- if (ib->fence && ib->fence->emitted) {
- if (radeon_fence_signaled(ib->fence)) {
- radeon_fence_unref(&ib->fence);
- radeon_sa_bo_free(rdev, &ib->sa_bo);
- done = true;
- }
- }
- return done;
-}
-
int radeon_ib_get(struct radeon_device *rdev, int ring,
struct radeon_ib **ib, unsigned size)
{
- struct radeon_fence *fence;
- unsigned cretry = 0;
- int r = 0, i, idx;
+ int r;
- *ib = NULL;
- /* align size on 256 bytes */
- size = ALIGN(size, 256);
+ *ib = kmalloc(sizeof(struct radeon_ib), GFP_KERNEL);
+ if (*ib == NULL) {
+ return -ENOMEM;
+ }
- r = radeon_fence_create(rdev, &fence, ring);
+ r = radeon_sa_bo_new(rdev, &rdev->sa_manager, &(*ib)->sa_bo, size, 256, true);
if (r) {
- dev_err(rdev->dev, "failed to create fence for new IB\n");
+ if (r != -ERESTARTSYS) {
+ dev_err(rdev->dev, "failed to get a new IB (%d)\n", r);
+ }
+ kfree(*ib);
+ *ib = NULL;
return r;
}
- radeon_mutex_lock(&rdev->ib_pool.mutex);
- idx = rdev->ib_pool.head_id;
-retry:
- if (cretry > 5) {
- dev_err(rdev->dev, "failed to get an ib after 5 retry\n");
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
- radeon_fence_unref(&fence);
- return -ENOMEM;
- }
- cretry++;
- for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
- radeon_ib_try_free(rdev, &rdev->ib_pool.ibs[idx]);
- if (rdev->ib_pool.ibs[idx].fence == NULL) {
- r = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager,
- &rdev->ib_pool.ibs[idx].sa_bo,
- size, 256, false);
- if (!r) {
- *ib = &rdev->ib_pool.ibs[idx];
- (*ib)->ptr = radeon_sa_bo_cpu_addr(&(*ib)->sa_bo);
- (*ib)->gpu_addr = radeon_sa_bo_gpu_addr(&(*ib)->sa_bo);
- (*ib)->fence = fence;
- (*ib)->vm_id = 0;
- (*ib)->is_const_ib = false;
- /* ib are most likely to be allocated in a ring fashion
- * thus rdev->ib_pool.head_id should be the id of the
- * oldest ib
- */
- rdev->ib_pool.head_id = (1 + idx);
- rdev->ib_pool.head_id &= (RADEON_IB_POOL_SIZE - 1);
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
- return 0;
- }
- }
- idx = (idx + 1) & (RADEON_IB_POOL_SIZE - 1);
- }
- /* this should be rare event, ie all ib scheduled none signaled yet.
- */
- for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
- if (rdev->ib_pool.ibs[idx].fence && rdev->ib_pool.ibs[idx].fence->emitted) {
- r = radeon_fence_wait(rdev->ib_pool.ibs[idx].fence, false);
- if (!r) {
- goto retry;
- }
- /* an error happened */
- break;
- }
- idx = (idx + 1) & (RADEON_IB_POOL_SIZE - 1);
+ r = radeon_fence_create(rdev, &(*ib)->fence, ring);
+ if (r) {
+ dev_err(rdev->dev, "failed to create fence for new IB (%d)\n", r);
+ radeon_sa_bo_free(rdev, &(*ib)->sa_bo);
+ kfree(*ib);
+ *ib = NULL;
+ return r;
}
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
- radeon_fence_unref(&fence);
- return r;
+
+ (*ib)->ptr = rdev->sa_manager.cpu_ptr;
+ (*ib)->ptr += ((*ib)->sa_bo.offset >> 2);
+ (*ib)->gpu_addr = rdev->sa_manager.gpu_addr;
+ (*ib)->gpu_addr += (*ib)->sa_bo.offset;
+ (*ib)->vm_id = 0;
+ (*ib)->is_const_ib = false;
+
+ return 0;
+}
+
+static void radeon_ib_destroy(struct radeon_device *rdev, void *data)
+{
+ struct radeon_ib *ib = data;
+ radeon_sa_bo_free(rdev, &ib->sa_bo);
+ radeon_fence_unref(&ib->fence);
+ kfree(ib);
}
void radeon_ib_free(struct radeon_device *rdev, struct radeon_ib **ib)
{
struct radeon_ib *tmp = *ib;
+ bool destroy = true;
*ib = NULL;
if (tmp == NULL) {
return;
}
- radeon_mutex_lock(&rdev->ib_pool.mutex);
- if (tmp->fence && !tmp->fence->emitted) {
- radeon_sa_bo_free(rdev, &tmp->sa_bo);
- radeon_fence_unref(&tmp->fence);
+
+ if (tmp->fence) {
+ destroy = !radeon_fence_set_signal_callback(
+ tmp->fence, radeon_ib_destroy, tmp);
+ }
+
+ if (destroy) {
+ radeon_ib_destroy(rdev, tmp);
}
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
}
int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
@@ -185,7 +138,7 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
if (!ib->length_dw || !ring->ready) {
/* TODO: Nothings in the ib we should report. */
- DRM_ERROR("radeon: couldn't schedule IB(%u).\n", ib->idx);
+ DRM_ERROR("radeon: couldn't schedule IB.\n");
return -EINVAL;
}
@@ -201,66 +154,58 @@ int radeon_ib_schedule(struct radeon_device *rdev, struct radeon_ib *ib)
return 0;
}
+static void radeon_ib_try_free(struct radeon_device *rdev)
+{
+ int ring;
+
+ for (ring = 0; ring < RADEON_NUM_RINGS; ring++) {
+ if (rdev->fence_drv[ring].initialized)
+ radeon_fence_process(rdev, ring);
+ }
+}
+
int radeon_ib_pool_init(struct radeon_device *rdev)
{
- int i, r;
+ int r;
- radeon_mutex_lock(&rdev->ib_pool.mutex);
- if (rdev->ib_pool.ready) {
+ if (rdev->ib_pool_ready) {
return 0;
}
- rdev->ib_pool.ready = true;
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
- r = radeon_sa_bo_manager_init(rdev, &rdev->ib_pool.sa_manager,
+ r = radeon_sa_bo_manager_init(rdev, &rdev->sa_manager,
RADEON_IB_POOL_SIZE*64*1024,
- RADEON_GEM_DOMAIN_GTT, NULL);
+ RADEON_GEM_DOMAIN_GTT,
+ radeon_ib_try_free);
if (r) {
return r;
}
- radeon_mutex_lock(&rdev->ib_pool.mutex);
- for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
- rdev->ib_pool.ibs[i].fence = NULL;
- rdev->ib_pool.ibs[i].idx = i;
- rdev->ib_pool.ibs[i].length_dw = 0;
- INIT_LIST_HEAD(&rdev->ib_pool.ibs[i].sa_bo.list);
+ if (radeon_debugfs_sa_init(rdev)) {
+ DRM_ERROR("Failed to register debugfs file for SA !\n");
}
- rdev->ib_pool.head_id = 0;
- rdev->ib_pool.ready = true;
+
DRM_INFO("radeon: ib pool ready.\n");
+ rdev->ib_pool_ready = true;
- if (radeon_debugfs_ib_init(rdev)) {
- DRM_ERROR("Failed to register debugfs file for IB !\n");
- }
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
return 0;
}
void radeon_ib_pool_fini(struct radeon_device *rdev)
{
- unsigned i;
-
- radeon_mutex_lock(&rdev->ib_pool.mutex);
- if (rdev->ib_pool.ready) {
- for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
- radeon_sa_bo_free(rdev, &rdev->ib_pool.ibs[i].sa_bo);
- radeon_fence_unref(&rdev->ib_pool.ibs[i].fence);
- }
- radeon_sa_bo_manager_fini(rdev, &rdev->ib_pool.sa_manager);
- rdev->ib_pool.ready = false;
+ if (rdev->ib_pool_ready) {
+ radeon_sa_bo_manager_fini(rdev, &rdev->sa_manager);
+ rdev->ib_pool_ready = false;
}
- radeon_mutex_unlock(&rdev->ib_pool.mutex);
}
int radeon_ib_pool_start(struct radeon_device *rdev)
{
- return radeon_sa_bo_manager_start(rdev, &rdev->ib_pool.sa_manager);
+ return radeon_sa_bo_manager_start(rdev, &rdev->sa_manager);
}
int radeon_ib_pool_suspend(struct radeon_device *rdev)
{
- return radeon_sa_bo_manager_suspend(rdev, &rdev->ib_pool.sa_manager);
+ return radeon_sa_bo_manager_suspend(rdev, &rdev->sa_manager);
}
int radeon_ib_ring_tests(struct radeon_device *rdev)
@@ -296,6 +241,21 @@ int radeon_ib_ring_tests(struct radeon_device *rdev)
/*
* Ring.
*/
+int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring);
+
+void radeon_ring_write(struct radeon_ring *ring, uint32_t v)
+{
+#if DRM_DEBUG_CODE
+ if (ring->count_dw <= 0) {
+ DRM_ERROR("radeon: writting more dword to ring than expected !\n");
+ }
+#endif
+ ring->ring[ring->wptr++] = v;
+ ring->wptr &= ring->ptr_mask;
+ ring->count_dw--;
+ ring->ring_free_dw--;
+}
+
int radeon_ring_index(struct radeon_device *rdev, struct radeon_ring *ring)
{
/* r1xx-r5xx only has CP ring */
@@ -502,37 +462,13 @@ static struct drm_info_list radeon_debugfs_ring_info_list[] = {
{"radeon_ring_cp2", radeon_debugfs_ring_info, 0, &cayman_ring_type_cp2_index},
};
-static int radeon_debugfs_ib_info(struct seq_file *m, void *data)
-{
- struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct drm_device *dev = node->minor->dev;
- struct radeon_device *rdev = dev->dev_private;
- struct radeon_ib *ib = &rdev->ib_pool.ibs[*((unsigned*)node->info_ent->data)];
- unsigned i;
-
- if (ib == NULL) {
- return 0;
- }
- seq_printf(m, "IB %04u\n", ib->idx);
- seq_printf(m, "IB fence %p\n", ib->fence);
- seq_printf(m, "IB size %05u dwords\n", ib->length_dw);
- for (i = 0; i < ib->length_dw; i++) {
- seq_printf(m, "[%05u]=0x%08X\n", i, ib->ptr[i]);
- }
- return 0;
-}
-
-static struct drm_info_list radeon_debugfs_ib_list[RADEON_IB_POOL_SIZE];
-static char radeon_debugfs_ib_names[RADEON_IB_POOL_SIZE][32];
-static unsigned radeon_debugfs_ib_idx[RADEON_IB_POOL_SIZE];
-
static int radeon_debugfs_sa_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
struct radeon_device *rdev = dev->dev_private;
- radeon_sa_bo_dump_debug_info(&rdev->ib_pool.sa_manager, m);
+ radeon_sa_bo_dump_debug_info(&rdev->sa_manager, m);
return 0;
@@ -564,26 +500,10 @@ int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *rin
return 0;
}
-int radeon_debugfs_ib_init(struct radeon_device *rdev)
+int radeon_debugfs_sa_init(struct radeon_device *rdev)
{
#if defined(CONFIG_DEBUG_FS)
- unsigned i;
- int r;
-
- r = radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1);
- if (r)
- return r;
-
- for (i = 0; i < RADEON_IB_POOL_SIZE; i++) {
- sprintf(radeon_debugfs_ib_names[i], "radeon_ib_%04u", i);
- radeon_debugfs_ib_idx[i] = i;
- radeon_debugfs_ib_list[i].name = radeon_debugfs_ib_names[i];
- radeon_debugfs_ib_list[i].show = &radeon_debugfs_ib_info;
- radeon_debugfs_ib_list[i].driver_features = 0;
- radeon_debugfs_ib_list[i].data = &radeon_debugfs_ib_idx[i];
- }
- return radeon_debugfs_add_files(rdev, radeon_debugfs_ib_list,
- RADEON_IB_POOL_SIZE);
+ return radeon_debugfs_add_files(rdev, radeon_debugfs_sa_list, 1);
#else
return 0;
#endif
diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c
index 5a4741a..f6929c7 100644
--- a/drivers/gpu/drm/radeon/radeon_semaphore.c
+++ b/drivers/gpu/drm/radeon/radeon_semaphore.c
@@ -41,7 +41,7 @@ int radeon_semaphore_create(struct radeon_device *rdev,
return -ENOMEM;
}
- r = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager,
+ r = radeon_sa_bo_new(rdev, &rdev->sa_manager,
&(*semaphore)->sa_bo, 8, 8, true);
if (r) {
kfree(*semaphore);
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 18/26] drm/radeon: fix a bug with the ring syncing code
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (16 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 17/26] drm/radeon: rip out the ib pool v2 Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 19/26] drm/radeon: rework recursive gpu reset handling Christian König
` (9 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 12d2003..cfd00a9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -445,6 +445,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 f6929c7..731ee65 100644
--- a/drivers/gpu/drm/radeon/radeon_semaphore.c
+++ b/drivers/gpu/drm/radeon/radeon_semaphore.c
@@ -70,6 +70,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 19/26] drm/radeon: rework recursive gpu reset handling
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (17 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 18/26] drm/radeon: fix a bug with the ring syncing code Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 20/26] drm/radeon: remove recursive mutex implementation Christian König
` (8 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 853e1cb..8a4a210 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -983,9 +983,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);
@@ -1000,8 +997,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 326bdb5..24a4d4b 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -264,6 +264,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])) {
@@ -274,13 +276,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 20/26] drm/radeon: remove recursive mutex implementation
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (18 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 19/26] drm/radeon: rework recursive gpu reset handling Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:50 ` [PATCH 21/26] drm/radeon: move lockup detection code into radeon_ring.c Christian König
` (7 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Not needed anymore.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
drivers/gpu/drm/radeon/radeon.h | 44 +-------------------------------
drivers/gpu/drm/radeon/radeon_cs.c | 10 +++---
drivers/gpu/drm/radeon/radeon_device.c | 2 +-
drivers/gpu/drm/radeon/radeon_gart.c | 16 ++++++------
drivers/gpu/drm/radeon/radeon_gem.c | 4 +-
5 files changed, 17 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index cfd00a9..b48b84c 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -155,48 +155,6 @@ static inline int radeon_atrm_get_bios_chunk(uint8_t *bios, int offset, int len)
#endif
bool radeon_get_bios(struct radeon_device *rdev);
-
-/*
- * Mutex which allows recursive locking from the same process.
- */
-struct radeon_mutex {
- struct mutex mutex;
- struct task_struct *owner;
- int level;
-};
-
-static inline void radeon_mutex_init(struct radeon_mutex *mutex)
-{
- mutex_init(&mutex->mutex);
- mutex->owner = NULL;
- mutex->level = 0;
-}
-
-static inline void radeon_mutex_lock(struct radeon_mutex *mutex)
-{
- if (mutex_trylock(&mutex->mutex)) {
- /* The mutex was unlocked before, so it's ours now */
- mutex->owner = current;
- } else if (mutex->owner != current) {
- /* Another process locked the mutex, take it */
- mutex_lock(&mutex->mutex);
- mutex->owner = current;
- }
- /* Otherwise the mutex was already locked by this process */
-
- mutex->level++;
-}
-
-static inline void radeon_mutex_unlock(struct radeon_mutex *mutex)
-{
- if (--mutex->level > 0)
- return;
-
- mutex->owner = NULL;
- mutex_unlock(&mutex->mutex);
-}
-
-
/*
* Dummy page
*/
@@ -1509,7 +1467,7 @@ struct radeon_device {
struct radeon_gem gem;
struct radeon_pm pm;
uint32_t bios_scratch[RADEON_BIOS_NUM_SCRATCH];
- struct radeon_mutex cs_mutex;
+ struct mutex cs_mutex;
struct radeon_wb wb;
struct radeon_dummy_page dummy_page;
bool shutdown;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 02eee4b..c3273b8 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -512,9 +512,9 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
struct radeon_cs_parser parser;
int r;
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
if (!rdev->accel_working) {
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
return -EBUSY;
}
/* initialize parser */
@@ -528,7 +528,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
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);
+ mutex_unlock(&rdev->cs_mutex);
return r;
}
r = radeon_cs_parser_relocs(&parser);
@@ -537,7 +537,7 @@ int radeon_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
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);
+ mutex_unlock(&rdev->cs_mutex);
return r;
}
r = radeon_cs_ib_chunk(rdev, &parser);
@@ -551,7 +551,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);
+ 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 8a4a210..94f8561 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -722,7 +722,7 @@ int radeon_device_init(struct radeon_device *rdev,
/* mutex initialization are all done here so we
* can recall function without having locking issues */
- radeon_mutex_init(&rdev->cs_mutex);
+ mutex_init(&rdev->cs_mutex);
for (i = 0; i < RADEON_NUM_RINGS; ++i)
mutex_init(&rdev->ring[i].mutex);
mutex_init(&rdev->dc_hw_i2c_mutex);
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 8eba1c6..71b28e91 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -356,13 +356,13 @@ int radeon_vm_manager_suspend(struct radeon_device *rdev)
{
struct radeon_vm *vm, *tmp;
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
/* unbind all active vm */
list_for_each_entry_safe(vm, tmp, &rdev->vm_manager.lru_vm, list) {
radeon_vm_unbind_locked(rdev, vm);
}
rdev->vm_manager.funcs->fini(rdev);
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
return radeon_sa_bo_manager_suspend(rdev, &rdev->vm_manager.sa_manager);
}
@@ -478,9 +478,9 @@ int radeon_vm_bo_add(struct radeon_device *rdev,
if (last_pfn > vm->last_pfn) {
/* grow va space 32M by 32M */
unsigned align = ((32 << 20) >> 12) - 1;
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
radeon_vm_unbind_locked(rdev, vm);
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
vm->last_pfn = (last_pfn + align) & ~align;
}
head = &vm->va;
@@ -596,9 +596,9 @@ int radeon_vm_bo_rmv(struct radeon_device *rdev,
return 0;
mutex_lock(&vm->mutex);
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
radeon_vm_bo_update_pte(rdev, vm, bo, NULL);
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
list_del(&bo_va->vm_list);
mutex_unlock(&vm->mutex);
list_del(&bo_va->bo_list);
@@ -643,9 +643,9 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm)
mutex_lock(&vm->mutex);
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
radeon_vm_unbind_locked(rdev, vm);
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
/* remove all bo */
r = radeon_bo_reserve(rdev->sa_manager.bo, false);
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index e15cb1f..bd4521e 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -157,11 +157,11 @@ void radeon_gem_object_close(struct drm_gem_object *obj,
static int radeon_gem_handle_lockup(struct radeon_device *rdev, int r)
{
if (r == -EDEADLK) {
- radeon_mutex_lock(&rdev->cs_mutex);
+ mutex_lock(&rdev->cs_mutex);
r = radeon_gpu_reset(rdev);
if (!r)
r = -EAGAIN;
- radeon_mutex_unlock(&rdev->cs_mutex);
+ mutex_unlock(&rdev->cs_mutex);
}
return r;
}
--
1.7.5.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 21/26] drm/radeon: move lockup detection code into radeon_ring.c
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (19 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 20/26] drm/radeon: remove recursive mutex implementation Christian König
@ 2012-04-30 14:50 ` Christian König
2012-04-30 14:51 ` [PATCH 22/26] drm/radeon: make lockup timeout a module param Christian König
` (6 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:50 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
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>
---
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 26848d6..e304858 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 c0b0956..4327b32 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 62f9dab..20bf498 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 659a09c..be2a586 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 b48b84c..bcf4cc4 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -600,6 +600,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;
@@ -743,6 +745,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);
@@ -1192,16 +1196,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 {
@@ -1209,7 +1207,6 @@ struct r300_asic {
unsigned reg_safe_bm_size;
u32 resync_scratch;
u32 hdp_cntl;
- struct r100_gpu_lockup lockup;
};
struct r600_asic {
@@ -1231,7 +1228,6 @@ struct r600_asic {
unsigned tiling_group_size;
unsigned tile_config;
unsigned backend_map;
- struct r100_gpu_lockup lockup;
};
struct rv770_asic {
@@ -1257,7 +1253,6 @@ struct rv770_asic {
unsigned tiling_group_size;
unsigned tile_config;
unsigned backend_map;
- struct r100_gpu_lockup lockup;
};
struct evergreen_asic {
@@ -1284,7 +1279,6 @@ struct evergreen_asic {
unsigned tiling_group_size;
unsigned tile_config;
unsigned backend_map;
- struct r100_gpu_lockup lockup;
};
struct cayman_asic {
@@ -1323,7 +1317,6 @@ struct cayman_asic {
unsigned multi_gpu_tile_size;
unsigned tile_config;
- struct r100_gpu_lockup lockup;
};
struct si_asic {
@@ -1354,7 +1347,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 a56e44e..4c76d21 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -352,6 +352,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 7c0110d..b11b9f0 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 22/26] drm/radeon: make lockup timeout a module param
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (20 preceding siblings ...)
2012-04-30 14:50 ` [PATCH 21/26] drm/radeon: move lockup detection code into radeon_ring.c Christian König
@ 2012-04-30 14:51 ` Christian König
2012-04-30 14:51 ` [PATCH 23/26] drm/radeon: unlock the ring mutex while waiting for the next fence Christian König
` (5 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:51 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Don't hard code the 10 seconds timeout. Compute jobs
can run much longer.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 bcf4cc4..aaeb8af 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 4c76d21..504fb8f 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -397,7 +397,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 23/26] drm/radeon: unlock the ring mutex while waiting for the next fence
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (21 preceding siblings ...)
2012-04-30 14:51 ` [PATCH 22/26] drm/radeon: make lockup timeout a module param Christian König
@ 2012-04-30 14:51 ` Christian König
2012-04-30 14:51 ` [PATCH 24/26] drm/radeon: make forcing ring activity a common function Christian König
` (4 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:51 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Fixing just another deadlock problem with gpu reset tests.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 504fb8f..0f74c99 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -302,7 +302,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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 24/26] drm/radeon: make forcing ring activity a common function
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (22 preceding siblings ...)
2012-04-30 14:51 ` [PATCH 23/26] drm/radeon: unlock the ring mutex while waiting for the next fence Christian König
@ 2012-04-30 14:51 ` Christian König
2012-04-30 14:51 ` [PATCH 25/26] drm/radeon: remove r300_gpu_is_lockup Christian König
` (3 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:51 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Nothing chipset or ring specific with it,
so also move it to radon_ring.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 e304858..7e7ac3d 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 4327b32..8a9c85d 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 20bf498..217cdc2 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 be2a586..cd06d39 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 aaeb8af..c59894f 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -746,6 +746,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 0f74c99..6682034 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -354,6 +354,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 b11b9f0..d6b7fbc 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 25/26] drm/radeon: remove r300_gpu_is_lockup
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (23 preceding siblings ...)
2012-04-30 14:51 ` [PATCH 24/26] drm/radeon: make forcing ring activity a common function Christian König
@ 2012-04-30 14:51 ` Christian König
2012-04-30 14:51 ` [PATCH 26/26] drm/radeon: remove cayman_gpu_is_lockup Christian König
` (2 subsequent siblings)
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:51 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Since it is now identical to r100_gpu_is_lockup.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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
^ permalink raw reply related [flat|nested] 39+ messages in thread* [PATCH 26/26] drm/radeon: remove cayman_gpu_is_lockup
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (24 preceding siblings ...)
2012-04-30 14:51 ` [PATCH 25/26] drm/radeon: remove r300_gpu_is_lockup Christian König
@ 2012-04-30 14:51 ` Christian König
2012-04-30 15:12 ` Include request for reset-rework branch Jerome Glisse
2012-05-01 13:37 ` Alex Deucher
27 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-04-30 14:51 UTC (permalink / raw)
To: airlied; +Cc: Christian König, dri-devel
Since it is now identical to evergreen_gpu_is_lockup.
Signed-off-by: Christian König <deathsimple@vodafone.de>
---
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 8a9c85d..107b217 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
^ permalink raw reply related [flat|nested] 39+ messages in thread* Re: Include request for reset-rework branch.
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (25 preceding siblings ...)
2012-04-30 14:51 ` [PATCH 26/26] drm/radeon: remove cayman_gpu_is_lockup Christian König
@ 2012-04-30 15:12 ` Jerome Glisse
2012-04-30 15:12 ` Jerome Glisse
2012-05-01 13:37 ` Alex Deucher
27 siblings, 1 reply; 39+ messages in thread
From: Jerome Glisse @ 2012-04-30 15:12 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Mon, Apr 30, 2012 at 10:50 AM, Christian König
<deathsimple@vodafone.de> wrote:
> Hi Dave,
>
> if nobody has a last moment concern please include the following patches in drm-next.
>
> Except for some minor fixes they have already been on the list for quite some time,
> but I intentional left out the debugfs related patches cause we haven't finished the
> discussion about them yet.
>
> If you prefer to merge them directly, I also made them available as reset-rework
> branch here: git://people.freedesktop.org/~deathsimple/linux
>
> Cheers,
> Christian.
>
I am not completely ok, i am against patch 5. I need sometime to review it.
Cheers,
Jerome
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: Include request for reset-rework branch.
2012-04-30 15:12 ` Include request for reset-rework branch Jerome Glisse
@ 2012-04-30 15:12 ` Jerome Glisse
2012-04-30 15:37 ` Christian König
0 siblings, 1 reply; 39+ messages in thread
From: Jerome Glisse @ 2012-04-30 15:12 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Mon, Apr 30, 2012 at 11:12 AM, Jerome Glisse <j.glisse@gmail.com> wrote:
> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> Hi Dave,
>>
>> if nobody has a last moment concern please include the following patches in drm-next.
>>
>> Except for some minor fixes they have already been on the list for quite some time,
>> but I intentional left out the debugfs related patches cause we haven't finished the
>> discussion about them yet.
>>
>> If you prefer to merge them directly, I also made them available as reset-rework
>> branch here: git://people.freedesktop.org/~deathsimple/linux
>>
>> Cheers,
>> Christian.
>>
>
> I am not completely ok, i am against patch 5. I need sometime to review it.
>
> Cheers,
> Jerome
Sorr mean patch 7
Cheers,
Jerome
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Include request for reset-rework branch.
2012-04-30 15:12 ` Jerome Glisse
@ 2012-04-30 15:37 ` Christian König
2012-04-30 16:26 ` Jerome Glisse
0 siblings, 1 reply; 39+ messages in thread
From: Christian König @ 2012-04-30 15:37 UTC (permalink / raw)
To: Jerome Glisse; +Cc: airlied, dri-devel
On 30.04.2012 17:12, Jerome Glisse wrote:
> On Mon, Apr 30, 2012 at 11:12 AM, Jerome Glisse<j.glisse@gmail.com> wrote:
>> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
>> <deathsimple@vodafone.de> wrote:
>>> Hi Dave,
>>>
>>> if nobody has a last moment concern please include the following patches in drm-next.
>>>
>>> Except for some minor fixes they have already been on the list for quite some time,
>>> but I intentional left out the debugfs related patches cause we haven't finished the
>>> discussion about them yet.
>>>
>>> If you prefer to merge them directly, I also made them available as reset-rework
>>> branch here: git://people.freedesktop.org/~deathsimple/linux
>>>
>>> Cheers,
>>> Christian.
>>>
>> I am not completely ok, i am against patch 5. I need sometime to review it.
>>
>> Cheers,
>> Jerome
> Sorr mean patch 7
I just started to wonder :) what's wrong with patch 7?
Please keep in mind that implementing proper locking in the lower level
objects allows us to remove quite some locking in the upper layers.
By the way, do you mind when I dig into the whole locking stuff of the
kernel driver a bit more? There seems to be allot of possibilities to
cleanup and simplify the overall driver.
Cheers,
Christian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Include request for reset-rework branch.
2012-04-30 15:37 ` Christian König
@ 2012-04-30 16:26 ` Jerome Glisse
2012-05-01 13:38 ` Christian König
0 siblings, 1 reply; 39+ messages in thread
From: Jerome Glisse @ 2012-04-30 16:26 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
On Mon, Apr 30, 2012 at 11:37 AM, Christian König
<deathsimple@vodafone.de> wrote:
> On 30.04.2012 17:12, Jerome Glisse wrote:
>>
>> On Mon, Apr 30, 2012 at 11:12 AM, Jerome Glisse<j.glisse@gmail.com>
>> wrote:
>>>
>>> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
>>> <deathsimple@vodafone.de> wrote:
>>>>
>>>> Hi Dave,
>>>>
>>>> if nobody has a last moment concern please include the following patches
>>>> in drm-next.
>>>>
>>>> Except for some minor fixes they have already been on the list for quite
>>>> some time,
>>>> but I intentional left out the debugfs related patches cause we haven't
>>>> finished the
>>>> discussion about them yet.
>>>>
>>>> If you prefer to merge them directly, I also made them available as
>>>> reset-rework
>>>> branch here: git://people.freedesktop.org/~deathsimple/linux
>>>>
>>>> Cheers,
>>>> Christian.
>>>>
>>> I am not completely ok, i am against patch 5. I need sometime to review
>>> it.
>>>
>>> Cheers,
>>> Jerome
>>
>> Sorr mean patch 7
>
> I just started to wonder :) what's wrong with patch 7?
>
> Please keep in mind that implementing proper locking in the lower level
> objects allows us to remove quite some locking in the upper layers.
>
> By the way, do you mind when I dig into the whole locking stuff of the
> kernel driver a bit more? There seems to be allot of possibilities to
> cleanup and simplify the overall driver.
>
> Cheers,
> Christian.
Well when it comes to lock, i have some kind of an idea i wanted to
work for a while. GPU is transaction based in a sense, we feed rings
and it works on them, 90% of the work is cs ioctl so we should be able
to have one and only one lock (ignoring the whole modesetting path
here for which i believe one lock too is enough). Things like PM would
need to take all lock but hey i believe that's a good thing as my
understanding is PM we do right now need most of the GPU idle.
So here is what we have
ih spinlock (can be ignored ie left as is)
irq spinlock (can be ignored ie left as is)
blit mutex
pm mutex
cs mutex
dc_hw_i2c mutex
vram mutex
ib mutex
rings[] lock
So the real issue is ttm calling back into the driver. So the idea i
had is to have a work thread that is the only one allowed to mess with
the GPU. The work thread would use some locking in which it has
preference over the writer (writer being cs ioctl or ttm call back or
anything else that needs to schedule GPU work). This would require
only two lock. I am actually thinking of having 2 list one where
writer add there "transaction" and one where the worker empty the
transaction, something like:
cs_ioct
{
sa_alloc_for_trans
...
lock(trans_lock)
list_add_tail(trans, trans_temp_list)
unlock(trans_lock)
}
workeur
{
lock(trans_lock)
list_splice_tail(trans_list, trans_temp_list)
unlock(trans_lock)
// schedule ib
...
// process fence & semaphore
}
So there would be one transaction lock and one lock for the
transaction memory allocation (ib, semaphore and alike) The workeur
would also be responsible for GPU reset, there wouldn't be any ring
lock as i believe one worker is enough for all rings.
For transaction the only issue really is ttm, cs is easy it's an
ib+fence+semaphore, ttm can be more complex, it can be bo move, bind,
unbind, ... Anyway it's doable and it's design i had in mind for a
while.
Cheers,
Jerome
^ permalink raw reply [flat|nested] 39+ messages in thread* Re: Include request for reset-rework branch.
2012-04-30 16:26 ` Jerome Glisse
@ 2012-05-01 13:38 ` Christian König
0 siblings, 0 replies; 39+ messages in thread
From: Christian König @ 2012-05-01 13:38 UTC (permalink / raw)
To: Jerome Glisse; +Cc: airlied, dri-devel
On 30.04.2012 18:26, Jerome Glisse wrote:
> On Mon, Apr 30, 2012 at 11:37 AM, Christian König
> <deathsimple@vodafone.de> wrote:
>> On 30.04.2012 17:12, Jerome Glisse wrote:
>>> On Mon, Apr 30, 2012 at 11:12 AM, Jerome Glisse<j.glisse@gmail.com>
>>> wrote:
>>>> On Mon, Apr 30, 2012 at 10:50 AM, Christian König
>>>> <deathsimple@vodafone.de> wrote:
>>>>> Hi Dave,
>>>>>
>>>>> if nobody has a last moment concern please include the following patches
>>>>> in drm-next.
>>>>>
>>>>> Except for some minor fixes they have already been on the list for quite
>>>>> some time,
>>>>> but I intentional left out the debugfs related patches cause we haven't
>>>>> finished the
>>>>> discussion about them yet.
>>>>>
>>>>> If you prefer to merge them directly, I also made them available as
>>>>> reset-rework
>>>>> branch here: git://people.freedesktop.org/~deathsimple/linux
>>>>>
>>>>> Cheers,
>>>>> Christian.
>>>>>
>>>> I am not completely ok, i am against patch 5. I need sometime to review
>>>> it.
>>>>
>>>> Cheers,
>>>> Jerome
>>> Sorr mean patch 7
>> I just started to wonder :) what's wrong with patch 7?
>>
>> Please keep in mind that implementing proper locking in the lower level
>> objects allows us to remove quite some locking in the upper layers.
>>
>> By the way, do you mind when I dig into the whole locking stuff of the
>> kernel driver a bit more? There seems to be allot of possibilities to
>> cleanup and simplify the overall driver.
>>
>> Cheers,
>> Christian.
> Well when it comes to lock, i have some kind of an idea i wanted to
> work for a while. GPU is transaction based in a sense, we feed rings
> and it works on them, 90% of the work is cs ioctl so we should be able
> to have one and only one lock (ignoring the whole modesetting path
> here for which i believe one lock too is enough). Things like PM would
> need to take all lock but hey i believe that's a good thing as my
> understanding is PM we do right now need most of the GPU idle.
>
> So here is what we have
> ih spinlock (can be ignored ie left as is)
> irq spinlock (can be ignored ie left as is)
> blit mutex
> pm mutex
> cs mutex
> dc_hw_i2c mutex
> vram mutex
> ib mutex
> rings[] lock
>
> So the real issue is ttm calling back into the driver. So the idea i
> had is to have a work thread that is the only one allowed to mess with
> the GPU. The work thread would use some locking in which it has
> preference over the writer (writer being cs ioctl or ttm call back or
> anything else that needs to schedule GPU work). This would require
> only two lock. I am actually thinking of having 2 list one where
> writer add there "transaction" and one where the worker empty the
> transaction, something like:
>
> cs_ioct
> {
> sa_alloc_for_trans
> ....
> lock(trans_lock)
> list_add_tail(trans, trans_temp_list)
> unlock(trans_lock)
> }
>
> workeur
> {
> lock(trans_lock)
> list_splice_tail(trans_list, trans_temp_list)
> unlock(trans_lock)
> // schedule ib
> ....
> // process fence& semaphore
> }
>
> So there would be one transaction lock and one lock for the
> transaction memory allocation (ib, semaphore and alike) The workeur
> would also be responsible for GPU reset, there wouldn't be any ring
> lock as i believe one worker is enough for all rings.
>
> For transaction the only issue really is ttm, cs is easy it's an
> ib+fence+semaphore, ttm can be more complex, it can be bo move, bind,
> unbind, ... Anyway it's doable and it's design i had in mind for a
> while.
Well, that sounds like the direction to go, but I would like to even
avoid the submission thread, since the GPU is working on the rings
asynchronously anyway.
The locking problems we are currently seeing are more a result of
abusing global variables for local data. or locking to much code with to
few primitives, instead of just having to much locking primitives over
all. For example, I really can't see why we have a blit mutex for the
r600 shader blit code? Also retrospective having one mutex per ring does
sound a bit superfluous.
To sum my ideas up:
1. I suggest that memory management is self containing, e.g. you can
request small amounts of memory for IBs, fences, semaphores, blitting
vertex buffer etc.. without worrying about others doing that at the same
time as you. That really sounds like your "lock for the transaction
memory allocation", and I'm pretty sure that I've extended the SA so far
to play that role pretty well.
2. Have exactly ONE ring submission mutex. This mutex is taken right
before an job (and it shouldn't matter if that's a ttm move/blit or an
IB) is pushed unto a ring. And it is strictly not allowed to allocate
more SA memory, call into TTM etc.. while this mutex is held. Everything
that's necessary to submit a job must happen before grabbing it and
stored in thread local memory, e.g. on the stack or a kmalloc allocated
patch of memory.
3. Protect data, not code! I.e. have locks to protect one data
structures and not just say: Those two things shouldn't happen at the
same time acquire that and this lock.
Over all it sounds like we are playing with similar ideas, I would
suggest that I just hack together some patches, probably starting with
the ring submission and r600 blit mutex and then we take a look again
where this leads us.
Cheers,
Christian.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: Include request for reset-rework branch.
2012-04-30 14:50 Include request for reset-rework branch Christian König
` (26 preceding siblings ...)
2012-04-30 15:12 ` Include request for reset-rework branch Jerome Glisse
@ 2012-05-01 13:37 ` Alex Deucher
27 siblings, 0 replies; 39+ messages in thread
From: Alex Deucher @ 2012-05-01 13:37 UTC (permalink / raw)
To: Christian König; +Cc: airlied, dri-devel
2012/4/30 Christian König <deathsimple@vodafone.de>:
> Hi Dave,
>
> if nobody has a last moment concern please include the following patches in drm-next.
>
> Except for some minor fixes they have already been on the list for quite some time,
> but I intentional left out the debugfs related patches cause we haven't finished the
> discussion about them yet.
>
> If you prefer to merge them directly, I also made them available as reset-rework
> branch here: git://people.freedesktop.org/~deathsimple/linux
For the series:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> Cheers,
> Christian.
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 39+ messages in thread