Am 25.09.24 um 21:59 schrieb Arunpravin Paneer Selvam:
Add user fence wait IOCTL timeline syncobj support.
v2:(Christian)
- handle dma_fence_wait() return value.
- shorten the variable name syncobj_timeline_points a bit.
- move num_points up to avoid padding issues.
Signed-off-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@xxxxxxx>
---
.../gpu/drm/amd/amdgpu/amdgpu_userq_fence.c | 82 +++++++++++++++++--
include/uapi/drm/amdgpu_drm.h | 16 +++-
2 files changed, 88 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
index 97b1af574407..2465ca307644 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq_fence.c
@@ -474,11 +474,11 @@ int amdgpu_userq_signal_ioctl(struct drm_device *dev, void *data,
int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
struct drm_file *filp)
{
+ u32 *syncobj_handles, *timeline_points, *timeline_handles, *bo_handles;
+ u32 num_syncobj, num_bo_handles, num_points;
struct drm_amdgpu_userq_fence_info *fence_info = NULL;
struct drm_amdgpu_userq_wait *wait_info = data;
- u32 *syncobj_handles, *bo_handles;
struct dma_fence **fences = NULL;
- u32 num_syncobj, num_bo_handles;
struct drm_gem_object **gobj;
struct drm_exec exec;
int r, i, entry, cnt;
@@ -498,11 +498,26 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
goto free_bo_handles;
}
+ num_points = wait_info->num_points;
+ timeline_handles = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_handles),
+ sizeof(u32) * num_points);
+ if (IS_ERR(timeline_handles)) {
+ r = PTR_ERR(timeline_handles);
+ goto free_syncobj_handles;
+ }
+
+ timeline_points = memdup_user(u64_to_user_ptr(wait_info->syncobj_timeline_points),
+ sizeof(u32) * num_points);
+ if (IS_ERR(timeline_points)) {
+ r = PTR_ERR(timeline_points);
+ goto free_timeline_handles;
+ }
+
/* Array of GEM object handles */
gobj = kmalloc_array(num_bo_handles, sizeof(*gobj), GFP_KERNEL);
if (!gobj) {
r = -ENOMEM;
- goto free_syncobj_handles;
+ goto free_timeline_points;
}
for (entry = 0; entry < num_bo_handles; entry++) {
@@ -524,17 +539,34 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
if (!wait_info->num_fences) {
+ if (num_points) {
+ struct dma_fence *fence;
++
+ for (i = 0; i < num_points; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r || !fence)
You can't simply ignore errors here. That needs some goto error handling
and cleanup.
+ continue;
+
+ dma_fence_put(fence);
+ num_fences++;
There can be more than one fence in the timeline we need to wait for.
You should probably use dma_fence_unwrap_for_each() here.
+ }
+ }
+
/* Count syncobj's fence */
for (i = 0; i < num_syncobj; i++) {
struct dma_fence *fence;
r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0, 0, &fence);
- dma_fence_put(fence);
-
+ 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
if (r || !fence)
continue;
+ dma_fence_put(fence);
num_fences++;
}
@@ -589,12 +621,34 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
}
}
+ if (num_points) {
+ struct dma_fence *fence;
+
+ for (i = 0; i < num_points; i++) {
+ r = drm_syncobj_find_fence(filp, timeline_handles[i],
+ timeline_points[i],
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
+ if (r || !fence)
Same here, just ignoring errors is a no-go.
Regards,
Christian.
+ continue;
+
+ if (WARN_ON_ONCE(num_fences >= wait_info->num_fences)) {
+ r = -EINVAL;
+ goto free_fences;
+ }
+
+ fences[num_fences++] = fence;
+ }
+ }
+
/* Retrieve syncobj's fence */
for (i = 0; i < num_syncobj; i++) {
struct dma_fence *fence;
r = drm_syncobj_find_fence(filp, syncobj_handles[i],
- 0, 0, &fence);
+ 0,
+ DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT,
+ &fence);
if (r || !fence)
continue;
@@ -617,9 +671,13 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
* Just waiting on other driver fences should
* be good for now
*/
- dma_fence_wait(fences[i], false);
- dma_fence_put(fences[i]);
+ r = dma_fence_wait(fences[i], true);
+ if (r) {
+ dma_fence_put(fences[i]);
+ goto free_fences;
+ }
+ dma_fence_put(fences[i]);
continue;
}
@@ -665,6 +723,8 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
drm_gem_object_put(gobj[i]);
kfree(gobj);
+ kfree(timeline_points);
+ kfree(timeline_handles);
kfree(syncobj_handles);
kfree(bo_handles);
@@ -682,6 +742,10 @@ int amdgpu_userq_wait_ioctl(struct drm_device *dev, void *data,
while (entry-- > 0)
drm_gem_object_put(gobj[entry]);
kfree(gobj);
+free_timeline_points:
+ kfree(timeline_points);
+free_timeline_handles:
+ kfree(timeline_handles);
free_syncobj_handles:
kfree(syncobj_handles);
free_bo_handles:
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index af42798e901d..3b24e0cb1b51 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -521,12 +521,26 @@ struct drm_amdgpu_userq_wait {
* matching fence wait info pair in @userq_fence_info.
*/
__u32 bo_wait_flags;
- __u32 pad;
+ /**
+ * @num_points: A count that represents the number of timeline syncobj handles in
+ * syncobj_handles_array.
+ */
+ __u32 num_points;
/**
* @syncobj_handles_array: An array of syncobj handles defined to get the
* fence wait information of every syncobj handles in the array.
*/
__u64 syncobj_handles_array;
+ /**
+ * @syncobj_timeline_handles: An array of timeline syncobj handles defined to get the
+ * fence wait information of every timeline syncobj handles in the array.
+ */
+ __u64 syncobj_timeline_handles;
+ /**
+ * @syncobj_timeline_points: An array of timeline syncobj points defined to get the
+ * fence wait points of every timeline syncobj handles in the syncobj_handles_array.
+ */
+ __u64 syncobj_timeline_points;
/**
* @bo_handles_array: An array of GEM BO handles defined to fetch the fence
* wait information of every BO handles in the array.
