Re: [PATCH 02/10] drm/radeon: add infrastructure for advanced ring synchronization

On Thu, May 24, 2012 at 09:49:06AM +0200, Christian König wrote:

Signed-off-by: Christian König deathsimple@vodafone.de

Need a small improvement see below, otherwise

Reviewed-by: Jerome Glisse jglisse@redhat.com

---

drivers/gpu/drm/radeon/radeon.h | 23 ++++++++++- drivers/gpu/drm/radeon/radeon_fence.c | 73 +++++++++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index 5e259b4..4e232c3 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -257,8 +257,8 @@ struct radeon_fence_driver { uint32_t scratch_reg; uint64_t gpu_addr; volatile uint32_t *cpu_addr;

• /* seq is protected by ring emission lock */
• uint64_t seq;

• /* sync_seq is protected by ring emission lock */
• uint64_t sync_seq[RADEON_NUM_RINGS]; atomic64_t last_seq; unsigned long last_activity; bool initialized;

@@ -288,6 +288,25 @@ int radeon_fence_wait_any(struct radeon_device *rdev, struct radeon_fence *radeon_fence_ref(struct radeon_fence *fence); void radeon_fence_unref(struct radeon_fence **fence); unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring); +bool radeon_fence_need_sync(struct radeon_fence *fence, int ring); +void radeon_fence_note_sync(struct radeon_fence *fence, int ring); +static inline struct radeon_fence *radeon_fence_later(struct radeon_fence *a,

• 				      struct radeon_fence *b)
  

+{

• if (!a) {

• return b;
  

• }
• if (!b) {

• return a;
  

• }

Please add : BUG_ON(a->ring != b->ring);

So we can catch if someone badly use this function.

• if (a->seq > b->seq) {

• return a;
  

• } else {

• return b;
  

• }

+}

/*

• Tiling registers

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index 401d346..7b55625 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -72,7 +72,7 @@ int radeon_fence_emit(struct radeon_device *rdev, } kref_init(&((*fence)->kref)); (*fence)->rdev = rdev;

• (*fence)->seq = ++rdev->fence_drv[ring].seq;

• (*fence)->seq = ++rdev->fence_drv[ring].sync_seq[ring]; (*fence)->ring = ring; radeon_fence_ring_emit(rdev, ring, *fence); trace_radeon_fence_emit(rdev->ddev, (*fence)->seq);

@@ -449,7 +449,7 @@ int radeon_fence_wait_next_locked(struct radeon_device *rdev, int ring) * wait. */ seq = atomic64_read(&rdev->fence_drv[ring].last_seq) + 1ULL;

• if (seq >= rdev->fence_drv[ring].seq) {

• if (seq >= rdev->fence_drv[ring].sync_seq[ring]) { /* nothing to wait for, last_seq is already the last emited fence */ return -ENOENT;

@@ -464,7 +464,7 @@ int radeon_fence_wait_empty_locked(struct radeon_device *rdev, int ring) * activity can be scheduled so there won't be concurrent access * to seq value. */

• return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].seq,

• return radeon_fence_wait_seq(rdev, rdev->fence_drv[ring].sync_seq[ring], ring, false, false);

}

@@ -492,7 +492,8 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) * but it's ok to report slightly wrong fence count here. */ radeon_fence_process(rdev, ring);

• emitted = rdev->fence_drv[ring].seq - atomic64_read(&rdev->fence_drv[ring].last_seq);

• emitted = rdev->fence_drv[ring].sync_seq[ring]

• - atomic64_read(&rdev->fence_drv[ring].last_seq);
  

  /* to avoid 32bits warp around */ if (emitted > 0x10000000) { emitted = 0x10000000;

@@ -500,6 +501,51 @@ unsigned radeon_fence_count_emitted(struct radeon_device *rdev, int ring) return (unsigned)emitted; }

+bool radeon_fence_need_sync(struct radeon_fence *fence, int dst_ring) +{

• struct radeon_fence_driver *fdrv;
• if (!fence) {

• return false;
  

• }
• if (fence->ring == dst_ring) {

• return false;
  

• }
• /* we are protected by the ring mutex */
• fdrv = &fence->rdev->fence_drv[dst_ring];
• if (fence->seq <= fdrv->sync_seq[fence->ring]) {

• return false;
  

• }
• return true;

+}

+void radeon_fence_note_sync(struct radeon_fence *fence, int dst_ring) +{

• struct radeon_fence_driver *dst, *src;
• unsigned i;
• if (!fence) {

• return;
  

• }
• if (fence->ring == dst_ring) {

• return;
  

• }
• /* we are protected by the ring mutex */
• src = &fence->rdev->fence_drv[fence->ring];
• dst = &fence->rdev->fence_drv[dst_ring];
• for (i = 0; i < RADEON_NUM_RINGS; ++i) {

• if (i == dst_ring) {
  

• 	continue;
  

• }
  

• dst->sync_seq[i] = max(dst->sync_seq[i], src->sync_seq[i]);
  

• }

+}

int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring) { uint64_t index; @@ -521,7 +567,7 @@ int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring) } rdev->fence_drv[ring].cpu_addr = &rdev->wb.wb[index/4]; rdev->fence_drv[ring].gpu_addr = rdev->wb.gpu_addr + index;

• radeon_fence_write(rdev, rdev->fence_drv[ring].seq, ring);

• radeon_fence_write(rdev, rdev->fence_drv[ring].sync_seq[ring], ring); rdev->fence_drv[ring].initialized = true; dev_info(rdev->dev, "fence driver on ring %d use gpu addr 0x%016llx and cpu addr 0x%p\n", ring, rdev->fence_drv[ring].gpu_addr, rdev->fence_drv[ring].cpu_addr);

@@ -530,10 +576,13 @@ int radeon_fence_driver_start_ring(struct radeon_device *rdev, int ring)

static void radeon_fence_driver_init_ring(struct radeon_device *rdev, int ring) {

• int i;
• rdev->fence_drv[ring].scratch_reg = -1; rdev->fence_drv[ring].cpu_addr = NULL; rdev->fence_drv[ring].gpu_addr = 0;

• rdev->fence_drv[ring].seq = 0;

• for (i = 0; i < RADEON_NUM_RINGS; ++i)

• rdev->fence_drv[ring].sync_seq[i] = 0;
  

  atomic64_set(&rdev->fence_drv[ring].last_seq, 0); rdev->fence_drv[ring].last_activity = jiffies; rdev->fence_drv[ring].initialized = false;

@@ -579,7 +628,7 @@ static int radeon_debugfs_fence_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;

• int i;

• int i, j;

  for (i = 0; i < RADEON_NUM_RINGS; ++i) { if (!rdev->fence_drv[i].initialized)

@@ -588,8 +637,14 @@ static int radeon_debugfs_fence_info(struct seq_file *m, void *data) seq_printf(m, "--- ring %d ---\n", i); seq_printf(m, "Last signaled fence 0x%016llx\n", (unsigned long long)atomic64_read(&rdev->fence_drv[i].last_seq));

• seq_printf(m, "Last emitted  0x%016llx\n",
  

• 	   rdev->fence_drv[i].seq);
  

• seq_printf(m, "Last emitted        0x%016llx\n",
  

• 	   rdev->fence_drv[i].sync_seq[i]);
  

• for (j = 0; j < RADEON_NUM_RINGS; ++j) {
  

• 	if (i != j && rdev->fence_drv[j].initialized)
  

• 		seq_printf(m, "Last sync to ring %d 0x%016llx\n",
  

• 			   j, rdev->fence_drv[i].sync_seq[j]);
  

• }
  

  } return 0;

}

1.7.9.5