Re: [PATCH 04/10] drm/syncobj: remove drm_syncobj_cb and cleanup
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
---
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
+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(>node, >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(>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(>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(>node, >cb_list);
spin_unlock(>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(>lock);
- drm_syncobj_add_callback_locked(syncobj, cb, func);
- spin_unlock(>lock);
-}
+ if (!wait->node.next)
+ return;
-void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
-struct drm_syncobj_cb *cb)
-{
spin_lock(>lock);
- list_del_init(>node);
+ list_del_init(>node);
spin_unlock(>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, >cb_list, node) {
list_del_init(>node);
- cur->func(syncobj, cur);
+ syncobj_wait_syncobj_func(syncobj, cur);
}
}
@@ -608,13 +593,6 @@
Re: [PATCH 04/10] drm/syncobj: remove drm_syncobj_cb and cleanup
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
> > ---
> > 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
> > +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(>node, >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(>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(>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(>node, >cb_list);
> > spin_unlock(>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(>lock);
> > - drm_syncobj_add_callback_locked(syncobj, cb, func);
> > - spin_unlock(>lock);
> > -}
> > + if (!wait->node.next)
> > + return;
> > -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
> > -struct drm_syncobj_cb *cb)
> > -{
> > spin_lock(>lock);
> > - list_del_init(>node);
> > + list_del_init(>node);
> > spin_unlock(>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, >cb_list, node) {
> > list_del_init(>node);
> > - cur->func(syncobj, cur);
> > +
Re: [PATCH 04/10] drm/syncobj: remove drm_syncobj_cb and cleanup
在 2018/12/4 19:59, 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
Reviewed-by: Chunming Zhou
> ---
> 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
>
> +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(>node, >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(>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(>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(>node, >cb_list);
> spin_unlock(>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(>lock);
> - drm_syncobj_add_callback_locked(syncobj, cb, func);
> - spin_unlock(>lock);
> -}
> + if (!wait->node.next)
> + return;
>
> -void drm_syncobj_remove_callback(struct drm_syncobj *syncobj,
> - struct drm_syncobj_cb *cb)
> -{
> spin_lock(>lock);
> - list_del_init(>node);
> + list_del_init(>node);
> spin_unlock(>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, >cb_list, node) {
> list_del_init(>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,
> >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,
>
