| Message ID | 20171115171057.17340-9-gustavo@padovan.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote: > From: Javier Martinez Canillas <javier@osg.samsung.com> > > Add a videobuf2-fence.h header file that contains different helpers > for DMA buffer sharing explicit fence support in videobuf2. > > v2: - use fence context provided by the caller in vb2_fence_alloc() > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.com> > --- > include/media/videobuf2-fence.h | 48 > +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > create mode 100644 include/media/videobuf2-fence.h > > diff --git a/include/media/videobuf2-fence.h > b/include/media/videobuf2-fence.h > new file mode 100644 > index 000000000000..b49cc1bf6bb4 > --- /dev/null > +++ b/include/media/videobuf2-fence.h > @@ -0,0 +1,48 @@ > +/* > + * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2 > + * > + * Copyright (C) 2016 Samsung Electronics > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation. > + */ > + > +#include <linux/dma-fence.h> > +#include <linux/slab.h> > + > +static DEFINE_SPINLOCK(vb2_fence_lock); > + > +static inline const char *vb2_fence_get_driver_name(struct > dma_fence *fence) > +{ > + return "vb2_fence"; > +} > + > +static inline const char *vb2_fence_get_timeline_name(struct > dma_fence *fence) > +{ > + return "vb2_fence_timeline"; > +} > + > +static inline bool vb2_fence_enable_signaling(struct dma_fence *fence) > +{ > + return true; > +} > + > +static const struct dma_fence_ops vb2_fence_ops = { > + .get_driver_name = vb2_fence_get_driver_name, > + .get_timeline_name = vb2_fence_get_timeline_name, > + .enable_signaling = vb2_fence_enable_signaling, > + .wait = dma_fence_default_wait, > +}; It is probably not a good idea to define that struct here since it will be deduplicated for every source file that includes it. Maybe change it to a simple declaration, and move the definition to videobuf2-core.c or a dedicated videobuf2-fence.c file? > + > +static inline struct dma_fence *vb2_fence_alloc(u64 context) > +{ > + struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL); > + > + if (!vb2_fence) > + return NULL; > + > + dma_fence_init(vb2_fence, &vb2_fence_ops, &vb2_fence_lock, context, 1); > + > + return vb2_fence; > +} Not sure we gain a lot by having this function static inline, but your call.
On Friday, November 17, 2017 4:02:56 PM JST, Alexandre Courbot wrote: > On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote: >> From: Javier Martinez Canillas <javier@osg.samsung.com> >> >> Add a videobuf2-fence.h header file that contains different helpers >> for DMA buffer sharing explicit fence support in videobuf2. >> >> v2: - use fence context provided by the caller in vb2_fence_alloc() >> >> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> ... > > It is probably not a good idea to define that struct here since it will be > deduplicated for every source file that includes it. > > Maybe change it to a simple declaration, and move the definition to > videobuf2-core.c or a dedicated videobuf2-fence.c file? > >> + >> +static inline struct dma_fence *vb2_fence_alloc(u64 context) >> +{ >> + struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL); >> + ... > > Not sure we gain a lot by having this function static inline, but your call. > Looking at the following patch, since it seems that this function is only to be called from vb2_setup_out_fence() anyway, you may as well make it static in videobuf2-core.c or even inline it in vb2_setup_out_fence() - it would only add a few extra lines to this function.
2017-11-17 Alexandre Courbot <acourbot@chromium.org>: > On Friday, November 17, 2017 4:02:56 PM JST, Alexandre Courbot wrote: > > On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote: > > > From: Javier Martinez Canillas <javier@osg.samsung.com> > > > > > > Add a videobuf2-fence.h header file that contains different helpers > > > for DMA buffer sharing explicit fence support in videobuf2. > > > > > > v2: - use fence context provided by the caller in vb2_fence_alloc() > > > > > > Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com> ... > > > > It is probably not a good idea to define that struct here since it will be > > deduplicated for every source file that includes it. > > > > Maybe change it to a simple declaration, and move the definition to > > videobuf2-core.c or a dedicated videobuf2-fence.c file? > > > > > + > > > +static inline struct dma_fence *vb2_fence_alloc(u64 context) > > > +{ > > > + struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL); > > > + ... > > > > Not sure we gain a lot by having this function static inline, but your call. > > > > Looking at the following patch, since it seems that this function is only to > be > called from vb2_setup_out_fence() anyway, you may as well make it static in > videobuf2-core.c or even inline it in vb2_setup_out_fence() - it would only > add a few extra lines to this function. Okay, that makes sense, I'll will move everything to videobuf2-core.c
diff --git a/include/media/videobuf2-fence.h b/include/media/videobuf2-fence.h new file mode 100644 index 000000000000..b49cc1bf6bb4 --- /dev/null +++ b/include/media/videobuf2-fence.h @@ -0,0 +1,48 @@ +/* + * videobuf2-fence.h - DMA buffer sharing fence helpers for videobuf 2 + * + * Copyright (C) 2016 Samsung Electronics + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation. + */ + +#include <linux/dma-fence.h> +#include <linux/slab.h> + +static DEFINE_SPINLOCK(vb2_fence_lock); + +static inline const char *vb2_fence_get_driver_name(struct dma_fence *fence) +{ + return "vb2_fence"; +} + +static inline const char *vb2_fence_get_timeline_name(struct dma_fence *fence) +{ + return "vb2_fence_timeline"; +} + +static inline bool vb2_fence_enable_signaling(struct dma_fence *fence) +{ + return true; +} + +static const struct dma_fence_ops vb2_fence_ops = { + .get_driver_name = vb2_fence_get_driver_name, + .get_timeline_name = vb2_fence_get_timeline_name, + .enable_signaling = vb2_fence_enable_signaling, + .wait = dma_fence_default_wait, +}; + +static inline struct dma_fence *vb2_fence_alloc(u64 context) +{ + struct dma_fence *vb2_fence = kzalloc(sizeof(*vb2_fence), GFP_KERNEL); + + if (!vb2_fence) + return NULL; + + dma_fence_init(vb2_fence, &vb2_fence_ops, &vb2_fence_lock, context, 1); + + return vb2_fence; +}