[PATCH 04/10] drm/syncobj: remove drm_syncobj_cb and cleanup
Christian König
ckoenig.leichtzumerken at gmail.com
Fri Dec 7 14:23:16 UTC 2018
Am 07.12.18 um 15:20 schrieb Daniel Vetter:
> On Wed, Dec 05, 2018 at 11:00:33AM +0100, Christian König wrote:
>> Hi Daniel,
>>
>> can I get a review for this one? It is essentially just a follow up cleanup
>> on one of your patches and shouldn't have any functional effect.
> Unfortunately badly backlogged an a handful of massive context switches
> away from getting back to this. Also brain's all mushed up :-/
>
> I think better to get Chris/Jason/Dave to take a look and ack.
>
> Apologies :-(
No problem, I'm totally overworked as well.
Christian.
> -Daniel
>
>> Thanks,
>> Christian.
>>
>> Am 04.12.18 um 12:59 schrieb Christian König:
>>> This completes "drm/syncobj: Drop add/remove_callback from driver
>>> interface" and cleans up the implementation a bit.
>>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> ---
>>> drivers/gpu/drm/drm_syncobj.c | 91 ++++++++++++++-----------------------------
>>> include/drm/drm_syncobj.h | 21 ----------
>>> 2 files changed, 30 insertions(+), 82 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index db30a0e89db8..e19525af0cce 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -56,6 +56,16 @@
>>> #include "drm_internal.h"
>>> #include <drm/drm_syncobj.h>
>>> +struct syncobj_wait_entry {
>>> + struct list_head node;
>>> + struct task_struct *task;
>>> + struct dma_fence *fence;
>>> + struct dma_fence_cb fence_cb;
>>> +};
>>> +
>>> +static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
>>> + struct syncobj_wait_entry *wait);
>>> +
>>> /**
>>> * drm_syncobj_find - lookup and reference a sync object.
>>> * @file_private: drm file private pointer
>>> @@ -82,58 +92,33 @@ struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,
>>> }
>>> EXPORT_SYMBOL(drm_syncobj_find);
>>> -static void drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
>>> - struct drm_syncobj_cb *cb,
>>> - drm_syncobj_func_t func)
>>> +static void drm_syncobj_fence_add_wait(struct drm_syncobj *syncobj,
>>> + struct syncobj_wait_entry *wait)
>>> {
>>> - cb->func = func;
>>> - list_add_tail(&cb->node, &syncobj->cb_list);
>>> -}
>>> -
>>> -static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>>> - struct dma_fence **fence,
>>> - struct drm_syncobj_cb *cb,
>>> - drm_syncobj_func_t func)
>>> -{
>>> - int ret;
>>> -
>>> - *fence = drm_syncobj_fence_get(syncobj);
>>> - if (*fence)
>>> - return 1;
>>> + if (wait->fence)
>>> + return;
>>> spin_lock(&syncobj->lock);
>>> /* We've already tried once to get a fence and failed. Now that we
>>> * have the lock, try one more time just to be sure we don't add a
>>> * callback when a fence has already been set.
>>> */
>>> - if (syncobj->fence) {
>>> - *fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
>>> - lockdep_is_held(&syncobj->lock)));
>>> - ret = 1;
>>> - } else {
>>> - *fence = NULL;
>>> - drm_syncobj_add_callback_locked(syncobj, cb, func);
>>> - ret = 0;
>>> - }
>>> + if (syncobj->fence)
>>> + wait->fence = dma_fence_get(
>>> + rcu_dereference_protected(syncobj->fence, 1));
>>> + else
>>> + list_add_tail(&wait->node, &syncobj->cb_list);
>>> spin_unlock(&syncobj->lock);
>>> -
>>> - return ret;
>>> }
>>> -void drm_syncobj_add_callback(struct drm_syncobj *syncobj,
>>> - struct drm_syncobj_cb *cb,
>>> - drm_syncobj_func_t func)
>>> +static void drm_syncobj_remove_wait(struct drm_syncobj *syncobj,
>>> + struct syncobj_wait_entry *wait)
>>> {
>>> - spin_lock(&syncobj->lock);
>>> - drm_syncobj_add_callback_locked(syncobj, cb, func);
>>> - spin_unlock(&syncobj->lock);
>>> -}
>>> + if (!wait->node.next)
>>> + return;
>>> -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
>>> - struct drm_syncobj_cb *cb)
>>> -{
>>> spin_lock(&syncobj->lock);
>>> - list_del_init(&cb->node);
>>> + list_del_init(&wait->node);
>>> spin_unlock(&syncobj->lock);
>>> }
>>> @@ -148,7 +133,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
>>> struct dma_fence *fence)
>>> {
>>> struct dma_fence *old_fence;
>>> - struct drm_syncobj_cb *cur, *tmp;
>>> + struct syncobj_wait_entry *cur, *tmp;
>>> if (fence)
>>> dma_fence_get(fence);
>>> @@ -162,7 +147,7 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
>>> if (fence != old_fence) {
>>> list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) {
>>> list_del_init(&cur->node);
>>> - cur->func(syncobj, cur);
>>> + syncobj_wait_syncobj_func(syncobj, cur);
>>> }
>>> }
>>> @@ -608,13 +593,6 @@ drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void *data,
>>> &args->handle);
>>> }
>>> -struct syncobj_wait_entry {
>>> - struct task_struct *task;
>>> - struct dma_fence *fence;
>>> - struct dma_fence_cb fence_cb;
>>> - struct drm_syncobj_cb syncobj_cb;
>>> -};
>>> -
>>> static void syncobj_wait_fence_func(struct dma_fence *fence,
>>> struct dma_fence_cb *cb)
>>> {
>>> @@ -625,11 +603,8 @@ static void syncobj_wait_fence_func(struct dma_fence *fence,
>>> }
>>> static void syncobj_wait_syncobj_func(struct drm_syncobj *syncobj,
>>> - struct drm_syncobj_cb *cb)
>>> + struct syncobj_wait_entry *wait)
>>> {
>>> - struct syncobj_wait_entry *wait =
>>> - container_of(cb, struct syncobj_wait_entry, syncobj_cb);
>>> -
>>> /* This happens inside the syncobj lock */
>>> wait->fence = dma_fence_get(rcu_dereference_protected(syncobj->fence,
>>> lockdep_is_held(&syncobj->lock)));
>>> @@ -688,12 +663,8 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>>> */
>>> if (flags & DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT) {
>>> - for (i = 0; i < count; ++i) {
>>> - drm_syncobj_fence_get_or_add_callback(syncobjs[i],
>>> - &entries[i].fence,
>>> - &entries[i].syncobj_cb,
>>> - syncobj_wait_syncobj_func);
>>> - }
>>> + for (i = 0; i < count; ++i)
>>> + drm_syncobj_fence_add_wait(syncobjs[i], &entries[i]);
>>> }
>>> do {
>>> @@ -742,9 +713,7 @@ static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,
>>> cleanup_entries:
>>> for (i = 0; i < count; ++i) {
>>> - if (entries[i].syncobj_cb.func)
>>> - drm_syncobj_remove_callback(syncobjs[i],
>>> - &entries[i].syncobj_cb);
>>> + drm_syncobj_remove_wait(syncobjs[i], &entries[i]);
>>> if (entries[i].fence_cb.func)
>>> dma_fence_remove_callback(entries[i].fence,
>>> &entries[i].fence_cb);
>>> diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
>>> index b1fe921f8e8f..7c6ed845c70d 100644
>>> --- a/include/drm/drm_syncobj.h
>>> +++ b/include/drm/drm_syncobj.h
>>> @@ -28,8 +28,6 @@
>>> #include "linux/dma-fence.h"
>>> -struct drm_syncobj_cb;
>>> -
>>> /**
>>> * struct drm_syncobj - sync object.
>>> *
>>> @@ -62,25 +60,6 @@ struct drm_syncobj {
>>> struct file *file;
>>> };
>>> -typedef void (*drm_syncobj_func_t)(struct drm_syncobj *syncobj,
>>> - struct drm_syncobj_cb *cb);
>>> -
>>> -/**
>>> - * struct drm_syncobj_cb - callback for drm_syncobj_add_callback
>>> - * @node: used by drm_syncob_add_callback to append this struct to
>>> - * &drm_syncobj.cb_list
>>> - * @func: drm_syncobj_func_t to call
>>> - *
>>> - * This struct will be initialized by drm_syncobj_add_callback, additional
>>> - * data can be passed along by embedding drm_syncobj_cb in another struct.
>>> - * The callback will get called the next time drm_syncobj_replace_fence is
>>> - * called.
>>> - */
>>> -struct drm_syncobj_cb {
>>> - struct list_head node;
>>> - drm_syncobj_func_t func;
>>> -};
>>> -
>>> void drm_syncobj_free(struct kref *kref);
>>> /**
More information about the dri-devel
mailing list