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

[zhoucm1]

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.z...@amd.com>
Cc: Jason Ekstrand <ja...@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

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