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 :-(
-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.koe...@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);
> > /**
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
