Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences

On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote:

From: Gustavo Padovan gustavo.padovan@collabora.co.uk

struct fence_collection inherits from struct fence and carries a collection of fences that needs to be waited together.

It is useful to translate a sync_file to a fence to remove the complexity of dealing with sync_files on DRM drivers. So even if there are many fences in the sync_file that needs to waited for a commit to happen, they all get added to the fence_collection and passed for DRM use as a standard struct fence.

That means that no changes needed to any driver besides supporting fences.

fence_collection's fence doesn't belong to any timeline context, so fence_is_later() and fence_later() are not meant to be called with fence_collections fences.

v2: Comments by Daniel Vetter:

• merge fence_collection_init() and fence_collection_add()
• only add callbacks at ->enable_signalling()
• remove fence_collection_put()
• check for type on to_fence_collection()
• adjust fence_is_later() and fence_later() to WARN_ON() if they

are used with collection fences.

Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk

FENCE_NO_CONTEXT semantics needs an ack from amdgpu maintainers. I'm not entirely sure they might not hit the new WARN_ON by accident now. Please cc Alex Deucher & Christian König. -Daniel

---

drivers/dma-buf/Makefile | 2 +- drivers/dma-buf/fence-collection.c | 159 +++++++++++++++++++++++++++++++++++++ drivers/dma-buf/fence.c | 2 +- include/linux/fence-collection.h | 73 +++++++++++++++++ include/linux/fence.h | 9 +++ 5 files changed, 243 insertions(+), 2 deletions(-) create mode 100644 drivers/dma-buf/fence-collection.c create mode 100644 include/linux/fence-collection.h

diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile index 4a424ec..52f818f 100644 --- a/drivers/dma-buf/Makefile +++ b/drivers/dma-buf/Makefile @@ -1,2 +1,2 @@ -obj-y := dma-buf.o fence.o reservation.o seqno-fence.o +obj-y := dma-buf.o fence.o reservation.o seqno-fence.o fence-collection.o obj-$(CONFIG_SYNC_FILE) += sync_file.o diff --git a/drivers/dma-buf/fence-collection.c b/drivers/dma-buf/fence-collection.c new file mode 100644 index 0000000..88872e5 --- /dev/null +++ b/drivers/dma-buf/fence-collection.c @@ -0,0 +1,159 @@ +/*

• • fence-collection: aggregate fences to be waited together
• • Copyright (C) 2016 Collabora Ltd
• • Authors:
• • Gustavo Padovan gustavo@padovan.org
• • This program is free software; you can redistribute it and/or modify it
• • under the terms of the GNU General Public License version 2 as published by
• • the Free Software Foundation.
• • This program is distributed in the hope that it will be useful, but WITHOUT
• • ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
• • FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
• • more details.
• */

+#include <linux/export.h> +#include <linux/slab.h> +#include <linux/fence-collection.h>

+static const char *fence_collection_get_driver_name(struct fence *fence) +{

• struct fence_collection *collection = to_fence_collection(fence);
• struct fence *f = collection->fences[0].fence;
• return f->ops->get_driver_name(fence);

+}

+static const char *fence_collection_get_timeline_name(struct fence *fence) +{

• return "no context";

+}

+static void collection_check_cb_func(struct fence *fence, struct fence_cb *cb) +{

• struct fence_collection_cb *f_cb;
• struct fence_collection *collection;
• f_cb = container_of(cb, struct fence_collection_cb, cb);
• collection = f_cb->collection;
• if (atomic_dec_and_test(&collection->num_pending_fences))

• fence_signal(&collection->base);
  

+}

+static bool fence_collection_enable_signaling(struct fence *fence) +{

• struct fence_collection *collection = to_fence_collection(fence);
• int i;
• for (i = 0 ; i < collection->num_fences ; i++) {

• if (fence_add_callback(collection->fences[i].fence,
  

• 		       &collection->fences[i].cb,
  

• 		       collection_check_cb_func)) {
  

• 	atomic_dec(&collection->num_pending_fences);
  

• 	return false;
  

• }
  

• }
• return !!atomic_read(&collection->num_pending_fences);

+}

+static bool fence_collection_signaled(struct fence *fence) +{

• struct fence_collection *collection = to_fence_collection(fence);
• return (atomic_read(&collection->num_pending_fences) == 0);

+}

+static void fence_collection_release(struct fence *fence) +{

• struct fence_collection *collection = to_fence_collection(fence);
• int i;
• for (i = 0 ; i < collection->num_fences ; i++)

• fence_put(collection->fences[i].fence);
  

• kfree(collection->fences);
• fence_free(fence);

+}

+static signed long fence_collection_wait(struct fence *fence, bool intr,

• 			 signed long timeout)
  

+{

• struct fence_collection *collection = to_fence_collection(fence);
• int i;
• if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))

• return timeout;
  

• fence_enable_sw_signaling(fence);
• for (i = 0 ; i < collection->num_fences ; i++) {

• if (test_bit(FENCE_FLAG_SIGNALED_BIT, &fence->flags))
  

• 	return timeout;
  

• timeout = fence_wait(collection->fences[i].fence, intr);
  

• if (timeout < 0)
  

• 	return timeout;
  

• }
• return timeout;

+}

+const struct fence_ops fence_collection_ops = {

• .get_driver_name = fence_collection_get_driver_name,
• .get_timeline_name = fence_collection_get_timeline_name,
• .enable_signaling = fence_collection_enable_signaling,
• .signaled = fence_collection_signaled,
• .wait = fence_collection_wait,
• .release = fence_collection_release,

+};

+/**

• • fence_collection_create - Create a custom fence collection
• • is signaled
• • @num_fences: [in] number of fences to add in the collection
• • @cb: [in] cb array containing the fences for the collection
• • Allocate a fence_collection and initialize the base fence with fence_init()
• • and FENCE_NO_CONTEXT. It also inits all fence_collections specific fields
• • and return the created fence_collection. In case of error it returns NULL.
• • The caller should allocte the fence_collection_cb array with num_fences size
• • and fill the fence fields with all the fences it wants to add to the
• • collection.
• • Collections have no context, thus fence_later() and fence_is_later() shall
• • not be used with fence_collection.
• • fence_put() on the base fence should be used to release the collection.
• */

+struct fence_collection *fence_collection_create(int num_fences,

• 				 struct fence_collection_cb *cb)
  

+{

• struct fence_collection *collection;
• int i;
• collection = kzalloc(sizeof(*collection), GFP_KERNEL);
• if (!collection)

• return NULL;
  

• spin_lock_init(&collection->lock);
• fence_init(&collection->base, &fence_collection_ops, &collection->lock,

•    FENCE_NO_CONTEXT, 0);
  

• collection->fences = cb;
• collection->num_fences = num_fences;
• atomic_set(&collection->num_pending_fences, num_fences);
• for (i = 0 ; i < collection->num_fences ; i++) {

• fence_get(collection->fences[i].fence);
  

• collection->fences[i].collection = collection;
  

• }
• return collection;

+} +EXPORT_SYMBOL(fence_collection_create); diff --git a/drivers/dma-buf/fence.c b/drivers/dma-buf/fence.c index 7b05dbe..486e95c 100644 --- a/drivers/dma-buf/fence.c +++ b/drivers/dma-buf/fence.c @@ -35,7 +35,7 @@ EXPORT_TRACEPOINT_SYMBOL(fence_emit);

• context or not. One device can have multiple separate contexts,
• and they're used if some engine can run independently of another.

*/ -static atomic_t fence_context_counter = ATOMIC_INIT(0); +static atomic_t fence_context_counter = ATOMIC_INIT(1);

/**

• fence_context_alloc - allocate an array of fence contexts

diff --git a/include/linux/fence-collection.h b/include/linux/fence-collection.h new file mode 100644 index 0000000..4087622 --- /dev/null +++ b/include/linux/fence-collection.h @@ -0,0 +1,73 @@ +/*

• • fence-collection: aggregates fence to be waited together
• • Copyright (C) 2016 Collabora Ltd
• • Authors:
• • Gustavo Padovan gustavo@padovan.org
• • This program is free software; you can redistribute it and/or modify it
• • under the terms of the GNU General Public License version 2 as published by
• • the Free Software Foundation.
• • This program is distributed in the hope that it will be useful, but WITHOUT
• • ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
• • FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
• • more details.
• */

+#ifndef __LINUX_FENCE_COLLECTION_H +#define __LINUX_FENCE_COLLECTION_H

+#include <linux/fence.h>

+/**

• • struct fence_collection_cb - callback for fence_add_callback
• • @cb: the base fence_cb object
• • @fence: fence we are storing in the collection
• • @collection: collection the fence belongs to
• • This struct is at the same time the storage for the fences in the
• • collection and the struct we pass to fence_add_callback()
• */

+struct fence_collection_cb {

• struct fence_cb cb;
• struct fence *fence;
• struct fence_collection *collection;

+};

+/**

• • struct fence_collection - fence to represent a collection of fences
• • @base: fence base class
• • @lock: spinlock for fence handling
• • @num_pending_fences: fences in the collection not signaled yet
• • @num_fences: number of fences in the collection
• • @fences: ponteir to fence_collection_cb with data about the fences
• */

+struct fence_collection {

• struct fence base;
• spinlock_t lock;
• atomic_t num_pending_fences;
• int num_fences;
• struct fence_collection_cb *fences;

+};

+extern const struct fence_ops fence_collection_ops;

+/**

• • to_fence_collection - cast a fence to a fence_collection
• • @fence: fence to cast to a fence_collection
• • Returns NULL if the fence is not a fence_collection,
• • or the fence_collection otherwise.
• */

+static inline struct fence_collection *to_fence_collection(struct fence *fence) +{

• if (fence->ops != &fence_collection_ops)

• return NULL;
  

• return container_of(fence, struct fence_collection, base);

+}

+struct fence_collection *fence_collection_create(int num_fences,

• 				struct fence_collection_cb *cb);
  

+#endif /* __LINUX_FENCE_COLLECTION_H */ diff --git a/include/linux/fence.h b/include/linux/fence.h index 5aa95eb..184ce79 100644 --- a/include/linux/fence.h +++ b/include/linux/fence.h @@ -30,6 +30,9 @@ #include <linux/printk.h> #include <linux/rcupdate.h>

+/* context number for fences without context, e.g: fence_collection */ +#define FENCE_NO_CONTEXT 0

struct fence; struct fence_ops; struct fence_cb; @@ -292,6 +295,9 @@ static inline bool fence_is_later(struct fence *f1, struct fence *f2) if (WARN_ON(f1->context != f2->context)) return false;

• if (WARN_ON(f1->context == FENCE_NO_CONTEXT))

• return false;
  

• return (int)(f1->seqno - f2->seqno) > 0;

}

@@ -309,6 +315,9 @@ static inline struct fence *fence_later(struct fence *f1, struct fence *f2) if (WARN_ON(f1->context != f2->context)) return NULL;

• if (WARN_ON(f1->context == FENCE_NO_CONTEXT))

• return NULL;
  

• /*
  • can't check just FENCE_FLAG_SIGNALED_BIT here, it may never have been
  • set if enable_signaling wasn't called, and enabling that here is

-- 2.5.5