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 at amd.com> >> Cc: Jason Ekstrand <jason at 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 at lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel