From: Christian König deathsimple@vodafone.de

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

v2: rebased on current drm-next

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

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 138b952..bea99e3 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -1144,7 +1144,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 @@ -1173,6 +1172,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 { @@ -1730,7 +1730,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)) @@ -1739,6 +1738,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);

From: Christian König deathsimple@vodafone.de

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

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

diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c index cc33b3d..b6eb1d2 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -34,7 +34,7 @@ #include "atom.h"

int radeon_debugfs_ib_init(struct radeon_device *rdev); -int radeon_debugfs_ring_init(struct radeon_device *rdev); +int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring);

u32 radeon_get_ib_value(struct radeon_cs_parser *p, int idx) { @@ -237,9 +237,6 @@ int radeon_ib_pool_init(struct radeon_device *rdev) if (radeon_debugfs_ib_init(rdev)) { DRM_ERROR("Failed to register debugfs file for IB !\n"); } - if (radeon_debugfs_ring_init(rdev)) { - DRM_ERROR("Failed to register debugfs file for rings !\n"); - } radeon_mutex_unlock(&rdev->ib_pool.mutex); return 0; } @@ -411,6 +408,9 @@ int radeon_ring_init(struct radeon_device *rdev, struct radeon_ring *ring, unsig } ring->ptr_mask = (ring->ring_size / 4) - 1; ring->ring_free_dw = ring->ring_size / 4; + if (radeon_debugfs_ring_init(rdev, ring)) { + DRM_ERROR("Failed to register debugfs file for rings !\n"); + } return 0; }

@@ -501,17 +501,24 @@ static char radeon_debugfs_ib_names[RADEON_IB_POOL_SIZE][32]; static unsigned radeon_debugfs_ib_idx[RADEON_IB_POOL_SIZE]; #endif

-int radeon_debugfs_ring_init(struct radeon_device *rdev) +int radeon_debugfs_ring_init(struct radeon_device *rdev, struct radeon_ring *ring) { #if defined(CONFIG_DEBUG_FS) - if (rdev->family >= CHIP_CAYMAN) - return radeon_debugfs_add_files(rdev, radeon_debugfs_ring_info_list, - ARRAY_SIZE(radeon_debugfs_ring_info_list)); - else - return radeon_debugfs_add_files(rdev, radeon_debugfs_ring_info_list, 1); -#else - return 0; + unsigned i; + for (i = 0; i < ARRAY_SIZE(radeon_debugfs_ring_info_list); ++i) { + struct drm_info_list *info = &radeon_debugfs_ring_info_list[i]; + int ridx = *(int*)radeon_debugfs_ring_info_list[i].data; + unsigned r; + + if (&rdev->ring[ridx] != ring) + continue; + + r = radeon_debugfs_add_files(rdev, info, 1); + if (r) + return r; + } #endif + return 0; }

int radeon_debugfs_ib_init(struct radeon_device *rdev)

From: Christian König deathsimple@vodafone.de

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

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

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 8801657..85a3aa9 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; }

From: Christian König deathsimple@vodafone.de

Make the suballocator self containing to locking.

v2: split the bugfix into a seperate patch.

Signed-off-by: Christian König deathsimple@vodafone.de Reviewed-by: Jerome Glisse jglisse@redhat.com --- drivers/gpu/drm/radeon/radeon.h | 1 + drivers/gpu/drm/radeon/radeon_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 85a3aa9..1aefbd9 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..472d346 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; @@ -139,15 +140,15 @@ int radeon_sa_bo_new(struct radeon_device *rdev,

BUG_ON(align > RADEON_GPU_PAGE_SIZE); BUG_ON(size > sa_manager->size); + spin_lock(&sa_manager->lock);

/* 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 +158,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 +167,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(&sa_manager->lock); return -ENOMEM; }

@@ -180,10 +180,15 @@ out: sa_bo->offset = offset; sa_bo->size = size; list_add(&sa_bo->list, head); + spin_unlock(&sa_manager->lock); return 0; }

void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo) { + struct radeon_sa_manager *sa_manager = sa_bo->manager; + + spin_lock(&sa_manager->lock); list_del_init(&sa_bo->list); + spin_unlock(&sa_manager->lock); }

From: Jerome Glisse jglisse@redhat.com

The sa allocator is suppose to be a ring allocator, ie allocation happen first at the end and if there is no more room we start at the begining again. This patch make the code match this design. sa_manager keep track of the start & end hole, it first try to allocate in the end hole, if it fails it allocate in the begining hole, if it fails it returns (caller is expected to retry).

When freeing we need to make sure that we properly grow the end hole and start hole. We take advantage of the fact that the sa_bo list is ordered by offset. That means that when we free an sa_bo the previous sa_bo in list is also the sa_bo just before the sa_bo we are freeing and reversly for the next.

v2: Use read ptr metaphore to mimic ring behavior and simplify code a bit.

Signed-off-by: Jerome Glisse jglisse@redhat.com --- drivers/gpu/drm/radeon/radeon.h | 4 +- drivers/gpu/drm/radeon/radeon_cs.c | 4 +- drivers/gpu/drm/radeon/radeon_gart.c | 6 +- drivers/gpu/drm/radeon/radeon_object.h | 11 +++ drivers/gpu/drm/radeon/radeon_ring.c | 6 +- drivers/gpu/drm/radeon/radeon_sa.c | 128 +++++++++++++++++++---------- drivers/gpu/drm/radeon/radeon_semaphore.c | 4 +- 7 files changed, 107 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 1aefbd9..dc4f4f3 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -385,6 +385,7 @@ struct radeon_sa_manager { struct radeon_bo *bo; struct list_head sa_bo; unsigned size; + struct radeon_sa_bo *last; uint64_t gpu_addr; void *cpu_ptr; uint32_t domain; @@ -396,7 +397,8 @@ struct radeon_sa_bo; struct radeon_sa_bo { struct list_head list; struct radeon_sa_manager *manager; - unsigned offset; + unsigned soffset; + unsigned eoffset; unsigned size; };

diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index 5cac832..8de6b3a 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -476,7 +476,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, /* ib pool is bind at 0 in virtual address space to gpu_addr is the * offset inside the pool bo */ - parser->const_ib->gpu_addr = parser->const_ib->sa_bo.offset; + parser->const_ib->gpu_addr = parser->const_ib->sa_bo.soffset; r = radeon_ib_schedule(rdev, parser->const_ib); if (r) goto out; @@ -486,7 +486,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, /* ib pool is bind at 0 in virtual address space to gpu_addr is the * offset inside the pool bo */ - parser->ib->gpu_addr = parser->ib->sa_bo.offset; + parser->ib->gpu_addr = parser->ib->sa_bo.soffset; parser->ib->is_const_ib = false; r = radeon_ib_schedule(rdev, parser->ib); out: diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index c58a036..4a5d9d4 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 d9b9333..99ab46a 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->soffset; +} + +static inline void * radeon_sa_bo_cpu_addr(struct radeon_sa_bo *sa_bo) +{ + return sa_bo->manager->cpu_ptr + sa_bo->soffset; +} + 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 1d9bce9..981ab95 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -127,10 +127,8 @@ retry: size, 256); 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; diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c index 1e1bec1..63b0cd2 100644 --- a/drivers/gpu/drm/radeon/radeon_sa.c +++ b/drivers/gpu/drm/radeon/radeon_sa.c @@ -41,6 +41,7 @@ int radeon_sa_bo_manager_init(struct radeon_device *rdev, sa_manager->bo = NULL; sa_manager->size = size; sa_manager->domain = domain; + sa_manager->last = NULL; INIT_LIST_HEAD(&sa_manager->sa_bo);

r = radeon_bo_create(rdev, size, RADEON_GPU_PAGE_SIZE, true, @@ -64,7 +65,9 @@ void radeon_sa_bo_manager_fini(struct radeon_device *rdev, list_for_each_entry_safe(sa_bo, tmp, &sa_manager->sa_bo, list) { list_del_init(&sa_bo->list); } - radeon_bo_unref(&sa_manager->bo); + if (sa_manager->bo) { + radeon_bo_unref(&sa_manager->bo); + } sa_manager->size = 0; }

@@ -114,18 +117,37 @@ int radeon_sa_bo_manager_suspend(struct radeon_device *rdev, return r; }

+static void radeon_sa_bo_free_locked(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo) +{ + struct radeon_sa_manager *sa_manager = sa_bo->manager; + struct list_head *prev; + + prev = sa_bo->list.prev; + list_del_init(&sa_bo->list); + if (list_empty(&sa_manager->sa_bo)) { + /* this bo was alone in the list */ + sa_manager->last = NULL; + } else if (sa_manager->last == sa_bo) { + if (prev == &sa_manager->sa_bo) { + /* sa_bo is begining of list, the new last became + * the last of the list + */ + sa_manager->last = list_entry(sa_manager->sa_bo.prev, struct radeon_sa_bo, list); + } else { + /* prev became the new last */ + sa_manager->last = list_entry(prev, struct radeon_sa_bo, list); + } + } +} + /* * 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 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. + * The last ptr serve as equivalent to read position in cp ring. + * last->prev is the previous last, while last->next is the oldest + * sa_bo allocated. * * Alignment can't be bigger than page size */ @@ -134,52 +156,65 @@ int radeon_sa_bo_new(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo, unsigned size, unsigned align) { - struct radeon_sa_bo *tmp; - struct list_head *head; - unsigned offset = 0, wasted = 0; + struct radeon_sa_bo *next, *oldest; + unsigned offset, wasted, hole_offset, hole_size; + bool try_begining = false, add_begining = false;

BUG_ON(align > RADEON_GPU_PAGE_SIZE); BUG_ON(size > sa_manager->size); - spin_lock(&sa_manager->lock);

- /* no one ? */ - if (list_empty(&sa_manager->sa_bo)) { - head = &sa_manager->sa_bo; + sa_bo->manager = sa_manager; + sa_bo->soffset = 0; + sa_bo->eoffset = 0; + sa_bo->size = 0; + INIT_LIST_HEAD(&sa_bo->list); + + spin_lock(&sa_manager->lock); + if (sa_manager->last == NULL) { + offset = 0; + add_begining = true; goto out; }

- /* 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; + hole_offset = sa_manager->last->eoffset; + wasted = (align - (hole_offset % align)) % align; + if (sa_manager->last->list.next == &sa_manager->sa_bo) { + /* no sa bo after that one */ + hole_size = sa_manager->size - hole_offset; + try_begining = true; + oldest = list_entry(sa_manager->sa_bo.next, struct radeon_sa_bo, list); + } else { + next = list_entry(sa_manager->last->list.next, struct radeon_sa_bo, list); + hole_size = next->soffset - hole_offset; + } + if ((size + wasted) >= hole_size) { + offset = hole_offset + wasted; + goto out; + } else if (try_begining) { + /* last was at end of list, so if we wrap over we might find + * room at the begining of the list + */ + offset = 0; + hole_size = oldest->soffset; + if (size >= hole_size) { + add_begining = true; goto out; } - offset = tmp->offset + tmp->size; - wasted = offset % align; - if (wasted) { - offset += align - wasted; - } - } - /* 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(&sa_manager->lock); - return -ENOMEM; }

+ spin_unlock(&sa_manager->lock); + return -ENOMEM; + out: - sa_bo->manager = sa_manager; - sa_bo->offset = offset; + if (add_begining) { + list_add(&sa_bo->list, &sa_manager->sa_bo); + } else { + list_add(&sa_bo->list, &sa_manager->last->list); + } + sa_manager->last = sa_bo; + sa_bo->soffset = offset; + sa_bo->eoffset = offset + size; sa_bo->size = size; - list_add(&sa_bo->list, head); spin_unlock(&sa_manager->lock); return 0; } @@ -189,7 +224,13 @@ void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo) struct radeon_sa_manager *sa_manager = sa_bo->manager;

spin_lock(&sa_manager->lock); - list_del_init(&sa_bo->list); + if (list_empty(&sa_bo->list)) { + /* it has already been free */ + goto out; + } + radeon_sa_bo_free_locked(rdev, sa_bo); + +out: spin_unlock(&sa_manager->lock); }

@@ -201,7 +242,8 @@ void radeon_sa_bo_dump_debug_info(struct radeon_sa_manager *sa_manager,

spin_lock(&sa_manager->lock); list_for_each_entry(i, &sa_manager->sa_bo, list) { - seq_printf(m, "offset %08d: size %4d\n", i->offset, i->size); + seq_printf(m, "[0x%08x 0x%08x]/0x%08x size %d\n", i->soffset, + i->eoffset, sa_manager->size, i->size); } spin_unlock(&sa_manager->lock); } diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c index 61dd4e3..c3763e4 100644 --- a/drivers/gpu/drm/radeon/radeon_semaphore.c +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c @@ -55,9 +55,9 @@ static int radeon_semaphore_add_bo(struct radeon_device *rdev) return r; } gpu_addr = rdev->ib_pool.sa_manager.gpu_addr; - gpu_addr += bo->ib->sa_bo.offset; + gpu_addr += bo->ib->sa_bo.soffset; cpu_ptr = rdev->ib_pool.sa_manager.cpu_ptr; - cpu_ptr += (bo->ib->sa_bo.offset >> 2); + cpu_ptr += (bo->ib->sa_bo.soffset >> 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;

From: Christian König deathsimple@vodafone.de

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

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

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index dc4f4f3..f1a9bd0 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 eb63a06..f6f9d6c 100644 --- a/drivers/gpu/drm/radeon/radeon_device.c +++ b/drivers/gpu/drm/radeon/radeon_device.c @@ -915,7 +915,7 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state) radeon_bo_evict_vram(rdev); /* wait for gpu to finish processing current batch */ for (i = 0; i < RADEON_NUM_RINGS; i++) - radeon_fence_wait_last(rdev, i); + radeon_fence_wait_empty(rdev, i);

radeon_save_bios_scratch_regs(rdev);

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index 2fbbc34..2d13843 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -297,7 +297,7 @@ int radeon_fence_wait_next(struct radeon_device *rdev, int ring) return r; }

-int radeon_fence_wait_last(struct radeon_device *rdev, int ring) +int radeon_fence_wait_empty(struct radeon_device *rdev, int ring) { unsigned long irq_flags; struct radeon_fence *fence; @@ -442,7 +442,7 @@ void radeon_fence_driver_fini(struct radeon_device *rdev) for (ring = 0; ring < RADEON_NUM_RINGS; ring++) { if (!rdev->fence_drv[ring].initialized) continue; - radeon_fence_wait_last(rdev, ring); + radeon_fence_wait_empty(rdev, ring); wake_up_all(&rdev->fence_drv[ring].queue); write_lock_irqsave(&rdev->fence_lock, irq_flags); radeon_scratch_free(rdev, rdev->fence_drv[ring].scratch_reg);

From: Jerome Glisse jglisse@redhat.com

This allow to associate a fence with sa bo and retry and wait if sa bo alloc can block.

v2: bug fixes

Signed-off-by: Jerome Glisse jglisse@redhat.com --- drivers/gpu/drm/radeon/radeon.h | 10 ++- drivers/gpu/drm/radeon/radeon_cs.c | 4 +- drivers/gpu/drm/radeon/radeon_gart.c | 14 ++-- drivers/gpu/drm/radeon/radeon_object.h | 10 ++-- drivers/gpu/drm/radeon/radeon_ring.c | 14 ++-- drivers/gpu/drm/radeon/radeon_sa.c | 102 ++++++++++++++++++++++++++--- drivers/gpu/drm/radeon/radeon_semaphore.c | 4 +- 7 files changed, 122 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 59bcfb9..4815ebe 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -379,6 +379,8 @@ struct radeon_bo_list { * Assumption is that there won't be hole (all object on same * alignment). */ +struct radeon_sa_bo; + struct radeon_sa_manager { spinlock_t lock; struct radeon_bo *bo; @@ -390,8 +392,6 @@ struct radeon_sa_manager { uint32_t domain; };

-struct radeon_sa_bo; - /* sub-allocation buffer */ struct radeon_sa_bo { struct list_head list; @@ -399,6 +399,8 @@ struct radeon_sa_bo { unsigned soffset; unsigned eoffset; unsigned size; + struct radeon_fence *fence; + bool free; };

/* @@ -626,7 +628,7 @@ void radeon_irq_kms_pflip_irq_put(struct radeon_device *rdev, int crtc); */

struct radeon_ib { - struct radeon_sa_bo sa_bo; + struct radeon_sa_bo *sa_bo; unsigned idx; uint32_t length_dw; uint64_t gpu_addr; @@ -680,7 +682,7 @@ struct radeon_vm { unsigned last_pfn; u64 pt_gpu_addr; u64 *pt; - struct radeon_sa_bo sa_bo; + struct radeon_sa_bo *sa_bo; struct mutex mutex; /* last fence for cs using this vm */ struct radeon_fence *fence; diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c index 8de6b3a..b39f22e 100644 --- a/drivers/gpu/drm/radeon/radeon_cs.c +++ b/drivers/gpu/drm/radeon/radeon_cs.c @@ -476,7 +476,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, /* ib pool is bind at 0 in virtual address space to gpu_addr is the * offset inside the pool bo */ - parser->const_ib->gpu_addr = parser->const_ib->sa_bo.soffset; + parser->const_ib->gpu_addr = parser->const_ib->sa_bo->soffset; r = radeon_ib_schedule(rdev, parser->const_ib); if (r) goto out; @@ -486,7 +486,7 @@ static int radeon_cs_ib_vm_chunk(struct radeon_device *rdev, /* ib pool is bind at 0 in virtual address space to gpu_addr is the * offset inside the pool bo */ - parser->ib->gpu_addr = parser->ib->sa_bo.soffset; + parser->ib->gpu_addr = parser->ib->sa_bo->soffset; parser->ib->is_const_ib = false; r = radeon_ib_schedule(rdev, parser->ib); out: diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 4a5d9d4..89328e3 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -393,19 +393,19 @@ 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); - if (r) { + vm->sa_bo = radeon_sa_bo_new(rdev, &rdev->vm_manager.sa_manager, + RADEON_GPU_PAGE_ALIGN(vm->last_pfn * 8), + RADEON_GPU_PAGE_SIZE, false, NULL); + if (vm->sa_bo == NULL) { if (list_empty(&rdev->vm_manager.lru_vm)) { - return r; + return -ENOMEM; } vm_evict = list_first_entry(&rdev->vm_manager.lru_vm, struct radeon_vm, list); radeon_vm_unbind(rdev, vm_evict); goto retry; } - vm->pt = radeon_sa_bo_cpu_addr(&vm->sa_bo); - vm->pt_gpu_addr = radeon_sa_bo_gpu_addr(&vm->sa_bo); + 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 99ab46a..7bbc319 100644 --- a/drivers/gpu/drm/radeon/radeon_object.h +++ b/drivers/gpu/drm/radeon/radeon_object.h @@ -166,12 +166,12 @@ extern int radeon_sa_bo_manager_start(struct radeon_device *rdev, struct radeon_sa_manager *sa_manager); extern int radeon_sa_bo_manager_suspend(struct radeon_device *rdev, struct radeon_sa_manager *sa_manager); -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); +extern struct radeon_sa_bo *radeon_sa_bo_new(struct radeon_device *rdev, + struct radeon_sa_manager *sa_manager, + unsigned size, unsigned align, + bool block, struct radeon_fence *fence); extern void radeon_sa_bo_free(struct radeon_device *rdev, - struct radeon_sa_bo *sa_bo); + 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); diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c index 981ab95..b646bdb 100644 --- a/drivers/gpu/drm/radeon/radeon_ring.c +++ b/drivers/gpu/drm/radeon/radeon_ring.c @@ -122,13 +122,12 @@ retry: 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); - if (!r) { + rdev->ib_pool.ibs[idx].sa_bo = radeon_sa_bo_new(rdev, &rdev->ib_pool.sa_manager, + size, 256, false, NULL); + if (rdev->ib_pool.ibs[idx].sa_bo) { *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)->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; @@ -146,6 +145,7 @@ retry: } /* this should be rare event, ie all ib scheduled none signaled yet. */ + r = -ENOMEM; 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); @@ -226,7 +226,7 @@ int radeon_ib_pool_init(struct radeon_device *rdev) 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); + rdev->ib_pool.ibs[i].sa_bo = NULL; } rdev->ib_pool.head_id = 0; rdev->ib_pool.ready = true; diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c index 63b0cd2..d7d7b7e 100644 --- a/drivers/gpu/drm/radeon/radeon_sa.c +++ b/drivers/gpu/drm/radeon/radeon_sa.c @@ -122,6 +122,12 @@ static void radeon_sa_bo_free_locked(struct radeon_device *rdev, struct radeon_s struct radeon_sa_manager *sa_manager = sa_bo->manager; struct list_head *prev;

+ if (sa_bo->fence) { + if (!radeon_fence_signaled(sa_bo->fence)) { + return; + } + radeon_fence_unref(&sa_bo->fence); + } prev = sa_bo->list.prev; list_del_init(&sa_bo->list); if (list_empty(&sa_manager->sa_bo)) { @@ -138,6 +144,25 @@ static void radeon_sa_bo_free_locked(struct radeon_device *rdev, struct radeon_s sa_manager->last = list_entry(prev, struct radeon_sa_bo, list); } } + /* in case try free already free the sa_bo but radeon_sa_bo_free + * wasn't yet call, the free bool protect us from freeing to + * early the structure + */ + if (sa_bo->free) { + kfree(sa_bo); + } +} + +static bool radeon_sa_manager_try_free(struct radeon_device *rdev, + struct radeon_sa_bo *oldest) +{ + if (oldest->fence && oldest->fence->emitted) { + if (radeon_fence_signaled(oldest->fence)) { + radeon_sa_bo_free_locked(rdev, oldest); + return true; + } + } + return false; }

/* @@ -151,25 +176,32 @@ static void radeon_sa_bo_free_locked(struct radeon_device *rdev, struct radeon_s * * 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) +struct radeon_sa_bo *radeon_sa_bo_new(struct radeon_device *rdev, + struct radeon_sa_manager *sa_manager, + unsigned size, unsigned align, + bool block, struct radeon_fence *fence) { - struct radeon_sa_bo *next, *oldest; + struct radeon_sa_bo *sa_bo, *next, *oldest; unsigned offset, wasted, hole_offset, hole_size; bool try_begining = false, add_begining = false;

BUG_ON(align > RADEON_GPU_PAGE_SIZE); BUG_ON(size > sa_manager->size);

+ sa_bo = kmalloc(sizeof(struct radeon_sa_bo), GFP_KERNEL); + if (sa_bo == NULL) { + return NULL; + } sa_bo->manager = sa_manager; + sa_bo->fence = NULL; + sa_bo->free = false; sa_bo->soffset = 0; sa_bo->eoffset = 0; sa_bo->size = 0; INIT_LIST_HEAD(&sa_bo->list);

spin_lock(&sa_manager->lock); +retry: if (sa_manager->last == NULL) { offset = 0; add_begining = true; @@ -186,6 +218,7 @@ int radeon_sa_bo_new(struct radeon_device *rdev, } else { next = list_entry(sa_manager->last->list.next, struct radeon_sa_bo, list); hole_size = next->soffset - hole_offset; + oldest = next; } if ((size + wasted) >= hole_size) { offset = hole_offset + wasted; @@ -201,9 +234,44 @@ int radeon_sa_bo_new(struct radeon_device *rdev, goto out; } } + /* try to be optimist and free the oldest one */ + if (radeon_sa_manager_try_free(rdev, oldest)) { + goto retry; + } + + /* if block is used all the sa_bo must be associated with a + * fence, we perform sanity check but expect things to go + * berserk if you don't follow this + */ + if (block) { + struct radeon_fence *fence = NULL; + int r; + + if (oldest->fence) { + fence = radeon_fence_ref(oldest->fence); + } + spin_unlock(&sa_manager->lock); + + if (fence == NULL) { + /* this should never happen */ + dev_warn(rdev->dev, "sa allocator nothing we can wait for\n"); + goto out_err; + }

+ r = radeon_fence_wait(fence, false); + radeon_fence_unref(&fence); + if (r) { + goto out_err; + } + + spin_lock(&sa_manager->lock); + goto retry; + } spin_unlock(&sa_manager->lock); - return -ENOMEM; + +out_err: + kfree(sa_bo); + return NULL;

out: if (add_begining) { @@ -212,22 +280,38 @@ out: list_add(&sa_bo->list, &sa_manager->last->list); } sa_manager->last = sa_bo; + if (fence) { + sa_bo->fence = radeon_fence_ref(fence); + } sa_bo->soffset = offset; sa_bo->eoffset = offset + size; sa_bo->size = size; spin_unlock(&sa_manager->lock); - return 0; + return sa_bo; }

-void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo *sa_bo) +void radeon_sa_bo_free(struct radeon_device *rdev, struct radeon_sa_bo **tmp) { - struct radeon_sa_manager *sa_manager = sa_bo->manager; + struct radeon_sa_bo *sa_bo; + struct radeon_sa_manager *sa_manager;

+ if (tmp == NULL || *tmp == NULL) { + return; + } + + sa_bo = *tmp; + sa_manager = sa_bo->manager; + *tmp = NULL; spin_lock(&sa_manager->lock); + sa_bo->free = true; if (list_empty(&sa_bo->list)) { /* it has already been free */ + kfree(sa_bo); goto out; } + if (sa_bo->fence && !sa_bo->fence->emitted) { + radeon_fence_unref(&sa_bo->fence); + } radeon_sa_bo_free_locked(rdev, sa_bo);

out: diff --git a/drivers/gpu/drm/radeon/radeon_semaphore.c b/drivers/gpu/drm/radeon/radeon_semaphore.c index c3763e4..d79afb3 100644 --- a/drivers/gpu/drm/radeon/radeon_semaphore.c +++ b/drivers/gpu/drm/radeon/radeon_semaphore.c @@ -55,9 +55,9 @@ static int radeon_semaphore_add_bo(struct radeon_device *rdev) return r; } gpu_addr = rdev->ib_pool.sa_manager.gpu_addr; - gpu_addr += bo->ib->sa_bo.soffset; + gpu_addr += bo->ib->sa_bo->soffset; cpu_ptr = rdev->ib_pool.sa_manager.cpu_ptr; - cpu_ptr += (bo->ib->sa_bo.soffset >> 2); + cpu_ptr += (bo->ib->sa_bo->soffset >> 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;