On 8/10/26 11:18, Bob Zhou wrote: > amdgpu_gem_va_update_vm() returns a fence for every immediate VA > update, but the ioctl caller only consumes that fence when userspace > requests a VM timeline syncobj. Without a timeline syncobj, the helper > may take fence references or allocate a merged fence that is > immediately released. > > Pass the timeline syncobj into the helper and skip fence construction > when no output fence is requested. Preserve the existing stub, > merged-fence and error behavior for callers that do request a timeline > update.
That just massively complicates the handling without any gain at all. Why the heck should we do that? Regards, Christian. > > Signed-off-by: Bob Zhou <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > index f754a4a3a1c22..2837553e4f58a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c > @@ -754,6 +754,7 @@ 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 > + * @timeline_syncobj: VM timeline syncobj the fence gets attached to, or NULL > * > * Update the bo_va directly after setting its address. Errors are not > * vital here, so they are not reported back to userspace. > @@ -765,15 +766,15 @@ 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) > + uint32_t operation, > + struct drm_syncobj *timeline_syncobj) > { > struct dma_fence *fence; > int r = 0; > > /* If the VM is not ready return only a stub. */ > if (!amdgpu_vm_ready(vm)) > - return dma_fence_get_stub(); > - > + return timeline_syncobj ? dma_fence_get_stub() : NULL; > > /* > * First clean up any freed mappings in the VM. > @@ -799,6 +800,14 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev, > if (r) > goto error; > > + /* > + * The VM update work above is already committed. If the caller does > + * not need a fence (no VM timeline syncobj was requested) skip building > + * the otherwise-unused merged/last-update fence. > + */ > + if (!timeline_syncobj) > + return NULL; > + > if ((operation == AMDGPU_VA_OP_MAP || > operation == AMDGPU_VA_OP_REPLACE) && > !amdgpu_vm_is_bo_always_valid(vm, bo_va->base.bo)) { > @@ -826,7 +835,7 @@ amdgpu_gem_va_update_vm(struct amdgpu_device *adev, > if (r && r != -ERESTARTSYS) > DRM_ERROR("Couldn't update BO_VA (%d)\n", r); > > - return dma_fence_get(vm->last_update); > + return timeline_syncobj ? dma_fence_get(vm->last_update) : NULL; > } > > int amdgpu_gem_va_ioctl(struct drm_device *dev, void *data, > @@ -996,7 +1005,8 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, void > *data, > struct dma_fence *fence; > > fence = amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va, > - args->operation); > + args->operation, > + timeline_syncobj); > if (timeline_syncobj) { > if (!args->vm_timeline_point) { > /* Replace the existing fence when no point is > given. */
