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.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.

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-de...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to