Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers
2017-11-17 Alexandre Courbot : > 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 > > > > > > 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 ... > > > > 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
Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers
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 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 ... 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.
Re: [RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers
On Thursday, November 16, 2017 2:10:54 AM JST, Gustavo Padovan wrote: From: Javier Martinez Canillas 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 Signed-off-by: Gustavo Padovan --- 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 ..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 +#include + +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.
[RFC v5 08/11] [media] vb2: add videobuf2 dma-buf fence helpers
From: Javier Martinez Canillas 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 Signed-off-by: Gustavo Padovan --- 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 ..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 +#include + +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; +} -- 2.13.6