Re: [PATCH 1/2] drm: rename null fence to stub fence in syncobj

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Cc: Jason Ekstrand <jason@xxxxxxxxxxxxxx>
---
  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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




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

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux