Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-17 Thread Christian König 

Am 16.05.24 um 14:21 schrieb Tvrtko Ursulin:


Hi Christian,

On 08/05/2024 09:26, Tvrtko Ursulin wrote:

On 08/05/2024 06:42, Christian König wrote:

Am 06.05.24 um 18:26 schrieb Tvrtko Ursulin:


On 03/05/2024 10:14, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

v3:
  * Use Christian's kerneldoc.

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
Reviewed-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 41 
-

  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 29 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
    if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
    r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 4e2391c83d7c..d451ff9d5af4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
  -    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,
   * For now ignore BOs which are currently locked and 
potentially

   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
    amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))


Beware the logic inversion here - luckily no one merged it yet! I 
will send a respin..


Let me know when that is done, I was about to merge it.


First three patches in the series 
https://lore.kernel.org/amd-gfx/20240506165959.50648-1-tursu...@igalia.com/ 
if you can pull it from there or I should resend standalone?


Please double check it though. Sorry about that I am so used to 
pre-merge CI we have on the i915 preventing silly mistakes such as 
that one.


Small reminder on the above since you have been out - are you okay to 
pick up the first three patches from 
https://lore.kernel.org/amd-gfx/20240506165959.50648-1-tursu...@igalia.com/, 
if they still look okay to you? Or I should re-send them standalone?


Ah, thanks for the reminder. Totally forgotten that I wanted to push 
this. Just done so.


Regards,
Christian.



Regards,

Tvrtko


+ dma_resv_unlock(bo->tbo.base.resv);
  }
    void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
  -    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
    if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  @@ -1942,7 +1940,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == 
vm->root.bo->tbo.base.resv &&

+    if

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-16 Thread Tvrtko Ursulin 



Hi Christian,

On 08/05/2024 09:26, Tvrtko Ursulin wrote:

On 08/05/2024 06:42, Christian König wrote:

Am 06.05.24 um 18:26 schrieb Tvrtko Ursulin:


On 03/05/2024 10:14, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

v3:
  * Use Christian's kerneldoc.

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
Reviewed-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 41 
-

  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 29 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
    if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
    r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 4e2391c83d7c..d451ff9d5af4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
  -    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
    dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
    amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))


Beware the logic inversion here - luckily no one merged it yet! I 
will send a respin..


Let me know when that is done, I was about to merge it.


First three patches in the series 
https://lore.kernel.org/amd-gfx/20240506165959.50648-1-tursu...@igalia.com/ if you can pull it from there or I should resend standalone?


Please double check it though. Sorry about that I am so used to 
pre-merge CI we have on the i915 preventing silly mistakes such as that 
one.


Small reminder on the above since you have been out - are you okay to 
pick up the first three patches from 
https://lore.kernel.org/amd-gfx/20240506165959.50648-1-tursu...@igalia.com/, 
if they still look okay to you? Or I should re-send them standalone?


Regards,

Tvrtko


+ dma_resv_unlock(bo->tbo.base.resv);
  }
    void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
  -    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
    if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  @@ -1942,7 +1940,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -1957,7 +1955,7 @@ int

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-09 Thread Tvrtko Ursulin 



On 08/05/2024 06:42, Christian König wrote:

Am 06.05.24 um 18:26 schrieb Tvrtko Ursulin:


On 03/05/2024 10:14, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

v3:
  * Use Christian's kerneldoc.

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
Reviewed-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 41 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 29 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
    if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
    r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 4e2391c83d7c..d451ff9d5af4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
  -    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
    dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
    amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))


Beware the logic inversion here - luckily no one merged it yet! I will 
send a respin..


Let me know when that is done, I was about to merge it.


First three patches in the series 
https://lore.kernel.org/amd-gfx/20240506165959.50648-1-tursu...@igalia.com/ 
if you can pull it from there or I should resend standalone?


Please double check it though. Sorry about that I am so used to 
pre-merge CI we have on the i915 preventing silly mistakes such as that one.


Regards,

Tvrtko



Regards,
Christian.



Regards,

Tvrtko


+ dma_resv_unlock(bo->tbo.base.resv);
  }
    void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
  -    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
    if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  @@ -1942,7 +1940,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -1957,7 +1955,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (after->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
 

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-07 Thread Christian König 

Am 06.05.24 um 18:26 schrieb Tvrtko Ursulin:


On 03/05/2024 10:14, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

v3:
  * Use Christian's kerneldoc.

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
Reviewed-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 41 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 29 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
    if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
    r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 4e2391c83d7c..d451ff9d5af4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
  -    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
    dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
    amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))


Beware the logic inversion here - luckily no one merged it yet! I will 
send a respin..


Let me know when that is done, I was about to merge it.

Regards,
Christian.



Regards,

Tvrtko


+ dma_resv_unlock(bo->tbo.base.resv);
  }
    void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
  -    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
    if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  @@ -1942,7 +1940,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -1957,7 +1955,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (after->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
  -    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !after->bo_va->base.moved)
  amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -2037,7 +2035,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
    if (bo) {
  dma_resv_assert_held(bo->tbo.base.resv);
-    if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))
  ttm_bo_set_bulk_move(>tbo, NULL);
 

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-07 Thread Tvrtko Ursulin 



On 03/05/2024 10:14, Tvrtko Ursulin wrote:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

v3:
  * Use Christian's kerneldoc.

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
Reviewed-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 41 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object 
*obj,
return -EPERM;
  
  	if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&

-   abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+   !amdgpu_vm_is_bo_always_valid(vm, abo))
return -EPERM;
  
  	r = amdgpu_bo_reserve(abo, false);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 4e2391c83d7c..d451ff9d5af4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
base->next = bo->vm_bo;
bo->vm_bo = base;
  
-	if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)

+   if (!amdgpu_vm_is_bo_always_valid(vm, bo))
return;
  
  	dma_resv_assert_held(vm->root.bo->tbo.base.resv);

@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct amdgpu_bo_va 
*bo_va,
 * For now ignore BOs which are currently locked and potentially
 * changing their location.
 */
-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+   if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
!dma_resv_trylock(bo->tbo.base.resv))
return;
  
  	amdgpu_bo_get_memory(bo, stats);

-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-   dma_resv_unlock(bo->tbo.base.resv);
+   if (amdgpu_vm_is_bo_always_valid(vm, bo))


Beware the logic inversion here - luckily no one merged it yet! I will 
send a respin..


Regards,

Tvrtko


+   dma_resv_unlock(bo->tbo.base.resv);
  }
  
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,

@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
uncached = false;
}
  
-	if (clear || (bo && bo->tbo.base.resv ==

- vm->root.bo->tbo.base.resv))
+   if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
last_update = >last_update;
else
last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
 * the evicted list so that it gets validated again on the
 * next command submission.
 */
-   if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+   if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
uint32_t mem_type = bo->tbo.resource->mem_type;
  
  		if (!(bo->preferred_domains &

@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device 
*adev,
if (mapping->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-	if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

-   !bo_va->base.moved) {
+   if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
amdgpu_vm_bo_moved(_va->base);
-   }
+
trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  
@@ -1942,7 +1940,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,

if (before->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

+   if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
!before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
} else {
@@ -1957,7 +1955,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device 
*adev,
if (after->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

+   if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
!after->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
} else {
@@ -2037,7 +2035,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
  
  	if (bo) {

dma_resv_assert_held(bo->tbo.base.resv);
-   if (bo->tbo.base.resv ==

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-03 Thread Tvrtko Ursulin 



On 03/05/2024 07:26, Christian König wrote:

Am 29.04.24 um 18:47 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 40 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 28 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
  if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
  r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 8af3f0fd3073..01ca4b35b369 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
  dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
  amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))
+    dma_resv_unlock(bo->tbo.base.resv);
  }
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
-    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
  if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
@@ -1922,7 +1920,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !before->bo_va->base.moved)
  amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -1937,7 +1935,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (after->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !after->bo_va->base.moved)
  amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -2017,7 +2015,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
  if (bo) {
  dma_resv_assert_held(bo->tbo.base.resv);
-    if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))
  ttm_bo_set_bulk_move(>tbo, NULL);
  for (base = _va->base.bo->vm_bo; *base;
@@ -2111,7 +2109,7 @@ void amdgpu_vm_bo_invalidate(struct 
amdgpu_device *adev,

  for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
  struct amdgpu_vm *vm = bo_base->vm;
-    if

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-03 Thread Christian König 

Am 03.05.24 um 10:23 schrieb Tvrtko Ursulin:


On 03/05/2024 07:26, Christian König wrote:

Am 29.04.24 um 18:47 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 40 
+++--

  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 28 insertions(+), 16 deletions(-)

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

index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
  if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_vm_is_bo_always_valid(vm, abo))
  return -EPERM;
  r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 8af3f0fd3073..01ca4b35b369 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo))
  return;
  dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+    if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !dma_resv_trylock(bo->tbo.base.resv))
  return;
  amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))
+    dma_resv_unlock(bo->tbo.base.resv);
  }
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
-    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
  if (!(bo->preferred_domains &
@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
@@ -1922,7 +1920,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (before->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -1937,7 +1935,7 @@ int amdgpu_vm_bo_clear_mappings(struct 
amdgpu_device *adev,

  if (after->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
+    if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
  !after->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
  } else {
@@ -2017,7 +2015,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
  if (bo) {
  dma_resv_assert_held(bo->tbo.base.resv);
-    if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
+    if (amdgpu_vm_is_bo_always_valid(vm, bo))
  ttm_bo_set_bulk_move(>tbo, NULL);
  for (base = _va->base.bo->vm_bo; *base;
@@ -2111,7 +2109,7 @@ void amdgpu_vm_bo_invalidate(struct 
amdgpu_device *adev,

  for (bo_base = bo->vm_bo; bo_base; bo_base = bo_base->next) {
  struct amdgpu_vm *vm =

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-05-03 Thread Christian König 




Am 29.04.24 um 18:47 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_vm_is_bo_always_valid(vm, bo)

No functional changes.

v2:
  * Rename helper and move to amdgpu_vm. (Christian)

Signed-off-by: Tvrtko Ursulin 
Cc: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 40 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 ++
  3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 67c234bcf89f..e698d65e9508 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object 
*obj,
return -EPERM;
  
  	if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&

-   abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+   !amdgpu_vm_is_bo_always_valid(vm, abo))
return -EPERM;
  
  	r = amdgpu_bo_reserve(abo, false);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8af3f0fd3073..01ca4b35b369 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
base->next = bo->vm_bo;
bo->vm_bo = base;
  
-	if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)

+   if (!amdgpu_vm_is_bo_always_valid(vm, bo))
return;
  
  	dma_resv_assert_held(vm->root.bo->tbo.base.resv);

@@ -1101,13 +1101,13 @@ static void amdgpu_vm_bo_get_memory(struct amdgpu_bo_va 
*bo_va,
 * For now ignore BOs which are currently locked and potentially
 * changing their location.
 */
-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
+   if (!amdgpu_vm_is_bo_always_valid(vm, bo) &&
!dma_resv_trylock(bo->tbo.base.resv))
return;
  
  	amdgpu_bo_get_memory(bo, stats);

-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-   dma_resv_unlock(bo->tbo.base.resv);
+   if (amdgpu_vm_is_bo_always_valid(vm, bo))
+   dma_resv_unlock(bo->tbo.base.resv);
  }
  
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,

@@ -1203,8 +1203,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
uncached = false;
}
  
-	if (clear || (bo && bo->tbo.base.resv ==

- vm->root.bo->tbo.base.resv))
+   if (clear || amdgpu_vm_is_bo_always_valid(vm, bo))
last_update = >last_update;
else
last_update = _va->last_pt_update;
@@ -1246,7 +1245,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
 * the evicted list so that it gets validated again on the
 * next command submission.
 */
-   if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+   if (amdgpu_vm_is_bo_always_valid(vm, bo)) {
uint32_t mem_type = bo->tbo.resource->mem_type;
  
  		if (!(bo->preferred_domains &

@@ -1640,10 +1639,9 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device 
*adev,
if (mapping->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-	if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

-   !bo_va->base.moved) {
+   if (amdgpu_vm_is_bo_always_valid(vm, bo) && !bo_va->base.moved)
amdgpu_vm_bo_moved(_va->base);
-   }
+
trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  
@@ -1922,7 +1920,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,

if (before->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

+   if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
!before->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
} else {
@@ -1937,7 +1935,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device 
*adev,
if (after->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

+   if (amdgpu_vm_is_bo_always_valid(vm, bo) &&
!after->bo_va->base.moved)
amdgpu_vm_bo_moved(>bo_va->base);
} else {
@@ -2017,7 +2015,7 @@ void amdgpu_vm_bo_del(struct amdgpu_device *adev,
  
  	if (bo) {

dma_resv_assert_held(bo->tbo.base.resv);
-   if (bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
+   if (amdgpu_vm_is_bo_always_valid(vm, bo))
ttm_bo_set_bulk_move(>tbo, NULL);
  
  		for (base = _va->base.bo->vm_bo; *base;

@@

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-04-30 Thread Tvrtko Ursulin 



On 29/04/2024 12:02, Christian König wrote:

Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_bo_is_vm_bo(bo, vm)

No functional changes.


Ah,yes that was on my TODO list as well.

But I would have rather added this to the VM instead. In other words 
move it to amdgpu_vm.h and call it amdgpu_vm_is_bo_always_valid() or 
something like that.


I am happy to move it around as long as you are sure amdgpu_vm.h is the 
location.


For instance main API there it seems to be amdgpu_vm_bo's. At least all 
the amdgpu_bo usages do not needing anything more that the struct 
forward declared.


So if I move the helper there I either need to make it include another 
header, or move the helper out of line to amdgpu_vm.c.


Thoughts?

Regards,

Tvrtko



Signed-off-by: Tvrtko Ursulin 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 14 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 31 +-
  3 files changed, 28 insertions(+), 19 deletions(-)

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

index 67c234bcf89f..32e4a9c6e805 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
  if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_bo_is_vm_bo(abo, vm))
  return -EPERM;
  r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index be679c42b0b8..f2bb6965cc77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -307,6 +307,20 @@ static inline struct amdgpu_bo 
*amdgpu_bo_shadowed(struct amdgpu_bo *bo)

  return NULL;
  }
+/**
+ * amdgpu_bo_is_vm_bo - check if the BO is VM always valid
+ *
+ * @abo: BO to be tested.
+ * @vm: VM to test against.
+ *
+ * Returns true if the BO is VM always valid.
+ */
+static inline bool amdgpu_bo_is_vm_bo(struct amdgpu_bo *bo,
+  struct amdgpu_vm *vm)
+{
+    return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
+}
+
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
  void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 
domain);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 8af3f0fd3073..6d6f0e325172 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_bo_is_vm_bo(bo, vm))
  return;
  dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,12 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
-    !dma_resv_trylock(bo->tbo.base.resv))
+    if (!amdgpu_bo_is_vm_bo(bo, vm) && 
!dma_resv_trylock(bo->tbo.base.resv))

  return;
  amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_bo_is_vm_bo(bo, vm))
+    dma_resv_unlock(bo->tbo.base.resv);
  }
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1202,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
-    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_bo_is_vm_bo(bo, vm))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1244,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_bo_is_vm_bo(bo, vm)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
  if (!(bo->preferred_domains &
@@ -1640,10 +1638,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&
-    !bo_va->base.moved) {
+    if (amdgpu_bo_is_vm_bo(bo, vm) && !bo_va->base.moved)
  amdgpu_vm_bo_moved(_va->base);
-    }
+
  trace_amdgpu_vm_bo_map(bo_va,

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-04-29 Thread Christian König 

Am 29.04.24 um 15:34 schrieb Tvrtko Ursulin:


On 29/04/2024 12:02, Christian König wrote:

Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_bo_is_vm_bo(bo, vm)

No functional changes.


Ah,yes that was on my TODO list as well.

But I would have rather added this to the VM instead. In other words 
move it to amdgpu_vm.h and call it amdgpu_vm_is_bo_always_valid() or 
something like that.


I am happy to move it around as long as you are sure amdgpu_vm.h is 
the location.


For instance main API there it seems to be amdgpu_vm_bo's. At least 
all the amdgpu_bo usages do not needing anything more that the struct 
forward declared.


So if I move the helper there I either need to make it include another 
header, or move the helper out of line to amdgpu_vm.c.


Thoughts?


amdgpu_vm.c is fine as well. I just though that something so simply 
could be an inline function in the header as well.


Regards,
Christian.



Regards,

Tvrtko



Signed-off-by: Tvrtko Ursulin 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c    |  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 14 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 31 
+-

  3 files changed, 28 insertions(+), 19 deletions(-)

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

index 67c234bcf89f..32e4a9c6e805 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct 
drm_gem_object *obj,

  return -EPERM;
  if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&
-    abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    !amdgpu_bo_is_vm_bo(abo, vm))
  return -EPERM;
  r = amdgpu_bo_reserve(abo, false);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index be679c42b0b8..f2bb6965cc77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -307,6 +307,20 @@ static inline struct amdgpu_bo 
*amdgpu_bo_shadowed(struct amdgpu_bo *bo)

  return NULL;
  }
+/**
+ * amdgpu_bo_is_vm_bo - check if the BO is VM always valid
+ *
+ * @abo: BO to be tested.
+ * @vm: VM to test against.
+ *
+ * Returns true if the BO is VM always valid.
+ */
+static inline bool amdgpu_bo_is_vm_bo(struct amdgpu_bo *bo,
+  struct amdgpu_vm *vm)
+{
+    return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
+}
+
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
  void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 
domain);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 8af3f0fd3073..6d6f0e325172 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct 
amdgpu_vm_bo_base *base,

  base->next = bo->vm_bo;
  bo->vm_bo = base;
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+    if (!amdgpu_bo_is_vm_bo(bo, vm))
  return;
  dma_resv_assert_held(vm->root.bo->tbo.base.resv);
@@ -1101,13 +1101,12 @@ static void amdgpu_vm_bo_get_memory(struct 
amdgpu_bo_va *bo_va,

   * For now ignore BOs which are currently locked and potentially
   * changing their location.
   */
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
-    !dma_resv_trylock(bo->tbo.base.resv))
+    if (!amdgpu_bo_is_vm_bo(bo, vm) && 
!dma_resv_trylock(bo->tbo.base.resv))

  return;
  amdgpu_bo_get_memory(bo, stats);
-    if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-    dma_resv_unlock(bo->tbo.base.resv);
+    if (amdgpu_bo_is_vm_bo(bo, vm))
+    dma_resv_unlock(bo->tbo.base.resv);
  }
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,
@@ -1203,8 +1202,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

  uncached = false;
  }
-    if (clear || (bo && bo->tbo.base.resv ==
-  vm->root.bo->tbo.base.resv))
+    if (clear || amdgpu_bo_is_vm_bo(bo, vm))
  last_update = >last_update;
  else
  last_update = _va->last_pt_update;
@@ -1246,7 +1244,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device 
*adev, struct amdgpu_bo_va *bo_va,

   * the evicted list so that it gets validated again on the
   * next command submission.
   */
-    if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+    if (amdgpu_bo_is_vm_bo(bo, vm)) {
  uint32_t mem_type = bo->tbo.resource->mem_type;
  if (!(bo->preferred_domains &
@@ -1640,10 +1638,9 @@ static void amdgpu_vm_bo_insert_map(struct 
amdgpu_device *adev,

  if (mapping->flags & AMDGPU_PTE_PRT)
  amdgpu_vm_prt_get(adev);
-    if (bo && bo->tbo.base.resv ==

Re: [PATCH 1/3] drm/amdgpu: Add amdgpu_bo_is_vm_bo helper

2024-04-29 Thread Christian König 

Am 26.04.24 um 18:43 schrieb Tvrtko Ursulin:

From: Tvrtko Ursulin 

Help code readability by replacing a bunch of:

bo->tbo.base.resv == vm->root.bo->tbo.base.resv

With:

amdgpu_bo_is_vm_bo(bo, vm)

No functional changes.


Ah,yes that was on my TODO list as well.

But I would have rather added this to the VM instead. In other words 
move it to amdgpu_vm.h and call it amdgpu_vm_is_bo_always_valid() or 
something like that.


Regards,
Christian.



Signed-off-by: Tvrtko Ursulin 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c|  2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 14 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 31 +-
  3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 67c234bcf89f..32e4a9c6e805 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -174,7 +174,7 @@ static int amdgpu_gem_object_open(struct drm_gem_object 
*obj,
return -EPERM;
  
  	if (abo->flags & AMDGPU_GEM_CREATE_VM_ALWAYS_VALID &&

-   abo->tbo.base.resv != vm->root.bo->tbo.base.resv)
+   !amdgpu_bo_is_vm_bo(abo, vm))
return -EPERM;
  
  	r = amdgpu_bo_reserve(abo, false);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index be679c42b0b8..f2bb6965cc77 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -307,6 +307,20 @@ static inline struct amdgpu_bo *amdgpu_bo_shadowed(struct 
amdgpu_bo *bo)
return NULL;
  }
  
+/**

+ * amdgpu_bo_is_vm_bo - check if the BO is VM always valid
+ *
+ * @abo: BO to be tested.
+ * @vm: VM to test against.
+ *
+ * Returns true if the BO is VM always valid.
+ */
+static inline bool amdgpu_bo_is_vm_bo(struct amdgpu_bo *bo,
+ struct amdgpu_vm *vm)
+{
+   return bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv;
+}
+
  bool amdgpu_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
  void amdgpu_bo_placement_from_domain(struct amdgpu_bo *abo, u32 domain);
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

index 8af3f0fd3073..6d6f0e325172 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -333,7 +333,7 @@ void amdgpu_vm_bo_base_init(struct amdgpu_vm_bo_base *base,
base->next = bo->vm_bo;
bo->vm_bo = base;
  
-	if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)

+   if (!amdgpu_bo_is_vm_bo(bo, vm))
return;
  
  	dma_resv_assert_held(vm->root.bo->tbo.base.resv);

@@ -1101,13 +1101,12 @@ static void amdgpu_vm_bo_get_memory(struct amdgpu_bo_va 
*bo_va,
 * For now ignore BOs which are currently locked and potentially
 * changing their location.
 */
-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv &&
-   !dma_resv_trylock(bo->tbo.base.resv))
+   if (!amdgpu_bo_is_vm_bo(bo, vm) && !dma_resv_trylock(bo->tbo.base.resv))
return;
  
  	amdgpu_bo_get_memory(bo, stats);

-   if (bo->tbo.base.resv != vm->root.bo->tbo.base.resv)
-   dma_resv_unlock(bo->tbo.base.resv);
+   if (amdgpu_bo_is_vm_bo(bo, vm))
+   dma_resv_unlock(bo->tbo.base.resv);
  }
  
  void amdgpu_vm_get_memory(struct amdgpu_vm *vm,

@@ -1203,8 +1202,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
uncached = false;
}
  
-	if (clear || (bo && bo->tbo.base.resv ==

- vm->root.bo->tbo.base.resv))
+   if (clear || amdgpu_bo_is_vm_bo(bo, vm))
last_update = >last_update;
else
last_update = _va->last_pt_update;
@@ -1246,7 +1244,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, 
struct amdgpu_bo_va *bo_va,
 * the evicted list so that it gets validated again on the
 * next command submission.
 */
-   if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv) {
+   if (amdgpu_bo_is_vm_bo(bo, vm)) {
uint32_t mem_type = bo->tbo.resource->mem_type;
  
  		if (!(bo->preferred_domains &

@@ -1640,10 +1638,9 @@ static void amdgpu_vm_bo_insert_map(struct amdgpu_device 
*adev,
if (mapping->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-	if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&

-   !bo_va->base.moved) {
+   if (amdgpu_bo_is_vm_bo(bo, vm) && !bo_va->base.moved)
amdgpu_vm_bo_moved(_va->base);
-   }
+
trace_amdgpu_vm_bo_map(bo_va, mapping);
  }
  
@@ -1922,8 +1919,7 @@ int amdgpu_vm_bo_clear_mappings(struct amdgpu_device *adev,

if (before->flags & AMDGPU_PTE_PRT)
amdgpu_vm_prt_get(adev);
  
-		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv &&