stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net --- drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);
-struct drm_syncobj_null_fence { - struct dma_fence base; - spinlock_t lock; -}; - -static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{ - return "syncobjnull"; -} - -static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{ - dma_fence_enable_sw_signaling(fence); - return !dma_fence_is_signaled(fence); -} - -static const struct dma_fence_ops drm_syncobj_null_fence_ops = { - .get_driver_name = drm_syncobj_null_fence_get_name, - .get_timeline_name = drm_syncobj_null_fence_get_name, - .enable_signaling = drm_syncobj_null_fence_enable_signaling, - .wait = dma_fence_default_wait, - .release = NULL, -}; - static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { - struct drm_syncobj_null_fence *fence; + struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM;
spin_lock_init(&fence->lock); - dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@
struct drm_syncobj_cb;
+struct drm_syncobj_stub_fence { + struct dma_fence base; + spinlock_t lock; +}; + +const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{ + return "syncobjstub"; +} + +bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence); + return !dma_fence_is_signaled(fence); +} + +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, + .enable_signaling = drm_syncobj_stub_fence_enable_signaling, + .wait = dma_fence_default_wait, + .release = NULL, +}; + /** * struct drm_syncobj - sync object. *
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);
-struct drm_syncobj_null_fence {
- struct dma_fence base;
- spinlock_t lock;
-};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{
return "syncobjnull";-}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
-}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
- .get_driver_name = drm_syncobj_null_fence_get_name,
- .get_timeline_name = drm_syncobj_null_fence_get_name,
- .enable_signaling = drm_syncobj_null_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
-};
- static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {
- struct drm_syncobj_null_fence *fence;
struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM;
spin_lock_init(&fence->lock);
- dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
- dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@
struct drm_syncobj_cb;
+struct drm_syncobj_stub_fence {
- struct dma_fence base;
- spinlock_t lock;
+};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{
return "syncobjstub";+}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{
- dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
- return !dma_fence_is_signaled(fence);
+}
+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,
- .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
- .wait = dma_fence_default_wait,
The .wait callback should be dropped.
Apart from that looks good to me.
Christian.
- .release = NULL,
+};
- /**
- struct drm_syncobj - sync object.
On Wed, Aug 22, 2018 at 10:52:16AM +0200, Christian König wrote:
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence {
- struct dma_fence base;
- spinlock_t lock;
-};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{
return "syncobjnull";-}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
-}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
- .get_driver_name = drm_syncobj_null_fence_get_name,
- .get_timeline_name = drm_syncobj_null_fence_get_name,
- .enable_signaling = drm_syncobj_null_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
-};
- static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {
- struct drm_syncobj_null_fence *fence;
- struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock);
- dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
- dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence {
- struct dma_fence base;
- spinlock_t lock;
+};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{
return "syncobjstub";+}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{
- dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
- return !dma_fence_is_signaled(fence);
+}
+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,
- .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
- .wait = dma_fence_default_wait,
The .wait callback should be dropped.
Apart from that looks good to me.
.enable_signaling should probably also be dropped. Same also for wherever you copied this from. -Daniel
Christian.
- .release = NULL,
+};
- /**
- struct drm_syncobj - sync object.
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 2018年08月22日 16:52, Christian König wrote:
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence { - struct dma_fence base; - spinlock_t lock; -};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{ - return "syncobjnull"; -}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{ - dma_fence_enable_sw_signaling(fence); - return !dma_fence_is_signaled(fence); -}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = { - .get_driver_name = drm_syncobj_null_fence_get_name, - .get_timeline_name = drm_syncobj_null_fence_get_name, - .enable_signaling = drm_syncobj_null_fence_enable_signaling, - .wait = dma_fence_default_wait, - .release = NULL, -};
static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { - struct drm_syncobj_null_fence *fence; + struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock); - dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence { + struct dma_fence base; + spinlock_t lock; +};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{ + return "syncobjstub"; +}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
indeed, will fix.
+ return !dma_fence_is_signaled(fence); +}
+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, + .enable_signaling = drm_syncobj_stub_fence_enable_signaling, + .wait = dma_fence_default_wait,
The .wait callback should be dropped.
why?
fence->ops->wait(fence, intr, timeout) is called by dma_fence_wait(). If dropped, how does dma_fence_wait() work?
Thanks, David
Apart from that looks good to me.
Christian.
+ .release = NULL, +};
/** * struct drm_syncobj - sync object. *
Am 22.08.2018 um 11:02 schrieb zhoucm1:
On 2018年08月22日 16:52, Christian König wrote:
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence { - struct dma_fence base; - spinlock_t lock; -};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{ - return "syncobjnull"; -}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{ - dma_fence_enable_sw_signaling(fence); - return !dma_fence_is_signaled(fence); -}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = { - .get_driver_name = drm_syncobj_null_fence_get_name, - .get_timeline_name = drm_syncobj_null_fence_get_name, - .enable_signaling = drm_syncobj_null_fence_enable_signaling, - .wait = dma_fence_default_wait, - .release = NULL, -};
static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { - struct drm_syncobj_null_fence *fence; + struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock); - dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence { + struct dma_fence base; + spinlock_t lock; +};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{ + return "syncobjstub"; +}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
indeed, will fix.
+ return !dma_fence_is_signaled(fence); +}
+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, + .enable_signaling = drm_syncobj_stub_fence_enable_signaling, + .wait = dma_fence_default_wait,
The .wait callback should be dropped.
why?
fence->ops->wait(fence, intr, timeout) is called by dma_fence_wait(). If dropped, how does dma_fence_wait() work?
You are working on an older code base, fence->ops->wait is optional by now.
Christian.
Thanks, David
Apart from that looks good to me.
Christian.
+ .release = NULL, +};
/** * struct drm_syncobj - sync object. *
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
On 2018年08月22日 17:05, Christian König wrote:
Am 22.08.2018 um 11:02 schrieb zhoucm1:
On 2018年08月22日 16:52, Christian König wrote:
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence { - struct dma_fence base; - spinlock_t lock; -};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{ - return "syncobjnull"; -}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{ - dma_fence_enable_sw_signaling(fence); - return !dma_fence_is_signaled(fence); -}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = { - .get_driver_name = drm_syncobj_null_fence_get_name, - .get_timeline_name = drm_syncobj_null_fence_get_name, - .enable_signaling = drm_syncobj_null_fence_enable_signaling, - .wait = dma_fence_default_wait, - .release = NULL, -};
static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { - struct drm_syncobj_null_fence *fence; + struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock); - dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence { + struct dma_fence base; + spinlock_t lock; +};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{ + return "syncobjstub"; +}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
indeed, will fix.
+ return !dma_fence_is_signaled(fence); +}
+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, + .enable_signaling = drm_syncobj_stub_fence_enable_signaling, + .wait = dma_fence_default_wait,
The .wait callback should be dropped.
why?
fence->ops->wait(fence, intr, timeout) is called by dma_fence_wait(). If dropped, how does dma_fence_wait() work?
You are working on an older code base, fence->ops->wait is optional by now.
Sorry, I still don't get it. My code is synced today from amd-staging-drm-next, and it's 4.18-rc1. I still see the dma_fence_wait_timeout is : signed long dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) { signed long ret;
if (WARN_ON(timeout < 0)) return -EINVAL;
trace_dma_fence_wait_start(fence); ret = fence->ops->wait(fence, intr, timeout); trace_dma_fence_wait_end(fence); return ret; }
.wait callback seems still a must have?
Thanks, David Zhou
Christian.
Thanks, David
Apart from that looks good to me.
Christian.
+ .release = NULL, +};
/** * struct drm_syncobj - sync object. *
amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Am 22.08.2018 um 11:10 schrieb zhoucm1:
On 2018年08月22日 17:05, Christian König wrote:
Am 22.08.2018 um 11:02 schrieb zhoucm1:
On 2018年08月22日 16:52, Christian König wrote:
Am 22.08.2018 um 10:38 schrieb Chunming Zhou:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence { - struct dma_fence base; - spinlock_t lock; -};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{ - return "syncobjnull"; -}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{ - dma_fence_enable_sw_signaling(fence); - return !dma_fence_is_signaled(fence); -}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = { - .get_driver_name = drm_syncobj_null_fence_get_name, - .get_timeline_name = drm_syncobj_null_fence_get_name, - .enable_signaling = drm_syncobj_null_fence_enable_signaling, - .wait = dma_fence_default_wait, - .release = NULL, -};
static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) { - struct drm_syncobj_null_fence *fence; + struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock); - dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops, + dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base); diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence { + struct dma_fence base; + spinlock_t lock; +};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{ + return "syncobjstub"; +}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{ + dma_fence_enable_sw_signaling(fence);
Copy&pasted from the old implementation, but that is certainly totally nonsense.
dma_fence_enable_sw_signaling() is the function who is calling this callback.
indeed, will fix.
+ return !dma_fence_is_signaled(fence); +}
+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, + .enable_signaling = drm_syncobj_stub_fence_enable_signaling, + .wait = dma_fence_default_wait,
The .wait callback should be dropped.
why?
fence->ops->wait(fence, intr, timeout) is called by dma_fence_wait(). If dropped, how does dma_fence_wait() work?
You are working on an older code base, fence->ops->wait is optional by now.
Sorry, I still don't get it. My code is synced today from amd-staging-drm-next, and it's 4.18-rc1.
That is outdated, when working on common drm stuff you need to work on drm-next or drm-misc-next.
I still see the dma_fence_wait_timeout is : signed long dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) { signed long ret;
if (WARN_ON(timeout < 0)) return -EINVAL;
trace_dma_fence_wait_start(fence); ret = fence->ops->wait(fence, intr, timeout); trace_dma_fence_wait_end(fence); return ret; }
.wait callback seems still a must have?
See drm-misc-next:
trace_dma_fence_wait_start(fence); if (fence->ops->wait) ret = fence->ops->wait(fence, intr, timeout); else ret = dma_fence_default_wait(fence, intr, timeout); trace_dma_fence_wait_end(fence);
Regards, Christian.
Thanks, David Zhou
Christian.
Thanks, David
Apart from that looks good to me.
Christian.
+ .release = NULL, +};
/** * struct drm_syncobj - sync object. *
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
On Wed, Aug 22, 2018 at 04:38:56PM +0800, Chunming Zhou wrote:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
Please don't expose stuff only used by the drm_syncobj implementation to drivers. Gives us a very unclean driver interface. Imo this should all be left within drm_syncobj.h.
See also my comments for patch 2, you leak all the implemenation details to drivers. We need to fix that and have a clear interface. -Daniel
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);
-struct drm_syncobj_null_fence {
- struct dma_fence base;
- spinlock_t lock;
-};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{
return "syncobjnull";-}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
-}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
- .get_driver_name = drm_syncobj_null_fence_get_name,
- .get_timeline_name = drm_syncobj_null_fence_get_name,
- .enable_signaling = drm_syncobj_null_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
-};
static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {
- struct drm_syncobj_null_fence *fence;
struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM;
spin_lock_init(&fence->lock);
- dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
- dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@
struct drm_syncobj_cb;
+struct drm_syncobj_stub_fence {
- struct dma_fence base;
- spinlock_t lock;
+};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{
return "syncobjstub";+}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
+}
+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,
- .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
+};
/**
- struct drm_syncobj - sync object.
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On 2018年08月22日 17:34, Daniel Vetter wrote:
On Wed, Aug 22, 2018 at 04:38:56PM +0800, Chunming Zhou wrote:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
Please don't expose stuff only used by the drm_syncobj implementation to drivers. Gives us a very unclean driver interface. Imo this should all be left within drm_syncobj.h.
.c? will fix that.
See also my comments for patch 2, you leak all the implemenation details to drivers. We need to fix that and have a clear interface.
Yes, I will address them when I do v2.
Thanks, David Zhou
-Daniel
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence);
-struct drm_syncobj_null_fence {
- struct dma_fence base;
- spinlock_t lock;
-};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{
return "syncobjnull";-}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
-}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
- .get_driver_name = drm_syncobj_null_fence_get_name,
- .get_timeline_name = drm_syncobj_null_fence_get_name,
- .enable_signaling = drm_syncobj_null_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
-};
- static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {
- struct drm_syncobj_null_fence *fence;
struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM;
spin_lock_init(&fence->lock);
- dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
- dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@
struct drm_syncobj_cb;
+struct drm_syncobj_stub_fence {
- struct dma_fence base;
- spinlock_t lock;
+};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{
return "syncobjstub";+}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
+}
+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,
- .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
+};
- /**
- struct drm_syncobj - sync object.
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Aug 22, 2018 at 05:56:10PM +0800, zhoucm1 wrote:
On 2018年08月22日 17:34, Daniel Vetter wrote:
On Wed, Aug 22, 2018 at 04:38:56PM +0800, Chunming Zhou wrote:
stub fence will be used by timeline syncobj as well.
Change-Id: Ia4252f03c07a8105491d2791dc7c8c6976682285 Signed-off-by: Chunming Zhou david1.zhou@amd.com Cc: Jason Ekstrand jason@jlekstrand.net
Please don't expose stuff only used by the drm_syncobj implementation to drivers. Gives us a very unclean driver interface. Imo this should all be left within drm_syncobj.h.
.c? will fix that.
Yup I meant to leave it all in drm_syncobj.c :-) -Daniel
See also my comments for patch 2, you leak all the implemenation details to drivers. We need to fix that and have a clear interface.
Yes, I will address them when I do v2.
Thanks, David Zhou
-Daniel
drivers/gpu/drm/drm_syncobj.c | 28 ++-------------------------- include/drm/drm_syncobj.h | 24 ++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index d4f4ce484529..70af32d0def1 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -187,39 +187,15 @@ void drm_syncobj_replace_fence(struct drm_syncobj *syncobj, } EXPORT_SYMBOL(drm_syncobj_replace_fence); -struct drm_syncobj_null_fence {
- struct dma_fence base;
- spinlock_t lock;
-};
-static const char *drm_syncobj_null_fence_get_name(struct dma_fence *fence) -{
return "syncobjnull";-}
-static bool drm_syncobj_null_fence_enable_signaling(struct dma_fence *fence) -{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
-}
-static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
- .get_driver_name = drm_syncobj_null_fence_get_name,
- .get_timeline_name = drm_syncobj_null_fence_get_name,
- .enable_signaling = drm_syncobj_null_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
-};
- static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) {
- struct drm_syncobj_null_fence *fence;
- struct drm_syncobj_stub_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); if (fence == NULL) return -ENOMEM; spin_lock_init(&fence->lock);
- dma_fence_init(&fence->base, &drm_syncobj_null_fence_ops,
- dma_fence_init(&fence->base, &drm_syncobj_stub_fence_ops, &fence->lock, 0, 0); dma_fence_signal(&fence->base);
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h index 3980602472c0..b04c492ddbb5 100644 --- a/include/drm/drm_syncobj.h +++ b/include/drm/drm_syncobj.h @@ -30,6 +30,30 @@ struct drm_syncobj_cb; +struct drm_syncobj_stub_fence {
- struct dma_fence base;
- spinlock_t lock;
+};
+const char *drm_syncobj_stub_fence_get_name(struct dma_fence *fence) +{
return "syncobjstub";+}
+bool drm_syncobj_stub_fence_enable_signaling(struct dma_fence *fence) +{
- dma_fence_enable_sw_signaling(fence);
- return !dma_fence_is_signaled(fence);
+}
+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,
- .enable_signaling = drm_syncobj_stub_fence_enable_signaling,
- .wait = dma_fence_default_wait,
- .release = NULL,
+};
- /**
- struct drm_syncobj - sync object.
-- 2.14.1
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Christian König -
Chunming Zhou -
Daniel Vetter -
zhoucm1