[PATCH 1/3] dma-buf: add dma_fence_get_stub

[PATCH 1/3] dma-buf: add dma_fence_get_stub

[Christian König]

Extract of useful code from the timeline work. This provides a function
to return a stub or dummy fence which is always signaled.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
 include/linux/dma-fence.h   |  1 +
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 1551ca7df394..136ec04d683f 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -30,13 +30,16 @@
 EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
 EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
 
+static DEFINE_SPINLOCK(dma_fence_stub_lock);
+static struct dma_fence dma_fence_stub;
+
 /*
  * fence context counter: each execution context should have its own
  * fence context, this allows checking if fences belong to the same
  * context or not. One device can have multiple separate contexts,
  * and they're used if some engine can run independently of another.
  */
-static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
+static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
 
 /**
  * DOC: DMA fences overview
@@ -68,6 +71,37 @@ static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
  *   &dma_buf.resv pointer.
  */
 
+static const char *dma_fence_stub_get_name(struct dma_fence *fence)
+{
+        return "stub";
+}
+
+static const struct dma_fence_ops dma_fence_stub_ops = {
+	.get_driver_name = dma_fence_stub_get_name,
+	.get_timeline_name = dma_fence_stub_get_name,
+};
+
+/**
+ * dma_fence_get_stub - return a signaled fence
+ *
+ * Return a stub fence which is already signaled.
+ */
+struct dma_fence *dma_fence_get_stub(void)
+{
+	spin_lock(&dma_fence_stub_lock);
+	if (!dma_fence_stub.ops) {
+		dma_fence_init(&dma_fence_stub,
+			       &dma_fence_stub_ops,
+			       &dma_fence_stub_lock,
+			       0, 0);
+		dma_fence_signal_locked(&dma_fence_stub);
+	}
+	spin_unlock(&dma_fence_stub_lock);
+
+	return dma_fence_get(&dma_fence_stub);
+}
+EXPORT_SYMBOL(dma_fence_get_stub);
+
 /**
  * dma_fence_context_alloc - allocate an array of fence contexts
  * @num: amount of contexts to allocate
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 02dba8cd033d..999e4b104410 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -541,6 +541,7 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
 	return ret < 0 ? ret : 0;
 }
 
+struct dma_fence *dma_fence_get_stub(void);
 u64 dma_fence_context_alloc(unsigned num);
 
 #define DMA_FENCE_TRACE(f, fmt, args...) \
-- 
2.14.1

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

[PATCH 2/3] drm/syncobj: use dma_fence_get_stub

[Christian König]

Extract of useful code from the timeline work. Let's use just a single
stub fence instance instead of allocating a new one all the time.

Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_syncobj.c | 58 +++++++++++--------------------------------
 1 file changed, 14 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index e2c5b3ca4824..5c5ba1f14307 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -56,22 +56,6 @@
 #include "drm_internal.h"
 #include <drm/drm_syncobj.h>
 
-struct drm_syncobj_stub_fence {
-	struct dma_fence base;
-	spinlock_t lock;
-};
-
-static const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence)
-{
-        return "syncobjstub";
-}
-
-static const struct dma_fence_ops drm_syncobj_stub_fence_ops = {
-	.get_driver_name = drm_syncobj_stub_fence_get_name,
-	.get_timeline_name = drm_syncobj_stub_fence_get_name,
-};
-
-
 /**
  * drm_syncobj_find - lookup and reference a sync object.
  * @file_private: drm file private pointer
@@ -190,23 +174,18 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
 }
 EXPORT_SYMBOL(drm_syncobj_replace_fence);
 
-static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
+/**
+ * drm_syncobj_assign_null_handle - assign a stub fence to the sync object
+ * @syncobj: sync object to assign the fence on
+ *
+ * Assign a already signaled stub fence to the sync object.
+ */
+static void drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj)
 {
-	struct drm_syncobj_stub_fence *fence;
-	fence = kzalloc(sizeof(*fence), GFP_KERNEL);
-	if (fence == NULL)
-		return -ENOMEM;
+	struct dma_fence *fence = dma_fence_get_stub();
 
-	spin_lock_init(&fence->lock);
-	dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops,
-		       &fence->lock, 0, 0);
-	dma_fence_signal(&fence->base);
-
-	drm_syncobj_replace_fence(syncobj, 0, &fence->base);
-
-	dma_fence_put(&fence->base);
-
-	return 0;
+	drm_syncobj_replace_fence(syncobj, 0, fence);
+	dma_fence_put(fence);
 }
 
 /**
@@ -274,7 +253,6 @@ EXPORT_SYMBOL(drm_syncobj_free);
 int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
 		       struct dma_fence *fence)
 {
-	int ret;
 	struct drm_syncobj *syncobj;
 
 	syncobj = kzalloc(sizeof(struct drm_syncobj), GFP_KERNEL);
@@ -285,13 +263,8 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags,
 	INIT_LIST_HEAD(&syncobj->cb_list);
 	spin_lock_init(&syncobj->lock);
 
-	if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) {
-		ret = drm_syncobj_assign_null_handle(syncobj);
-		if (ret < 0) {
-			drm_syncobj_put(syncobj);
-			return ret;
-		}
-	}
+	if (flags & DRM_SYNCOBJ_CREATE_SIGNALED)
+		drm_syncobj_assign_null_handle(syncobj);
 
 	if (fence)
 		drm_syncobj_replace_fence(syncobj, 0, fence);
@@ -982,11 +955,8 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
 	if (ret < 0)
 		return ret;
 
-	for (i = 0; i < args->count_handles; i++) {
-		ret = drm_syncobj_assign_null_handle(syncobjs[i]);
-		if (ret < 0)
-			break;
-	}
+	for (i = 0; i < args->count_handles; i++)
+		drm_syncobj_assign_null_handle(syncobjs[i]);
 
 	drm_syncobj_array_free(syncobjs, args->count_handles);
 
-- 
2.14.1

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

[PATCH 3/3] drm/amdgpu: fix NULL fence handling in amdgpu_cs_fence_to_handle_ioctl

[Christian König]

When the fence is already signaled it is perfectly normal to get a NULL
fence here. But since we can't export that we need to use a stub fence.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 024dfbd87f11..fe6bec2075c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -1421,6 +1421,9 @@ int amdgpu_cs_fence_to_handle_ioctl(struct drm_device *dev, void *data,
 	if (IS_ERR(fence))
 		return PTR_ERR(fence);
 
+	if (!fence)
+		fence = dma_fence_get_stub();
+
 	switch (info->in.what) {
 	case AMDGPU_FENCE_TO_HANDLE_GET_SYNCOBJ:
 		r = drm_syncobj_create(&syncobj, 0, fence);
-- 
2.14.1

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

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Chunming Zhou]

The series is Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

for patch#2, please remove my Signed-off-by, it's new when using stub 
from dma-fence.


-David


在 2018/12/3 21:07, Christian König 写道:
> Extract of useful code from the timeline work. This provides a function
> to return a stub or dummy fence which is always signaled.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>   include/linux/dma-fence.h   |  1 +
>   2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 1551ca7df394..136ec04d683f 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -30,13 +30,16 @@
>   EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
>   EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
>   
> +static DEFINE_SPINLOCK(dma_fence_stub_lock);
> +static struct dma_fence dma_fence_stub;
> +
>   /*
>    * fence context counter: each execution context should have its own
>    * fence context, this allows checking if fences belong to the same
>    * context or not. One device can have multiple separate contexts,
>    * and they're used if some engine can run independently of another.
>    */
> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
>   
>   /**
>    * DOC: DMA fences overview
> @@ -68,6 +71,37 @@ static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>    *   &dma_buf.resv pointer.
>    */
>   
> +static const char *dma_fence_stub_get_name(struct dma_fence *fence)
> +{
> +        return "stub";
> +}
> +
> +static const struct dma_fence_ops dma_fence_stub_ops = {
> +	.get_driver_name = dma_fence_stub_get_name,
> +	.get_timeline_name = dma_fence_stub_get_name,
> +};
> +
> +/**
> + * dma_fence_get_stub - return a signaled fence
> + *
> + * Return a stub fence which is already signaled.
> + */
> +struct dma_fence *dma_fence_get_stub(void)
> +{
> +	spin_lock(&dma_fence_stub_lock);
> +	if (!dma_fence_stub.ops) {
> +		dma_fence_init(&dma_fence_stub,
> +			       &dma_fence_stub_ops,
> +			       &dma_fence_stub_lock,
> +			       0, 0);
> +		dma_fence_signal_locked(&dma_fence_stub);
> +	}
> +	spin_unlock(&dma_fence_stub_lock);
> +
> +	return dma_fence_get(&dma_fence_stub);
> +}
> +EXPORT_SYMBOL(dma_fence_get_stub);
> +
>   /**
>    * dma_fence_context_alloc - allocate an array of fence contexts
>    * @num: amount of contexts to allocate
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 02dba8cd033d..999e4b104410 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -541,6 +541,7 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
>   	return ret < 0 ? ret : 0;
>   }
>   
> +struct dma_fence *dma_fence_get_stub(void);
>   u64 dma_fence_context_alloc(unsigned num);
>   
>   #define DMA_FENCE_TRACE(f, fmt, args...) \

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

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Christian König]

Am 03.12.18 um 14:33 schrieb Chunming Zhou:
> The series is Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
>
> for patch#2, please remove my Signed-off-by, it's new when using stub
> from dma-fence.

Yeah, ok. There is indeed nothing left from your original code.

Alex, Daniel, Chris any objections that I push the first two patches in 
this series to drm-misc-next?

Shouldn't be any functional change,
Christian.

>
>
> -David
>
>
> 在 2018/12/3 21:07, Christian König 写道:
>> Extract of useful code from the timeline work. This provides a function
>> to return a stub or dummy fence which is always signaled.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>    drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>>    include/linux/dma-fence.h   |  1 +
>>    2 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 1551ca7df394..136ec04d683f 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>> @@ -30,13 +30,16 @@
>>    EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
>>    EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
>>    
>> +static DEFINE_SPINLOCK(dma_fence_stub_lock);
>> +static struct dma_fence dma_fence_stub;
>> +
>>    /*
>>     * fence context counter: each execution context should have its own
>>     * fence context, this allows checking if fences belong to the same
>>     * context or not. One device can have multiple separate contexts,
>>     * and they're used if some engine can run independently of another.
>>     */
>> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
>>    
>>    /**
>>     * DOC: DMA fences overview
>> @@ -68,6 +71,37 @@ static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>>     *   &dma_buf.resv pointer.
>>     */
>>    
>> +static const char *dma_fence_stub_get_name(struct dma_fence *fence)
>> +{
>> +        return "stub";
>> +}
>> +
>> +static const struct dma_fence_ops dma_fence_stub_ops = {
>> +	.get_driver_name = dma_fence_stub_get_name,
>> +	.get_timeline_name = dma_fence_stub_get_name,
>> +};
>> +
>> +/**
>> + * dma_fence_get_stub - return a signaled fence
>> + *
>> + * Return a stub fence which is already signaled.
>> + */
>> +struct dma_fence *dma_fence_get_stub(void)
>> +{
>> +	spin_lock(&dma_fence_stub_lock);
>> +	if (!dma_fence_stub.ops) {
>> +		dma_fence_init(&dma_fence_stub,
>> +			       &dma_fence_stub_ops,
>> +			       &dma_fence_stub_lock,
>> +			       0, 0);
>> +		dma_fence_signal_locked(&dma_fence_stub);
>> +	}
>> +	spin_unlock(&dma_fence_stub_lock);
>> +
>> +	return dma_fence_get(&dma_fence_stub);
>> +}
>> +EXPORT_SYMBOL(dma_fence_get_stub);
>> +
>>    /**
>>     * dma_fence_context_alloc - allocate an array of fence contexts
>>     * @num: amount of contexts to allocate
>> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
>> index 02dba8cd033d..999e4b104410 100644
>> --- a/include/linux/dma-fence.h
>> +++ b/include/linux/dma-fence.h
>> @@ -541,6 +541,7 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
>>    	return ret < 0 ? ret : 0;
>>    }
>>    
>> +struct dma_fence *dma_fence_get_stub(void);
>>    u64 dma_fence_context_alloc(unsigned num);
>>    
>>    #define DMA_FENCE_TRACE(f, fmt, args...) \
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Alex Deucher]

On Mon, Dec 3, 2018 at 8:40 AM Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
>
> Am 03.12.18 um 14:33 schrieb Chunming Zhou:
> > The series is Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
> >
> > for patch#2, please remove my Signed-off-by, it's new when using stub
> > from dma-fence.
>
> Yeah, ok. There is indeed nothing left from your original code.
>
> Alex, Daniel, Chris any objections that I push the first two patches in
> this series to drm-misc-next?

No objections from me.  Does Intel want to run it through their CI?

Alex

>
> Shouldn't be any functional change,
> Christian.
>
> >
> >
> > -David
> >
> >
> > 在 2018/12/3 21:07, Christian König 写道:
> >> Extract of useful code from the timeline work. This provides a function
> >> to return a stub or dummy fence which is always signaled.
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> ---
> >>    drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
> >>    include/linux/dma-fence.h   |  1 +
> >>    2 files changed, 36 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> >> index 1551ca7df394..136ec04d683f 100644
> >> --- a/drivers/dma-buf/dma-fence.c
> >> +++ b/drivers/dma-buf/dma-fence.c
> >> @@ -30,13 +30,16 @@
> >>    EXPORT_TRACEPOINT_SYMBOL(dma_fence_emit);
> >>    EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
> >>
> >> +static DEFINE_SPINLOCK(dma_fence_stub_lock);
> >> +static struct dma_fence dma_fence_stub;
> >> +
> >>    /*
> >>     * fence context counter: each execution context should have its own
> >>     * fence context, this allows checking if fences belong to the same
> >>     * context or not. One device can have multiple separate contexts,
> >>     * and they're used if some engine can run independently of another.
> >>     */
> >> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> >> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
> >>
> >>    /**
> >>     * DOC: DMA fences overview
> >> @@ -68,6 +71,37 @@ static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> >>     *   &dma_buf.resv pointer.
> >>     */
> >>
> >> +static const char *dma_fence_stub_get_name(struct dma_fence *fence)
> >> +{
> >> +        return "stub";
> >> +}
> >> +
> >> +static const struct dma_fence_ops dma_fence_stub_ops = {
> >> +    .get_driver_name = dma_fence_stub_get_name,
> >> +    .get_timeline_name = dma_fence_stub_get_name,
> >> +};
> >> +
> >> +/**
> >> + * dma_fence_get_stub - return a signaled fence
> >> + *
> >> + * Return a stub fence which is already signaled.
> >> + */
> >> +struct dma_fence *dma_fence_get_stub(void)
> >> +{
> >> +    spin_lock(&dma_fence_stub_lock);
> >> +    if (!dma_fence_stub.ops) {
> >> +            dma_fence_init(&dma_fence_stub,
> >> +                           &dma_fence_stub_ops,
> >> +                           &dma_fence_stub_lock,
> >> +                           0, 0);
> >> +            dma_fence_signal_locked(&dma_fence_stub);
> >> +    }
> >> +    spin_unlock(&dma_fence_stub_lock);
> >> +
> >> +    return dma_fence_get(&dma_fence_stub);
> >> +}
> >> +EXPORT_SYMBOL(dma_fence_get_stub);
> >> +
> >>    /**
> >>     * dma_fence_context_alloc - allocate an array of fence contexts
> >>     * @num: amount of contexts to allocate
> >> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> >> index 02dba8cd033d..999e4b104410 100644
> >> --- a/include/linux/dma-fence.h
> >> +++ b/include/linux/dma-fence.h
> >> @@ -541,6 +541,7 @@ static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
> >>      return ret < 0 ? ret : 0;
> >>    }
> >>
> >> +struct dma_fence *dma_fence_get_stub(void);
> >>    u64 dma_fence_context_alloc(unsigned num);
> >>
> >>    #define DMA_FENCE_TRACE(f, fmt, args...) \
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Chris Wilson]

Quoting Christian König (2018-12-03 13:07:57)
> Extract of useful code from the timeline work. This provides a function
> to return a stub or dummy fence which is always signaled.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/syncobj: use dma_fence_get_stub

[Chris Wilson]

Quoting Christian König (2018-12-03 13:07:58)
> Extract of useful code from the timeline work. Let's use just a single
> stub fence instance instead of allocating a new one all the time.
> 
> Signed-off-by: Chunming Zhou <david1.zhou@amd.com>
> Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Chris Wilson]

Quoting Alex Deucher (2018-12-03 15:53:21)
> On Mon, Dec 3, 2018 at 8:40 AM Christian König
> <ckoenig.leichtzumerken@gmail.com> wrote:
> >
> > Am 03.12.18 um 14:33 schrieb Chunming Zhou:
> > > The series is Reviewed-by: Chunming Zhou <david1.zhou@amd.com>
> > >
> > > for patch#2, please remove my Signed-off-by, it's new when using stub
> > > from dma-fence.
> >
> > Yeah, ok. There is indeed nothing left from your original code.
> >
> > Alex, Daniel, Chris any objections that I push the first two patches in
> > this series to drm-misc-next?
> 
> No objections from me.  Does Intel want to run it through their CI?

Gut feeling says we have no test coverage for SYNCOBJ_CREATE_SIGNALED.
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1268 bytes --]

Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Extract of useful code from the timeline work. This provides a function
> to return a stub or dummy fence which is always signaled.
>
> Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>  include/linux/dma-fence.h   |  1 +
>  2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 1551ca7df394..136ec04d683f 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c

>  /*
>   * fence context counter: each execution context should have its own
>   * fence context, this allows checking if fences belong to the same
>   * context or not. One device can have multiple separate contexts,
>   * and they're used if some engine can run independently of another.
>   */
> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);

What's this change for?  I don't see a context allocation in patch #2
where the stub fence is being moved from, so this seems out of place.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Christian König]

Am 03.12.18 um 17:08 schrieb Eric Anholt:
> Christian König <ckoenig.leichtzumerken@gmail.com> writes:
>
>> Extract of useful code from the timeline work. This provides a function
>> to return a stub or dummy fence which is always signaled.
>>
>> Signed-off-by: Christian König <christian.koenig@amd.com>
>> ---
>>   drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>>   include/linux/dma-fence.h   |  1 +
>>   2 files changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>> index 1551ca7df394..136ec04d683f 100644
>> --- a/drivers/dma-buf/dma-fence.c
>> +++ b/drivers/dma-buf/dma-fence.c
>>   /*
>>    * fence context counter: each execution context should have its own
>>    * fence context, this allows checking if fences belong to the same
>>    * context or not. One device can have multiple separate contexts,
>>    * and they're used if some engine can run independently of another.
>>    */
>> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
> What's this change for?  I don't see a context allocation in patch #2
> where the stub fence is being moved from, so this seems out of place.

The stub fence is using context 0 and seqno 0, but since it is always 
signaled this actually shouldn't matter.

So this is just to be on the extra clean side I made sure that nobody 
else is using context 0.

Alternatively we could also just allocate one when we initialize it for 
the first time.

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

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Chris Wilson]

Quoting Eric Anholt (2018-12-03 16:08:40)
> Christian König <ckoenig.leichtzumerken@gmail.com> writes:
> 
> > Extract of useful code from the timeline work. This provides a function
> > to return a stub or dummy fence which is always signaled.
> >
> > Signed-off-by: Christian König <christian.koenig@amd.com>
> > ---
> >  drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
> >  include/linux/dma-fence.h   |  1 +
> >  2 files changed, 36 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 1551ca7df394..136ec04d683f 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> 
> >  /*
> >   * fence context counter: each execution context should have its own
> >   * fence context, this allows checking if fences belong to the same
> >   * context or not. One device can have multiple separate contexts,
> >   * and they're used if some engine can run independently of another.
> >   */
> > -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> > +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
> 
> What's this change for?  I don't see a context allocation in patch #2
> where the stub fence is being moved from, so this seems out of place.

I looked as it as being recognition that the stub-fence already exists
on context 0, and so we should reserve that context for private use.
-Chris
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 1825 bytes --]

Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:

> Am 03.12.18 um 17:08 schrieb Eric Anholt:
>> Christian König <ckoenig.leichtzumerken-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>>
>>> Extract of useful code from the timeline work. This provides a function
>>> to return a stub or dummy fence which is always signaled.
>>>
>>> Signed-off-by: Christian König <christian.koenig-5C7GfCeVMHo@public.gmane.org>
>>> ---
>>>   drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>>>   include/linux/dma-fence.h   |  1 +
>>>   2 files changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>> index 1551ca7df394..136ec04d683f 100644
>>> --- a/drivers/dma-buf/dma-fence.c
>>> +++ b/drivers/dma-buf/dma-fence.c
>>>   /*
>>>    * fence context counter: each execution context should have its own
>>>    * fence context, this allows checking if fences belong to the same
>>>    * context or not. One device can have multiple separate contexts,
>>>    * and they're used if some engine can run independently of another.
>>>    */
>>> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>>> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
>> What's this change for?  I don't see a context allocation in patch #2
>> where the stub fence is being moved from, so this seems out of place.
>
> The stub fence is using context 0 and seqno 0, but since it is always 
> signaled this actually shouldn't matter.
>
> So this is just to be on the extra clean side I made sure that nobody 
> else is using context 0.
>
> Alternatively we could also just allocate one when we initialize it for 
> the first time.

I like the allocate at init idea.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Chris Wilson]

Quoting Christian König (2018-12-03 16:12:14)
> Am 03.12.18 um 17:08 schrieb Eric Anholt:
> > Christian König <ckoenig.leichtzumerken@gmail.com> writes:
> >
> >> Extract of useful code from the timeline work. This provides a function
> >> to return a stub or dummy fence which is always signaled.
> >>
> >> Signed-off-by: Christian König <christian.koenig@amd.com>
> >> ---
> >>   drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
> >>   include/linux/dma-fence.h   |  1 +
> >>   2 files changed, 36 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> >> index 1551ca7df394..136ec04d683f 100644
> >> --- a/drivers/dma-buf/dma-fence.c
> >> +++ b/drivers/dma-buf/dma-fence.c
> >>   /*
> >>    * fence context counter: each execution context should have its own
> >>    * fence context, this allows checking if fences belong to the same
> >>    * context or not. One device can have multiple separate contexts,
> >>    * and they're used if some engine can run independently of another.
> >>    */
> >> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
> >> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
> > What's this change for?  I don't see a context allocation in patch #2
> > where the stub fence is being moved from, so this seems out of place.
> 
> The stub fence is using context 0 and seqno 0, but since it is always 
> signaled this actually shouldn't matter.
> 
> So this is just to be on the extra clean side I made sure that nobody 
> else is using context 0.

I would like to be able to reserve 0 for NO_CONTEXT! The stub being but
one example.
-Chris
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] dma-buf: add dma_fence_get_stub

[Christian König]

Am 03.12.18 um 18:06 schrieb Chris Wilson:
> Quoting Christian König (2018-12-03 16:12:14)
>> Am 03.12.18 um 17:08 schrieb Eric Anholt:
>>> Christian König <ckoenig.leichtzumerken@gmail.com> writes:
>>>
>>>> Extract of useful code from the timeline work. This provides a function
>>>> to return a stub or dummy fence which is always signaled.
>>>>
>>>> Signed-off-by: Christian König <christian.koenig@amd.com>
>>>> ---
>>>>    drivers/dma-buf/dma-fence.c | 36 +++++++++++++++++++++++++++++++++++-
>>>>    include/linux/dma-fence.h   |  1 +
>>>>    2 files changed, 36 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
>>>> index 1551ca7df394..136ec04d683f 100644
>>>> --- a/drivers/dma-buf/dma-fence.c
>>>> +++ b/drivers/dma-buf/dma-fence.c
>>>>    /*
>>>>     * fence context counter: each execution context should have its own
>>>>     * fence context, this allows checking if fences belong to the same
>>>>     * context or not. One device can have multiple separate contexts,
>>>>     * and they're used if some engine can run independently of another.
>>>>     */
>>>> -static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>>>> +static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(1);
>>> What's this change for?  I don't see a context allocation in patch #2
>>> where the stub fence is being moved from, so this seems out of place.
>> The stub fence is using context 0 and seqno 0, but since it is always
>> signaled this actually shouldn't matter.
>>
>> So this is just to be on the extra clean side I made sure that nobody
>> else is using context 0.
> I would like to be able to reserve 0 for NO_CONTEXT! The stub being but
> one example.

I'm slightly leaning into this direction as well and additional to that 
I've already pushed the patches to drm-misc-next.

The only thing we should avoid is to over use 0 as NO_context, e.g. with 
complicated sequence numbers or similar.

Christian.

> -Chris
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx