All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support
@ 2020-07-08 13:17 Lionel Landwerlin
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 1/3] drm/i915: introduce a mechanism to extend execbuf2 Lionel Landwerlin
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-08 13:17 UTC (permalink / raw)
  To: intel-gfx

Hi all,

This is resuming the work on trying to get timeline semaphore support
for i915 upstream, now that some selftests have been added to
dma-fence-chain.

There are a few fix from the last iteration and a rebase following the
changes in the upstream execbuf code.

Cheers,

Lionel Landwerlin (3):
  drm/i915: introduce a mechanism to extend execbuf2
  drm/i915: add syncobj timeline support
  drm/i915: peel dma-fence-chains wait fences

 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 333 +++++++++++++++---
 drivers/gpu/drm/i915/i915_drv.c               |   3 +-
 drivers/gpu/drm/i915/i915_getparam.c          |   1 +
 include/uapi/drm/i915_drm.h                   |  65 +++-
 4 files changed, 342 insertions(+), 60 deletions(-)

--
2.27.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] [PATCH v12 1/3] drm/i915: introduce a mechanism to extend execbuf2
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
@ 2020-07-08 13:17 ` Lionel Landwerlin
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support Lionel Landwerlin
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-08 13:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson

We're planning to use this for a couple of new feature where we need
to provide additional parameters to execbuf.

v2: Check for invalid flags in execbuffer2 (Lionel)

v3: Rename I915_EXEC_EXT -> I915_EXEC_USE_EXTENSIONS (Chris)

v4: Rebase
    Move array fence parsing in i915_gem_do_execbuffer()

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v1)
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 131 +++++++++++-------
 include/uapi/drm/i915_drm.h                   |  26 +++-
 2 files changed, 103 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index dd15a799f9d6..03a656feb7b8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -26,6 +26,7 @@
 #include "i915_gem_ioctls.h"
 #include "i915_sw_fence_work.h"
 #include "i915_trace.h"
+#include "i915_user_extensions.h"
 
 struct eb_vma {
 	struct i915_vma *vma;
@@ -281,6 +282,13 @@ struct i915_execbuffer {
 	int lut_size;
 	struct hlist_head *buckets; /** ht for relocation handles */
 	struct eb_vma_array *array;
+
+	struct i915_eb_fence {
+		struct drm_syncobj *syncobj; /* Use with ptr_mask_bits() */
+	} *fences;
+	u32 n_fences;
+
+	u64 extension_flags; /** Available extensions parameters */
 };
 
 static inline bool eb_use_cmdparser(const struct i915_execbuffer *eb)
@@ -1622,7 +1630,8 @@ static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
 		return -EINVAL;
 
 	/* Kernel clipping was a DRI1 misfeature */
-	if (!(exec->flags & I915_EXEC_FENCE_ARRAY)) {
+	if (!(exec->flags & (I915_EXEC_FENCE_ARRAY |
+			     I915_EXEC_USE_EXTENSIONS))) {
 		if (exec->num_cliprects || exec->cliprects_ptr)
 			return -EINVAL;
 	}
@@ -2202,41 +2211,41 @@ eb_pin_engine(struct i915_execbuffer *eb,
 }
 
 static void
-__free_fence_array(struct drm_syncobj **fences, unsigned int n)
+__free_fence_array(struct i915_eb_fence *fences, unsigned int n)
 {
 	while (n--)
-		drm_syncobj_put(ptr_mask_bits(fences[n], 2));
+		drm_syncobj_put(ptr_mask_bits(fences[n].syncobj, 2));
 	kvfree(fences);
 }
 
-static struct drm_syncobj **
+static int
 get_fence_array(struct drm_i915_gem_execbuffer2 *args,
-		struct drm_file *file)
+		struct i915_execbuffer *eb)
 {
 	const unsigned long nfences = args->num_cliprects;
 	struct drm_i915_gem_exec_fence __user *user;
-	struct drm_syncobj **fences;
+	struct i915_eb_fence *fences;
 	unsigned long n;
 	int err;
 
 	if (!(args->flags & I915_EXEC_FENCE_ARRAY))
-		return NULL;
+		return 0;
 
 	/* Check multiplication overflow for access_ok() and kvmalloc_array() */
 	BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
 	if (nfences > min_t(unsigned long,
 			    ULONG_MAX / sizeof(*user),
 			    SIZE_MAX / sizeof(*fences)))
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	user = u64_to_user_ptr(args->cliprects_ptr);
 	if (!access_ok(user, nfences * sizeof(*user)))
-		return ERR_PTR(-EFAULT);
+		return -EFAULT;
 
 	fences = kvmalloc_array(nfences, sizeof(*fences),
 				__GFP_NOWARN | GFP_KERNEL);
 	if (!fences)
-		return ERR_PTR(-ENOMEM);
+		return -ENOMEM;
 
 	for (n = 0; n < nfences; n++) {
 		struct drm_i915_gem_exec_fence fence;
@@ -2252,7 +2261,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
 			goto err;
 		}
 
-		syncobj = drm_syncobj_find(file, fence.handle);
+		syncobj = drm_syncobj_find(eb->file, fence.handle);
 		if (!syncobj) {
 			DRM_DEBUG("Invalid syncobj handle provided\n");
 			err = -ENOENT;
@@ -2262,38 +2271,31 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
 		BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) &
 			     ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
 
-		fences[n] = ptr_pack_bits(syncobj, fence.flags, 2);
+		fences[n].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
 	}
 
-	return fences;
+	eb->fences = fences;
+	eb->n_fences = nfences;
+
+	return 0;
 
 err:
 	__free_fence_array(fences, n);
-	return ERR_PTR(err);
-}
-
-static void
-put_fence_array(struct drm_i915_gem_execbuffer2 *args,
-		struct drm_syncobj **fences)
-{
-	if (fences)
-		__free_fence_array(fences, args->num_cliprects);
+	return err;
 }
 
 static int
-await_fence_array(struct i915_execbuffer *eb,
-		  struct drm_syncobj **fences)
+await_fence_array(struct i915_execbuffer *eb)
 {
-	const unsigned int nfences = eb->args->num_cliprects;
 	unsigned int n;
 	int err;
 
-	for (n = 0; n < nfences; n++) {
+	for (n = 0; n < eb->n_fences; n++) {
 		struct drm_syncobj *syncobj;
 		struct dma_fence *fence;
 		unsigned int flags;
 
-		syncobj = ptr_unpack_bits(fences[n], &flags, 2);
+		syncobj = ptr_unpack_bits(eb->fences[n].syncobj, &flags, 2);
 		if (!(flags & I915_EXEC_FENCE_WAIT))
 			continue;
 
@@ -2311,18 +2313,16 @@ await_fence_array(struct i915_execbuffer *eb,
 }
 
 static void
-signal_fence_array(struct i915_execbuffer *eb,
-		   struct drm_syncobj **fences)
+signal_fence_array(struct i915_execbuffer *eb)
 {
-	const unsigned int nfences = eb->args->num_cliprects;
 	struct dma_fence * const fence = &eb->request->fence;
 	unsigned int n;
 
-	for (n = 0; n < nfences; n++) {
+	for (n = 0; n < eb->n_fences; n++) {
 		struct drm_syncobj *syncobj;
 		unsigned int flags;
 
-		syncobj = ptr_unpack_bits(fences[n], &flags, 2);
+		syncobj = ptr_unpack_bits(eb->fences[n].syncobj, &flags, 2);
 		if (!(flags & I915_EXEC_FENCE_SIGNAL))
 			continue;
 
@@ -2371,12 +2371,38 @@ static void eb_request_add(struct i915_execbuffer *eb)
 	mutex_unlock(&tl->mutex);
 }
 
+static const i915_user_extension_fn execbuf_extensions[] = {
+};
+
+static int
+parse_execbuf2_extensions(struct drm_i915_gem_execbuffer2 *args,
+			  struct i915_execbuffer *eb)
+{
+	eb->extension_flags = 0;
+
+	if (!(args->flags & I915_EXEC_USE_EXTENSIONS))
+		return 0;
+
+	/* The execbuf2 extension mechanism reuses cliprects_ptr. So we cannot
+	 * have another flag also using it at the same time.
+	 */
+	if (eb->args->flags & I915_EXEC_FENCE_ARRAY)
+		return -EINVAL;
+
+	if (args->num_cliprects != 0)
+		return -EINVAL;
+
+	return i915_user_extensions(u64_to_user_ptr(args->cliprects_ptr),
+				    execbuf_extensions,
+				    ARRAY_SIZE(execbuf_extensions),
+				    eb);
+}
+
 static int
 i915_gem_do_execbuffer(struct drm_device *dev,
 		       struct drm_file *file,
 		       struct drm_i915_gem_execbuffer2 *args,
-		       struct drm_i915_gem_exec_object2 *exec,
-		       struct drm_syncobj **fences)
+		       struct drm_i915_gem_exec_object2 *exec)
 {
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct i915_execbuffer eb;
@@ -2406,6 +2432,9 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	eb.batch_len = args->batch_len;
 	eb.trampoline = NULL;
 
+	eb.fences = NULL;
+	eb.n_fences = 0;
+
 	eb.batch_flags = 0;
 	if (args->flags & I915_EXEC_SECURE) {
 		if (INTEL_GEN(i915) >= 11)
@@ -2442,10 +2471,18 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 		}
 	}
 
-	err = eb_create(&eb);
+	err = parse_execbuf2_extensions(args, &eb);
 	if (err)
 		goto err_out_fence;
 
+	err = get_fence_array(args, &eb);
+	if (err)
+		goto err_arr_fence;
+
+	err = eb_create(&eb);
+	if (err)
+		goto err_arr_fence;
+
 	GEM_BUG_ON(!eb.lut_size);
 
 	err = eb_select_context(&eb);
@@ -2540,8 +2577,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 			goto err_request;
 	}
 
-	if (fences) {
-		err = await_fence_array(&eb, fences);
+	if (eb.n_fences) {
+		err = await_fence_array(&eb);
 		if (err)
 			goto err_request;
 	}
@@ -2572,8 +2609,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	i915_request_get(eb.request);
 	eb_request_add(&eb);
 
-	if (fences)
-		signal_fence_array(&eb, fences);
+	if (eb.n_fences)
+		signal_fence_array(&eb);
 
 	if (out_fence) {
 		if (err == 0) {
@@ -2601,6 +2638,8 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	i915_gem_context_put(eb.gem_context);
 err_destroy:
 	eb_destroy(&eb);
+err_arr_fence:
+	__free_fence_array(eb.fences, eb.n_fences);
 err_out_fence:
 	if (out_fence_fd != -1)
 		put_unused_fd(out_fence_fd);
@@ -2700,7 +2739,7 @@ i915_gem_execbuffer_ioctl(struct drm_device *dev, void *data,
 			exec2_list[i].flags = 0;
 	}
 
-	err = i915_gem_do_execbuffer(dev, file, &exec2, exec2_list, NULL);
+	err = i915_gem_do_execbuffer(dev, file, &exec2, exec2_list);
 	if (exec2.flags & __EXEC_HAS_RELOC) {
 		struct drm_i915_gem_exec_object __user *user_exec_list =
 			u64_to_user_ptr(args->buffers_ptr);
@@ -2732,7 +2771,6 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_private *i915 = to_i915(dev);
 	struct drm_i915_gem_execbuffer2 *args = data;
 	struct drm_i915_gem_exec_object2 *exec2_list;
-	struct drm_syncobj **fences = NULL;
 	const size_t count = args->buffer_count;
 	int err;
 
@@ -2760,15 +2798,7 @@ i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 		return -EFAULT;
 	}
 
-	if (args->flags & I915_EXEC_FENCE_ARRAY) {
-		fences = get_fence_array(args, file);
-		if (IS_ERR(fences)) {
-			kvfree(exec2_list);
-			return PTR_ERR(fences);
-		}
-	}
-
-	err = i915_gem_do_execbuffer(dev, file, args, exec2_list, fences);
+	err = i915_gem_do_execbuffer(dev, file, args, exec2_list);
 
 	/*
 	 * Now that we have begun execution of the batchbuffer, we ignore
@@ -2809,7 +2839,6 @@ end:;
 	}
 
 	args->flags &= ~__I915_EXEC_UNKNOWN_FLAGS;
-	put_fence_array(args, fences);
 	kvfree(exec2_list);
 	return err;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 14b67cd6b54b..7ea38aa6502c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1046,6 +1046,10 @@ struct drm_i915_gem_exec_fence {
 	__u32 flags;
 };
 
+enum drm_i915_gem_execbuffer_ext {
+	DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */
+};
+
 struct drm_i915_gem_execbuffer2 {
 	/**
 	 * List of gem_exec_object2 structs
@@ -1062,8 +1066,15 @@ struct drm_i915_gem_execbuffer2 {
 	__u32 num_cliprects;
 	/**
 	 * This is a struct drm_clip_rect *cliprects if I915_EXEC_FENCE_ARRAY
-	 * is not set.  If I915_EXEC_FENCE_ARRAY is set, then this is a
-	 * struct drm_i915_gem_exec_fence *fences.
+	 * & I915_EXEC_USE_EXTENSIONS are not set.
+	 *
+	 * If I915_EXEC_FENCE_ARRAY is set, then this is a pointer to an array
+	 * of struct drm_i915_gem_exec_fence and num_cliprects is the length
+	 * of the array.
+	 *
+	 * If I915_EXEC_USE_EXTENSIONS is set, then this is a pointer to a
+	 * single struct drm_i915_gem_base_execbuffer_ext and num_cliprects is
+	 * 0.
 	 */
 	__u64 cliprects_ptr;
 #define I915_EXEC_RING_MASK              (0x3f)
@@ -1181,7 +1192,16 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_FENCE_SUBMIT		(1 << 20)
 
-#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_FENCE_SUBMIT << 1))
+/*
+ * Setting I915_EXEC_USE_EXTENSIONS implies that
+ * drm_i915_gem_execbuffer2.cliprects_ptr is treated as a pointer to an linked
+ * list of i915_user_extension. Each i915_user_extension node is the base of a
+ * larger structure. The list of supported structures are listed in the
+ * drm_i915_gem_execbuffer_ext enum.
+ */
+#define I915_EXEC_USE_EXTENSIONS	(1 << 21)
+
+#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS<<1))
 
 #define I915_EXEC_CONTEXT_ID_MASK	(0xffffffff)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
2.27.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 1/3] drm/i915: introduce a mechanism to extend execbuf2 Lionel Landwerlin
@ 2020-07-08 13:17 ` Lionel Landwerlin
  2020-07-29 12:24     ` Daniel Vetter
  2020-07-29 12:34   ` daniel
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences Lionel Landwerlin
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-08 13:17 UTC (permalink / raw)
  To: intel-gfx

Introduces a new parameters to execbuf so that we can specify syncobj
handles as well as timeline points.

v2: Reuse i915_user_extension_fn

v3: Check that the chained extension is only present once (Chris)

v4: Check that dma_fence_chain_find_seqno returns a non NULL fence (Lionel)

v5: Use BIT_ULL (Chris)

v6: Fix issue with already signaled timeline points,
    dma_fence_chain_find_seqno() setting fence to NULL (Chris)

v7: Report ENOENT with invalid syncobj handle (Lionel)

v8: Check for out of order timeline point insertion (Chris)

v9: After explanations on
    https://lists.freedesktop.org/archives/dri-devel/2019-August/229287.html
    drop the ordering check from v8 (Lionel)

v10: Set first extension enum item to 1 (Jason)

v11: Rebase

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 167 +++++++++++++++++-
 drivers/gpu/drm/i915/i915_drv.c               |   3 +-
 drivers/gpu/drm/i915/i915_getparam.c          |   1 +
 include/uapi/drm/i915_drm.h                   |  39 ++++
 4 files changed, 203 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 03a656feb7b8..d8814e637e71 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -285,6 +285,8 @@ struct i915_execbuffer {
 
 	struct i915_eb_fence {
 		struct drm_syncobj *syncobj; /* Use with ptr_mask_bits() */
+		u64 value;
+		struct dma_fence_chain *chain_fence;
 	} *fences;
 	u32 n_fences;
 
@@ -2213,14 +2215,121 @@ eb_pin_engine(struct i915_execbuffer *eb,
 static void
 __free_fence_array(struct i915_eb_fence *fences, unsigned int n)
 {
-	while (n--)
+	while (n--) {
 		drm_syncobj_put(ptr_mask_bits(fences[n].syncobj, 2));
+		kfree(fences[n].chain_fence);
+	}
 	kvfree(fences);
 }
 
 static int
-get_fence_array(struct drm_i915_gem_execbuffer2 *args,
-		struct i915_execbuffer *eb)
+get_timeline_fence_array(const struct drm_i915_gem_execbuffer_ext_timeline_fences *timeline_fences,
+			 struct i915_execbuffer *eb)
+{
+	struct drm_i915_gem_exec_fence __user *user_fences;
+	struct i915_eb_fence *fences;
+	u64 __user *user_values;
+	u64 num_fences, num_user_fences = timeline_fences->fence_count;
+	unsigned long n;
+	int err;
+
+	/* Check multiplication overflow for access_ok() and kvmalloc_array() */
+	BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
+	if (num_user_fences > min_t(unsigned long,
+				    ULONG_MAX / sizeof(*user_fences),
+				    SIZE_MAX / sizeof(*fences)))
+		return -EINVAL;
+
+	user_fences = u64_to_user_ptr(timeline_fences->handles_ptr);
+	if (!access_ok(user_fences, num_user_fences * sizeof(*user_fences)))
+		return -EFAULT;
+
+	user_values = u64_to_user_ptr(timeline_fences->values_ptr);
+	if (!access_ok(user_values, num_user_fences * sizeof(*user_values)))
+		return -EFAULT;
+
+	fences = kvmalloc_array(num_user_fences, sizeof(*fences),
+				__GFP_NOWARN | GFP_KERNEL);
+	if (!fences)
+		return -ENOMEM;
+
+	BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) &
+		     ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
+
+	for (n = 0, num_fences = 0; n < timeline_fences->fence_count; n++) {
+		struct drm_i915_gem_exec_fence fence;
+		struct drm_syncobj *syncobj;
+		u64 point;
+
+		if (__copy_from_user(&fence, user_fences++, sizeof(fence))) {
+			err = -EFAULT;
+			goto err;
+		}
+
+		if (fence.flags & __I915_EXEC_FENCE_UNKNOWN_FLAGS) {
+			err = -EINVAL;
+			goto err;
+		}
+
+		if (__get_user(point, user_values++)) {
+			err = -EFAULT;
+			goto err;
+		}
+
+		syncobj = drm_syncobj_find(eb->file, fence.handle);
+		if (!syncobj) {
+			DRM_DEBUG("Invalid syncobj handle provided\n");
+			err = -ENOENT;
+			goto err;
+		}
+
+		/*
+		 * For timeline syncobjs we need to preallocate chains for
+		 * later signaling.
+		 */
+		if (point != 0 && fence.flags & I915_EXEC_FENCE_SIGNAL) {
+			/*
+			 * Waiting and signaling the same point (when point !=
+			 * 0) would break the timeline.
+			 */
+			if (fence.flags & I915_EXEC_FENCE_WAIT) {
+				DRM_DEBUG("Tring to wait & signal the same timeline point.\n");
+				err = -EINVAL;
+				drm_syncobj_put(syncobj);
+				goto err;
+			}
+
+			fences[num_fences].chain_fence =
+				kmalloc(sizeof(*fences[num_fences].chain_fence),
+					GFP_KERNEL);
+			if (!fences[num_fences].chain_fence) {
+				drm_syncobj_put(syncobj);
+				err = -ENOMEM;
+				DRM_DEBUG("Unable to alloc chain_fence\n");
+				goto err;
+			}
+		} else {
+			fences[num_fences].chain_fence = NULL;
+		}
+
+		fences[num_fences].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
+		fences[num_fences].value = point;
+		num_fences++;
+	}
+
+	eb->fences = fences;
+	eb->n_fences = num_fences;
+
+	return 0;
+
+err:
+	__free_fence_array(fences, num_fences);
+	return err;
+}
+
+static int
+get_legacy_fence_array(struct drm_i915_gem_execbuffer2 *args,
+		       struct i915_execbuffer *eb)
 {
 	const unsigned long nfences = args->num_cliprects;
 	struct drm_i915_gem_exec_fence __user *user;
@@ -2235,7 +2344,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
 	BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
 	if (nfences > min_t(unsigned long,
 			    ULONG_MAX / sizeof(*user),
-			    SIZE_MAX / sizeof(*fences)))
+			    SIZE_MAX / sizeof(*eb->fences)))
 		return -EINVAL;
 
 	user = u64_to_user_ptr(args->cliprects_ptr);
@@ -2272,6 +2381,8 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
 			     ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
 
 		fences[n].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
+		fences[n].value = 0;
+		fences[n].chain_fence = NULL;
 	}
 
 	eb->fences = fences;
@@ -2303,6 +2414,15 @@ await_fence_array(struct i915_execbuffer *eb)
 		if (!fence)
 			return -EINVAL;
 
+		if (eb->fences[n].value) {
+			err = dma_fence_chain_find_seqno(&fence, eb->fences[n].value);
+			if (err)
+				return err;
+
+			if (!fence)
+				continue;
+		}
+
 		err = i915_request_await_dma_fence(eb->request, fence);
 		dma_fence_put(fence);
 		if (err < 0)
@@ -2326,7 +2446,17 @@ signal_fence_array(struct i915_execbuffer *eb)
 		if (!(flags & I915_EXEC_FENCE_SIGNAL))
 			continue;
 
-		drm_syncobj_replace_fence(syncobj, fence);
+		if (eb->fences[n].chain_fence) {
+			drm_syncobj_add_point(syncobj, eb->fences[n].chain_fence,
+					      fence, eb->fences[n].value);
+			/*
+			 * The chain's ownership is transferred to the
+			 * timeline.
+			 */
+			eb->fences[n].chain_fence = NULL;
+		} else {
+			drm_syncobj_replace_fence(syncobj, fence);
+		}
 	}
 }
 
@@ -2371,7 +2501,32 @@ static void eb_request_add(struct i915_execbuffer *eb)
 	mutex_unlock(&tl->mutex);
 }
 
+static int parse_timeline_fences(struct i915_user_extension __user *ext, void *data)
+{
+	struct drm_i915_gem_execbuffer_ext_timeline_fences timeline_fences;
+	struct i915_execbuffer *eb = data;
+
+	/* Timeline fences are incompatible with the fence array flag. */
+	if (eb->args->flags & I915_EXEC_FENCE_ARRAY)
+		return -EINVAL;
+
+	/*
+	 * Prevent the drm_i915_gem_execbuffer_ext_timeline_fences structure
+	 * to be included multiple times.
+	 */
+	if (eb->extension_flags & BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES))
+		return -EINVAL;
+
+	if (copy_from_user(&timeline_fences, ext, sizeof(timeline_fences)))
+		return -EFAULT;
+
+	eb->extension_flags |= BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES);
+
+	return get_timeline_fence_array(&timeline_fences, eb);
+}
+
 static const i915_user_extension_fn execbuf_extensions[] = {
+	[DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES] = parse_timeline_fences,
 };
 
 static int
@@ -2475,7 +2630,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
 	if (err)
 		goto err_out_fence;
 
-	err = get_fence_array(args, &eb);
+	err = get_legacy_fence_array(args, &eb);
 	if (err)
 		goto err_arr_fence;
 
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 67102dc26fce..f450d8782ac1 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1851,7 +1851,8 @@ static struct drm_driver driver = {
 	 */
 	.driver_features =
 	    DRIVER_GEM |
-	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
+	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ |
+	    DRIVER_SYNCOBJ_TIMELINE,
 	.release = i915_driver_release,
 	.open = i915_driver_open,
 	.lastclose = i915_driver_lastclose,
diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
index 40390b2352b1..e50bdf676955 100644
--- a/drivers/gpu/drm/i915/i915_getparam.c
+++ b/drivers/gpu/drm/i915/i915_getparam.c
@@ -132,6 +132,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_EXEC_BATCH_FIRST:
 	case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
 	case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
+	case I915_PARAM_HAS_EXEC_TIMELINE_FENCES:
 		/* For the time being all of these are always true;
 		 * if some supported hardware does not have one of these
 		 * features this value needs to be provided from
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 7ea38aa6502c..85ff36faf15a 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -619,6 +619,13 @@ typedef struct drm_i915_irq_wait {
  */
 #define I915_PARAM_PERF_REVISION	54
 
+/* Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying an array of
+ * timeline syncobj through drm_i915_gem_execbuf_ext_timeline_fences. See
+ * I915_EXEC_USE_EXTENSIONS.
+ */
+#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
+
+
 /* Must be kept compact -- no holes and well documented */
 
 typedef struct drm_i915_getparam {
@@ -1047,9 +1054,41 @@ struct drm_i915_gem_exec_fence {
 };
 
 enum drm_i915_gem_execbuffer_ext {
+	/**
+	 * See drm_i915_gem_execbuf_ext_timeline_fences.
+	 */
+	DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES = 1,
+
 	DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */
 };
 
+/**
+ * This structure describes an array of drm_syncobj and associated points for
+ * timeline variants of drm_syncobj. It is invalid to append this structure to
+ * the execbuf if I915_EXEC_FENCE_ARRAY is set.
+ */
+struct drm_i915_gem_execbuffer_ext_timeline_fences {
+	struct i915_user_extension base;
+
+	/**
+	 * Number of element in the handles_ptr & value_ptr arrays.
+	 */
+	__u64 fence_count;
+
+	/**
+	 * Pointer to an array of struct drm_i915_gem_exec_fence of length
+	 * fence_count.
+	 */
+	__u64 handles_ptr;
+
+	/**
+	 * Pointer to an array of u64 values of length fence_count. Values
+	 * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
+	 * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
+	 */
+	__u64 values_ptr;
+};
+
 struct drm_i915_gem_execbuffer2 {
 	/**
 	 * List of gem_exec_object2 structs
-- 
2.27.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 1/3] drm/i915: introduce a mechanism to extend execbuf2 Lionel Landwerlin
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support Lionel Landwerlin
@ 2020-07-08 13:17 ` Lionel Landwerlin
  2020-07-29 12:36   ` daniel
  2020-07-08 14:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: timeline semaphore support Patchwork
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-08 13:17 UTC (permalink / raw)
  To: intel-gfx

To allow faster engine to engine synchronization, peel the layer of
dma-fence-chain to expose potential i915 fences so that the
i915-request code can emit HW semaphore wait/signal operations in the
ring which is faster than waking up the host to submit unblocked
workloads after interrupt notification.

v2: Also deal with chains where the last node is not a dma-fence-chain

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 39 ++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index d8814e637e71..3ffd95d1dc2c 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2403,6 +2403,7 @@ await_fence_array(struct i915_execbuffer *eb)
 
 	for (n = 0; n < eb->n_fences; n++) {
 		struct drm_syncobj *syncobj;
+		struct dma_fence_chain *chain;
 		struct dma_fence *fence;
 		unsigned int flags;
 
@@ -2423,7 +2424,43 @@ await_fence_array(struct i915_execbuffer *eb)
 				continue;
 		}
 
-		err = i915_request_await_dma_fence(eb->request, fence);
+		chain = to_dma_fence_chain(fence);
+		if (chain) {
+			struct dma_fence *iter;
+
+			/*
+			 * If we're dealing with a dma-fence-chain, peel the
+			 * chain by adding all of the unsignaled fences
+			 * (dma_fence_chain_for_each does that for us) the
+			 * chain points to.
+			 *
+			 * This enables us to identify waits on i915 fences
+			 * and allows for faster engine-to-engine
+			 * synchronization using HW semaphores.
+			 */
+			dma_fence_chain_for_each(iter, fence) {
+				struct dma_fence_chain *iter_chain =
+					to_dma_fence_chain(iter);
+
+				/*
+				 * It is possible that the last item in the
+				 * chain is not a dma_fence_chain.
+				 */
+				if (iter_chain) {
+					err = i915_request_await_dma_fence(eb->request,
+									   iter_chain->fence);
+				} else {
+					err = i915_request_await_dma_fence(eb->request, iter);
+				}
+				if (err < 0) {
+					dma_fence_put(iter);
+					break;
+				}
+			}
+		} else {
+			err = i915_request_await_dma_fence(eb->request, fence);
+		}
+
 		dma_fence_put(fence);
 		if (err < 0)
 			return err;
-- 
2.27.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: timeline semaphore support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
                   ` (2 preceding siblings ...)
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences Lionel Landwerlin
@ 2020-07-08 14:28 ` Patchwork
  2020-07-08 14:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-07-08 14:28 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: timeline semaphore support
URL   : https://patchwork.freedesktop.org/series/79247/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
7c4ea18112db drm/i915: introduce a mechanism to extend execbuf2
-:376: CHECK:SPACING: spaces preferred around that '<<' (ctx:VxV)
#376: FILE: include/uapi/drm/i915_drm.h:1204:
+#define __I915_EXEC_UNKNOWN_FLAGS (-(I915_EXEC_USE_EXTENSIONS<<1))
                                                              ^

total: 0 errors, 0 warnings, 1 checks, 333 lines checked
49e6f714bbd2 drm/i915: add syncobj timeline support
-:25: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#25: 
    https://lists.freedesktop.org/archives/dri-devel/2019-August/229287.html

-:307: CHECK:LINE_SPACING: Please don't use multiple blank lines
#307: FILE: include/uapi/drm/i915_drm.h:628:
+
+

total: 0 errors, 1 warnings, 1 checks, 291 lines checked
f2a110c31221 drm/i915: peel dma-fence-chains wait fences

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: timeline semaphore support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
                   ` (3 preceding siblings ...)
  2020-07-08 14:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: timeline semaphore support Patchwork
@ 2020-07-08 14:29 ` Patchwork
  2020-07-08 14:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-07-08 14:29 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: timeline semaphore support
URL   : https://patchwork.freedesktop.org/series/79247/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/display/intel_display.c:1223:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1226:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1229:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/display/intel_display.c:1232:22: error: Expected constant expression in case statement
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2270:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2271:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2272:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2273:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2274:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gem/i915_gem_context.c:2275:17: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/intel_lrc.c:2785:17: error: too long token expansion
+drivers/gpu/drm/i915/gt/intel_lrc.c:2785:17: error: too long token expansion
+drivers/gpu/drm/i915/gt/intel_reset.c:1310:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gt/sysfs_engines.c:61:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/sysfs_engines.c:62:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gt/sysfs_engines.c:66:10: error: bad integer constant expression
+drivers/gpu/drm/i915/gvt/mmio.c:287:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1425:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1479:15: warning: memset with byte count of 16777216
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:408:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: timeline semaphore support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
                   ` (4 preceding siblings ...)
  2020-07-08 14:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2020-07-08 14:49 ` Patchwork
  2020-07-08 17:57 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  2020-07-15 11:11 ` [Intel-gfx] [PATCH v12 0/3] " Lionel Landwerlin
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-07-08 14:49 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: timeline semaphore support
URL   : https://patchwork.freedesktop.org/series/79247/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8715 -> Patchwork_18107
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/index.html

Known issues
------------

  Here are the changes found in Patchwork_18107 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@double-flink:
    - fi-tgl-y:           [PASS][1] -> [DMESG-WARN][2] ([i915#402])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-tgl-y/igt@gem_flink_basic@double-flink.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-tgl-y/igt@gem_flink_basic@double-flink.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-kefka:       [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-bsw-kefka/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Possible fixes ####

  * igt@gem_exec_store@basic:
    - fi-tgl-y:           [DMESG-WARN][5] ([i915#402]) -> [PASS][6] +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-tgl-y/igt@gem_exec_store@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-tgl-y/igt@gem_exec_store@basic.html

  * igt@i915_module_load@reload:
    - fi-tgl-u2:          [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-tgl-u2/igt@i915_module_load@reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-tgl-u2/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@gem_contexts:
    - fi-tgl-u2:          [INCOMPLETE][9] ([i915#1932] / [i915#2045]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-icl-u2:          [DMESG-WARN][11] ([i915#1982]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-modeset@b-dsi1:
    - {fi-tgl-dsi}:       [DMESG-WARN][13] ([i915#1982]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-modeset@b-dsi1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-modeset@b-dsi1.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][15] ([i915#62] / [i915#95]) -> [SKIP][16] ([fdo#109271])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([i915#62] / [i915#92]) -> [DMESG-WARN][18] ([i915#62] / [i915#92] / [i915#95]) +5 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_force_connector_basic@force-edid:
    - fi-kbl-x1275:       [DMESG-WARN][19] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][20] ([i915#62] / [i915#92]) +3 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1932]: https://gitlab.freedesktop.org/drm/intel/issues/1932
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (44 -> 37)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8715 -> Patchwork_18107

  CI-20190529: 20190529
  CI_DRM_8715: 76c2d43437601608d75f61a9eb6cf3a7aae5e02b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5728: 6988eebf78e9ce9746b8c2b7d21cb4174d6623a9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18107: f2a110c3122153a26a5c39feaab16b398ae0bdb2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f2a110c31221 drm/i915: peel dma-fence-chains wait fences
49e6f714bbd2 drm/i915: add syncobj timeline support
7c4ea18112db drm/i915: introduce a mechanism to extend execbuf2

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915: timeline semaphore support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
                   ` (5 preceding siblings ...)
  2020-07-08 14:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-07-08 17:57 ` Patchwork
  2020-07-15 11:11 ` [Intel-gfx] [PATCH v12 0/3] " Lionel Landwerlin
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-07-08 17:57 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: timeline semaphore support
URL   : https://patchwork.freedesktop.org/series/79247/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8715_full -> Patchwork_18107_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_18107_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_18107_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_18107_full:

### IGT changes ###

#### Possible regressions ####

  * igt@gem_eio@unwedge-stress:
    - shard-tglb:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-tglb5/igt@gem_eio@unwedge-stress.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-tglb6/igt@gem_eio@unwedge-stress.html

  * igt@kms_draw_crc@draw-method-rgb565-blt-xtiled:
    - shard-hsw:          NOTRUN -> [INCOMPLETE][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-hsw6/igt@kms_draw_crc@draw-method-rgb565-blt-xtiled.html

  
Known issues
------------

  Here are the changes found in Patchwork_18107_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@engines-mixed-process@vecs0:
    - shard-apl:          [PASS][4] -> [FAIL][5] ([i915#1528])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl8/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl4/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html

  * igt@gem_ctx_persistence@legacy-engines-mixed-process@render:
    - shard-skl:          [PASS][6] -> [FAIL][7] ([i915#1528])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl8/igt@gem_ctx_persistence@legacy-engines-mixed-process@render.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl4/igt@gem_ctx_persistence@legacy-engines-mixed-process@render.html

  * igt@gem_exec_fence@basic-wait-all:
    - shard-apl:          [PASS][8] -> [DMESG-WARN][9] ([i915#1635] / [i915#95]) +17 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl8/igt@gem_exec_fence@basic-wait-all.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl4/igt@gem_exec_fence@basic-wait-all.html

  * igt@gem_mmap_offset@isolation:
    - shard-iclb:         [PASS][10] -> [DMESG-WARN][11] ([i915#1982])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-iclb5/igt@gem_mmap_offset@isolation.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-iclb5/igt@gem_mmap_offset@isolation.html

  * igt@i915_pm_rc6_residency@rc6-fence:
    - shard-hsw:          [PASS][12] -> [WARN][13] ([i915#1519])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-hsw7/igt@i915_pm_rc6_residency@rc6-fence.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-hsw1/igt@i915_pm_rc6_residency@rc6-fence.html

  * igt@i915_selftest@mock@requests:
    - shard-skl:          [PASS][14] -> [INCOMPLETE][15] ([i915#198] / [i915#2110])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl9/igt@i915_selftest@mock@requests.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl10/igt@i915_selftest@mock@requests.html

  * igt@kms_addfb_basic@size-max:
    - shard-skl:          [PASS][16] -> [DMESG-WARN][17] ([i915#1982]) +8 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl6/igt@kms_addfb_basic@size-max.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl9/igt@kms_addfb_basic@size-max.html

  * igt@kms_big_fb@linear-16bpp-rotate-0:
    - shard-kbl:          [PASS][18] -> [DMESG-FAIL][19] ([i915#95])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl3/igt@kms_big_fb@linear-16bpp-rotate-0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl7/igt@kms_big_fb@linear-16bpp-rotate-0.html
    - shard-apl:          [PASS][20] -> [DMESG-FAIL][21] ([i915#1635] / [i915#95])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl7/igt@kms_big_fb@linear-16bpp-rotate-0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl4/igt@kms_big_fb@linear-16bpp-rotate-0.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding:
    - shard-snb:          [PASS][22] -> [TIMEOUT][23] ([i915#1958] / [i915#2119])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-snb5/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-snb6/igt@kms_cursor_crc@pipe-a-cursor-256x256-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement:
    - shard-kbl:          [PASS][24] -> [DMESG-WARN][25] ([i915#93] / [i915#95]) +1 similar issue
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl1/igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl3/igt@kms_cursor_crc@pipe-b-cursor-128x128-rapid-movement.html

  * igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge:
    - shard-tglb:         [PASS][26] -> [DMESG-WARN][27] ([i915#402])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-tglb1/igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-tglb7/igt@kms_cursor_edge_walk@pipe-a-64x64-right-edge.html

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size:
    - shard-hsw:          [PASS][28] -> [FAIL][29] ([i915#57])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-hsw4/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-hsw6/igt@kms_cursor_legacy@cursor-vs-flip-atomic-transitions-varying-size.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [PASS][30] -> [DMESG-WARN][31] ([i915#180]) +5 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl2/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate@c-edp1:
    - shard-skl:          [PASS][32] -> [FAIL][33] ([i915#2122])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl5/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl2/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-tglb:         [PASS][34] -> [DMESG-WARN][35] ([i915#1982])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-tglb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [PASS][36] -> [FAIL][37] ([fdo#108145] / [i915#265])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][38] -> [SKIP][39] ([fdo#109441]) +3 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@perf@blocking-parameterized:
    - shard-iclb:         [PASS][40] -> [FAIL][41] ([i915#1542])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-iclb7/igt@perf@blocking-parameterized.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-iclb4/igt@perf@blocking-parameterized.html

  
#### Possible fixes ####

  * igt@drm_read@short-buffer-wakeup:
    - shard-skl:          [DMESG-WARN][42] ([i915#1982]) -> [PASS][43] +6 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl5/igt@drm_read@short-buffer-wakeup.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl7/igt@drm_read@short-buffer-wakeup.html

  * igt@gem_ctx_param@non-root-set:
    - shard-apl:          [DMESG-WARN][44] ([i915#1635] / [i915#95]) -> [PASS][45] +15 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl6/igt@gem_ctx_param@non-root-set.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl8/igt@gem_ctx_param@non-root-set.html

  * igt@gem_exec_balancer@bonded-early:
    - shard-iclb:         [FAIL][46] ([i915#926]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-iclb1/igt@gem_exec_balancer@bonded-early.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-iclb4/igt@gem_exec_balancer@bonded-early.html

  * igt@gem_exec_reloc@basic-concurrent0:
    - shard-glk:          [FAIL][48] ([i915#1930]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-glk8/igt@gem_exec_reloc@basic-concurrent0.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-glk3/igt@gem_exec_reloc@basic-concurrent0.html

  * igt@gem_exec_whisper@basic-contexts:
    - shard-glk:          [DMESG-WARN][50] ([i915#118] / [i915#95]) -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-glk2/igt@gem_exec_whisper@basic-contexts.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-glk9/igt@gem_exec_whisper@basic-contexts.html

  * igt@gem_set_tiling_vs_pwrite:
    - shard-kbl:          [DMESG-WARN][52] ([i915#93] / [i915#95]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl1/igt@gem_set_tiling_vs_pwrite.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl3/igt@gem_set_tiling_vs_pwrite.html

  * igt@i915_module_load@reload-with-fault-injection:
    - shard-tglb:         [DMESG-WARN][54] ([i915#402]) -> [PASS][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-tglb5/igt@i915_module_load@reload-with-fault-injection.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-tglb1/igt@i915_module_load@reload-with-fault-injection.html

  * igt@kms_flip@flip-vs-suspend-interruptible@b-edp1:
    - shard-skl:          [INCOMPLETE][56] ([i915#198]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl1/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl5/igt@kms_flip@flip-vs-suspend-interruptible@b-edp1.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1:
    - shard-hsw:          [INCOMPLETE][58] ([i915#2055]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-hsw4/igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-hsw6/igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-kbl:          [DMESG-WARN][60] ([i915#180]) -> [PASS][61] +6 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl2/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl1/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_frontbuffer_tracking@fbc-badstride:
    - shard-glk:          [DMESG-WARN][62] ([i915#1982]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-glk8/igt@kms_frontbuffer_tracking@fbc-badstride.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-glk3/igt@kms_frontbuffer_tracking@fbc-badstride.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-skl:          [FAIL][64] ([i915#49]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl4/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-cpu.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-tglb:         [DMESG-WARN][66] ([i915#1982]) -> [PASS][67] +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-tglb2/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-tglb6/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-pwrite.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [FAIL][68] ([fdo#108145] / [i915#265]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-skl6/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][70] ([fdo#109441]) -> [PASS][71] +1 similar issue
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-iclb4/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@perf_pmu@semaphore-busy@rcs0:
    - shard-kbl:          [FAIL][72] ([i915#1820]) -> [PASS][73]
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl3/igt@perf_pmu@semaphore-busy@rcs0.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl6/igt@perf_pmu@semaphore-busy@rcs0.html

  
#### Warnings ####

  * igt@gem_exec_reloc@basic-concurrent16:
    - shard-snb:          [FAIL][74] ([i915#1930]) -> [TIMEOUT][75] ([i915#1958] / [i915#2119])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-snb5/igt@gem_exec_reloc@basic-concurrent16.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-snb6/igt@gem_exec_reloc@basic-concurrent16.html

  * igt@gem_vm_create@execbuf:
    - shard-snb:          [SKIP][76] ([fdo#109271]) -> [TIMEOUT][77] ([i915#1958] / [i915#2119]) +1 similar issue
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-snb5/igt@gem_vm_create@execbuf.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-snb6/igt@gem_vm_create@execbuf.html

  * igt@kms_ccs@pipe-c-bad-rotation-90:
    - shard-apl:          [SKIP][78] ([fdo#109271]) -> [SKIP][79] ([fdo#109271] / [i915#1635]) +5 similar issues
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl8/igt@kms_ccs@pipe-c-bad-rotation-90.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl4/igt@kms_ccs@pipe-c-bad-rotation-90.html

  * igt@kms_cursor_legacy@cursora-vs-flipb-varying-size:
    - shard-apl:          [SKIP][80] ([fdo#109271] / [i915#1635]) -> [SKIP][81] ([fdo#109271]) +5 similar issues
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl6/igt@kms_cursor_legacy@cursora-vs-flipb-varying-size.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl3/igt@kms_cursor_legacy@cursora-vs-flipb-varying-size.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-apl:          [FAIL][82] ([i915#49]) -> [DMESG-FAIL][83] ([i915#1635] / [i915#49] / [i915#95])
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][84] ([i915#93] / [i915#95]) -> [DMESG-WARN][85] ([i915#180] / [i915#93] / [i915#95])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-kbl2/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          [DMESG-FAIL][86] ([fdo#108145] / [i915#1635] / [i915#95]) -> [FAIL][87] ([fdo#108145] / [i915#265])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl8/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl4/igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][88], [FAIL][89], [FAIL][90]) ([i915#1610] / [i915#1635] / [i915#2110]) -> [FAIL][91] ([i915#1635] / [i915#2110])
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl1/igt@runner@aborted.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl3/igt@runner@aborted.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8715/shard-apl2/igt@runner@aborted.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/shard-apl3/igt@runner@aborted.html

  
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1519]: https://gitlab.freedesktop.org/drm/intel/issues/1519
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1820]: https://gitlab.freedesktop.org/drm/intel/issues/1820
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
  [i915#2110]: https://gitlab.freedesktop.org/drm/intel/issues/2110
  [i915#2119]: https://gitlab.freedesktop.org/drm/intel/issues/2119
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#57]: https://gitlab.freedesktop.org/drm/intel/issues/57
  [i915#926]: https://gitlab.freedesktop.org/drm/intel/issues/926
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8715 -> Patchwork_18107

  CI-20190529: 20190529
  CI_DRM_8715: 76c2d43437601608d75f61a9eb6cf3a7aae5e02b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5728: 6988eebf78e9ce9746b8c2b7d21cb4174d6623a9 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_18107: f2a110c3122153a26a5c39feaab16b398ae0bdb2 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18107/index.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support
  2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
                   ` (6 preceding siblings ...)
  2020-07-08 17:57 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2020-07-15 11:11 ` Lionel Landwerlin
  2020-07-22  7:10   ` Lionel Landwerlin
  7 siblings, 1 reply; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-15 11:11 UTC (permalink / raw)
  To: intel-gfx

Ping?

On 08/07/2020 16:17, Lionel Landwerlin wrote:
> Hi all,
>
> This is resuming the work on trying to get timeline semaphore support
> for i915 upstream, now that some selftests have been added to
> dma-fence-chain.
>
> There are a few fix from the last iteration and a rebase following the
> changes in the upstream execbuf code.
>
> Cheers,
>
> Lionel Landwerlin (3):
>    drm/i915: introduce a mechanism to extend execbuf2
>    drm/i915: add syncobj timeline support
>    drm/i915: peel dma-fence-chains wait fences
>
>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 333 +++++++++++++++---
>   drivers/gpu/drm/i915/i915_drv.c               |   3 +-
>   drivers/gpu/drm/i915/i915_getparam.c          |   1 +
>   include/uapi/drm/i915_drm.h                   |  65 +++-
>   4 files changed, 342 insertions(+), 60 deletions(-)
>
> --
> 2.27.0


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support
  2020-07-15 11:11 ` [Intel-gfx] [PATCH v12 0/3] " Lionel Landwerlin
@ 2020-07-22  7:10   ` Lionel Landwerlin
  0 siblings, 0 replies; 18+ messages in thread
From: Lionel Landwerlin @ 2020-07-22  7:10 UTC (permalink / raw)
  To: intel-gfx

Ping?

On 15/07/2020 14:11, Lionel Landwerlin wrote:
> Ping?
>
> On 08/07/2020 16:17, Lionel Landwerlin wrote:
>> Hi all,
>>
>> This is resuming the work on trying to get timeline semaphore support
>> for i915 upstream, now that some selftests have been added to
>> dma-fence-chain.
>>
>> There are a few fix from the last iteration and a rebase following the
>> changes in the upstream execbuf code.
>>
>> Cheers,
>>
>> Lionel Landwerlin (3):
>>    drm/i915: introduce a mechanism to extend execbuf2
>>    drm/i915: add syncobj timeline support
>>    drm/i915: peel dma-fence-chains wait fences
>>
>>   .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 333 +++++++++++++++---
>>   drivers/gpu/drm/i915/i915_drv.c               |   3 +-
>>   drivers/gpu/drm/i915/i915_getparam.c          |   1 +
>>   include/uapi/drm/i915_drm.h                   |  65 +++-
>>   4 files changed, 342 insertions(+), 60 deletions(-)
>>
>> -- 
>> 2.27.0
>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support Lionel Landwerlin
@ 2020-07-29 12:24     ` Daniel Vetter
  2020-07-29 12:34   ` daniel
  1 sibling, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-07-29 12:24 UTC (permalink / raw)
  To: Lionel Landwerlin, Kees Cook, Linus Torvalds, Linux Kernel Mailing List
  Cc: intel-gfx

Hi Kees&Linus,

Not sure you're the right person, but figured I start with you with an
idea I had while looking at this patch. Cut out everything except the
relevant part:

On Wed, Jul 8, 2020 at 3:18 PM Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote:
> +       /* Check multiplication overflow for access_ok() and kvmalloc_array() */
> +       BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
> +       if (num_user_fences > min_t(unsigned long,
> +                                   ULONG_MAX / sizeof(*user_fences),
> +                                   SIZE_MAX / sizeof(*fences)))
> +               return -EINVAL;
> +
> +       user_fences = u64_to_user_ptr(timeline_fences->handles_ptr);
> +       if (!access_ok(user_fences, num_user_fences * sizeof(*user_fences)))
> +               return -EFAULT;
> +
> +       user_values = u64_to_user_ptr(timeline_fences->values_ptr);
> +       if (!access_ok(user_values, num_user_fences * sizeof(*user_values)))
> +               return -EFAULT;

Do we have a access_ok_array or so? Instead of duplicating overflow checks
everywhere and getting it all wrong ... Similar I guess for
copy_from/to_user_array.

Just random idea that crossed my mind and I figured I'll throw it out
there.

Cheers, Daniel

> +
> +       fences = kvmalloc_array(num_user_fences, sizeof(*fences),
> +                               __GFP_NOWARN | GFP_KERNEL);
> +       if (!fences)
> +               return -ENOMEM;
> +
> +       BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) &
> +                    ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
> +
> +       for (n = 0, num_fences = 0; n < timeline_fences->fence_count; n++) {
> +               struct drm_i915_gem_exec_fence fence;
> +               struct drm_syncobj *syncobj;
> +               u64 point;
> +
> +               if (__copy_from_user(&fence, user_fences++, sizeof(fence))) {
> +                       err = -EFAULT;
> +                       goto err;
> +               }
> +
> +               if (fence.flags & __I915_EXEC_FENCE_UNKNOWN_FLAGS) {
> +                       err = -EINVAL;
> +                       goto err;
> +               }
> +
> +               if (__get_user(point, user_values++)) {
> +                       err = -EFAULT;
> +                       goto err;
> +               }
> +
> +               syncobj = drm_syncobj_find(eb->file, fence.handle);
> +               if (!syncobj) {
> +                       DRM_DEBUG("Invalid syncobj handle provided\n");
> +                       err = -ENOENT;
> +                       goto err;
> +               }
> +
> +               /*
> +                * For timeline syncobjs we need to preallocate chains for
> +                * later signaling.
> +                */
> +               if (point != 0 && fence.flags & I915_EXEC_FENCE_SIGNAL) {
> +                       /*
> +                        * Waiting and signaling the same point (when point !=
> +                        * 0) would break the timeline.
> +                        */
> +                       if (fence.flags & I915_EXEC_FENCE_WAIT) {
> +                               DRM_DEBUG("Tring to wait & signal the same timeline point.\n");
> +                               err = -EINVAL;
> +                               drm_syncobj_put(syncobj);
> +                               goto err;
> +                       }
> +
> +                       fences[num_fences].chain_fence =
> +                               kmalloc(sizeof(*fences[num_fences].chain_fence),
> +                                       GFP_KERNEL);
> +                       if (!fences[num_fences].chain_fence) {
> +                               drm_syncobj_put(syncobj);
> +                               err = -ENOMEM;
> +                               DRM_DEBUG("Unable to alloc chain_fence\n");
> +                               goto err;
> +                       }
> +               } else {
> +                       fences[num_fences].chain_fence = NULL;
> +               }
> +
> +               fences[num_fences].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +               fences[num_fences].value = point;
> +               num_fences++;
> +       }
> +
> +       eb->fences = fences;
> +       eb->n_fences = num_fences;
> +
> +       return 0;
> +
> +err:
> +       __free_fence_array(fences, num_fences);
> +       return err;
> +}
> +
> +static int
> +get_legacy_fence_array(struct drm_i915_gem_execbuffer2 *args,
> +                      struct i915_execbuffer *eb)
>  {
>         const unsigned long nfences = args->num_cliprects;
>         struct drm_i915_gem_exec_fence __user *user;
> @@ -2235,7 +2344,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>         BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
>         if (nfences > min_t(unsigned long,
>                             ULONG_MAX / sizeof(*user),
> -                           SIZE_MAX / sizeof(*fences)))
> +                           SIZE_MAX / sizeof(*eb->fences)))
>                 return -EINVAL;
>
>         user = u64_to_user_ptr(args->cliprects_ptr);
> @@ -2272,6 +2381,8 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>                              ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
>
>                 fences[n].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +               fences[n].value = 0;
> +               fences[n].chain_fence = NULL;
>         }
>
>         eb->fences = fences;
> @@ -2303,6 +2414,15 @@ await_fence_array(struct i915_execbuffer *eb)
>                 if (!fence)
>                         return -EINVAL;
>
> +               if (eb->fences[n].value) {
> +                       err = dma_fence_chain_find_seqno(&fence, eb->fences[n].value);
> +                       if (err)
> +                               return err;
> +
> +                       if (!fence)
> +                               continue;
> +               }
> +
>                 err = i915_request_await_dma_fence(eb->request, fence);
>                 dma_fence_put(fence);
>                 if (err < 0)
> @@ -2326,7 +2446,17 @@ signal_fence_array(struct i915_execbuffer *eb)
>                 if (!(flags & I915_EXEC_FENCE_SIGNAL))
>                         continue;
>
> -               drm_syncobj_replace_fence(syncobj, fence);
> +               if (eb->fences[n].chain_fence) {
> +                       drm_syncobj_add_point(syncobj, eb->fences[n].chain_fence,
> +                                             fence, eb->fences[n].value);
> +                       /*
> +                        * The chain's ownership is transferred to the
> +                        * timeline.
> +                        */
> +                       eb->fences[n].chain_fence = NULL;
> +               } else {
> +                       drm_syncobj_replace_fence(syncobj, fence);
> +               }
>         }
>  }
>
> @@ -2371,7 +2501,32 @@ static void eb_request_add(struct i915_execbuffer *eb)
>         mutex_unlock(&tl->mutex);
>  }
>
> +static int parse_timeline_fences(struct i915_user_extension __user *ext, void *data)
> +{
> +       struct drm_i915_gem_execbuffer_ext_timeline_fences timeline_fences;
> +       struct i915_execbuffer *eb = data;
> +
> +       /* Timeline fences are incompatible with the fence array flag. */
> +       if (eb->args->flags & I915_EXEC_FENCE_ARRAY)
> +               return -EINVAL;
> +
> +       /*
> +        * Prevent the drm_i915_gem_execbuffer_ext_timeline_fences structure
> +        * to be included multiple times.
> +        */
> +       if (eb->extension_flags & BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&timeline_fences, ext, sizeof(timeline_fences)))
> +               return -EFAULT;
> +
> +       eb->extension_flags |= BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES);
> +
> +       return get_timeline_fence_array(&timeline_fences, eb);
> +}
> +
>  static const i915_user_extension_fn execbuf_extensions[] = {
> +       [DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES] = parse_timeline_fences,
>  };
>
>  static int
> @@ -2475,7 +2630,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>         if (err)
>                 goto err_out_fence;
>
> -       err = get_fence_array(args, &eb);
> +       err = get_legacy_fence_array(args, &eb);
>         if (err)
>                 goto err_arr_fence;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 67102dc26fce..f450d8782ac1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1851,7 +1851,8 @@ static struct drm_driver driver = {
>          */
>         .driver_features =
>             DRIVER_GEM |
> -           DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
> +           DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ |
> +           DRIVER_SYNCOBJ_TIMELINE,
>         .release = i915_driver_release,
>         .open = i915_driver_open,
>         .lastclose = i915_driver_lastclose,
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 40390b2352b1..e50bdf676955 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -132,6 +132,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>         case I915_PARAM_HAS_EXEC_BATCH_FIRST:
>         case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
>         case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
> +       case I915_PARAM_HAS_EXEC_TIMELINE_FENCES:
>                 /* For the time being all of these are always true;
>                  * if some supported hardware does not have one of these
>                  * features this value needs to be provided from
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ea38aa6502c..85ff36faf15a 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -619,6 +619,13 @@ typedef struct drm_i915_irq_wait {
>   */
>  #define I915_PARAM_PERF_REVISION       54
>
> +/* Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying an array of
> + * timeline syncobj through drm_i915_gem_execbuf_ext_timeline_fences. See
> + * I915_EXEC_USE_EXTENSIONS.
> + */
> +#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
> +
> +
>  /* Must be kept compact -- no holes and well documented */
>
>  typedef struct drm_i915_getparam {
> @@ -1047,9 +1054,41 @@ struct drm_i915_gem_exec_fence {
>  };
>
>  enum drm_i915_gem_execbuffer_ext {
> +       /**
> +        * See drm_i915_gem_execbuf_ext_timeline_fences.
> +        */
> +       DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES = 1,
> +
>         DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */
>  };
>
> +/**
> + * This structure describes an array of drm_syncobj and associated points for
> + * timeline variants of drm_syncobj. It is invalid to append this structure to
> + * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> + */
> +struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +       struct i915_user_extension base;
> +
> +       /**
> +        * Number of element in the handles_ptr & value_ptr arrays.
> +        */
> +       __u64 fence_count;
> +
> +       /**
> +        * Pointer to an array of struct drm_i915_gem_exec_fence of length
> +        * fence_count.
> +        */
> +       __u64 handles_ptr;
> +
> +       /**
> +        * Pointer to an array of u64 values of length fence_count. Values
> +        * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> +        * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> +        */
> +       __u64 values_ptr;
> +};
> +
>  struct drm_i915_gem_execbuffer2 {
>         /**
>          * List of gem_exec_object2 structs
> --
> 2.27.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
@ 2020-07-29 12:24     ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2020-07-29 12:24 UTC (permalink / raw)
  To: Lionel Landwerlin, Kees Cook, Linus Torvalds, Linux Kernel Mailing List
  Cc: intel-gfx

Hi Kees&Linus,

Not sure you're the right person, but figured I start with you with an
idea I had while looking at this patch. Cut out everything except the
relevant part:

On Wed, Jul 8, 2020 at 3:18 PM Lionel Landwerlin <lionel.g.landwerlin@intel.com> wrote:
> +       /* Check multiplication overflow for access_ok() and kvmalloc_array() */
> +       BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
> +       if (num_user_fences > min_t(unsigned long,
> +                                   ULONG_MAX / sizeof(*user_fences),
> +                                   SIZE_MAX / sizeof(*fences)))
> +               return -EINVAL;
> +
> +       user_fences = u64_to_user_ptr(timeline_fences->handles_ptr);
> +       if (!access_ok(user_fences, num_user_fences * sizeof(*user_fences)))
> +               return -EFAULT;
> +
> +       user_values = u64_to_user_ptr(timeline_fences->values_ptr);
> +       if (!access_ok(user_values, num_user_fences * sizeof(*user_values)))
> +               return -EFAULT;

Do we have a access_ok_array or so? Instead of duplicating overflow checks
everywhere and getting it all wrong ... Similar I guess for
copy_from/to_user_array.

Just random idea that crossed my mind and I figured I'll throw it out
there.

Cheers, Daniel

> +
> +       fences = kvmalloc_array(num_user_fences, sizeof(*fences),
> +                               __GFP_NOWARN | GFP_KERNEL);
> +       if (!fences)
> +               return -ENOMEM;
> +
> +       BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) &
> +                    ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
> +
> +       for (n = 0, num_fences = 0; n < timeline_fences->fence_count; n++) {
> +               struct drm_i915_gem_exec_fence fence;
> +               struct drm_syncobj *syncobj;
> +               u64 point;
> +
> +               if (__copy_from_user(&fence, user_fences++, sizeof(fence))) {
> +                       err = -EFAULT;
> +                       goto err;
> +               }
> +
> +               if (fence.flags & __I915_EXEC_FENCE_UNKNOWN_FLAGS) {
> +                       err = -EINVAL;
> +                       goto err;
> +               }
> +
> +               if (__get_user(point, user_values++)) {
> +                       err = -EFAULT;
> +                       goto err;
> +               }
> +
> +               syncobj = drm_syncobj_find(eb->file, fence.handle);
> +               if (!syncobj) {
> +                       DRM_DEBUG("Invalid syncobj handle provided\n");
> +                       err = -ENOENT;
> +                       goto err;
> +               }
> +
> +               /*
> +                * For timeline syncobjs we need to preallocate chains for
> +                * later signaling.
> +                */
> +               if (point != 0 && fence.flags & I915_EXEC_FENCE_SIGNAL) {
> +                       /*
> +                        * Waiting and signaling the same point (when point !=
> +                        * 0) would break the timeline.
> +                        */
> +                       if (fence.flags & I915_EXEC_FENCE_WAIT) {
> +                               DRM_DEBUG("Tring to wait & signal the same timeline point.\n");
> +                               err = -EINVAL;
> +                               drm_syncobj_put(syncobj);
> +                               goto err;
> +                       }
> +
> +                       fences[num_fences].chain_fence =
> +                               kmalloc(sizeof(*fences[num_fences].chain_fence),
> +                                       GFP_KERNEL);
> +                       if (!fences[num_fences].chain_fence) {
> +                               drm_syncobj_put(syncobj);
> +                               err = -ENOMEM;
> +                               DRM_DEBUG("Unable to alloc chain_fence\n");
> +                               goto err;
> +                       }
> +               } else {
> +                       fences[num_fences].chain_fence = NULL;
> +               }
> +
> +               fences[num_fences].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +               fences[num_fences].value = point;
> +               num_fences++;
> +       }
> +
> +       eb->fences = fences;
> +       eb->n_fences = num_fences;
> +
> +       return 0;
> +
> +err:
> +       __free_fence_array(fences, num_fences);
> +       return err;
> +}
> +
> +static int
> +get_legacy_fence_array(struct drm_i915_gem_execbuffer2 *args,
> +                      struct i915_execbuffer *eb)
>  {
>         const unsigned long nfences = args->num_cliprects;
>         struct drm_i915_gem_exec_fence __user *user;
> @@ -2235,7 +2344,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>         BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
>         if (nfences > min_t(unsigned long,
>                             ULONG_MAX / sizeof(*user),
> -                           SIZE_MAX / sizeof(*fences)))
> +                           SIZE_MAX / sizeof(*eb->fences)))
>                 return -EINVAL;
>
>         user = u64_to_user_ptr(args->cliprects_ptr);
> @@ -2272,6 +2381,8 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>                              ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
>
>                 fences[n].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +               fences[n].value = 0;
> +               fences[n].chain_fence = NULL;
>         }
>
>         eb->fences = fences;
> @@ -2303,6 +2414,15 @@ await_fence_array(struct i915_execbuffer *eb)
>                 if (!fence)
>                         return -EINVAL;
>
> +               if (eb->fences[n].value) {
> +                       err = dma_fence_chain_find_seqno(&fence, eb->fences[n].value);
> +                       if (err)
> +                               return err;
> +
> +                       if (!fence)
> +                               continue;
> +               }
> +
>                 err = i915_request_await_dma_fence(eb->request, fence);
>                 dma_fence_put(fence);
>                 if (err < 0)
> @@ -2326,7 +2446,17 @@ signal_fence_array(struct i915_execbuffer *eb)
>                 if (!(flags & I915_EXEC_FENCE_SIGNAL))
>                         continue;
>
> -               drm_syncobj_replace_fence(syncobj, fence);
> +               if (eb->fences[n].chain_fence) {
> +                       drm_syncobj_add_point(syncobj, eb->fences[n].chain_fence,
> +                                             fence, eb->fences[n].value);
> +                       /*
> +                        * The chain's ownership is transferred to the
> +                        * timeline.
> +                        */
> +                       eb->fences[n].chain_fence = NULL;
> +               } else {
> +                       drm_syncobj_replace_fence(syncobj, fence);
> +               }
>         }
>  }
>
> @@ -2371,7 +2501,32 @@ static void eb_request_add(struct i915_execbuffer *eb)
>         mutex_unlock(&tl->mutex);
>  }
>
> +static int parse_timeline_fences(struct i915_user_extension __user *ext, void *data)
> +{
> +       struct drm_i915_gem_execbuffer_ext_timeline_fences timeline_fences;
> +       struct i915_execbuffer *eb = data;
> +
> +       /* Timeline fences are incompatible with the fence array flag. */
> +       if (eb->args->flags & I915_EXEC_FENCE_ARRAY)
> +               return -EINVAL;
> +
> +       /*
> +        * Prevent the drm_i915_gem_execbuffer_ext_timeline_fences structure
> +        * to be included multiple times.
> +        */
> +       if (eb->extension_flags & BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES))
> +               return -EINVAL;
> +
> +       if (copy_from_user(&timeline_fences, ext, sizeof(timeline_fences)))
> +               return -EFAULT;
> +
> +       eb->extension_flags |= BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES);
> +
> +       return get_timeline_fence_array(&timeline_fences, eb);
> +}
> +
>  static const i915_user_extension_fn execbuf_extensions[] = {
> +       [DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES] = parse_timeline_fences,
>  };
>
>  static int
> @@ -2475,7 +2630,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>         if (err)
>                 goto err_out_fence;
>
> -       err = get_fence_array(args, &eb);
> +       err = get_legacy_fence_array(args, &eb);
>         if (err)
>                 goto err_arr_fence;
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 67102dc26fce..f450d8782ac1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1851,7 +1851,8 @@ static struct drm_driver driver = {
>          */
>         .driver_features =
>             DRIVER_GEM |
> -           DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
> +           DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ |
> +           DRIVER_SYNCOBJ_TIMELINE,
>         .release = i915_driver_release,
>         .open = i915_driver_open,
>         .lastclose = i915_driver_lastclose,
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 40390b2352b1..e50bdf676955 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -132,6 +132,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>         case I915_PARAM_HAS_EXEC_BATCH_FIRST:
>         case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
>         case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
> +       case I915_PARAM_HAS_EXEC_TIMELINE_FENCES:
>                 /* For the time being all of these are always true;
>                  * if some supported hardware does not have one of these
>                  * features this value needs to be provided from
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ea38aa6502c..85ff36faf15a 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -619,6 +619,13 @@ typedef struct drm_i915_irq_wait {
>   */
>  #define I915_PARAM_PERF_REVISION       54
>
> +/* Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying an array of
> + * timeline syncobj through drm_i915_gem_execbuf_ext_timeline_fences. See
> + * I915_EXEC_USE_EXTENSIONS.
> + */
> +#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
> +
> +
>  /* Must be kept compact -- no holes and well documented */
>
>  typedef struct drm_i915_getparam {
> @@ -1047,9 +1054,41 @@ struct drm_i915_gem_exec_fence {
>  };
>
>  enum drm_i915_gem_execbuffer_ext {
> +       /**
> +        * See drm_i915_gem_execbuf_ext_timeline_fences.
> +        */
> +       DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES = 1,
> +
>         DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */
>  };
>
> +/**
> + * This structure describes an array of drm_syncobj and associated points for
> + * timeline variants of drm_syncobj. It is invalid to append this structure to
> + * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> + */
> +struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +       struct i915_user_extension base;
> +
> +       /**
> +        * Number of element in the handles_ptr & value_ptr arrays.
> +        */
> +       __u64 fence_count;
> +
> +       /**
> +        * Pointer to an array of struct drm_i915_gem_exec_fence of length
> +        * fence_count.
> +        */
> +       __u64 handles_ptr;
> +
> +       /**
> +        * Pointer to an array of u64 values of length fence_count. Values
> +        * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> +        * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> +        */
> +       __u64 values_ptr;
> +};
> +
>  struct drm_i915_gem_execbuffer2 {
>         /**
>          * List of gem_exec_object2 structs
> --
> 2.27.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



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

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support Lionel Landwerlin
  2020-07-29 12:24     ` Daniel Vetter
@ 2020-07-29 12:34   ` daniel
  1 sibling, 0 replies; 18+ messages in thread
From: daniel @ 2020-07-29 12:34 UTC (permalink / raw)
  Cc: intel-gfx

On Wed, Jul 08, 2020 at 04:17:50PM +0300, Lionel Landwerlin wrote:
> Introduces a new parameters to execbuf so that we can specify syncobj
> handles as well as timeline points.
> 
> v2: Reuse i915_user_extension_fn
> 
> v3: Check that the chained extension is only present once (Chris)
> 
> v4: Check that dma_fence_chain_find_seqno returns a non NULL fence (Lionel)
> 
> v5: Use BIT_ULL (Chris)
> 
> v6: Fix issue with already signaled timeline points,
>     dma_fence_chain_find_seqno() setting fence to NULL (Chris)
> 
> v7: Report ENOENT with invalid syncobj handle (Lionel)
> 
> v8: Check for out of order timeline point insertion (Chris)
> 
> v9: After explanations on
>     https://lists.freedesktop.org/archives/dri-devel/2019-August/229287.html
>     drop the ordering check from v8 (Lionel)
> 
> v10: Set first extension enum item to 1 (Jason)
> 
> v11: Rebase
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 167 +++++++++++++++++-
>  drivers/gpu/drm/i915/i915_drv.c               |   3 +-
>  drivers/gpu/drm/i915/i915_getparam.c          |   1 +
>  include/uapi/drm/i915_drm.h                   |  39 ++++
>  4 files changed, 203 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 03a656feb7b8..d8814e637e71 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -285,6 +285,8 @@ struct i915_execbuffer {
>  
>  	struct i915_eb_fence {
>  		struct drm_syncobj *syncobj; /* Use with ptr_mask_bits() */
> +		u64 value;
> +		struct dma_fence_chain *chain_fence;
>  	} *fences;
>  	u32 n_fences;
>  
> @@ -2213,14 +2215,121 @@ eb_pin_engine(struct i915_execbuffer *eb,
>  static void
>  __free_fence_array(struct i915_eb_fence *fences, unsigned int n)
>  {
> -	while (n--)
> +	while (n--) {
>  		drm_syncobj_put(ptr_mask_bits(fences[n].syncobj, 2));
> +		kfree(fences[n].chain_fence);
> +	}
>  	kvfree(fences);
>  }
>  
>  static int
> -get_fence_array(struct drm_i915_gem_execbuffer2 *args,
> -		struct i915_execbuffer *eb)
> +get_timeline_fence_array(const struct drm_i915_gem_execbuffer_ext_timeline_fences *timeline_fences,
> +			 struct i915_execbuffer *eb)
> +{
> +	struct drm_i915_gem_exec_fence __user *user_fences;
> +	struct i915_eb_fence *fences;
> +	u64 __user *user_values;
> +	u64 num_fences, num_user_fences = timeline_fences->fence_count;
> +	unsigned long n;
> +	int err;
> +
> +	/* Check multiplication overflow for access_ok() and kvmalloc_array() */
> +	BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
> +	if (num_user_fences > min_t(unsigned long,
> +				    ULONG_MAX / sizeof(*user_fences),
> +				    SIZE_MAX / sizeof(*fences)))
> +		return -EINVAL;
> +
> +	user_fences = u64_to_user_ptr(timeline_fences->handles_ptr);
> +	if (!access_ok(user_fences, num_user_fences * sizeof(*user_fences)))
> +		return -EFAULT;
> +
> +	user_values = u64_to_user_ptr(timeline_fences->values_ptr);
> +	if (!access_ok(user_values, num_user_fences * sizeof(*user_values)))
> +		return -EFAULT;
> +
> +	fences = kvmalloc_array(num_user_fences, sizeof(*fences),
> +				__GFP_NOWARN | GFP_KERNEL);
> +	if (!fences)
> +		return -ENOMEM;
> +
> +	BUILD_BUG_ON(~(ARCH_KMALLOC_MINALIGN - 1) &
> +		     ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
> +
> +	for (n = 0, num_fences = 0; n < timeline_fences->fence_count; n++) {
> +		struct drm_i915_gem_exec_fence fence;
> +		struct drm_syncobj *syncobj;
> +		u64 point;
> +
> +		if (__copy_from_user(&fence, user_fences++, sizeof(fence))) {
> +			err = -EFAULT;
> +			goto err;
> +		}
> +
> +		if (fence.flags & __I915_EXEC_FENCE_UNKNOWN_FLAGS) {
> +			err = -EINVAL;
> +			goto err;
> +		}
> +
> +		if (__get_user(point, user_values++)) {
> +			err = -EFAULT;
> +			goto err;
> +		}
> +
> +		syncobj = drm_syncobj_find(eb->file, fence.handle);
> +		if (!syncobj) {
> +			DRM_DEBUG("Invalid syncobj handle provided\n");
> +			err = -ENOENT;
> +			goto err;
> +		}
> +
> +		/*
> +		 * For timeline syncobjs we need to preallocate chains for
> +		 * later signaling.
> +		 */
> +		if (point != 0 && fence.flags & I915_EXEC_FENCE_SIGNAL) {
> +			/*
> +			 * Waiting and signaling the same point (when point !=
> +			 * 0) would break the timeline.
> +			 */
> +			if (fence.flags & I915_EXEC_FENCE_WAIT) {
> +				DRM_DEBUG("Tring to wait & signal the same timeline point.\n");
> +				err = -EINVAL;
> +				drm_syncobj_put(syncobj);
> +				goto err;
> +			}
> +
> +			fences[num_fences].chain_fence =
> +				kmalloc(sizeof(*fences[num_fences].chain_fence),
> +					GFP_KERNEL);

Uh this kmalloc here I don't like, it's kinda fragile, plus we might want
to use a kmemcache with slab freeing for these eventually. But amdgpu and
msm do the same thing, so maybe a follow-up patch which adds a
dma_fence_chain_alloc() which is used everywhere?

The thing is that drm_syncobj.c allocates with kzalloc, so guaranteed all
cleared to 0.

Not a blocker for this, but would be really nice if you volunteer for this
:-)

> +			if (!fences[num_fences].chain_fence) {
> +				drm_syncobj_put(syncobj);
> +				err = -ENOMEM;
> +				DRM_DEBUG("Unable to alloc chain_fence\n");
> +				goto err;
> +			}
> +		} else {
> +			fences[num_fences].chain_fence = NULL;
> +		}
> +
> +		fences[num_fences].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +		fences[num_fences].value = point;
> +		num_fences++;
> +	}
> +
> +	eb->fences = fences;
> +	eb->n_fences = num_fences;
> +
> +	return 0;
> +
> +err:
> +	__free_fence_array(fences, num_fences);
> +	return err;
> +}
> +
> +static int
> +get_legacy_fence_array(struct drm_i915_gem_execbuffer2 *args,
> +		       struct i915_execbuffer *eb)
>  {
>  	const unsigned long nfences = args->num_cliprects;
>  	struct drm_i915_gem_exec_fence __user *user;
> @@ -2235,7 +2344,7 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>  	BUILD_BUG_ON(sizeof(size_t) > sizeof(unsigned long));
>  	if (nfences > min_t(unsigned long,
>  			    ULONG_MAX / sizeof(*user),
> -			    SIZE_MAX / sizeof(*fences)))
> +			    SIZE_MAX / sizeof(*eb->fences)))
>  		return -EINVAL;
>  
>  	user = u64_to_user_ptr(args->cliprects_ptr);
> @@ -2272,6 +2381,8 @@ get_fence_array(struct drm_i915_gem_execbuffer2 *args,
>  			     ~__I915_EXEC_FENCE_UNKNOWN_FLAGS);
>  
>  		fences[n].syncobj = ptr_pack_bits(syncobj, fence.flags, 2);
> +		fences[n].value = 0;
> +		fences[n].chain_fence = NULL;
>  	}
>  
>  	eb->fences = fences;
> @@ -2303,6 +2414,15 @@ await_fence_array(struct i915_execbuffer *eb)
>  		if (!fence)
>  			return -EINVAL;
>  
> +		if (eb->fences[n].value) {
> +			err = dma_fence_chain_find_seqno(&fence, eb->fences[n].value);
> +			if (err)
> +				return err;
> +
> +			if (!fence)
> +				continue;
> +		}
> +
>  		err = i915_request_await_dma_fence(eb->request, fence);
>  		dma_fence_put(fence);
>  		if (err < 0)
> @@ -2326,7 +2446,17 @@ signal_fence_array(struct i915_execbuffer *eb)
>  		if (!(flags & I915_EXEC_FENCE_SIGNAL))
>  			continue;
>  
> -		drm_syncobj_replace_fence(syncobj, fence);
> +		if (eb->fences[n].chain_fence) {
> +			drm_syncobj_add_point(syncobj, eb->fences[n].chain_fence,
> +					      fence, eb->fences[n].value);
> +			/*
> +			 * The chain's ownership is transferred to the
> +			 * timeline.
> +			 */
> +			eb->fences[n].chain_fence = NULL;
> +		} else {
> +			drm_syncobj_replace_fence(syncobj, fence);
> +		}
>  	}
>  }
>  
> @@ -2371,7 +2501,32 @@ static void eb_request_add(struct i915_execbuffer *eb)
>  	mutex_unlock(&tl->mutex);
>  }
>  
> +static int parse_timeline_fences(struct i915_user_extension __user *ext, void *data)
> +{
> +	struct drm_i915_gem_execbuffer_ext_timeline_fences timeline_fences;
> +	struct i915_execbuffer *eb = data;
> +
> +	/* Timeline fences are incompatible with the fence array flag. */
> +	if (eb->args->flags & I915_EXEC_FENCE_ARRAY)
> +		return -EINVAL;
> +
> +	/*
> +	 * Prevent the drm_i915_gem_execbuffer_ext_timeline_fences structure
> +	 * to be included multiple times.
> +	 */
> +	if (eb->extension_flags & BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES))
> +		return -EINVAL;
> +
> +	if (copy_from_user(&timeline_fences, ext, sizeof(timeline_fences)))
> +		return -EFAULT;
> +
> +	eb->extension_flags |= BIT_ULL(DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES);
> +
> +	return get_timeline_fence_array(&timeline_fences, eb);
> +}
> +
>  static const i915_user_extension_fn execbuf_extensions[] = {
> +	[DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES] = parse_timeline_fences,
>  };
>  
>  static int
> @@ -2475,7 +2630,7 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>  	if (err)
>  		goto err_out_fence;
>  
> -	err = get_fence_array(args, &eb);
> +	err = get_legacy_fence_array(args, &eb);
>  	if (err)
>  		goto err_arr_fence;
>  
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 67102dc26fce..f450d8782ac1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1851,7 +1851,8 @@ static struct drm_driver driver = {
>  	 */
>  	.driver_features =
>  	    DRIVER_GEM |
> -	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ,
> +	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_ATOMIC | DRIVER_SYNCOBJ |
> +	    DRIVER_SYNCOBJ_TIMELINE,
>  	.release = i915_driver_release,
>  	.open = i915_driver_open,
>  	.lastclose = i915_driver_lastclose,
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 40390b2352b1..e50bdf676955 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -132,6 +132,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_EXEC_BATCH_FIRST:
>  	case I915_PARAM_HAS_EXEC_FENCE_ARRAY:
>  	case I915_PARAM_HAS_EXEC_SUBMIT_FENCE:
> +	case I915_PARAM_HAS_EXEC_TIMELINE_FENCES:
>  		/* For the time being all of these are always true;
>  		 * if some supported hardware does not have one of these
>  		 * features this value needs to be provided from
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 7ea38aa6502c..85ff36faf15a 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -619,6 +619,13 @@ typedef struct drm_i915_irq_wait {
>   */
>  #define I915_PARAM_PERF_REVISION	54
>  
> +/* Query whether DRM_I915_GEM_EXECBUFFER2 supports supplying an array of
> + * timeline syncobj through drm_i915_gem_execbuf_ext_timeline_fences. See
> + * I915_EXEC_USE_EXTENSIONS.
> + */
> +#define I915_PARAM_HAS_EXEC_TIMELINE_FENCES 55
> +
> +
>  /* Must be kept compact -- no holes and well documented */
>  
>  typedef struct drm_i915_getparam {
> @@ -1047,9 +1054,41 @@ struct drm_i915_gem_exec_fence {
>  };
>  
>  enum drm_i915_gem_execbuffer_ext {
> +	/**
> +	 * See drm_i915_gem_execbuf_ext_timeline_fences.
> +	 */
> +	DRM_I915_GEM_EXECBUFFER_EXT_TIMELINE_FENCES = 1,
> +
>  	DRM_I915_GEM_EXECBUFFER_EXT_MAX /* non-ABI */
>  };
>  
> +/**
> + * This structure describes an array of drm_syncobj and associated points for
> + * timeline variants of drm_syncobj. It is invalid to append this structure to
> + * the execbuf if I915_EXEC_FENCE_ARRAY is set.
> + */
> +struct drm_i915_gem_execbuffer_ext_timeline_fences {
> +	struct i915_user_extension base;
> +
> +	/**
> +	 * Number of element in the handles_ptr & value_ptr arrays.
> +	 */
> +	__u64 fence_count;
> +
> +	/**
> +	 * Pointer to an array of struct drm_i915_gem_exec_fence of length
> +	 * fence_count.
> +	 */
> +	__u64 handles_ptr;
> +
> +	/**
> +	 * Pointer to an array of u64 values of length fence_count. Values
> +	 * must be 0 for a binary drm_syncobj. A Value of 0 for a timeline
> +	 * drm_syncobj is invalid as it turns a drm_syncobj into a binary one.
> +	 */
> +	__u64 values_ptr;
> +};
> +
>  struct drm_i915_gem_execbuffer2 {
>  	/**
>  	 * List of gem_exec_object2 structs
> -- 
> 2.27.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences
  2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences Lionel Landwerlin
@ 2020-07-29 12:36   ` daniel
  0 siblings, 0 replies; 18+ messages in thread
From: daniel @ 2020-07-29 12:36 UTC (permalink / raw)
  Cc: intel-gfx

On Wed, Jul 08, 2020 at 04:17:51PM +0300, Lionel Landwerlin wrote:
> To allow faster engine to engine synchronization, peel the layer of
> dma-fence-chain to expose potential i915 fences so that the
> i915-request code can emit HW semaphore wait/signal operations in the
> ring which is faster than waking up the host to submit unblocked
> workloads after interrupt notification.
> 
> v2: Also deal with chains where the last node is not a dma-fence-chain
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> ---
>  .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 39 ++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index d8814e637e71..3ffd95d1dc2c 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -2403,6 +2403,7 @@ await_fence_array(struct i915_execbuffer *eb)
>  
>  	for (n = 0; n < eb->n_fences; n++) {
>  		struct drm_syncobj *syncobj;
> +		struct dma_fence_chain *chain;
>  		struct dma_fence *fence;
>  		unsigned int flags;
>  
> @@ -2423,7 +2424,43 @@ await_fence_array(struct i915_execbuffer *eb)
>  				continue;
>  		}
>  
> -		err = i915_request_await_dma_fence(eb->request, fence);
> +		chain = to_dma_fence_chain(fence);
> +		if (chain) {
> +			struct dma_fence *iter;
> +
> +			/*
> +			 * If we're dealing with a dma-fence-chain, peel the
> +			 * chain by adding all of the unsignaled fences
> +			 * (dma_fence_chain_for_each does that for us) the
> +			 * chain points to.
> +			 *
> +			 * This enables us to identify waits on i915 fences
> +			 * and allows for faster engine-to-engine
> +			 * synchronization using HW semaphores.
> +			 */
> +			dma_fence_chain_for_each(iter, fence) {
> +				struct dma_fence_chain *iter_chain =
> +					to_dma_fence_chain(iter);
> +
> +				/*
> +				 * It is possible that the last item in the
> +				 * chain is not a dma_fence_chain.
> +				 */
> +				if (iter_chain) {
> +					err = i915_request_await_dma_fence(eb->request,
> +									   iter_chain->fence);
> +				} else {
> +					err = i915_request_await_dma_fence(eb->request, iter);

I'm kinda wondering whether there should be a limit to how deep we go
before we just give up and wait on the chain, since all we're doing here
(in the worst case at least) is rebuilding the chain.

But hey we can figure this out later on when it actually hurts ...

On the series:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +				}
> +				if (err < 0) {
> +					dma_fence_put(iter);
> +					break;
> +				}
> +			}
> +		} else {
> +			err = i915_request_await_dma_fence(eb->request, fence);
> +		}
> +
>  		dma_fence_put(fence);
>  		if (err < 0)
>  			return err;
> -- 
> 2.27.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
  2020-07-29 12:24     ` Daniel Vetter
@ 2020-07-29 17:51       ` Linus Torvalds
  -1 siblings, 0 replies; 18+ messages in thread
From: Linus Torvalds @ 2020-07-29 17:51 UTC (permalink / raw)
  To: Lionel Landwerlin, Kees Cook, Linus Torvalds,
	Linux Kernel Mailing List, intel-gfx

On Wed, Jul 29, 2020 at 5:24 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> Do we have a access_ok_array or so? Instead of duplicating overflow checks
> everywhere and getting it all wrong ...

I really really think you should get away from access_ok() entirely.

Please just get rid of it, and use "copy_from_user()" instead.

Seriously.

access_ok() is completely wrong, because

 (a) it doesn't actually protect from any fault returns, it only doe
sthe high-level check of "is the pointer even ok".

So you can't say "ok, I did access_ok(), so I don't have to check the
return value", and you're actually making the source code more
complicated, and only introducing the possibility of bugs.

Overflow is just _one_ such bug. Missing the access_ok() entirely
because it was in some caller but not another is another common bug.

 (b) it no longer even makes the code faster.

It never really did for the the copy_to/from_user() case _anyway_, it
was always for the "I will now do several get/put_user() accesses and
I only want to do the range check once".

And that has simply not been true for the last few CPU generations -
because the cost isn't in the range check any more. Now allk the real
costs are about CLAC/STAC. The range check takes two cycles and
schedules well (so it's generally not even visible). The CLAC/STAC
takes 30+ cycles, and stalls the pipeline.

>Similar I guess for copy_from/to_user_array.

No. I refuse to add complexity onto the garbage that is access_ok().

Just remove it. It's not helping. People who think it's helping
haven't actually looked at profiles, or are working with hardware that
is no longer relevant.

                Linus

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
@ 2020-07-29 17:51       ` Linus Torvalds
  0 siblings, 0 replies; 18+ messages in thread
From: Linus Torvalds @ 2020-07-29 17:51 UTC (permalink / raw)
  To: Lionel Landwerlin, Kees Cook, Linus Torvalds,
	Linux Kernel Mailing List, intel-gfx

On Wed, Jul 29, 2020 at 5:24 AM Daniel Vetter <daniel@ffwll.ch> wrote:
>
> Do we have a access_ok_array or so? Instead of duplicating overflow checks
> everywhere and getting it all wrong ...

I really really think you should get away from access_ok() entirely.

Please just get rid of it, and use "copy_from_user()" instead.

Seriously.

access_ok() is completely wrong, because

 (a) it doesn't actually protect from any fault returns, it only doe
sthe high-level check of "is the pointer even ok".

So you can't say "ok, I did access_ok(), so I don't have to check the
return value", and you're actually making the source code more
complicated, and only introducing the possibility of bugs.

Overflow is just _one_ such bug. Missing the access_ok() entirely
because it was in some caller but not another is another common bug.

 (b) it no longer even makes the code faster.

It never really did for the the copy_to/from_user() case _anyway_, it
was always for the "I will now do several get/put_user() accesses and
I only want to do the range check once".

And that has simply not been true for the last few CPU generations -
because the cost isn't in the range check any more. Now allk the real
costs are about CLAC/STAC. The range check takes two cycles and
schedules well (so it's generally not even visible). The CLAC/STAC
takes 30+ cycles, and stalls the pipeline.

>Similar I guess for copy_from/to_user_array.

No. I refuse to add complexity onto the garbage that is access_ok().

Just remove it. It's not helping. People who think it's helping
haven't actually looked at profiles, or are working with hardware that
is no longer relevant.

                Linus
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
  2020-07-29 17:51       ` Linus Torvalds
@ 2020-07-29 19:19         ` Kees Cook
  -1 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2020-07-29 19:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Lionel Landwerlin, Linux Kernel Mailing List, intel-gfx

On Wed, Jul 29, 2020 at 10:51:23AM -0700, Linus Torvalds wrote:
> On Wed, Jul 29, 2020 at 5:24 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > Do we have a access_ok_array or so? Instead of duplicating overflow checks
> > everywhere and getting it all wrong ...
> 
> I really really think you should get away from access_ok() entirely.
> 
> Please just get rid of it, and use "copy_from_user()" instead.

Yes please. :) It also makes code SO much easier to audit (both
automatically and manually).

-- 
Kees Cook

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

* Re: [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support
@ 2020-07-29 19:19         ` Kees Cook
  0 siblings, 0 replies; 18+ messages in thread
From: Kees Cook @ 2020-07-29 19:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: intel-gfx, Linux Kernel Mailing List

On Wed, Jul 29, 2020 at 10:51:23AM -0700, Linus Torvalds wrote:
> On Wed, Jul 29, 2020 at 5:24 AM Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > Do we have a access_ok_array or so? Instead of duplicating overflow checks
> > everywhere and getting it all wrong ...
> 
> I really really think you should get away from access_ok() entirely.
> 
> Please just get rid of it, and use "copy_from_user()" instead.

Yes please. :) It also makes code SO much easier to audit (both
automatically and manually).

-- 
Kees Cook
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2020-07-29 19:19 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-08 13:17 [Intel-gfx] [PATCH v12 0/3] drm/i915: timeline semaphore support Lionel Landwerlin
2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 1/3] drm/i915: introduce a mechanism to extend execbuf2 Lionel Landwerlin
2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 2/3] drm/i915: add syncobj timeline support Lionel Landwerlin
2020-07-29 12:24   ` Daniel Vetter
2020-07-29 12:24     ` Daniel Vetter
2020-07-29 17:51     ` Linus Torvalds
2020-07-29 17:51       ` Linus Torvalds
2020-07-29 19:19       ` Kees Cook
2020-07-29 19:19         ` Kees Cook
2020-07-29 12:34   ` daniel
2020-07-08 13:17 ` [Intel-gfx] [PATCH v12 3/3] drm/i915: peel dma-fence-chains wait fences Lionel Landwerlin
2020-07-29 12:36   ` daniel
2020-07-08 14:28 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: timeline semaphore support Patchwork
2020-07-08 14:29 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-07-08 14:49 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-07-08 17:57 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-07-15 11:11 ` [Intel-gfx] [PATCH v12 0/3] " Lionel Landwerlin
2020-07-22  7:10   ` Lionel Landwerlin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.