[PATCH 1/3] drm/radeon: fix VMID use tracking

List overview All Threads
Download

newer

older

[PATCH RFC 0/9] Beaglebone-Black...

...

Christian König

23 Jan 2014 23 Jan '14

1:24 p.m.

From: Christian König christian.koenig@amd.com

Otherwise we allocate a new VMID on nearly every submit.

Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon.h | 2 ++ drivers/gpu/drm/radeon/radeon_gart.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index c5519ca..4a8ac1c 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -867,6 +867,8 @@ struct radeon_vm { struct radeon_fence *fence; /* last flush or NULL if we still need to flush */ struct radeon_fence *last_flush; + /* last use of vmid */ + struct radeon_fence *last_id_use; };

struct radeon_vm_manager { diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 96e4400..0e9143b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -713,7 +713,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, unsigned i;

/* check if the id is still valid */ - if (vm->fence && vm->fence == rdev->vm_manager.active[vm->id]) + if (vm->last_id_use && vm->last_id_use == rdev->vm_manager.active[vm->id]) return NULL;

/* we definately need to flush */ @@ -769,6 +769,9 @@ void radeon_vm_fence(struct radeon_device *rdev,

radeon_fence_unref(&vm->fence); vm->fence = radeon_fence_ref(fence); + + radeon_fence_unref(&vm->last_id_use); + vm->last_id_use = radeon_fence_ref(fence); }

/** @@ -1303,6 +1306,8 @@ void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) { vm->id = 0; vm->fence = NULL; + vm->last_flush = NULL; + vm->last_id_use = NULL; mutex_init(&vm->mutex); INIT_LIST_HEAD(&vm->list); INIT_LIST_HEAD(&vm->va); @@ -1341,5 +1346,6 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm) } radeon_fence_unref(&vm->fence); radeon_fence_unref(&vm->last_flush); + radeon_fence_unref(&vm->last_id_use); mutex_unlock(&vm->mutex); }

-- 1.8.1.2

Show replies by date

Christian König

23 Jan 23 Jan

1:24 p.m.

New subject: [PATCH 2/3] drm/radeon: add missing trace point

From: Christian König christian.koenig@amd.com

Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon_gart.c | 1 + 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 0e9143b..a8f9b46 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -726,6 +726,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, if (fence == NULL) { /* found a free one */ vm->id = i; + trace_radeon_vm_grab_id(vm->id, ring); return NULL; }

-- 1.8.1.2

Christian König

1:24 p.m.

New subject: [PATCH 3/3] drm/radeon: add ring to fence trace functions

From: Christian König christian.koenig@amd.com

Signed-off-by: Christian König christian.koenig@amd.com --- drivers/gpu/drm/radeon/radeon_fence.c | 6 +++--- drivers/gpu/drm/radeon/radeon_trace.h | 21 ++++++++++++--------- 2 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c index 866744e..c37cb79 100644 --- a/drivers/gpu/drm/radeon/radeon_fence.c +++ b/drivers/gpu/drm/radeon/radeon_fence.c @@ -121,7 +121,7 @@ int radeon_fence_emit(struct radeon_device *rdev, (*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); + trace_radeon_fence_emit(rdev->ddev, ring, (*fence)->seq); return 0; }

@@ -313,7 +313,7 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq, continue;

last_seq[i] = atomic64_read(&rdev->fence_drv[i].last_seq); - trace_radeon_fence_wait_begin(rdev->ddev, target_seq[i]); + trace_radeon_fence_wait_begin(rdev->ddev, i, target_seq[i]); radeon_irq_kms_sw_irq_get(rdev, i); }

@@ -332,7 +332,7 @@ static int radeon_fence_wait_seq(struct radeon_device *rdev, u64 *target_seq, continue;

radeon_irq_kms_sw_irq_put(rdev, i); - trace_radeon_fence_wait_end(rdev->ddev, target_seq[i]); + trace_radeon_fence_wait_end(rdev->ddev, i, target_seq[i]); }

if (unlikely(r < 0)) diff --git a/drivers/gpu/drm/radeon/radeon_trace.h b/drivers/gpu/drm/radeon/radeon_trace.h index 0473257..f749f2c 100644 --- a/drivers/gpu/drm/radeon/radeon_trace.h +++ b/drivers/gpu/drm/radeon/radeon_trace.h @@ -106,42 +106,45 @@ TRACE_EVENT(radeon_vm_set_page,

DECLARE_EVENT_CLASS(radeon_fence_request,

- TP_PROTO(struct drm_device *dev, u32 seqno), + TP_PROTO(struct drm_device *dev, int ring, u32 seqno),

- TP_ARGS(dev, seqno), + TP_ARGS(dev, ring, seqno),

TP_STRUCT__entry( __field(u32, dev) + __field(int, ring) __field(u32, seqno) ),

TP_fast_assign( __entry->dev = dev->primary->index; + __entry->ring = ring; __entry->seqno = seqno; ),

- TP_printk("dev=%u, seqno=%u", __entry->dev, __entry->seqno) + TP_printk("dev=%u, ring=%d, seqno=%u", + __entry->dev, __entry->ring, __entry->seqno) );

DEFINE_EVENT(radeon_fence_request, radeon_fence_emit,

- TP_PROTO(struct drm_device *dev, u32 seqno), + TP_PROTO(struct drm_device *dev, int ring, u32 seqno),

- TP_ARGS(dev, seqno) + TP_ARGS(dev, ring, seqno) );

DEFINE_EVENT(radeon_fence_request, radeon_fence_wait_begin,

- TP_PROTO(struct drm_device *dev, u32 seqno), + TP_PROTO(struct drm_device *dev, int ring, u32 seqno),

- TP_ARGS(dev, seqno) + TP_ARGS(dev, ring, seqno) );

DEFINE_EVENT(radeon_fence_request, radeon_fence_wait_end,

- TP_PROTO(struct drm_device *dev, u32 seqno), + TP_PROTO(struct drm_device *dev, int ring, u32 seqno),

- TP_ARGS(dev, seqno) + TP_ARGS(dev, ring, seqno) );

DECLARE_EVENT_CLASS(radeon_semaphore_request,

-- 1.8.1.2

Alex Deucher

5:46 p.m.

On Thu, Jan 23, 2014 at 8:24 AM, Christian König deathsimple@vodafone.de wrote:

...

From: Christian König christian.koenig@amd.com

Otherwise we allocate a new VMID on nearly every submit.

I wonder if this fix would allow us to fix up this: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=46...

Alex

...

Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/radeon/radeon.h | 2 ++ drivers/gpu/drm/radeon/radeon_gart.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index c5519ca..4a8ac1c 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -867,6 +867,8 @@ struct radeon_vm { struct radeon_fence *fence; /* last flush or NULL if we still need to flush */ struct radeon_fence *last_flush;
  /* last use of vmid */
  struct radeon_fence             *last_id_use;
};

struct radeon_vm_manager { diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 96e4400..0e9143b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -713,7 +713,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, unsigned i;
    /* check if the id is still valid */
  if (vm->fence && vm->fence == rdev->vm_manager.active[vm->id])
  if (vm->last_id_use && vm->last_id_use == rdev->vm_manager.active[vm->id])
          return NULL;

  /* we definately need to flush */
@@ -769,6 +769,9 @@ void radeon_vm_fence(struct radeon_device *rdev,
    radeon_fence_unref(&vm->fence);
    vm->fence = radeon_fence_ref(fence);
  radeon_fence_unref(&vm->last_id_use);
  vm->last_id_use = radeon_fence_ref(fence);
}

/** @@ -1303,6 +1306,8 @@ void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) { vm->id = 0; vm->fence = NULL;
  vm->last_flush = NULL;
  vm->last_id_use = NULL;
  mutex_init(&vm->mutex);
  INIT_LIST_HEAD(&vm->list);
  INIT_LIST_HEAD(&vm->va);
@@ -1341,5 +1346,6 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm) } radeon_fence_unref(&vm->fence); radeon_fence_unref(&vm->last_flush);
  radeon_fence_unref(&vm->last_id_use);
  mutex_unlock(&vm->mutex);
}

1.8.1.2

Christian König

24 Jan 24 Jan

3:52 p.m.

Am 23.01.2014 18:46, schrieb Alex Deucher:

...

On Thu, Jan 23, 2014 at 8:24 AM, Christian König deathsimple@vodafone.de wrote:

...
From: Christian König christian.koenig@amd.com

Otherwise we allocate a new VMID on nearly every submit.

I wonder if this fix would allow us to fix up this: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=46...

It's probably a step into the right direction, but not necessary the complete solution.

For now I would say we should keep the extra flushing as it is.

Christian.

...

Alex

...
Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/radeon/radeon.h | 2 ++ drivers/gpu/drm/radeon/radeon_gart.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index c5519ca..4a8ac1c 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -867,6 +867,8 @@ struct radeon_vm { struct radeon_fence *fence; /* last flush or NULL if we still need to flush */ struct radeon_fence *last_flush;
  /* last use of vmid */
  struct radeon_fence             *last_id_use;
};

struct radeon_vm_manager {
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 96e4400..0e9143b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -713,7 +713,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, unsigned i;
     /* check if the id is still valid */
  if (vm->fence && vm->fence == rdev->vm_manager.active[vm->id])
  if (vm->last_id_use && vm->last_id_use == rdev->vm_manager.active[vm->id])
           return NULL;

   /* we definately need to flush */
@@ -769,6 +769,9 @@ void radeon_vm_fence(struct radeon_device *rdev,
     radeon_fence_unref(&vm->fence);
     vm->fence = radeon_fence_ref(fence);
  radeon_fence_unref(&vm->last_id_use);
  vm->last_id_use = radeon_fence_ref(fence);
}

/**
@@ -1303,6 +1306,8 @@ void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) { vm->id = 0; vm->fence = NULL;
  vm->last_flush = NULL;
  vm->last_id_use = NULL;
   mutex_init(&vm->mutex);
   INIT_LIST_HEAD(&vm->list);
   INIT_LIST_HEAD(&vm->va);
@@ -1341,5 +1346,6 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm) } radeon_fence_unref(&vm->fence); radeon_fence_unref(&vm->last_flush);
  radeon_fence_unref(&vm->last_id_use);
   mutex_unlock(&vm->mutex);
}
-- 1.8.1.2

Alex Deucher

3:55 p.m.

On Fri, Jan 24, 2014 at 10:52 AM, Christian König deathsimple@vodafone.de wrote:

...

Am 23.01.2014 18:46, schrieb Alex Deucher:

...
On Thu, Jan 23, 2014 at 8:24 AM, Christian König deathsimple@vodafone.de wrote:

...
From: Christian König christian.koenig@amd.com

Otherwise we allocate a new VMID on nearly every submit.

I wonder if this fix would allow us to fix up this:

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=46...

It's probably a step into the right direction, but not necessary the complete solution.

For now I would say we should keep the extra flushing as it is.

Patches applied. thanks!

Alex

...

Christian.

...
Alex

...
Signed-off-by: Christian König christian.koenig@amd.com

drivers/gpu/drm/radeon/radeon.h | 2 ++ drivers/gpu/drm/radeon/radeon_gart.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h index c5519ca..4a8ac1c 100644 --- a/drivers/gpu/drm/radeon/radeon.h +++ b/drivers/gpu/drm/radeon/radeon.h @@ -867,6 +867,8 @@ struct radeon_vm { struct radeon_fence *fence; /* last flush or NULL if we still need to flush */ struct radeon_fence *last_flush;
  /* last use of vmid */
  struct radeon_fence             *last_id_use;
};

struct radeon_vm_manager {
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c index 96e4400..0e9143b 100644 --- a/drivers/gpu/drm/radeon/radeon_gart.c +++ b/drivers/gpu/drm/radeon/radeon_gart.c @@ -713,7 +713,7 @@ struct radeon_fence *radeon_vm_grab_id(struct radeon_device *rdev, unsigned i;
     /* check if the id is still valid */
  if (vm->fence && vm->fence == rdev->vm_manager.active[vm->id])
  if (vm->last_id_use && vm->last_id_use ==
rdev->vm_manager.active[vm->id]) return NULL;
     /* we definately need to flush */
@@ -769,6 +769,9 @@ void radeon_vm_fence(struct radeon_device *rdev,
     radeon_fence_unref(&vm->fence);
     vm->fence = radeon_fence_ref(fence);
  radeon_fence_unref(&vm->last_id_use);
  vm->last_id_use = radeon_fence_ref(fence);
}

/**
@@ -1303,6 +1306,8 @@ void radeon_vm_init(struct radeon_device *rdev, struct radeon_vm *vm) { vm->id = 0; vm->fence = NULL;
  vm->last_flush = NULL;
  vm->last_id_use = NULL;
   mutex_init(&vm->mutex);
   INIT_LIST_HEAD(&vm->list);
   INIT_LIST_HEAD(&vm->va);
@@ -1341,5 +1346,6 @@ void radeon_vm_fini(struct radeon_device *rdev, struct radeon_vm *vm) } radeon_fence_unref(&vm->fence); radeon_fence_unref(&vm->last_flush);
  radeon_fence_unref(&vm->last_id_use);
   mutex_unlock(&vm->mutex);
}
-- 1.8.1.2

4130

Age (days ago)

4131

Last active (days ago)

dri-devel@lists.freedesktop.org

5 comments

2 participants

tags (0)

participants (2)

Alex Deucher
Christian König