[PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-10-09 Thread Chunming Zhou 
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.
v3:
userspace can specify two kinds waits::
a. Wait for time point to be completed.
b. and wait for time point to become available

Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/drm_internal.h |   2 +
 drivers/gpu/drm/drm_ioctl.c|   2 +
 drivers/gpu/drm/drm_syncobj.c  | 118 -
 include/uapi/drm/drm.h |  18 +
 4 files changed, 124 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
 int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
 int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
 int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, 
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..f3f11ac2ef28 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
 }
 
 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
int ret;
 
-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
 
@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -354,13 +355,17 @@ void drm_syncobj_replace_fence(struct drm_syncobj 
*syncobj,
drm_syncobj_create_signal_pt(syncobj, fence, pt_value);
if (fence) {
struct drm_syncobj_cb *cur, *tmp;
+   struct list_head cb_list;
+
+   INIT_LIST_HEAD(_list);
 
spin_lock(>lock);
-   list_for_each_entry_safe(cur, tmp, >cb_list, node) {
+   list_splice_init(>cb_list, _list);
+   spin_unlock(>lock);
+   list_for_each_entry_safe(cur, tmp, _list, node) {
list_del_init(>node);
cur->func(syncobj, cur);
}
-   spin_unlock(>lock);
}
 }
 EXPORT_SYMBOL(drm_syncobj_replace_fence);
@@ -856,6 +861,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+   u64point;
 };
 
 static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -873,12 +879,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
 
-   drm_syncobj_search_fence(syncobj, 0, 0, >fence);
+   drm_syncobj_search_fence(syncobj, wait->point, 0, >fence);
 
wake_up_process(wait->task);
 }
 
 static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj 
**syncobjs,
+ void __user *user_points,
  uint32_t count,

Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-10-08 Thread Jason Ekstrand 
On Mon, Oct 8, 2018 at 12:53 AM Zhou, David(ChunMing) 
wrote:

> >> Another general comment (no good place to put it) is that I think we
> want two kinds of waits:  Wait for time point to be completed and wait for
> time point to become available.  The first is the usual CPU wait for
> completion while the second is for use by userspace drivers to wait until
> the first moment where they can submit work which depends on a given time
> point.
>
>
>
> Hi Jason,
>
>
>
> How about adding two new wait flags?
>
> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_COMPLETED
>
> DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE
>

Those seem like fine names to me.  We should require that one of the two
flags be present when the sync object is a timeline.

--Jason


> Thanks,
>
> David
>
>
>
> *From:* Christian König 
> *Sent:* Tuesday, September 25, 2018 5:50 PM
> *To:* Jason Ekstrand ; Zhou, David(ChunMing) <
> david1.z...@amd.com>
> *Cc:* amd-gfx mailing list ; Maling list -
> DRI developers 
> *Subject:* Re: [PATCH 3/6] drm: add support of syncobj timeline point
> wait v2
>
>
>
> Am 25.09.2018 um 11:22 schrieb Jason Ekstrand:
>
> On Thu, Sep 20, 2018 at 6:04 AM Chunming Zhou  wrote:
>
> points array is one-to-one match with syncobjs array.
> v2:
> add seperate ioctl for timeline point wait, otherwise break uapi.
>
>
>
> I think ioctl structs can be extended as long as fields aren't
> re-ordered.  I'm not sure on the details of this though as I'm not a
> particularly experienced kernel developer.
>
>
> Yeah, that is correct. The problem in this particular case is that we
> don't change the direct IOCTL parameter, but rather the array it points to.
>
> We could do something like keep the existing handles array and add a
> separate optional one for the timeline points. That would also drop the
> need for the padding of the structure.
>
>
> Another general comment (no good place to put it) is that I think we want
> two kinds of waits:  Wait for time point to be completed and wait for time
> point to become available.  The first is the usual CPU wait for completion
> while the second is for use by userspace drivers to wait until the first
> moment where they can submit work which depends on a given time point.
>
>
> Oh, yeah that is a really good point as ell.
>
> Christian.
>
>
>
>
> Signed-off-by: Chunming Zhou 
> ---
>  drivers/gpu/drm/drm_internal.h |  2 +
>  drivers/gpu/drm/drm_ioctl.c|  2 +
>  drivers/gpu/drm/drm_syncobj.c  | 99 +-
>  include/uapi/drm/drm.h | 14 +
>  4 files changed, 103 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_internal.h
> b/drivers/gpu/drm/drm_internal.h
> index 0c4eb4a9ab31..566d44e3c782 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device
> *dev, void *data,
>struct drm_file *file_private);
>  int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>struct drm_file *file_private);
> +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
> +   struct drm_file *file_private);
>  int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private);
>  int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 6b4a633b4240..c0891614f516 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
> drm_syncobj_timeline_wait_ioctl,
> + DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 67472bd77c83..a43de0e4616c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct
> drm_syncobj *syncobj,
>  }
>
>  static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj
> *syncobj,
> +

RE: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-10-07 Thread Zhou, David(ChunMing) 
>> Another general comment (no good place to put it) is that I think we want 
>> two kinds of waits:  Wait for time point to be completed and wait for time 
>> point to become available.  The first is the usual CPU wait for completion 
>> while the second is for use by userspace drivers to wait until the first 
>> moment where they can submit work which depends on a given time point.

Hi Jason,

How about adding two new wait flags?
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_COMPLETED
DRM_SYNCOBJ_WAIT_FLAGS_WAIT_AVAILABLE

Thanks,
David

From: Christian König 
Sent: Tuesday, September 25, 2018 5:50 PM
To: Jason Ekstrand ; Zhou, David(ChunMing) 

Cc: amd-gfx mailing list ; Maling list - DRI 
developers 
Subject: Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

Am 25.09.2018 um 11:22 schrieb Jason Ekstrand:
On Thu, Sep 20, 2018 at 6:04 AM Chunming Zhou 
mailto:david1.z...@amd.com>> wrote:
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

I think ioctl structs can be extended as long as fields aren't re-ordered.  I'm 
not sure on the details of this though as I'm not a particularly experienced 
kernel developer.

Yeah, that is correct. The problem in this particular case is that we don't 
change the direct IOCTL parameter, but rather the array it points to.

We could do something like keep the existing handles array and add a separate 
optional one for the timeline points. That would also drop the need for the 
padding of the structure.


Another general comment (no good place to put it) is that I think we want two 
kinds of waits:  Wait for time point to be completed and wait for time point to 
become available.  The first is the usual CPU wait for completion while the 
second is for use by userspace drivers to wait until the first moment where 
they can submit work which depends on a given time point.

Oh, yeah that is a really good point as ell.

Christian.



Signed-off-by: Chunming Zhou mailto:david1.z...@amd.com>>
---
 drivers/gpu/drm/drm_internal.h |  2 +
 drivers/gpu/drm/drm_ioctl.c|  2 +
 drivers/gpu/drm/drm_syncobj.c  | 99 +-
 include/uapi/drm/drm.h | 14 +
 4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
 int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
 int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
 int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, 
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..a43de0e4616c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
 }

 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
int ret;

-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;

@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0,

Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-25 Thread Christian König 

Am 25.09.2018 um 11:22 schrieb Jason Ekstrand:
On Thu, Sep 20, 2018 at 6:04 AM Chunming Zhou > wrote:


points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.


I think ioctl structs can be extended as long as fields aren't 
re-ordered.  I'm not sure on the details of this though as I'm not a 
particularly experienced kernel developer.


Yeah, that is correct. The problem in this particular case is that we 
don't change the direct IOCTL parameter, but rather the array it points to.


We could do something like keep the existing handles array and add a 
separate optional one for the timeline points. That would also drop the 
need for the padding of the structure.


Another general comment (no good place to put it) is that I think we 
want two kinds of waits:  Wait for time point to be completed and wait 
for time point to become available. The first is the usual CPU wait 
for completion while the second is for use by userspace drivers to 
wait until the first moment where they can submit work which depends 
on a given time point.


Oh, yeah that is a really good point as ell.

Christian.


Signed-off-by: Chunming Zhou mailto:david1.z...@amd.com>>
---
 drivers/gpu/drm/drm_internal.h |  2 +
 drivers/gpu/drm/drm_ioctl.c    |  2 +
 drivers/gpu/drm/drm_syncobj.c  | 99
+-
 include/uapi/drm/drm.h         | 14 +
 4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct
drm_device *dev, void *data,
                                   struct drm_file *file_private);
 int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
                           struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void
*data,
+                                   struct drm_file *file_private);
 int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
                            struct drm_file *file_private);
 int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc
drm_ioctls[] = {
                      DRM_UNLOCKED|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
                      DRM_UNLOCKED|DRM_RENDER_ALLOW),
+       DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
drm_syncobj_timeline_wait_ioctl,
+                     DRM_UNLOCKED|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET,
drm_syncobj_reset_ioctl,
                      DRM_UNLOCKED|DRM_RENDER_ALLOW),
        DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL,
drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..a43de0e4616c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void
drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
 }

 static int drm_syncobj_fence_get_or_add_callback(struct
drm_syncobj *syncobj,
+                                                u64 point,
                                                 struct dma_fence
**fence,
                                                 struct
drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
        int ret;

-       ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+       ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
        if (!ret)
                return 1;

@@ -143,7 +144,7 @@ static int
drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
         */
        if (!list_empty(>signal_pt_list)) {
                spin_unlock(>lock);
-               drm_syncobj_search_fence(syncobj, 0, 0, fence);
+               drm_syncobj_search_fence(syncobj, point, 0, fence);
                if (*fence)
                        return 1;
                spin_lock(>lock);
@@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct
drm_syncobj *syncobj,
                spin_lock(>lock);
                list_for_each_entry_safe(cur, tmp,
>cb_list, node) {
                        list_del_init(>node);
+                       spin_unlock(>lock);
                        cur->func(syncobj, cur);
+                       

Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-25 Thread Jason Ekstrand 
On Thu, Sep 20, 2018 at 6:04 AM Chunming Zhou  wrote:

> points array is one-to-one match with syncobjs array.
> v2:
> add seperate ioctl for timeline point wait, otherwise break uapi.
>

I think ioctl structs can be extended as long as fields aren't re-ordered.
I'm not sure on the details of this though as I'm not a particularly
experienced kernel developer.

Another general comment (no good place to put it) is that I think we want
two kinds of waits:  Wait for time point to be completed and wait for time
point to become available.  The first is the usual CPU wait for completion
while the second is for use by userspace drivers to wait until the first
moment where they can submit work which depends on a given time point.


> Signed-off-by: Chunming Zhou 
> ---
>  drivers/gpu/drm/drm_internal.h |  2 +
>  drivers/gpu/drm/drm_ioctl.c|  2 +
>  drivers/gpu/drm/drm_syncobj.c  | 99 +-
>  include/uapi/drm/drm.h | 14 +
>  4 files changed, 103 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_internal.h
> b/drivers/gpu/drm/drm_internal.h
> index 0c4eb4a9ab31..566d44e3c782 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device
> *dev, void *data,
>struct drm_file *file_private);
>  int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
>struct drm_file *file_private);
> +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
> +   struct drm_file *file_private);
>  int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
> struct drm_file *file_private);
>  int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 6b4a633b4240..c0891614f516 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
> drm_syncobj_timeline_wait_ioctl,
> + DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
>   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 67472bd77c83..a43de0e4616c 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct
> drm_syncobj *syncobj,
>  }
>
>  static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj
> *syncobj,
> +u64 point,
>  struct dma_fence **fence,
>  struct drm_syncobj_cb *cb,
>  drm_syncobj_func_t func)
>  {
> int ret;
>
> -   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
> +   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
> if (!ret)
> return 1;
>
> @@ -143,7 +144,7 @@ static int
> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
>  */
> if (!list_empty(>signal_pt_list)) {
> spin_unlock(>lock);
> -   drm_syncobj_search_fence(syncobj, 0, 0, fence);
> +   drm_syncobj_search_fence(syncobj, point, 0, fence);
> if (*fence)
> return 1;
> spin_lock(>lock);
> @@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj
> *syncobj,
> spin_lock(>lock);
> list_for_each_entry_safe(cur, tmp, >cb_list,
> node) {
> list_del_init(>node);
> +   spin_unlock(>lock);
> cur->func(syncobj, cur);
> +   spin_lock(>lock);
> }
> spin_unlock(>lock);
> }
> @@ -856,6 +859,7 @@ struct syncobj_wait_entry {
> struct dma_fence *fence;
> struct dma_fence_cb fence_cb;
> struct drm_syncobj_cb syncobj_cb;
> +   u64point;
>  };
>
>  static void syncobj_wait_fence_func(struct dma_fence *fence,
> @@ -873,12 +877,13 @@ static void syncobj_wait_syncobj_func(struct
> drm_syncobj *syncobj,
> struct syncobj_wait_entry *wait =
> container_of(cb, struct syncobj_wait_entry, syncobj_cb);
>
> -   drm_syncobj_search_fence(syncobj, 0, 0, >fence);
> +

Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-21 Thread Christian König 

Am 21.09.2018 um 09:15 schrieb Zhou, David(ChunMing):



-Original Message-
From: amd-gfx  On Behalf Of
Christian K?nig
Sent: Thursday, September 20, 2018 7:11 PM
To: Zhou, David(ChunMing) ; dri-
de...@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

Am 20.09.2018 um 13:03 schrieb Chunming Zhou:

points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

Signed-off-by: Chunming Zhou 
---
   drivers/gpu/drm/drm_internal.h |  2 +
   drivers/gpu/drm/drm_ioctl.c|  2 +
   drivers/gpu/drm/drm_syncobj.c  | 99

+-

   include/uapi/drm/drm.h | 14 +
   4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h index 0c4eb4a9ab31..566d44e3c782
100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct

drm_device *dev, void *data,

   struct drm_file *file_private);
   int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
   int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
   int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT,

drm_syncobj_wait_ioctl,

  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,

drm_syncobj_timeline_wait_ioctl,

+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET,

drm_syncobj_reset_ioctl,

  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL,

drm_syncobj_signal_ioctl,

diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c index 67472bd77c83..a43de0e4616c
100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void

drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,

   }

   static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj
*syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
   {
int ret;

-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;

@@ -143,7 +144,7 @@ static int

drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct

drm_syncobj *syncobj,

spin_lock(>lock);
list_for_each_entry_safe(cur, tmp, >cb_list, node)

{

list_del_init(>node);
+   spin_unlock(>lock);
cur->func(syncobj, cur);
+   spin_lock(>lock);

That looks fishy to me. Why do we need to unlock

Cb func will call _search_fence, which will need to grab the lock, otherwise 
deadlock.



and who guarantees that
tmp is still valid when we grab the lock again?

Sorry for that, quickly  fix deadlock and forget to care that when debug.
How about splice to a tmp list, and then list_for _xxx without lock?


Yeah, that should work. Alternative is to use something like "while 
(!list_empty()) { e = list_first_entry(); list_del(e)". But either 
way should work.



Any other comment on patch series and libdrm patches?   That one comment 
increases a patch set version seems be overcommit.


On a first glance that stuff looked good, but give me till monday for 
that. I'm currently in the middle of debugging issues.


Thanks,
Christian.



Thanks,
David Zhou


Apart from that can't see anything obvious wrong, but I certainly n

RE: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-21 Thread Zhou, David(ChunMing) 


> -Original Message-
> From: amd-gfx  On Behalf Of
> Christian K?nig
> Sent: Thursday, September 20, 2018 7:11 PM
> To: Zhou, David(ChunMing) ; dri-
> de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Subject: Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2
> 
> Am 20.09.2018 um 13:03 schrieb Chunming Zhou:
> > points array is one-to-one match with syncobjs array.
> > v2:
> > add seperate ioctl for timeline point wait, otherwise break uapi.
> >
> > Signed-off-by: Chunming Zhou 
> > ---
> >   drivers/gpu/drm/drm_internal.h |  2 +
> >   drivers/gpu/drm/drm_ioctl.c|  2 +
> >   drivers/gpu/drm/drm_syncobj.c  | 99
> +-
> >   include/uapi/drm/drm.h | 14 +
> >   4 files changed, 103 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_internal.h
> > b/drivers/gpu/drm/drm_internal.h index 0c4eb4a9ab31..566d44e3c782
> > 100644
> > --- a/drivers/gpu/drm/drm_internal.h
> > +++ b/drivers/gpu/drm/drm_internal.h
> > @@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct
> drm_device *dev, void *data,
> >struct drm_file *file_private);
> >   int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
> >struct drm_file *file_private);
> > +int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
> > +   struct drm_file *file_private);
> >   int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
> > struct drm_file *file_private);
> >   int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 6b4a633b4240..c0891614f516 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
> >   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT,
> drm_syncobj_wait_ioctl,
> >   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > +   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT,
> drm_syncobj_timeline_wait_ioctl,
> > + DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET,
> drm_syncobj_reset_ioctl,
> >   DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL,
> drm_syncobj_signal_ioctl,
> > diff --git a/drivers/gpu/drm/drm_syncobj.c
> > b/drivers/gpu/drm/drm_syncobj.c index 67472bd77c83..a43de0e4616c
> > 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -126,13 +126,14 @@ static void
> drm_syncobj_add_callback_locked(struct drm_syncobj *syncobj,
> >   }
> >
> >   static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj
> > *syncobj,
> > +u64 point,
> >  struct dma_fence **fence,
> >  struct drm_syncobj_cb *cb,
> >  drm_syncobj_func_t func)
> >   {
> > int ret;
> >
> > -   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
> > +   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
> > if (!ret)
> > return 1;
> >
> > @@ -143,7 +144,7 @@ static int
> drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
> >  */
> > if (!list_empty(>signal_pt_list)) {
> > spin_unlock(>lock);
> > -   drm_syncobj_search_fence(syncobj, 0, 0, fence);
> > +   drm_syncobj_search_fence(syncobj, point, 0, fence);
> > if (*fence)
> > return 1;
> > spin_lock(>lock);
> > @@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct
> drm_syncobj *syncobj,
> > spin_lock(>lock);
> > list_for_each_entry_safe(cur, tmp, >cb_list, node)
> {
> > list_del_init(>node);
> > +   spin_unlock(>lock);
> > cur->func(syncobj, cur);
> > +   spin_lock(>lock);
> 
> That looks fishy to me. Why do we need to unlock 

Cb func will call _search_fence, which will need to grab the lock, otherwise 
deadlock.


>and who guarantees that
> tmp is still valid when we grab the lock again?

Sorry for that, quickly  fix deadlock and forget to

Re: [PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-20 Thread Christian König 

Am 20.09.2018 um 13:03 schrieb Chunming Zhou:

points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

Signed-off-by: Chunming Zhou 
---
  drivers/gpu/drm/drm_internal.h |  2 +
  drivers/gpu/drm/drm_ioctl.c|  2 +
  drivers/gpu/drm/drm_syncobj.c  | 99 +-
  include/uapi/drm/drm.h | 14 +
  4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
  int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
  int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
  int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, 
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..a43de0e4616c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
  }
  
  static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
  {
int ret;
  
-	ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);

+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
  
@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,

 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
spin_lock(>lock);
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
list_del_init(>node);
+   spin_unlock(>lock);
cur->func(syncobj, cur);
+   spin_lock(>lock);


That looks fishy to me. Why do we need to unlock and who guarantees that 
tmp is still valid when we grab the lock again?


Apart from that can't see anything obvious wrong, but I certainly need 
to take a closer look.


Christian.


}
spin_unlock(>lock);
}
@@ -856,6 +859,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+   u64point;
  };
  
  static void syncobj_wait_fence_func(struct dma_fence *fence,

@@ -873,12 +877,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
  
-	drm_syncobj_search_fence(syncobj, 0, 0, >fence);

+   drm_syncobj_search_fence(syncobj, wait->point, 0, >fence);
  
  	wake_up_process(wait->task);

  }
  
  static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj **syncobjs,

+ void __user *user_points,
  uint32_t count,
  uint32_t flags,
  signed long timeout,
@@ -886,13 +891,27 @@ static signed

[PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-20 Thread Chunming Zhou 
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/drm_internal.h |  2 +
 drivers/gpu/drm/drm_ioctl.c|  2 +
 drivers/gpu/drm/drm_syncobj.c  | 99 +-
 include/uapi/drm/drm.h | 14 +
 4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
 int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
 int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
 int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, 
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 67472bd77c83..a43de0e4616c 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
 }
 
 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
int ret;
 
-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
 
@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -358,7 +359,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
spin_lock(>lock);
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
list_del_init(>node);
+   spin_unlock(>lock);
cur->func(syncobj, cur);
+   spin_lock(>lock);
}
spin_unlock(>lock);
}
@@ -856,6 +859,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+   u64point;
 };
 
 static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -873,12 +877,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
 
-   drm_syncobj_search_fence(syncobj, 0, 0, >fence);
+   drm_syncobj_search_fence(syncobj, wait->point, 0, >fence);
 
wake_up_process(wait->task);
 }
 
 static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj 
**syncobjs,
+ void __user *user_points,
  uint32_t count,
  uint32_t flags,
  signed long timeout,
@@ -886,13 +891,27 @@ static signed long drm_syncobj_array_wait_timeout(struct 
drm_syncobj **syncobjs,
 {
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
+   uint64_t *points;
signed long ret;
uint32_t signaled_count, i;
 
-   entries = kcalloc(count, sizeof(*entries),

[PATCH 3/6] drm: add support of syncobj timeline point wait v2

2018-09-19 Thread Chunming Zhou 
points array is one-to-one match with syncobjs array.
v2:
add seperate ioctl for timeline point wait, otherwise break uapi.

Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/drm_internal.h |  2 +
 drivers/gpu/drm/drm_ioctl.c|  2 +
 drivers/gpu/drm/drm_syncobj.c  | 99 +-
 include/uapi/drm/drm.h | 14 +
 4 files changed, 103 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index 0c4eb4a9ab31..566d44e3c782 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -183,6 +183,8 @@ int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, 
void *data,
   struct drm_file *file_private);
 int drm_syncobj_wait_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
+int drm_syncobj_timeline_wait_ioctl(struct drm_device *dev, void *data,
+   struct drm_file *file_private);
 int drm_syncobj_reset_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
 int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 6b4a633b4240..c0891614f516 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -669,6 +669,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_WAIT, drm_syncobj_wait_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_WAIT, 
drm_syncobj_timeline_wait_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_RESET, drm_syncobj_reset_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index d69126b690c2..251d48a18999 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -126,13 +126,14 @@ static void drm_syncobj_add_callback_locked(struct 
drm_syncobj *syncobj,
 }
 
 static int drm_syncobj_fence_get_or_add_callback(struct drm_syncobj *syncobj,
+u64 point,
 struct dma_fence **fence,
 struct drm_syncobj_cb *cb,
 drm_syncobj_func_t func)
 {
int ret;
 
-   ret = drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   ret = drm_syncobj_search_fence(syncobj, point, 0, fence);
if (!ret)
return 1;
 
@@ -143,7 +144,7 @@ static int drm_syncobj_fence_get_or_add_callback(struct 
drm_syncobj *syncobj,
 */
if (!list_empty(>signal_pt_list)) {
spin_unlock(>lock);
-   drm_syncobj_search_fence(syncobj, 0, 0, fence);
+   drm_syncobj_search_fence(syncobj, point, 0, fence);
if (*fence)
return 1;
spin_lock(>lock);
@@ -356,7 +357,9 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
spin_lock(>lock);
list_for_each_entry_safe(cur, tmp, >cb_list, node) {
list_del_init(>node);
+   spin_unlock(>lock);
cur->func(syncobj, cur);
+   spin_lock(>lock);
}
spin_unlock(>lock);
}
@@ -860,6 +863,7 @@ struct syncobj_wait_entry {
struct dma_fence *fence;
struct dma_fence_cb fence_cb;
struct drm_syncobj_cb syncobj_cb;
+   u64point;
 };
 
 static void syncobj_wait_fence_func(struct dma_fence *fence,
@@ -877,12 +881,13 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
struct syncobj_wait_entry *wait =
container_of(cb, struct syncobj_wait_entry, syncobj_cb);
 
-   drm_syncobj_search_fence(syncobj, 0, 0, >fence);
+   drm_syncobj_search_fence(syncobj, wait->point, 0, >fence);
 
wake_up_process(wait->task);
 }
 
 static signed long drm_syncobj_array_wait_timeout(struct drm_syncobj 
**syncobjs,
+ void __user *user_points,
  uint32_t count,
  uint32_t flags,
  signed long timeout,
@@ -890,13 +895,27 @@ static signed long drm_syncobj_array_wait_timeout(struct 
drm_syncobj **syncobjs,
 {
struct syncobj_wait_entry *entries;
struct dma_fence *fence;
+   uint64_t *points;
signed long ret;
uint32_t signaled_count, i;
 
-   entries = kcalloc(count, sizeof(*entries),