[PATCH] drm/amdgpu: Refactor amdgpu_gem_va_ioctl for Handling Last Fence Update and Timeline Management

Srinivasan Shanmugam srinivasan.shanmugam at amd.com
Thu Apr 24 03:21:23 UTC 2025


This commit simplifies the amdgpu_gem_va_ioctl function, key updates
include:
 - Moved the logic for managing the last update fence directly into
   amdgpu_gem_va_update_vm.
 - Introduced checks for the timeline point to enable conditional
   replacement or addition of fences.

Cc: Alex Deucher <alexander.deucher at amd.com>
Cc: Christian König <christian.koenig at amd.com>
Suggested-by: Christian König <christian.koenig at amd.com>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam at amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 96 +++++++++++--------------
 1 file changed, 41 insertions(+), 55 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index f03fc3cf4d50..c83947c0269b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -112,47 +112,6 @@ amdgpu_gem_update_timeline_node(struct drm_file *filp,
 	return 0;
 }
 
-static void
-amdgpu_gem_update_bo_mapping(struct drm_file *filp,
-			     struct amdgpu_bo_va *bo_va,
-			     uint32_t operation,
-			     uint64_t point,
-			     struct dma_fence *fence,
-			     struct drm_syncobj *syncobj,
-			     struct dma_fence_chain *chain)
-{
-	struct amdgpu_bo *bo = bo_va ? bo_va->base.bo : NULL;
-	struct amdgpu_fpriv *fpriv = filp->driver_priv;
-	struct amdgpu_vm *vm = &fpriv->vm;
-	struct dma_fence *last_update;
-
-	if (!syncobj)
-		return;
-
-	/* Find the last update fence */
-	switch (operation) {
-	case AMDGPU_VA_OP_MAP:
-	case AMDGPU_VA_OP_REPLACE:
-		if (bo && (bo->tbo.base.resv == vm->root.bo->tbo.base.resv))
-			last_update = vm->last_update;
-		else
-			last_update = bo_va->last_pt_update;
-		break;
-	case AMDGPU_VA_OP_UNMAP:
-	case AMDGPU_VA_OP_CLEAR:
-		last_update = fence;
-		break;
-	default:
-		return;
-	}
-
-	/* Add fence to timeline */
-	if (!point)
-		drm_syncobj_replace_fence(syncobj, last_update);
-	else
-		drm_syncobj_add_point(syncobj, chain, last_update, point);
-}
-
 static vm_fault_t amdgpu_gem_fault(struct vm_fault *vmf)
 {
 	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
@@ -751,6 +710,8 @@ int amdgpu_gem_metadata_ioctl(struct drm_device *dev, void *data,
  * @vm: vm to update
  * @bo_va: bo_va to update
  * @operation: map, unmap or clear
+ * @last_update: a pointer to a pointer where the last update fence will
+ * be stored, reflecting the most recent mapping or update operation.
  *
  * Update the bo_va directly after setting its address. Errors are not
  * vital here, so they are not reported back to userspace.
@@ -762,9 +723,11 @@ static struct dma_fence *
 amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 			struct amdgpu_vm *vm,
 			struct amdgpu_bo_va *bo_va,
-			uint32_t operation)
+			u32 operation,
+			struct dma_fence **last_update)
 {
 	struct dma_fence *fence = dma_fence_get_stub();
+	struct amdgpu_bo *bo = bo_va ? bo_va->base.bo : NULL;
 	int r;
 
 	if (!amdgpu_vm_ready(vm))
@@ -774,11 +737,22 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
 	if (r)
 		goto error;
 
-	if (operation == AMDGPU_VA_OP_MAP ||
-	    operation == AMDGPU_VA_OP_REPLACE) {
+	if (operation == AMDGPU_VA_OP_MAP || operation == AMDGPU_VA_OP_REPLACE) {
 		r = amdgpu_vm_bo_update(adev, bo_va, false);
 		if (r)
 			goto error;
+
+		/* Set the last_update fence based on the operation */
+		if (bo && bo->tbo.base.resv == vm->root.bo->tbo.base.resv)
+			/* Use VM's last update fence */
+			*last_update = vm->last_update;
+		else
+			/* Use buffer object's last update fence */
+			*last_update = bo_va->last_pt_update;
+
+	} else if (operation == AMDGPU_VA_OP_UNMAP || operation == AMDGPU_VA_OP_CLEAR) {
+		/* Assigning the temporary fence for unmap/clear */
+		*last_update = fence;
 	}
 
 	r = amdgpu_vm_update_pdes(adev, vm, false);
@@ -839,6 +813,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	struct drm_syncobj *timeline_syncobj = NULL;
 	struct dma_fence_chain *timeline_chain = NULL;
 	struct dma_fence *fence;
+	struct dma_fence *last_update_fence = NULL;
 	struct drm_exec exec;
 	uint64_t va_flags;
 	uint64_t vm_size;
@@ -934,6 +909,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 		bo_va = NULL;
 	}
 
+	/* Update timeline node for synchronization */
 	r = amdgpu_gem_update_timeline_node(filp,
 					    args->vm_timeline_syncobj_out,
 					    args->vm_timeline_point,
@@ -942,6 +918,10 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	if (r)
 		goto error;
 
+	/* Call to update VM and retrieve the resulting fence */
+	fence = amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va,
+					args->operation, &last_update_fence);
+
 	switch (args->operation) {
 	case AMDGPU_VA_OP_MAP:
 		va_flags = amdgpu_gem_va_map_flags(adev, args->flags);
@@ -967,19 +947,25 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data,
 	default:
 		break;
 	}
+
 	if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && !adev->debug_vm) {
-		fence = amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va,
-						args->operation);
-
-		if (timeline_syncobj)
-			amdgpu_gem_update_bo_mapping(filp, bo_va,
-					     args->operation,
-					     args->vm_timeline_point,
-					     fence, timeline_syncobj,
-					     timeline_chain);
-		else
+		if (timeline_syncobj) {
+			if (last_update_fence) {
+				if (!args->vm_timeline_point) {
+					/* Replace the existing fence if point is not set */
+					drm_syncobj_replace_fence(timeline_syncobj,
+								  last_update_fence);
+				} else {
+					/* Add last_update_fence at a specific point */
+					drm_syncobj_add_point(timeline_syncobj,
+							      timeline_chain,
+							      last_update_fence,
+							      args->vm_timeline_point);
+				}
+			}
+		} else {
 			dma_fence_put(fence);
-
+		}
 	}
 
 error:
-- 
2.34.1



More information about the amd-gfx mailing list