Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed

2017-03-24 Thread Christian König 

Am 23.03.2017 um 20:27 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

We will add the fence to freed buffer objects in a later commit, to ensure
that the underlying memory can only be re-used after all references in
page tables have been cleared.

Signed-off-by: Nicolai Hähnle 


Reviewed-by: Christian König  for both.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
  4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 55d553a..85e6070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct 
amdgpu_cs_parser *p)
int i, r;
  
  	r = amdgpu_vm_update_page_directory(adev, vm);

if (r)
return r;
  
  	r = amdgpu_sync_fence(adev, >job->sync, vm->page_directory_fence);

if (r)
return r;
  
-	r = amdgpu_vm_clear_freed(adev, vm);

+   r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (r)
return r;
  
  	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);

if (r)
return r;
  
  	r = amdgpu_sync_fence(adev, >job->sync,

  fpriv->prt_va->last_pt_update);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index be9fb2c..4a53c43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device 
*adev,
  
  	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,

  NULL);
if (r)
goto error;
  
  	r = amdgpu_vm_update_page_directory(adev, vm);

if (r)
goto error;
  
-	r = amdgpu_vm_clear_freed(adev, vm);

+   r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (r)
goto error;
  
  	if (operation == AMDGPU_VA_OP_MAP ||

operation == AMDGPU_VA_OP_REPLACE)
r = amdgpu_vm_bo_update(adev, bo_va, false);
  
  error:

if (r && r != -ERESTARTSYS)
DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dd7df45..2c95a75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)
}
  
  	kfree(shared);

  }
  
  /**

   * amdgpu_vm_clear_freed - clear freed BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
+ * @fence: optional resulting fence (unchanged if no work needed to be done
+ * or if an error occurred)
   *
   * Make sure all freed BOs are cleared in the PT.
   * Returns 0 for success.
   *
   * PTs have to be reserved and mutex must be locked!
   */
  int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
- struct amdgpu_vm *vm)
+ struct amdgpu_vm *vm,
+ struct fence **fence)
  {
struct amdgpu_bo_va_mapping *mapping;
-   struct fence *fence = NULL;
+   struct fence *f = NULL;
int r;
  
  	while (!list_empty(>freed)) {

mapping = list_first_entry(>freed,
struct amdgpu_bo_va_mapping, list);
list_del(>list);
  
  		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,

-  0, 0, );
-   amdgpu_vm_free_mapping(adev, vm, mapping, fence);
+  0, 0, );
+   amdgpu_vm_free_mapping(adev, vm, mapping, f);
if (r) {
-   fence_put(fence);
+   fence_put(f);
return r;
}
+   }
  
+	if (fence && f) {

+   fence_put(*fence);
+   *fence = f;
+   } else {
+   fence_put(f);
}
-   fence_put(fence);
+
return 0;
  
  }
  
  /**

   * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
   *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ff10fa5..9d5a572 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
uint64_t saddr, uint64_t size);
  int

Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed

2017-03-23 Thread Zhang, Jerry (Junwei) 

On 03/24/2017 10:30 AM, zhoucm1 wrote:



On 2017年03月24日 03:27, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

We will add the fence to freed buffer objects in a later commit, to ensure
that the underlying memory can only be re-used after all references in
page tables have been cleared.

Signed-off-by: Nicolai Hähnle 

Reviewed-by: Chunming Zhou 

Reviewed-by: Junwei Zhang 




---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
  4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 55d553a..85e6070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct
amdgpu_cs_parser *p)
  int i, r;
  r = amdgpu_vm_update_page_directory(adev, vm);
  if (r)
  return r;
  r = amdgpu_sync_fence(adev, >job->sync, vm->page_directory_fence);
  if (r)
  return r;
-r = amdgpu_vm_clear_freed(adev, vm);
+r = amdgpu_vm_clear_freed(adev, vm, NULL);
  if (r)
  return r;
  r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
  if (r)
  return r;
  r = amdgpu_sync_fence(adev, >job->sync,
fpriv->prt_va->last_pt_update);
  if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index be9fb2c..4a53c43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct
amdgpu_device *adev,
  r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,
NULL);
  if (r)
  goto error;
  r = amdgpu_vm_update_page_directory(adev, vm);
  if (r)
  goto error;
-r = amdgpu_vm_clear_freed(adev, vm);
+r = amdgpu_vm_clear_freed(adev, vm, NULL);
  if (r)
  goto error;
  if (operation == AMDGPU_VA_OP_MAP ||
  operation == AMDGPU_VA_OP_REPLACE)
  r = amdgpu_vm_bo_update(adev, bo_va, false);
  error:
  if (r && r != -ERESTARTSYS)
  DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dd7df45..2c95a75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device
*adev, struct amdgpu_vm *vm)
  }
  kfree(shared);
  }
  /**
   * amdgpu_vm_clear_freed - clear freed BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
+ * @fence: optional resulting fence (unchanged if no work needed to be done
+ * or if an error occurred)
   *
   * Make sure all freed BOs are cleared in the PT.
   * Returns 0 for success.
   *
   * PTs have to be reserved and mutex must be locked!
   */
  int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
-  struct amdgpu_vm *vm)
+  struct amdgpu_vm *vm,
+  struct fence **fence)
  {
  struct amdgpu_bo_va_mapping *mapping;
-struct fence *fence = NULL;
+struct fence *f = NULL;
  int r;
  while (!list_empty(>freed)) {
  mapping = list_first_entry(>freed,
  struct amdgpu_bo_va_mapping, list);
  list_del(>list);
  r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,
-   0, 0, );
-amdgpu_vm_free_mapping(adev, vm, mapping, fence);
+   0, 0, );
+amdgpu_vm_free_mapping(adev, vm, mapping, f);
  if (r) {
-fence_put(fence);
+fence_put(f);
  return r;
  }
+}
+if (fence && f) {
+fence_put(*fence);
+*fence = f;
+} else {
+fence_put(f);
  }
-fence_put(fence);
+
  return 0;
  }
  /**
   * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
   *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ff10fa5..9d5a572 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
  struct amdgpu_vm *vm,
  uint64_t saddr, uint64_t size);
  int amdgpu_vm_grab_id(struct amdgpu_vm *vm, struct amdgpu_ring *ring,
struct amdgpu_sync *sync, struct fence *fence,
struct amdgpu_job *job);
  int amdgpu_vm_flush(struct amdgpu_ring *ring, struct amdgpu_job *job);
  void amdgpu_vm_reset_id(struct amdgpu_device

Re: [PATCH v2 1/2] drm/amdgpu: add optional fence out-parameter to amdgpu_vm_clear_freed

2017-03-23 Thread zhoucm1 



On 2017年03月24日 03:27, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

We will add the fence to freed buffer objects in a later commit, to ensure
that the underlying memory can only be re-used after all references in
page tables have been cleared.

Signed-off-by: Nicolai Hähnle 

Reviewed-by: Chunming Zhou 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 21 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  3 ++-
  4 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 55d553a..85e6070 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -778,21 +778,21 @@ static int amdgpu_bo_vm_update_pte(struct 
amdgpu_cs_parser *p)
int i, r;
  
  	r = amdgpu_vm_update_page_directory(adev, vm);

if (r)
return r;
  
  	r = amdgpu_sync_fence(adev, >job->sync, vm->page_directory_fence);

if (r)
return r;
  
-	r = amdgpu_vm_clear_freed(adev, vm);

+   r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (r)
return r;
  
  	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);

if (r)
return r;
  
  	r = amdgpu_sync_fence(adev, >job->sync,

  fpriv->prt_va->last_pt_update);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index be9fb2c..4a53c43 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -535,21 +535,21 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device 
*adev,
  
  	r = amdgpu_vm_validate_pt_bos(adev, vm, amdgpu_gem_va_check,

  NULL);
if (r)
goto error;
  
  	r = amdgpu_vm_update_page_directory(adev, vm);

if (r)
goto error;
  
-	r = amdgpu_vm_clear_freed(adev, vm);

+   r = amdgpu_vm_clear_freed(adev, vm, NULL);
if (r)
goto error;
  
  	if (operation == AMDGPU_VA_OP_MAP ||

operation == AMDGPU_VA_OP_REPLACE)
r = amdgpu_vm_bo_update(adev, bo_va, false);
  
  error:

if (r && r != -ERESTARTSYS)
DRM_ERROR("Couldn't update BO_VA (%d)\n", r);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index dd7df45..2c95a75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1397,48 +1397,57 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)
}
  
  	kfree(shared);

  }
  
  /**

   * amdgpu_vm_clear_freed - clear freed BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
+ * @fence: optional resulting fence (unchanged if no work needed to be done
+ * or if an error occurred)
   *
   * Make sure all freed BOs are cleared in the PT.
   * Returns 0 for success.
   *
   * PTs have to be reserved and mutex must be locked!
   */
  int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
- struct amdgpu_vm *vm)
+ struct amdgpu_vm *vm,
+ struct fence **fence)
  {
struct amdgpu_bo_va_mapping *mapping;
-   struct fence *fence = NULL;
+   struct fence *f = NULL;
int r;
  
  	while (!list_empty(>freed)) {

mapping = list_first_entry(>freed,
struct amdgpu_bo_va_mapping, list);
list_del(>list);
  
  		r = amdgpu_vm_bo_split_mapping(adev, NULL, 0, NULL, vm, mapping,

-  0, 0, );
-   amdgpu_vm_free_mapping(adev, vm, mapping, fence);
+  0, 0, );
+   amdgpu_vm_free_mapping(adev, vm, mapping, f);
if (r) {
-   fence_put(fence);
+   fence_put(f);
return r;
}
+   }
  
+	if (fence && f) {

+   fence_put(*fence);
+   *fence = f;
+   } else {
+   fence_put(f);
}
-   fence_put(fence);
+
return 0;
  
  }
  
  /**

   * amdgpu_vm_clear_invalids - clear invalidated BOs in the PT
   *
   * @adev: amdgpu_device pointer
   * @vm: requested vm
   *
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index ff10fa5..9d5a572 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -187,21 +187,22 @@ int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
struct amdgpu_vm *vm,
uint64_t saddr, uint64_t size);
  int amdgpu_vm_grab_id(struct