linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage
@ 2016-01-27 13:30 Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file Gustavo Padovan
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

This patch series de-stage the sync framework and it a follow up on the
clean up series I've sent last week:

http://thread.gmane.org/gmane.comp.video.dri.devel/145509

Now in part 2 we finish the de-stage of the sync framework. It start with the
move of sync_file from staging to drivers/dma-buf, followed by a bunch of clean
ups on sync_timeline and sw_sync. Finally we de-stage the later two plus the
debug routines.

v2: use git format-patch -M

Gustavo Padovan (11):
  dma-buf/sync_file: de-stage sync_file
  staging/android: store last signaled value on sync timeline
  staging/android: remove .fill_driver_data() timeline ops
  staging/android: remove .{fence,timeline}_value_str() from
    timeline_ops
  staging/android: remove struct sync_timeline_ops
  staging/android: remove sw_sync_timeline and sw_sync_pt
  staging/android: remove sw_sync.[ch] files
  staging/android: rename android_fence to timeline_fence
  dma-buf/sync_timeline: de-stage sync_timeline
  dma-buf/sync_file: bring debug back to sync file
  dma-buf/sync_file: bring sync_dump() back

 drivers/Kconfig                                    |   2 +
 drivers/dma-buf/Kconfig                            |  21 ++
 drivers/dma-buf/Makefile                           |   2 +
 .../{staging/android/uapi => dma-buf}/sw_sync.h    |   0
 drivers/{staging/android => dma-buf}/sync_debug.c  |  40 ++--
 drivers/dma-buf/sync_debug.h                       |  35 +++
 .../android/sync.c => dma-buf/sync_file.c}         | 245 ++-----------------
 drivers/dma-buf/sync_timeline.c                    | 222 ++++++++++++++++++
 drivers/staging/android/Kconfig                    |  19 --
 drivers/staging/android/Makefile                   |   1 -
 drivers/staging/android/sw_sync.c                  | 103 --------
 drivers/staging/android/sw_sync.h                  |  59 -----
 drivers/staging/android/sync.h                     | 261 ---------------------
 include/linux/sync_file.h                          | 123 ++++++++++
 include/linux/sync_timeline.h                      | 114 +++++++++
 .../sync.h => include/trace/events/sync_file.h     |  35 +--
 include/trace/events/sync_timeline.h               |  31 +++
 .../android/uapi => include/uapi/linux}/sync.h     |  12 +-
 18 files changed, 604 insertions(+), 721 deletions(-)
 create mode 100644 drivers/dma-buf/Kconfig
 rename drivers/{staging/android/uapi => dma-buf}/sw_sync.h (100%)
 rename drivers/{staging/android => dma-buf}/sync_debug.c (91%)
 create mode 100644 drivers/dma-buf/sync_debug.h
 rename drivers/{staging/android/sync.c => dma-buf/sync_file.c} (62%)
 create mode 100644 drivers/dma-buf/sync_timeline.c
 delete mode 100644 drivers/staging/android/sw_sync.c
 delete mode 100644 drivers/staging/android/sw_sync.h
 delete mode 100644 drivers/staging/android/sync.h
 create mode 100644 include/linux/sync_file.h
 create mode 100644 include/linux/sync_timeline.h
 rename drivers/staging/android/trace/sync.h => include/trace/events/sync_file.h (58%)
 create mode 100644 include/trace/events/sync_timeline.h
 rename {drivers/staging/android/uapi => include/uapi/linux}/sync.h (90%)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 14:31   ` Maarten Lankhorst
  2016-01-27 13:30 ` [PATCH v2 02/11] staging/android: store last signaled value on sync timeline Gustavo Padovan
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

sync_file is useful to connect one or more fences to the file. The file is
used by userspace to track fences.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/Kconfig                                    |   2 +
 drivers/dma-buf/Kconfig                            |  11 +
 drivers/dma-buf/Makefile                           |   1 +
 drivers/dma-buf/sync_file.c                        | 440 +++++++++++++++++++++
 drivers/staging/android/Kconfig                    |   1 +
 drivers/staging/android/sync.c                     | 419 --------------------
 drivers/staging/android/sync.h                     | 105 +----
 drivers/staging/android/sync_debug.c               |   1 +
 drivers/staging/android/trace/sync.h               |  44 ---
 include/linux/sync_file.h                          | 123 ++++++
 include/trace/events/sync_file.h                   |  57 +++
 .../android/uapi => include/uapi/linux}/sync.h     |  12 +-
 12 files changed, 644 insertions(+), 572 deletions(-)
 create mode 100644 drivers/dma-buf/Kconfig
 create mode 100644 drivers/dma-buf/sync_file.c
 create mode 100644 include/linux/sync_file.h
 create mode 100644 include/trace/events/sync_file.h
 rename {drivers/staging/android/uapi => include/uapi/linux}/sync.h (90%)

diff --git a/drivers/Kconfig b/drivers/Kconfig
index d2ac339..430f761 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -114,6 +114,8 @@ source "drivers/rtc/Kconfig"
 
 source "drivers/dma/Kconfig"
 
+source "drivers/dma-buf/Kconfig"
+
 source "drivers/dca/Kconfig"
 
 source "drivers/auxdisplay/Kconfig"
diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
new file mode 100644
index 0000000..9824bc4
--- /dev/null
+++ b/drivers/dma-buf/Kconfig
@@ -0,0 +1,11 @@
+menu "DMABUF options"
+
+config SYNC_FILE
+	bool "sync_file support for fences"
+	default n
+	select ANON_INODES
+	select DMA_SHARED_BUFFER
+	---help---
+	  This option enables the fence framework synchronization to export
+	  sync_files to userspace that can represent one or more fences.
+endmenu
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 57a675f..4a424ec 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1 +1,2 @@
 obj-y := dma-buf.o fence.o reservation.o seqno-fence.o
+obj-$(CONFIG_SYNC_FILE)		+= sync_file.o
diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
new file mode 100644
index 0000000..92474dd
--- /dev/null
+++ b/drivers/dma-buf/sync_file.c
@@ -0,0 +1,440 @@
+/*
+ * drivers/dma-buf/sync_file.c
+ *
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * 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/file.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/poll.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/anon_inodes.h>
+#include <linux/sync_file.h>
+#include <uapi/linux/sync.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/sync_file.h>
+
+static const struct file_operations sync_file_fops;
+
+static struct sync_file *sync_file_alloc(int size, const char *name)
+{
+	struct sync_file *sync_file;
+
+	sync_file = kzalloc(size, GFP_KERNEL);
+	if (!sync_file)
+		return NULL;
+
+	sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops,
+					     sync_file, 0);
+	if (IS_ERR(sync_file->file))
+		goto err;
+
+	kref_init(&sync_file->kref);
+	strlcpy(sync_file->name, name, sizeof(sync_file->name));
+
+	init_waitqueue_head(&sync_file->wq);
+
+	return sync_file;
+
+err:
+	kfree(sync_file);
+	return NULL;
+}
+
+static void fence_check_cb_func(struct fence *f, struct fence_cb *cb)
+{
+	struct sync_file_cb *check;
+	struct sync_file *sync_file;
+
+	check = container_of(cb, struct sync_file_cb, cb);
+	sync_file = check->sync_file;
+
+	if (atomic_dec_and_test(&sync_file->status))
+		wake_up_all(&sync_file->wq);
+}
+
+/* TODO: implement a create which takes more that one fence */
+struct sync_file *sync_file_create(const char *name, struct fence *fence)
+{
+	struct sync_file *sync_file;
+
+	sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]),
+				    name);
+	if (!sync_file)
+		return NULL;
+
+	sync_file->num_fences = 1;
+	atomic_set(&sync_file->status, 1);
+
+	sync_file->cbs[0].fence = fence;
+	sync_file->cbs[0].sync_file = sync_file;
+	if (fence_add_callback(fence, &sync_file->cbs[0].cb,
+			       fence_check_cb_func))
+		atomic_dec(&sync_file->status);
+
+	return sync_file;
+}
+EXPORT_SYMBOL(sync_file_create);
+
+struct sync_file *sync_file_fdget(int fd)
+{
+	struct file *file = fget(fd);
+
+	if (!file)
+		return NULL;
+
+	if (file->f_op != &sync_file_fops)
+		goto err;
+
+	return file->private_data;
+
+err:
+	fput(file);
+	return NULL;
+}
+EXPORT_SYMBOL(sync_file_fdget);
+
+void sync_file_put(struct sync_file *sync_file)
+{
+	fput(sync_file->file);
+}
+EXPORT_SYMBOL(sync_file_put);
+
+void sync_file_install(struct sync_file *sync_file, int fd)
+{
+	fd_install(fd, sync_file->file);
+}
+EXPORT_SYMBOL(sync_file_install);
+
+static void sync_file_add_fence(struct sync_file *sync_file, int *i,
+				struct fence *fence)
+{
+	sync_file->cbs[*i].fence = fence;
+	sync_file->cbs[*i].sync_file = sync_file;
+
+	if (!fence_add_callback(fence, &sync_file->cbs[*i].cb,
+				fence_check_cb_func)) {
+		fence_get(fence);
+		(*i)++;
+	}
+}
+
+struct sync_file *sync_file_merge(const char *name,
+				  struct sync_file *a, struct sync_file *b)
+{
+	int num_fences = a->num_fences + b->num_fences;
+	struct sync_file *sync_file;
+	int i, i_a, i_b;
+	unsigned long size = offsetof(struct sync_file, cbs[num_fences]);
+
+	sync_file = sync_file_alloc(size, name);
+	if (!sync_file)
+		return NULL;
+
+	atomic_set(&sync_file->status, num_fences);
+
+	/*
+	 * Assume sync_file a and b are both ordered and have no
+	 * duplicates with the same context.
+	 *
+	 * If a sync_file can only be created with sync_file_merge
+	 * and sync_file_create, this is a reasonable assumption.
+	 */
+	for (i = i_a = i_b = 0; i_a < a->num_fences && i_b < b->num_fences; ) {
+		struct fence *pt_a = a->cbs[i_a].fence;
+		struct fence *pt_b = b->cbs[i_b].fence;
+
+		if (pt_a->context < pt_b->context) {
+			sync_file_add_fence(sync_file, &i, pt_a);
+
+			i_a++;
+		} else if (pt_a->context > pt_b->context) {
+			sync_file_add_fence(sync_file, &i, pt_b);
+
+			i_b++;
+		} else {
+			if (pt_a->seqno - pt_b->seqno <= INT_MAX)
+				sync_file_add_fence(sync_file, &i, pt_a);
+			else
+				sync_file_add_fence(sync_file, &i, pt_b);
+
+			i_a++;
+			i_b++;
+		}
+	}
+
+	for (; i_a < a->num_fences; i_a++)
+		sync_file_add_fence(sync_file, &i, a->cbs[i_a].fence);
+
+	for (; i_b < b->num_fences; i_b++)
+		sync_file_add_fence(sync_file, &i, b->cbs[i_b].fence);
+
+	if (num_fences > i)
+		atomic_sub(num_fences - i, &sync_file->status);
+	sync_file->num_fences = i;
+
+	return sync_file;
+}
+EXPORT_SYMBOL(sync_file_merge);
+
+int sync_file_wait(struct sync_file *sync_file, long timeout)
+{
+	long ret;
+	int i;
+
+	if (timeout < 0)
+		timeout = MAX_SCHEDULE_TIMEOUT;
+	else
+		timeout = msecs_to_jiffies(timeout);
+
+	trace_sync_wait(sync_file, 1);
+	for (i = 0; i < sync_file->num_fences; ++i)
+		trace_fence(sync_file->cbs[i].fence);
+	ret = wait_event_interruptible_timeout(sync_file->wq,
+					       atomic_read(&sync_file->status) <= 0,
+					       timeout);
+	trace_sync_wait(sync_file, 0);
+
+	if (ret < 0) {
+		return ret;
+	} else if (ret == 0) {
+		if (timeout)
+			pr_info("sync_file timeout on [%p] after %dms\n",
+				sync_file, jiffies_to_msecs(timeout));
+		return -ETIME;
+	}
+
+	ret = atomic_read(&sync_file->status);
+	if (ret)
+		pr_info("sync_file error %ld on [%p]\n", ret, sync_file);
+
+	return ret;
+}
+EXPORT_SYMBOL(sync_file_wait);
+
+static void sync_file_free(struct kref *kref)
+{
+	struct sync_file *sync_file = container_of(kref, struct sync_file,
+						     kref);
+	int i;
+
+	for (i = 0; i < sync_file->num_fences; ++i) {
+		fence_remove_callback(sync_file->cbs[i].fence,
+				      &sync_file->cbs[i].cb);
+		fence_put(sync_file->cbs[i].fence);
+	}
+
+	kfree(sync_file);
+}
+
+static int sync_file_release(struct inode *inode, struct file *file)
+{
+	struct sync_file *sync_file = file->private_data;
+
+	kref_put(&sync_file->kref, sync_file_free);
+	return 0;
+}
+
+static unsigned int sync_file_poll(struct file *file, poll_table *wait)
+{
+	struct sync_file *sync_file = file->private_data;
+	int status;
+
+	poll_wait(file, &sync_file->wq, wait);
+
+	status = atomic_read(&sync_file->status);
+
+	if (!status)
+		return POLLIN;
+	else if (status < 0)
+		return POLLERR;
+	return 0;
+}
+
+static long sync_file_ioctl_wait(struct sync_file *sync_file,
+				 unsigned long arg)
+{
+	__s32 value;
+
+	if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
+		return -EFAULT;
+
+	return sync_file_wait(sync_file, value);
+}
+
+static long sync_file_ioctl_merge(struct sync_file *sync_file,
+				  unsigned long arg)
+{
+	int fd = get_unused_fd_flags(O_CLOEXEC);
+	int err;
+	struct sync_file *fence2, *fence3;
+	struct sync_merge_data data;
+
+	if (fd < 0)
+		return fd;
+
+	if (copy_from_user(&data, (void __user *)arg, sizeof(data))) {
+		err = -EFAULT;
+		goto err_put_fd;
+	}
+
+	fence2 = sync_file_fdget(data.fd2);
+	if (!fence2) {
+		err = -ENOENT;
+		goto err_put_fd;
+	}
+
+	data.name[sizeof(data.name) - 1] = '\0';
+	fence3 = sync_file_merge(data.name, sync_file, fence2);
+	if (!fence3) {
+		err = -ENOMEM;
+		goto err_put_fence2;
+	}
+
+	data.fence = fd;
+	if (copy_to_user((void __user *)arg, &data, sizeof(data))) {
+		err = -EFAULT;
+		goto err_put_fence3;
+	}
+
+	sync_file_install(fence3, fd);
+	sync_file_put(fence2);
+	return 0;
+
+err_put_fence3:
+	sync_file_put(fence3);
+
+err_put_fence2:
+	sync_file_put(fence2);
+
+err_put_fd:
+	put_unused_fd(fd);
+	return err;
+}
+
+static int sync_fill_pt_info(struct fence *fence, void *data, int size)
+{
+	struct fence_info *info = data;
+	int ret;
+
+	if (size < sizeof(struct fence_info))
+		return -ENOMEM;
+
+	info->len = sizeof(struct fence_info);
+
+	if (fence->ops->fill_driver_data) {
+		ret = fence->ops->fill_driver_data(fence, info->driver_data,
+						   size - sizeof(*info));
+		if (ret < 0)
+			return ret;
+
+		info->len += ret;
+	}
+
+	strlcpy(info->obj_name, fence->ops->get_timeline_name(fence),
+		sizeof(info->obj_name));
+	strlcpy(info->driver_name, fence->ops->get_driver_name(fence),
+		sizeof(info->driver_name));
+	if (fence_is_signaled(fence))
+		info->status = fence->status >= 0 ? 1 : fence->status;
+	else
+		info->status = 0;
+	info->timestamp_ns = ktime_to_ns(fence->timestamp);
+
+	return info->len;
+}
+
+static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
+				       unsigned long arg)
+{
+	struct sync_file_info_data *data;
+	__u32 size;
+	__u32 len = 0;
+	int ret, i;
+
+	if (copy_from_user(&size, (void __user *)arg, sizeof(size)))
+		return -EFAULT;
+
+	if (size < sizeof(struct sync_file_info_data))
+		return -EINVAL;
+
+	if (size > 4096)
+		size = 4096;
+
+	data = kzalloc(size, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	strlcpy(data->name, sync_file->name, sizeof(data->name));
+	data->status = atomic_read(&sync_file->status);
+	if (data->status >= 0)
+		data->status = !data->status;
+
+	len = sizeof(struct sync_file_info_data);
+
+	for (i = 0; i < sync_file->num_fences; ++i) {
+		struct fence *fence = sync_file->cbs[i].fence;
+
+		ret = sync_fill_pt_info(fence, (u8 *)data + len, size - len);
+
+		if (ret < 0)
+			goto out;
+
+		len += ret;
+	}
+
+	data->len = len;
+
+	if (copy_to_user((void __user *)arg, data, len))
+		ret = -EFAULT;
+	else
+		ret = 0;
+
+out:
+	kfree(data);
+
+	return ret;
+}
+
+static long sync_file_ioctl(struct file *file, unsigned int cmd,
+			    unsigned long arg)
+{
+	struct sync_file *sync_file = file->private_data;
+
+	switch (cmd) {
+	case SYNC_IOC_WAIT:
+		return sync_file_ioctl_wait(sync_file, arg);
+
+	case SYNC_IOC_MERGE:
+		return sync_file_ioctl_merge(sync_file, arg);
+
+	case SYNC_IOC_FENCE_INFO:
+		return sync_file_ioctl_fence_info(sync_file, arg);
+
+	default:
+		return -ENOTTY;
+	}
+}
+
+static const struct file_operations sync_file_fops = {
+	.release = sync_file_release,
+	.poll = sync_file_poll,
+	.unlocked_ioctl = sync_file_ioctl,
+	.compat_ioctl = sync_file_ioctl,
+};
+
diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index bd90d20..2756988 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -52,6 +52,7 @@ config SW_SYNC
 	bool "Software synchronization objects"
 	default n
 	depends on SYNC
+	depends on SYNC_FILE
 	---help---
 	  A sync object driver that uses a 32bit counter to coordinate
 	  synchronization.  Useful when there is no hardware primitive backing
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 5fa4779..1e1c009 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -16,10 +16,7 @@
 
 #include <linux/debugfs.h>
 #include <linux/export.h>
-#include <linux/file.h>
-#include <linux/fs.h>
 #include <linux/kernel.h>
-#include <linux/poll.h>
 #include <linux/sched.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
@@ -32,7 +29,6 @@
 #include "trace/sync.h"
 
 static const struct fence_ops android_fence_ops;
-static const struct file_operations sync_file_fops;
 
 struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
 					   int size, const char *name)
@@ -136,208 +132,6 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size)
 }
 EXPORT_SYMBOL(sync_pt_create);
 
-static struct sync_file *sync_file_alloc(int size, const char *name)
-{
-	struct sync_file *sync_file;
-
-	sync_file = kzalloc(size, GFP_KERNEL);
-	if (!sync_file)
-		return NULL;
-
-	sync_file->file = anon_inode_getfile("sync_file", &sync_file_fops,
-					     sync_file, 0);
-	if (IS_ERR(sync_file->file))
-		goto err;
-
-	kref_init(&sync_file->kref);
-	strlcpy(sync_file->name, name, sizeof(sync_file->name));
-
-	init_waitqueue_head(&sync_file->wq);
-
-	return sync_file;
-
-err:
-	kfree(sync_file);
-	return NULL;
-}
-
-static void fence_check_cb_func(struct fence *f, struct fence_cb *cb)
-{
-	struct sync_file_cb *check;
-	struct sync_file *sync_file;
-
-	check = container_of(cb, struct sync_file_cb, cb);
-	sync_file = check->sync_file;
-
-	if (atomic_dec_and_test(&sync_file->status))
-		wake_up_all(&sync_file->wq);
-}
-
-/* TODO: implement a create which takes more that one fence */
-struct sync_file *sync_file_create(const char *name, struct fence *fence)
-{
-	struct sync_file *sync_file;
-
-	sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]),
-				    name);
-	if (!sync_file)
-		return NULL;
-
-	sync_file->num_fences = 1;
-	atomic_set(&sync_file->status, 1);
-
-	sync_file->cbs[0].fence = fence;
-	sync_file->cbs[0].sync_file = sync_file;
-	if (fence_add_callback(fence, &sync_file->cbs[0].cb,
-			       fence_check_cb_func))
-		atomic_dec(&sync_file->status);
-
-	sync_file_debug_add(sync_file);
-
-	return sync_file;
-}
-EXPORT_SYMBOL(sync_file_create);
-
-struct sync_file *sync_file_fdget(int fd)
-{
-	struct file *file = fget(fd);
-
-	if (!file)
-		return NULL;
-
-	if (file->f_op != &sync_file_fops)
-		goto err;
-
-	return file->private_data;
-
-err:
-	fput(file);
-	return NULL;
-}
-EXPORT_SYMBOL(sync_file_fdget);
-
-void sync_file_put(struct sync_file *sync_file)
-{
-	fput(sync_file->file);
-}
-EXPORT_SYMBOL(sync_file_put);
-
-void sync_file_install(struct sync_file *sync_file, int fd)
-{
-	fd_install(fd, sync_file->file);
-}
-EXPORT_SYMBOL(sync_file_install);
-
-static void sync_file_add_pt(struct sync_file *sync_file, int *i,
-			     struct fence *fence)
-{
-	sync_file->cbs[*i].fence = fence;
-	sync_file->cbs[*i].sync_file = sync_file;
-
-	if (!fence_add_callback(fence, &sync_file->cbs[*i].cb,
-				fence_check_cb_func)) {
-		fence_get(fence);
-		(*i)++;
-	}
-}
-
-struct sync_file *sync_file_merge(const char *name,
-				  struct sync_file *a, struct sync_file *b)
-{
-	int num_fences = a->num_fences + b->num_fences;
-	struct sync_file *sync_file;
-	int i, i_a, i_b;
-	unsigned long size = offsetof(struct sync_file, cbs[num_fences]);
-
-	sync_file = sync_file_alloc(size, name);
-	if (!sync_file)
-		return NULL;
-
-	atomic_set(&sync_file->status, num_fences);
-
-	/*
-	 * Assume sync_file a and b are both ordered and have no
-	 * duplicates with the same context.
-	 *
-	 * If a sync_file can only be created with sync_file_merge
-	 * and sync_file_create, this is a reasonable assumption.
-	 */
-	for (i = i_a = i_b = 0; i_a < a->num_fences && i_b < b->num_fences; ) {
-		struct fence *pt_a = a->cbs[i_a].fence;
-		struct fence *pt_b = b->cbs[i_b].fence;
-
-		if (pt_a->context < pt_b->context) {
-			sync_file_add_pt(sync_file, &i, pt_a);
-
-			i_a++;
-		} else if (pt_a->context > pt_b->context) {
-			sync_file_add_pt(sync_file, &i, pt_b);
-
-			i_b++;
-		} else {
-			if (pt_a->seqno - pt_b->seqno <= INT_MAX)
-				sync_file_add_pt(sync_file, &i, pt_a);
-			else
-				sync_file_add_pt(sync_file, &i, pt_b);
-
-			i_a++;
-			i_b++;
-		}
-	}
-
-	for (; i_a < a->num_fences; i_a++)
-		sync_file_add_pt(sync_file, &i, a->cbs[i_a].fence);
-
-	for (; i_b < b->num_fences; i_b++)
-		sync_file_add_pt(sync_file, &i, b->cbs[i_b].fence);
-
-	if (num_fences > i)
-		atomic_sub(num_fences - i, &sync_file->status);
-	sync_file->num_fences = i;
-
-	sync_file_debug_add(sync_file);
-	return sync_file;
-}
-EXPORT_SYMBOL(sync_file_merge);
-
-int sync_file_wait(struct sync_file *sync_file, long timeout)
-{
-	long ret;
-	int i;
-
-	if (timeout < 0)
-		timeout = MAX_SCHEDULE_TIMEOUT;
-	else
-		timeout = msecs_to_jiffies(timeout);
-
-	trace_sync_wait(sync_file, 1);
-	for (i = 0; i < sync_file->num_fences; ++i)
-		trace_fence(sync_file->cbs[i].fence);
-	ret = wait_event_interruptible_timeout(sync_file->wq,
-					       atomic_read(&sync_file->status) <= 0,
-					       timeout);
-	trace_sync_wait(sync_file, 0);
-
-	if (ret < 0) {
-		return ret;
-	} else if (ret == 0) {
-		if (timeout) {
-			pr_info("sync_file timeout on [%p] after %dms\n",
-				sync_file, jiffies_to_msecs(timeout));
-			sync_dump();
-		}
-		return -ETIME;
-	}
-
-	ret = atomic_read(&sync_file->status);
-	if (ret) {
-		pr_info("sync_file error %ld on [%p]\n", ret, sync_file);
-		sync_dump();
-	}
-	return ret;
-}
-EXPORT_SYMBOL(sync_file_wait);
-
 static const char *android_fence_get_driver_name(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
@@ -437,216 +231,3 @@ static const struct fence_ops android_fence_ops = {
 	.timeline_value_str = android_fence_timeline_value_str,
 };
 
-static void sync_file_free(struct kref *kref)
-{
-	struct sync_file *sync_file = container_of(kref, struct sync_file,
-						     kref);
-	int i;
-
-	for (i = 0; i < sync_file->num_fences; ++i) {
-		fence_remove_callback(sync_file->cbs[i].fence,
-				      &sync_file->cbs[i].cb);
-		fence_put(sync_file->cbs[i].fence);
-	}
-
-	kfree(sync_file);
-}
-
-static int sync_file_release(struct inode *inode, struct file *file)
-{
-	struct sync_file *sync_file = file->private_data;
-
-	sync_file_debug_remove(sync_file);
-
-	kref_put(&sync_file->kref, sync_file_free);
-	return 0;
-}
-
-static unsigned int sync_file_poll(struct file *file, poll_table *wait)
-{
-	struct sync_file *sync_file = file->private_data;
-	int status;
-
-	poll_wait(file, &sync_file->wq, wait);
-
-	status = atomic_read(&sync_file->status);
-
-	if (!status)
-		return POLLIN;
-	else if (status < 0)
-		return POLLERR;
-	return 0;
-}
-
-static long sync_file_ioctl_wait(struct sync_file *sync_file,
-				  unsigned long arg)
-{
-	__s32 value;
-
-	if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
-		return -EFAULT;
-
-	return sync_file_wait(sync_file, value);
-}
-
-static long sync_file_ioctl_merge(struct sync_file *sync_file,
-				   unsigned long arg)
-{
-	int fd = get_unused_fd_flags(O_CLOEXEC);
-	int err;
-	struct sync_file *fence2, *fence3;
-	struct sync_merge_data data;
-
-	if (fd < 0)
-		return fd;
-
-	if (copy_from_user(&data, (void __user *)arg, sizeof(data))) {
-		err = -EFAULT;
-		goto err_put_fd;
-	}
-
-	fence2 = sync_file_fdget(data.fd2);
-	if (!fence2) {
-		err = -ENOENT;
-		goto err_put_fd;
-	}
-
-	data.name[sizeof(data.name) - 1] = '\0';
-	fence3 = sync_file_merge(data.name, sync_file, fence2);
-	if (!fence3) {
-		err = -ENOMEM;
-		goto err_put_fence2;
-	}
-
-	data.fence = fd;
-	if (copy_to_user((void __user *)arg, &data, sizeof(data))) {
-		err = -EFAULT;
-		goto err_put_fence3;
-	}
-
-	sync_file_install(fence3, fd);
-	sync_file_put(fence2);
-	return 0;
-
-err_put_fence3:
-	sync_file_put(fence3);
-
-err_put_fence2:
-	sync_file_put(fence2);
-
-err_put_fd:
-	put_unused_fd(fd);
-	return err;
-}
-
-static int sync_fill_pt_info(struct fence *fence, void *data, int size)
-{
-	struct sync_pt_info *info = data;
-	int ret;
-
-	if (size < sizeof(struct sync_pt_info))
-		return -ENOMEM;
-
-	info->len = sizeof(struct sync_pt_info);
-
-	if (fence->ops->fill_driver_data) {
-		ret = fence->ops->fill_driver_data(fence, info->driver_data,
-						   size - sizeof(*info));
-		if (ret < 0)
-			return ret;
-
-		info->len += ret;
-	}
-
-	strlcpy(info->obj_name, fence->ops->get_timeline_name(fence),
-		sizeof(info->obj_name));
-	strlcpy(info->driver_name, fence->ops->get_driver_name(fence),
-		sizeof(info->driver_name));
-	if (fence_is_signaled(fence))
-		info->status = fence->status >= 0 ? 1 : fence->status;
-	else
-		info->status = 0;
-	info->timestamp_ns = ktime_to_ns(fence->timestamp);
-
-	return info->len;
-}
-
-static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
-					unsigned long arg)
-{
-	struct sync_file_info_data *data;
-	__u32 size;
-	__u32 len = 0;
-	int ret, i;
-
-	if (copy_from_user(&size, (void __user *)arg, sizeof(size)))
-		return -EFAULT;
-
-	if (size < sizeof(struct sync_file_info_data))
-		return -EINVAL;
-
-	if (size > 4096)
-		size = 4096;
-
-	data = kzalloc(size, GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
-
-	strlcpy(data->name, sync_file->name, sizeof(data->name));
-	data->status = atomic_read(&sync_file->status);
-	if (data->status >= 0)
-		data->status = !data->status;
-
-	len = sizeof(struct sync_file_info_data);
-
-	for (i = 0; i < sync_file->num_fences; ++i) {
-		struct fence *fence = sync_file->cbs[i].fence;
-
-		ret = sync_fill_pt_info(fence, (u8 *)data + len, size - len);
-
-		if (ret < 0)
-			goto out;
-
-		len += ret;
-	}
-
-	data->len = len;
-
-	if (copy_to_user((void __user *)arg, data, len))
-		ret = -EFAULT;
-	else
-		ret = 0;
-
-out:
-	kfree(data);
-
-	return ret;
-}
-
-static long sync_file_ioctl(struct file *file, unsigned int cmd,
-			     unsigned long arg)
-{
-	struct sync_file *sync_file = file->private_data;
-
-	switch (cmd) {
-	case SYNC_IOC_WAIT:
-		return sync_file_ioctl_wait(sync_file, arg);
-
-	case SYNC_IOC_MERGE:
-		return sync_file_ioctl_merge(sync_file, arg);
-
-	case SYNC_IOC_FENCE_INFO:
-		return sync_file_ioctl_fence_info(sync_file, arg);
-
-	default:
-		return -ENOTTY;
-	}
-}
-
-static const struct file_operations sync_file_fops = {
-	.release = sync_file_release,
-	.poll = sync_file_poll,
-	.unlocked_ioctl = sync_file_ioctl,
-	.compat_ioctl = sync_file_ioctl,
-};
-
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 8980b55..fb209fc 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -18,13 +18,11 @@
 #include <linux/ktime.h>
 #include <linux/list.h>
 #include <linux/spinlock.h>
-#include <linux/wait.h>
 #include <linux/fence.h>
-
-#include "uapi/sync.h"
+#include <linux/sync_file.h>
+#include <uapi/linux/sync.h>
 
 struct sync_timeline;
-struct sync_file;
 
 /**
  * struct sync_timeline_ops - sync object implementation ops
@@ -94,38 +92,6 @@ static inline struct sync_timeline *fence_parent(struct fence *fence)
 			    child_list_lock);
 }
 
-struct sync_file_cb {
-	struct fence_cb cb;
-	struct fence *fence;
-	struct sync_file *sync_file;
-};
-
-/**
- * struct sync_file - sync file to export to the userspace
- * @file:		file representing this fence
- * @kref:		reference count on fence.
- * @name:		name of sync_file.  Useful for debugging
- * @sync_file_list:	membership in global file list
- * @num_fences		number of sync_pts in the fence
- * @wq:			wait queue for fence signaling
- * @status:		0: signaled, >0:active, <0: error
- * @cbs:		sync_pts callback information
- */
-struct sync_file {
-	struct file		*file;
-	struct kref		kref;
-	char			name[32];
-#ifdef CONFIG_DEBUG_FS
-	struct list_head	sync_file_list;
-#endif
-	int num_fences;
-
-	wait_queue_head_t	wq;
-	atomic_t		status;
-
-	struct sync_file_cb	cbs[];
-};
-
 /*
  * API for sync_timeline implementers
  */
@@ -175,73 +141,6 @@ void sync_timeline_signal(struct sync_timeline *obj);
  */
 struct fence *sync_pt_create(struct sync_timeline *parent, int size);
 
-/**
- * sync_fence_create() - creates a sync fence
- * @name:	name of fence to create
- * @fence:	fence to add to the sync_fence
- *
- * Creates a sync_file containg @fence. Once this is called, the sync_file
- * takes ownership of @fence.
- */
-struct sync_file *sync_file_create(const char *name, struct fence *fence);
-
-/*
- * API for sync_file consumers
- */
-
-/**
- * sync_file_merge() - merge two sync_files
- * @name:	name of new fence
- * @a:		sync_file a
- * @b:		sync_file b
- *
- * Creates a new sync_file which contains copies of all the fences in both
- * @a and @b.  @a and @b remain valid, independent sync_file. Returns the
- * new merged sync_file or NULL in case of error.
- */
-struct sync_file *sync_file_merge(const char *name,
-				    struct sync_file *a, struct sync_file *b);
-
-/**
- * sync_file_fdget() - get a sync_file from an fd
- * @fd:		fd referencing a fence
- *
- * Ensures @fd references a valid sync_file, increments the refcount of the
- * backing file. Returns the sync_file or NULL in case of error.
- */
-struct sync_file *sync_file_fdget(int fd);
-
-/**
- * sync_file_put() - puts a reference of a sync_file
- * @sync_file:	sync_file to put
- *
- * Puts a reference on @sync_fence.  If this is the last reference, the
- * sync_fil and all it's sync_pts will be freed
- */
-void sync_file_put(struct sync_file *sync_file);
-
-/**
- * sync_file_install() - installs a sync_file into a file descriptor
- * @sync_file:	sync_file to install
- * @fd:		file descriptor in which to install the fence
- *
- * Installs @sync_file into @fd.  @fd's should be acquired through
- * get_unused_fd_flags(O_CLOEXEC).
- */
-void sync_file_install(struct sync_file *sync_file, int fd);
-
-/**
- * sync_file_wait() - wait on sync file
- * @sync_file:	file to wait on
- * @tiemout:	timeout in ms
- *
- * Wait for @sync_file to be signaled or have an error. Waits indefinitely
- * if @timeout < 0.
- *
- * Returns 0 if fence signaled, > 0 if it is still active and <0 on error
- */
-int sync_file_wait(struct sync_file *sync_file, long timeout);
-
 #ifdef CONFIG_DEBUG_FS
 
 void sync_timeline_debug_add(struct sync_timeline *obj);
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index 7e53da7..b37412d 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -27,6 +27,7 @@
 #include <linux/uaccess.h>
 #include <linux/anon_inodes.h>
 #include <linux/time64.h>
+#include <linux/sync_file.h>
 #include "sw_sync.h"
 
 #ifdef CONFIG_DEBUG_FS
diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/android/trace/sync.h
index 87c60e9..a0f80f4 100644
--- a/drivers/staging/android/trace/sync.h
+++ b/drivers/staging/android/trace/sync.h
@@ -32,50 +32,6 @@ TRACE_EVENT(sync_timeline,
 	TP_printk("name=%s value=%s", __get_str(name), __entry->value)
 );
 
-TRACE_EVENT(sync_wait,
-	TP_PROTO(struct sync_file *sync_file, int begin),
-
-	TP_ARGS(sync_file, begin),
-
-	TP_STRUCT__entry(
-			__string(name, sync_file->name)
-			__field(s32, status)
-			__field(u32, begin)
-	),
-
-	TP_fast_assign(
-			__assign_str(name, sync_file->name);
-			__entry->status = atomic_read(&sync_file->status);
-			__entry->begin = begin;
-	),
-
-	TP_printk("%s name=%s state=%d", __entry->begin ? "begin" : "end",
-			__get_str(name), __entry->status)
-);
-
-TRACE_EVENT(fence,
-	TP_PROTO(struct fence *fence),
-
-	TP_ARGS(fence),
-
-	TP_STRUCT__entry(
-		__string(timeline, fence->ops->get_timeline_name(fence))
-		__array(char, value, 32)
-	),
-
-	TP_fast_assign(
-		__assign_str(timeline, fence->ops->get_timeline_name(fence));
-		if (fence->ops->fence_value_str) {
-			fence->ops->fence_value_str(fence, __entry->value,
-							sizeof(__entry->value));
-		} else {
-			__entry->value[0] = '\0';
-		}
-	),
-
-	TP_printk("name=%s value=%s", __get_str(timeline), __entry->value)
-);
-
 #endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
 
 /* This part must be outside protection */
diff --git a/include/linux/sync_file.h b/include/linux/sync_file.h
new file mode 100644
index 0000000..ac2b4c6
--- /dev/null
+++ b/include/linux/sync_file.h
@@ -0,0 +1,123 @@
+/*
+ * include/linux/sync_file.h
+ *
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * 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_SYNC_FILE_H
+#define _LINUX_SYNC_FILE_H
+
+#include <linux/types.h>
+#include <linux/kref.h>
+#include <linux/ktime.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/wait.h>
+#include <linux/fence.h>
+
+struct sync_file_cb {
+	struct fence_cb cb;
+	struct fence *fence;
+	struct sync_file *sync_file;
+};
+
+/**
+ * struct sync_file - sync file to export to the userspace
+ * @file:		file representing this fence
+ * @kref:		reference count on fence.
+ * @name:		name of sync_file.  Useful for debugging
+ * @sync_file_list:	membership in global file list
+ * @num_fences		number of sync_pts in the fence
+ * @wq:			wait queue for fence signaling
+ * @status:		0: signaled, >0:active, <0: error
+ * @cbs:		sync_pts callback information
+ */
+struct sync_file {
+	struct file		*file;
+	struct kref		kref;
+	char			name[32];
+#ifdef CONFIG_DEBUG_FS
+	struct list_head	sync_file_list;
+#endif
+	int num_fences;
+
+	wait_queue_head_t	wq;
+	atomic_t		status;
+
+	struct sync_file_cb	cbs[];
+};
+
+/**
+ * sync_file_create() - creates a sync fence
+ * @name:	name of fence to create
+ * @fence:	fence to add to the sync_fence
+ *
+ * Creates a sync_file containg @fence. Once this is called, the sync_file
+ * takes ownership of @fence.
+ */
+struct sync_file *sync_file_create(const char *name, struct fence *fence);
+
+/*
+ * API for sync_file consumers
+ */
+
+/**
+ * sync_file_merge() - merge two sync_files
+ * @name:	name of new fence
+ * @a:		sync_file a
+ * @b:		sync_file b
+ *
+ * Creates a new sync_file which contains copies of all the fences in both
+ * @a and @b.  @a and @b remain valid, independent sync_file. Returns the
+ * new merged sync_file or NULL in case of error.
+ */
+struct sync_file *sync_file_merge(const char *name,
+				    struct sync_file *a, struct sync_file *b);
+
+/**
+ * sync_file_fdget() - get a sync_file from an fd
+ * @fd:		fd referencing a fence
+ *
+ * Ensures @fd references a valid sync_file, increments the refcount of the
+ * backing file. Returns the sync_file or NULL in case of error.
+ */
+struct sync_file *sync_file_fdget(int fd);
+
+/**
+ * sync_file_put() - puts a reference of a sync_file
+ * @sync_file:	sync_file to put
+ *
+ * Puts a reference on @sync_fence.  If this is the last reference, the
+ * sync_fil and all it's sync_pts will be freed
+ */
+void sync_file_put(struct sync_file *sync_file);
+
+/**
+ * sync_file_install() - installs a sync_file into a file descriptor
+ * @sync_file:	sync_file to install
+ * @fd:		file descriptor in which to install the fence
+ *
+ * Installs @sync_file into @fd.  @fd's should be acquired through
+ * get_unused_fd_flags(O_CLOEXEC).
+ */
+void sync_file_install(struct sync_file *sync_file, int fd);
+
+/**
+ * sync_file_wait() - wait on sync file
+ * @sync_file:	file to wait on
+ * @tiemout:	timeout in ms
+ *
+ * Wait for @sync_file to be signaled or have an error. Waits indefinitely
+ * if @timeout < 0.
+ *
+ * Returns 0 if fence signaled, > 0 if it is still active and <0 on error
+ */
+int sync_file_wait(struct sync_file *sync_file, long timeout);
+
+#endif /* _LINUX_SYNC_H */
diff --git a/include/trace/events/sync_file.h b/include/trace/events/sync_file.h
new file mode 100644
index 0000000..659b5f3
--- /dev/null
+++ b/include/trace/events/sync_file.h
@@ -0,0 +1,57 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM sync_file
+
+#if !defined(_TRACE_SYNC_FILE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SYNC_FILE_H
+
+#include <linux/sync.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(sync_wait,
+	TP_PROTO(struct sync_file *sync_file, int begin),
+
+	TP_ARGS(sync_file, begin),
+
+	TP_STRUCT__entry(
+			__string(name, sync_file->name)
+			__field(s32, status)
+			__field(u32, begin)
+	),
+
+	TP_fast_assign(
+			__assign_str(name, sync_file->name);
+			__entry->status = atomic_read(&sync_file->status);
+			__entry->begin = begin;
+	),
+
+	TP_printk("%s name=%s state=%d", __entry->begin ? "begin" : "end",
+			__get_str(name), __entry->status)
+);
+
+TRACE_EVENT(fence,
+	TP_PROTO(struct fence *fence),
+
+	TP_ARGS(fence),
+
+	TP_STRUCT__entry(
+		__string(timeline, fence->ops->get_timeline_name(fence))
+		__array(char, value, 32)
+	),
+
+	TP_fast_assign(
+		__assign_str(timeline, fence->ops->get_timeline_name(fence));
+		if (fence->ops->fence_value_str) {
+			fence->ops->fence_value_str(fence, __entry->value,
+							sizeof(__entry->value));
+		} else {
+			__entry->value[0] = '\0';
+		}
+	),
+
+	TP_printk("name=%s value=%s", __get_str(timeline), __entry->value)
+);
+
+#endif /* if !defined(_TRACE_SYNC_FILE_H) || defined(TRACE_HEADER_MULTI_READ) */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/drivers/staging/android/uapi/sync.h b/include/uapi/linux/sync.h
similarity index 90%
rename from drivers/staging/android/uapi/sync.h
rename to include/uapi/linux/sync.h
index 73deb69..2a8ab04 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/include/uapi/linux/sync.h
@@ -27,15 +27,15 @@ struct sync_merge_data {
 };
 
 /**
- * struct sync_pt_info - detailed sync_pt information
- * @len:		length of sync_pt_info including any driver_data
+ * struct fence_info - detailed fence information
+ * @len:		length of fence_info including any driver_data
  * @obj_name:		name of parent sync_timeline
  * @driver_name:	name of driver implementing the parent
- * @status:		status of the sync_pt 0:active 1:signaled <0:error
+ * @status:		status of the fence 0:active 1:signaled <0:error
  * @timestamp_ns:	timestamp of status change in nanoseconds
  * @driver_data:	any driver dependent data
  */
-struct sync_pt_info {
+struct fence_info {
 	__u32	len;
 	char	obj_name[32];
 	char	driver_name[32];
@@ -52,14 +52,14 @@ struct sync_pt_info {
  *		userspace including pt_info.
  * @name:	name of fence
  * @status:	status of fence. 1: signaled 0:active <0:error
- * @pt_info:	a sync_pt_info struct for every sync_pt in the fence
+ * @fence_info:	a fence_info struct for every fence in the sync_file
  */
 struct sync_file_info_data {
 	__u32	len;
 	char	name[32];
 	__s32	status;
 
-	__u8	pt_info[0];
+	__u8	fence_info[0];
 };
 
 #define SYNC_IOC_MAGIC		'>'
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 02/11] staging/android: store last signaled value on sync timeline
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 03/11] staging/android: remove .fill_driver_data() timeline ops Gustavo Padovan
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

Now fence timeline is aware of the last signaled fence, as it
receives the increment to the current value in sync_timeline_signal().

That allow us to remove .has_signaled() from timeline_ops as we can
directly compare using timeline->value and fence->seqno in sync.c

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c | 16 ++--------------
 drivers/staging/android/sync.c    | 15 +++++++--------
 drivers/staging/android/sync.h    | 14 +++++---------
 3 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 3bee959..b9d53d3 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -30,7 +30,7 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
 	struct sw_sync_pt *pt;
 
 	pt = (struct sw_sync_pt *)
-		sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt));
+		sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value);
 
 	pt->value = value;
 
@@ -38,15 +38,6 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
 }
 EXPORT_SYMBOL(sw_sync_pt_create);
 
-static int sw_sync_fence_has_signaled(struct fence *fence)
-{
-	struct sw_sync_pt *pt = (struct sw_sync_pt *)fence;
-	struct sw_sync_timeline *obj =
-		(struct sw_sync_timeline *)fence_parent(fence);
-
-	return (pt->value > obj->value) ? 0 : 1;
-}
-
 static int sw_sync_fill_driver_data(struct fence *fence,
 				    void *data, int size)
 {
@@ -77,7 +68,6 @@ static void sw_sync_fence_value_str(struct fence *fence, char *str, int size)
 
 static struct sync_timeline_ops sw_sync_timeline_ops = {
 	.driver_name = "sw_sync",
-	.has_signaled = sw_sync_fence_has_signaled,
 	.fill_driver_data = sw_sync_fill_driver_data,
 	.timeline_value_str = sw_sync_timeline_value_str,
 	.fence_value_str = sw_sync_fence_value_str,
@@ -96,8 +86,6 @@ EXPORT_SYMBOL(sw_sync_timeline_create);
 
 void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
 {
-	obj->value += inc;
-
-	sync_timeline_signal(&obj->obj);
+	sync_timeline_signal(&obj->obj, inc);
 }
 EXPORT_SYMBOL(sw_sync_timeline_inc);
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 1e1c009..4eea5c3 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -90,7 +90,7 @@ void sync_timeline_destroy(struct sync_timeline *obj)
 }
 EXPORT_SYMBOL(sync_timeline_destroy);
 
-void sync_timeline_signal(struct sync_timeline *obj)
+void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc)
 {
 	unsigned long flags;
 	struct fence *fence, *next;
@@ -99,6 +99,8 @@ void sync_timeline_signal(struct sync_timeline *obj)
 
 	spin_lock_irqsave(&obj->child_list_lock, flags);
 
+	obj->value += inc;
+
 	list_for_each_entry_safe(fence, next, &obj->active_list_head,
 				 active_list) {
 		if (fence_is_signaled_locked(fence))
@@ -109,7 +111,8 @@ void sync_timeline_signal(struct sync_timeline *obj)
 }
 EXPORT_SYMBOL(sync_timeline_signal);
 
-struct fence *sync_pt_create(struct sync_timeline *obj, int size)
+struct fence *sync_pt_create(struct sync_timeline *obj, int size,
+			     unsigned int value)
 {
 	unsigned long flags;
 	struct fence *fence;
@@ -124,7 +127,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size)
 	spin_lock_irqsave(&obj->child_list_lock, flags);
 	sync_timeline_get(obj);
 	fence_init(fence, &android_fence_ops, &obj->child_list_lock,
-		   obj->context, ++obj->value);
+		   obj->context, value);
 	list_add_tail(&fence->child_list, &obj->child_list_head);
 	INIT_LIST_HEAD(&fence->active_list);
 	spin_unlock_irqrestore(&obj->child_list_lock, flags);
@@ -164,12 +167,8 @@ static void android_fence_release(struct fence *fence)
 static bool android_fence_signaled(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
-	int ret;
 
-	ret = parent->ops->has_signaled(fence);
-	if (ret < 0)
-		fence->status = ret;
-	return ret;
+	return (fence->seqno > parent->value) ? false : true;
 }
 
 static bool android_fence_enable_signaling(struct fence *fence)
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index fb209fc..4d3dfbf 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -27,10 +27,6 @@ struct sync_timeline;
 /**
  * struct sync_timeline_ops - sync object implementation ops
  * @driver_name:	name of the implementation
- * @has_signaled:	returns:
- *			  1 if pt has signaled
- *			  0 if pt has not signaled
- *			 <0 on error
  * @fill_driver_data:	write implementation specific driver data to data.
  *			  should return an error if there is not enough room
  *			  as specified by size.  This information is returned
@@ -41,9 +37,6 @@ struct sync_timeline;
 struct sync_timeline_ops {
 	const char *driver_name;
 
-	/* required */
-	int (*has_signaled)(struct fence *fence);
-
 	/* optional */
 	int (*fill_driver_data)(struct fence *fence, void *data, int size);
 
@@ -123,23 +116,26 @@ void sync_timeline_destroy(struct sync_timeline *obj);
 /**
  * sync_timeline_signal() - signal a status change on a sync_timeline
  * @obj:	sync_timeline to signal
+ * @inc:	num to increment on timeline->value
  *
  * A sync implementation should call this any time one of it's fences
  * has signaled or has an error condition.
  */
-void sync_timeline_signal(struct sync_timeline *obj);
+void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc);
 
 /**
  * sync_pt_create() - creates a sync pt
  * @parent:	fence's parent sync_timeline
  * @size:	size to allocate for this pt
+ * @inc:	value of the fence
  *
  * Creates a new fence as a child of @parent.  @size bytes will be
  * allocated allowing for implementation specific data to be kept after
  * the generic sync_timeline struct. Returns the fence object or
  * NULL in case of error.
  */
-struct fence *sync_pt_create(struct sync_timeline *parent, int size);
+struct fence *sync_pt_create(struct sync_timeline *parent, int size,
+			     unsigned int inc);
 
 #ifdef CONFIG_DEBUG_FS
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 03/11] staging/android: remove .fill_driver_data() timeline ops
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 02/11] staging/android: store last signaled value on sync timeline Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 04/11] staging/android: remove .{fence,timeline}_value_str() from timeline_ops Gustavo Padovan
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

The .fill_driver_data() ops was just a useless abstraction for
fence_ops op of the same name.

Now that we use fence->seqno to store the value it is cleaner to
remove the abstraction and fill the data directly.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c | 14 --------------
 drivers/staging/android/sync.c    |  9 +++++----
 drivers/staging/android/sync.h    |  7 -------
 3 files changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index b9d53d3..428e22c 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -38,19 +38,6 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
 }
 EXPORT_SYMBOL(sw_sync_pt_create);
 
-static int sw_sync_fill_driver_data(struct fence *fence,
-				    void *data, int size)
-{
-	struct sw_sync_pt *pt = (struct sw_sync_pt *)fence;
-
-	if (size < sizeof(pt->value))
-		return -ENOMEM;
-
-	memcpy(data, &pt->value, sizeof(pt->value));
-
-	return sizeof(pt->value);
-}
-
 static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline,
 				       char *str, int size)
 {
@@ -68,7 +55,6 @@ static void sw_sync_fence_value_str(struct fence *fence, char *str, int size)
 
 static struct sync_timeline_ops sw_sync_timeline_ops = {
 	.driver_name = "sw_sync",
-	.fill_driver_data = sw_sync_fill_driver_data,
 	.timeline_value_str = sw_sync_timeline_value_str,
 	.fence_value_str = sw_sync_fence_value_str,
 };
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 4eea5c3..39af5fb 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -185,11 +185,12 @@ static bool android_fence_enable_signaling(struct fence *fence)
 static int android_fence_fill_driver_data(struct fence *fence,
 					  void *data, int size)
 {
-	struct sync_timeline *parent = fence_parent(fence);
+	if (size < sizeof(fence->seqno))
+		return -ENOMEM;
+
+	memcpy(data, &fence->seqno, sizeof(fence->seqno));
 
-	if (!parent->ops->fill_driver_data)
-		return 0;
-	return parent->ops->fill_driver_data(fence, data, size);
+	return sizeof(fence->seqno);
 }
 
 static void android_fence_value_str(struct fence *fence,
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 4d3dfbf..500838b 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -27,10 +27,6 @@ struct sync_timeline;
 /**
  * struct sync_timeline_ops - sync object implementation ops
  * @driver_name:	name of the implementation
- * @fill_driver_data:	write implementation specific driver data to data.
- *			  should return an error if there is not enough room
- *			  as specified by size.  This information is returned
- *			  to userspace by SYNC_IOC_FENCE_INFO.
  * @timeline_value_str: fill str with the value of the sync_timeline's counter
  * @fence_value_str:	fill str with the value of the fence
  */
@@ -38,9 +34,6 @@ struct sync_timeline_ops {
 	const char *driver_name;
 
 	/* optional */
-	int (*fill_driver_data)(struct fence *fence, void *data, int size);
-
-	/* optional */
 	void (*timeline_value_str)(struct sync_timeline *timeline, char *str,
 				   int size);
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 04/11] staging/android: remove .{fence,timeline}_value_str() from timeline_ops
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (2 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 03/11] staging/android: remove .fill_driver_data() timeline ops Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 05/11] staging/android: remove struct sync_timeline_ops Gustavo Padovan
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

Now that the value of fence and the timeline are not stored by sw_sync
anymore we can remove this extra abstraction to retrieve this data.

This patch changes both fence_ops (.fence_value_str and
.timeline_value_str) to return the str directly.

It also clean up struct sync_timeline_ops by removing both ops from there.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c    | 17 -----------------
 drivers/staging/android/sync.c       | 16 ++--------------
 drivers/staging/android/sync.h       |  9 ---------
 drivers/staging/android/sync_debug.c | 12 ++----------
 drivers/staging/android/trace/sync.h | 12 +++---------
 5 files changed, 7 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 428e22c..4200b12 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -38,25 +38,8 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
 }
 EXPORT_SYMBOL(sw_sync_pt_create);
 
-static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline,
-				       char *str, int size)
-{
-	struct sw_sync_timeline *timeline =
-		(struct sw_sync_timeline *)sync_timeline;
-	snprintf(str, size, "%d", timeline->value);
-}
-
-static void sw_sync_fence_value_str(struct fence *fence, char *str, int size)
-{
-	struct sw_sync_pt *pt = (struct sw_sync_pt *)fence;
-
-	snprintf(str, size, "%d", pt->value);
-}
-
 static struct sync_timeline_ops sw_sync_timeline_ops = {
 	.driver_name = "sw_sync",
-	.timeline_value_str = sw_sync_timeline_value_str,
-	.fence_value_str = sw_sync_fence_value_str,
 };
 
 struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 39af5fb..f2d298c 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -196,14 +196,7 @@ static int android_fence_fill_driver_data(struct fence *fence,
 static void android_fence_value_str(struct fence *fence,
 				    char *str, int size)
 {
-	struct sync_timeline *parent = fence_parent(fence);
-
-	if (!parent->ops->fence_value_str) {
-		if (size)
-			*str = 0;
-		return;
-	}
-	parent->ops->fence_value_str(fence, str, size);
+	snprintf(str, size, "%d", fence->seqno);
 }
 
 static void android_fence_timeline_value_str(struct fence *fence,
@@ -211,12 +204,7 @@ static void android_fence_timeline_value_str(struct fence *fence,
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
-	if (!parent->ops->timeline_value_str) {
-		if (size)
-			*str = 0;
-		return;
-	}
-	parent->ops->timeline_value_str(parent, str, size);
+	snprintf(str, size, "%d", parent->value);
 }
 
 static const struct fence_ops android_fence_ops = {
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 500838b..b1a4b06 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -27,18 +27,9 @@ struct sync_timeline;
 /**
  * struct sync_timeline_ops - sync object implementation ops
  * @driver_name:	name of the implementation
- * @timeline_value_str: fill str with the value of the sync_timeline's counter
- * @fence_value_str:	fill str with the value of the fence
  */
 struct sync_timeline_ops {
 	const char *driver_name;
-
-	/* optional */
-	void (*timeline_value_str)(struct sync_timeline *timeline, char *str,
-				   int size);
-
-	/* optional */
-	void (*fence_value_str)(struct fence *fence, char *str, int size);
 };
 
 /**
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index b37412d..7517fb3 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -134,16 +134,8 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj)
 	struct list_head *pos;
 	unsigned long flags;
 
-	seq_printf(s, "%s %s", obj->name, obj->ops->driver_name);
-
-	if (obj->ops->timeline_value_str) {
-		char value[64];
-
-		obj->ops->timeline_value_str(obj, value, sizeof(value));
-		seq_printf(s, ": %s", value);
-	}
-
-	seq_puts(s, "\n");
+	seq_printf(s, "%s %s: %d\n", obj->name, obj->ops->driver_name,
+		   obj->value);
 
 	spin_lock_irqsave(&obj->child_list_lock, flags);
 	list_for_each(pos, &obj->child_list_head) {
diff --git a/drivers/staging/android/trace/sync.h b/drivers/staging/android/trace/sync.h
index a0f80f4..d7f6457f 100644
--- a/drivers/staging/android/trace/sync.h
+++ b/drivers/staging/android/trace/sync.h
@@ -15,21 +15,15 @@ TRACE_EVENT(sync_timeline,
 
 	TP_STRUCT__entry(
 			__string(name, timeline->name)
-			__array(char, value, 32)
+			__field(u32, value)
 	),
 
 	TP_fast_assign(
 			__assign_str(name, timeline->name);
-			if (timeline->ops->timeline_value_str) {
-				timeline->ops->timeline_value_str(timeline,
-							__entry->value,
-							sizeof(__entry->value));
-			} else {
-				__entry->value[0] = '\0';
-			}
+			__entry->value = timeline->value;
 	),
 
-	TP_printk("name=%s value=%s", __get_str(name), __entry->value)
+	TP_printk("name=%s value=%d", __get_str(name), __entry->value)
 );
 
 #endif /* if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ) */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 05/11] staging/android: remove struct sync_timeline_ops
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (3 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 04/11] staging/android: remove .{fence,timeline}_value_str() from timeline_ops Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 06/11] staging/android: remove sw_sync_timeline and sw_sync_pt Gustavo Padovan
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

Move drv_name, the last field of sync_timeline_ops, to sync_timeline
and remove sync_timeline_ops.

struct sync_timeline_ops was just an extra abstraction on top of
fence_ops, and in the last few commits we removed all it ops in favor
of cleaner fence_ops.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c    |  9 ++-------
 drivers/staging/android/sync.c       |  8 ++++----
 drivers/staging/android/sync.h       | 28 +++++++++-------------------
 drivers/staging/android/sync_debug.c |  3 +--
 4 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index 4200b12..c5e92c6 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -38,16 +38,11 @@ struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
 }
 EXPORT_SYMBOL(sw_sync_pt_create);
 
-static struct sync_timeline_ops sw_sync_timeline_ops = {
-	.driver_name = "sw_sync",
-};
-
 struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
 {
 	struct sw_sync_timeline *obj = (struct sw_sync_timeline *)
-		sync_timeline_create(&sw_sync_timeline_ops,
-				     sizeof(struct sw_sync_timeline),
-				     name);
+		sync_timeline_create(sizeof(struct sw_sync_timeline),
+				     "sw_sync", name);
 
 	return obj;
 }
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index f2d298c..07fe995 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -30,8 +30,8 @@
 
 static const struct fence_ops android_fence_ops;
 
-struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
-					   int size, const char *name)
+struct sync_timeline *sync_timeline_create(int size, const char *drv_name,
+					   const char *name)
 {
 	struct sync_timeline *obj;
 
@@ -43,9 +43,9 @@ struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
 		return NULL;
 
 	kref_init(&obj->kref);
-	obj->ops = ops;
 	obj->context = fence_context_alloc(1);
 	strlcpy(obj->name, name, sizeof(obj->name));
+	strlcpy(obj->drv_name, drv_name, sizeof(obj->drv_name));
 
 	INIT_LIST_HEAD(&obj->child_list_head);
 	INIT_LIST_HEAD(&obj->active_list_head);
@@ -139,7 +139,7 @@ static const char *android_fence_get_driver_name(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
-	return parent->ops->driver_name;
+	return parent->drv_name;
 }
 
 static const char *android_fence_get_timeline_name(struct fence *fence)
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index b1a4b06..be94a80 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -22,20 +22,10 @@
 #include <linux/sync_file.h>
 #include <uapi/linux/sync.h>
 
-struct sync_timeline;
-
-/**
- * struct sync_timeline_ops - sync object implementation ops
- * @driver_name:	name of the implementation
- */
-struct sync_timeline_ops {
-	const char *driver_name;
-};
-
 /**
  * struct sync_timeline - sync object
  * @kref:		reference count on fence.
- * @ops:		ops that define the implementation of the sync_timeline
+ * @drv_name:		drv_name of the driver using the sync_timeline
  * @name:		name of the sync_timeline. Useful for debugging
  * @destroyed:		set when sync_timeline is destroyed
  * @child_list_head:	list of children sync_pts for this sync_timeline
@@ -46,7 +36,7 @@ struct sync_timeline_ops {
  */
 struct sync_timeline {
 	struct kref		kref;
-	const struct sync_timeline_ops	*ops;
+	char			drv_name[32];
 	char			name[32];
 
 	/* protected by child_list_lock */
@@ -75,17 +65,17 @@ static inline struct sync_timeline *fence_parent(struct fence *fence)
 
 /**
  * sync_timeline_create() - creates a sync object
- * @ops:	specifies the implementation ops for the object
  * @size:	size to allocate for this obj
+ * @drv_name:	sync_timeline driver name
  * @name:	sync_timeline name
  *
- * Creates a new sync_timeline which will use the implementation specified by
- * @ops.  @size bytes will be allocated allowing for implementation specific
- * data to be kept after the generic sync_timeline struct. Returns the
- * sync_timeline object or NULL in case of error.
+ * Creates a new sync_timeline. @size bytes will be allocated allowing
+ * for implementation specific data to be kept after the generic
+ * sync_timeline struct. Returns the sync_timeline object or NULL in
+ * case of error.
  */
-struct sync_timeline *sync_timeline_create(const struct sync_timeline_ops *ops,
-					   int size, const char *name);
+struct sync_timeline *sync_timeline_create(int size, const char *drv_name,
+					   const char *name);
 
 /**
  * sync_timeline_destroy() - destroys a sync object
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index 7517fb3..26ba9e86 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -134,8 +134,7 @@ static void sync_print_obj(struct seq_file *s, struct sync_timeline *obj)
 	struct list_head *pos;
 	unsigned long flags;
 
-	seq_printf(s, "%s %s: %d\n", obj->name, obj->ops->driver_name,
-		   obj->value);
+	seq_printf(s, "%s %s: %d\n", obj->name, obj->drv_name, obj->value);
 
 	spin_lock_irqsave(&obj->child_list_lock, flags);
 	list_for_each(pos, &obj->child_list_head) {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 06/11] staging/android: remove sw_sync_timeline and sw_sync_pt
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (4 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 05/11] staging/android: remove struct sync_timeline_ops Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 07/11] staging/android: remove sw_sync.[ch] files Gustavo Padovan
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

As we moved value storage to sync_timeline and fence those two structs
became useless and can be removed now.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sw_sync.c    | 24 +++++++-----------------
 drivers/staging/android/sw_sync.h    | 24 ++++++------------------
 drivers/staging/android/sync_debug.c | 12 ++++++------
 3 files changed, 19 insertions(+), 41 deletions(-)

diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
index c5e92c6..461dbd9 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -25,31 +25,21 @@
 
 #include "sw_sync.h"
 
-struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value)
+struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value)
 {
-	struct sw_sync_pt *pt;
-
-	pt = (struct sw_sync_pt *)
-		sync_pt_create(&obj->obj, sizeof(struct sw_sync_pt), value);
-
-	pt->value = value;
-
-	return (struct fence *)pt;
+	return sync_pt_create(obj, sizeof(struct fence), value);
 }
 EXPORT_SYMBOL(sw_sync_pt_create);
 
-struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
+struct sync_timeline *sw_sync_timeline_create(const char *name)
 {
-	struct sw_sync_timeline *obj = (struct sw_sync_timeline *)
-		sync_timeline_create(sizeof(struct sw_sync_timeline),
-				     "sw_sync", name);
-
-	return obj;
+	return sync_timeline_create(sizeof(struct sync_timeline),
+				    "sw_sync", name);
 }
 EXPORT_SYMBOL(sw_sync_timeline_create);
 
-void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
+void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc)
 {
-	sync_timeline_signal(&obj->obj, inc);
+	sync_timeline_signal(obj, inc);
 }
 EXPORT_SYMBOL(sw_sync_timeline_inc);
diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h
index e18667b..9f26c62 100644
--- a/drivers/staging/android/sw_sync.h
+++ b/drivers/staging/android/sw_sync.h
@@ -22,34 +22,22 @@
 #include "sync.h"
 #include "uapi/sw_sync.h"
 
-struct sw_sync_timeline {
-	struct	sync_timeline	obj;
-
-	u32			value;
-};
-
-struct sw_sync_pt {
-	struct fence		pt;
-
-	u32			value;
-};
-
 #if IS_ENABLED(CONFIG_SW_SYNC)
-struct sw_sync_timeline *sw_sync_timeline_create(const char *name);
-void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc);
+struct sync_timeline *sw_sync_timeline_create(const char *name);
+void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc);
 
-struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj, u32 value);
+struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value);
 #else
-static inline struct sw_sync_timeline *sw_sync_timeline_create(const char *name)
+static inline struct sync_timeline *sw_sync_timeline_create(const char *name)
 {
 	return NULL;
 }
 
-static inline void sw_sync_timeline_inc(struct sw_sync_timeline *obj, u32 inc)
+static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc)
 {
 }
 
-static inline struct fence *sw_sync_pt_create(struct sw_sync_timeline *obj,
+static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj,
 					      u32 value)
 {
 	return NULL;
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index 26ba9e86..e984955 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -211,7 +211,7 @@ static const struct file_operations sync_info_debugfs_fops = {
 /* opening sw_sync create a new sync obj */
 static int sw_sync_debugfs_open(struct inode *inode, struct file *file)
 {
-	struct sw_sync_timeline *obj;
+	struct sync_timeline *obj;
 	char task_comm[TASK_COMM_LEN];
 
 	get_task_comm(task_comm, current);
@@ -227,13 +227,13 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file)
 
 static int sw_sync_debugfs_release(struct inode *inode, struct file *file)
 {
-	struct sw_sync_timeline *obj = file->private_data;
+	struct sync_timeline *obj = file->private_data;
 
-	sync_timeline_destroy(&obj->obj);
+	sync_timeline_destroy(obj);
 	return 0;
 }
 
-static long sw_sync_ioctl_create_fence(struct sw_sync_timeline *obj,
+static long sw_sync_ioctl_create_fence(struct sync_timeline *obj,
 				       unsigned long arg)
 {
 	int fd = get_unused_fd_flags(O_CLOEXEC);
@@ -280,7 +280,7 @@ err:
 	return err;
 }
 
-static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg)
+static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg)
 {
 	u32 value;
 
@@ -295,7 +295,7 @@ static long sw_sync_ioctl_inc(struct sw_sync_timeline *obj, unsigned long arg)
 static long sw_sync_ioctl(struct file *file, unsigned int cmd,
 			  unsigned long arg)
 {
-	struct sw_sync_timeline *obj = file->private_data;
+	struct sync_timeline *obj = file->private_data;
 
 	switch (cmd) {
 	case SW_SYNC_IOC_CREATE_FENCE:
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 07/11] staging/android: remove sw_sync.[ch] files
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (5 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 06/11] staging/android: remove sw_sync_timeline and sw_sync_pt Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 08/11] staging/android: rename android_fence to timeline_fence Gustavo Padovan
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

We can glue the sw_sync file operations directly on the sync framework
without the need to pass through sw_sync wrappers.

It only builds sw_sync debugfs file support if CONFIG_SW_SYNC is enabled.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/Makefile     |  1 -
 drivers/staging/android/sw_sync.c    | 45 ----------------------------------
 drivers/staging/android/sw_sync.h    | 47 ------------------------------------
 drivers/staging/android/sync_debug.c | 17 ++++++++++---
 4 files changed, 13 insertions(+), 97 deletions(-)
 delete mode 100644 drivers/staging/android/sw_sync.c
 delete mode 100644 drivers/staging/android/sw_sync.h

diff --git a/drivers/staging/android/Makefile b/drivers/staging/android/Makefile
index c7b6c99..2c1d97f 100644
--- a/drivers/staging/android/Makefile
+++ b/drivers/staging/android/Makefile
@@ -7,4 +7,3 @@ obj-$(CONFIG_ANDROID_TIMED_OUTPUT)	+= timed_output.o
 obj-$(CONFIG_ANDROID_TIMED_GPIO)	+= timed_gpio.o
 obj-$(CONFIG_ANDROID_LOW_MEMORY_KILLER)	+= lowmemorykiller.o
 obj-$(CONFIG_SYNC)			+= sync.o sync_debug.o
-obj-$(CONFIG_SW_SYNC)			+= sw_sync.o
diff --git a/drivers/staging/android/sw_sync.c b/drivers/staging/android/sw_sync.c
deleted file mode 100644
index 461dbd9..0000000
--- a/drivers/staging/android/sw_sync.c
+++ /dev/null
@@ -1,45 +0,0 @@
-/*
- * drivers/base/sw_sync.c
- *
- * Copyright (C) 2012 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * 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/kernel.h>
-#include <linux/init.h>
-#include <linux/export.h>
-#include <linux/file.h>
-#include <linux/fs.h>
-#include <linux/miscdevice.h>
-#include <linux/syscalls.h>
-#include <linux/uaccess.h>
-
-#include "sw_sync.h"
-
-struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value)
-{
-	return sync_pt_create(obj, sizeof(struct fence), value);
-}
-EXPORT_SYMBOL(sw_sync_pt_create);
-
-struct sync_timeline *sw_sync_timeline_create(const char *name)
-{
-	return sync_timeline_create(sizeof(struct sync_timeline),
-				    "sw_sync", name);
-}
-EXPORT_SYMBOL(sw_sync_timeline_create);
-
-void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc)
-{
-	sync_timeline_signal(obj, inc);
-}
-EXPORT_SYMBOL(sw_sync_timeline_inc);
diff --git a/drivers/staging/android/sw_sync.h b/drivers/staging/android/sw_sync.h
deleted file mode 100644
index 9f26c62..0000000
--- a/drivers/staging/android/sw_sync.h
+++ /dev/null
@@ -1,47 +0,0 @@
-/*
- * include/linux/sw_sync.h
- *
- * Copyright (C) 2012 Google, Inc.
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * 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_SW_SYNC_H
-#define _LINUX_SW_SYNC_H
-
-#include <linux/types.h>
-#include <linux/kconfig.h>
-#include "sync.h"
-#include "uapi/sw_sync.h"
-
-#if IS_ENABLED(CONFIG_SW_SYNC)
-struct sync_timeline *sw_sync_timeline_create(const char *name);
-void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc);
-
-struct fence *sw_sync_pt_create(struct sync_timeline *obj, u32 value);
-#else
-static inline struct sync_timeline *sw_sync_timeline_create(const char *name)
-{
-	return NULL;
-}
-
-static inline void sw_sync_timeline_inc(struct sync_timeline *obj, u32 inc)
-{
-}
-
-static inline struct fence *sw_sync_pt_create(struct sync_timeline *obj,
-					      u32 value)
-{
-	return NULL;
-}
-#endif /* IS_ENABLED(CONFIG_SW_SYNC) */
-
-#endif /* _LINUX_SW_SYNC_H */
diff --git a/drivers/staging/android/sync_debug.c b/drivers/staging/android/sync_debug.c
index e984955..9312e6f 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/staging/android/sync_debug.c
@@ -28,7 +28,11 @@
 #include <linux/anon_inodes.h>
 #include <linux/time64.h>
 #include <linux/sync_file.h>
-#include "sw_sync.h"
+#include <linux/types.h>
+#include <linux/kconfig.h>
+
+#include "uapi/sw_sync.h"
+#include "sync.h"
 
 #ifdef CONFIG_DEBUG_FS
 
@@ -202,6 +206,7 @@ static const struct file_operations sync_info_debugfs_fops = {
 	.release        = single_release,
 };
 
+#if IS_ENABLED(CONFIG_SW_SYNC)
 /*
  * *WARNING*
  *
@@ -216,7 +221,7 @@ static int sw_sync_debugfs_open(struct inode *inode, struct file *file)
 
 	get_task_comm(task_comm, current);
 
-	obj = sw_sync_timeline_create(task_comm);
+	obj = sync_timeline_create(sizeof(*obj), "sw_sync", task_comm);
 	if (!obj)
 		return -ENOMEM;
 
@@ -250,7 +255,7 @@ static long sw_sync_ioctl_create_fence(struct sync_timeline *obj,
 		goto err;
 	}
 
-	fence = sw_sync_pt_create(obj, data.value);
+	fence = sync_pt_create(obj, sizeof(*fence), data.value);
 	if (!fence) {
 		err = -ENOMEM;
 		goto err;
@@ -287,7 +292,7 @@ static long sw_sync_ioctl_inc(struct sync_timeline *obj, unsigned long arg)
 	if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
 		return -EFAULT;
 
-	sw_sync_timeline_inc(obj, value);
+	sync_timeline_signal(obj, value);
 
 	return 0;
 }
@@ -315,14 +320,18 @@ static const struct file_operations sw_sync_debugfs_fops = {
 	.unlocked_ioctl = sw_sync_ioctl,
 	.compat_ioctl = sw_sync_ioctl,
 };
+#endif
 
 static __init int sync_debugfs_init(void)
 {
 	dbgfs = debugfs_create_dir("sync", NULL);
 
 	debugfs_create_file("info", 0444, dbgfs, NULL, &sync_info_debugfs_fops);
+
+#if IS_ENABLED(CONFIG_SW_SYNC)
 	debugfs_create_file("sw_sync", 0644, dbgfs, NULL,
 			    &sw_sync_debugfs_fops);
+#endif
 
 	return 0;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 08/11] staging/android: rename android_fence to timeline_fence
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (6 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 07/11] staging/android: remove sw_sync.[ch] files Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 09/11] dma-buf/sync_timeline: de-stage sync_timeline Gustavo Padovan
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

We are moving out of staging/adroid so rename it to a name that is not
related to android anymore.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/staging/android/sync.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 07fe995..ea816dd 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -28,7 +28,7 @@
 #define CREATE_TRACE_POINTS
 #include "trace/sync.h"
 
-static const struct fence_ops android_fence_ops;
+static const struct fence_ops timeline_fence_ops;
 
 struct sync_timeline *sync_timeline_create(int size, const char *drv_name,
 					   const char *name)
@@ -126,7 +126,7 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size,
 
 	spin_lock_irqsave(&obj->child_list_lock, flags);
 	sync_timeline_get(obj);
-	fence_init(fence, &android_fence_ops, &obj->child_list_lock,
+	fence_init(fence, &timeline_fence_ops, &obj->child_list_lock,
 		   obj->context, value);
 	list_add_tail(&fence->child_list, &obj->child_list_head);
 	INIT_LIST_HEAD(&fence->active_list);
@@ -135,21 +135,21 @@ struct fence *sync_pt_create(struct sync_timeline *obj, int size,
 }
 EXPORT_SYMBOL(sync_pt_create);
 
-static const char *android_fence_get_driver_name(struct fence *fence)
+static const char *timeline_fence_get_driver_name(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
 	return parent->drv_name;
 }
 
-static const char *android_fence_get_timeline_name(struct fence *fence)
+static const char *timeline_fence_get_timeline_name(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
 	return parent->name;
 }
 
-static void android_fence_release(struct fence *fence)
+static void timeline_fence_release(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 	unsigned long flags;
@@ -164,25 +164,25 @@ static void android_fence_release(struct fence *fence)
 	fence_free(fence);
 }
 
-static bool android_fence_signaled(struct fence *fence)
+static bool timeline_fence_signaled(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
 	return (fence->seqno > parent->value) ? false : true;
 }
 
-static bool android_fence_enable_signaling(struct fence *fence)
+static bool timeline_fence_enable_signaling(struct fence *fence)
 {
 	struct sync_timeline *parent = fence_parent(fence);
 
-	if (android_fence_signaled(fence))
+	if (timeline_fence_signaled(fence))
 		return false;
 
 	list_add_tail(&fence->active_list, &parent->active_list_head);
 	return true;
 }
 
-static int android_fence_fill_driver_data(struct fence *fence,
+static int timeline_fence_fill_driver_data(struct fence *fence,
 					  void *data, int size)
 {
 	if (size < sizeof(fence->seqno))
@@ -193,13 +193,13 @@ static int android_fence_fill_driver_data(struct fence *fence,
 	return sizeof(fence->seqno);
 }
 
-static void android_fence_value_str(struct fence *fence,
+static void timeline_fence_value_str(struct fence *fence,
 				    char *str, int size)
 {
 	snprintf(str, size, "%d", fence->seqno);
 }
 
-static void android_fence_timeline_value_str(struct fence *fence,
+static void timeline_fence_timeline_value_str(struct fence *fence,
 					     char *str, int size)
 {
 	struct sync_timeline *parent = fence_parent(fence);
@@ -207,15 +207,15 @@ static void android_fence_timeline_value_str(struct fence *fence,
 	snprintf(str, size, "%d", parent->value);
 }
 
-static const struct fence_ops android_fence_ops = {
-	.get_driver_name = android_fence_get_driver_name,
-	.get_timeline_name = android_fence_get_timeline_name,
-	.enable_signaling = android_fence_enable_signaling,
-	.signaled = android_fence_signaled,
+static const struct fence_ops timeline_fence_ops = {
+	.get_driver_name = timeline_fence_get_driver_name,
+	.get_timeline_name = timeline_fence_get_timeline_name,
+	.enable_signaling = timeline_fence_enable_signaling,
+	.signaled = timeline_fence_signaled,
 	.wait = fence_default_wait,
-	.release = android_fence_release,
-	.fill_driver_data = android_fence_fill_driver_data,
-	.fence_value_str = android_fence_value_str,
-	.timeline_value_str = android_fence_timeline_value_str,
+	.release = timeline_fence_release,
+	.fill_driver_data = timeline_fence_fill_driver_data,
+	.fence_value_str = timeline_fence_value_str,
+	.timeline_value_str = timeline_fence_timeline_value_str,
 };
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 09/11] dma-buf/sync_timeline: de-stage sync_timeline
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (7 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 08/11] staging/android: rename android_fence to timeline_fence Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 10/11] dma-buf/sync_file: bring debug back to sync file Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 11/11] dma-buf/sync_file: bring sync_dump() back Gustavo Padovan
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

De-stage the remaining bit of sync framework: sync_timeline and sw_sync
plus some debugging routines.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/dma-buf/Kconfig                            | 10 +++++++
 drivers/dma-buf/Makefile                           |  3 +-
 .../{staging/android/uapi => dma-buf}/sw_sync.h    |  0
 drivers/{staging/android => dma-buf}/sync_debug.c  |  5 ++--
 drivers/dma-buf/sync_debug.h                       | 35 ++++++++++++++++++++++
 .../android/sync.c => dma-buf/sync_timeline.c}     |  7 +++--
 drivers/staging/android/Kconfig                    | 20 -------------
 .../sync.h => include/linux/sync_timeline.h        | 24 +++------------
 .../sync.h => include/trace/events/sync_timeline.h |  9 +++---
 9 files changed, 62 insertions(+), 51 deletions(-)
 rename drivers/{staging/android/uapi => dma-buf}/sw_sync.h (100%)
 rename drivers/{staging/android => dma-buf}/sync_debug.c (99%)
 create mode 100644 drivers/dma-buf/sync_debug.h
 rename drivers/{staging/android/sync.c => dma-buf/sync_timeline.c} (98%)
 rename drivers/staging/android/sync.h => include/linux/sync_timeline.h (85%)
 rename drivers/staging/android/trace/sync.h => include/trace/events/sync_timeline.h (73%)

diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
index 9824bc4..73df024 100644
--- a/drivers/dma-buf/Kconfig
+++ b/drivers/dma-buf/Kconfig
@@ -8,4 +8,14 @@ config SYNC_FILE
 	---help---
 	  This option enables the fence framework synchronization to export
 	  sync_files to userspace that can represent one or more fences.
+
+config SW_SYNC
+	bool "Software synchronization objects"
+	default n
+	depends on SYNC_FILE
+	---help---
+	  A sync object driver that uses a 32bit counter to coordinate
+	  synchronization. Useful when there is no hardware primitive backing
+	  the synchronization.
+
 endmenu
diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
index 4a424ec..78d8ec4 100644
--- a/drivers/dma-buf/Makefile
+++ b/drivers/dma-buf/Makefile
@@ -1,2 +1,3 @@
 obj-y := dma-buf.o fence.o reservation.o seqno-fence.o
-obj-$(CONFIG_SYNC_FILE)		+= sync_file.o
+obj-$(CONFIG_SYNC_FILE)		+= sync_file.o sync_debug.o
+obj-$(CONFIG_SW_SYNC)		+= sync_timeline.o
diff --git a/drivers/staging/android/uapi/sw_sync.h b/drivers/dma-buf/sw_sync.h
similarity index 100%
rename from drivers/staging/android/uapi/sw_sync.h
rename to drivers/dma-buf/sw_sync.h
diff --git a/drivers/staging/android/sync_debug.c b/drivers/dma-buf/sync_debug.c
similarity index 99%
rename from drivers/staging/android/sync_debug.c
rename to drivers/dma-buf/sync_debug.c
index 9312e6f..7da9ff5 100644
--- a/drivers/staging/android/sync_debug.c
+++ b/drivers/dma-buf/sync_debug.c
@@ -30,9 +30,10 @@
 #include <linux/sync_file.h>
 #include <linux/types.h>
 #include <linux/kconfig.h>
+#include <linux/sync_timeline.h>
 
-#include "uapi/sw_sync.h"
-#include "sync.h"
+#include "sync_debug.h"
+#include "sw_sync.h"
 
 #ifdef CONFIG_DEBUG_FS
 
diff --git a/drivers/dma-buf/sync_debug.h b/drivers/dma-buf/sync_debug.h
new file mode 100644
index 0000000..5b71685
--- /dev/null
+++ b/drivers/dma-buf/sync_debug.h
@@ -0,0 +1,35 @@
+/*
+ * drivers/dma-buf/sync_debug.h
+ *
+ * Copyright (C) 2012 Google, Inc.
+ *
+ * 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_SYNC_DEBUG_H
+#define _LINUX_SYNC_DEBUG_H
+
+#include <linux/types.h>
+#include <linux/sync_timeline.h>
+#include <linux/sync_file.h>
+
+#ifdef CONFIG_DEBUG_FS
+void sync_timeline_debug_add(struct sync_timeline *obj);
+void sync_timeline_debug_remove(struct sync_timeline *obj);
+void sync_file_debug_add(struct sync_file *sync_file);
+void sync_file_debug_remove(struct sync_file *sync_file);
+void sync_dump(void);
+
+#else
+#define sync_timeline_debug_add(obj)
+#define sync_timeline_debug_remove(obj)
+#define sync_file_debug_add(fence)
+#define sync_file_debug_remove(fence)
+#define sync_dump()
+#endif
+
+#endif /* _LINUX_SYNC_DEBUG_H */
diff --git a/drivers/staging/android/sync.c b/drivers/dma-buf/sync_timeline.c
similarity index 98%
rename from drivers/staging/android/sync.c
rename to drivers/dma-buf/sync_timeline.c
index ea816dd..b354b0c 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/dma-buf/sync_timeline.c
@@ -22,11 +22,12 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/anon_inodes.h>
-
-#include "sync.h"
+#include <linux/sync_timeline.h>
 
 #define CREATE_TRACE_POINTS
-#include "trace/sync.h"
+#include <trace/events/sync_timeline.h>
+
+#include "sync_debug.h"
 
 static const struct fence_ops timeline_fence_ops;
 
diff --git a/drivers/staging/android/Kconfig b/drivers/staging/android/Kconfig
index 2756988..4b18fee 100644
--- a/drivers/staging/android/Kconfig
+++ b/drivers/staging/android/Kconfig
@@ -38,26 +38,6 @@ config ANDROID_LOW_MEMORY_KILLER
 	  scripts (/init.rc), and it defines priority values with minimum free memory size
 	  for each priority.
 
-config SYNC
-	bool "Synchronization framework"
-	default n
-	select ANON_INODES
-	select DMA_SHARED_BUFFER
-	---help---
-	  This option enables the framework for synchronization between multiple
-	  drivers.  Sync implementations can take advantage of hardware
-	  synchronization built into devices like GPUs.
-
-config SW_SYNC
-	bool "Software synchronization objects"
-	default n
-	depends on SYNC
-	depends on SYNC_FILE
-	---help---
-	  A sync object driver that uses a 32bit counter to coordinate
-	  synchronization.  Useful when there is no hardware primitive backing
-	  the synchronization.
-
 source "drivers/staging/android/ion/Kconfig"
 
 endif # if ANDROID
diff --git a/drivers/staging/android/sync.h b/include/linux/sync_timeline.h
similarity index 85%
rename from drivers/staging/android/sync.h
rename to include/linux/sync_timeline.h
index be94a80..27a2eda 100644
--- a/drivers/staging/android/sync.h
+++ b/include/linux/sync_timeline.h
@@ -1,5 +1,5 @@
 /*
- * include/linux/sync.h
+ * include/linux/sync_timeline.h
  *
  * Copyright (C) 2012 Google, Inc.
  *
@@ -10,8 +10,8 @@
  *
  */
 
-#ifndef _LINUX_SYNC_H
-#define _LINUX_SYNC_H
+#ifndef _LINUX_SYNC_TIMELINE_H
+#define _LINUX_SYNC_TIMELINE_H
 
 #include <linux/types.h>
 #include <linux/kref.h>
@@ -111,20 +111,4 @@ void sync_timeline_signal(struct sync_timeline *obj, unsigned int inc);
 struct fence *sync_pt_create(struct sync_timeline *parent, int size,
 			     unsigned int inc);
 
-#ifdef CONFIG_DEBUG_FS
-
-void sync_timeline_debug_add(struct sync_timeline *obj);
-void sync_timeline_debug_remove(struct sync_timeline *obj);
-void sync_file_debug_add(struct sync_file *fence);
-void sync_file_debug_remove(struct sync_file *fence);
-void sync_dump(void);
-
-#else
-# define sync_timeline_debug_add(obj)
-# define sync_timeline_debug_remove(obj)
-# define sync_file_debug_add(fence)
-# define sync_file_debug_remove(fence)
-# define sync_dump()
-#endif
-
-#endif /* _LINUX_SYNC_H */
+#endif /* _LINUX_SYNC_TIMELINE_H */
diff --git a/drivers/staging/android/trace/sync.h b/include/trace/events/sync_timeline.h
similarity index 73%
rename from drivers/staging/android/trace/sync.h
rename to include/trace/events/sync_timeline.h
index d7f6457f..c4b769b 100644
--- a/drivers/staging/android/trace/sync.h
+++ b/include/trace/events/sync_timeline.h
@@ -1,11 +1,10 @@
 #undef TRACE_SYSTEM
-#define TRACE_INCLUDE_PATH ../../drivers/staging/android/trace
-#define TRACE_SYSTEM sync
+#define TRACE_SYSTEM sync_timeline
 
-#if !defined(_TRACE_SYNC_H) || defined(TRACE_HEADER_MULTI_READ)
-#define _TRACE_SYNC_H
+#if !defined(_TRACE_SYNC_TIMELINE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_SYNC_TIMELINE_H
 
-#include "../sync.h"
+#include <linux/sync_timeline.h>
 #include <linux/tracepoint.h>
 
 TRACE_EVENT(sync_timeline,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 10/11] dma-buf/sync_file: bring debug back to sync file
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (8 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 09/11] dma-buf/sync_timeline: de-stage sync_timeline Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  2016-01-27 13:30 ` [PATCH v2 11/11] dma-buf/sync_file: bring sync_dump() back Gustavo Padovan
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

Enable reports of sync_files through <debugfs>/sync/info

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/dma-buf/sync_file.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index 92474dd..aa1215d 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -29,6 +29,8 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/sync_file.h>
 
+#include "sync_debug.h"
+
 static const struct file_operations sync_file_fops;
 
 static struct sync_file *sync_file_alloc(int size, const char *name)
@@ -87,6 +89,8 @@ struct sync_file *sync_file_create(const char *name, struct fence *fence)
 			       fence_check_cb_func))
 		atomic_dec(&sync_file->status);
 
+	sync_file_debug_add(sync_file);
+
 	return sync_file;
 }
 EXPORT_SYMBOL(sync_file_create);
@@ -188,6 +192,7 @@ struct sync_file *sync_file_merge(const char *name,
 		atomic_sub(num_fences - i, &sync_file->status);
 	sync_file->num_fences = i;
 
+	sync_file_debug_add(sync_file);
 	return sync_file;
 }
 EXPORT_SYMBOL(sync_file_merge);
@@ -246,6 +251,8 @@ static int sync_file_release(struct inode *inode, struct file *file)
 {
 	struct sync_file *sync_file = file->private_data;
 
+	sync_file_debug_remove(sync_file);
+
 	kref_put(&sync_file->kref, sync_file_free);
 	return 0;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 11/11] dma-buf/sync_file: bring sync_dump() back
  2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
                   ` (9 preceding siblings ...)
  2016-01-27 13:30 ` [PATCH v2 10/11] dma-buf/sync_file: bring debug back to sync file Gustavo Padovan
@ 2016-01-27 13:30 ` Gustavo Padovan
  10 siblings, 0 replies; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 13:30 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Maarten Lankhorst,
	Gustavo Padovan

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

During the de-stage of sync framework it was easy to keep sync_dump() out
to avoid an early de-stage of all debug code, but now that sync_debug.c
was de-staged bring sync_dump() back.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/dma-buf/sync_file.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/sync_file.c b/drivers/dma-buf/sync_file.c
index aa1215d..fd7e3b9 100644
--- a/drivers/dma-buf/sync_file.c
+++ b/drivers/dma-buf/sync_file.c
@@ -218,15 +218,19 @@ int sync_file_wait(struct sync_file *sync_file, long timeout)
 	if (ret < 0) {
 		return ret;
 	} else if (ret == 0) {
-		if (timeout)
+		if (timeout) {
 			pr_info("sync_file timeout on [%p] after %dms\n",
 				sync_file, jiffies_to_msecs(timeout));
+			sync_dump();
+		}
 		return -ETIME;
 	}
 
 	ret = atomic_read(&sync_file->status);
-	if (ret)
+	if (ret) {
 		pr_info("sync_file error %ld on [%p]\n", ret, sync_file);
+		sync_dump();
+	}
 
 	return ret;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 13:30 ` [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file Gustavo Padovan
@ 2016-01-27 14:31   ` Maarten Lankhorst
  2016-01-27 17:03     ` Gustavo Padovan
  0 siblings, 1 reply; 19+ messages in thread
From: Maarten Lankhorst @ 2016-01-27 14:31 UTC (permalink / raw)
  To: Gustavo Padovan, Greg Kroah-Hartman
  Cc: linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Gustavo Padovan

Hey,

Op 27-01-16 om 14:30 schreef Gustavo Padovan:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> sync_file is useful to connect one or more fences to the file. The file is
> used by userspace to track fences.
>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
Is there a value in keeping the abi unchanged?
If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.

Looking at the patch, it seems you kept SYNC_IOC_WAIT, won't it be better to remove it, and only support waiting with polling?
The code for polling should already work.

It's very unclear what format @driver_data has. I kept it for compatibility with android, but it's not clear to me how a userspace consumer would print it.
Is there a usecase for this, or could it be removed from fence and sync_file?

~Maarten

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 14:31   ` Maarten Lankhorst
@ 2016-01-27 17:03     ` Gustavo Padovan
  2016-01-27 18:17       ` Emil Velikov
  0 siblings, 1 reply; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 17:03 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Greg Kroah-Hartman, linux-kernel, devel, dri-devel, Daniel Stone,
	Arve Hjønnevåg, Riley Andrews, Daniel Vetter,
	Rob Clark, Greg Hackmann, John Harrison, Gustavo Padovan

Hi Maarten,

2016-01-27 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:

> Hey,
> 
> Op 27-01-16 om 14:30 schreef Gustavo Padovan:
> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> > sync_file is useful to connect one or more fences to the file. The file is
> > used by userspace to track fences.
> >
> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >
> Is there a value in keeping the abi unchanged?
> If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.

None from me. I'll look where we can improve the ABI.

> 
> Looking at the patch, it seems you kept SYNC_IOC_WAIT, won't it be better to remove it, and only support waiting with polling?
> The code for polling should already work.

Sure, that makes sense for me.

> 
> It's very unclear what format @driver_data has. I kept it for compatibility with android, but it's not clear to me how a userspace consumer would print it.
> Is there a usecase for this, or could it be removed from fence and sync_file?

I don't have any usecase for this. I'd say we remove it for now and if
someone needs this in the future we can talk about this again.

	Gustavo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 17:03     ` Gustavo Padovan
@ 2016-01-27 18:17       ` Emil Velikov
  2016-01-27 20:25         ` Gustavo Padovan
  0 siblings, 1 reply; 19+ messages in thread
From: Emil Velikov @ 2016-01-27 18:17 UTC (permalink / raw)
  To: Gustavo Padovan, Maarten Lankhorst, Greg Kroah-Hartman,
	Linux-Kernel@Vger. Kernel. Org, devel, ML dri-devel,
	Daniel Stone, Arve Hjønnevåg, Riley Andrews,
	Daniel Vetter, Rob Clark, Greg Hackmann, John Harrison,
	Gustavo Padovan

Hi Gustavo,

On 27 January 2016 at 17:03, Gustavo Padovan <gustavo@padovan.org> wrote:
> Hi Maarten,
>
> 2016-01-27 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:
>
>> Hey,
>>
>> Op 27-01-16 om 14:30 schreef Gustavo Padovan:
>> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> >
>> > sync_file is useful to connect one or more fences to the file. The file is
>> > used by userspace to track fences.
>> >
>> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> >
>> Is there a value in keeping the abi unchanged?
>> If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
>
> None from me. I'll look where we can improve the ABI.
>
Speaking of ABI... there are a couple of things that rang bells here:
 - In most/all of the kernel a len/size named member variable
indicates the length of the extra data (zero sized array). While here
it includes the size of the struct as well.
 - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
fence_info struct for every fence in the sync_file". Thus shouldn't
one use "struct fence_info" as the type ?

-Emil

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 18:17       ` Emil Velikov
@ 2016-01-27 20:25         ` Gustavo Padovan
  2016-01-27 21:41           ` Greg Hackmann
  0 siblings, 1 reply; 19+ messages in thread
From: Gustavo Padovan @ 2016-01-27 20:25 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Maarten Lankhorst, Greg Kroah-Hartman,
	Linux-Kernel@Vger. Kernel. Org, devel, ML dri-devel,
	Daniel Stone, Arve Hjønnevåg, Riley Andrews,
	Daniel Vetter, Rob Clark, Greg Hackmann, John Harrison,
	Gustavo Padovan

Hi Emil,

2016-01-27 Emil Velikov <emil.l.velikov@gmail.com>:

> Hi Gustavo,
> 
> On 27 January 2016 at 17:03, Gustavo Padovan <gustavo@padovan.org> wrote:
> > Hi Maarten,
> >
> > 2016-01-27 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>:
> >
> >> Hey,
> >>
> >> Op 27-01-16 om 14:30 schreef Gustavo Padovan:
> >> > From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >> >
> >> > sync_file is useful to connect one or more fences to the file. The file is
> >> > used by userspace to track fences.
> >> >
> >> > Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> >> >
> >> Is there a value in keeping the abi unchanged?
> >> If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
> >
> > None from me. I'll look where we can improve the ABI.
> >
> Speaking of ABI... there are a couple of things that rang bells here:
>  - In most/all of the kernel a len/size named member variable
> indicates the length of the extra data (zero sized array). While here
> it includes the size of the struct as well.

len in this case is the size of the buffer sent to the kernel,
the total length (including the whole struct) is returned in the ioctl.

>  - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
> fence_info struct for every fence in the sync_file". Thus shouldn't
> one use "struct fence_info" as the type ?

Agreed. But I'm currently thinking if we really should keep this ioctl. 

	Gustavo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 20:25         ` Gustavo Padovan
@ 2016-01-27 21:41           ` Greg Hackmann
  2016-01-28  9:23             ` Daniel Vetter
  0 siblings, 1 reply; 19+ messages in thread
From: Greg Hackmann @ 2016-01-27 21:41 UTC (permalink / raw)
  To: Gustavo Padovan, Emil Velikov, Maarten Lankhorst,
	Greg Kroah-Hartman, Linux-Kernel@Vger. Kernel. Org, devel,
	ML dri-devel, Daniel Stone, Arve Hjønnevåg,
	Riley Andrews, Daniel Vetter, Rob Clark, John Harrison,
	Gustavo Padovan

On 01/27/2016 12:25 PM, Gustavo Padovan wrote:
>>>> Is there a value in keeping the abi unchanged?
>>>> If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
>>>
>>> None from me. I'll look where we can improve the ABI.

Android has existing clients of the current ABI.  Thankfully they're all 
contained in system services like SurfaceFlinger, since end-user apps 
don't get direct access to fence fds.

As long the ABI breaks don't remove functionality we depend on, we can 
wrap around them in our userspace libsync.  I'd rather not have to do 
that, but it's a price I'm willing to pay to get this moved out of staging.

>>   - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
>> fence_info struct for every fence in the sync_file". Thus shouldn't
>> one use "struct fence_info" as the type ?
>
> Agreed. But I'm currently thinking if we really should keep this ioctl.
>
> 	Gustavo
>

I'm not seeing any consumers of driver_data in our tree.  OTOH 
completely getting rid of the ioctl would be a problem, since 
SurfaceFlinger depends on the timestamp information for its own bookkeeping.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-27 21:41           ` Greg Hackmann
@ 2016-01-28  9:23             ` Daniel Vetter
  2016-01-29 17:46               ` Greg Hackmann
  0 siblings, 1 reply; 19+ messages in thread
From: Daniel Vetter @ 2016-01-28  9:23 UTC (permalink / raw)
  To: Greg Hackmann
  Cc: Gustavo Padovan, Emil Velikov, Maarten Lankhorst,
	Greg Kroah-Hartman, Linux-Kernel@Vger. Kernel. Org, devel,
	ML dri-devel, Daniel Stone, Arve Hjønnevåg,
	Riley Andrews, Daniel Vetter, Rob Clark, John Harrison,
	Gustavo Padovan

On Wed, Jan 27, 2016 at 01:41:03PM -0800, Greg Hackmann wrote:
> On 01/27/2016 12:25 PM, Gustavo Padovan wrote:
> >>>>Is there a value in keeping the abi unchanged?
> >>>>If not, then Documentation/ioctl/botching-up-ioctls.txt is worth a read.
> >>>
> >>>None from me. I'll look where we can improve the ABI.
> 
> Android has existing clients of the current ABI.  Thankfully they're all
> contained in system services like SurfaceFlinger, since end-user apps don't
> get direct access to fence fds.
> 
> As long the ABI breaks don't remove functionality we depend on, we can wrap
> around them in our userspace libsync.  I'd rather not have to do that, but
> it's a price I'm willing to pay to get this moved out of staging.
> 
> >>  - struct sync_file_info_data::fence_info is of type __u8 yet it is "a
> >>fence_info struct for every fence in the sync_file". Thus shouldn't
> >>one use "struct fence_info" as the type ?
> >
> >Agreed. But I'm currently thinking if we really should keep this ioctl.
> >
> >	Gustavo
> >
> 
> I'm not seeing any consumers of driver_data in our tree.  OTOH completely
> getting rid of the ioctl would be a problem, since SurfaceFlinger depends on
> the timestamp information for its own bookkeeping.

If we remove driver_data (and len is superflous too), then I think we
should also make the master struct use common ioctl pattern:
- Add a num_fences field or similar that the kernel fills out.
- Make pt_info an __u64 pointer instead of a variable-length array (and
  length) - ioctl payload sizes are somewhat limited.

This way the interface is future-proofed for truly patalogical number of
fences (which surface flinger won't do, but could happen in
server/opencl/media workloads I'd imagine).

And I think driver_data really shouldn't be there, it makes things
complicated with the array of variable-sized objects, and generic
userspace can't really use it - for debug output we already have
obj/driver_name per fence point, which I think is good enough.  

Would that be ok for you from the Android side if Gustavo also provides a
patch to update libsync? I don't think the ABI is fundamentally broken,
but this light cleanup would be nice.

Wrt keeping SYNC_WAIT: I think that's totally fine. Redundant since
polling is supported, but not really an issue imo either. If we're totally
lazy we could implement SYNC_WAIT internally using poll and shave off a
few lines of the implementation.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file
  2016-01-28  9:23             ` Daniel Vetter
@ 2016-01-29 17:46               ` Greg Hackmann
  0 siblings, 0 replies; 19+ messages in thread
From: Greg Hackmann @ 2016-01-29 17:46 UTC (permalink / raw)
  To: Gustavo Padovan, Emil Velikov, Maarten Lankhorst,
	Greg Kroah-Hartman, Linux-Kernel@Vger. Kernel. Org, devel,
	ML dri-devel, Daniel Stone, Arve Hjønnevåg,
	Riley Andrews, Rob Clark, John Harrison, Gustavo Padovan

On 01/28/16 01:23, Daniel Vetter wrote:
> And I think driver_data really shouldn't be there, it makes things
> complicated with the array of variable-sized objects, and generic
> userspace can't really use it - for debug output we already have
> obj/driver_name per fence point, which I think is good enough.

I looked at our device kernels, and some vendors actually are filling in 
driver_data.  I'm just not seeing any accesses to them in our 
*userspace* tree.  And in a lot of cases it looks like they're just 
filling in debugging information that they could get elsewhere.

I'm checking with our vendor contacts to see what they're actually using 
this for (if anything).

> Would that be ok for you from the Android side if Gustavo also provides a
> patch to update libsync? I don't think the ABI is fundamentally broken,
> but this light cleanup would be nice.

No objections here.  Just upload the changes to AOSP and add me as a 
reviewer.

> Wrt keeping SYNC_WAIT: I think that's totally fine. Redundant since
> polling is supported, but not really an issue imo either. If we're totally
> lazy we could implement SYNC_WAIT internally using poll and shave off a
> few lines of the implementation.

Honestly this is the change I'm least worried about, since poll() will 
work with existing kernels too.  The only difference would be that the 
SYNC_WAIT ioctl fails when given something that's not specifically a 
sync fence; but I'm skeptical that anything actually depends on that 
behavior.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2016-01-29 17:46 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27 13:30 [PATCH v2 00/11] sync framework de-staging: part 2 - de-stage Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 01/11] dma-buf/sync_file: de-stage sync_file Gustavo Padovan
2016-01-27 14:31   ` Maarten Lankhorst
2016-01-27 17:03     ` Gustavo Padovan
2016-01-27 18:17       ` Emil Velikov
2016-01-27 20:25         ` Gustavo Padovan
2016-01-27 21:41           ` Greg Hackmann
2016-01-28  9:23             ` Daniel Vetter
2016-01-29 17:46               ` Greg Hackmann
2016-01-27 13:30 ` [PATCH v2 02/11] staging/android: store last signaled value on sync timeline Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 03/11] staging/android: remove .fill_driver_data() timeline ops Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 04/11] staging/android: remove .{fence,timeline}_value_str() from timeline_ops Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 05/11] staging/android: remove struct sync_timeline_ops Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 06/11] staging/android: remove sw_sync_timeline and sw_sync_pt Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 07/11] staging/android: remove sw_sync.[ch] files Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 08/11] staging/android: rename android_fence to timeline_fence Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 09/11] dma-buf/sync_timeline: de-stage sync_timeline Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 10/11] dma-buf/sync_file: bring debug back to sync file Gustavo Padovan
2016-01-27 13:30 ` [PATCH v2 11/11] dma-buf/sync_file: bring sync_dump() back Gustavo Padovan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).