Hi Tvrtko,
Some nits inline, but feel free to ignore them if you don't think they
are reasonable. Apart from that,
Reviewed-by: Maíra Canal <mca...@igalia.com>
On 18/03/25 12:54, Tvrtko Ursulin wrote:
Helper which fails to consolidate the code and instead just forks into two
copies of the code based on a boolean parameter is not very helpful or
readable. Lets just remove it and proof in the pudding is the net smaller
code.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
---
drivers/gpu/drm/drm_syncobj.c | 98 ++++++++++++++++-------------------
1 file changed, 44 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4f2ab8a7b50f..d0d60c331df8 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1221,42 +1221,6 @@ signed long drm_timeout_abs_to_jiffies(int64_t
timeout_nsec)
}
EXPORT_SYMBOL(drm_timeout_abs_to_jiffies);
-static int drm_syncobj_array_wait(struct drm_device *dev,
- struct drm_file *file_private,
- struct drm_syncobj_wait *wait,
- struct drm_syncobj_timeline_wait
*timeline_wait,
- struct drm_syncobj **syncobjs, bool timeline,
- ktime_t *deadline)
-{
- signed long timeout = 0;
- uint32_t first = ~0;
-
- if (!timeline) {
- timeout = drm_timeout_abs_to_jiffies(wait->timeout_nsec);
- timeout = drm_syncobj_array_wait_timeout(syncobjs,
- NULL,
- wait->count_handles,
- wait->flags,
- timeout, &first,
- deadline);
- if (timeout < 0)
- return timeout;
- wait->first_signaled = first;
- } else {
- timeout =
drm_timeout_abs_to_jiffies(timeline_wait->timeout_nsec);
- timeout = drm_syncobj_array_wait_timeout(syncobjs,
-
u64_to_user_ptr(timeline_wait->points),
-
timeline_wait->count_handles,
- timeline_wait->flags,
- timeout, &first,
- deadline);
- if (timeout < 0)
- return timeout;
- timeline_wait->first_signaled = first;
- }
- return 0;
-}
-
static int drm_syncobj_array_find(struct drm_file *file_private,
void __user *user_handles,
uint32_t count_handles,
@@ -1319,9 +1283,12 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void
*data,
struct drm_file *file_private)
{
struct drm_syncobj_wait *args = data;
+ ktime_t deadline, *pdeadline = NULL;
+ u32 count = args->count_handles;
From my point of view, this variable didn't make the code more readable.
I'd prefer to keep using `args->count_handles` in the code.
struct drm_syncobj **syncobjs;
unsigned int possible_flags;
- ktime_t t, *tp = NULL;
+ u32 first = ~0;
+ long timeout;
int ret = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
@@ -1334,27 +1301,37 @@ drm_syncobj_wait_ioctl(struct drm_device *dev, void
*data,
if (args->flags & ~possible_flags)
return -EINVAL;
- if (args->count_handles == 0)
+ if (count == 0)
return 0;
ret = drm_syncobj_array_find(file_private,
u64_to_user_ptr(args->handles),
- args->count_handles,
+ count,
&syncobjs);
if (ret < 0)
return ret;
if (args->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE) {
- t = ns_to_ktime(args->deadline_nsec);
- tp = &t;
+ deadline = ns_to_ktime(args->deadline_nsec);
+ pdeadline = &deadline;
}
- ret = drm_syncobj_array_wait(dev, file_private,
- args, NULL, syncobjs, false, tp);
+ timeout = drm_syncobj_array_wait_timeout(syncobjs,
+ NULL,
+ count,
+ args->flags,
+
drm_timeout_abs_to_jiffies(args->timeout_nsec),
Could you create a variable for the timeout instead of adding the
function as a parameter?
Same comments for `drm_syncobj_timeline_wait_ioctl()`.
Best Regards,
- Maíra
+ &first,
+ pdeadline);
- drm_syncobj_array_free(syncobjs, args->count_handles);
+ drm_syncobj_array_free(syncobjs, count);
- return ret;
+ if (timeout < 0)
+ return timeout;
+
+ args->first_signaled = first;
+
+ return 0;
}
int
@@ -1362,9 +1339,12 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_private)
{
struct drm_syncobj_timeline_wait *args = data;
+ ktime_t deadline, *pdeadline = NULL;
+ u32 count = args->count_handles;
struct drm_syncobj **syncobjs;
unsigned int possible_flags;
- ktime_t t, *tp = NULL;
+ u32 first = ~0;
+ long timeout;
int ret = 0;
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
@@ -1378,27 +1358,37 @@ drm_syncobj_timeline_wait_ioctl(struct drm_device *dev,
void *data,
if (args->flags & ~possible_flags)
return -EINVAL;
- if (args->count_handles == 0)
+ if (count == 0)
return 0;
ret = drm_syncobj_array_find(file_private,
u64_to_user_ptr(args->handles),
- args->count_handles,
+ count,
&syncobjs);
if (ret < 0)
return ret;
if (args->flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_DEADLINE) {
- t = ns_to_ktime(args->deadline_nsec);
- tp = &t;
+ deadline = ns_to_ktime(args->deadline_nsec);
+ pdeadline = &deadline;
}
- ret = drm_syncobj_array_wait(dev, file_private,
- NULL, args, syncobjs, true, tp);
+ timeout = drm_syncobj_array_wait_timeout(syncobjs,
+ u64_to_user_ptr(args->points),
+ count,
+ args->flags,
+
drm_timeout_abs_to_jiffies(args->timeout_nsec),
+ &first,
+ pdeadline);
- drm_syncobj_array_free(syncobjs, args->count_handles);
+ drm_syncobj_array_free(syncobjs, count);
- return ret;
+ if (timeout < 0)
+ return timeout;
+
+ args->first_signaled = first;
+
+ return 0;
}
static void syncobj_eventfd_entry_fence_func(struct dma_fence *fence,
