date:20170426

Re: [PATCH 5/6] drm/amdgpu: implement grab reserved vmid V3

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/27/2017 01:00 PM, Chunming Zhou wrote:

v2: move sync waiting only when flush needs
v3: fix racy

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 72 --
  1 file changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e6fdfa4..7752279 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,65 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device 
*adev,
atomic_read(>gpu_reset_counter);
  }

+static bool amdgpu_vm_reserved_vmid_ready(struct amdgpu_vm *vm, unsigned vmhub)
+{
+   return !!vm->reserved_vmid[vmhub];
+}


It may be better to populate it into alloc reserved_vmid func as well, getting 
easy maintenance in the future.




+
+/* idr_mgr->lock must be held */
+static int amdgpu_vm_grab_reserved_vmid_locked(struct amdgpu_vm *vm,
+  struct amdgpu_ring *ring,
+  struct amdgpu_sync *sync,
+  struct fence *fence,
+  struct amdgpu_job *job)
+{
+   struct amdgpu_device *adev = ring->adev;
+   unsigned vmhub = ring->funcs->vmhub;
+   struct amdgpu_vm_id *id = vm->reserved_vmid[vmhub];
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+   struct fence *updates = sync->last_vm_update;
+   int r = 0;
+   struct fence *flushed, *tmp;
+   bool needs_flush = false;
+
+   flushed  = id->flushed_updates;
+   if ((amdgpu_vm_had_gpu_reset(adev, id)) ||
+   (atomic64_read(>owner) != vm->client_id) ||
+   (job->vm_pd_addr != id->pd_gpu_addr) ||
+   (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed {
+   needs_flush = true;
+   tmp = amdgpu_sync_get_fence(>active);
+   if (tmp) {
+   r = amdgpu_sync_fence(adev, sync, tmp);
+   fence_put(tmp);
+   return r;


Sorry, I didn't catch up this.
Could you give me some tips?
(in this case, we no need to update id info as below?)

Jerry


+   }
+   }
+
+   /* Good we can use this VMID. Remember this submission as
+   * user of the VMID.
+   */
+   r = amdgpu_sync_fence(ring->adev, >active, fence);
+   if (r)
+   goto out;
+
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed))) {
+   fence_put(id->flushed_updates);
+   id->flushed_updates = fence_get(updates);
+   }
+   id->pd_gpu_addr = job->vm_pd_addr;
+   id->current_gpu_reset_count = atomic_read(>gpu_reset_counter);
+   atomic64_set(>owner, vm->client_id);
+   job->vm_needs_flush = needs_flush;
+
+   job->vm_id = id - id_mgr->ids;
+   trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+   return r;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -421,12 +480,17 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
unsigned i;
int r = 0;

+   mutex_lock(_mgr->lock);
+   if (amdgpu_vm_reserved_vmid_ready(vm, vmhub)) {
+   r = amdgpu_vm_grab_reserved_vmid_locked(vm, ring, sync, fence, 
job);
+   mutex_unlock(_mgr->lock);
+   return r;
+   }
fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, GFP_KERNEL);
-   if (!fences)
+   if (!fences) {
+   mutex_unlock(_mgr->lock);
return -ENOMEM;
-
-   mutex_lock(_mgr->lock);
-
+   }
/* Check if we have an idle VMID */
i = 0;
list_for_each_entry(idle, _mgr->ids_lru, list) {


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v4

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/27/2017 01:00 PM, Chunming Zhou wrote:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub
v4:
fix racy condition

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 62 +++---
  1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 261aaea..e37421e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -546,6 +546,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
return r;
  }

+static void amdgpu_vm_free_reserved_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   if (vm->reserved_vmid[vmhub]) {
+   list_add(>reserved_vmid[vmhub]->list,
+   _mgr->ids_lru);
+   vm->reserved_vmid[vmhub] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev,
+struct amdgpu_vm *vm,
+unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr;
+   struct amdgpu_vm_id *idle;
+   int r = 0;
+
+   id_mgr = >vm_manager.id_mgr[vmhub];
+   mutex_lock(_mgr->lock);
+   if (vm->reserved_vmid[vmhub])
+   goto unlock;
+   /* Select the first entry VMID */
+   idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+   list_del_init(>list);


Could you give a hint how to handle the busy id, what if it's not idle when 
allocating the reserved vmid?


Jerry


+   vm->reserved_vmid[vmhub] = idle;
+   mutex_unlock(_mgr->lock);
+
+   return 0;
+unlock:
+   mutex_unlock(_mgr->lock);
+   return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2355,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)

amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
-   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-   struct amdgpu_vm_id_manager *id_mgr =
-   >vm_manager.id_mgr[i];
-
-   mutex_lock(_mgr->lock);
-   if (vm->reserved_vmid[i]) {
-   list_add(>reserved_vmid[i]->list,
-_mgr->ids_lru);
-   vm->reserved_vmid[i] = NULL;
-   }
-   mutex_unlock(_mgr->lock);
-   }
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   amdgpu_vm_free_reserved_vmid(adev, vm, i);
  }

  /**
@@ -2400,11 +2429,18 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   int r;

switch (args->in.op) {
case AMDGPU_VM_OP_RESERVE_VMID:
+   /* current, we only have requirement to reserve vmid from 
gfxhub */
+   r = amdgpu_vm_alloc_reserved_vmid(adev, >vm,
+ AMDGPU_GFXHUB);
+   if (r)
+   return r;
+   break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
-   return -EINVAL;
+   amdgpu_vm_free_reserved_vmid(adev, >vm, AMDGPU_GFXHUB);
break;
default:
return -EINVAL;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/6] drm/amdgpu: implement grab reserved vmid V3

2017-04-26 Thread Chunming Zhou

v2: move sync waiting only when flush needs
v3: fix racy

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 72 --
 1 file changed, 68 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e6fdfa4..7752279 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,65 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device 
*adev,
atomic_read(>gpu_reset_counter);
 }
 
+static bool amdgpu_vm_reserved_vmid_ready(struct amdgpu_vm *vm, unsigned vmhub)
+{
+   return !!vm->reserved_vmid[vmhub];
+}
+
+/* idr_mgr->lock must be held */
+static int amdgpu_vm_grab_reserved_vmid_locked(struct amdgpu_vm *vm,
+  struct amdgpu_ring *ring,
+  struct amdgpu_sync *sync,
+  struct fence *fence,
+  struct amdgpu_job *job)
+{
+   struct amdgpu_device *adev = ring->adev;
+   unsigned vmhub = ring->funcs->vmhub;
+   struct amdgpu_vm_id *id = vm->reserved_vmid[vmhub];
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+   struct fence *updates = sync->last_vm_update;
+   int r = 0;
+   struct fence *flushed, *tmp;
+   bool needs_flush = false;
+
+   flushed  = id->flushed_updates;
+   if ((amdgpu_vm_had_gpu_reset(adev, id)) ||
+   (atomic64_read(>owner) != vm->client_id) ||
+   (job->vm_pd_addr != id->pd_gpu_addr) ||
+   (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed {
+   needs_flush = true;
+   tmp = amdgpu_sync_get_fence(>active);
+   if (tmp) {
+   r = amdgpu_sync_fence(adev, sync, tmp);
+   fence_put(tmp);
+   return r;
+   }
+   }
+
+   /* Good we can use this VMID. Remember this submission as
+   * user of the VMID.
+   */
+   r = amdgpu_sync_fence(ring->adev, >active, fence);
+   if (r)
+   goto out;
+
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed))) {
+   fence_put(id->flushed_updates);
+   id->flushed_updates = fence_get(updates);
+   }
+   id->pd_gpu_addr = job->vm_pd_addr;
+   id->current_gpu_reset_count = atomic_read(>gpu_reset_counter);
+   atomic64_set(>owner, vm->client_id);
+   job->vm_needs_flush = needs_flush;
+
+   job->vm_id = id - id_mgr->ids;
+   trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+   return r;
+}
+
 /**
  * amdgpu_vm_grab_id - allocate the next free VMID
  *
@@ -421,12 +480,17 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
unsigned i;
int r = 0;
 
+   mutex_lock(_mgr->lock);
+   if (amdgpu_vm_reserved_vmid_ready(vm, vmhub)) {
+   r = amdgpu_vm_grab_reserved_vmid_locked(vm, ring, sync, fence, 
job);
+   mutex_unlock(_mgr->lock);
+   return r;
+   }
fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, GFP_KERNEL);
-   if (!fences)
+   if (!fences) {
+   mutex_unlock(_mgr->lock);
return -ENOMEM;
-
-   mutex_lock(_mgr->lock);
-
+   }
/* Check if we have an idle VMID */
i = 0;
list_for_each_entry(idle, _mgr->ids_lru, list) {
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 6/6] drm/amdgpu: bump module verion for reserved vmid

2017-04-26 Thread Chunming Zhou

Change-Id: I1065e0430ed44f7ee6c29214b72e35a7343ea02b
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 55322b4..6799829 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -64,9 +64,10 @@
  * - 3.12.0 - Add query for double offchip LDS buffers
  * - 3.13.0 - Add PRT support
  * - 3.14.0 - Fix race in amdgpu_ctx_get_fence() and note new functionality
+ * - 3.15.0 - Add reserved vmid support
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   14
+#define KMS_DRIVER_MINOR   15
 #define KMS_DRIVER_PATCHLEVEL  0
 
 int amdgpu_vram_limit = 0;
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/6] drm/amdgpu: add limitation for dedicated vm number v4

2017-04-26 Thread Chunming Zhou

v2: move #define to amdgpu_vm.h
v3: move reserved vmid counter to id_manager,
and increase counter before allocating vmid
v4: rename to reserved_vmid_num

Change-Id: Ie5958cf6dbdc1c8278e61d9158483472d6f5c6e3
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index e37421e..e6fdfa4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -557,6 +557,7 @@ static void amdgpu_vm_free_reserved_vmid(struct 
amdgpu_device *adev,
list_add(>reserved_vmid[vmhub]->list,
_mgr->ids_lru);
vm->reserved_vmid[vmhub] = NULL;
+   atomic_dec(_mgr->reserved_vmid_num);
}
mutex_unlock(_mgr->lock);
 }
@@ -573,6 +574,13 @@ static int amdgpu_vm_alloc_reserved_vmid(struct 
amdgpu_device *adev,
mutex_lock(_mgr->lock);
if (vm->reserved_vmid[vmhub])
goto unlock;
+   if (atomic_inc_return(_mgr->reserved_vmid_num) >
+   AMDGPU_VM_MAX_RESERVED_VMID) {
+   DRM_ERROR("Over limitation of reserved vmid\n");
+   atomic_dec(_mgr->reserved_vmid_num);
+   r = -EINVAL;
+   goto unlock;
+   }
/* Select the first entry VMID */
idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
list_del_init(>list);
@@ -2376,6 +2384,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 
mutex_init(_mgr->lock);
INIT_LIST_HEAD(_mgr->ids_lru);
+   atomic_set(_mgr->reserved_vmid_num, 0);
 
/* skip over VMID 0, since it is the system VM */
for (j = 1; j < id_mgr->num_ids; ++j) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 8eedca1..9828fcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -84,6 +84,8 @@
 
 /* hardcode that limit for now */
 #define AMDGPU_VA_RESERVED_SIZE(8 << 20)
+/* max vmids dedicated for process */
+#define AMDGPU_VM_MAX_RESERVED_VMID1
 
 struct amdgpu_vm_pt {
struct amdgpu_bo*bo;
@@ -157,6 +159,7 @@ struct amdgpu_vm_id_manager {
unsignednum_ids;
struct list_headids_lru;
struct amdgpu_vm_id ids[AMDGPU_NUM_VM];
+   atomic_treserved_vmid_num;
 };
 
 struct amdgpu_vm_manager {
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v4

2017-04-26 Thread Chunming Zhou

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub
v4:
fix racy condition

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 62 +++---
 1 file changed, 49 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 261aaea..e37421e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -546,6 +546,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
return r;
 }
 
+static void amdgpu_vm_free_reserved_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   if (vm->reserved_vmid[vmhub]) {
+   list_add(>reserved_vmid[vmhub]->list,
+   _mgr->ids_lru);
+   vm->reserved_vmid[vmhub] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_reserved_vmid(struct amdgpu_device *adev,
+struct amdgpu_vm *vm,
+unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr;
+   struct amdgpu_vm_id *idle;
+   int r = 0;
+
+   id_mgr = >vm_manager.id_mgr[vmhub];
+   mutex_lock(_mgr->lock);
+   if (vm->reserved_vmid[vmhub])
+   goto unlock;
+   /* Select the first entry VMID */
+   idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+   list_del_init(>list);
+   vm->reserved_vmid[vmhub] = idle;
+   mutex_unlock(_mgr->lock);
+
+   return 0;
+unlock:
+   mutex_unlock(_mgr->lock);
+   return r;
+}
+
 static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
 {
struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2355,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
-   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-   struct amdgpu_vm_id_manager *id_mgr =
-   >vm_manager.id_mgr[i];
-
-   mutex_lock(_mgr->lock);
-   if (vm->reserved_vmid[i]) {
-   list_add(>reserved_vmid[i]->list,
-_mgr->ids_lru);
-   vm->reserved_vmid[i] = NULL;
-   }
-   mutex_unlock(_mgr->lock);
-   }
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   amdgpu_vm_free_reserved_vmid(adev, vm, i);
 }
 
 /**
@@ -2400,11 +2429,18 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   int r;
 
switch (args->in.op) {
case AMDGPU_VM_OP_RESERVE_VMID:
+   /* current, we only have requirement to reserve vmid from 
gfxhub */
+   r = amdgpu_vm_alloc_reserved_vmid(adev, >vm,
+ AMDGPU_GFXHUB);
+   if (r)
+   return r;
+   break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
-   return -EINVAL;
+   amdgpu_vm_free_reserved_vmid(adev, >vm, AMDGPU_GFXHUB);
break;
default:
return -EINVAL;
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/6] drm/amdgpu: add reserved vmid field in vm struct v2

2017-04-26 Thread Chunming Zhou

v2: rename dedicated_vmid to reserved_vmid

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8698177..261aaea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2163,10 +2163,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
unsigned ring_instance;
struct amdgpu_ring *ring;
struct amd_sched_rq *rq;
-   int r;
+   int r, i;
 
vm->va = RB_ROOT;
vm->client_id = atomic64_inc_return(>vm_manager.client_counter);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   vm->reserved_vmid[i] = NULL;
spin_lock_init(>status_lock);
INIT_LIST_HEAD(>invalidated);
INIT_LIST_HEAD(>cleared);
@@ -2270,6 +2272,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 {
struct amdgpu_bo_va_mapping *mapping, *tmp;
bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+   int i;
 
if (vm->is_kfd_vm) {
struct amdgpu_vm_id_manager *id_mgr =
@@ -2313,6 +2316,18 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+   struct amdgpu_vm_id_manager *id_mgr =
+   >vm_manager.id_mgr[i];
+
+   mutex_lock(_mgr->lock);
+   if (vm->reserved_vmid[i]) {
+   list_add(>reserved_vmid[i]->list,
+_mgr->ids_lru);
+   vm->reserved_vmid[i] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+   }
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 16e0aaa..8eedca1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -123,6 +123,8 @@ struct amdgpu_vm {
 
/* client id */
u64 client_id;
+   /* dedicated to vm */
+   struct amdgpu_vm_id *reserved_vmid[AMDGPU_MAX_VMHUBS];
/* each VM will map on CSA */
struct amdgpu_bo_va *csa_bo_va;
 
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 0/6 v4] * Dedicated vmid per process v4 *

2017-04-26 Thread Chunming Zhou

The current kernel implementation, which grabs the idle VMID from pool when 
emitting the job may:

The back-to-back submission from one process could use different VMID.
The submission to different queues from single process could use different 
VMID

It works well in most case but cannot work for the SQ thread trace capture.

The VMID for the submission that set the {SQTT}_BASE, which refers to the 
address of the trace buffer, is stored in shader engine.

If the profiling application have to use different VMIDs to submit IBs in its 
life cycle:

Some trace is not captured since it actually uses different VMID to submit 
jobs.
Some part of captured trace may come from different application since they 
are accidentally uses the owner’s VMID to submit jobs.

V2:
1. address Christian's comments:
a. drop context flags for tag process, instead, add vm ioctl.
b. change order of patches.
c. sync waiting only when vm flush needs.

2. address Alex's comments;
bump module version

V3:
  address Jerry and Christian's comments.
  and only reserve gfxhub vmid

v4:
  address Jerry and Christian's comments.
  fix some race condistions.

Chunming Zhou (6):
  drm/amdgpu: add vm ioctl
  drm/amdgpu: add reserved vmid field in vm struct v2
  drm/amdgpu: reserve/unreserve vmid by vm ioctl v4
  drm/amdgpu: add limitation for dedicated vm number v4
  drm/amdgpu: implement grab reserved vmid V3
  drm/amdgpu: bump module verion for reserved vmid

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 152 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |   6 ++
 include/uapi/drm/amdgpu_drm.h   |  22 +
 5 files changed, 178 insertions(+), 6 deletions(-)

-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/6] drm/amdgpu: add vm ioctl

2017-04-26 Thread Chunming Zhou

It will be used for reserving vmid.

Change-Id: Ib7169ea999690c8e82d0dcbccdd2d97760c0270a
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 18 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  1 +
 include/uapi/drm/amdgpu_drm.h   | 22 ++
 4 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 052e6a5..8ea33c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1062,6 +1062,7 @@ int amdgpu_get_vblank_timestamp_kms(struct drm_device 
*dev, unsigned int pipe,
 const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
/* KMS */
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a8cd23b..8698177 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2379,3 +2379,21 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
}
}
 }
+
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
+{
+   union drm_amdgpu_vm *args = data;
+   struct amdgpu_device *adev = dev->dev_private;
+   struct amdgpu_fpriv *fpriv = filp->driver_priv;
+
+   switch (args->in.op) {
+   case AMDGPU_VM_OP_RESERVE_VMID:
+   case AMDGPU_VM_OP_UNRESERVE_VMID:
+   return -EINVAL;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 8eba2d3..16e0aaa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -248,5 +248,6 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
 void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
  struct amdgpu_bo_va *bo_va);
 void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size);
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp);
 
 #endif
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index b98b371..1c95775 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -51,6 +51,7 @@
 #define DRM_AMDGPU_GEM_OP  0x10
 #define DRM_AMDGPU_GEM_USERPTR 0x11
 #define DRM_AMDGPU_WAIT_FENCES 0x12
+#define DRM_AMDGPU_VM  0x13
 
 /* hybrid specific ioctls */
 #define DRM_AMDGPU_SEM 0x5b
@@ -71,6 +72,7 @@
 #define DRM_IOCTL_AMDGPU_GEM_OPDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
 #define DRM_IOCTL_AMDGPU_GEM_USERPTR   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
 #define DRM_IOCTL_AMDGPU_WAIT_FENCES   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
+#define DRM_IOCTL_AMDGPU_VMDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_VM, union drm_amdgpu_vm)
 
 /* hybrid specific ioctls */
 #define DRM_IOCTL_AMDGPU_GEM_DGMA  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_DGMA, struct drm_amdgpu_gem_dgma)
@@ -212,6 +214,26 @@ struct drm_amdgpu_ctx_in {
union drm_amdgpu_ctx_out out;
 };
 
+/* vm ioctl */
+#define AMDGPU_VM_OP_RESERVE_VMID  1
+#define AMDGPU_VM_OP_UNRESERVE_VMID2
+
+struct drm_amdgpu_vm_in {
+   /** AMDGPU_VM_OP_* */
+   __u32   op;
+   __u32   flags;
+};
+
+struct drm_amdgpu_vm_out {
+   /** For future use, no flags defined so far */
+   __u64   flags;
+};
+
+union drm_amdgpu_vm {
+   struct drm_amdgpu_vm_in in;
+   struct drm_amdgpu_vm_out out;
+};
+
 /* sem related */
 #define AMDGPU_SEM_OP_CREATE_SEM1
 #define AMDGPU_SEM_OP_WAIT_SEM 2
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 5/6] drm/amdgpu: implement grab dedicated vmid V2

2017-04-26 Thread zhoucm1




On 2017年04月27日 10:52, Zhang, Jerry (Junwei) wrote:

On 04/26/2017 07:10 PM, Chunming Zhou wrote:

v2: move sync waiting only when flush needs

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 
++

  1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 214ac50..bce7701 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -402,6 +402,63 @@ static bool 
amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, unsigned vmhub)

  return !!vm->dedicated_vmid[vmhub];
  }

+static int amdgpu_vm_grab_dedicated_vmid(struct amdgpu_vm *vm,
+ struct amdgpu_ring *ring,
+ struct amdgpu_sync *sync,
+ struct fence *fence,
+ struct amdgpu_job *job)
+{
+struct amdgpu_device *adev = ring->adev;
+unsigned vmhub = ring->funcs->vmhub;
+struct amdgpu_vm_id *id = vm->dedicated_vmid[vmhub];
+struct amdgpu_vm_id_manager *id_mgr = 
>vm_manager.id_mgr[vmhub];

+struct fence *updates = sync->last_vm_update;
+int r = 0;
+struct fence *flushed, *tmp;
+bool needs_flush = false;
+
+mutex_lock(_mgr->lock);
+if (amdgpu_vm_had_gpu_reset(adev, id))
+needs_flush = true;
+
+flushed  = id->flushed_updates;
+if (updates && (!flushed || updates->context != flushed->context ||
+fence_is_later(updates, flushed)))
+needs_flush = true;


Just question:
Do we need to consider concurrent flush for Vega10 like grab id func?

Christian has pointed that old asic has hardware bug.

Regards,
David Zhou


Jerry


+if (needs_flush) {
+tmp = amdgpu_sync_get_fence(>active);
+if (tmp) {
+r = amdgpu_sync_fence(adev, sync, tmp);
+fence_put(tmp);
+mutex_unlock(_mgr->lock);
+return r;
+}
+}
+
+/* Good we can use this VMID. Remember this submission as
+* user of the VMID.
+*/
+r = amdgpu_sync_fence(ring->adev, >active, fence);
+if (r)
+goto out;
+
+if (updates && (!flushed || updates->context != flushed->context ||
+fence_is_later(updates, flushed))) {
+fence_put(id->flushed_updates);
+id->flushed_updates = fence_get(updates);
+}
+id->pd_gpu_addr = job->vm_pd_addr;
+id->current_gpu_reset_count = 
atomic_read(>gpu_reset_counter);

+atomic64_set(>owner, vm->client_id);
+job->vm_needs_flush = needs_flush;
+
+job->vm_id = id - id_mgr->ids;
+trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+mutex_unlock(_mgr->lock);
+return r;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -426,6 +483,10 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, 
struct amdgpu_ring *ring,

  unsigned i;
  int r = 0;

+if (amdgpu_vm_dedicated_vmid_ready(vm, vmhub))
+return amdgpu_vm_grab_dedicated_vmid(vm, ring, sync,
+ fence, job);
+
  fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, 
GFP_KERNEL);

  if (!fences)
  return -ENOMEM;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread zhoucm1




On 2017年04月27日 10:24, Zhang, Jerry (Junwei) wrote:

On 04/27/2017 10:13 AM, zhoucm1 wrote:



On 2017年04月26日 20:26, Christian König wrote:

Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70
+++---
  1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct
amdgpu_device *adev,
  atomic_read(>gpu_reset_counter);
  }
  +static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, 
unsigned

vmhub)
+{
+return !!vm->dedicated_vmid[vmhub];
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, 
struct

amdgpu_ring *ring,
  return r;
  }
  +static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device 
*adev,

+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr = 
>vm_manager.id_mgr[vmhub];

+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub]) {
+ list_add(>dedicated_vmid[vmhub]->list,
+_mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+int r;
+
+id_mgr = >vm_manager.id_mgr[vmhub];
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, 
list);

+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;


We need a check here if there isn't an ID already reserved for the VM.

Additional to that I would still rename the field to reserved_vmid, 
that

describes better what it is actually doing.
the vmid is global, reserve it from global lru, so it makes sense to 
name

'reserved_vmid' like in patch#4.
the reserved vmid is dedicated to one vm, so the field in vm is
'dedicated_vmid' like here, which is my reason to name it.


Just the same thing looking at different views.

IMHO, it's reserved vmid from global group, naming "reserved_vmid".
And in Patch#4, it constrains the number, possibly naming 
"reserved_vmid_seq" or "reserved_vmid_ref".

How about reserved_vmid_num?

Regards,
David Zhou


Any of them looks reasonable, select the one you like. :)

Jerry



Anyway, the alternative is ok to me although I prefer mine, I hope we 
can

deliver this solution by today, many RGP issues depend on it.

Thanks,
David Zhou



+ mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;


This is racy. A command submission could happen while we wait for 
the id to

become idle.

The easiest way to solve this is to hold the lock while waiting for 
the ID to

become available.


+
+return 0;
+err:
+amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring 
*ring)

  {
  struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
-for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-struct amdgpu_vm_id_manager *id_mgr =
->vm_manager.id_mgr[i];
-
-mutex_lock(_mgr->lock);
-if (vm->dedicated_vmid[i]) {
- list_add(>dedicated_vmid[i]->list,
- _mgr->ids_lru);
-vm->dedicated_vmid[i] = NULL;
-}
-mutex_unlock(_mgr->lock);
-}
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+amdgpu_vm_free_dedicated_vmid(adev, vm, i);
  }
/**
@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
void

*data, struct drm_file *filp)
  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;
switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+/* current, we only have requirement to reserve vmid from 
gfxhub */

+if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);


This is racy as well.

Two threads could try to reserve a VMID at the same time. You need to
integrate the check if it's already allocated into
amdgpu_vm_alloc_dedicated_vmid().

Regards,
Christian.


+if (r)
+return r;
+}

Re: [PATCH 5/6] drm/amdgpu: implement grab dedicated vmid V2

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/26/2017 07:10 PM, Chunming Zhou wrote:

v2: move sync waiting only when flush needs

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 ++
  1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 214ac50..bce7701 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -402,6 +402,63 @@ static bool amdgpu_vm_dedicated_vmid_ready(struct 
amdgpu_vm *vm, unsigned vmhub)
return !!vm->dedicated_vmid[vmhub];
  }

+static int amdgpu_vm_grab_dedicated_vmid(struct amdgpu_vm *vm,
+struct amdgpu_ring *ring,
+struct amdgpu_sync *sync,
+struct fence *fence,
+struct amdgpu_job *job)
+{
+   struct amdgpu_device *adev = ring->adev;
+   unsigned vmhub = ring->funcs->vmhub;
+   struct amdgpu_vm_id *id = vm->dedicated_vmid[vmhub];
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+   struct fence *updates = sync->last_vm_update;
+   int r = 0;
+   struct fence *flushed, *tmp;
+   bool needs_flush = false;
+
+   mutex_lock(_mgr->lock);
+   if (amdgpu_vm_had_gpu_reset(adev, id))
+   needs_flush = true;
+
+   flushed  = id->flushed_updates;
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed)))
+   needs_flush = true;


Just question:
Do we need to consider concurrent flush for Vega10 like grab id func?

Jerry


+   if (needs_flush) {
+   tmp = amdgpu_sync_get_fence(>active);
+   if (tmp) {
+   r = amdgpu_sync_fence(adev, sync, tmp);
+   fence_put(tmp);
+   mutex_unlock(_mgr->lock);
+   return r;
+   }
+   }
+
+   /* Good we can use this VMID. Remember this submission as
+   * user of the VMID.
+   */
+   r = amdgpu_sync_fence(ring->adev, >active, fence);
+   if (r)
+   goto out;
+
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed))) {
+   fence_put(id->flushed_updates);
+   id->flushed_updates = fence_get(updates);
+   }
+   id->pd_gpu_addr = job->vm_pd_addr;
+   id->current_gpu_reset_count = atomic_read(>gpu_reset_counter);
+   atomic64_set(>owner, vm->client_id);
+   job->vm_needs_flush = needs_flush;
+
+   job->vm_id = id - id_mgr->ids;
+   trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+   mutex_unlock(_mgr->lock);
+   return r;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -426,6 +483,10 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
unsigned i;
int r = 0;

+   if (amdgpu_vm_dedicated_vmid_ready(vm, vmhub))
+   return amdgpu_vm_grab_dedicated_vmid(vm, ring, sync,
+fence, job);
+
fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, GFP_KERNEL);
if (!fences)
return -ENOMEM;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/27/2017 10:13 AM, zhoucm1 wrote:



On 2017年04月26日 20:26, Christian König wrote:

Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70
+++---
  1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct
amdgpu_device *adev,
  atomic_read(>gpu_reset_counter);
  }
  +static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, unsigned
vmhub)
+{
+return !!vm->dedicated_vmid[vmhub];
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct
amdgpu_ring *ring,
  return r;
  }
  +static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub]) {
+list_add(>dedicated_vmid[vmhub]->list,
+_mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+int r;
+
+id_mgr = >vm_manager.id_mgr[vmhub];
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;


We need a check here if there isn't an ID already reserved for the VM.

Additional to that I would still rename the field to reserved_vmid, that
describes better what it is actually doing.

the vmid is global, reserve it from global lru, so it makes sense to name
'reserved_vmid' like in patch#4.
the reserved vmid is dedicated to one vm, so the field in vm is
'dedicated_vmid' like here, which is my reason to name it.


Just the same thing looking at different views.

IMHO, it's reserved vmid from global group, naming "reserved_vmid".
And in Patch#4, it constrains the number, possibly naming "reserved_vmid_seq" 
or "reserved_vmid_ref".


Any of them looks reasonable, select the one you like. :)

Jerry



Anyway, the alternative is ok to me although I prefer mine, I hope we can
deliver this solution by today, many RGP issues depend on it.

Thanks,
David Zhou



+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;


This is racy. A command submission could happen while we wait for the id to
become idle.

The easiest way to solve this is to hold the lock while waiting for the ID to
become available.


+
+return 0;
+err:
+amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
  struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev,
struct amdgpu_vm *vm)
amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
-for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-struct amdgpu_vm_id_manager *id_mgr =
->vm_manager.id_mgr[i];
-
-mutex_lock(_mgr->lock);
-if (vm->dedicated_vmid[i]) {
-list_add(>dedicated_vmid[i]->list,
- _mgr->ids_lru);
-vm->dedicated_vmid[i] = NULL;
-}
-mutex_unlock(_mgr->lock);
-}
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+amdgpu_vm_free_dedicated_vmid(adev, vm, i);
  }
/**
@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void
*data, struct drm_file *filp)
  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;
switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+/* current, we only have requirement to reserve vmid from gfxhub */
+if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);


This is racy as well.

Two threads could try to reserve a VMID at the same time. You need to
integrate the check if it's already allocated into
amdgpu_vm_alloc_dedicated_vmid().

Regards,
Christian.


+if (r)
+return r;
+}
+break;
  case AMDGPU_VM_OP_UNRESERVE_VMID:
-return -EINVAL;
+

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/26/2017 08:26 PM, Christian König wrote:

Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++---
  1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device
*adev,
  atomic_read(>gpu_reset_counter);
  }
+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, unsigned
vmhub)
+{
+return !!vm->dedicated_vmid[vmhub];
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct
amdgpu_ring *ring,
  return r;
  }
+static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub]) {
+list_add(>dedicated_vmid[vmhub]->list,
+_mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+int r;
+
+id_mgr = >vm_manager.id_mgr[vmhub];
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;


We need a check here if there isn't an ID already reserved for the VM.

Additional to that I would still rename the field to reserved_vmid, that
describes better what it is actually doing.


+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;


This is racy. A command submission could happen while we wait for the id to
become idle.

The easiest way to solve this is to hold the lock while waiting for the ID to
become available.


+
+return 0;
+err:
+amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
  struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct
amdgpu_vm *vm)
  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
-for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-struct amdgpu_vm_id_manager *id_mgr =
->vm_manager.id_mgr[i];
-
-mutex_lock(_mgr->lock);
-if (vm->dedicated_vmid[i]) {
-list_add(>dedicated_vmid[i]->list,
- _mgr->ids_lru);
-vm->dedicated_vmid[i] = NULL;
-}
-mutex_unlock(_mgr->lock);
-}
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+amdgpu_vm_free_dedicated_vmid(adev, vm, i);
  }
  /**
@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void
*data, struct drm_file *filp)
  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;
  switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+/* current, we only have requirement to reserve vmid from gfxhub */
+if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);


This is racy as well.

Two threads could try to reserve a VMID at the same time. You need to integrate
the check if it's already allocated into amdgpu_vm_alloc_dedicated_vmid().


Apart from that, small tip.
AMDGPU_GFXHUB instead of "0" looks more self-explanatory.

Jerry



Regards,
Christian.


+if (r)
+return r;
+}
+break;
  case AMDGPU_VM_OP_UNRESERVE_VMID:
-return -EINVAL;
+amdgpu_vm_free_dedicated_vmid(adev, >vm, 0);
  break;
  default:
  return -EINVAL;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread zhoucm1




On 2017年04月26日 20:26, Christian König wrote:

Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 
+++---

  1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct 
amdgpu_device *adev,

  atomic_read(>gpu_reset_counter);
  }
  +static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, 
unsigned vmhub)

+{
+return !!vm->dedicated_vmid[vmhub];
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, 
struct amdgpu_ring *ring,

  return r;
  }
  +static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr = 
>vm_manager.id_mgr[vmhub];

+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub]) {
+list_add(>dedicated_vmid[vmhub]->list,
+_mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm,
+  unsigned vmhub)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+int r;
+
+id_mgr = >vm_manager.id_mgr[vmhub];
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, 
list);

+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;


We need a check here if there isn't an ID already reserved for the VM.

Additional to that I would still rename the field to reserved_vmid, 
that describes better what it is actually doing.
the vmid is global, reserve it from global lru, so it makes sense to 
name 'reserved_vmid' like in patch#4.
the reserved vmid is dedicated to one vm, so the field in vm is 
'dedicated_vmid' like here, which is my reason to name it.


Anyway, the alternative is ok to me although I prefer mine, I hope we 
can deliver this solution by today, many RGP issues depend on it.


Thanks,
David Zhou



+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;


This is racy. A command submission could happen while we wait for the 
id to become idle.


The easiest way to solve this is to hold the lock while waiting for 
the ID to become available.



+
+return 0;
+err:
+amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring 
*ring)

  {
  struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
-for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-struct amdgpu_vm_id_manager *id_mgr =
->vm_manager.id_mgr[i];
-
-mutex_lock(_mgr->lock);
-if (vm->dedicated_vmid[i]) {
-list_add(>dedicated_vmid[i]->list,
- _mgr->ids_lru);
-vm->dedicated_vmid[i] = NULL;
-}
-mutex_unlock(_mgr->lock);
-}
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+amdgpu_vm_free_dedicated_vmid(adev, vm, i);
  }
/**
@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
void *data, struct drm_file *filp)

  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;
switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+/* current, we only have requirement to reserve vmid from 
gfxhub */

+if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);


This is racy as well.

Two threads could try to reserve a VMID at the same time. You need to 
integrate the check if it's already allocated into 
amdgpu_vm_alloc_dedicated_vmid().


Regards,
Christian.


+if (r)
+return r;
+}
+break;
  case AMDGPU_VM_OP_UNRESERVE_VMID:
-return -EINVAL;
+amdgpu_vm_free_dedicated_vmid(adev, >vm, 0);
  break;
  default:
  return -EINVAL;





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Michel Dänzer

On 26/04/17 09:11 PM, Gerd Hoffmann wrote:
>   Hi,
> 
 Just to reiterate, this won't work for the radeon driver, which programs
 the GPU to use (effectively, per the current definition that these are
 little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
 DRM_FORMAT_BGRX with >= R600.
>>>
>>> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
>>
>> Using a GPU byte swapping mechanism which only affects CPU access to
>> video RAM.
> 
> That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
> another thread?

Right.


> What about dumb bos?  You've mentioned the swap flag isn't used for
> those.  Which implies they are in little endian byte order (both gpu and
> cpu view).

Right, AFAICT from looking at the code.


> Does the modesetting driver work correctly on that hardware?

Not sure.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/radeon: Avoid overflows/divide-by-zero in latency_watermark calculations.

2017-04-26 Thread Alex Deucher

On Sun, Apr 23, 2017 at 7:33 PM, Mario Kleiner
 wrote:
> At dot clocks > approx. 250 Mhz, some of these calcs will overflow and
> cause miscalculation of latency watermarks, and for some overflows also
> divide-by-zero driver crash. Make calcs more overflow resistant.
>
> This is a direct port of the corresponding patch from amdgpu-kms,
> copy-paste for cik from dce-8 and si from dce-6, with a slightly
> simpler variant for evergreen dce-4/5.
>
> Only tested on DCE-4 evergreen with a Radeon HD-5770.
>
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 

Applied the series.  thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/cik.c   | 19 +++
>  drivers/gpu/drm/radeon/evergreen.c |  8 +---
>  drivers/gpu/drm/radeon/si.c| 19 +++
>  3 files changed, 7 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 53710dd..4f034cb 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -9150,23 +9150,10 @@ static u32 dce8_latency_watermark(struct 
> dce8_wm_params *wm)
> a.full = dfixed_const(available_bandwidth);
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
> +   tmp = div_u64((u64) dmif_size * (u64) wm->disp_clk, mc_latency + 512);
> +   tmp = min(dfixed_trunc(a), tmp);
>
> -   b.full = dfixed_const(mc_latency + 512);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(b, c);
> -
> -   c.full = dfixed_const(dmif_size);
> -   b.full = dfixed_div(c, b);
> -
> -   tmp = min(dfixed_trunc(a), dfixed_trunc(b));
> -
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(tmp, dfixed_trunc(b));
> +   lb_fill_bw = min(tmp, wm->disp_clk * wm->bytes_per_pixel / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> diff --git a/drivers/gpu/drm/radeon/evergreen.c 
> b/drivers/gpu/drm/radeon/evergreen.c
> index d1b1e0c..3c9c133 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -2188,13 +2188,7 @@ static u32 evergreen_latency_watermark(struct 
> evergreen_wm_params *wm)
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
>
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(dfixed_trunc(a), dfixed_trunc(b));
> +   lb_fill_bw = min(dfixed_trunc(a), wm->disp_clk * wm->bytes_per_pixel 
> / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 528e5a4..3efdfd0 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -2204,23 +2204,10 @@ static u32 dce6_latency_watermark(struct 
> dce6_wm_params *wm)
> a.full = dfixed_const(available_bandwidth);
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
> +   tmp = div_u64((u64) dmif_size * (u64) wm->disp_clk, mc_latency + 512);
> +   tmp = min(dfixed_trunc(a), tmp);
>
> -   b.full = dfixed_const(mc_latency + 512);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(b, c);
> -
> -   c.full = dfixed_const(dmif_size);
> -   b.full = dfixed_div(c, b);
> -
> -   tmp = min(dfixed_trunc(a), dfixed_trunc(b));
> -
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(tmp, dfixed_trunc(b));
> +   lb_fill_bw = min(tmp, wm->disp_clk * wm->bytes_per_pixel / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Add missing lb_vblank_lead_lines setup to DCE-6 path.

2017-04-26 Thread Alex Deucher

On Sun, Apr 23, 2017 at 7:02 PM, Mario Kleiner
 wrote:
> This apparently got lost when implementing the new DCE-6 support
> and would cause failures in pageflip scheduling and timestamping.
>
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 
> Cc: sta...@vger.kernel.org

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 307269b..e146d25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -979,7 +979,7 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> u32 priority_a_mark = 0, priority_b_mark = 0;
> u32 priority_a_cnt = PRIORITY_OFF;
> u32 priority_b_cnt = PRIORITY_OFF;
> -   u32 tmp, arb_control3;
> +   u32 tmp, arb_control3, lb_vblank_lead_lines = 0;
> fixed20_12 a, b, c;
>
> if (amdgpu_crtc->base.enabled && num_heads && mode) {
> @@ -1091,6 +1091,8 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> c.full = dfixed_div(c, a);
> priority_b_mark = dfixed_trunc(c);
> priority_b_cnt |= priority_b_mark & PRIORITY_MARK_MASK;
> +
> +   lb_vblank_lead_lines = DIV_ROUND_UP(lb_size, 
> mode->crtc_hdisplay);
> }
>
> /* select wm A */
> @@ -1120,6 +1122,9 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> /* save values for DPM */
> amdgpu_crtc->line_time = line_time;
> amdgpu_crtc->wm_high = latency_watermark_a;
> +
> +   /* Save number of lines the linebuffer leads before the scanout */
> +   amdgpu_crtc->lb_vblank_lead_lines = lb_vblank_lead_lines;
>  }
>
>  /* watermark setup */
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

2017-04-26 Thread Felix Kuehling

On 17-04-25 02:28 AM, Michel Dänzer wrote:
> On 22/04/17 02:05 AM, Felix Kuehling wrote:
>> __setup doesn't work in modules.
> Right. We could build something like
> drivers/video/fbdev/core/fb_cmdline.c:video_setup() into the kernel to
> handle this, but it's a bit ugly, which is one reason why I was leaning
> towards:
>
>
>> s8250_options is only compiled if the driver is not a module.
> That doesn't prevent us from using __module_param_call directly, does it?
>
> Although, that still doesn't work as I envision if only one driver's
> option is set e.g. in /etc/modprobe.d/*.conf .
>
>
> So, I'm starting to think we need a shared module for this, which
> provides one or multiple module parameters to choose which driver to use
> for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
> That might make things easier for amdgpu-pro / other standalone amdgpu
> versions in the long run as well, as they could add files to
> /etc/modprobe.d/ choosing themselves by default, without having to
> blacklist radeon.
>
> What do you guys think?

Hi Michel,

You said in an earlier email that it doesn't have to be convenient. With
that in mind, I went back to a minimalistic solution that doesn't need
any additional Kconfig options and only uses two module parameters (one
in radeon, one in amdgpu).

I'm attaching my WIP patches for reference (currently based on
amd-kfd-staging). For an official review I'd rebase them on
amd-staging-4.9, remove the #if-0-ed hack that didn't work out, and add
the same for SI.

Can everyone can agree that this is good enough?

Regards,
  Felix

>
>
> [0] or possibly even more fine-grained in the future
>

>From e1d5fc320dbe2f60d922c466986863489f4bfe77 Mon Sep 17 00:00:00 2001
From: Felix Kuehling 
Date: Thu, 20 Apr 2017 14:41:34 -0400
Subject: [PATCH 1/2] drm/radeon: Add module param to control CIK support

If AMDGPU supports CIK, add a module parameter to control CIK
support in radeon. It's off by default in radeon, while it will be
on by default in AMDGPU.

Change-Id: Ib594f7359b9b3143c859b3be5c0244a9a2a773cd
Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/radeon/radeon.h |  4 
 drivers/gpu/drm/radeon/radeon_drv.c |  6 ++
 drivers/gpu/drm/radeon/radeon_kms.c | 18 ++
 3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 5f16bf3..a05aaea 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -116,6 +116,10 @@
 extern int radeon_uvd;
 extern int radeon_vce;
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+extern int radeon_cik_support;
+#endif
+
 /*
  * Copy from radeon_drv.h so we don't have to include both and have conflicting
  * symbol;
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 2e5d680..2cdb01b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -301,6 +301,12 @@ static inline void radeon_unregister_atpx_handler(void) {}
 MODULE_PARM_DESC(vce, "vce enable/disable vce support (1 = enable, 0 = disable)");
 module_param_named(vce, radeon_vce, int, 0444);
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+int radeon_cik_support = 0;
+MODULE_PARM_DESC(cik_support, "CIK support (1 = enabled, 0 = disabled (default))");
+module_param_named(cik_support, radeon_cik_support, int, 0444);
+#endif
+
 static struct pci_device_id pciidlist[] = {
 	radeon_PCI_IDS
 };
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index d6d58bc..28ab255 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -99,6 +99,24 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
 	struct radeon_device *rdev;
 	int r, acpi_status;
 
+#ifdef CONFIG_DRM_AMDGPU_CIK
+	if (!radeon_cik_support) {
+		switch (flags & RADEON_FAMILY_MASK) {
+		case CHIP_KAVERI:
+		case CHIP_BONAIRE:
+		case CHIP_HAWAII:
+		case CHIP_KABINI:
+		case CHIP_MULLINS:
+			dev_warn(dev->dev,
+ "CIK support provided by amdgpu.\n");
+			dev_warn(dev->dev,
+		"Use radeon.cik_support=1 amdgpu.cik_support=0 to override.\n"
+);
+			return -ENODEV;
+		}
+	}
+#endif
+
 	rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
 	if (rdev == NULL) {
 		return -ENOMEM;
-- 
1.9.1

>From 18d4b8b6282215073c3c6762f36c9009b1218f3c Mon Sep 17 00:00:00 2001
From: Felix Kuehling 
Date: Thu, 20 Apr 2017 14:42:04 -0400
Subject: [PATCH 2/2] drm/amdgpu: Add module param to control CIK support

If AMDGPU supports CIK, add a module parameter to control CIK
support. It's on by default in AMDGPU, while it is off by default
in radeon.

Change-Id: Ica1296beae96b1c5f48698c06cfd2453fe1ff080
Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 41 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

Re: [PATCH 2/2] drm/amdgpu: Fix use of interruptible waiting

2017-04-26 Thread Alex Xie


Hi,

I knew this is part of ioctl. And I intended to fix this interruptible 
waiting in an ioctl.


ioctl is one of driver interfaces, where interruptible waiting is good 
in some scenarios. For example, if ioctl performs IO operations in 
atomic way, we don't want ioctl to be interrupted by a signal.


Interruptible waiting is good when we need to wait for longer time, for 
example, waiting for an input data for indefinite time. We don't want 
the process not responding to signals. Interruptible waitings can be 
good in read/write/ioctl interfaces.


For this patch,

1. The wait is short. There is not much benefit by interruptible 
waiting.  The BO is a frame buffer BO. Are there many competitors to 
lock this BO? If not, the wait is very short. This is my main reason to 
change.
2. In this function and caller functions, the error handling for such 
interrupt is complicated and risky. When the waiting is interrupted by a 
signal, the callers of this function need to handle this interrupt 
error. I traced the calling stack all the way back to function 
drm_mode_page_flip_ioctl. I could not find an issue. But I am not sure 
about even upper layer. Is this IOCTL restartable? How about further 
higher layer? Since I didn't see benefit in point 1. So I thought it was 
a good idea to change.


Anyway, it is up to you. A change might bring other unseen risk. If you 
still have concern, I will drop this patch. This IOCTL is used by 
graphic operation. There may not be a lot of signals to a GUI process 
which uses this IOCTL.


Thanks,
Alex Bin


On 17-04-26 04:34 AM, Christian König wrote:

Am 26.04.2017 um 03:17 schrieb Michel Dänzer:

On 26/04/17 06:25 AM, Alex Xie wrote:

1. The wait is short. There is not much benefit by
interruptible waiting.
2. In this function and caller functions, the error
handling for such interrupt is complicated and risky.

Change-Id: I289674ecd3f5ef20c93fe63e33df6d668b3c2edc
Signed-off-by: Alex Xie 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c

index 43082bf..c006cc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -227,7 +227,7 @@ int amdgpu_crtc_prepare_flip(
  new_abo = gem_to_amdgpu_bo(obj);
/* pin the new buffer */
-r = amdgpu_bo_reserve(new_abo, false);
+r = amdgpu_bo_reserve(new_abo, true);
  if (unlikely(r != 0)) {
  DRM_ERROR("failed to reserve new abo buffer before flip\n");
  goto cleanup;


I'm afraid we have no choice but to use interruptible here, because this
code runs as part of an ioctl (DRM_IOCTL_MODE_PAGE_FLIP).


Yes, agree. But the error message is incorrect here and should be 
removed.


Christian.


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] Improve pipe split between amdgpu and amdkfd v4

2017-04-26 Thread Andres Rodriguez

Thanks for taking the time with the multiple iterations of the series!

Regards,
Andres

On Wed, Apr 26, 2017 at 12:54 PM, Oded Gabbay  wrote:
> On Wed, Apr 26, 2017 at 6:50 PM, Andres Rodriguez  wrote:
>> Hey Oded,
>>
>> Let us know if you can reproduce the kfdtest results on your side.
>>
>> Regards,
>> Andres
>>
>>
>
> Hi Andres,
> I run the kfdtest on your patch-set with KAVERI machine, and all the
> tests passed :)
> I've also gone over your code and it seems fine.
>
> So, this patch-set is:
> Acked-by: Oded Gabbay 
>
>
>
>> On 2017-04-21 04:02 PM, Andres Rodriguez wrote:
>>>
>>> V4 updates:
>>>  * Rebased onto latest 4.12-wip (ba16d6c), since I got test reports that
>>> the
>>>patch series failed to apply
>>>
>>> V3 updates:
>>>  * Fixed kfd set resources grabbing MEC1 queues
>>>  * kfdtest now passes on Kaveri with the amdgpu or radeon driver
>>>
>>> V2 updates:
>>>  * Fixed wrong HPD offset in compute_pipe_init for gfx7
>>>  * Fixed compute_pipe_init using wrong ME for gfx7
>>>
>>>
>>> This is a split of patches that are ready to land from the series:
>>> Add support for high priority scheduling in amdgpu v8
>>>
>>> I've included Felix and Alex's feedback from the thread above. This
>>> includes:
>>>  * Separate MEC_HPD_SIZE rename into a separate patch (patch 01)
>>>  * Added a patch to fix the kgd_hqd_load bug Felix pointed out (patch 06)
>>>  * Fixes for various off-by-one errors
>>>  * Use gfx_v8_0_deactivate_hqd
>>>
>>> Only comment I didn't address was changing the queue allocation policy for
>>> gfx9 (similar to gfx7/8). See inline reply in that thread for more details
>>> on why this was skipped.
>>>
>>>
>>> Series available in the wip-queue-policy-v4 branch at:
>>> g...@github.com:lostgoat/linux.git
>>> Or:
>>> https://github.com/lostgoat/linux.git
>>>
>>>
>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] Improve pipe split between amdgpu and amdkfd v4

2017-04-26 Thread Oded Gabbay

On Wed, Apr 26, 2017 at 6:50 PM, Andres Rodriguez  wrote:
> Hey Oded,
>
> Let us know if you can reproduce the kfdtest results on your side.
>
> Regards,
> Andres
>
>

Hi Andres,
I run the kfdtest on your patch-set with KAVERI machine, and all the
tests passed :)
I've also gone over your code and it seems fine.

So, this patch-set is:
Acked-by: Oded Gabbay 



> On 2017-04-21 04:02 PM, Andres Rodriguez wrote:
>>
>> V4 updates:
>>  * Rebased onto latest 4.12-wip (ba16d6c), since I got test reports that
>> the
>>patch series failed to apply
>>
>> V3 updates:
>>  * Fixed kfd set resources grabbing MEC1 queues
>>  * kfdtest now passes on Kaveri with the amdgpu or radeon driver
>>
>> V2 updates:
>>  * Fixed wrong HPD offset in compute_pipe_init for gfx7
>>  * Fixed compute_pipe_init using wrong ME for gfx7
>>
>>
>> This is a split of patches that are ready to land from the series:
>> Add support for high priority scheduling in amdgpu v8
>>
>> I've included Felix and Alex's feedback from the thread above. This
>> includes:
>>  * Separate MEC_HPD_SIZE rename into a separate patch (patch 01)
>>  * Added a patch to fix the kgd_hqd_load bug Felix pointed out (patch 06)
>>  * Fixes for various off-by-one errors
>>  * Use gfx_v8_0_deactivate_hqd
>>
>> Only comment I didn't address was changing the queue allocation policy for
>> gfx9 (similar to gfx7/8). See inline reply in that thread for more details
>> on why this was skipped.
>>
>>
>> Series available in the wip-queue-policy-v4 branch at:
>> g...@github.com:lostgoat/linux.git
>> Or:
>> https://github.com/lostgoat/linux.git
>>
>>
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH] drm/amdgpu: remove unused and mostly unimplemented CGS functions

2017-04-26 Thread Deucher, Alexander

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Christian König
> Sent: Wednesday, April 26, 2017 9:14 AM
> To: amd-gfx@lists.freedesktop.org
> Subject: [PATCH] drm/amdgpu: remove unused and mostly unimplemented
> CGS functions
> 
> From: Christian König 
> 
> Those functions are all unused and some not even implemented.
> 
> Signed-off-by: Christian König 

I think some of this is used by the ACP driver (cgs_get_pci_resource for 
example).  Make sure that is enabled in your build.  You might also want to 
check with Felix for KFD.

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c  | 233 
>  drivers/gpu/drm/amd/include/cgs_common.h | 303 -
> --
>  2 files changed, 536 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> index a1a2d44..93f5dcc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
> @@ -42,82 +42,6 @@ struct amdgpu_cgs_device {
>   struct amdgpu_device *adev =\
>   ((struct amdgpu_cgs_device *)cgs_device)->adev
> 
> -static int amdgpu_cgs_gpu_mem_info(struct cgs_device *cgs_device, enum
> cgs_gpu_mem_type type,
> -uint64_t *mc_start, uint64_t *mc_size,
> -uint64_t *mem_size)
> -{
> - CGS_FUNC_ADEV;
> - switch(type) {
> - case CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB:
> - case CGS_GPU_MEM_TYPE__VISIBLE_FB:
> - *mc_start = 0;
> - *mc_size = adev->mc.visible_vram_size;
> - *mem_size = adev->mc.visible_vram_size - adev-
> >vram_pin_size;
> - break;
> - case CGS_GPU_MEM_TYPE__INVISIBLE_CONTIG_FB:
> - case CGS_GPU_MEM_TYPE__INVISIBLE_FB:
> - *mc_start = adev->mc.visible_vram_size;
> - *mc_size = adev->mc.real_vram_size - adev-
> >mc.visible_vram_size;
> - *mem_size = *mc_size;
> - break;
> - case CGS_GPU_MEM_TYPE__GART_CACHEABLE:
> - case CGS_GPU_MEM_TYPE__GART_WRITECOMBINE:
> - *mc_start = adev->mc.gtt_start;
> - *mc_size = adev->mc.gtt_size;
> - *mem_size = adev->mc.gtt_size - adev->gart_pin_size;
> - break;
> - default:
> - return -EINVAL;
> - }
> -
> - return 0;
> -}
> -
> -static int amdgpu_cgs_gmap_kmem(struct cgs_device *cgs_device, void
> *kmem,
> - uint64_t size,
> - uint64_t min_offset, uint64_t max_offset,
> - cgs_handle_t *kmem_handle, uint64_t
> *mcaddr)
> -{
> - CGS_FUNC_ADEV;
> - int ret;
> - struct amdgpu_bo *bo;
> - struct page *kmem_page = vmalloc_to_page(kmem);
> - int npages = ALIGN(size, PAGE_SIZE) >> PAGE_SHIFT;
> -
> - struct sg_table *sg = drm_prime_pages_to_sg(_page,
> npages);
> - ret = amdgpu_bo_create(adev, size, PAGE_SIZE, false,
> -AMDGPU_GEM_DOMAIN_GTT, 0, sg, NULL,
> );
> - if (ret)
> - return ret;
> - ret = amdgpu_bo_reserve(bo, false);
> - if (unlikely(ret != 0))
> - return ret;
> -
> - /* pin buffer into GTT */
> - ret = amdgpu_bo_pin_restricted(bo,
> AMDGPU_GEM_DOMAIN_GTT,
> -min_offset, max_offset, mcaddr);
> - amdgpu_bo_unreserve(bo);
> -
> - *kmem_handle = (cgs_handle_t)bo;
> - return ret;
> -}
> -
> -static int amdgpu_cgs_gunmap_kmem(struct cgs_device *cgs_device,
> cgs_handle_t kmem_handle)
> -{
> - struct amdgpu_bo *obj = (struct amdgpu_bo *)kmem_handle;
> -
> - if (obj) {
> - int r = amdgpu_bo_reserve(obj, false);
> - if (likely(r == 0)) {
> - amdgpu_bo_unpin(obj);
> - amdgpu_bo_unreserve(obj);
> - }
> - amdgpu_bo_unref();
> -
> - }
> - return 0;
> -}
> -
>  static int amdgpu_cgs_alloc_gpu_mem(struct cgs_device *cgs_device,
>   enum cgs_gpu_mem_type type,
>   uint64_t size, uint64_t align,
> @@ -349,96 +273,6 @@ static void amdgpu_cgs_write_ind_register(struct
> cgs_device *cgs_device,
>   WARN(1, "Invalid indirect register space");
>  }
> 
> -static uint8_t amdgpu_cgs_read_pci_config_byte(struct cgs_device
> *cgs_device, unsigned addr)
> -{
> - CGS_FUNC_ADEV;
> - uint8_t val;
> - int ret = pci_read_config_byte(adev->pdev, addr, );
> - if (WARN(ret, "pci_read_config_byte error"))
> - return 0;
> - return val;
> -}
> -
> -static uint16_t amdgpu_cgs_read_pci_config_word(struct cgs_device
> *cgs_device, unsigned addr)
> -{
> - CGS_FUNC_ADEV;
> - uint16_t val;
> - int ret = pci_read_config_word(adev->pdev, addr, );
> - if (WARN(ret,

RE: [PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.

2017-04-26 Thread Deucher, Alexander

> -Original Message-
> From: Deucher, Alexander
> Sent: Wednesday, April 26, 2017 12:11 PM
> To: Zhu, Rex; amd-gfx@lists.freedesktop.org
> Cc: Zhu, Rex
> Subject: RE: [PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.
> 
> > -Original Message-
> > From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> > Of Rex Zhu
> > Sent: Wednesday, April 26, 2017 4:36 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Zhu, Rex
> > Subject: [PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.
> >
> > Change-Id: I46d7f2531eaec8135ccaf64b5ce8abc433c54e1a
> > Signed-off-by: Rex Zhu 
> 
> Acked-by: Alex Deucher 


Might also want to mention the JIRA ticket id in the commit message.

Alex

> 
> > ---
> >  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 10 --
> >  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 10 --
> >  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 10 --
> >  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c |  4 ++--
> >  4 files changed, 26 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > index f7579de..25c3703 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> > @@ -3719,6 +3719,12 @@ static void gfx_v6_0_get_cu_info(struct
> > amdgpu_device *adev)
> > u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
> > struct amdgpu_cu_info *cu_info = >gfx.cu_info;
> > unsigned disable_masks[4 * 2];
> > +   u32 ao_cu_num;
> > +
> > +   if (adev->flags & AMD_IS_APU)
> > +   ao_cu_num = 2;
> > +   else
> > +   ao_cu_num = adev->gfx.config.max_cu_per_sh;
> >
> > memset(cu_info, 0, sizeof(*cu_info));
> >
> > @@ -3737,9 +3743,9 @@ static void gfx_v6_0_get_cu_info(struct
> > amdgpu_device *adev)
> > bitmap = gfx_v6_0_get_cu_enabled(adev);
> > cu_info->bitmap[i][j] = bitmap;
> >
> > -   for (k = 0; k < 16; k++) {
> > +   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k++)
> > {
> > if (bitmap & mask) {
> > -   if (counter < 2)
> > +   if (counter < ao_cu_num)
> > ao_bitmap |= mask;
> > counter ++;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > index fa5a0d2..75cca54 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> > @@ -5339,6 +5339,12 @@ static void gfx_v7_0_get_cu_info(struct
> > amdgpu_device *adev)
> > u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
> > struct amdgpu_cu_info *cu_info = >gfx.cu_info;
> > unsigned disable_masks[4 * 2];
> > +   u32 ao_cu_num;
> > +
> > +   if (adev->flags & AMD_IS_APU)
> > +   ao_cu_num = 2;
> > +   else
> > +   ao_cu_num = adev->gfx.config.max_cu_per_sh;
> >
> > memset(cu_info, 0, sizeof(*cu_info));
> >
> > @@ -5357,9 +5363,9 @@ static void gfx_v7_0_get_cu_info(struct
> > amdgpu_device *adev)
> > bitmap = gfx_v7_0_get_cu_active_bitmap(adev);
> > cu_info->bitmap[i][j] = bitmap;
> >
> > -   for (k = 0; k < 16; k ++) {
> > +   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++)
> > {
> > if (bitmap & mask) {
> > -   if (counter < 2)
> > +   if (counter < ao_cu_num)
> > ao_bitmap |= mask;
> > counter ++;
> > }
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > index 2ff5f19..0025ef6 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> > @@ -7098,9 +7098,15 @@ static void gfx_v8_0_get_cu_info(struct
> > amdgpu_device *adev)
> > u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
> > struct amdgpu_cu_info *cu_info = >gfx.cu_info;
> > unsigned disable_masks[4 * 2];
> > +   u32 ao_cu_num;
> >
> > memset(cu_info, 0, sizeof(*cu_info));
> >
> > +   if (adev->flags & AMD_IS_APU)
> > +   ao_cu_num = 2;
> > +   else
> > +   ao_cu_num = adev->gfx.config.max_cu_per_sh;
> > +
> > amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
> >
> > mutex_lock(>grbm_idx_mutex);
> > @@ -7116,9 +7122,9 @@ static void gfx_v8_0_get_cu_info(struct
> > amdgpu_device *adev)
> > bitmap = gfx_v8_0_get_cu_active_bitmap(adev);
> > cu_info->bitmap[i][j] = bitmap;
> >
> > -   for (k = 0; k < 16; k ++) {
> > +   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++)
> > {
> > if

RE: [PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.

2017-04-26 Thread Deucher, Alexander

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Rex Zhu
> Sent: Wednesday, April 26, 2017 4:36 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Zhu, Rex
> Subject: [PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.
> 
> Change-Id: I46d7f2531eaec8135ccaf64b5ce8abc433c54e1a
> Signed-off-by: Rex Zhu 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 10 --
>  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 10 --
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 10 --
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c |  4 ++--
>  4 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index f7579de..25c3703 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -3719,6 +3719,12 @@ static void gfx_v6_0_get_cu_info(struct
> amdgpu_device *adev)
>   u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
>   struct amdgpu_cu_info *cu_info = >gfx.cu_info;
>   unsigned disable_masks[4 * 2];
> + u32 ao_cu_num;
> +
> + if (adev->flags & AMD_IS_APU)
> + ao_cu_num = 2;
> + else
> + ao_cu_num = adev->gfx.config.max_cu_per_sh;
> 
>   memset(cu_info, 0, sizeof(*cu_info));
> 
> @@ -3737,9 +3743,9 @@ static void gfx_v6_0_get_cu_info(struct
> amdgpu_device *adev)
>   bitmap = gfx_v6_0_get_cu_enabled(adev);
>   cu_info->bitmap[i][j] = bitmap;
> 
> - for (k = 0; k < 16; k++) {
> + for (k = 0; k < adev->gfx.config.max_cu_per_sh; k++)
> {
>   if (bitmap & mask) {
> - if (counter < 2)
> + if (counter < ao_cu_num)
>   ao_bitmap |= mask;
>   counter ++;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index fa5a0d2..75cca54 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -5339,6 +5339,12 @@ static void gfx_v7_0_get_cu_info(struct
> amdgpu_device *adev)
>   u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
>   struct amdgpu_cu_info *cu_info = >gfx.cu_info;
>   unsigned disable_masks[4 * 2];
> + u32 ao_cu_num;
> +
> + if (adev->flags & AMD_IS_APU)
> + ao_cu_num = 2;
> + else
> + ao_cu_num = adev->gfx.config.max_cu_per_sh;
> 
>   memset(cu_info, 0, sizeof(*cu_info));
> 
> @@ -5357,9 +5363,9 @@ static void gfx_v7_0_get_cu_info(struct
> amdgpu_device *adev)
>   bitmap = gfx_v7_0_get_cu_active_bitmap(adev);
>   cu_info->bitmap[i][j] = bitmap;
> 
> - for (k = 0; k < 16; k ++) {
> + for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++)
> {
>   if (bitmap & mask) {
> - if (counter < 2)
> + if (counter < ao_cu_num)
>   ao_bitmap |= mask;
>   counter ++;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 2ff5f19..0025ef6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -7098,9 +7098,15 @@ static void gfx_v8_0_get_cu_info(struct
> amdgpu_device *adev)
>   u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
>   struct amdgpu_cu_info *cu_info = >gfx.cu_info;
>   unsigned disable_masks[4 * 2];
> + u32 ao_cu_num;
> 
>   memset(cu_info, 0, sizeof(*cu_info));
> 
> + if (adev->flags & AMD_IS_APU)
> + ao_cu_num = 2;
> + else
> + ao_cu_num = adev->gfx.config.max_cu_per_sh;
> +
>   amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
> 
>   mutex_lock(>grbm_idx_mutex);
> @@ -7116,9 +7122,9 @@ static void gfx_v8_0_get_cu_info(struct
> amdgpu_device *adev)
>   bitmap = gfx_v8_0_get_cu_active_bitmap(adev);
>   cu_info->bitmap[i][j] = bitmap;
> 
> - for (k = 0; k < 16; k ++) {
> + for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++)
> {
>   if (bitmap & mask) {
> - if (counter < 2)
> + if (counter < ao_cu_num)
>   ao_bitmap |= mask;
>   counter ++;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 42cfd3b..13da795 100644
> ---

Re: [PATCH] Improve pipe split between amdgpu and amdkfd v4

2017-04-26 Thread Andres Rodriguez


Hey Oded,

Let us know if you can reproduce the kfdtest results on your side.

Regards,
Andres

On 2017-04-21 04:02 PM, Andres Rodriguez wrote:

V4 updates:
 * Rebased onto latest 4.12-wip (ba16d6c), since I got test reports that the
   patch series failed to apply

V3 updates:
 * Fixed kfd set resources grabbing MEC1 queues
 * kfdtest now passes on Kaveri with the amdgpu or radeon driver

V2 updates:
 * Fixed wrong HPD offset in compute_pipe_init for gfx7
 * Fixed compute_pipe_init using wrong ME for gfx7


This is a split of patches that are ready to land from the series:
Add support for high priority scheduling in amdgpu v8

I've included Felix and Alex's feedback from the thread above. This includes:
 * Separate MEC_HPD_SIZE rename into a separate patch (patch 01)
 * Added a patch to fix the kgd_hqd_load bug Felix pointed out (patch 06)
 * Fixes for various off-by-one errors
 * Use gfx_v8_0_deactivate_hqd

Only comment I didn't address was changing the queue allocation policy for
gfx9 (similar to gfx7/8). See inline reply in that thread for more details
on why this was skipped.


Series available in the wip-queue-policy-v4 branch at:
g...@github.com:lostgoat/linux.git
Or:
https://github.com/lostgoat/linux.git



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Christian König


Am 26.04.2017 um 11:57 schrieb Dave Airlie:

On 26 April 2017 at 18:45, Christian König  wrote:

Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported to
an fd and imported on the other side we get the same sync_obj again?

In other words the fd is exported and imported only once and the expectation
is that we fence containing it will change on each signaling operation.

As far as I can see with the current implementation you get two sync_objects
on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.


Ah, ok that makes some more sense.


You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.


I would prefer dealing only with one type of fd in userspace, but that 
approach should work as well.



I'd like to move any drm APIs away from sync_file to avoid the fd wastages,


That sounds like it make sense, but I would still rather vote for 
extending the sync_file interface than deprecating it with another one.



I'd also like to move the driver specific fences to syncobj where I can.


I'm pretty sure that is not a good idea.

Beating the sequence based fence values we currently use for CPU sync in 
performance would be pretty hard. E.g. at least on amdgpu we can even 
check those by doing a simple 64bit read from a memory address, without 
IOCTL or any other overhead involved.


Regards,
Christian.



Dave.



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Ville Syrjälä

On Wed, Apr 26, 2017 at 11:00:09AM +0900, Michel Dänzer wrote:
> On 25/04/17 06:52 PM, Ville Syrjälä wrote:
> > On Tue, Apr 25, 2017 at 12:18:52PM +0900, Michel Dänzer wrote:
> >> On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> >>> +#ifdef __BIG_ENDIAN
> >>> + switch (bpp) {
> >>> + case 8:
> >>> + fmt = DRM_FORMAT_C8;
> >>> + break;
> >>> + case 24:
> >>> + fmt = DRM_FORMAT_BGR888;
> >>> + break;
> >>
> >> BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> > 
> > To 8bpp no, but it can easily apply to 24bpp.
> 
> Any byte swapping rips apart the bytes of a 24bpp pixel, so those
> formats only make sense as straight array formats.

In my book little endian just means "lsb is stored in the lowest
memory address". The fact that your CPU/GPU can't do 3 byte swaps
is not relevant for that definition IMO.

-- 
Ville Syrjälä
Intel OTC
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Gerd Hoffmann

>   uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>   {
>   uint32_t fmt;
>   #ifdef __BIG_ENDIAN
>   enum { LITTLE_ENDIAN = 0 };
>   #else
>   enum { LITTLE_ENDIAN = 1 };
>   #endif
>   /* ... */
> 
> (using an enum for compile-time constness)
> 
> and then
>   fmt = DRM_FORMAT_ARGB;
> becomes
>   fmt = LITTLE_ENDIAN ? DRM_FORMAT_ARGB : DRM_FORMAT_BGRA;
> 
> Might be easier to read than duplicating the whole switch?

Well, there are more differences, like rgb565 and xrgb2101010 not being
supported for bigendian, so it isn't *that* simple.

cheers,
  Gerd

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Eric Engestrom

On Wednesday, 2017-04-26 07:53:10 +0200, Gerd Hoffmann wrote:
> On Di, 2017-04-25 at 12:18 +0900, Michel Dänzer wrote:
> > On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> > > Return correct fourcc codes on bigendian.  Drivers must be adapted to
> > > this change.
> > > 
> > > Signed-off-by: Gerd Hoffmann 
> > 
> > Just to reiterate, this won't work for the radeon driver, which programs
> > the GPU to use (effectively, per the current definition that these are
> > little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
> > DRM_FORMAT_BGRX with >= R600.
> 
> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
> 
> > > +#ifdef __BIG_ENDIAN
> > > + switch (bpp) {
> > > + case 8:
> > > + fmt = DRM_FORMAT_C8;
> > > + break;
> > > + case 24:
> > > + fmt = DRM_FORMAT_BGR888;
> > > + break;
> > 
> > BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> 
> I could move the 8 bpp case out of the #ifdef somehow, but code
> readability will suffer then I think ...

How about something like this?

uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
{
uint32_t fmt;
#ifdef __BIG_ENDIAN
enum { LITTLE_ENDIAN = 0 };
#else
enum { LITTLE_ENDIAN = 1 };
#endif
/* ... */

(using an enum for compile-time constness)

and then
fmt = DRM_FORMAT_ARGB;
becomes
fmt = LITTLE_ENDIAN ? DRM_FORMAT_ARGB : DRM_FORMAT_BGRA;

Might be easier to read than duplicating the whole switch?

> 
> For 24 we have different byte orderings, but yes, you can't switch from
> one to the other with byteswapping.  Probably one of the reasons why
> this format is pretty much out of fashion these days ...
> 
> cheers,
>   Gerd
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: remove unused and mostly unimplemented CGS functions

2017-04-26 Thread Christian König

From: Christian König 

Those functions are all unused and some not even implemented.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c  | 233 
 drivers/gpu/drm/amd/include/cgs_common.h | 303 ---
 2 files changed, 536 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index a1a2d44..93f5dcc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -42,82 +42,6 @@ struct amdgpu_cgs_device {
struct amdgpu_device *adev =\
((struct amdgpu_cgs_device *)cgs_device)->adev
 
-static int amdgpu_cgs_gpu_mem_info(struct cgs_device *cgs_device, enum 
cgs_gpu_mem_type type,
-  uint64_t *mc_start, uint64_t *mc_size,
-  uint64_t *mem_size)
-{
-   CGS_FUNC_ADEV;
-   switch(type) {
-   case CGS_GPU_MEM_TYPE__VISIBLE_CONTIG_FB:
-   case CGS_GPU_MEM_TYPE__VISIBLE_FB:
-   *mc_start = 0;
-   *mc_size = adev->mc.visible_vram_size;
-   *mem_size = adev->mc.visible_vram_size - adev->vram_pin_size;
-   break;
-   case CGS_GPU_MEM_TYPE__INVISIBLE_CONTIG_FB:
-   case CGS_GPU_MEM_TYPE__INVISIBLE_FB:
-   *mc_start = adev->mc.visible_vram_size;
-   *mc_size = adev->mc.real_vram_size - adev->mc.visible_vram_size;
-   *mem_size = *mc_size;
-   break;
-   case CGS_GPU_MEM_TYPE__GART_CACHEABLE:
-   case CGS_GPU_MEM_TYPE__GART_WRITECOMBINE:
-   *mc_start = adev->mc.gtt_start;
-   *mc_size = adev->mc.gtt_size;
-   *mem_size = adev->mc.gtt_size - adev->gart_pin_size;
-   break;
-   default:
-   return -EINVAL;
-   }
-
-   return 0;
-}
-
-static int amdgpu_cgs_gmap_kmem(struct cgs_device *cgs_device, void *kmem,
-   uint64_t size,
-   uint64_t min_offset, uint64_t max_offset,
-   cgs_handle_t *kmem_handle, uint64_t *mcaddr)
-{
-   CGS_FUNC_ADEV;
-   int ret;
-   struct amdgpu_bo *bo;
-   struct page *kmem_page = vmalloc_to_page(kmem);
-   int npages = ALIGN(size, PAGE_SIZE) >> PAGE_SHIFT;
-
-   struct sg_table *sg = drm_prime_pages_to_sg(_page, npages);
-   ret = amdgpu_bo_create(adev, size, PAGE_SIZE, false,
-  AMDGPU_GEM_DOMAIN_GTT, 0, sg, NULL, );
-   if (ret)
-   return ret;
-   ret = amdgpu_bo_reserve(bo, false);
-   if (unlikely(ret != 0))
-   return ret;
-
-   /* pin buffer into GTT */
-   ret = amdgpu_bo_pin_restricted(bo, AMDGPU_GEM_DOMAIN_GTT,
-  min_offset, max_offset, mcaddr);
-   amdgpu_bo_unreserve(bo);
-
-   *kmem_handle = (cgs_handle_t)bo;
-   return ret;
-}
-
-static int amdgpu_cgs_gunmap_kmem(struct cgs_device *cgs_device, cgs_handle_t 
kmem_handle)
-{
-   struct amdgpu_bo *obj = (struct amdgpu_bo *)kmem_handle;
-
-   if (obj) {
-   int r = amdgpu_bo_reserve(obj, false);
-   if (likely(r == 0)) {
-   amdgpu_bo_unpin(obj);
-   amdgpu_bo_unreserve(obj);
-   }
-   amdgpu_bo_unref();
-
-   }
-   return 0;
-}
-
 static int amdgpu_cgs_alloc_gpu_mem(struct cgs_device *cgs_device,
enum cgs_gpu_mem_type type,
uint64_t size, uint64_t align,
@@ -349,96 +273,6 @@ static void amdgpu_cgs_write_ind_register(struct 
cgs_device *cgs_device,
WARN(1, "Invalid indirect register space");
 }
 
-static uint8_t amdgpu_cgs_read_pci_config_byte(struct cgs_device *cgs_device, 
unsigned addr)
-{
-   CGS_FUNC_ADEV;
-   uint8_t val;
-   int ret = pci_read_config_byte(adev->pdev, addr, );
-   if (WARN(ret, "pci_read_config_byte error"))
-   return 0;
-   return val;
-}
-
-static uint16_t amdgpu_cgs_read_pci_config_word(struct cgs_device *cgs_device, 
unsigned addr)
-{
-   CGS_FUNC_ADEV;
-   uint16_t val;
-   int ret = pci_read_config_word(adev->pdev, addr, );
-   if (WARN(ret, "pci_read_config_word error"))
-   return 0;
-   return val;
-}
-
-static uint32_t amdgpu_cgs_read_pci_config_dword(struct cgs_device *cgs_device,
-unsigned addr)
-{
-   CGS_FUNC_ADEV;
-   uint32_t val;
-   int ret = pci_read_config_dword(adev->pdev, addr, );
-   if (WARN(ret, "pci_read_config_dword error"))
-   return 0;
-   return val;
-}
-
-static void amdgpu_cgs_write_pci_config_byte(struct cgs_device *cgs_device, 
unsigned addr,
-uint8_t

Re: [PATCH 5/6] drm/amdgpu: implement grab dedicated vmid V2

2017-04-26 Thread Christian König


Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

v2: move sync waiting only when flush needs

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 ++
  1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 214ac50..bce7701 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -402,6 +402,63 @@ static bool amdgpu_vm_dedicated_vmid_ready(struct 
amdgpu_vm *vm, unsigned vmhub)
return !!vm->dedicated_vmid[vmhub];
  }
  
+static int amdgpu_vm_grab_dedicated_vmid(struct amdgpu_vm *vm,

+struct amdgpu_ring *ring,
+struct amdgpu_sync *sync,
+struct fence *fence,
+struct amdgpu_job *job)
+{
+   struct amdgpu_device *adev = ring->adev;
+   unsigned vmhub = ring->funcs->vmhub;
+   struct amdgpu_vm_id *id = vm->dedicated_vmid[vmhub];
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+   struct fence *updates = sync->last_vm_update;
+   int r = 0;
+   struct fence *flushed, *tmp;
+   bool needs_flush = false;
+
+   mutex_lock(_mgr->lock);
+   if (amdgpu_vm_had_gpu_reset(adev, id))
+   needs_flush = true;
+
+   flushed  = id->flushed_updates;
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed)))
+   needs_flush = true;
+   if (needs_flush) {
+   tmp = amdgpu_sync_get_fence(>active);
+   if (tmp) {
+   r = amdgpu_sync_fence(adev, sync, tmp);
+   fence_put(tmp);
+   mutex_unlock(_mgr->lock);
+   return r;
+   }
+   }
+
+   /* Good we can use this VMID. Remember this submission as
+   * user of the VMID.
+   */
+   r = amdgpu_sync_fence(ring->adev, >active, fence);
+   if (r)
+   goto out;
+
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed))) {
+   fence_put(id->flushed_updates);
+   id->flushed_updates = fence_get(updates);
+   }
+   id->pd_gpu_addr = job->vm_pd_addr;
+   id->current_gpu_reset_count = atomic_read(>gpu_reset_counter);
+   atomic64_set(>owner, vm->client_id);
+   job->vm_needs_flush = needs_flush;
+
+   job->vm_id = id - id_mgr->ids;
+   trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+   mutex_unlock(_mgr->lock);
+   return r;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -426,6 +483,10 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
unsigned i;
int r = 0;
  
+	if (amdgpu_vm_dedicated_vmid_ready(vm, vmhub))

+   return amdgpu_vm_grab_dedicated_vmid(vm, ring, sync,
+fence, job);


That is racy as well. The reserved VMID could be unreserved while we try 
to grab it leading to a NULL pointer dereference in 
amdgpu_vm_grab_dedicated_vmid().


Christian.


+
fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, GFP_KERNEL);
if (!fences)
return -ENOMEM;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread Christian König


Am 26.04.2017 um 13:10 schrieb Chunming Zhou:

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++---
  1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device 
*adev,
atomic_read(>gpu_reset_counter);
  }
  
+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, unsigned vmhub)

+{
+   return !!vm->dedicated_vmid[vmhub];
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
return r;
  }
  
+static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device *adev,

+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   if (vm->dedicated_vmid[vmhub]) {
+   list_add(>dedicated_vmid[vmhub]->list,
+   _mgr->ids_lru);
+   vm->dedicated_vmid[vmhub] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr;
+   struct amdgpu_vm_id *idle;
+   int r;
+
+   id_mgr = >vm_manager.id_mgr[vmhub];
+   mutex_lock(_mgr->lock);
+   /* Select the first entry VMID */
+   idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+   list_del_init(>list);
+   vm->dedicated_vmid[vmhub] = idle;


We need a check here if there isn't an ID already reserved for the VM.

Additional to that I would still rename the field to reserved_vmid, that 
describes better what it is actually doing.



+   mutex_unlock(_mgr->lock);
+
+   r = amdgpu_sync_wait(>active);
+   if (r)
+   goto err;


This is racy. A command submission could happen while we wait for the id 
to become idle.


The easiest way to solve this is to hold the lock while waiting for the 
ID to become available.



+
+   return 0;
+err:
+   amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+   return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
  
  	amdgpu_vm_free_levels(>root);

fence_put(vm->last_dir_update);
-   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-   struct amdgpu_vm_id_manager *id_mgr =
-   >vm_manager.id_mgr[i];
-
-   mutex_lock(_mgr->lock);
-   if (vm->dedicated_vmid[i]) {
-   list_add(>dedicated_vmid[i]->list,
-_mgr->ids_lru);
-   vm->dedicated_vmid[i] = NULL;
-   }
-   mutex_unlock(_mgr->lock);
-   }
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   amdgpu_vm_free_dedicated_vmid(adev, vm, i);
  }
  
  /**

@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   int r;
  
  	switch (args->in.op) {

case AMDGPU_VM_OP_RESERVE_VMID:
+   /* current, we only have requirement to reserve vmid from 
gfxhub */
+   if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+   r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);


This is racy as well.

Two threads could try to reserve a VMID at the same time. You need to 
integrate the check if it's already allocated into 
amdgpu_vm_alloc_dedicated_vmid().


Regards,
Christian.


+   if (r)
+   return r;
+   }
+   break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
-   return -EINVAL;
+   amdgpu_vm_free_dedicated_vmid(adev, >vm, 0);
break;
default:
return -EINVAL;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Gerd Hoffmann

  Hi,

> >> Just to reiterate, this won't work for the radeon driver, which programs
> >> the GPU to use (effectively, per the current definition that these are
> >> little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
> >> DRM_FORMAT_BGRX with >= R600.
> > 
> > Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
> 
> Using a GPU byte swapping mechanism which only affects CPU access to
> video RAM.

That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
another thread?

Ok, so the cpu view to fbdev is DRM_FORMAT_BGRX in all cases.

What about dumb bos?  You've mentioned the swap flag isn't used for
those.  Which implies they are in little endian byte order (both gpu and
cpu view).  Does the modesetting driver work correctly on that hardware?

cheers,
  Gerd

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 6/6] drm/amdgpu: bump module verion for reserved vmid

2017-04-26 Thread Chunming Zhou

Change-Id: I1065e0430ed44f7ee6c29214b72e35a7343ea02b
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 55322b4..6799829 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -64,9 +64,10 @@
  * - 3.12.0 - Add query for double offchip LDS buffers
  * - 3.13.0 - Add PRT support
  * - 3.14.0 - Fix race in amdgpu_ctx_get_fence() and note new functionality
+ * - 3.15.0 - Add reserved vmid support
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   14
+#define KMS_DRIVER_MINOR   15
 #define KMS_DRIVER_PATCHLEVEL  0
 
 int amdgpu_vram_limit = 0;
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/6] drm/amdgpu: reserve/unreserve vmid by vm ioctl v3

2017-04-26 Thread Chunming Zhou

add reserve/unreserve vmid funtions.
v3:
only reserve vmid from gfxhub

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 70 +++---
 1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index ec87d64..a783e8e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,11 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device 
*adev,
atomic_read(>gpu_reset_counter);
 }
 
+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm, unsigned 
vmhub)
+{
+   return !!vm->dedicated_vmid[vmhub];
+}
+
 /**
  * amdgpu_vm_grab_id - allocate the next free VMID
  *
@@ -546,6 +551,47 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
return r;
 }
 
+static void amdgpu_vm_free_dedicated_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   if (vm->dedicated_vmid[vmhub]) {
+   list_add(>dedicated_vmid[vmhub]->list,
+   _mgr->ids_lru);
+   vm->dedicated_vmid[vmhub] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+}
+
+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm,
+ unsigned vmhub)
+{
+   struct amdgpu_vm_id_manager *id_mgr;
+   struct amdgpu_vm_id *idle;
+   int r;
+
+   id_mgr = >vm_manager.id_mgr[vmhub];
+   mutex_lock(_mgr->lock);
+   /* Select the first entry VMID */
+   idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
+   list_del_init(>list);
+   vm->dedicated_vmid[vmhub] = idle;
+   mutex_unlock(_mgr->lock);
+
+   r = amdgpu_sync_wait(>active);
+   if (r)
+   goto err;
+
+   return 0;
+err:
+   amdgpu_vm_free_dedicated_vmid(adev, vm, vmhub);
+   return r;
+}
+
 static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
 {
struct amdgpu_device *adev = ring->adev;
@@ -2316,18 +2362,8 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
-   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
-   struct amdgpu_vm_id_manager *id_mgr =
-   >vm_manager.id_mgr[i];
-
-   mutex_lock(_mgr->lock);
-   if (vm->dedicated_vmid[i]) {
-   list_add(>dedicated_vmid[i]->list,
-_mgr->ids_lru);
-   vm->dedicated_vmid[i] = NULL;
-   }
-   mutex_unlock(_mgr->lock);
-   }
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   amdgpu_vm_free_dedicated_vmid(adev, vm, i);
 }
 
 /**
@@ -2400,11 +2436,19 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   int r;
 
switch (args->in.op) {
case AMDGPU_VM_OP_RESERVE_VMID:
+   /* current, we only have requirement to reserve vmid from 
gfxhub */
+   if (!amdgpu_vm_dedicated_vmid_ready(>vm, 0)) {
+   r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm, 0);
+   if (r)
+   return r;
+   }
+   break;
case AMDGPU_VM_OP_UNRESERVE_VMID:
-   return -EINVAL;
+   amdgpu_vm_free_dedicated_vmid(adev, >vm, 0);
break;
default:
return -EINVAL;
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/6] drm/amdgpu: add limitation for dedicated vm number v3

2017-04-26 Thread Chunming Zhou

v2: move #define to amdgpu_vm.h
v3: move reserved vmid counter to id_manager,
and increase counter before allocating vmid

Change-Id: Ie5958cf6dbdc1c8278e61d9158483472d6f5c6e3
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a783e8e..214ac50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -562,6 +562,7 @@ static void amdgpu_vm_free_dedicated_vmid(struct 
amdgpu_device *adev,
list_add(>dedicated_vmid[vmhub]->list,
_mgr->ids_lru);
vm->dedicated_vmid[vmhub] = NULL;
+   atomic_dec(_mgr->reserved_vmid);
}
mutex_unlock(_mgr->lock);
 }
@@ -575,6 +576,12 @@ static int amdgpu_vm_alloc_dedicated_vmid(struct 
amdgpu_device *adev,
int r;
 
id_mgr = >vm_manager.id_mgr[vmhub];
+   if (atomic_inc_return(_mgr->reserved_vmid) >
+   AMDGPU_VM_MAX_RESERVED_VMID) {
+   DRM_ERROR("Over limitation of reserved vmid\n");
+   atomic_dec(_mgr->reserved_vmid);
+   return -EINVAL;
+   }
mutex_lock(_mgr->lock);
/* Select the first entry VMID */
idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id, list);
@@ -2383,6 +2390,7 @@ void amdgpu_vm_manager_init(struct amdgpu_device *adev)
 
mutex_init(_mgr->lock);
INIT_LIST_HEAD(_mgr->ids_lru);
+   atomic_set(_mgr->reserved_vmid, 0);
 
/* skip over VMID 0, since it is the system VM */
for (j = 1; j < id_mgr->num_ids; ++j) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 97ad67d..067c262 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -84,6 +84,8 @@
 
 /* hardcode that limit for now */
 #define AMDGPU_VA_RESERVED_SIZE(8 << 20)
+/* max vmids dedicated for process */
+#define AMDGPU_VM_MAX_RESERVED_VMID1
 
 struct amdgpu_vm_pt {
struct amdgpu_bo*bo;
@@ -157,6 +159,7 @@ struct amdgpu_vm_id_manager {
unsignednum_ids;
struct list_headids_lru;
struct amdgpu_vm_id ids[AMDGPU_NUM_VM];
+   atomic_treserved_vmid;
 };
 
 struct amdgpu_vm_manager {
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 5/6] drm/amdgpu: implement grab dedicated vmid V2

2017-04-26 Thread Chunming Zhou

v2: move sync waiting only when flush needs

Change-Id: I64da2701c9fdcf986afb90ba1492a78d5bef1b6c
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 ++
 1 file changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 214ac50..bce7701 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -402,6 +402,63 @@ static bool amdgpu_vm_dedicated_vmid_ready(struct 
amdgpu_vm *vm, unsigned vmhub)
return !!vm->dedicated_vmid[vmhub];
 }
 
+static int amdgpu_vm_grab_dedicated_vmid(struct amdgpu_vm *vm,
+struct amdgpu_ring *ring,
+struct amdgpu_sync *sync,
+struct fence *fence,
+struct amdgpu_job *job)
+{
+   struct amdgpu_device *adev = ring->adev;
+   unsigned vmhub = ring->funcs->vmhub;
+   struct amdgpu_vm_id *id = vm->dedicated_vmid[vmhub];
+   struct amdgpu_vm_id_manager *id_mgr = >vm_manager.id_mgr[vmhub];
+   struct fence *updates = sync->last_vm_update;
+   int r = 0;
+   struct fence *flushed, *tmp;
+   bool needs_flush = false;
+
+   mutex_lock(_mgr->lock);
+   if (amdgpu_vm_had_gpu_reset(adev, id))
+   needs_flush = true;
+
+   flushed  = id->flushed_updates;
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed)))
+   needs_flush = true;
+   if (needs_flush) {
+   tmp = amdgpu_sync_get_fence(>active);
+   if (tmp) {
+   r = amdgpu_sync_fence(adev, sync, tmp);
+   fence_put(tmp);
+   mutex_unlock(_mgr->lock);
+   return r;
+   }
+   }
+
+   /* Good we can use this VMID. Remember this submission as
+   * user of the VMID.
+   */
+   r = amdgpu_sync_fence(ring->adev, >active, fence);
+   if (r)
+   goto out;
+
+   if (updates && (!flushed || updates->context != flushed->context ||
+   fence_is_later(updates, flushed))) {
+   fence_put(id->flushed_updates);
+   id->flushed_updates = fence_get(updates);
+   }
+   id->pd_gpu_addr = job->vm_pd_addr;
+   id->current_gpu_reset_count = atomic_read(>gpu_reset_counter);
+   atomic64_set(>owner, vm->client_id);
+   job->vm_needs_flush = needs_flush;
+
+   job->vm_id = id - id_mgr->ids;
+   trace_amdgpu_vm_grab_id(vm, ring, job);
+out:
+   mutex_unlock(_mgr->lock);
+   return r;
+}
+
 /**
  * amdgpu_vm_grab_id - allocate the next free VMID
  *
@@ -426,6 +483,10 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
unsigned i;
int r = 0;
 
+   if (amdgpu_vm_dedicated_vmid_ready(vm, vmhub))
+   return amdgpu_vm_grab_dedicated_vmid(vm, ring, sync,
+fence, job);
+
fences = kmalloc_array(sizeof(void *), id_mgr->num_ids, GFP_KERNEL);
if (!fences)
return -ENOMEM;
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/6] drm/amdgpu: add vm ioctl

2017-04-26 Thread Chunming Zhou

It will be used for reserving vmid.

Change-Id: Ib7169ea999690c8e82d0dcbccdd2d97760c0270a
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 18 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  1 +
 include/uapi/drm/amdgpu_drm.h   | 22 ++
 4 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 052e6a5..8ea33c8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1062,6 +1062,7 @@ int amdgpu_get_vblank_timestamp_kms(struct drm_device 
*dev, unsigned int pipe,
 const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
/* KMS */
DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a8cd23b..8698177 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2379,3 +2379,21 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev)
}
}
 }
+
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
+{
+   union drm_amdgpu_vm *args = data;
+   struct amdgpu_device *adev = dev->dev_private;
+   struct amdgpu_fpriv *fpriv = filp->driver_priv;
+
+   switch (args->in.op) {
+   case AMDGPU_VM_OP_RESERVE_VMID:
+   case AMDGPU_VM_OP_UNRESERVE_VMID:
+   return -EINVAL;
+   break;
+   default:
+   return -EINVAL;
+   }
+
+   return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 8eba2d3..16e0aaa 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -248,5 +248,6 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,
 void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
  struct amdgpu_bo_va *bo_va);
 void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t vm_size);
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp);
 
 #endif
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index b98b371..1c95775 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -51,6 +51,7 @@
 #define DRM_AMDGPU_GEM_OP  0x10
 #define DRM_AMDGPU_GEM_USERPTR 0x11
 #define DRM_AMDGPU_WAIT_FENCES 0x12
+#define DRM_AMDGPU_VM  0x13
 
 /* hybrid specific ioctls */
 #define DRM_AMDGPU_SEM 0x5b
@@ -71,6 +72,7 @@
 #define DRM_IOCTL_AMDGPU_GEM_OPDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
 #define DRM_IOCTL_AMDGPU_GEM_USERPTR   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
 #define DRM_IOCTL_AMDGPU_WAIT_FENCES   DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
+#define DRM_IOCTL_AMDGPU_VMDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_VM, union drm_amdgpu_vm)
 
 /* hybrid specific ioctls */
 #define DRM_IOCTL_AMDGPU_GEM_DGMA  DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_DGMA, struct drm_amdgpu_gem_dgma)
@@ -212,6 +214,26 @@ struct drm_amdgpu_ctx_in {
union drm_amdgpu_ctx_out out;
 };
 
+/* vm ioctl */
+#define AMDGPU_VM_OP_RESERVE_VMID  1
+#define AMDGPU_VM_OP_UNRESERVE_VMID2
+
+struct drm_amdgpu_vm_in {
+   /** AMDGPU_VM_OP_* */
+   __u32   op;
+   __u32   flags;
+};
+
+struct drm_amdgpu_vm_out {
+   /** For future use, no flags defined so far */
+   __u64   flags;
+};
+
+union drm_amdgpu_vm {
+   struct drm_amdgpu_vm_in in;
+   struct drm_amdgpu_vm_out out;
+};
+
 /* sem related */
 #define AMDGPU_SEM_OP_CREATE_SEM1
 #define AMDGPU_SEM_OP_WAIT_SEM 2
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread Chunming Zhou

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8698177..ec87d64 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2163,10 +2163,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
unsigned ring_instance;
struct amdgpu_ring *ring;
struct amd_sched_rq *rq;
-   int r;
+   int r, i;
 
vm->va = RB_ROOT;
vm->client_id = atomic64_inc_return(>vm_manager.client_counter);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   vm->dedicated_vmid[i] = NULL;
spin_lock_init(>status_lock);
INIT_LIST_HEAD(>invalidated);
INIT_LIST_HEAD(>cleared);
@@ -2270,6 +2272,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 {
struct amdgpu_bo_va_mapping *mapping, *tmp;
bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+   int i;
 
if (vm->is_kfd_vm) {
struct amdgpu_vm_id_manager *id_mgr =
@@ -2313,6 +2316,18 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
 
amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+   struct amdgpu_vm_id_manager *id_mgr =
+   >vm_manager.id_mgr[i];
+
+   mutex_lock(_mgr->lock);
+   if (vm->dedicated_vmid[i]) {
+   list_add(>dedicated_vmid[i]->list,
+_mgr->ids_lru);
+   vm->dedicated_vmid[i] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+   }
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 16e0aaa..97ad67d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -123,6 +123,8 @@ struct amdgpu_vm {
 
/* client id */
u64 client_id;
+   /* dedicated vmid */
+   struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
/* each VM will map on CSA */
struct amdgpu_bo_va *csa_bo_va;
 
-- 
1.9.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Dave Airlie

On 26 April 2017 at 18:45, Christian König  wrote:
> Am 26.04.2017 um 05:28 schrieb Dave Airlie:
>>
>> Okay I've gone around the sun with these a few times, and
>> pretty much implemented what I said last week.
>>
>> This is pretty much a complete revamp.
>>
>> 1. sync objects are self contained drm objects, they
>> have a file reference so can be passed between processes.
>>
>> 2. Added a sync object wait interface modelled on the vulkan
>> fence waiting API.
>>
>> 3. sync_file interaction is explicitly different than
>> opaque fd passing, you import a sync file state into an
>> existing syncobj, or create a new sync_file from an
>> existing syncobj. This means no touching the sync file code
>> at all. \o/
>
>
> Doesn't that break the Vulkan requirement that if a sync_obj is exported to
> an fd and imported on the other side we get the same sync_obj again?
>
> In other words the fd is exported and imported only once and the expectation
> is that we fence containing it will change on each signaling operation.
>
> As far as I can see with the current implementation you get two sync_objects
> on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.

You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.

I'd like to move any drm APIs away from sync_file to avoid the fd wastages, I'd
also like to move the driver specific fences to syncobj where I can.

Dave.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread zhoucm1




On 2017年04月26日 17:45, Christian König wrote:

Am 26.04.2017 um 11:14 schrieb zhoucm1:



On 2017年04月26日 17:10, Christian König wrote:

Am 26.04.2017 um 11:05 schrieb zhoucm1:



On 2017年04月26日 16:49, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree.
I think the reserved vmid is dedicated to this vm, I don't know 
where this name doesn't make sense.


And if I'm not completely mistaken we still should only apply that 
to the GFX hub on Vega10.
David Mao required mmhub as well. IIRC, we don't have necessary to 
argue more on this.


I think we still have. There is no technical reason why we should 
use the reserved/dedicated VM for other engines than the one 
involved in the SQ trace.
How about removing SQ trace reason? is it ok just that one process 
wants to do experiment for other purpose? :)


In this case I would clearly NAK the whole approach.


Which makes me very difficult to talk between you and David Mao.

Regards,
David Zhou


Regards,
Christian.



Regards,
David Zhou


So I would say we should limit this to GFX and Compute jobs and only 
allocate the dedicated VMID for those.


Regards,
Christian.



Regards,
David Zhou


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+ list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx









___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread Christian König


Am 26.04.2017 um 11:14 schrieb zhoucm1:



On 2017年04月26日 17:10, Christian König wrote:

Am 26.04.2017 um 11:05 schrieb zhoucm1:



On 2017年04月26日 16:49, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree.
I think the reserved vmid is dedicated to this vm, I don't know 
where this name doesn't make sense.


And if I'm not completely mistaken we still should only apply that 
to the GFX hub on Vega10.
David Mao required mmhub as well. IIRC, we don't have necessary to 
argue more on this.


I think we still have. There is no technical reason why we should use 
the reserved/dedicated VM for other engines than the one involved in 
the SQ trace.
How about removing SQ trace reason? is it ok just that one process 
wants to do experiment for other purpose? :)


In this case I would clearly NAK the whole approach.

Regards,
Christian.



Regards,
David Zhou


So I would say we should limit this to GFX and Compute jobs and only 
allocate the dedicated VMID for those.


Regards,
Christian.



Regards,
David Zhou


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+ list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx







___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

2017-04-26 Thread Michel Dänzer

On 26/04/17 02:53 PM, Gerd Hoffmann wrote:
> On Di, 2017-04-25 at 12:18 +0900, Michel Dänzer wrote:
>> On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
>>> Return correct fourcc codes on bigendian.  Drivers must be adapted to
>>> this change.
>>>
>>> Signed-off-by: Gerd Hoffmann 
>>
>> Just to reiterate, this won't work for the radeon driver, which programs
>> the GPU to use (effectively, per the current definition that these are
>> little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
>> DRM_FORMAT_BGRX with >= R600.
> 
> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?

Using a GPU byte swapping mechanism which only affects CPU access to
video RAM.


>>> +#ifdef __BIG_ENDIAN
>>> +   switch (bpp) {
>>> +   case 8:
>>> +   fmt = DRM_FORMAT_C8;
>>> +   break;
>>> +   case 24:
>>> +   fmt = DRM_FORMAT_BGR888;
>>> +   break;
>>
>> BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> 
> I could move the 8 bpp case out of the #ifdef somehow, but code
> readability will suffer then I think ...

How so?

At least it would make clearer which formats are affected by endianness
and which aren't.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread zhoucm1




On 2017年04月26日 17:10, Christian König wrote:

Am 26.04.2017 um 11:05 schrieb zhoucm1:



On 2017年04月26日 16:49, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree.
I think the reserved vmid is dedicated to this vm, I don't know where 
this name doesn't make sense.


And if I'm not completely mistaken we still should only apply that 
to the GFX hub on Vega10.
David Mao required mmhub as well. IIRC, we don't have necessary to 
argue more on this.


I think we still have. There is no technical reason why we should use 
the reserved/dedicated VM for other engines than the one involved in 
the SQ trace.
How about removing SQ trace reason? is it ok just that one process wants 
to do experiment for other purpose? :)


Regards,
David Zhou


So I would say we should limit this to GFX and Compute jobs and only 
allocate the dedicated VMID for those.


Regards,
Christian.



Regards,
David Zhou


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+ list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm: Harmonize CIK ASIC support in radeon and amdgpu (v2)

2017-04-26 Thread Michel Dänzer

On 26/04/17 03:06 AM, Nicolai Hähnle wrote:
> On 25.04.2017 08:28, Michel Dänzer wrote:
>> On 22/04/17 02:05 AM, Felix Kuehling wrote:
>>
>> So, I'm starting to think we need a shared module for this, which
>> provides one or multiple module parameters to choose which driver to use
>> for CIK/SI[0], and provides the result to the amdgpu/radeon drivers.
>> That might make things easier for amdgpu-pro / other standalone amdgpu
>> versions in the long run as well, as they could add files to
>> /etc/modprobe.d/ choosing themselves by default, without having to
>> blacklist radeon.
>>
>> What do you guys think?
> 
> I suspect that adding an entire module

That sounds quite dramatic. :) Which particular aspect(s) of that are
you concerned about?


> just to select between two other modules

FWIW, I suspect that could actually end up smaller overall than the
alternatives we've discussed at least by some metrics.

And maybe this module could be used for sharing / coordinating more
stuff between the drivers in the future.


> is the kind of thing that should be discussed in a wider audience first.

I'd like to have at least a proof of concept before that.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread Christian König


Am 26.04.2017 um 11:05 schrieb zhoucm1:



On 2017年04月26日 16:49, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree.
I think the reserved vmid is dedicated to this vm, I don't know where 
this name doesn't make sense.


And if I'm not completely mistaken we still should only apply that to 
the GFX hub on Vega10.
David Mao required mmhub as well. IIRC, we don't have necessary to 
argue more on this.


I think we still have. There is no technical reason why we should use 
the reserved/dedicated VM for other engines than the one involved in the 
SQ trace.


So I would say we should limit this to GFX and Compute jobs and only 
allocate the dedicated VMID for those.


Regards,
Christian.



Regards,
David Zhou


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+ list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve vmid by vm ioctl

2017-04-26 Thread Christian König


Am 26.04.2017 um 11:04 schrieb Zhang, Jerry (Junwei):

On 04/26/2017 04:51 PM, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):



On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
++
  1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index acf9102..5f4dcc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,17 @@ static bool amdgpu_vm_had_gpu_reset(struct
amdgpu_device *adev,
  atomic_read(>gpu_reset_counter);
  }

+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm)
+{
+unsigned vmhub;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+if (!vm->dedicated_vmid[vmhub])
+return false;


Just note:
Seemingly for pre-gmcv9, it will always allocate one more 
dedicated/reserved

vmid for the 2nd un-used vmhub.
anyway it will not be used either.


It's even worse. IIRC we don't initialize the 2nd hub on pre-gmcv9. 
So that

could work with uninitialized data.


I saw AMDGPU_MAX_VMHUBS is defined 2 for all ASICs.
So the 2nd dedicated_vmid here will be 0 always, returning false.
Right?

Additionally, in amdgpu_vm_manager_init() the id_mgr[i] is initialized 
in loop of AMDGPU_MAX_VMHUBS as well.

Thus I though both vmhub are initialized.
Any missing, please correct me.
Thanks.


Indeed you are right we do always initialize all HUBs here, so that 
should work but is still a bit awkward.


Christian.



Jerry



Christian.



Jerry

+}
+return true;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +557,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, 
struct

amdgpu_ring *ring,
  return r;
  }

+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+unsigned vmhub;
+int r;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct 
amdgpu_vm_id,

+list);
+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;
+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;
+}
+
+return 0;
+err:
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub])
+ list_add(>dedicated_vmid[vmhub]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+mutex_unlock(_mgr->lock);
+}
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring 
*ring)

  {
  struct amdgpu_device *adev = ring->adev;
@@ -2379,9 +2429,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
void

*data, struct drm_file *filp)
  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;

  switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+if (!amdgpu_vm_dedicated_vmid_ready(>vm)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm);


Could you change the return value in this ready checking func?
Usually we can easily get to know this kind of things.
if (sth_ready())
do_sth();



+if (r)
+return r;
+}
  break;
  default:
  return -EINVAL;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread zhoucm1




On 2017年04月26日 16:49, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree.
I think the reserved vmid is dedicated to this vm, I don't know where 
this name doesn't make sense.


And if I'm not completely mistaken we still should only apply that to 
the GFX hub on Vega10.
David Mao required mmhub as well. IIRC, we don't have necessary to argue 
more on this.


Regards,
David Zhou


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx





___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve vmid by vm ioctl

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/26/2017 04:51 PM, Christian König wrote:

Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):



On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
++
  1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index acf9102..5f4dcc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,17 @@ static bool amdgpu_vm_had_gpu_reset(struct
amdgpu_device *adev,
  atomic_read(>gpu_reset_counter);
  }

+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm)
+{
+unsigned vmhub;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+if (!vm->dedicated_vmid[vmhub])
+return false;


Just note:
Seemingly for pre-gmcv9, it will always allocate one more dedicated/reserved
vmid for the 2nd un-used vmhub.
anyway it will not be used either.


It's even worse. IIRC we don't initialize the 2nd hub on pre-gmcv9. So that
could work with uninitialized data.


I saw AMDGPU_MAX_VMHUBS is defined 2 for all ASICs.
So the 2nd dedicated_vmid here will be 0 always, returning false.
Right?

Additionally, in amdgpu_vm_manager_init() the id_mgr[i] is initialized in loop 
of AMDGPU_MAX_VMHUBS as well.

Thus I though both vmhub are initialized.
Any missing, please correct me.
Thanks.

Jerry



Christian.



Jerry

+}
+return true;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +557,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct
amdgpu_ring *ring,
  return r;
  }

+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+unsigned vmhub;
+int r;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id,
+list);
+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;
+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;
+}
+
+return 0;
+err:
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub])
+ list_add(>dedicated_vmid[vmhub]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+mutex_unlock(_mgr->lock);
+}
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
  struct amdgpu_device *adev = ring->adev;
@@ -2379,9 +2429,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void
*data, struct drm_file *filp)
  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;

  switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+if (!amdgpu_vm_dedicated_vmid_ready(>vm)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm);


Could you change the return value in this ready checking func?
Usually we can easily get to know this kind of things.
if (sth_ready())
do_sth();



+if (r)
+return r;
+}
  break;
  default:
  return -EINVAL;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/6] drm/amdgpu: add limitation for dedicated vm number v2

2017-04-26 Thread Christian König


Am 26.04.2017 um 09:10 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

v2: move #define to amdgpu_vm.h

Change-Id: Ie5958cf6dbdc1c8278e61d9158483472d6f5c6e3
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 ++
  4 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index 0831cd2..ba9d3d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1591,6 +1591,7 @@ struct amdgpu_device {
  struct amdgpu_dummy_pagedummy_page;
  struct amdgpu_vm_managervm_manager;
  struct amdgpu_vmhub vmhub[AMDGPU_MAX_VMHUBS];
+atomic_treserved_vmid;


Perhaps it's more reasonable to belongs to vm_manager.


Yes, agree.



Jerry



  /* memory management */
  struct amdgpu_mmanmman;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c

index a175dfd..9993085 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1889,6 +1889,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
  adev->vm_manager.vm_pte_num_rings = 0;
  adev->gart.gart_funcs = NULL;
  adev->fence_context = kcl_fence_context_alloc(AMDGPU_MAX_RINGS);
+atomic_set(>reserved_vmid, 0);

  adev->smc_rreg = _invalid_rreg;
  adev->smc_wreg = _invalid_wreg;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 5f4dcc9..f7113b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -565,6 +565,10 @@ static int amdgpu_vm_alloc_dedicated_vmid(struct 
amdgpu_device *adev,

  unsigned vmhub;
  int r;

+if (atomic_read(>reserved_vmid) >= 
AMDGPU_VM_MAX_RESERVED_VMID) {

+DRM_ERROR("Over limitation of reserved vmid\n");
+return -EINVAL;
+}
  for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
  id_mgr = >vm_manager.id_mgr[vmhub];

@@ -580,6 +584,7 @@ static int amdgpu_vm_alloc_dedicated_vmid(struct 
amdgpu_device *adev,

  if (r)
  goto err;
  }
+atomic_inc(>reserved_vmid);


That's not atomic at all, two threads could try to reserve a VMID at the 
same time and race.


You need to use atomic_inc_return for this before allocating the IDs and 
properly clean up if allocating them fails.




  return 0;
  err:
@@ -2302,6 +2307,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, 
struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+bool dedicated = false;
  int i;

  if (vm->is_kfd_vm) {
@@ -2354,9 +2360,12 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)

  list_add(>dedicated_vmid[i]->list,
   _mgr->ids_lru);
  vm->dedicated_vmid[i] = NULL;
+dedicated = true;
  }
  mutex_unlock(_mgr->lock);
  }
+if (dedicated)
+atomic_dec(>reserved_vmid);
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 23981ee..2d3e6ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -83,6 +83,8 @@

  /* hardcode that limit for now */
  #define AMDGPU_VA_RESERVED_SIZE(8 << 20)
+/* max vmids dedicated for process */
+#define AMDGPU_VM_MAX_RESERVED_VMID 2


This should be 1 instead of 2, or did I miss something?

Regards,
Christian.



  struct amdgpu_vm_pt {
  struct amdgpu_bo*bo;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve vmid by vm ioctl

2017-04-26 Thread Christian König


Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):



On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 
++

  1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index acf9102..5f4dcc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,17 @@ static bool amdgpu_vm_had_gpu_reset(struct 
amdgpu_device *adev,

  atomic_read(>gpu_reset_counter);
  }

+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm)
+{
+unsigned vmhub;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+if (!vm->dedicated_vmid[vmhub])
+return false;


Just note:
Seemingly for pre-gmcv9, it will always allocate one more 
dedicated/reserved vmid for the 2nd un-used vmhub.

anyway it will not be used either.


It's even worse. IIRC we don't initialize the 2nd hub on pre-gmcv9. So 
that could work with uninitialized data.


Christian.



Jerry

+}
+return true;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +557,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, 
struct amdgpu_ring *ring,

  return r;
  }

+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+  struct amdgpu_vm *vm)
+{
+struct amdgpu_vm_id_manager *id_mgr;
+struct amdgpu_vm_id *idle;
+unsigned vmhub;
+int r;
+
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+/* Select the first entry VMID */
+idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id,
+list);
+list_del_init(>list);
+vm->dedicated_vmid[vmhub] = idle;
+mutex_unlock(_mgr->lock);
+
+r = amdgpu_sync_wait(>active);
+if (r)
+goto err;
+}
+
+return 0;
+err:
+for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+id_mgr = >vm_manager.id_mgr[vmhub];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[vmhub])
+ list_add(>dedicated_vmid[vmhub]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[vmhub] = NULL;
+mutex_unlock(_mgr->lock);
+}
+return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring 
*ring)

  {
  struct amdgpu_device *adev = ring->adev;
@@ -2379,9 +2429,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, 
void *data, struct drm_file *filp)

  union drm_amdgpu_vm *args = data;
  struct amdgpu_device *adev = dev->dev_private;
  struct amdgpu_fpriv *fpriv = filp->driver_priv;
+int r;

  switch (args->in.op) {
  case AMDGPU_VM_OP_RESERVE_VMID:
+if (!amdgpu_vm_dedicated_vmid_ready(>vm)) {
+r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm);


Could you change the return value in this ready checking func?
Usually we can easily get to know this kind of things.
if (sth_ready())
do_sth();



+if (r)
+return r;
+}
  break;
  default:
  return -EINVAL;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread Christian König


Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device 
*adev, struct amdgpu_vm *vm,

  unsigned ring_instance;
  struct amdgpu_ring *ring;
  struct amd_sched_rq *rq;
-int r;
+int r, i;

  vm->va = RB_ROOT;
  vm->client_id = 
atomic64_inc_return(>vm_manager.client_counter);

+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or 
anything like that.


Yes, agree. And if I'm not completely mistaken we still should only 
apply that to the GFX hub on Vega10.


Christian.



Jerry

spin_lock_init(>status_lock);
  INIT_LIST_HEAD(>invalidated);
  INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, 
struct amdgpu_vm *vm)

  {
  struct amdgpu_bo_va_mapping *mapping, *tmp;
  bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+int i;

  if (vm->is_kfd_vm) {
  struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)


  amdgpu_vm_free_levels(>root);
  fence_put(vm->last_dir_update);
+for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+struct amdgpu_vm_id_manager *id_mgr =
+>vm_manager.id_mgr[i];
+
+mutex_lock(_mgr->lock);
+if (vm->dedicated_vmid[i]) {
+list_add(>dedicated_vmid[i]->list,
+ _mgr->ids_lru);
+vm->dedicated_vmid[i] = NULL;
+}
+mutex_unlock(_mgr->lock);
+}
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

  /* client id */
  u64 client_id;
+/* dedicated vmid */
+struct amdgpu_vm_id*dedicated_vmid[AMDGPU_MAX_VMHUBS];
  /* each VM will map on CSA */
  struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu:fix get wrong gfx always on cu masks.

2017-04-26 Thread Rex Zhu

Change-Id: I46d7f2531eaec8135ccaf64b5ce8abc433c54e1a
Signed-off-by: Rex Zhu 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c | 10 --
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c | 10 --
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 10 --
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c |  4 ++--
 4 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index f7579de..25c3703 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -3719,6 +3719,12 @@ static void gfx_v6_0_get_cu_info(struct amdgpu_device 
*adev)
u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
struct amdgpu_cu_info *cu_info = >gfx.cu_info;
unsigned disable_masks[4 * 2];
+   u32 ao_cu_num;
+
+   if (adev->flags & AMD_IS_APU)
+   ao_cu_num = 2;
+   else
+   ao_cu_num = adev->gfx.config.max_cu_per_sh;
 
memset(cu_info, 0, sizeof(*cu_info));
 
@@ -3737,9 +3743,9 @@ static void gfx_v6_0_get_cu_info(struct amdgpu_device 
*adev)
bitmap = gfx_v6_0_get_cu_enabled(adev);
cu_info->bitmap[i][j] = bitmap;
 
-   for (k = 0; k < 16; k++) {
+   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k++) {
if (bitmap & mask) {
-   if (counter < 2)
+   if (counter < ao_cu_num)
ao_bitmap |= mask;
counter ++;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index fa5a0d2..75cca54 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -5339,6 +5339,12 @@ static void gfx_v7_0_get_cu_info(struct amdgpu_device 
*adev)
u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
struct amdgpu_cu_info *cu_info = >gfx.cu_info;
unsigned disable_masks[4 * 2];
+   u32 ao_cu_num;
+
+   if (adev->flags & AMD_IS_APU)
+   ao_cu_num = 2;
+   else
+   ao_cu_num = adev->gfx.config.max_cu_per_sh;
 
memset(cu_info, 0, sizeof(*cu_info));
 
@@ -5357,9 +5363,9 @@ static void gfx_v7_0_get_cu_info(struct amdgpu_device 
*adev)
bitmap = gfx_v7_0_get_cu_active_bitmap(adev);
cu_info->bitmap[i][j] = bitmap;
 
-   for (k = 0; k < 16; k ++) {
+   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++) {
if (bitmap & mask) {
-   if (counter < 2)
+   if (counter < ao_cu_num)
ao_bitmap |= mask;
counter ++;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 2ff5f19..0025ef6 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -7098,9 +7098,15 @@ static void gfx_v8_0_get_cu_info(struct amdgpu_device 
*adev)
u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
struct amdgpu_cu_info *cu_info = >gfx.cu_info;
unsigned disable_masks[4 * 2];
+   u32 ao_cu_num;
 
memset(cu_info, 0, sizeof(*cu_info));
 
+   if (adev->flags & AMD_IS_APU)
+   ao_cu_num = 2;
+   else
+   ao_cu_num = adev->gfx.config.max_cu_per_sh;
+
amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
 
mutex_lock(>grbm_idx_mutex);
@@ -7116,9 +7122,9 @@ static void gfx_v8_0_get_cu_info(struct amdgpu_device 
*adev)
bitmap = gfx_v8_0_get_cu_active_bitmap(adev);
cu_info->bitmap[i][j] = bitmap;
 
-   for (k = 0; k < 16; k ++) {
+   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++) {
if (bitmap & mask) {
-   if (counter < 2)
+   if (counter < ao_cu_num)
ao_bitmap |= mask;
counter ++;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 42cfd3b..13da795 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3667,9 +3667,9 @@ static int gfx_v9_0_get_cu_info(struct amdgpu_device 
*adev,
bitmap = gfx_v9_0_get_cu_active_bitmap(adev);
cu_info->bitmap[i][j] = bitmap;
 
-   for (k = 0; k < 16; k ++) {
+   for (k = 0; k < adev->gfx.config.max_cu_per_sh; k ++) {

Re: [PATCH 1/6] drm/amdgpu: add vm ioctl

2017-04-26 Thread Christian König


Am 26.04.2017 um 09:09 schrieb Zhang, Jerry (Junwei):

On 04/24/2017 01:57 PM, Chunming Zhou wrote:

It will be used for reserving vmid.

Change-Id: Ib7169ea999690c8e82d0dcbccdd2d97760c0270a
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 16 
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  1 +
  include/uapi/drm/amdgpu_drm.h   | 20 
  4 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c

index cad589a..7004e6c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1051,6 +1051,7 @@ int amdgpu_get_vblank_timestamp_kms(struct 
drm_device *dev, unsigned int pipe,

  const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
  DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
  DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
+DRM_IOCTL_DEF_DRV(AMDGPU_VM, amdgpu_vm_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
  DRM_IOCTL_DEF_DRV(AMDGPU_BO_LIST, amdgpu_bo_list_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),

  /* KMS */
  DRM_IOCTL_DEF_DRV(AMDGPU_GEM_MMAP, amdgpu_gem_mmap_ioctl, 
DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index f804d38..eb429c5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2358,3 +2358,19 @@ void amdgpu_vm_manager_fini(struct 
amdgpu_device *adev)

  }
  }
  }
+
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct 
drm_file *filp)

+{
+union drm_amdgpu_vm *args = data;
+struct amdgpu_device *adev = dev->dev_private;
+struct amdgpu_fpriv *fpriv = filp->driver_priv;
+
+switch (args->in.op) {
+case AMDGPU_VM_OP_RESERVE_VMID:


How do you think to add an op to release the reserved vmid as well?


Yeah, that's a really good point.

Additional to Jerrys comment you either need to reorder the patches or 
return an error here as long as the implementation is missing.


Otherwise you just ignore th IOCTL till it is really implemented and 
that isn't really an option.


Christian.



Jerry

+break;
+default:
+return -EINVAL;
+}
+
+return 0;
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h

index 0f547c6..62dbace 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -247,5 +247,6 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  void amdgpu_vm_bo_rmv(struct amdgpu_device *adev,
struct amdgpu_bo_va *bo_va);
  void amdgpu_vm_adjust_size(struct amdgpu_device *adev, uint64_t 
vm_size);
+int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct 
drm_file *filp);


  #endif
diff --git a/include/uapi/drm/amdgpu_drm.h 
b/include/uapi/drm/amdgpu_drm.h

index 56b7a2f3..5ee639b 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -51,6 +51,7 @@
  #define DRM_AMDGPU_GEM_OP0x10
  #define DRM_AMDGPU_GEM_USERPTR0x11
  #define DRM_AMDGPU_WAIT_FENCES0x12
+#define DRM_AMDGPU_VM0x13

  /* hybrid specific ioctls */
  #define DRM_AMDGPU_SEM0x5b
@@ -71,6 +72,7 @@
  #define DRM_IOCTL_AMDGPU_GEM_OP DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_OP, struct drm_amdgpu_gem_op)
  #define DRM_IOCTL_AMDGPU_GEM_USERPTR DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_USERPTR, struct drm_amdgpu_gem_userptr)
  #define DRM_IOCTL_AMDGPU_WAIT_FENCES DRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_WAIT_FENCES, union drm_amdgpu_wait_fences)
+#define DRM_IOCTL_AMDGPU_VMDRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_VM, union drm_amdgpu_vm)


  /* hybrid specific ioctls */
  #define DRM_IOCTL_AMDGPU_GEM_DGMADRM_IOWR(DRM_COMMAND_BASE + 
DRM_AMDGPU_GEM_DGMA, struct drm_amdgpu_gem_dgma)

@@ -212,6 +214,24 @@ struct drm_amdgpu_ctx_in {
  union drm_amdgpu_ctx_out out;
  };

+/* vm ioctl */
+#define AMDGPU_VM_OP_RESERVE_VMID1
+struct drm_amdgpu_vm_in {
+/** AMDGPU_VM_OP_* */
+__u32op;
+__u32flags;
+};
+
+struct drm_amdgpu_vm_out {
+/** For future use, no flags defined so far */
+__u64flags;
+};
+
+union drm_amdgpu_vm {
+struct drm_amdgpu_vm_in in;
+struct drm_amdgpu_vm_out out;
+};
+
  /* sem related */
  #define AMDGPU_SEM_OP_CREATE_SEM1
  #define AMDGPU_SEM_OP_WAIT_SEM2


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [rfc] drm sync objects (search for spock)

2017-04-26 Thread Christian König


Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported 
to an fd and imported on the other side we get the same sync_obj again?


In other words the fd is exported and imported only once and the 
expectation is that we fence containing it will change on each signaling 
operation.


As far as I can see with the current implementation you get two 
sync_objects on in the exporting process and one in the importing one.


Regards,
Christian.



I haven't used rcu anywhere here, I've used xchg to swap
fence pointers in the hope that's safe. If this does need
rcu'ing I suggest we do it in a follow on patch to minimise
the review pain.

Dave.

___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Fix use of interruptible waiting

2017-04-26 Thread Christian König


Am 25.04.2017 um 23:25 schrieb Alex Xie:

There is no good mechanism to handle the corresponding error.
When signal interrupt happens, unpin is not called.
As a result, inside AMDGPU, the statistic of pin size will be wrong.

Change-Id: I4a06a227c2757c447cec0058ace4b028553658a2
Signed-off-by: Alex Xie 


Reviewed-by: Christian König  for this one.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 7cf226d..43082bf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -172,7 +172,7 @@ void amdgpu_crtc_cleanup_flip_ctx(
struct amdgpu_flip_work *work,
struct amdgpu_bo *new_abo)
  {
-   if (unlikely(amdgpu_bo_reserve(new_abo, false) != 0)) {
+   if (unlikely(amdgpu_bo_reserve(new_abo, true) != 0)) {
DRM_ERROR("failed to reserve new abo in error path\n");
amdgpu_flip_work_cleanup(work);
return;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu: Fix use of interruptible waiting

2017-04-26 Thread Christian König


Am 26.04.2017 um 03:17 schrieb Michel Dänzer:

On 26/04/17 06:25 AM, Alex Xie wrote:

1. The wait is short. There is not much benefit by
interruptible waiting.
2. In this function and caller functions, the error
handling for such interrupt is complicated and risky.

Change-Id: I289674ecd3f5ef20c93fe63e33df6d668b3c2edc
Signed-off-by: Alex Xie 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index 43082bf..c006cc4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -227,7 +227,7 @@ int amdgpu_crtc_prepare_flip(
new_abo = gem_to_amdgpu_bo(obj);
  
  	/* pin the new buffer */

-   r = amdgpu_bo_reserve(new_abo, false);
+   r = amdgpu_bo_reserve(new_abo, true);
if (unlikely(r != 0)) {
DRM_ERROR("failed to reserve new abo buffer before flip\n");
goto cleanup;


I'm afraid we have no choice but to use interruptible here, because this
code runs as part of an ioctl (DRM_IOCTL_MODE_PAGE_FLIP).


Yes, agree. But the error message is incorrect here and should be removed.

Christian.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Fix use of interruptible waiting

2017-04-26 Thread Christian König


Am 25.04.2017 um 21:32 schrieb Alex Xie:

Either in cgs functions or for callers of cgs functions:
1. The signal interrupt can affect the expected behaviour
2. There is no good mechanism to handle the corresponding error
3. There is no chance of deadlock in these single BO waiting
4. There is no clear benefit for interruptible waiting
5. Future caller of these functions might have same issue.

Change-Id: Ifc0e0ab862f98cdc6cdaef87cd96f11c91d64f27
Signed-off-by: Alex Xie 


Reviewed-by: Christian König 

BTW: We should probably tidy up amdgpu_cgs.c. Most of this stuff is 
actually not used anywhere.


Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c | 14 +++---
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
index a1a2d44..31fe4ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c
@@ -89,7 +89,7 @@ static int amdgpu_cgs_gmap_kmem(struct cgs_device 
*cgs_device, void *kmem,
   AMDGPU_GEM_DOMAIN_GTT, 0, sg, NULL, );
if (ret)
return ret;
-   ret = amdgpu_bo_reserve(bo, false);
+   ret = amdgpu_bo_reserve(bo, true);
if (unlikely(ret != 0))
return ret;
  
@@ -107,7 +107,7 @@ static int amdgpu_cgs_gunmap_kmem(struct cgs_device *cgs_device, cgs_handle_t km

struct amdgpu_bo *obj = (struct amdgpu_bo *)kmem_handle;
  
  	if (obj) {

-   int r = amdgpu_bo_reserve(obj, false);
+   int r = amdgpu_bo_reserve(obj, true);
if (likely(r == 0)) {
amdgpu_bo_unpin(obj);
amdgpu_bo_unreserve(obj);
@@ -215,7 +215,7 @@ static int amdgpu_cgs_free_gpu_mem(struct cgs_device 
*cgs_device, cgs_handle_t h
struct amdgpu_bo *obj = (struct amdgpu_bo *)handle;
  
  	if (obj) {

-   int r = amdgpu_bo_reserve(obj, false);
+   int r = amdgpu_bo_reserve(obj, true);
if (likely(r == 0)) {
amdgpu_bo_kunmap(obj);
amdgpu_bo_unpin(obj);
@@ -239,7 +239,7 @@ static int amdgpu_cgs_gmap_gpu_mem(struct cgs_device 
*cgs_device, cgs_handle_t h
min_offset = obj->placements[0].fpfn << PAGE_SHIFT;
max_offset = obj->placements[0].lpfn << PAGE_SHIFT;
  
-	r = amdgpu_bo_reserve(obj, false);

+   r = amdgpu_bo_reserve(obj, true);
if (unlikely(r != 0))
return r;
r = amdgpu_bo_pin_restricted(obj, obj->prefered_domains,
@@ -252,7 +252,7 @@ static int amdgpu_cgs_gunmap_gpu_mem(struct cgs_device 
*cgs_device, cgs_handle_t
  {
int r;
struct amdgpu_bo *obj = (struct amdgpu_bo *)handle;
-   r = amdgpu_bo_reserve(obj, false);
+   r = amdgpu_bo_reserve(obj, true);
if (unlikely(r != 0))
return r;
r = amdgpu_bo_unpin(obj);
@@ -265,7 +265,7 @@ static int amdgpu_cgs_kmap_gpu_mem(struct cgs_device 
*cgs_device, cgs_handle_t h
  {
int r;
struct amdgpu_bo *obj = (struct amdgpu_bo *)handle;
-   r = amdgpu_bo_reserve(obj, false);
+   r = amdgpu_bo_reserve(obj, true);
if (unlikely(r != 0))
return r;
r = amdgpu_bo_kmap(obj, map);
@@ -277,7 +277,7 @@ static int amdgpu_cgs_kunmap_gpu_mem(struct cgs_device 
*cgs_device, cgs_handle_t
  {
int r;
struct amdgpu_bo *obj = (struct amdgpu_bo *)handle;
-   r = amdgpu_bo_reserve(obj, false);
+   r = amdgpu_bo_reserve(obj, true);
if (unlikely(r != 0))
return r;
amdgpu_bo_kunmap(obj);



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/6] drm/amdgpu: reserve vmid by vm ioctl

2017-04-26 Thread Zhang, Jerry (Junwei)




On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: I5f80dc39dc9d44660a96a2b710b0dbb4d3b9039d
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 ++
  1 file changed, 56 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index acf9102..5f4dcc9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -397,6 +397,17 @@ static bool amdgpu_vm_had_gpu_reset(struct amdgpu_device 
*adev,
atomic_read(>gpu_reset_counter);
  }

+static bool amdgpu_vm_dedicated_vmid_ready(struct amdgpu_vm *vm)
+{
+   unsigned vmhub;
+
+   for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+   if (!vm->dedicated_vmid[vmhub])
+   return false;


Just note:
Seemingly for pre-gmcv9, it will always allocate one more dedicated/reserved 
vmid for the 2nd un-used vmhub.

anyway it will not be used either.

Jerry

+   }
+   return true;
+}
+
  /**
   * amdgpu_vm_grab_id - allocate the next free VMID
   *
@@ -546,6 +557,45 @@ int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct 
amdgpu_ring *ring,
return r;
  }

+static int amdgpu_vm_alloc_dedicated_vmid(struct amdgpu_device *adev,
+ struct amdgpu_vm *vm)
+{
+   struct amdgpu_vm_id_manager *id_mgr;
+   struct amdgpu_vm_id *idle;
+   unsigned vmhub;
+   int r;
+
+   for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+   id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   /* Select the first entry VMID */
+   idle = list_first_entry(_mgr->ids_lru, struct amdgpu_vm_id,
+   list);
+   list_del_init(>list);
+   vm->dedicated_vmid[vmhub] = idle;
+   mutex_unlock(_mgr->lock);
+
+   r = amdgpu_sync_wait(>active);
+   if (r)
+   goto err;
+   }
+
+   return 0;
+err:
+   for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
+   id_mgr = >vm_manager.id_mgr[vmhub];
+
+   mutex_lock(_mgr->lock);
+   if (vm->dedicated_vmid[vmhub])
+   list_add(>dedicated_vmid[vmhub]->list,
+_mgr->ids_lru);
+   vm->dedicated_vmid[vmhub] = NULL;
+   mutex_unlock(_mgr->lock);
+   }
+   return r;
+}
+
  static bool amdgpu_vm_ring_has_compute_vm_bug(struct amdgpu_ring *ring)
  {
struct amdgpu_device *adev = ring->adev;
@@ -2379,9 +2429,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, 
struct drm_file *filp)
union drm_amdgpu_vm *args = data;
struct amdgpu_device *adev = dev->dev_private;
struct amdgpu_fpriv *fpriv = filp->driver_priv;
+   int r;

switch (args->in.op) {
case AMDGPU_VM_OP_RESERVE_VMID:
+   if (!amdgpu_vm_dedicated_vmid_ready(>vm)) {
+   r = amdgpu_vm_alloc_dedicated_vmid(adev, >vm);


Could you change the return value in this ready checking func?
Usually we can easily get to know this kind of things.
if (sth_ready())
do_sth();



+   if (r)
+   return r;
+   }
break;
default:
return -EINVAL;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/6] drm/amdgpu: add dedicated vmid field in vm struct

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/24/2017 01:57 PM, Chunming Zhou wrote:

Change-Id: Id728e20366c8a1ae90d4e901dc80e136e2a613bb
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  2 ++
  2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index eb429c5..acf9102 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2144,10 +2144,12 @@ int amdgpu_vm_init(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
unsigned ring_instance;
struct amdgpu_ring *ring;
struct amd_sched_rq *rq;
-   int r;
+   int r, i;

vm->va = RB_ROOT;
vm->client_id = atomic64_inc_return(>vm_manager.client_counter);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++)
+   vm->dedicated_vmid[i] = NULL;


Maybe it's better to give it a consistent name as resv_vmid, or anything like 
that.

Jerry

spin_lock_init(>status_lock);
INIT_LIST_HEAD(>invalidated);
INIT_LIST_HEAD(>cleared);
@@ -2250,6 +2252,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
  {
struct amdgpu_bo_va_mapping *mapping, *tmp;
bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+   int i;

if (vm->is_kfd_vm) {
struct amdgpu_vm_id_manager *id_mgr =
@@ -2292,6 +2295,18 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)

amdgpu_vm_free_levels(>root);
fence_put(vm->last_dir_update);
+   for (i = 0; i < AMDGPU_MAX_VMHUBS; i++) {
+   struct amdgpu_vm_id_manager *id_mgr =
+   >vm_manager.id_mgr[i];
+
+   mutex_lock(_mgr->lock);
+   if (vm->dedicated_vmid[i]) {
+   list_add(>dedicated_vmid[i]->list,
+_mgr->ids_lru);
+   vm->dedicated_vmid[i] = NULL;
+   }
+   mutex_unlock(_mgr->lock);
+   }
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 62dbace..23981ee 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -122,6 +122,8 @@ struct amdgpu_vm {

/* client id */
u64 client_id;
+   /* dedicated vmid */
+   struct amdgpu_vm_id *dedicated_vmid[AMDGPU_MAX_VMHUBS];
/* each VM will map on CSA */
struct amdgpu_bo_va *csa_bo_va;



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/6] drm/amdgpu: add limitation for dedicated vm number v2

2017-04-26 Thread Zhang, Jerry (Junwei)


On 04/24/2017 01:57 PM, Chunming Zhou wrote:

v2: move #define to amdgpu_vm.h

Change-Id: Ie5958cf6dbdc1c8278e61d9158483472d6f5c6e3
Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 ++
  4 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0831cd2..ba9d3d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1591,6 +1591,7 @@ struct amdgpu_device {
struct amdgpu_dummy_pagedummy_page;
struct amdgpu_vm_managervm_manager;
struct amdgpu_vmhub vmhub[AMDGPU_MAX_VMHUBS];
+   atomic_treserved_vmid;


Perhaps it's more reasonable to belongs to vm_manager.

Jerry



/* memory management */
struct amdgpu_mman  mman;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index a175dfd..9993085 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1889,6 +1889,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
adev->vm_manager.vm_pte_num_rings = 0;
adev->gart.gart_funcs = NULL;
adev->fence_context = kcl_fence_context_alloc(AMDGPU_MAX_RINGS);
+   atomic_set(>reserved_vmid, 0);

adev->smc_rreg = _invalid_rreg;
adev->smc_wreg = _invalid_wreg;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 5f4dcc9..f7113b9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -565,6 +565,10 @@ static int amdgpu_vm_alloc_dedicated_vmid(struct 
amdgpu_device *adev,
unsigned vmhub;
int r;

+   if (atomic_read(>reserved_vmid) >= AMDGPU_VM_MAX_RESERVED_VMID) {
+   DRM_ERROR("Over limitation of reserved vmid\n");
+   return -EINVAL;
+   }
for (vmhub = 0; vmhub < AMDGPU_MAX_VMHUBS; vmhub++) {
id_mgr = >vm_manager.id_mgr[vmhub];

@@ -580,6 +584,7 @@ static int amdgpu_vm_alloc_dedicated_vmid(struct 
amdgpu_device *adev,
if (r)
goto err;
}
+   atomic_inc(>reserved_vmid);

return 0;
  err:
@@ -2302,6 +2307,7 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
  {
struct amdgpu_bo_va_mapping *mapping, *tmp;
bool prt_fini_needed = !!adev->gart.gart_funcs->set_prt;
+   bool dedicated = false;
int i;

if (vm->is_kfd_vm) {
@@ -2354,9 +2360,12 @@ void amdgpu_vm_fini(struct amdgpu_device *adev, struct 
amdgpu_vm *vm)
list_add(>dedicated_vmid[i]->list,
 _mgr->ids_lru);
vm->dedicated_vmid[i] = NULL;
+   dedicated = true;
}
mutex_unlock(_mgr->lock);
}
+   if (dedicated)
+   atomic_dec(>reserved_vmid);
  }

  /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 23981ee..2d3e6ed 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -83,6 +83,8 @@

  /* hardcode that limit for now */
  #define AMDGPU_VA_RESERVED_SIZE   (8 << 20)
+/* max vmids dedicated for process */
+#define AMDGPU_VM_MAX_RESERVED_VMID 2

  struct amdgpu_vm_pt {
struct amdgpu_bo*bo;


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 4/5] amdgpu/cs: split out fence dependency checking

2017-04-26 Thread zhoucm1




On 2017年04月26日 11:28, Dave Airlie wrote:

From: Dave Airlie 

This just splits out the fence depenency checking into it's
own function to make it easier to add semaphore dependencies.

Reviewed-by: Christian König 
Signed-off-by: Dave Airlie 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 85 +++---
  1 file changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 99424cb..df25b32 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -963,56 +963,65 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
return 0;
  }
  
-static int amdgpu_cs_dependencies(struct amdgpu_device *adev,

- struct amdgpu_cs_parser *p)
+static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,
To consistent, we'd like amdgpu_cs prefix in amdgpu_cs.c, same as other 
patches.


Regards,
David Zhou

+   struct amdgpu_cs_chunk *chunk)
  {
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
-   int i, j, r;
-
-   for (i = 0; i < p->nchunks; ++i) {
-   struct drm_amdgpu_cs_chunk_dep *deps;
-   struct amdgpu_cs_chunk *chunk;
-   unsigned num_deps;
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_dep *deps;
  
-		chunk = >chunks[i];

+   deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_dep);
  
-		if (chunk->chunk_id != AMDGPU_CHUNK_ID_DEPENDENCIES)

-   continue;
+   for (i = 0; i < num_deps; ++i) {
+   struct amdgpu_ring *ring;
+   struct amdgpu_ctx *ctx;
+   struct dma_fence *fence;
  
-		deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;

-   num_deps = chunk->length_dw * 4 /
-   sizeof(struct drm_amdgpu_cs_chunk_dep);
+   r = amdgpu_cs_get_ring(p->adev, deps[i].ip_type,
+  deps[i].ip_instance,
+  deps[i].ring, );
+   if (r)
+   return r;
  
-		for (j = 0; j < num_deps; ++j) {

-   struct amdgpu_ring *ring;
-   struct amdgpu_ctx *ctx;
-   struct dma_fence *fence;
+   ctx = amdgpu_ctx_get(fpriv, deps[i].ctx_id);
+   if (ctx == NULL)
+   return -EINVAL;
  
-			r = amdgpu_cs_get_ring(adev, deps[j].ip_type,

-  deps[j].ip_instance,
-  deps[j].ring, );
+   fence = amdgpu_ctx_get_fence(ctx, ring,
+deps[i].handle);
+   if (IS_ERR(fence)) {
+   r = PTR_ERR(fence);
+   amdgpu_ctx_put(ctx);
+   return r;
+   } else if (fence) {
+   r = amdgpu_sync_fence(p->adev, >job->sync,
+ fence);
+   dma_fence_put(fence);
+   amdgpu_ctx_put(ctx);
if (r)
return r;
+   }
+   }
+   return 0;
+}
  
-			ctx = amdgpu_ctx_get(fpriv, deps[j].ctx_id);

-   if (ctx == NULL)
-   return -EINVAL;
+static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
+ struct amdgpu_cs_parser *p)
+{
+   int i, r;
  
-			fence = amdgpu_ctx_get_fence(ctx, ring,

-deps[j].handle);
-   if (IS_ERR(fence)) {
-   r = PTR_ERR(fence);
-   amdgpu_ctx_put(ctx);
-   return r;
+   for (i = 0; i < p->nchunks; ++i) {
+   struct amdgpu_cs_chunk *chunk;
  
-			} else if (fence) {

-   r = amdgpu_sync_fence(adev, >job->sync,
- fence);
-   dma_fence_put(fence);
-   amdgpu_ctx_put(ctx);
-   if (r)
-   return r;
-   }
+   chunk = >chunks[i];
+
+   if (chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) {
+   r = amdgpu_process_fence_dep(p, chunk);
+   if (r)
+   return r;
}
}
  


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

Re: [PATCH 1/5] drm: introduce sync objects

2017-04-26 Thread zhoucm1




On 2017年04月26日 11:28, Dave Airlie wrote:

From: Dave Airlie 

Sync objects are new toplevel drm object, that contain a
pointer to a fence. This fence can be updated via command
submission ioctls via drivers.

There is also a generic wait obj API modelled on the vulkan
wait API (with code modelled on some amdgpu code).

These objects can be converted to an opaque fd that can be
passes between processes.

TODO: define sync_file interaction.

Signed-off-by: Dave Airlie 
---
  Documentation/gpu/drm-internals.rst |   3 +
  Documentation/gpu/drm-mm.rst|   6 +
  drivers/gpu/drm/Makefile|   2 +-
  drivers/gpu/drm/drm_fops.c  |   8 +
  drivers/gpu/drm/drm_internal.h  |  13 ++
  drivers/gpu/drm/drm_ioctl.c |  12 ++
  drivers/gpu/drm/drm_syncobj.c   | 374 
  include/drm/drmP.h  |   5 +
  include/drm/drm_drv.h   |   1 +
  include/drm/drm_syncobj.h   | 104 ++
  include/uapi/drm/drm.h  |  25 +++
  11 files changed, 552 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/drm_syncobj.c
  create mode 100644 include/drm/drm_syncobj.h

diff --git a/Documentation/gpu/drm-internals.rst 
b/Documentation/gpu/drm-internals.rst
index e35920d..2ea3bce 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -98,6 +98,9 @@ DRIVER_ATOMIC
  implement appropriate obj->atomic_get_property() vfuncs for any
  modeset objects with driver specific properties.
  
+DRIVER_SYNCOBJ

+Driver support drm sync objects.
+
  Major, Minor and Patchlevel
  ~~~
  
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst

index f5760b1..28aebe8 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -483,3 +483,9 @@ DRM Cache Handling
  
  .. kernel-doc:: drivers/gpu/drm/drm_cache.c

 :export:
+
+DRM Sync Objects
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3ee9579..b5e565c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
  
  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o

  drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index afdf5b1..9a61df2 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, priv);
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_open(priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_init_file_private(>prime);
  
@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)

  out_prime_destroy:
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(>prime);
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(priv);
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, priv);
put_pid(priv->pid);
@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
drm_property_destroy_user_blobs(dev, file_priv);
}
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_release(file_priv);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, file_priv);
  
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h

index f37388c..44ef903 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc 
*crtc)
  {
return 0;
  }
+
  #endif
+
+/* drm_syncobj.c */
+void drm_syncobj_open(struct drm_file *file_private);
+void drm_syncobj_release(struct drm_file *file_private);
+int drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_private);
+int drm_syncobj_destroy_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void

64 matches

Mail list logo