All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] RESEND: dma-buf cleanup
@ 2018-07-04  9:29 Daniel Vetter
  2018-07-04  9:29 ` [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops Daniel Vetter
                   ` (7 more replies)
  0 siblings, 8 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development

Hi all,

This is a resend of my dma-buf cleanup with the patches that didn't yet
get and ack/r-b. Feedback very much welcome.

Thanks, Daniel

Daniel Vetter (5):
  drm/i915: Remove unecessary dma_fence_ops
  drm/msm: Remove unecessary dma_fence_ops
  drm/nouveau: Remove unecessary dma_fence_ops
  drm/vgem: Remove unecessary dma_fence_ops
  dma-fence: Polish kernel-doc for dma-fence.c

 Documentation/driver-api/dma-buf.rst          |   6 +
 drivers/dma-buf/dma-fence.c                   | 147 ++++++++++++------
 drivers/gpu/drm/i915/i915_gem_clflush.c       |   7 -
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |   8 -
 drivers/gpu/drm/msm/msm_fence.c               |   8 -
 drivers/gpu/drm/nouveau/nouveau_fence.c       |   1 -
 drivers/gpu/drm/vgem/vgem_fence.c             |  14 --
 7 files changed, 109 insertions(+), 82 deletions(-)

-- 
2.18.0

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

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

* [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
@ 2018-07-04  9:29 ` Daniel Vetter
  2018-07-04 12:03   ` Emil Velikov
       [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Tvrtko Ursulin, Daniel Vetter, Intel Graphics Development,
	Rodrigo Vivi, Colin Ian King, Mika Kuoppala

dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.

v2: Also remove the relase hook, dma_fence_free is the default.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Colin Ian King <colin.king@canonical.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: intel-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/i915/i915_gem_clflush.c        | 7 -------
 drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8 --------
 2 files changed, 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
index f5c570d35b2a..8e74c23cbd91 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
 	return "clflush";
 }
 
-static bool i915_clflush_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static void i915_clflush_release(struct dma_fence *fence)
 {
 	struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
@@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
 static const struct dma_fence_ops i915_clflush_ops = {
 	.get_driver_name = i915_clflush_get_driver_name,
 	.get_timeline_name = i915_clflush_get_timeline_name,
-	.enable_signaling = i915_clflush_enable_signaling,
-	.wait = dma_fence_default_wait,
 	.release = i915_clflush_release,
 };
 
diff --git a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
index 570e325af93e..cdbc8f134e5e 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -611,17 +611,9 @@ static const char *mock_name(struct dma_fence *fence)
 	return "mock";
 }
 
-static bool mock_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static const struct dma_fence_ops mock_fence_ops = {
 	.get_driver_name = mock_name,
 	.get_timeline_name = mock_name,
-	.enable_signaling = mock_enable_signaling,
-	.wait = dma_fence_default_wait,
-	.release = dma_fence_free,
 };
 
 static DEFINE_SPINLOCK(mock_fence_lock);
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* [PATCH 2/5] drm/msm: Remove unecessary dma_fence_ops
       [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
@ 2018-07-04  9:29   ` Daniel Vetter
  2018-07-04  9:29   ` [PATCH 3/5] drm/nouveau: " Daniel Vetter
  1 sibling, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development,
	freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA

dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.

v2: Also remove the relase hook, dma_fence_free is the default.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rob Clark <robdclark@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
---
 drivers/gpu/drm/msm/msm_fence.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 349c12f670eb..77263cf97b20 100644
--- a/drivers/gpu/drm/msm/msm_fence.c
+++ b/drivers/gpu/drm/msm/msm_fence.c
@@ -119,11 +119,6 @@ static const char *msm_fence_get_timeline_name(struct dma_fence *fence)
 	return f->fctx->name;
 }
 
-static bool msm_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static bool msm_fence_signaled(struct dma_fence *fence)
 {
 	struct msm_fence *f = to_msm_fence(fence);
@@ -133,10 +128,7 @@ static bool msm_fence_signaled(struct dma_fence *fence)
 static const struct dma_fence_ops msm_fence_ops = {
 	.get_driver_name = msm_fence_get_driver_name,
 	.get_timeline_name = msm_fence_get_timeline_name,
-	.enable_signaling = msm_fence_enable_signaling,
 	.signaled = msm_fence_signaled,
-	.wait = dma_fence_default_wait,
-	.release = dma_fence_free,
 };
 
 struct dma_fence *
-- 
2.18.0

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

* [PATCH 3/5] drm/nouveau: Remove unecessary dma_fence_ops
       [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
  2018-07-04  9:29   ` [PATCH 2/5] drm/msm: " Daniel Vetter
@ 2018-07-04  9:29   ` Daniel Vetter
  1 sibling, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Ben Skeggs,
	nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

dma_fence_default_wait is the default now.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: nouveau@lists.freedesktop.org
---
 drivers/gpu/drm/nouveau/nouveau_fence.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c
index 412d49bc6e56..99be61ddeb75 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -526,6 +526,5 @@ static const struct dma_fence_ops nouveau_fence_ops_uevent = {
 	.get_timeline_name = nouveau_fence_get_timeline_name,
 	.enable_signaling = nouveau_fence_enable_signaling,
 	.signaled = nouveau_fence_is_signaled,
-	.wait = dma_fence_default_wait,
 	.release = nouveau_fence_release
 };
-- 
2.18.0

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

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

* [PATCH 4/5] drm/vgem: Remove unecessary dma_fence_ops
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
  2018-07-04  9:29 ` [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops Daniel Vetter
       [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
@ 2018-07-04  9:29 ` Daniel Vetter
  2018-08-09  8:33   ` Daniel Vetter
  2018-08-09 12:45   ` [PATCH] " Daniel Vetter
  2018-07-04  9:29   ` Daniel Vetter
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Kees Cook, Cihangir Akturk

dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.

Also remove the ->signaled callback, vgem can't peek ahead with a
fastpath, returning false is the default implementation.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Kees Cook <keescook@chromium.org>
Cc: Cihangir Akturk <cakturk@gmail.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/vgem/vgem_fence.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
index b28876c222b4..75adedeaa384 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -43,16 +43,6 @@ static const char *vgem_fence_get_timeline_name(struct dma_fence *fence)
 	return "unbound";
 }
 
-static bool vgem_fence_signaled(struct dma_fence *fence)
-{
-	return false;
-}
-
-static bool vgem_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static void vgem_fence_release(struct dma_fence *base)
 {
 	struct vgem_fence *fence = container_of(base, typeof(*fence), base);
@@ -76,11 +66,7 @@ static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
 static const struct dma_fence_ops vgem_fence_ops = {
 	.get_driver_name = vgem_fence_get_driver_name,
 	.get_timeline_name = vgem_fence_get_timeline_name,
-	.enable_signaling = vgem_fence_enable_signaling,
-	.signaled = vgem_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = vgem_fence_release,
-
 	.fence_value_str = vgem_fence_value_str,
 	.timeline_value_str = vgem_fence_timeline_value_str,
 };
-- 
2.18.0

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

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

* [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
@ 2018-07-04  9:29   ` Daniel Vetter
       [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Sumit Semwal,
	Gustavo Padovan, linux-media, linaro-mm-sig

- Intro section that links to how this is exposed to userspace.
- Lots more hyperlinks.
- Minor clarifications and style polish

v2: Add misplaced hunk of kerneldoc from a different patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 Documentation/driver-api/dma-buf.rst |   6 ++
 drivers/dma-buf/dma-fence.c          | 147 +++++++++++++++++++--------
 2 files changed, 109 insertions(+), 44 deletions(-)

diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
index dc384f2f7f34..b541e97c7ab1 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -130,6 +130,12 @@ Reservation Objects
 DMA Fences
 ----------
 
+.. kernel-doc:: drivers/dma-buf/dma-fence.c
+   :doc: DMA fences overview
+
+DMA Fences Functions Reference
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 .. kernel-doc:: drivers/dma-buf/dma-fence.c
    :export:
 
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7a92f85a4cec..1551ca7df394 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
  */
 static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
 
+/**
+ * DOC: DMA fences overview
+ *
+ * DMA fences, represented by &struct dma_fence, are the kernel internal
+ * synchronization primitive for DMA operations like GPU rendering, video
+ * encoding/decoding, or displaying buffers on a screen.
+ *
+ * A fence is initialized using dma_fence_init() and completed using
+ * dma_fence_signal(). Fences are associated with a context, allocated through
+ * dma_fence_context_alloc(), and all fences on the same context are
+ * fully ordered.
+ *
+ * Since the purposes of fences is to facilitate cross-device and
+ * cross-application synchronization, there's multiple ways to use one:
+ *
+ * - Individual fences can be exposed as a &sync_file, accessed as a file
+ *   descriptor from userspace, created by calling sync_file_create(). This is
+ *   called explicit fencing, since userspace passes around explicit
+ *   synchronization points.
+ *
+ * - Some subsystems also have their own explicit fencing primitives, like
+ *   &drm_syncobj. Compared to &sync_file, a &drm_syncobj allows the underlying
+ *   fence to be updated.
+ *
+ * - Then there's also implicit fencing, where the synchronization points are
+ *   implicitly passed around as part of shared &dma_buf instances. Such
+ *   implicit fences are stored in &struct reservation_object through the
+ *   &dma_buf.resv pointer.
+ */
+
 /**
  * dma_fence_context_alloc - allocate an array of fence contexts
- * @num:	[in]	amount of contexts to allocate
+ * @num: amount of contexts to allocate
  *
- * This function will return the first index of the number of fences allocated.
- * The fence context is used for setting fence->context to a unique number.
+ * This function will return the first index of the number of fence contexts
+ * allocated.  The fence context is used for setting &dma_fence.context to a
+ * unique number by passing the context to dma_fence_init().
  */
 u64 dma_fence_context_alloc(unsigned num)
 {
@@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
  * Signal completion for software callbacks on a fence, this will unblock
  * dma_fence_wait() calls and run all the callbacks added with
  * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
+ * held.
  *
- * Unlike dma_fence_signal, this function must be called with fence->lock held.
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
  */
 int dma_fence_signal_locked(struct dma_fence *fence)
 {
@@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
  * Signal completion for software callbacks on a fence, this will unblock
  * dma_fence_wait() calls and run all the callbacks added with
  * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
  */
 int dma_fence_signal(struct dma_fence *fence)
 {
@@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
 /**
  * dma_fence_wait_timeout - sleep until the fence gets signaled
  * or until timeout elapses
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
  * remaining timeout in jiffies on success. Other error values may be
@@ -148,6 +186,8 @@ EXPORT_SYMBOL(dma_fence_signal);
  * directly or indirectly (buf-mgr between reservation and committing)
  * holds a reference to the fence, otherwise the fence might be
  * freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait() and dma_fence_wait_any_timeout().
  */
 signed long
 dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
@@ -167,6 +207,13 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
 }
 EXPORT_SYMBOL(dma_fence_wait_timeout);
 
+/**
+ * dma_fence_release - default relese function for fences
+ * @kref: &dma_fence.recfount
+ *
+ * This is the default release functions for &dma_fence. Drivers shouldn't call
+ * this directly, but instead call dma_fence_put().
+ */
 void dma_fence_release(struct kref *kref)
 {
 	struct dma_fence *fence =
@@ -184,6 +231,13 @@ void dma_fence_release(struct kref *kref)
 }
 EXPORT_SYMBOL(dma_fence_release);
 
+/**
+ * dma_fence_free - default release function for &dma_fence.
+ * @fence: fence to release
+ *
+ * This is the default implementation for &dma_fence_ops.release. It calls
+ * kfree_rcu() on @fence.
+ */
 void dma_fence_free(struct dma_fence *fence)
 {
 	kfree_rcu(fence, rcu);
@@ -192,10 +246,11 @@ EXPORT_SYMBOL(dma_fence_free);
 
 /**
  * dma_fence_enable_sw_signaling - enable signaling on fence
- * @fence:	[in]	the fence to enable
+ * @fence: the fence to enable
  *
- * this will request for sw signaling to be enabled, to make the fence
- * complete as soon as possible
+ * This will request for sw signaling to be enabled, to make the fence
+ * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
+ * internally.
  */
 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
 {
@@ -220,24 +275,24 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 /**
  * dma_fence_add_callback - add a callback to be called when the fence
  * is signaled
- * @fence:	[in]	the fence to wait on
- * @cb:		[in]	the callback to register
- * @func:	[in]	the function to call
+ * @fence: the fence to wait on
+ * @cb: the callback to register
+ * @func: the function to call
  *
- * cb will be initialized by dma_fence_add_callback, no initialization
+ * @cb will be initialized by dma_fence_add_callback(), no initialization
  * by the caller is required. Any number of callbacks can be registered
  * to a fence, but a callback can only be registered to one fence at a time.
  *
  * Note that the callback can be called from an atomic context.  If
  * fence is already signaled, this function will return -ENOENT (and
- * *not* call the callback)
+ * *not* call the callback).
  *
  * Add a software callback to the fence. Same restrictions apply to
- * refcount as it does to dma_fence_wait, however the caller doesn't need to
- * keep a refcount to fence afterwards: when software access is enabled,
- * the creator of the fence is required to keep the fence alive until
- * after it signals with dma_fence_signal. The callback itself can be called
- * from irq context.
+ * refcount as it does to dma_fence_wait(), however the caller doesn't need to
+ * keep a refcount to fence afterward dma_fence_add_callback() has returned:
+ * when software access is enabled, the creator of the fence is required to keep
+ * the fence alive until after it signals with dma_fence_signal(). The callback
+ * itself can be called from irq context.
  *
  * Returns 0 in case of success, -ENOENT if the fence is already signaled
  * and -EINVAL in case of error.
@@ -286,7 +341,7 @@ EXPORT_SYMBOL(dma_fence_add_callback);
 
 /**
  * dma_fence_get_status - returns the status upon completion
- * @fence: [in]	the dma_fence to query
+ * @fence: the dma_fence to query
  *
  * This wraps dma_fence_get_status_locked() to return the error status
  * condition on a signaled fence. See dma_fence_get_status_locked() for more
@@ -311,8 +366,8 @@ EXPORT_SYMBOL(dma_fence_get_status);
 
 /**
  * dma_fence_remove_callback - remove a callback from the signaling list
- * @fence:	[in]	the fence to wait on
- * @cb:		[in]	the callback to remove
+ * @fence: the fence to wait on
+ * @cb: the callback to remove
  *
  * Remove a previously queued callback from the fence. This function returns
  * true if the callback is successfully removed, or false if the fence has
@@ -323,6 +378,9 @@ EXPORT_SYMBOL(dma_fence_get_status);
  * doing, since deadlocks and race conditions could occur all too easily. For
  * this reason, it should only ever be done on hardware lockup recovery,
  * with a reference held to the fence.
+ *
+ * Behaviour is undefined if @cb has not been added to @fence using
+ * dma_fence_add_callback() beforehand.
  */
 bool
 dma_fence_remove_callback(struct dma_fence *fence, struct dma_fence_cb *cb)
@@ -359,9 +417,9 @@ dma_fence_default_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
 /**
  * dma_fence_default_wait - default sleep until the fence gets signaled
  * or until timeout elapses
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
  * remaining timeout in jiffies on success. If timeout is zero the value one is
@@ -454,12 +512,12 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
 /**
  * dma_fence_wait_any_timeout - sleep until any fence gets signaled
  * or until timeout elapses
- * @fences:	[in]	array of fences to wait on
- * @count:	[in]	number of fences to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
- * @idx:       [out]	the first signaled fence index, meaningful only on
- *			positive return
+ * @fences: array of fences to wait on
+ * @count: number of fences to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @idx: used to store the first signaled fence index, meaningful only on
+ *	positive return
  *
  * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
  * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
@@ -468,6 +526,8 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
  * Synchronous waits for the first fence in the array to be signaled. The
  * caller needs to hold a reference to all fences in the array, otherwise a
  * fence might be freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait() and dma_fence_wait_timeout().
  */
 signed long
 dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
@@ -540,19 +600,18 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
 
 /**
  * dma_fence_init - Initialize a custom fence.
- * @fence:	[in]	the fence to initialize
- * @ops:	[in]	the dma_fence_ops for operations on this fence
- * @lock:	[in]	the irqsafe spinlock to use for locking this fence
- * @context:	[in]	the execution context this fence is run on
- * @seqno:	[in]	a linear increasing sequence number for this context
+ * @fence: the fence to initialize
+ * @ops: the dma_fence_ops for operations on this fence
+ * @lock: the irqsafe spinlock to use for locking this fence
+ * @context: the execution context this fence is run on
+ * @seqno: a linear increasing sequence number for this context
  *
  * Initializes an allocated fence, the caller doesn't have to keep its
  * refcount after committing with this fence, but it will need to hold a
- * refcount again if dma_fence_ops.enable_signaling gets called. This can
- * be used for other implementing other types of fence.
+ * refcount again if &dma_fence_ops.enable_signaling gets called.
  *
  * context and seqno are used for easy comparison between fences, allowing
- * to check which fence is later by simply using dma_fence_later.
+ * to check which fence is later by simply using dma_fence_later().
  */
 void
 dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
-- 
2.18.0

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

* [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
@ 2018-07-04  9:29   ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04  9:29 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, linaro-mm-sig, linux-media

- Intro section that links to how this is exposed to userspace.
- Lots more hyperlinks.
- Minor clarifications and style polish

v2: Add misplaced hunk of kerneldoc from a different patch.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 Documentation/driver-api/dma-buf.rst |   6 ++
 drivers/dma-buf/dma-fence.c          | 147 +++++++++++++++++++--------
 2 files changed, 109 insertions(+), 44 deletions(-)

diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
index dc384f2f7f34..b541e97c7ab1 100644
--- a/Documentation/driver-api/dma-buf.rst
+++ b/Documentation/driver-api/dma-buf.rst
@@ -130,6 +130,12 @@ Reservation Objects
 DMA Fences
 ----------
 
+.. kernel-doc:: drivers/dma-buf/dma-fence.c
+   :doc: DMA fences overview
+
+DMA Fences Functions Reference
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
 .. kernel-doc:: drivers/dma-buf/dma-fence.c
    :export:
 
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7a92f85a4cec..1551ca7df394 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
  */
 static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
 
+/**
+ * DOC: DMA fences overview
+ *
+ * DMA fences, represented by &struct dma_fence, are the kernel internal
+ * synchronization primitive for DMA operations like GPU rendering, video
+ * encoding/decoding, or displaying buffers on a screen.
+ *
+ * A fence is initialized using dma_fence_init() and completed using
+ * dma_fence_signal(). Fences are associated with a context, allocated through
+ * dma_fence_context_alloc(), and all fences on the same context are
+ * fully ordered.
+ *
+ * Since the purposes of fences is to facilitate cross-device and
+ * cross-application synchronization, there's multiple ways to use one:
+ *
+ * - Individual fences can be exposed as a &sync_file, accessed as a file
+ *   descriptor from userspace, created by calling sync_file_create(). This is
+ *   called explicit fencing, since userspace passes around explicit
+ *   synchronization points.
+ *
+ * - Some subsystems also have their own explicit fencing primitives, like
+ *   &drm_syncobj. Compared to &sync_file, a &drm_syncobj allows the underlying
+ *   fence to be updated.
+ *
+ * - Then there's also implicit fencing, where the synchronization points are
+ *   implicitly passed around as part of shared &dma_buf instances. Such
+ *   implicit fences are stored in &struct reservation_object through the
+ *   &dma_buf.resv pointer.
+ */
+
 /**
  * dma_fence_context_alloc - allocate an array of fence contexts
- * @num:	[in]	amount of contexts to allocate
+ * @num: amount of contexts to allocate
  *
- * This function will return the first index of the number of fences allocated.
- * The fence context is used for setting fence->context to a unique number.
+ * This function will return the first index of the number of fence contexts
+ * allocated.  The fence context is used for setting &dma_fence.context to a
+ * unique number by passing the context to dma_fence_init().
  */
 u64 dma_fence_context_alloc(unsigned num)
 {
@@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
  * Signal completion for software callbacks on a fence, this will unblock
  * dma_fence_wait() calls and run all the callbacks added with
  * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
+ * held.
  *
- * Unlike dma_fence_signal, this function must be called with fence->lock held.
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
  */
 int dma_fence_signal_locked(struct dma_fence *fence)
 {
@@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
  * Signal completion for software callbacks on a fence, this will unblock
  * dma_fence_wait() calls and run all the callbacks added with
  * dma_fence_add_callback(). Can be called multiple times, but since a fence
- * can only go from unsignaled to signaled state, it will only be effective
- * the first time.
+ * can only go from the unsignaled to the signaled state and not back, it will
+ * only be effective the first time.
+ *
+ * Returns 0 on success and a negative error value when @fence has been
+ * signalled already.
  */
 int dma_fence_signal(struct dma_fence *fence)
 {
@@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
 /**
  * dma_fence_wait_timeout - sleep until the fence gets signaled
  * or until timeout elapses
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
  * remaining timeout in jiffies on success. Other error values may be
@@ -148,6 +186,8 @@ EXPORT_SYMBOL(dma_fence_signal);
  * directly or indirectly (buf-mgr between reservation and committing)
  * holds a reference to the fence, otherwise the fence might be
  * freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait() and dma_fence_wait_any_timeout().
  */
 signed long
 dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
@@ -167,6 +207,13 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
 }
 EXPORT_SYMBOL(dma_fence_wait_timeout);
 
+/**
+ * dma_fence_release - default relese function for fences
+ * @kref: &dma_fence.recfount
+ *
+ * This is the default release functions for &dma_fence. Drivers shouldn't call
+ * this directly, but instead call dma_fence_put().
+ */
 void dma_fence_release(struct kref *kref)
 {
 	struct dma_fence *fence =
@@ -184,6 +231,13 @@ void dma_fence_release(struct kref *kref)
 }
 EXPORT_SYMBOL(dma_fence_release);
 
+/**
+ * dma_fence_free - default release function for &dma_fence.
+ * @fence: fence to release
+ *
+ * This is the default implementation for &dma_fence_ops.release. It calls
+ * kfree_rcu() on @fence.
+ */
 void dma_fence_free(struct dma_fence *fence)
 {
 	kfree_rcu(fence, rcu);
@@ -192,10 +246,11 @@ EXPORT_SYMBOL(dma_fence_free);
 
 /**
  * dma_fence_enable_sw_signaling - enable signaling on fence
- * @fence:	[in]	the fence to enable
+ * @fence: the fence to enable
  *
- * this will request for sw signaling to be enabled, to make the fence
- * complete as soon as possible
+ * This will request for sw signaling to be enabled, to make the fence
+ * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
+ * internally.
  */
 void dma_fence_enable_sw_signaling(struct dma_fence *fence)
 {
@@ -220,24 +275,24 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 /**
  * dma_fence_add_callback - add a callback to be called when the fence
  * is signaled
- * @fence:	[in]	the fence to wait on
- * @cb:		[in]	the callback to register
- * @func:	[in]	the function to call
+ * @fence: the fence to wait on
+ * @cb: the callback to register
+ * @func: the function to call
  *
- * cb will be initialized by dma_fence_add_callback, no initialization
+ * @cb will be initialized by dma_fence_add_callback(), no initialization
  * by the caller is required. Any number of callbacks can be registered
  * to a fence, but a callback can only be registered to one fence at a time.
  *
  * Note that the callback can be called from an atomic context.  If
  * fence is already signaled, this function will return -ENOENT (and
- * *not* call the callback)
+ * *not* call the callback).
  *
  * Add a software callback to the fence. Same restrictions apply to
- * refcount as it does to dma_fence_wait, however the caller doesn't need to
- * keep a refcount to fence afterwards: when software access is enabled,
- * the creator of the fence is required to keep the fence alive until
- * after it signals with dma_fence_signal. The callback itself can be called
- * from irq context.
+ * refcount as it does to dma_fence_wait(), however the caller doesn't need to
+ * keep a refcount to fence afterward dma_fence_add_callback() has returned:
+ * when software access is enabled, the creator of the fence is required to keep
+ * the fence alive until after it signals with dma_fence_signal(). The callback
+ * itself can be called from irq context.
  *
  * Returns 0 in case of success, -ENOENT if the fence is already signaled
  * and -EINVAL in case of error.
@@ -286,7 +341,7 @@ EXPORT_SYMBOL(dma_fence_add_callback);
 
 /**
  * dma_fence_get_status - returns the status upon completion
- * @fence: [in]	the dma_fence to query
+ * @fence: the dma_fence to query
  *
  * This wraps dma_fence_get_status_locked() to return the error status
  * condition on a signaled fence. See dma_fence_get_status_locked() for more
@@ -311,8 +366,8 @@ EXPORT_SYMBOL(dma_fence_get_status);
 
 /**
  * dma_fence_remove_callback - remove a callback from the signaling list
- * @fence:	[in]	the fence to wait on
- * @cb:		[in]	the callback to remove
+ * @fence: the fence to wait on
+ * @cb: the callback to remove
  *
  * Remove a previously queued callback from the fence. This function returns
  * true if the callback is successfully removed, or false if the fence has
@@ -323,6 +378,9 @@ EXPORT_SYMBOL(dma_fence_get_status);
  * doing, since deadlocks and race conditions could occur all too easily. For
  * this reason, it should only ever be done on hardware lockup recovery,
  * with a reference held to the fence.
+ *
+ * Behaviour is undefined if @cb has not been added to @fence using
+ * dma_fence_add_callback() beforehand.
  */
 bool
 dma_fence_remove_callback(struct dma_fence *fence, struct dma_fence_cb *cb)
@@ -359,9 +417,9 @@ dma_fence_default_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
 /**
  * dma_fence_default_wait - default sleep until the fence gets signaled
  * or until timeout elapses
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
  *
  * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
  * remaining timeout in jiffies on success. If timeout is zero the value one is
@@ -454,12 +512,12 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
 /**
  * dma_fence_wait_any_timeout - sleep until any fence gets signaled
  * or until timeout elapses
- * @fences:	[in]	array of fences to wait on
- * @count:	[in]	number of fences to wait on
- * @intr:	[in]	if true, do an interruptible wait
- * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
- * @idx:       [out]	the first signaled fence index, meaningful only on
- *			positive return
+ * @fences: array of fences to wait on
+ * @count: number of fences to wait on
+ * @intr: if true, do an interruptible wait
+ * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
+ * @idx: used to store the first signaled fence index, meaningful only on
+ *	positive return
  *
  * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
  * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
@@ -468,6 +526,8 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
  * Synchronous waits for the first fence in the array to be signaled. The
  * caller needs to hold a reference to all fences in the array, otherwise a
  * fence might be freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait() and dma_fence_wait_timeout().
  */
 signed long
 dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
@@ -540,19 +600,18 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
 
 /**
  * dma_fence_init - Initialize a custom fence.
- * @fence:	[in]	the fence to initialize
- * @ops:	[in]	the dma_fence_ops for operations on this fence
- * @lock:	[in]	the irqsafe spinlock to use for locking this fence
- * @context:	[in]	the execution context this fence is run on
- * @seqno:	[in]	a linear increasing sequence number for this context
+ * @fence: the fence to initialize
+ * @ops: the dma_fence_ops for operations on this fence
+ * @lock: the irqsafe spinlock to use for locking this fence
+ * @context: the execution context this fence is run on
+ * @seqno: a linear increasing sequence number for this context
  *
  * Initializes an allocated fence, the caller doesn't have to keep its
  * refcount after committing with this fence, but it will need to hold a
- * refcount again if dma_fence_ops.enable_signaling gets called. This can
- * be used for other implementing other types of fence.
+ * refcount again if &dma_fence_ops.enable_signaling gets called.
  *
  * context and seqno are used for easy comparison between fences, allowing
- * to check which fence is later by simply using dma_fence_later.
+ * to check which fence is later by simply using dma_fence_later().
  */
 void
 dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
-- 
2.18.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
  2018-07-04  9:29   ` Daniel Vetter
@ 2018-07-04  9:36     ` Christian König
  -1 siblings, 0 replies; 22+ messages in thread
From: Christian König @ 2018-07-04  9:36 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Intel Graphics Development, linaro-mm-sig, linux-media

Am 04.07.2018 um 11:29 schrieb Daniel Vetter:
> - Intro section that links to how this is exposed to userspace.
> - Lots more hyperlinks.
> - Minor clarifications and style polish
>
> v2: Add misplaced hunk of kerneldoc from a different patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   Documentation/driver-api/dma-buf.rst |   6 ++
>   drivers/dma-buf/dma-fence.c          | 147 +++++++++++++++++++--------
>   2 files changed, 109 insertions(+), 44 deletions(-)
>
> diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
> index dc384f2f7f34..b541e97c7ab1 100644
> --- a/Documentation/driver-api/dma-buf.rst
> +++ b/Documentation/driver-api/dma-buf.rst
> @@ -130,6 +130,12 @@ Reservation Objects
>   DMA Fences
>   ----------
>   
> +.. kernel-doc:: drivers/dma-buf/dma-fence.c
> +   :doc: DMA fences overview
> +
> +DMA Fences Functions Reference
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
>   .. kernel-doc:: drivers/dma-buf/dma-fence.c
>      :export:
>   
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 7a92f85a4cec..1551ca7df394 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
>    */
>   static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>   
> +/**
> + * DOC: DMA fences overview
> + *
> + * DMA fences, represented by &struct dma_fence, are the kernel internal
> + * synchronization primitive for DMA operations like GPU rendering, video
> + * encoding/decoding, or displaying buffers on a screen.
> + *
> + * A fence is initialized using dma_fence_init() and completed using
> + * dma_fence_signal(). Fences are associated with a context, allocated through
> + * dma_fence_context_alloc(), and all fences on the same context are
> + * fully ordered.
> + *
> + * Since the purposes of fences is to facilitate cross-device and
> + * cross-application synchronization, there's multiple ways to use one:
> + *
> + * - Individual fences can be exposed as a &sync_file, accessed as a file
> + *   descriptor from userspace, created by calling sync_file_create(). This is
> + *   called explicit fencing, since userspace passes around explicit
> + *   synchronization points.
> + *
> + * - Some subsystems also have their own explicit fencing primitives, like
> + *   &drm_syncobj. Compared to &sync_file, a &drm_syncobj allows the underlying
> + *   fence to be updated.
> + *
> + * - Then there's also implicit fencing, where the synchronization points are
> + *   implicitly passed around as part of shared &dma_buf instances. Such
> + *   implicit fences are stored in &struct reservation_object through the
> + *   &dma_buf.resv pointer.
> + */
> +
>   /**
>    * dma_fence_context_alloc - allocate an array of fence contexts
> - * @num:	[in]	amount of contexts to allocate
> + * @num: amount of contexts to allocate
>    *
> - * This function will return the first index of the number of fences allocated.
> - * The fence context is used for setting fence->context to a unique number.
> + * This function will return the first index of the number of fence contexts
> + * allocated.  The fence context is used for setting &dma_fence.context to a
> + * unique number by passing the context to dma_fence_init().
>    */
>   u64 dma_fence_context_alloc(unsigned num)
>   {
> @@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
>    * Signal completion for software callbacks on a fence, this will unblock
>    * dma_fence_wait() calls and run all the callbacks added with
>    * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from unsignaled to signaled state, it will only be effective
> - * the first time.
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time.
> + *
> + * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
> + * held.
>    *
> - * Unlike dma_fence_signal, this function must be called with fence->lock held.
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
>    */
>   int dma_fence_signal_locked(struct dma_fence *fence)
>   {
> @@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
>    * Signal completion for software callbacks on a fence, this will unblock
>    * dma_fence_wait() calls and run all the callbacks added with
>    * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from unsignaled to signaled state, it will only be effective
> - * the first time.
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time.
> + *
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
>    */
>   int dma_fence_signal(struct dma_fence *fence)
>   {
> @@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
>   /**
>    * dma_fence_wait_timeout - sleep until the fence gets signaled
>    * or until timeout elapses
> - * @fence:	[in]	the fence to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @fence: the fence to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>    *
>    * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
>    * remaining timeout in jiffies on success. Other error values may be
> @@ -148,6 +186,8 @@ EXPORT_SYMBOL(dma_fence_signal);
>    * directly or indirectly (buf-mgr between reservation and committing)
>    * holds a reference to the fence, otherwise the fence might be
>    * freed before return, resulting in undefined behavior.
> + *
> + * See also dma_fence_wait() and dma_fence_wait_any_timeout().
>    */
>   signed long
>   dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
> @@ -167,6 +207,13 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
>   }
>   EXPORT_SYMBOL(dma_fence_wait_timeout);
>   
> +/**
> + * dma_fence_release - default relese function for fences
> + * @kref: &dma_fence.recfount
> + *
> + * This is the default release functions for &dma_fence. Drivers shouldn't call
> + * this directly, but instead call dma_fence_put().
> + */
>   void dma_fence_release(struct kref *kref)
>   {
>   	struct dma_fence *fence =
> @@ -184,6 +231,13 @@ void dma_fence_release(struct kref *kref)
>   }
>   EXPORT_SYMBOL(dma_fence_release);
>   
> +/**
> + * dma_fence_free - default release function for &dma_fence.
> + * @fence: fence to release
> + *
> + * This is the default implementation for &dma_fence_ops.release. It calls
> + * kfree_rcu() on @fence.
> + */
>   void dma_fence_free(struct dma_fence *fence)
>   {
>   	kfree_rcu(fence, rcu);
> @@ -192,10 +246,11 @@ EXPORT_SYMBOL(dma_fence_free);
>   
>   /**
>    * dma_fence_enable_sw_signaling - enable signaling on fence
> - * @fence:	[in]	the fence to enable
> + * @fence: the fence to enable
>    *
> - * this will request for sw signaling to be enabled, to make the fence
> - * complete as soon as possible
> + * This will request for sw signaling to be enabled, to make the fence
> + * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
> + * internally.
>    */
>   void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>   {
> @@ -220,24 +275,24 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   /**
>    * dma_fence_add_callback - add a callback to be called when the fence
>    * is signaled
> - * @fence:	[in]	the fence to wait on
> - * @cb:		[in]	the callback to register
> - * @func:	[in]	the function to call
> + * @fence: the fence to wait on
> + * @cb: the callback to register
> + * @func: the function to call
>    *
> - * cb will be initialized by dma_fence_add_callback, no initialization
> + * @cb will be initialized by dma_fence_add_callback(), no initialization
>    * by the caller is required. Any number of callbacks can be registered
>    * to a fence, but a callback can only be registered to one fence at a time.
>    *
>    * Note that the callback can be called from an atomic context.  If
>    * fence is already signaled, this function will return -ENOENT (and
> - * *not* call the callback)
> + * *not* call the callback).
>    *
>    * Add a software callback to the fence. Same restrictions apply to
> - * refcount as it does to dma_fence_wait, however the caller doesn't need to
> - * keep a refcount to fence afterwards: when software access is enabled,
> - * the creator of the fence is required to keep the fence alive until
> - * after it signals with dma_fence_signal. The callback itself can be called
> - * from irq context.
> + * refcount as it does to dma_fence_wait(), however the caller doesn't need to
> + * keep a refcount to fence afterward dma_fence_add_callback() has returned:
> + * when software access is enabled, the creator of the fence is required to keep
> + * the fence alive until after it signals with dma_fence_signal(). The callback
> + * itself can be called from irq context.
>    *
>    * Returns 0 in case of success, -ENOENT if the fence is already signaled
>    * and -EINVAL in case of error.
> @@ -286,7 +341,7 @@ EXPORT_SYMBOL(dma_fence_add_callback);
>   
>   /**
>    * dma_fence_get_status - returns the status upon completion
> - * @fence: [in]	the dma_fence to query
> + * @fence: the dma_fence to query
>    *
>    * This wraps dma_fence_get_status_locked() to return the error status
>    * condition on a signaled fence. See dma_fence_get_status_locked() for more
> @@ -311,8 +366,8 @@ EXPORT_SYMBOL(dma_fence_get_status);
>   
>   /**
>    * dma_fence_remove_callback - remove a callback from the signaling list
> - * @fence:	[in]	the fence to wait on
> - * @cb:		[in]	the callback to remove
> + * @fence: the fence to wait on
> + * @cb: the callback to remove
>    *
>    * Remove a previously queued callback from the fence. This function returns
>    * true if the callback is successfully removed, or false if the fence has
> @@ -323,6 +378,9 @@ EXPORT_SYMBOL(dma_fence_get_status);
>    * doing, since deadlocks and race conditions could occur all too easily. For
>    * this reason, it should only ever be done on hardware lockup recovery,
>    * with a reference held to the fence.
> + *
> + * Behaviour is undefined if @cb has not been added to @fence using
> + * dma_fence_add_callback() beforehand.
>    */
>   bool
>   dma_fence_remove_callback(struct dma_fence *fence, struct dma_fence_cb *cb)
> @@ -359,9 +417,9 @@ dma_fence_default_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
>   /**
>    * dma_fence_default_wait - default sleep until the fence gets signaled
>    * or until timeout elapses
> - * @fence:	[in]	the fence to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @fence: the fence to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>    *
>    * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
>    * remaining timeout in jiffies on success. If timeout is zero the value one is
> @@ -454,12 +512,12 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>   /**
>    * dma_fence_wait_any_timeout - sleep until any fence gets signaled
>    * or until timeout elapses
> - * @fences:	[in]	array of fences to wait on
> - * @count:	[in]	number of fences to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> - * @idx:       [out]	the first signaled fence index, meaningful only on
> - *			positive return
> + * @fences: array of fences to wait on
> + * @count: number of fences to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @idx: used to store the first signaled fence index, meaningful only on
> + *	positive return
>    *
>    * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
>    * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
> @@ -468,6 +526,8 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>    * Synchronous waits for the first fence in the array to be signaled. The
>    * caller needs to hold a reference to all fences in the array, otherwise a
>    * fence might be freed before return, resulting in undefined behavior.
> + *
> + * See also dma_fence_wait() and dma_fence_wait_timeout().
>    */
>   signed long
>   dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
> @@ -540,19 +600,18 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
>   
>   /**
>    * dma_fence_init - Initialize a custom fence.
> - * @fence:	[in]	the fence to initialize
> - * @ops:	[in]	the dma_fence_ops for operations on this fence
> - * @lock:	[in]	the irqsafe spinlock to use for locking this fence
> - * @context:	[in]	the execution context this fence is run on
> - * @seqno:	[in]	a linear increasing sequence number for this context
> + * @fence: the fence to initialize
> + * @ops: the dma_fence_ops for operations on this fence
> + * @lock: the irqsafe spinlock to use for locking this fence
> + * @context: the execution context this fence is run on
> + * @seqno: a linear increasing sequence number for this context
>    *
>    * Initializes an allocated fence, the caller doesn't have to keep its
>    * refcount after committing with this fence, but it will need to hold a
> - * refcount again if dma_fence_ops.enable_signaling gets called. This can
> - * be used for other implementing other types of fence.
> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>    *
>    * context and seqno are used for easy comparison between fences, allowing
> - * to check which fence is later by simply using dma_fence_later.
> + * to check which fence is later by simply using dma_fence_later().
>    */
>   void
>   dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,

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

* Re: [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c
@ 2018-07-04  9:36     ` Christian König
  0 siblings, 0 replies; 22+ messages in thread
From: Christian König @ 2018-07-04  9:36 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: linaro-mm-sig, Intel Graphics Development, linux-media

Am 04.07.2018 um 11:29 schrieb Daniel Vetter:
> - Intro section that links to how this is exposed to userspace.
> - Lots more hyperlinks.
> - Minor clarifications and style polish
>
> v2: Add misplaced hunk of kerneldoc from a different patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Sumit Semwal <sumit.semwal@linaro.org>
> Cc: Gustavo Padovan <gustavo@padovan.org>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   Documentation/driver-api/dma-buf.rst |   6 ++
>   drivers/dma-buf/dma-fence.c          | 147 +++++++++++++++++++--------
>   2 files changed, 109 insertions(+), 44 deletions(-)
>
> diff --git a/Documentation/driver-api/dma-buf.rst b/Documentation/driver-api/dma-buf.rst
> index dc384f2f7f34..b541e97c7ab1 100644
> --- a/Documentation/driver-api/dma-buf.rst
> +++ b/Documentation/driver-api/dma-buf.rst
> @@ -130,6 +130,12 @@ Reservation Objects
>   DMA Fences
>   ----------
>   
> +.. kernel-doc:: drivers/dma-buf/dma-fence.c
> +   :doc: DMA fences overview
> +
> +DMA Fences Functions Reference
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
>   .. kernel-doc:: drivers/dma-buf/dma-fence.c
>      :export:
>   
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 7a92f85a4cec..1551ca7df394 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -38,12 +38,43 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_enable_signal);
>    */
>   static atomic64_t dma_fence_context_counter = ATOMIC64_INIT(0);
>   
> +/**
> + * DOC: DMA fences overview
> + *
> + * DMA fences, represented by &struct dma_fence, are the kernel internal
> + * synchronization primitive for DMA operations like GPU rendering, video
> + * encoding/decoding, or displaying buffers on a screen.
> + *
> + * A fence is initialized using dma_fence_init() and completed using
> + * dma_fence_signal(). Fences are associated with a context, allocated through
> + * dma_fence_context_alloc(), and all fences on the same context are
> + * fully ordered.
> + *
> + * Since the purposes of fences is to facilitate cross-device and
> + * cross-application synchronization, there's multiple ways to use one:
> + *
> + * - Individual fences can be exposed as a &sync_file, accessed as a file
> + *   descriptor from userspace, created by calling sync_file_create(). This is
> + *   called explicit fencing, since userspace passes around explicit
> + *   synchronization points.
> + *
> + * - Some subsystems also have their own explicit fencing primitives, like
> + *   &drm_syncobj. Compared to &sync_file, a &drm_syncobj allows the underlying
> + *   fence to be updated.
> + *
> + * - Then there's also implicit fencing, where the synchronization points are
> + *   implicitly passed around as part of shared &dma_buf instances. Such
> + *   implicit fences are stored in &struct reservation_object through the
> + *   &dma_buf.resv pointer.
> + */
> +
>   /**
>    * dma_fence_context_alloc - allocate an array of fence contexts
> - * @num:	[in]	amount of contexts to allocate
> + * @num: amount of contexts to allocate
>    *
> - * This function will return the first index of the number of fences allocated.
> - * The fence context is used for setting fence->context to a unique number.
> + * This function will return the first index of the number of fence contexts
> + * allocated.  The fence context is used for setting &dma_fence.context to a
> + * unique number by passing the context to dma_fence_init().
>    */
>   u64 dma_fence_context_alloc(unsigned num)
>   {
> @@ -59,10 +90,14 @@ EXPORT_SYMBOL(dma_fence_context_alloc);
>    * Signal completion for software callbacks on a fence, this will unblock
>    * dma_fence_wait() calls and run all the callbacks added with
>    * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from unsignaled to signaled state, it will only be effective
> - * the first time.
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time.
> + *
> + * Unlike dma_fence_signal(), this function must be called with &dma_fence.lock
> + * held.
>    *
> - * Unlike dma_fence_signal, this function must be called with fence->lock held.
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
>    */
>   int dma_fence_signal_locked(struct dma_fence *fence)
>   {
> @@ -102,8 +137,11 @@ EXPORT_SYMBOL(dma_fence_signal_locked);
>    * Signal completion for software callbacks on a fence, this will unblock
>    * dma_fence_wait() calls and run all the callbacks added with
>    * dma_fence_add_callback(). Can be called multiple times, but since a fence
> - * can only go from unsignaled to signaled state, it will only be effective
> - * the first time.
> + * can only go from the unsignaled to the signaled state and not back, it will
> + * only be effective the first time.
> + *
> + * Returns 0 on success and a negative error value when @fence has been
> + * signalled already.
>    */
>   int dma_fence_signal(struct dma_fence *fence)
>   {
> @@ -136,9 +174,9 @@ EXPORT_SYMBOL(dma_fence_signal);
>   /**
>    * dma_fence_wait_timeout - sleep until the fence gets signaled
>    * or until timeout elapses
> - * @fence:	[in]	the fence to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @fence: the fence to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>    *
>    * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
>    * remaining timeout in jiffies on success. Other error values may be
> @@ -148,6 +186,8 @@ EXPORT_SYMBOL(dma_fence_signal);
>    * directly or indirectly (buf-mgr between reservation and committing)
>    * holds a reference to the fence, otherwise the fence might be
>    * freed before return, resulting in undefined behavior.
> + *
> + * See also dma_fence_wait() and dma_fence_wait_any_timeout().
>    */
>   signed long
>   dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
> @@ -167,6 +207,13 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
>   }
>   EXPORT_SYMBOL(dma_fence_wait_timeout);
>   
> +/**
> + * dma_fence_release - default relese function for fences
> + * @kref: &dma_fence.recfount
> + *
> + * This is the default release functions for &dma_fence. Drivers shouldn't call
> + * this directly, but instead call dma_fence_put().
> + */
>   void dma_fence_release(struct kref *kref)
>   {
>   	struct dma_fence *fence =
> @@ -184,6 +231,13 @@ void dma_fence_release(struct kref *kref)
>   }
>   EXPORT_SYMBOL(dma_fence_release);
>   
> +/**
> + * dma_fence_free - default release function for &dma_fence.
> + * @fence: fence to release
> + *
> + * This is the default implementation for &dma_fence_ops.release. It calls
> + * kfree_rcu() on @fence.
> + */
>   void dma_fence_free(struct dma_fence *fence)
>   {
>   	kfree_rcu(fence, rcu);
> @@ -192,10 +246,11 @@ EXPORT_SYMBOL(dma_fence_free);
>   
>   /**
>    * dma_fence_enable_sw_signaling - enable signaling on fence
> - * @fence:	[in]	the fence to enable
> + * @fence: the fence to enable
>    *
> - * this will request for sw signaling to be enabled, to make the fence
> - * complete as soon as possible
> + * This will request for sw signaling to be enabled, to make the fence
> + * complete as soon as possible. This calls &dma_fence_ops.enable_signaling
> + * internally.
>    */
>   void dma_fence_enable_sw_signaling(struct dma_fence *fence)
>   {
> @@ -220,24 +275,24 @@ EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
>   /**
>    * dma_fence_add_callback - add a callback to be called when the fence
>    * is signaled
> - * @fence:	[in]	the fence to wait on
> - * @cb:		[in]	the callback to register
> - * @func:	[in]	the function to call
> + * @fence: the fence to wait on
> + * @cb: the callback to register
> + * @func: the function to call
>    *
> - * cb will be initialized by dma_fence_add_callback, no initialization
> + * @cb will be initialized by dma_fence_add_callback(), no initialization
>    * by the caller is required. Any number of callbacks can be registered
>    * to a fence, but a callback can only be registered to one fence at a time.
>    *
>    * Note that the callback can be called from an atomic context.  If
>    * fence is already signaled, this function will return -ENOENT (and
> - * *not* call the callback)
> + * *not* call the callback).
>    *
>    * Add a software callback to the fence. Same restrictions apply to
> - * refcount as it does to dma_fence_wait, however the caller doesn't need to
> - * keep a refcount to fence afterwards: when software access is enabled,
> - * the creator of the fence is required to keep the fence alive until
> - * after it signals with dma_fence_signal. The callback itself can be called
> - * from irq context.
> + * refcount as it does to dma_fence_wait(), however the caller doesn't need to
> + * keep a refcount to fence afterward dma_fence_add_callback() has returned:
> + * when software access is enabled, the creator of the fence is required to keep
> + * the fence alive until after it signals with dma_fence_signal(). The callback
> + * itself can be called from irq context.
>    *
>    * Returns 0 in case of success, -ENOENT if the fence is already signaled
>    * and -EINVAL in case of error.
> @@ -286,7 +341,7 @@ EXPORT_SYMBOL(dma_fence_add_callback);
>   
>   /**
>    * dma_fence_get_status - returns the status upon completion
> - * @fence: [in]	the dma_fence to query
> + * @fence: the dma_fence to query
>    *
>    * This wraps dma_fence_get_status_locked() to return the error status
>    * condition on a signaled fence. See dma_fence_get_status_locked() for more
> @@ -311,8 +366,8 @@ EXPORT_SYMBOL(dma_fence_get_status);
>   
>   /**
>    * dma_fence_remove_callback - remove a callback from the signaling list
> - * @fence:	[in]	the fence to wait on
> - * @cb:		[in]	the callback to remove
> + * @fence: the fence to wait on
> + * @cb: the callback to remove
>    *
>    * Remove a previously queued callback from the fence. This function returns
>    * true if the callback is successfully removed, or false if the fence has
> @@ -323,6 +378,9 @@ EXPORT_SYMBOL(dma_fence_get_status);
>    * doing, since deadlocks and race conditions could occur all too easily. For
>    * this reason, it should only ever be done on hardware lockup recovery,
>    * with a reference held to the fence.
> + *
> + * Behaviour is undefined if @cb has not been added to @fence using
> + * dma_fence_add_callback() beforehand.
>    */
>   bool
>   dma_fence_remove_callback(struct dma_fence *fence, struct dma_fence_cb *cb)
> @@ -359,9 +417,9 @@ dma_fence_default_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
>   /**
>    * dma_fence_default_wait - default sleep until the fence gets signaled
>    * or until timeout elapses
> - * @fence:	[in]	the fence to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @fence: the fence to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
>    *
>    * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or the
>    * remaining timeout in jiffies on success. If timeout is zero the value one is
> @@ -454,12 +512,12 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>   /**
>    * dma_fence_wait_any_timeout - sleep until any fence gets signaled
>    * or until timeout elapses
> - * @fences:	[in]	array of fences to wait on
> - * @count:	[in]	number of fences to wait on
> - * @intr:	[in]	if true, do an interruptible wait
> - * @timeout:	[in]	timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> - * @idx:       [out]	the first signaled fence index, meaningful only on
> - *			positive return
> + * @fences: array of fences to wait on
> + * @count: number of fences to wait on
> + * @intr: if true, do an interruptible wait
> + * @timeout: timeout value in jiffies, or MAX_SCHEDULE_TIMEOUT
> + * @idx: used to store the first signaled fence index, meaningful only on
> + *	positive return
>    *
>    * Returns -EINVAL on custom fence wait implementation, -ERESTARTSYS if
>    * interrupted, 0 if the wait timed out, or the remaining timeout in jiffies
> @@ -468,6 +526,8 @@ dma_fence_test_signaled_any(struct dma_fence **fences, uint32_t count,
>    * Synchronous waits for the first fence in the array to be signaled. The
>    * caller needs to hold a reference to all fences in the array, otherwise a
>    * fence might be freed before return, resulting in undefined behavior.
> + *
> + * See also dma_fence_wait() and dma_fence_wait_timeout().
>    */
>   signed long
>   dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
> @@ -540,19 +600,18 @@ EXPORT_SYMBOL(dma_fence_wait_any_timeout);
>   
>   /**
>    * dma_fence_init - Initialize a custom fence.
> - * @fence:	[in]	the fence to initialize
> - * @ops:	[in]	the dma_fence_ops for operations on this fence
> - * @lock:	[in]	the irqsafe spinlock to use for locking this fence
> - * @context:	[in]	the execution context this fence is run on
> - * @seqno:	[in]	a linear increasing sequence number for this context
> + * @fence: the fence to initialize
> + * @ops: the dma_fence_ops for operations on this fence
> + * @lock: the irqsafe spinlock to use for locking this fence
> + * @context: the execution context this fence is run on
> + * @seqno: a linear increasing sequence number for this context
>    *
>    * Initializes an allocated fence, the caller doesn't have to keep its
>    * refcount after committing with this fence, but it will need to hold a
> - * refcount again if dma_fence_ops.enable_signaling gets called. This can
> - * be used for other implementing other types of fence.
> + * refcount again if &dma_fence_ops.enable_signaling gets called.
>    *
>    * context and seqno are used for easy comparison between fences, allowing
> - * to check which fence is later by simply using dma_fence_later.
> + * to check which fence is later by simply using dma_fence_later().
>    */
>   void
>   dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
                   ` (3 preceding siblings ...)
  2018-07-04  9:29   ` Daniel Vetter
@ 2018-07-04  9:53 ` Patchwork
  2018-07-04 10:08 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-07-04  9:53 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: RESEND: dma-buf cleanup
URL   : https://patchwork.freedesktop.org/series/45890/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
80713ee8fc35 drm/i915: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/i915: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 36 lines checked
d134ee0947b1 drm/msm: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/msm: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
6ab47e6389fa drm/nouveau: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/nouveau: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 6 lines checked
ec7d80472d56 drm/vgem: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/vgem: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 27 lines checked
8c7adfdbb9b6 dma-fence: Polish kernel-doc for dma-fence.c

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

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

* ✓ Fi.CI.BAT: success for RESEND: dma-buf cleanup
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
                   ` (4 preceding siblings ...)
  2018-07-04  9:53 ` ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup Patchwork
@ 2018-07-04 10:08 ` Patchwork
  2018-07-04 11:27 ` ✓ Fi.CI.IGT: " Patchwork
  2018-08-09 14:21 ` ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup (rev2) Patchwork
  7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-07-04 10:08 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: RESEND: dma-buf cleanup
URL   : https://patchwork.freedesktop.org/series/45890/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4425 -> Patchwork_9520 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/45890/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         INCOMPLETE (fdo#103927) -> PASS

    
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927


== Participating hosts (46 -> 41) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4425 -> Patchwork_9520

  CI_DRM_4425: 655f2c563a64861ef52008d968092a62644caf96 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4534: aeb3f4143572b81a907921ad9442858aafe1931e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9520: 8c7adfdbb9b6239efac49d24cb88a15f0b9cf1d4 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

8c7adfdbb9b6 dma-fence: Polish kernel-doc for dma-fence.c
ec7d80472d56 drm/vgem: Remove unecessary dma_fence_ops
6ab47e6389fa drm/nouveau: Remove unecessary dma_fence_ops
d134ee0947b1 drm/msm: Remove unecessary dma_fence_ops
80713ee8fc35 drm/i915: Remove unecessary dma_fence_ops

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for RESEND: dma-buf cleanup
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
                   ` (5 preceding siblings ...)
  2018-07-04 10:08 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-07-04 11:27 ` Patchwork
  2018-08-09 14:21 ` ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup (rev2) Patchwork
  7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-07-04 11:27 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: RESEND: dma-buf cleanup
URL   : https://patchwork.freedesktop.org/series/45890/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4425_full -> Patchwork_9520_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9520_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9520_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_9520_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-bsd2:
      shard-kbl:          PASS -> SKIP +1

    igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
      shard-snb:          SKIP -> PASS +1

    igt@perf_pmu@rc6:
      shard-kbl:          SKIP -> PASS

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-glk:          PASS -> FAIL (fdo#106886)

    igt@gem_ppgtt@blt-vs-render-ctx0:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665, fdo#106023)

    igt@kms_flip@modeset-vs-vblank-race-interruptible:
      shard-hsw:          PASS -> FAIL (fdo#103060)

    igt@kms_flip_tiling@flip-to-y-tiled:
      shard-glk:          PASS -> FAIL (fdo#103822, fdo#104724)

    igt@kms_flip_tiling@flip-y-tiled:
      shard-glk:          NOTRUN -> FAIL (fdo#104724)

    
    ==== Possible fixes ====

    igt@kms_flip@2x-flip-vs-expired-vblank:
      shard-hsw:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@wf_vblank-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103822 https://bugs.freedesktop.org/show_bug.cgi?id=103822
  fdo#104724 https://bugs.freedesktop.org/show_bug.cgi?id=104724
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4425 -> Patchwork_9520

  CI_DRM_4425: 655f2c563a64861ef52008d968092a62644caf96 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4534: aeb3f4143572b81a907921ad9442858aafe1931e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9520: 8c7adfdbb9b6239efac49d24cb88a15f0b9cf1d4 @ 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_9520/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
  2018-07-04  9:29 ` [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops Daniel Vetter
@ 2018-07-04 12:03   ` Emil Velikov
  2018-07-04 12:34     ` Daniel Vetter
  0 siblings, 1 reply; 22+ messages in thread
From: Emil Velikov @ 2018-07-04 12:03 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Intel Graphics Development, DRI Development, Rodrigo Vivi,
	Colin Ian King, Mika Kuoppala

Hi Daniel,

On 4 July 2018 at 10:29, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
>
> v2: Also remove the relase hook, dma_fence_free is the default.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Colin Ian King <colin.king@canonical.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/i915/i915_gem_clflush.c        | 7 -------
>  drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8 --------
>  2 files changed, 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
> index f5c570d35b2a..8e74c23cbd91 100644
> --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
> +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
> @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
>         return "clflush";
>  }
>
> -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
> -{
> -       return true;
> -}
> -
>  static void i915_clflush_release(struct dma_fence *fence)
>  {
>         struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
> @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
>  static const struct dma_fence_ops i915_clflush_ops = {
>         .get_driver_name = i915_clflush_get_driver_name,
>         .get_timeline_name = i915_clflush_get_timeline_name,
> -       .enable_signaling = i915_clflush_enable_signaling,
From a quick look through drm-misc/drm-misc-next removing the
enable_signalling hook may cause functional changes.

Namely:
A call to trace_dma_fence_enable_signal() (in
dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
will be omitted.

Removing the default .wait and .release hooks is fine.

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

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

* Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
  2018-07-04 12:03   ` Emil Velikov
@ 2018-07-04 12:34     ` Daniel Vetter
  2018-07-04 17:22       ` Emil Velikov
  0 siblings, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04 12:34 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Rodrigo Vivi, Colin Ian King, Mika Kuoppala

On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
> Hi Daniel,
> 
> On 4 July 2018 at 10:29, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > dma_fence_default_wait is the default now, same for the trivial
> > enable_signaling implementation.
> >
> > v2: Also remove the relase hook, dma_fence_free is the default.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Colin Ian King <colin.king@canonical.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/i915/i915_gem_clflush.c        | 7 -------
> >  drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8 --------
> >  2 files changed, 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
> > index f5c570d35b2a..8e74c23cbd91 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
> > @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
> >         return "clflush";
> >  }
> >
> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
> > -{
> > -       return true;
> > -}
> > -
> >  static void i915_clflush_release(struct dma_fence *fence)
> >  {
> >         struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
> >  static const struct dma_fence_ops i915_clflush_ops = {
> >         .get_driver_name = i915_clflush_get_driver_name,
> >         .get_timeline_name = i915_clflush_get_timeline_name,
> > -       .enable_signaling = i915_clflush_enable_signaling,
> From a quick look through drm-misc/drm-misc-next removing the
> enable_signalling hook may cause functional changes.
> 
> Namely:
> A call to trace_dma_fence_enable_signal() (in
> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
> will be omitted.

I'm not sure what this tracepoint is useful for in the absenve of a real
signaling mechanism that must be enabled (like interrupts).

For all the other bits (begin/end wait, fence signalling itsefl) we have
tracepoints already, so I think we're all covered. What do you think will
be lost with the tracepoint here? If there's a real need for it I think I
can rework the already merged patch to still call the tracpoint, while
avoiding everything else. I just don't see the use-case for that.
-Daniel

> 
> Removing the default .wait and .release hooks is fine.
> 
> HTH
> Emil

-- 
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] 22+ messages in thread

* Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
  2018-07-04 12:34     ` Daniel Vetter
@ 2018-07-04 17:22       ` Emil Velikov
  2018-07-04 20:03         ` Daniel Vetter
  0 siblings, 1 reply; 22+ messages in thread
From: Emil Velikov @ 2018-07-04 17:22 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Rodrigo Vivi, Colin Ian King, Mika Kuoppala

On 4 July 2018 at 13:34, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
>> Hi Daniel,
>>
>> On 4 July 2018 at 10:29, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> > dma_fence_default_wait is the default now, same for the trivial
>> > enable_signaling implementation.
>> >
>> > v2: Also remove the relase hook, dma_fence_free is the default.
>> >
>> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> > Cc: Colin Ian King <colin.king@canonical.com>
>> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> > Cc: intel-gfx@lists.freedesktop.org
>> > ---
>> >  drivers/gpu/drm/i915/i915_gem_clflush.c        | 7 -------
>> >  drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8 --------
>> >  2 files changed, 15 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > index f5c570d35b2a..8e74c23cbd91 100644
>> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
>> > @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
>> >         return "clflush";
>> >  }
>> >
>> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
>> > -{
>> > -       return true;
>> > -}
>> > -
>> >  static void i915_clflush_release(struct dma_fence *fence)
>> >  {
>> >         struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
>> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
>> >  static const struct dma_fence_ops i915_clflush_ops = {
>> >         .get_driver_name = i915_clflush_get_driver_name,
>> >         .get_timeline_name = i915_clflush_get_timeline_name,
>> > -       .enable_signaling = i915_clflush_enable_signaling,
>> From a quick look through drm-misc/drm-misc-next removing the
>> enable_signalling hook may cause functional changes.
>>
>> Namely:
>> A call to trace_dma_fence_enable_signal() (in
>> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
>> will be omitted.
>
> I'm not sure what this tracepoint is useful for in the absenve of a real
> signaling mechanism that must be enabled (like interrupts).
>
> For all the other bits (begin/end wait, fence signalling itsefl) we have
> tracepoints already, so I think we're all covered. What do you think will
> be lost with the tracepoint here? If there's a real need for it I think I
> can rework the already merged patch to still call the tracpoint, while
> avoiding everything else. I just don't see the use-case for that.

Nothing obvious comes to mind, yet again my knowledge of the fence API
is limited.
Was simply pointing out something that was removed without a small
note covering it.

A fraction of your explanation would have been great, but obviously
not a big deal either way.

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

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

* Re: [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops
  2018-07-04 17:22       ` Emil Velikov
@ 2018-07-04 20:03         ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-07-04 20:03 UTC (permalink / raw)
  To: Emil Velikov
  Cc: Intel Graphics Development, DRI Development, Rodrigo Vivi,
	Colin Ian King, Mika Kuoppala

On Wed, Jul 4, 2018 at 7:22 PM, Emil Velikov <emil.l.velikov@gmail.com> wrote:
> On 4 July 2018 at 13:34, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Wed, Jul 04, 2018 at 01:03:18PM +0100, Emil Velikov wrote:
>>> Hi Daniel,
>>>
>>> On 4 July 2018 at 10:29, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>> > dma_fence_default_wait is the default now, same for the trivial
>>> > enable_signaling implementation.
>>> >
>>> > v2: Also remove the relase hook, dma_fence_free is the default.
>>> >
>>> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>> > Cc: Colin Ian King <colin.king@canonical.com>
>>> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> > Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>>> > Cc: intel-gfx@lists.freedesktop.org
>>> > ---
>>> >  drivers/gpu/drm/i915/i915_gem_clflush.c        | 7 -------
>>> >  drivers/gpu/drm/i915/selftests/i915_sw_fence.c | 8 --------
>>> >  2 files changed, 15 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > index f5c570d35b2a..8e74c23cbd91 100644
>>> > --- a/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > +++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
>>> > @@ -45,11 +45,6 @@ static const char *i915_clflush_get_timeline_name(struct dma_fence *fence)
>>> >         return "clflush";
>>> >  }
>>> >
>>> > -static bool i915_clflush_enable_signaling(struct dma_fence *fence)
>>> > -{
>>> > -       return true;
>>> > -}
>>> > -
>>> >  static void i915_clflush_release(struct dma_fence *fence)
>>> >  {
>>> >         struct clflush *clflush = container_of(fence, typeof(*clflush), dma);
>>> > @@ -63,8 +58,6 @@ static void i915_clflush_release(struct dma_fence *fence)
>>> >  static const struct dma_fence_ops i915_clflush_ops = {
>>> >         .get_driver_name = i915_clflush_get_driver_name,
>>> >         .get_timeline_name = i915_clflush_get_timeline_name,
>>> > -       .enable_signaling = i915_clflush_enable_signaling,
>>> From a quick look through drm-misc/drm-misc-next removing the
>>> enable_signalling hook may cause functional changes.
>>>
>>> Namely:
>>> A call to trace_dma_fence_enable_signal() (in
>>> dma_fence_enable_sw_signaling(), dma_fence_add_callback() and others)
>>> will be omitted.
>>
>> I'm not sure what this tracepoint is useful for in the absenve of a real
>> signaling mechanism that must be enabled (like interrupts).
>>
>> For all the other bits (begin/end wait, fence signalling itsefl) we have
>> tracepoints already, so I think we're all covered. What do you think will
>> be lost with the tracepoint here? If there's a real need for it I think I
>> can rework the already merged patch to still call the tracpoint, while
>> avoiding everything else. I just don't see the use-case for that.
>
> Nothing obvious comes to mind, yet again my knowledge of the fence API
> is limited.
> Was simply pointing out something that was removed without a small
> note covering it.
>
> A fraction of your explanation would have been great, but obviously
> not a big deal either way.

Yeah would have been good to add to the commit message that made
->enable_signaling optional, but that's now baked into history already
:-/
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - 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] 22+ messages in thread

* Re: [PATCH 4/5] drm/vgem: Remove unecessary dma_fence_ops
  2018-07-04  9:29 ` [PATCH 4/5] drm/vgem: " Daniel Vetter
@ 2018-08-09  8:33   ` Daniel Vetter
  2018-08-09  8:38     ` Chris Wilson
  2018-08-09 12:45   ` [PATCH] " Daniel Vetter
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2018-08-09  8:33 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Kees Cook, Cihangir Akturk

On Wed, Jul 04, 2018 at 11:29:08AM +0200, Daniel Vetter wrote:
> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
> 
> Also remove the ->signaled callback, vgem can't peek ahead with a
> fastpath, returning false is the default implementation.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Cihangir Akturk <cakturk@gmail.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Anyone feel like reviewing patches 1-4 here?

Thanks, Daniel

> ---
>  drivers/gpu/drm/vgem/vgem_fence.c | 14 --------------
>  1 file changed, 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
> index b28876c222b4..75adedeaa384 100644
> --- a/drivers/gpu/drm/vgem/vgem_fence.c
> +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> @@ -43,16 +43,6 @@ static const char *vgem_fence_get_timeline_name(struct dma_fence *fence)
>  	return "unbound";
>  }
>  
> -static bool vgem_fence_signaled(struct dma_fence *fence)
> -{
> -	return false;
> -}
> -
> -static bool vgem_fence_enable_signaling(struct dma_fence *fence)
> -{
> -	return true;
> -}
> -
>  static void vgem_fence_release(struct dma_fence *base)
>  {
>  	struct vgem_fence *fence = container_of(base, typeof(*fence), base);
> @@ -76,11 +66,7 @@ static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
>  static const struct dma_fence_ops vgem_fence_ops = {
>  	.get_driver_name = vgem_fence_get_driver_name,
>  	.get_timeline_name = vgem_fence_get_timeline_name,
> -	.enable_signaling = vgem_fence_enable_signaling,
> -	.signaled = vgem_fence_signaled,
> -	.wait = dma_fence_default_wait,
>  	.release = vgem_fence_release,
> -
>  	.fence_value_str = vgem_fence_value_str,
>  	.timeline_value_str = vgem_fence_timeline_value_str,
>  };
> -- 
> 2.18.0
> 

-- 
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] 22+ messages in thread

* Re: [PATCH 4/5] drm/vgem: Remove unecessary dma_fence_ops
  2018-08-09  8:33   ` Daniel Vetter
@ 2018-08-09  8:38     ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2018-08-09  8:38 UTC (permalink / raw)
  To: DRI Development, Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, Kees Cook, Cihangir Akturk

Quoting Daniel Vetter (2018-08-09 09:33:49)
> On Wed, Jul 04, 2018 at 11:29:08AM +0200, Daniel Vetter wrote:
> >  static void vgem_fence_release(struct dma_fence *base)
> >  {
> >       struct vgem_fence *fence = container_of(base, typeof(*fence), base);
> > @@ -76,11 +66,7 @@ static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
> >  static const struct dma_fence_ops vgem_fence_ops = {
> >       .get_driver_name = vgem_fence_get_driver_name,
> >       .get_timeline_name = vgem_fence_get_timeline_name,
> > -     .enable_signaling = vgem_fence_enable_signaling,
> > -     .signaled = vgem_fence_signaled,
> > -     .wait = dma_fence_default_wait,
> >       .release = vgem_fence_release,
> > -

That space was to separate the interesting ops from the debug!
-Chris

> >       .fence_value_str = vgem_fence_value_str,
> >       .timeline_value_str = vgem_fence_timeline_value_str,
> >  };
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/vgem: Remove unecessary dma_fence_ops
  2018-07-04  9:29 ` [PATCH 4/5] drm/vgem: " Daniel Vetter
  2018-08-09  8:33   ` Daniel Vetter
@ 2018-08-09 12:45   ` Daniel Vetter
  2018-08-09 12:48     ` Chris Wilson
  1 sibling, 1 reply; 22+ messages in thread
From: Daniel Vetter @ 2018-08-09 12:45 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Kees Cook, Daniel Vetter, DRI Development, Cihangir Akturk

dma_fence_default_wait is the default now, same for the trivial
enable_signaling implementation.

Also remove the ->signaled callback, vgem can't peek ahead with a
fastpath, returning false is the default implementation.

v2: Protect the meaningful space! (Chris)

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Kees Cook <keescook@chromium.org>
Cc: Cihangir Akturk <cakturk@gmail.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/vgem/vgem_fence.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
index b28876c222b4..e6ee71323a66 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -43,16 +43,6 @@ static const char *vgem_fence_get_timeline_name(struct dma_fence *fence)
 	return "unbound";
 }
 
-static bool vgem_fence_signaled(struct dma_fence *fence)
-{
-	return false;
-}
-
-static bool vgem_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static void vgem_fence_release(struct dma_fence *base)
 {
 	struct vgem_fence *fence = container_of(base, typeof(*fence), base);
@@ -76,9 +66,6 @@ static void vgem_fence_timeline_value_str(struct dma_fence *fence, char *str,
 static const struct dma_fence_ops vgem_fence_ops = {
 	.get_driver_name = vgem_fence_get_driver_name,
 	.get_timeline_name = vgem_fence_get_timeline_name,
-	.enable_signaling = vgem_fence_enable_signaling,
-	.signaled = vgem_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = vgem_fence_release,
 
 	.fence_value_str = vgem_fence_value_str,
-- 
2.18.0

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

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

* Re: [PATCH] drm/vgem: Remove unecessary dma_fence_ops
  2018-08-09 12:45   ` [PATCH] " Daniel Vetter
@ 2018-08-09 12:48     ` Chris Wilson
  2018-08-17  9:23       ` Daniel Vetter
  0 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2018-08-09 12:48 UTC (permalink / raw)
  To: Intel Graphics Development
  Cc: Daniel Vetter, Kees Cook, DRI Development, Cihangir Akturk

Quoting Daniel Vetter (2018-08-09 13:45:44)
> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
> 
> Also remove the ->signaled callback, vgem can't peek ahead with a
> fastpath, returning false is the default implementation.
> 
> v2: Protect the meaningful space! (Chris)
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Cihangir Akturk <cakturk@gmail.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

1-4,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup (rev2)
  2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
                   ` (6 preceding siblings ...)
  2018-07-04 11:27 ` ✓ Fi.CI.IGT: " Patchwork
@ 2018-08-09 14:21 ` Patchwork
  7 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2018-08-09 14:21 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: RESEND: dma-buf cleanup (rev2)
URL   : https://patchwork.freedesktop.org/series/45890/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e8a45f5bac39 drm/i915: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/i915: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 36 lines checked
2074c167fc25 drm/msm: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/msm: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 21 lines checked
c696f9f7b3eb drm/nouveau: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/nouveau: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 6 lines checked
c7d18e95768f drm/vgem: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/vgem: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 25 lines checked

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

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

* Re: [PATCH] drm/vgem: Remove unecessary dma_fence_ops
  2018-08-09 12:48     ` Chris Wilson
@ 2018-08-17  9:23       ` Daniel Vetter
  0 siblings, 0 replies; 22+ messages in thread
From: Daniel Vetter @ 2018-08-17  9:23 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Kees Cook, Daniel Vetter, Intel Graphics Development,
	DRI Development, Cihangir Akturk, Sean Paul

On Thu, Aug 09, 2018 at 01:48:42PM +0100, Chris Wilson wrote:
> Quoting Daniel Vetter (2018-08-09 13:45:44)
> > dma_fence_default_wait is the default now, same for the trivial
> > enable_signaling implementation.
> > 
> > Also remove the ->signaled callback, vgem can't peek ahead with a
> > fastpath, returning false is the default implementation.
> > 
> > v2: Protect the meaningful space! (Chris)
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Cihangir Akturk <cakturk@gmail.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Sean Paul <seanpaul@chromium.org>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> 1-4,
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks for your reviews, all four pushed to drm-misc-next now.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2018-08-17  9:23 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-04  9:29 [PATCH 0/5] RESEND: dma-buf cleanup Daniel Vetter
2018-07-04  9:29 ` [PATCH 1/5] drm/i915: Remove unecessary dma_fence_ops Daniel Vetter
2018-07-04 12:03   ` Emil Velikov
2018-07-04 12:34     ` Daniel Vetter
2018-07-04 17:22       ` Emil Velikov
2018-07-04 20:03         ` Daniel Vetter
     [not found] ` <20180704092909.6599-1-daniel.vetter-/w4YWyX8dFk@public.gmane.org>
2018-07-04  9:29   ` [PATCH 2/5] drm/msm: " Daniel Vetter
2018-07-04  9:29   ` [PATCH 3/5] drm/nouveau: " Daniel Vetter
2018-07-04  9:29 ` [PATCH 4/5] drm/vgem: " Daniel Vetter
2018-08-09  8:33   ` Daniel Vetter
2018-08-09  8:38     ` Chris Wilson
2018-08-09 12:45   ` [PATCH] " Daniel Vetter
2018-08-09 12:48     ` Chris Wilson
2018-08-17  9:23       ` Daniel Vetter
2018-07-04  9:29 ` [PATCH 5/5] dma-fence: Polish kernel-doc for dma-fence.c Daniel Vetter
2018-07-04  9:29   ` Daniel Vetter
2018-07-04  9:36   ` Christian König
2018-07-04  9:36     ` Christian König
2018-07-04  9:53 ` ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup Patchwork
2018-07-04 10:08 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-04 11:27 ` ✓ Fi.CI.IGT: " Patchwork
2018-08-09 14:21 ` ✗ Fi.CI.CHECKPATCH: warning for RESEND: dma-buf cleanup (rev2) Patchwork

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.