Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-10-24 Thread Chunming Zhou 

在 2019/10/24 4:21, Sean Paul 写道:
> On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou  wrote:
>>
>> 在 2019/7/30 21:17, Koenig, Christian 写道:
>>> Am 30.07.19 um 15:02 schrieb Chunming Zhou:
 user space needs a flexiable query ability.
 So that umd can get last signaled or submitted point.
 v2:
 add sanitizer checking.
 v3:
 rebase

 Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
 Signed-off-by: Chunming Zhou 
 Cc: Lionel Landwerlin 
 Cc: Christian König 
 Reviewed-by: Lionel Landwerlin 
>>> Reviewed-by: Christian König 
>>>
>>> Lionel is the Intel code using this already public? Or David any chance
>>> that we can get a public amdvlk release using this?
>> In latest public amdvlk, We should be able to see how timeline syncobj
>> is used in it.
>>
> I couldn't find this anywhere, could you please provide a link?

https://github.com/GPUOpen-Drivers/xgl/blob/dev/icd/api/vk_semaphore.cpp

You can check the source here.


Cheers,

-David

>
> Sean
>
>> -David
>>
>>> Christian.
>>>
 ---
 drivers/gpu/drm/drm_syncobj.c | 37 +--
 include/uapi/drm/drm.h|  3 ++-
 2 files changed, 24 insertions(+), 16 deletions(-)

 diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
 index cecff2e447b1..d4432f1513ac 100644
 --- a/drivers/gpu/drm/drm_syncobj.c
 +++ b/drivers/gpu/drm/drm_syncobj.c
 @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
 *dev, void *data,
   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
   return -EOPNOTSUPP;

 -if (args->pad != 0)
 +if (args->flags != 0)
   return -EINVAL;

   if (args->count_handles == 0)
 @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
 void *data,
   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
   return -EOPNOTSUPP;

 -if (args->pad != 0)
 +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
   return -EINVAL;

   if (args->count_handles == 0)
 @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device 
 *dev, void *data,
   fence = drm_syncobj_fence_get(syncobjs[i]);
   chain = to_dma_fence_chain(fence);
   if (chain) {
 -struct dma_fence *iter, *last_signaled = NULL;
 -
 -dma_fence_chain_for_each(iter, fence) {
 -if (iter->context != fence->context) {
 -dma_fence_put(iter);
 -/* It is most likely that timeline has
 - * unorder points. */
 -break;
 +struct dma_fence *iter, *last_signaled =
 +dma_fence_get(fence);
 +
 +if (args->flags &
 +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
 +point = fence->seqno;
 +} else {
 +dma_fence_chain_for_each(iter, fence) {
 +if (iter->context != fence->context) {
 +dma_fence_put(iter);
 +/* It is most likely that 
 timeline has
 +* unorder points. */
 +break;
 +}
 +dma_fence_put(last_signaled);
 +last_signaled = dma_fence_get(iter);
   }
 -dma_fence_put(last_signaled);
 -last_signaled = dma_fence_get(iter);
 +point = dma_fence_is_signaled(last_signaled) ?
 +last_signaled->seqno :
 +
 to_dma_fence_chain(last_signaled)->prev_seqno;
   }
 -point = dma_fence_is_signaled(last_signaled) ?
 -last_signaled->seqno :
 -to_dma_fence_chain(last_signaled)->prev_seqno;
   dma_fence_put(last_signaled);
   } else {
   point = 0;
   }
 +dma_fence_put(fence);
   ret = copy_to_user([i], , sizeof(uint64_t));
   ret = ret ? -EFAULT : 0;
   if (ret)
 diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
 index 661d73f9a919..fd987ce24d9f 100644
 ---

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-10-24 Thread Daniel Vetter 
On Wed, Oct 23, 2019 at 10:22 PM Sean Paul  wrote:
>
> On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou  wrote:
> >
> >
> > 在 2019/7/30 21:17, Koenig, Christian 写道:
> > > Am 30.07.19 um 15:02 schrieb Chunming Zhou:
> > >> user space needs a flexiable query ability.
> > >> So that umd can get last signaled or submitted point.
> > >> v2:
> > >> add sanitizer checking.
> > >> v3:
> > >> rebase
> > >>
> > >> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> > >> Signed-off-by: Chunming Zhou 
> > >> Cc: Lionel Landwerlin 
> > >> Cc: Christian König 
> > >> Reviewed-by: Lionel Landwerlin 
> > > Reviewed-by: Christian König 
> > >
> > > Lionel is the Intel code using this already public? Or David any chance
> > > that we can get a public amdvlk release using this?
> >
> > In latest public amdvlk, We should be able to see how timeline syncobj
> > is used in it.
> >
>
> I couldn't find this anywhere, could you please provide a link?

I thought there was also a radv implementation somewhere ... adding
some of the usual suspects for that.
-Daniel

>
> Sean
>
> >
> > -David
> >
> > >
> > > Christian.
> > >
> > >> ---
> > >>drivers/gpu/drm/drm_syncobj.c | 37 +--
> > >>include/uapi/drm/drm.h|  3 ++-
> > >>2 files changed, 24 insertions(+), 16 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/drm_syncobj.c 
> > >> b/drivers/gpu/drm/drm_syncobj.c
> > >> index cecff2e447b1..d4432f1513ac 100644
> > >> --- a/drivers/gpu/drm/drm_syncobj.c
> > >> +++ b/drivers/gpu/drm/drm_syncobj.c
> > >> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct 
> > >> drm_device *dev, void *data,
> > >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> > >>  return -EOPNOTSUPP;
> > >>
> > >> -if (args->pad != 0)
> > >> +if (args->flags != 0)
> > >>  return -EINVAL;
> > >>
> > >>  if (args->count_handles == 0)
> > >> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device 
> > >> *dev, void *data,
> > >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> > >>  return -EOPNOTSUPP;
> > >>
> > >> -if (args->pad != 0)
> > >> +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> > >>  return -EINVAL;
> > >>
> > >>  if (args->count_handles == 0)
> > >> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device 
> > >> *dev, void *data,
> > >>  fence = drm_syncobj_fence_get(syncobjs[i]);
> > >>  chain = to_dma_fence_chain(fence);
> > >>  if (chain) {
> > >> -struct dma_fence *iter, *last_signaled = NULL;
> > >> -
> > >> -dma_fence_chain_for_each(iter, fence) {
> > >> -if (iter->context != fence->context) {
> > >> -dma_fence_put(iter);
> > >> -/* It is most likely that timeline 
> > >> has
> > >> - * unorder points. */
> > >> -break;
> > >> +struct dma_fence *iter, *last_signaled =
> > >> +dma_fence_get(fence);
> > >> +
> > >> +if (args->flags &
> > >> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> > >> +point = fence->seqno;
> > >> +} else {
> > >> +dma_fence_chain_for_each(iter, fence) {
> > >> +if (iter->context != 
> > >> fence->context) {
> > >> +dma_fence_put(iter);
> > >> +/* It is most likely that 
> > >> timeline has
> > >> +* unorder points. */
> > >> +break;
> > >> +}
> > >> +dma_fence_put(last_signaled);
> > >> +last_signaled = dma_fence_get(iter);
> > >>  }
> > >> -dma_fence_put(last_signaled);
> > >> -last_signaled = dma_fence_get(iter);
> > >> +point = 
> > >> dma_fence_is_signaled(last_signaled) ?
> > >> +last_signaled->seqno :
> > >> +
> > >> to_dma_fence_chain(last_signaled)->prev_seqno;
> > >>  }
> > >> -point = dma_fence_is_signaled(last_signaled) ?
> > >> -last_signaled->seqno :
> > >> -
> > >> to_dma_fence_chain(last_signaled)->prev_seqno;
> > >>  dma_fence_put(last_signaled);
> > >>  } else {
> > >>  point = 0;
> > >>  }
> > >> +dma_fence_put(fence);
> > >>

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-10-23 Thread Sean Paul 
On Tue, Jul 30, 2019 at 9:22 AM Chunming Zhou  wrote:
>
>
> 在 2019/7/30 21:17, Koenig, Christian 写道:
> > Am 30.07.19 um 15:02 schrieb Chunming Zhou:
> >> user space needs a flexiable query ability.
> >> So that umd can get last signaled or submitted point.
> >> v2:
> >> add sanitizer checking.
> >> v3:
> >> rebase
> >>
> >> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> >> Signed-off-by: Chunming Zhou 
> >> Cc: Lionel Landwerlin 
> >> Cc: Christian König 
> >> Reviewed-by: Lionel Landwerlin 
> > Reviewed-by: Christian König 
> >
> > Lionel is the Intel code using this already public? Or David any chance
> > that we can get a public amdvlk release using this?
>
> In latest public amdvlk, We should be able to see how timeline syncobj
> is used in it.
>

I couldn't find this anywhere, could you please provide a link?

Sean

>
> -David
>
> >
> > Christian.
> >
> >> ---
> >>drivers/gpu/drm/drm_syncobj.c | 37 +--
> >>include/uapi/drm/drm.h|  3 ++-
> >>2 files changed, 24 insertions(+), 16 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> >> index cecff2e447b1..d4432f1513ac 100644
> >> --- a/drivers/gpu/drm/drm_syncobj.c
> >> +++ b/drivers/gpu/drm/drm_syncobj.c
> >> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
> >> *dev, void *data,
> >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>  return -EOPNOTSUPP;
> >>
> >> -if (args->pad != 0)
> >> +if (args->flags != 0)
> >>  return -EINVAL;
> >>
> >>  if (args->count_handles == 0)
> >> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> >> void *data,
> >>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>  return -EOPNOTSUPP;
> >>
> >> -if (args->pad != 0)
> >> +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> >>  return -EINVAL;
> >>
> >>  if (args->count_handles == 0)
> >> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device 
> >> *dev, void *data,
> >>  fence = drm_syncobj_fence_get(syncobjs[i]);
> >>  chain = to_dma_fence_chain(fence);
> >>  if (chain) {
> >> -struct dma_fence *iter, *last_signaled = NULL;
> >> -
> >> -dma_fence_chain_for_each(iter, fence) {
> >> -if (iter->context != fence->context) {
> >> -dma_fence_put(iter);
> >> -/* It is most likely that timeline has
> >> - * unorder points. */
> >> -break;
> >> +struct dma_fence *iter, *last_signaled =
> >> +dma_fence_get(fence);
> >> +
> >> +if (args->flags &
> >> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> >> +point = fence->seqno;
> >> +} else {
> >> +dma_fence_chain_for_each(iter, fence) {
> >> +if (iter->context != fence->context) {
> >> +dma_fence_put(iter);
> >> +/* It is most likely that 
> >> timeline has
> >> +* unorder points. */
> >> +break;
> >> +}
> >> +dma_fence_put(last_signaled);
> >> +last_signaled = dma_fence_get(iter);
> >>  }
> >> -dma_fence_put(last_signaled);
> >> -last_signaled = dma_fence_get(iter);
> >> +point = dma_fence_is_signaled(last_signaled) ?
> >> +last_signaled->seqno :
> >> +
> >> to_dma_fence_chain(last_signaled)->prev_seqno;
> >>  }
> >> -point = dma_fence_is_signaled(last_signaled) ?
> >> -last_signaled->seqno :
> >> -to_dma_fence_chain(last_signaled)->prev_seqno;
> >>  dma_fence_put(last_signaled);
> >>  } else {
> >>  point = 0;
> >>  }
> >> +dma_fence_put(fence);
> >>  ret = copy_to_user([i], , sizeof(uint64_t));
> >>  ret = ret ? -EFAULT : 0;
> >>  if (ret)
> >> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> >> index 661d73f9a919..fd987ce24d9f 100644
> >> --- a/include/uapi/drm/drm.h
> >> +++ b/include/uapi/drm/drm.h
> >> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
> >>  __u32 pad;
> >>};
> >>
> >> +#define

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-07-30 Thread Lionel Landwerlin 

On 30/07/2019 16:17, Koenig, Christian wrote:

Am 30.07.19 um 15:02 schrieb Chunming Zhou:

user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.
v3:
rebase

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
Reviewed-by: Lionel Landwerlin 

Reviewed-by: Christian König 

Lionel is the Intel code using this already public? Or David any chance
that we can get a public amdvlk release using this?

Christian.



The Khronos specification for timeline semaphore isn't public yet 
unfortunately, which is blocking the release of code associated with the 
whole timeline stuff.


We also have no use for this feature currently in the Anv driver.


-Lionel





---
   drivers/gpu/drm/drm_syncobj.c | 37 +--
   include/uapi/drm/drm.h|  3 ++-
   2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index cecff2e447b1..d4432f1513ac 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
   
-	if (args->pad != 0)

+   if (args->flags != 0)
return -EINVAL;
   
   	if (args->count_handles == 0)

@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
   
-	if (args->pad != 0)

+   if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
return -EINVAL;
   
   	if (args->count_handles == 0)

@@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
fence = drm_syncobj_fence_get(syncobjs[i]);
chain = to_dma_fence_chain(fence);
if (chain) {
-   struct dma_fence *iter, *last_signaled = NULL;
-
-   dma_fence_chain_for_each(iter, fence) {
-   if (iter->context != fence->context) {
-   dma_fence_put(iter);
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   struct dma_fence *iter, *last_signaled =
+   dma_fence_get(fence);
+
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (iter->context != fence->context) {
+   dma_fence_put(iter);
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
}
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
   };
   
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */

   struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
   };
   
   



___
dri-devel mailing list

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-07-30 Thread Chunming Zhou 

在 2019/7/30 21:17, Koenig, Christian 写道:
> Am 30.07.19 um 15:02 schrieb Chunming Zhou:
>> user space needs a flexiable query ability.
>> So that umd can get last signaled or submitted point.
>> v2:
>> add sanitizer checking.
>> v3:
>> rebase
>>
>> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
>> Signed-off-by: Chunming Zhou 
>> Cc: Lionel Landwerlin 
>> Cc: Christian König 
>> Reviewed-by: Lionel Landwerlin 
> Reviewed-by: Christian König 
>
> Lionel is the Intel code using this already public? Or David any chance
> that we can get a public amdvlk release using this?

In latest public amdvlk, We should be able to see how timeline syncobj 
is used in it.


-David

>
> Christian.
>
>> ---
>>drivers/gpu/drm/drm_syncobj.c | 37 +--
>>include/uapi/drm/drm.h|  3 ++-
>>2 files changed, 24 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index cecff2e447b1..d4432f1513ac 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
>> *dev, void *data,
>>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>  return -EOPNOTSUPP;
>>
>> -if (args->pad != 0)
>> +if (args->flags != 0)
>>  return -EINVAL;
>>
>>  if (args->count_handles == 0)
>> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>> void *data,
>>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>  return -EOPNOTSUPP;
>>
>> -if (args->pad != 0)
>> +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
>>  return -EINVAL;
>>
>>  if (args->count_handles == 0)
>> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>> void *data,
>>  fence = drm_syncobj_fence_get(syncobjs[i]);
>>  chain = to_dma_fence_chain(fence);
>>  if (chain) {
>> -struct dma_fence *iter, *last_signaled = NULL;
>> -
>> -dma_fence_chain_for_each(iter, fence) {
>> -if (iter->context != fence->context) {
>> -dma_fence_put(iter);
>> -/* It is most likely that timeline has
>> - * unorder points. */
>> -break;
>> +struct dma_fence *iter, *last_signaled =
>> +dma_fence_get(fence);
>> +
>> +if (args->flags &
>> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>> +point = fence->seqno;
>> +} else {
>> +dma_fence_chain_for_each(iter, fence) {
>> +if (iter->context != fence->context) {
>> +dma_fence_put(iter);
>> +/* It is most likely that 
>> timeline has
>> +* unorder points. */
>> +break;
>> +}
>> +dma_fence_put(last_signaled);
>> +last_signaled = dma_fence_get(iter);
>>  }
>> -dma_fence_put(last_signaled);
>> -last_signaled = dma_fence_get(iter);
>> +point = dma_fence_is_signaled(last_signaled) ?
>> +last_signaled->seqno :
>> +
>> to_dma_fence_chain(last_signaled)->prev_seqno;
>>  }
>> -point = dma_fence_is_signaled(last_signaled) ?
>> -last_signaled->seqno :
>> -to_dma_fence_chain(last_signaled)->prev_seqno;
>>  dma_fence_put(last_signaled);
>>  } else {
>>  point = 0;
>>  }
>> +dma_fence_put(fence);
>>  ret = copy_to_user([i], , sizeof(uint64_t));
>>  ret = ret ? -EFAULT : 0;
>>  if (ret)
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 661d73f9a919..fd987ce24d9f 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>>  __u32 pad;
>>};
>>
>> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
>> point on timeline syncobj */
>>struct drm_syncobj_timeline_array {
>>  __u64 handles;
>>  __u64 points;
>>  __u32 count_handles;
>> -__u32 pad;
>> +__u32 flags;
>>};
>>
>>
___
dri-devel mailing list

Re: [PATCH] drm/syncobj: extend syncobj query ability v3

2019-07-30 Thread Koenig, Christian 
Am 30.07.19 um 15:02 schrieb Chunming Zhou:
> user space needs a flexiable query ability.
> So that umd can get last signaled or submitted point.
> v2:
> add sanitizer checking.
> v3:
> rebase
>
> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> Signed-off-by: Chunming Zhou 
> Cc: Lionel Landwerlin 
> Cc: Christian König 
> Reviewed-by: Lionel Landwerlin 

Reviewed-by: Christian König 

Lionel is the Intel code using this already public? Or David any chance 
that we can get a public amdvlk release using this?

Christian.

> ---
>   drivers/gpu/drm/drm_syncobj.c | 37 +--
>   include/uapi/drm/drm.h|  3 ++-
>   2 files changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index cecff2e447b1..d4432f1513ac 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
> *dev, void *data,
>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   return -EOPNOTSUPP;
>   
> - if (args->pad != 0)
> + if (args->flags != 0)
>   return -EINVAL;
>   
>   if (args->count_handles == 0)
> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> void *data,
>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   return -EOPNOTSUPP;
>   
> - if (args->pad != 0)
> + if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
>   return -EINVAL;
>   
>   if (args->count_handles == 0)
> @@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> void *data,
>   fence = drm_syncobj_fence_get(syncobjs[i]);
>   chain = to_dma_fence_chain(fence);
>   if (chain) {
> - struct dma_fence *iter, *last_signaled = NULL;
> -
> - dma_fence_chain_for_each(iter, fence) {
> - if (iter->context != fence->context) {
> - dma_fence_put(iter);
> - /* It is most likely that timeline has
> -  * unorder points. */
> - break;
> + struct dma_fence *iter, *last_signaled =
> + dma_fence_get(fence);
> +
> + if (args->flags &
> + DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> + point = fence->seqno;
> + } else {
> + dma_fence_chain_for_each(iter, fence) {
> + if (iter->context != fence->context) {
> + dma_fence_put(iter);
> + /* It is most likely that 
> timeline has
> + * unorder points. */
> + break;
> + }
> + dma_fence_put(last_signaled);
> + last_signaled = dma_fence_get(iter);
>   }
> - dma_fence_put(last_signaled);
> - last_signaled = dma_fence_get(iter);
> + point = dma_fence_is_signaled(last_signaled) ?
> + last_signaled->seqno :
> + 
> to_dma_fence_chain(last_signaled)->prev_seqno;
>   }
> - point = dma_fence_is_signaled(last_signaled) ?
> - last_signaled->seqno :
> - to_dma_fence_chain(last_signaled)->prev_seqno;
>   dma_fence_put(last_signaled);
>   } else {
>   point = 0;
>   }
> + dma_fence_put(fence);
>   ret = copy_to_user([i], , sizeof(uint64_t));
>   ret = ret ? -EFAULT : 0;
>   if (ret)
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 661d73f9a919..fd987ce24d9f 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>   __u32 pad;
>   };
>   
> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
> point on timeline syncobj */
>   struct drm_syncobj_timeline_array {
>   __u64 handles;
>   __u64 points;
>   __u32 count_handles;
> - __u32 pad;
> + __u32 flags;
>   };
>   
>   

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/syncobj: extend syncobj query ability v3

2019-07-30 Thread Chunming Zhou 
user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.
v3:
rebase

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
Reviewed-by: Lionel Landwerlin 
---
 drivers/gpu/drm/drm_syncobj.c | 37 +--
 include/uapi/drm/drm.h|  3 ++-
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index cecff2e447b1..d4432f1513ac 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
+   if (args->flags != 0)
return -EINVAL;
 
if (args->count_handles == 0)
@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
+   if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
return -EINVAL;
 
if (args->count_handles == 0)
@@ -1289,25 +1289,32 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
fence = drm_syncobj_fence_get(syncobjs[i]);
chain = to_dma_fence_chain(fence);
if (chain) {
-   struct dma_fence *iter, *last_signaled = NULL;
-
-   dma_fence_chain_for_each(iter, fence) {
-   if (iter->context != fence->context) {
-   dma_fence_put(iter);
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   struct dma_fence *iter, *last_signaled =
+   dma_fence_get(fence);
+
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (iter->context != fence->context) {
+   dma_fence_put(iter);
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
}
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
 };
 
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
point on timeline syncobj */
 struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
 };
 
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: extend syncobj query ability

2019-07-30 Thread Koenig, Christian 
Am 23.07.19 um 16:08 schrieb Christian König:
> Am 23.07.19 um 16:07 schrieb Zhou, David(ChunMing):
>> 在 2019/7/23 21:58, Koenig, Christian 写道:
>>> Am 23.07.19 um 07:22 schrieb Chunming Zhou:
 user space needs a flexiable query ability.
 So that umd can get last signaled or submitted point.

 Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
 Signed-off-by: Chunming Zhou 
 Cc: Lionel Landwerlin 
 Cc: Christian König 
>>> I've recently found a bug in drm_syncobj_query_ioctl() which I'm going
>>> to commit tomorrow.
>> Yes, I've realized. Loinel has put RB on it.
>>
>>
>>> Apart from that it looks good to me,
>> Thanks,
>>
>>
>>>    but I think we should cleanup a bit
>>> and move the dma_fence_chain_for_each()... into a helper function in
>>> dma-fence-chain.c
>> Do you mind a saperate cleanup patch for that? I don't want to touch
>> kernel directory in this patch, so that we can cherry-pick it easily.
>
> Yeah, works for me as well.

Sorry this got delayed a bit. I've just pushed my fix to drm-misc-next 
and amd-staging-drm-next.

Can you rebase and send this one out again? Going to push it to 
drm-misc-next then.

Christian.

>
> Christian.
>
>>
>>
>> -David
>>
>>> Christian.
>>>
 ---
     drivers/gpu/drm/drm_syncobj.c | 36 
 +--
     include/uapi/drm/drm.h    |  3 ++-
     2 files changed, 20 insertions(+), 19 deletions(-)

 diff --git a/drivers/gpu/drm/drm_syncobj.c 
 b/drivers/gpu/drm/drm_syncobj.c
 index 3d400905100b..f70dedf3ef4f 100644
 --- a/drivers/gpu/drm/drm_syncobj.c
 +++ b/drivers/gpu/drm/drm_syncobj.c
 @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct 
 drm_device *dev, void *data,
     if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
     return -EOPNOTSUPP;
     -    if (args->pad != 0)
 -    return -EINVAL;
 -
     if (args->count_handles == 0)
     return -EINVAL;
     @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct 
 drm_device *dev, void *data,
     if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
     return -EOPNOTSUPP;
     -    if (args->pad != 0)
 -    return -EINVAL;
 -
     if (args->count_handles == 0)
     return -EINVAL;
     @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct 
 drm_device *dev, void *data,
     if (chain) {
     struct dma_fence *iter, *last_signaled = NULL;
     -    dma_fence_chain_for_each(iter, fence) {
 -    if (!iter)
 -    break;
 -    dma_fence_put(last_signaled);
 -    last_signaled = dma_fence_get(iter);
 -    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
 -    /* It is most likely that timeline has
 - * unorder points. */
 -    break;
 +    if (args->flags &
 +    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
 +    point = fence->seqno;
 +    } else {
 +    dma_fence_chain_for_each(iter, fence) {
 +    if (!iter)
 +    break;
 +    dma_fence_put(last_signaled);
 +    last_signaled = dma_fence_get(iter);
 +    if 
 (!to_dma_fence_chain(last_signaled)->prev_seqno)
 +    /* It is most likely that timeline has
 +    * unorder points. */
 +    break;
 +    }
 +    point = dma_fence_is_signaled(last_signaled) ?
 +    last_signaled->seqno :
 + to_dma_fence_chain(last_signaled)->prev_seqno;
     }
 -    point = dma_fence_is_signaled(last_signaled) ?
 -    last_signaled->seqno :
 - to_dma_fence_chain(last_signaled)->prev_seqno;
     dma_fence_put(last_signaled);
     } else {
     point = 0;
     }
 +    dma_fence_put(fence);
     ret = copy_to_user([i], , sizeof(uint64_t));
     ret = ret ? -EFAULT : 0;
     if (ret)
 diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
 index 661d73f9a919..fd987ce24d9f 100644
 --- a/include/uapi/drm/drm.h
 +++ b/include/uapi/drm/drm.h
 @@ -777,11 +777,12 @@ struct drm_syncobj_array {
     __u32 pad;
     };
     +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* 
 last available point on timeline syncobj */
     struct drm_syncobj_timeline_array {
     __u64 handles;
     __u64 points;
     __u32 count_handles;
 -    __u32 pad;
 +    __u32 flags;
     };
>

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-25 Thread Daniel Vetter 
On Thu, Jul 25, 2019 at 5:01 PM Chunming Zhou  wrote:
>
>
> 在 2019/7/25 20:21, Daniel Vetter 写道:
> > On Wed, Jul 24, 2019 at 07:48:23AM +, Koenig, Christian wrote:
> >> Am 24.07.19 um 08:32 schrieb zhoucm1:
> >>>
> >>> On 2019年07月24日 03:20, Lionel Landwerlin wrote:
>  On 23/07/2019 17:21, Chunming Zhou wrote:
> > user space needs a flexiable query ability.
> > So that umd can get last signaled or submitted point.
> > v2:
> > add sanitizer checking.
> >
> > Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> > Signed-off-by: Chunming Zhou 
> > Cc: Lionel Landwerlin 
> > Cc: Christian König 
>  Reviewed-by: Lionel Landwerlin 
> >>> Thanks.
> >>> Which branch should this patch go to?
> >>> Is it OK to push to amd-staging-drm-next?
> >>> Or should it go to drm-misc?
> >>> If drm-misc, I need your help to push it since I have no permission to
> >>> write.
> >> drm-misc-next is probably more appropriated.
> >>
> >> I'm going to take of that, but first let me push my fix now.
> > btw we once discussed to hide the syncobj stuff until the userspace side
> > is public too. Not seeing that merged anywhere. Can you pls take care of
> > this?
> >
> > I'm not against developing new uapi in upstream, that's occasionally just
> > the most reasonable thing to do here. But we are quite badly flaunting the
> > entire "needs open source consumer in userspace" rule here ...
>
> Hi Daniel,
>
> I got your means. The enablement feature flag is already removed from
> drm-misc, so it shouldn't have other effect.

Ah I totally missed the patch from Alex which just reverts it. I
thought the plan was to have a module options that taints the kernel
when you enable this code. That's what we've done for atomic (or any
of the other hidden uapi). With the current approach we're essentially
merging dead code, which doesn't sound like a good idea really. Iirc
there was a patch from Lionel to add that module option, but I didn't
see it land. That's why I asked.

> If only amd driver stuff and only amd uses that, we can flexiblely deal
> with that.
>
> Since syncobj is moved to drm level, we are working here. I think you
> should already see that timeline syncobj is also used by Intel ANV, and
> Lionel is co-working with us. If we hide these stuff, how do we know who
> is working on that? How do we share one same implementation? Where
> should we discuss? everyone implements own's desgin behind?

Uh ... what are you talking about? Nowhere did I propose to hide stuff
and not work together ... how did you come up with that idea? I'm
confused.
-Daniel

>
> -David
>
> > -Daniel
> >
> >> Christian.
> >>
> >>> -David
> > ---
> >drivers/gpu/drm/drm_syncobj.c | 34 --
> >include/uapi/drm/drm.h|  3 ++-
> >2 files changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_syncobj.c
> > b/drivers/gpu/drm/drm_syncobj.c
> > index 3d400905100b..3fc8f66ada68 100644
> > --- a/drivers/gpu/drm/drm_syncobj.c
> > +++ b/drivers/gpu/drm/drm_syncobj.c
> > @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct
> > drm_device *dev, void *data,
> >if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >return -EOPNOTSUPP;
> >-if (args->pad != 0)
> > +if (args->flags != 0)
> >return -EINVAL;
> >  if (args->count_handles == 0)
> > @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device
> > *dev, void *data,
> >if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >return -EOPNOTSUPP;
> >-if (args->pad != 0)
> > +if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> >return -EINVAL;
> >  if (args->count_handles == 0)
> > @@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct
> > drm_device *dev, void *data,
> >if (chain) {
> >struct dma_fence *iter, *last_signaled = NULL;
> >-dma_fence_chain_for_each(iter, fence) {
> > -if (!iter)
> > -break;
> > -dma_fence_put(last_signaled);
> > -last_signaled = dma_fence_get(iter);
> > -if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> > -/* It is most likely that timeline has
> > - * unorder points. */
> > -break;
> > +if (args->flags &
> > +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> > +point = fence->seqno;
> > +} else {
> > +dma_fence_chain_for_each(iter, fence) {
> > +if (!iter)
> > +break;
> > +dma_fence_put(last_signaled);
> > +

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-25 Thread Chunming Zhou 

在 2019/7/25 20:21, Daniel Vetter 写道:
> On Wed, Jul 24, 2019 at 07:48:23AM +, Koenig, Christian wrote:
>> Am 24.07.19 um 08:32 schrieb zhoucm1:
>>>
>>> On 2019年07月24日 03:20, Lionel Landwerlin wrote:
 On 23/07/2019 17:21, Chunming Zhou wrote:
> user space needs a flexiable query ability.
> So that umd can get last signaled or submitted point.
> v2:
> add sanitizer checking.
>
> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> Signed-off-by: Chunming Zhou 
> Cc: Lionel Landwerlin 
> Cc: Christian König 
 Reviewed-by: Lionel Landwerlin 
>>> Thanks.
>>> Which branch should this patch go to?
>>> Is it OK to push to amd-staging-drm-next?
>>> Or should it go to drm-misc?
>>> If drm-misc, I need your help to push it since I have no permission to
>>> write.
>> drm-misc-next is probably more appropriated.
>>
>> I'm going to take of that, but first let me push my fix now.
> btw we once discussed to hide the syncobj stuff until the userspace side
> is public too. Not seeing that merged anywhere. Can you pls take care of
> this?
>
> I'm not against developing new uapi in upstream, that's occasionally just
> the most reasonable thing to do here. But we are quite badly flaunting the
> entire "needs open source consumer in userspace" rule here ...

Hi Daniel,

I got your means. The enablement feature flag is already removed from 
drm-misc, so it shouldn't have other effect.

If only amd driver stuff and only amd uses that, we can flexiblely deal 
with that.

Since syncobj is moved to drm level, we are working here. I think you 
should already see that timeline syncobj is also used by Intel ANV, and 
Lionel is co-working with us. If we hide these stuff, how do we know who 
is working on that? How do we share one same implementation? Where 
should we discuss? everyone implements own's desgin behind?

-David

> -Daniel
>
>> Christian.
>>
>>> -David
> ---
>    drivers/gpu/drm/drm_syncobj.c | 34 --
>    include/uapi/drm/drm.h    |  3 ++-
>    2 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c
> b/drivers/gpu/drm/drm_syncobj.c
> index 3d400905100b..3fc8f66ada68 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct
> drm_device *dev, void *data,
>    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>    return -EOPNOTSUPP;
>    -    if (args->pad != 0)
> +    if (args->flags != 0)
>    return -EINVAL;
>      if (args->count_handles == 0)
> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device
> *dev, void *data,
>    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>    return -EOPNOTSUPP;
>    -    if (args->pad != 0)
> +    if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
>    return -EINVAL;
>      if (args->count_handles == 0)
> @@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct
> drm_device *dev, void *data,
>    if (chain) {
>    struct dma_fence *iter, *last_signaled = NULL;
>    -    dma_fence_chain_for_each(iter, fence) {
> -    if (!iter)
> -    break;
> -    dma_fence_put(last_signaled);
> -    last_signaled = dma_fence_get(iter);
> -    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> -    /* It is most likely that timeline has
> - * unorder points. */
> -    break;
> +    if (args->flags &
> +    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> +    point = fence->seqno;
> +    } else {
> +    dma_fence_chain_for_each(iter, fence) {
> +    if (!iter)
> +    break;
> +    dma_fence_put(last_signaled);
> +    last_signaled = dma_fence_get(iter);
> +    if
> (!to_dma_fence_chain(last_signaled)->prev_seqno)
> +    /* It is most likely that timeline has
> +    * unorder points. */
> +    break;
> +    }
> +    point = dma_fence_is_signaled(last_signaled) ?
> +    last_signaled->seqno :
> + to_dma_fence_chain(last_signaled)->prev_seqno;
>    }
> -    point = dma_fence_is_signaled(last_signaled) ?
> -    last_signaled->seqno :
> - to_dma_fence_chain(last_signaled)->prev_seqno;
>    dma_fence_put(last_signaled);
>    } else {
>    point = 0;
>    }
> +    dma_fence_put(fence);
>   

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-25 Thread Daniel Vetter 
On Wed, Jul 24, 2019 at 07:48:23AM +, Koenig, Christian wrote:
> Am 24.07.19 um 08:32 schrieb zhoucm1:
> >
> >
> > On 2019年07月24日 03:20, Lionel Landwerlin wrote:
> >> On 23/07/2019 17:21, Chunming Zhou wrote:
> >>> user space needs a flexiable query ability.
> >>> So that umd can get last signaled or submitted point.
> >>> v2:
> >>> add sanitizer checking.
> >>>
> >>> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> >>> Signed-off-by: Chunming Zhou 
> >>> Cc: Lionel Landwerlin 
> >>> Cc: Christian König 
> >>
> >> Reviewed-by: Lionel Landwerlin 
> >
> > Thanks.
> > Which branch should this patch go to?
> > Is it OK to push to amd-staging-drm-next?
> > Or should it go to drm-misc?
> > If drm-misc, I need your help to push it since I have no permission to 
> > write.
> 
> drm-misc-next is probably more appropriated.
> 
> I'm going to take of that, but first let me push my fix now.

btw we once discussed to hide the syncobj stuff until the userspace side
is public too. Not seeing that merged anywhere. Can you pls take care of
this?

I'm not against developing new uapi in upstream, that's occasionally just
the most reasonable thing to do here. But we are quite badly flaunting the
entire "needs open source consumer in userspace" rule here ...
-Daniel

> 
> Christian.
> 
> >
> > -David
> >>
> >>> ---
> >>>   drivers/gpu/drm/drm_syncobj.c | 34 --
> >>>   include/uapi/drm/drm.h    |  3 ++-
> >>>   2 files changed, 22 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
> >>> b/drivers/gpu/drm/drm_syncobj.c
> >>> index 3d400905100b..3fc8f66ada68 100644
> >>> --- a/drivers/gpu/drm/drm_syncobj.c
> >>> +++ b/drivers/gpu/drm/drm_syncobj.c
> >>> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct 
> >>> drm_device *dev, void *data,
> >>>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>>   return -EOPNOTSUPP;
> >>>   -    if (args->pad != 0)
> >>> +    if (args->flags != 0)
> >>>   return -EINVAL;
> >>>     if (args->count_handles == 0)
> >>> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device 
> >>> *dev, void *data,
> >>>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
> >>>   return -EOPNOTSUPP;
> >>>   -    if (args->pad != 0)
> >>> +    if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
> >>>   return -EINVAL;
> >>>     if (args->count_handles == 0)
> >>> @@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct 
> >>> drm_device *dev, void *data,
> >>>   if (chain) {
> >>>   struct dma_fence *iter, *last_signaled = NULL;
> >>>   -    dma_fence_chain_for_each(iter, fence) {
> >>> -    if (!iter)
> >>> -    break;
> >>> -    dma_fence_put(last_signaled);
> >>> -    last_signaled = dma_fence_get(iter);
> >>> -    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
> >>> -    /* It is most likely that timeline has
> >>> - * unorder points. */
> >>> -    break;
> >>> +    if (args->flags &
> >>> +    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> >>> +    point = fence->seqno;
> >>> +    } else {
> >>> +    dma_fence_chain_for_each(iter, fence) {
> >>> +    if (!iter)
> >>> +    break;
> >>> +    dma_fence_put(last_signaled);
> >>> +    last_signaled = dma_fence_get(iter);
> >>> +    if 
> >>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
> >>> +    /* It is most likely that timeline has
> >>> +    * unorder points. */
> >>> +    break;
> >>> +    }
> >>> +    point = dma_fence_is_signaled(last_signaled) ?
> >>> +    last_signaled->seqno :
> >>> + to_dma_fence_chain(last_signaled)->prev_seqno;
> >>>   }
> >>> -    point = dma_fence_is_signaled(last_signaled) ?
> >>> -    last_signaled->seqno :
> >>> - to_dma_fence_chain(last_signaled)->prev_seqno;
> >>>   dma_fence_put(last_signaled);
> >>>   } else {
> >>>   point = 0;
> >>>   }
> >>> +    dma_fence_put(fence);
> >>>   ret = copy_to_user([i], , sizeof(uint64_t));
> >>>   ret = ret ? -EFAULT : 0;
> >>>   if (ret)
> >>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> >>> index 661d73f9a919..fd987ce24d9f 100644
> >>> --- a/include/uapi/drm/drm.h
> >>> +++ b/include/uapi/drm/drm.h
> >>> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
> >>>   __u32 pad;
> >>>   };
> >>>   +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last 
> >>> available point on timeline syncobj */
> >>>   struct drm_syncobj_timeline_array {
> >>>   __u64 handles;
> >>>   __u64 points;
> >>>  

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-24 Thread Koenig, Christian 
Am 24.07.19 um 08:32 schrieb zhoucm1:
>
>
> On 2019年07月24日 03:20, Lionel Landwerlin wrote:
>> On 23/07/2019 17:21, Chunming Zhou wrote:
>>> user space needs a flexiable query ability.
>>> So that umd can get last signaled or submitted point.
>>> v2:
>>> add sanitizer checking.
>>>
>>> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
>>> Signed-off-by: Chunming Zhou 
>>> Cc: Lionel Landwerlin 
>>> Cc: Christian König 
>>
>> Reviewed-by: Lionel Landwerlin 
>
> Thanks.
> Which branch should this patch go to?
> Is it OK to push to amd-staging-drm-next?
> Or should it go to drm-misc?
> If drm-misc, I need your help to push it since I have no permission to 
> write.

drm-misc-next is probably more appropriated.

I'm going to take of that, but first let me push my fix now.

Christian.

>
> -David
>>
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 34 --
>>>   include/uapi/drm/drm.h    |  3 ++-
>>>   2 files changed, 22 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 3d400905100b..3fc8f66ada68 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct 
>>> drm_device *dev, void *data,
>>>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>>   return -EOPNOTSUPP;
>>>   -    if (args->pad != 0)
>>> +    if (args->flags != 0)
>>>   return -EINVAL;
>>>     if (args->count_handles == 0)
>>> @@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device 
>>> *dev, void *data,
>>>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>>   return -EOPNOTSUPP;
>>>   -    if (args->pad != 0)
>>> +    if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
>>>   return -EINVAL;
>>>     if (args->count_handles == 0)
>>> @@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct 
>>> drm_device *dev, void *data,
>>>   if (chain) {
>>>   struct dma_fence *iter, *last_signaled = NULL;
>>>   -    dma_fence_chain_for_each(iter, fence) {
>>> -    if (!iter)
>>> -    break;
>>> -    dma_fence_put(last_signaled);
>>> -    last_signaled = dma_fence_get(iter);
>>> -    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
>>> -    /* It is most likely that timeline has
>>> - * unorder points. */
>>> -    break;
>>> +    if (args->flags &
>>> +    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>>> +    point = fence->seqno;
>>> +    } else {
>>> +    dma_fence_chain_for_each(iter, fence) {
>>> +    if (!iter)
>>> +    break;
>>> +    dma_fence_put(last_signaled);
>>> +    last_signaled = dma_fence_get(iter);
>>> +    if 
>>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
>>> +    /* It is most likely that timeline has
>>> +    * unorder points. */
>>> +    break;
>>> +    }
>>> +    point = dma_fence_is_signaled(last_signaled) ?
>>> +    last_signaled->seqno :
>>> + to_dma_fence_chain(last_signaled)->prev_seqno;
>>>   }
>>> -    point = dma_fence_is_signaled(last_signaled) ?
>>> -    last_signaled->seqno :
>>> - to_dma_fence_chain(last_signaled)->prev_seqno;
>>>   dma_fence_put(last_signaled);
>>>   } else {
>>>   point = 0;
>>>   }
>>> +    dma_fence_put(fence);
>>>   ret = copy_to_user([i], , sizeof(uint64_t));
>>>   ret = ret ? -EFAULT : 0;
>>>   if (ret)
>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>>> index 661d73f9a919..fd987ce24d9f 100644
>>> --- a/include/uapi/drm/drm.h
>>> +++ b/include/uapi/drm/drm.h
>>> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>>>   __u32 pad;
>>>   };
>>>   +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last 
>>> available point on timeline syncobj */
>>>   struct drm_syncobj_timeline_array {
>>>   __u64 handles;
>>>   __u64 points;
>>>   __u32 count_handles;
>>> -    __u32 pad;
>>> +    __u32 flags;
>>>   };
>>
>>
>

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-24 Thread zhoucm1 



On 2019年07月24日 03:20, Lionel Landwerlin wrote:

On 23/07/2019 17:21, Chunming Zhou wrote:

user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 


Reviewed-by: Lionel Landwerlin 


Thanks.
Which branch should this patch go to?
Is it OK to push to amd-staging-drm-next?
Or should it go to drm-misc?
If drm-misc, I need your help to push it since I have no permission to 
write.


-David



---
  drivers/gpu/drm/drm_syncobj.c | 34 --
  include/uapi/drm/drm.h    |  3 ++-
  2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c 
b/drivers/gpu/drm/drm_syncobj.c

index 3d400905100b..3fc8f66ada68 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct 
drm_device *dev, void *data,

  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
  return -EOPNOTSUPP;
  -    if (args->pad != 0)
+    if (args->flags != 0)
  return -EINVAL;
    if (args->count_handles == 0)
@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device 
*dev, void *data,

  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
  return -EOPNOTSUPP;
  -    if (args->pad != 0)
+    if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
  return -EINVAL;
    if (args->count_handles == 0)
@@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct drm_device 
*dev, void *data,

  if (chain) {
  struct dma_fence *iter, *last_signaled = NULL;
  -    dma_fence_chain_for_each(iter, fence) {
-    if (!iter)
-    break;
-    dma_fence_put(last_signaled);
-    last_signaled = dma_fence_get(iter);
-    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
-    /* It is most likely that timeline has
- * unorder points. */
-    break;
+    if (args->flags &
+    DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+    point = fence->seqno;
+    } else {
+    dma_fence_chain_for_each(iter, fence) {
+    if (!iter)
+    break;
+    dma_fence_put(last_signaled);
+    last_signaled = dma_fence_get(iter);
+    if (!to_dma_fence_chain(last_signaled)->prev_seqno)
+    /* It is most likely that timeline has
+    * unorder points. */
+    break;
+    }
+    point = dma_fence_is_signaled(last_signaled) ?
+    last_signaled->seqno :
+ to_dma_fence_chain(last_signaled)->prev_seqno;
  }
-    point = dma_fence_is_signaled(last_signaled) ?
-    last_signaled->seqno :
- to_dma_fence_chain(last_signaled)->prev_seqno;
  dma_fence_put(last_signaled);
  } else {
  point = 0;
  }
+    dma_fence_put(fence);
  ret = copy_to_user([i], , sizeof(uint64_t));
  ret = ret ? -EFAULT : 0;
  if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
  __u32 pad;
  };
  +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last 
available point on timeline syncobj */

  struct drm_syncobj_timeline_array {
  __u64 handles;
  __u64 points;
  __u32 count_handles;
-    __u32 pad;
+    __u32 flags;
  };





___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-23 Thread Lionel Landwerlin 

On 23/07/2019 17:21, Chunming Zhou wrote:

user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 


Reviewed-by: Lionel Landwerlin 


---
  drivers/gpu/drm/drm_syncobj.c | 34 --
  include/uapi/drm/drm.h|  3 ++-
  2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 3d400905100b..3fc8f66ada68 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
  
-	if (args->pad != 0)

+   if (args->flags != 0)
return -EINVAL;
  
  	if (args->count_handles == 0)

@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
  
-	if (args->pad != 0)

+   if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
return -EINVAL;
  
  	if (args->count_handles == 0)

@@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
if (chain) {
struct dma_fence *iter, *last_signaled = NULL;
  
-			dma_fence_chain_for_each(iter, fence) {

-   if (!iter)
-   break;
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
-   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (!iter)
+   break;
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
+   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
  };
  
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */

  struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
  };
  
  



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/syncobj: extend syncobj query ability v2

2019-07-23 Thread Chunming Zhou 
user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.
v2:
add sanitizer checking.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
---
 drivers/gpu/drm/drm_syncobj.c | 34 --
 include/uapi/drm/drm.h|  3 ++-
 2 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 3d400905100b..3fc8f66ada68 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,7 +1197,7 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
+   if (args->flags != 0)
return -EINVAL;
 
if (args->count_handles == 0)
@@ -1268,7 +1268,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
+   if (args->flags & ~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED)
return -EINVAL;
 
if (args->count_handles == 0)
@@ -1291,23 +1291,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
if (chain) {
struct dma_fence *iter, *last_signaled = NULL;
 
-   dma_fence_chain_for_each(iter, fence) {
-   if (!iter)
-   break;
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
-   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (!iter)
+   break;
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
+   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
 };
 
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
point on timeline syncobj */
 struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
 };
 
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: extend syncobj query ability

2019-07-23 Thread Koenig, Christian 
Am 23.07.19 um 16:07 schrieb Zhou, David(ChunMing):
> 在 2019/7/23 21:58, Koenig, Christian 写道:
>> Am 23.07.19 um 07:22 schrieb Chunming Zhou:
>>> user space needs a flexiable query ability.
>>> So that umd can get last signaled or submitted point.
>>>
>>> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
>>> Signed-off-by: Chunming Zhou 
>>> Cc: Lionel Landwerlin 
>>> Cc: Christian König 
>> I've recently found a bug in drm_syncobj_query_ioctl() which I'm going
>> to commit tomorrow.
> Yes, I've realized. Loinel has put RB on it.
>
>
>> Apart from that it looks good to me,
> Thanks,
>
>
>>but I think we should cleanup a bit
>> and move the dma_fence_chain_for_each()... into a helper function in
>> dma-fence-chain.c
> Do you mind a saperate cleanup patch for that? I don't want to touch
> kernel directory in this patch, so that we can cherry-pick it easily.

Yeah, works for me as well.

Christian.

>
>
> -David
>
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/drm_syncobj.c | 36 +--
>>> include/uapi/drm/drm.h|  3 ++-
>>> 2 files changed, 20 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>>> index 3d400905100b..f70dedf3ef4f 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
>>> *dev, void *data,
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>> return -EOPNOTSUPP;
>>> 
>>> -   if (args->pad != 0)
>>> -   return -EINVAL;
>>> -
>>> if (args->count_handles == 0)
>>> return -EINVAL;
>>> 
>>> @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>>> void *data,
>>> if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>> return -EOPNOTSUPP;
>>> 
>>> -   if (args->pad != 0)
>>> -   return -EINVAL;
>>> -
>>> if (args->count_handles == 0)
>>> return -EINVAL;
>>> 
>>> @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>>> void *data,
>>> if (chain) {
>>> struct dma_fence *iter, *last_signaled = NULL;
>>> 
>>> -   dma_fence_chain_for_each(iter, fence) {
>>> -   if (!iter)
>>> -   break;
>>> -   dma_fence_put(last_signaled);
>>> -   last_signaled = dma_fence_get(iter);
>>> -   if 
>>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
>>> -   /* It is most likely that timeline has
>>> -* unorder points. */
>>> -   break;
>>> +   if (args->flags &
>>> +   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>>> +   point = fence->seqno;
>>> +   } else {
>>> +   dma_fence_chain_for_each(iter, fence) {
>>> +   if (!iter)
>>> +   break;
>>> +   dma_fence_put(last_signaled);
>>> +   last_signaled = dma_fence_get(iter);
>>> +   if 
>>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
>>> +   /* It is most likely that 
>>> timeline has
>>> +   * unorder points. */
>>> +   break;
>>> +   }
>>> +   point = dma_fence_is_signaled(last_signaled) ?
>>> +   last_signaled->seqno :
>>> +   
>>> to_dma_fence_chain(last_signaled)->prev_seqno;
>>> }
>>> -   point = dma_fence_is_signaled(last_signaled) ?
>>> -   last_signaled->seqno :
>>> -   to_dma_fence_chain(last_signaled)->prev_seqno;
>>> dma_fence_put(last_signaled);
>>> } else {
>>> point = 0;
>>> }
>>> +   dma_fence_put(fence);
>>> ret = copy_to_user([i], , 
>>> sizeof(uint64_t));
>>> ret = ret ? -EFAULT : 0;
>>> if (ret)
>>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>>> index 661d73f9a919..fd987ce24d9f 100644
>>> --- a/include/uapi/drm/drm.h
>>> +++ b/include/uapi/drm/drm.h
>>> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>>> __u32 pad;
>>> };
>>> 
>>> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
>>> point on timeline syncobj */

Re: [PATCH] drm/syncobj: extend syncobj query ability

2019-07-23 Thread Chunming Zhou 

在 2019/7/23 21:58, Koenig, Christian 写道:
> Am 23.07.19 um 07:22 schrieb Chunming Zhou:
>> user space needs a flexiable query ability.
>> So that umd can get last signaled or submitted point.
>>
>> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
>> Signed-off-by: Chunming Zhou 
>> Cc: Lionel Landwerlin 
>> Cc: Christian König 
> I've recently found a bug in drm_syncobj_query_ioctl() which I'm going
> to commit tomorrow.

Yes, I've realized. Loinel has put RB on it.


>
> Apart from that it looks good to me,

Thanks,


>   but I think we should cleanup a bit
> and move the dma_fence_chain_for_each()... into a helper function in
> dma-fence-chain.c

Do you mind a saperate cleanup patch for that? I don't want to touch 
kernel directory in this patch, so that we can cherry-pick it easily.


-David

>
> Christian.
>
>> ---
>>drivers/gpu/drm/drm_syncobj.c | 36 +--
>>include/uapi/drm/drm.h|  3 ++-
>>2 files changed, 20 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
>> index 3d400905100b..f70dedf3ef4f 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
>> *dev, void *data,
>>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>  return -EOPNOTSUPP;
>>
>> -if (args->pad != 0)
>> -return -EINVAL;
>> -
>>  if (args->count_handles == 0)
>>  return -EINVAL;
>>
>> @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>> void *data,
>>  if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>>  return -EOPNOTSUPP;
>>
>> -if (args->pad != 0)
>> -return -EINVAL;
>> -
>>  if (args->count_handles == 0)
>>  return -EINVAL;
>>
>> @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
>> void *data,
>>  if (chain) {
>>  struct dma_fence *iter, *last_signaled = NULL;
>>
>> -dma_fence_chain_for_each(iter, fence) {
>> -if (!iter)
>> -break;
>> -dma_fence_put(last_signaled);
>> -last_signaled = dma_fence_get(iter);
>> -if 
>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
>> -/* It is most likely that timeline has
>> - * unorder points. */
>> -break;
>> +if (args->flags &
>> +DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
>> +point = fence->seqno;
>> +} else {
>> +dma_fence_chain_for_each(iter, fence) {
>> +if (!iter)
>> +break;
>> +dma_fence_put(last_signaled);
>> +last_signaled = dma_fence_get(iter);
>> +if 
>> (!to_dma_fence_chain(last_signaled)->prev_seqno)
>> +/* It is most likely that 
>> timeline has
>> +* unorder points. */
>> +break;
>> +}
>> +point = dma_fence_is_signaled(last_signaled) ?
>> +last_signaled->seqno :
>> +
>> to_dma_fence_chain(last_signaled)->prev_seqno;
>>  }
>> -point = dma_fence_is_signaled(last_signaled) ?
>> -last_signaled->seqno :
>> -to_dma_fence_chain(last_signaled)->prev_seqno;
>>  dma_fence_put(last_signaled);
>>  } else {
>>  point = 0;
>>  }
>> +dma_fence_put(fence);
>>  ret = copy_to_user([i], , sizeof(uint64_t));
>>  ret = ret ? -EFAULT : 0;
>>  if (ret)
>> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
>> index 661d73f9a919..fd987ce24d9f 100644
>> --- a/include/uapi/drm/drm.h
>> +++ b/include/uapi/drm/drm.h
>> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>>  __u32 pad;
>>};
>>
>> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
>> point on timeline syncobj */
>>struct drm_syncobj_timeline_array {
>>  __u64 handles;
>>  __u64 points;
>>  __u32 count_handles;
>> -__u32 pad;
>> +__u32 flags;
>>};
>>
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH] drm/syncobj: extend syncobj query ability

2019-07-23 Thread Lionel Landwerlin 

On 23/07/2019 08:22, Chunming Zhou wrote:

user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
---
  drivers/gpu/drm/drm_syncobj.c | 36 +--
  include/uapi/drm/drm.h|  3 ++-
  2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 3d400905100b..f70dedf3ef4f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
  
-	if (args->pad != 0)

-   return -EINVAL;



I think args->flags should still be 0 for this ioctl.



-
if (args->count_handles == 0)
return -EINVAL;
  
@@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,

if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
  
-	if (args->pad != 0)

-   return -EINVAL;



You probably want to verify that (args->flags & 
~DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) == 0.




-
if (args->count_handles == 0)
return -EINVAL;
  
@@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,

if (chain) {
struct dma_fence *iter, *last_signaled = NULL;
  
-			dma_fence_chain_for_each(iter, fence) {

-   if (!iter)
-   break;
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
-   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (!iter)
+   break;
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
+   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
  };
  
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available point on timeline syncobj */

  struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
  };
  
  



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/syncobj: extend syncobj query ability

2019-07-23 Thread Koenig, Christian 
Am 23.07.19 um 07:22 schrieb Chunming Zhou:
> user space needs a flexiable query ability.
> So that umd can get last signaled or submitted point.
>
> Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
> Signed-off-by: Chunming Zhou 
> Cc: Lionel Landwerlin 
> Cc: Christian König 

I've recently found a bug in drm_syncobj_query_ioctl() which I'm going 
to commit tomorrow.

Apart from that it looks good to me, but I think we should cleanup a bit 
and move the dma_fence_chain_for_each()... into a helper function in 
dma-fence-chain.c

Christian.

> ---
>   drivers/gpu/drm/drm_syncobj.c | 36 +--
>   include/uapi/drm/drm.h|  3 ++-
>   2 files changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
> index 3d400905100b..f70dedf3ef4f 100644
> --- a/drivers/gpu/drm/drm_syncobj.c
> +++ b/drivers/gpu/drm/drm_syncobj.c
> @@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
> *dev, void *data,
>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   return -EOPNOTSUPP;
>   
> - if (args->pad != 0)
> - return -EINVAL;
> -
>   if (args->count_handles == 0)
>   return -EINVAL;
>   
> @@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> void *data,
>   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
>   return -EOPNOTSUPP;
>   
> - if (args->pad != 0)
> - return -EINVAL;
> -
>   if (args->count_handles == 0)
>   return -EINVAL;
>   
> @@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
> void *data,
>   if (chain) {
>   struct dma_fence *iter, *last_signaled = NULL;
>   
> - dma_fence_chain_for_each(iter, fence) {
> - if (!iter)
> - break;
> - dma_fence_put(last_signaled);
> - last_signaled = dma_fence_get(iter);
> - if 
> (!to_dma_fence_chain(last_signaled)->prev_seqno)
> - /* It is most likely that timeline has
> -  * unorder points. */
> - break;
> + if (args->flags &
> + DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
> + point = fence->seqno;
> + } else {
> + dma_fence_chain_for_each(iter, fence) {
> + if (!iter)
> + break;
> + dma_fence_put(last_signaled);
> + last_signaled = dma_fence_get(iter);
> + if 
> (!to_dma_fence_chain(last_signaled)->prev_seqno)
> + /* It is most likely that 
> timeline has
> + * unorder points. */
> + break;
> + }
> + point = dma_fence_is_signaled(last_signaled) ?
> + last_signaled->seqno :
> + 
> to_dma_fence_chain(last_signaled)->prev_seqno;
>   }
> - point = dma_fence_is_signaled(last_signaled) ?
> - last_signaled->seqno :
> - to_dma_fence_chain(last_signaled)->prev_seqno;
>   dma_fence_put(last_signaled);
>   } else {
>   point = 0;
>   }
> + dma_fence_put(fence);
>   ret = copy_to_user([i], , sizeof(uint64_t));
>   ret = ret ? -EFAULT : 0;
>   if (ret)
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 661d73f9a919..fd987ce24d9f 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -777,11 +777,12 @@ struct drm_syncobj_array {
>   __u32 pad;
>   };
>   
> +#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
> point on timeline syncobj */
>   struct drm_syncobj_timeline_array {
>   __u64 handles;
>   __u64 points;
>   __u32 count_handles;
> - __u32 pad;
> + __u32 flags;
>   };
>   
>   

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/syncobj: extend syncobj query ability

2019-07-22 Thread Chunming Zhou 
user space needs a flexiable query ability.
So that umd can get last signaled or submitted point.

Change-Id: I6512b430524ebabe715e602a2bf5abb0a7e780ea
Signed-off-by: Chunming Zhou 
Cc: Lionel Landwerlin 
Cc: Christian König 
---
 drivers/gpu/drm/drm_syncobj.c | 36 +--
 include/uapi/drm/drm.h|  3 ++-
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 3d400905100b..f70dedf3ef4f 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1197,9 +1197,6 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, 
void *data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
-   return -EINVAL;
-
if (args->count_handles == 0)
return -EINVAL;
 
@@ -1268,9 +1265,6 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void 
*data,
if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
return -EOPNOTSUPP;
 
-   if (args->pad != 0)
-   return -EINVAL;
-
if (args->count_handles == 0)
return -EINVAL;
 
@@ -1291,23 +1285,29 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, 
void *data,
if (chain) {
struct dma_fence *iter, *last_signaled = NULL;
 
-   dma_fence_chain_for_each(iter, fence) {
-   if (!iter)
-   break;
-   dma_fence_put(last_signaled);
-   last_signaled = dma_fence_get(iter);
-   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
-   /* It is most likely that timeline has
-* unorder points. */
-   break;
+   if (args->flags &
+   DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED) {
+   point = fence->seqno;
+   } else {
+   dma_fence_chain_for_each(iter, fence) {
+   if (!iter)
+   break;
+   dma_fence_put(last_signaled);
+   last_signaled = dma_fence_get(iter);
+   if 
(!to_dma_fence_chain(last_signaled)->prev_seqno)
+   /* It is most likely that 
timeline has
+   * unorder points. */
+   break;
+   }
+   point = dma_fence_is_signaled(last_signaled) ?
+   last_signaled->seqno :
+   
to_dma_fence_chain(last_signaled)->prev_seqno;
}
-   point = dma_fence_is_signaled(last_signaled) ?
-   last_signaled->seqno :
-   to_dma_fence_chain(last_signaled)->prev_seqno;
dma_fence_put(last_signaled);
} else {
point = 0;
}
+   dma_fence_put(fence);
ret = copy_to_user([i], , sizeof(uint64_t));
ret = ret ? -EFAULT : 0;
if (ret)
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 661d73f9a919..fd987ce24d9f 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -777,11 +777,12 @@ struct drm_syncobj_array {
__u32 pad;
 };
 
+#define DRM_SYNCOBJ_QUERY_FLAGS_LAST_SUBMITTED (1 << 0) /* last available 
point on timeline syncobj */
 struct drm_syncobj_timeline_array {
__u64 handles;
__u64 points;
__u32 count_handles;
-   __u32 pad;
+   __u32 flags;
 };
 
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel