[PATCH 00/17] dma-fence doc polish and small cleanup

[PATCH 00/17] dma-fence doc polish and small cleanup

[Daniel Vetter]

Hi all,

Somewhat motivated by me looking at the v3d patch I went and dug around
in the dma-fence code a bit. Result was a bit of doc polish and making a
bunch of callbacks optional for cases where most implementations wanted
the same default behaviour.

Comments, testing, feedback very much welcome.

Thanks, Daniel

Daniel Vetter (17):
  dma-fence: Some kerneldoc polish for dma-fence.h
  dma-fence: remove fill_driver_data callback
  dma-fence: Make ->enable_signaling optional
  dma-fence: Allow wait_any_timeout for all fences
  dma-fence: Make ->wait callback optional
  drm/amdgpu: Remove unecessary dma_fence_ops
  drm: Remove unecessary dma_fence_ops
  drm/etnaviv: Remove unecessary dma_fence_ops
  drm/i915: Remove unecessary dma_fence_ops
  drm/msm: Remove unecessary dma_fence_ops
  drm/nouveau: Remove unecessary dma_fence_ops
  drm/qxl: Remove unecessary dma_fence_ops
  drm/radeon: Remove custom dma_fence_ops->wait implementation
  drm/vc4: Remove unecessary dma_fence_ops
  drm/vgem: Remove unecessary dma_fence_ops
  drm/virtio: 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-array.c             |   1 -
 drivers/dma-buf/dma-fence.c                   | 163 ++++++++----
 drivers/dma-buf/sw_sync.c                     |   1 -
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c  |   2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c     |   1 -
 drivers/gpu/drm/drm_crtc.c                    |   7 -
 drivers/gpu/drm/drm_syncobj.c                 |   1 -
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c         |   7 -
 drivers/gpu/drm/i915/i915_gem_clflush.c       |   7 -
 .../gpu/drm/i915/selftests/i915_sw_fence.c    |   7 -
 drivers/gpu/drm/msm/msm_fence.c               |   7 -
 drivers/gpu/drm/nouveau/nouveau_fence.c       |   1 -
 drivers/gpu/drm/qxl/qxl_release.c             |   7 -
 drivers/gpu/drm/radeon/radeon_fence.c         |  63 -----
 drivers/gpu/drm/scheduler/sched_fence.c       |  11 -
 drivers/gpu/drm/vc4/vc4_fence.c               |   7 -
 drivers/gpu/drm/vgem/vgem_fence.c             |  14 --
 drivers/gpu/drm/virtio/virtgpu_fence.c        |   7 -
 include/linux/dma-fence.h                     | 231 +++++++++++-------
 20 files changed, 267 insertions(+), 284 deletions(-)

-- 
2.17.0

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

[PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Daniel Vetter]

- Switch to inline member docs for dma_fence_ops.
- Mild polish all around.
- hyperlink all the things!

v2: - Remove the various [in] annotations, they seem really uncommon
in kerneldoc and look funny.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 include/linux/dma-fence.h | 235 +++++++++++++++++++++++++-------------
 1 file changed, 154 insertions(+), 81 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 4c008170fe65..9d6f39bf2111 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -94,11 +94,11 @@ typedef void (*dma_fence_func_t)(struct dma_fence *fence,
 				 struct dma_fence_cb *cb);
 
 /**
- * struct dma_fence_cb - callback for dma_fence_add_callback
- * @node: used by dma_fence_add_callback to append this struct to fence::cb_list
+ * struct dma_fence_cb - callback for dma_fence_add_callback()
+ * @node: used by dma_fence_add_callback() to append this struct to fence::cb_list
  * @func: dma_fence_func_t to call
  *
- * This struct will be initialized by dma_fence_add_callback, additional
+ * This struct will be initialized by dma_fence_add_callback(), additional
  * data can be passed along by embedding dma_fence_cb in another struct.
  */
 struct dma_fence_cb {
@@ -108,75 +108,142 @@ struct dma_fence_cb {
 
 /**
  * struct dma_fence_ops - operations implemented for fence
- * @get_driver_name: returns the driver name.
- * @get_timeline_name: return the name of the context this fence belongs to.
- * @enable_signaling: enable software signaling of fence.
- * @signaled: [optional] peek whether the fence is signaled, can be null.
- * @wait: custom wait implementation, or dma_fence_default_wait.
- * @release: [optional] called on destruction of fence, can be null
- * @fill_driver_data: [optional] callback to fill in free-form debug info
- * Returns amount of bytes filled, or -errno.
- * @fence_value_str: [optional] fills in the value of the fence as a string
- * @timeline_value_str: [optional] fills in the current value of the timeline
- * as a string
  *
- * Notes on enable_signaling:
- * For fence implementations that have the capability for hw->hw
- * signaling, they can implement this op to enable the necessary
- * irqs, or insert commands into cmdstream, etc.  This is called
- * in the first wait() or add_callback() path to let the fence
- * implementation know that there is another driver waiting on
- * the signal (ie. hw->sw case).
- *
- * This function can be called from atomic context, but not
- * from irq context, so normal spinlocks can be used.
- *
- * A return value of false indicates the fence already passed,
- * or some failure occurred that made it impossible to enable
- * signaling. True indicates successful enabling.
- *
- * fence->error may be set in enable_signaling, but only when false is
- * returned.
- *
- * Calling dma_fence_signal before enable_signaling is called allows
- * for a tiny race window in which enable_signaling is called during,
- * before, or after dma_fence_signal. To fight this, it is recommended
- * that before enable_signaling returns true an extra reference is
- * taken on the fence, to be released when the fence is signaled.
- * This will mean dma_fence_signal will still be called twice, but
- * the second time will be a noop since it was already signaled.
- *
- * Notes on signaled:
- * May set fence->error if returning true.
- *
- * Notes on wait:
- * Must not be NULL, set to dma_fence_default_wait for default implementation.
- * the dma_fence_default_wait implementation should work for any fence, as long
- * as enable_signaling works correctly.
- *
- * Must return -ERESTARTSYS if the wait is intr = true and the wait was
- * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
- * timed out. Can also return other error values on custom implementations,
- * which should be treated as if the fence is signaled. For example a hardware
- * lockup could be reported like that.
- *
- * Notes on release:
- * Can be NULL, this function allows additional commands to run on
- * destruction of the fence. Can be called from irq context.
- * If pointer is set to NULL, kfree will get called instead.
  */
-
 struct dma_fence_ops {
+	/**
+	 * @get_driver_name:
+	 *
+	 * Returns the driver name. This is a callback to allow drivers to
+	 * compute the name at runtime, without having it to store permanently
+	 * for each fence, or build a cache of some sort.
+	 *
+	 * This callback is mandatory.
+	 */
 	const char * (*get_driver_name)(struct dma_fence *fence);
+
+	/**
+	 * @get_timeline_name:
+	 *
+	 * Return the name of the context this fence belongs to. This is a
+	 * callback to allow drivers to compute the name at runtime, without
+	 * having it to store permanently for each fence, or build a cache of
+	 * some sort.
+	 *
+	 * This callback is mandatory.
+	 */
 	const char * (*get_timeline_name)(struct dma_fence *fence);
+
+	/**
+	 * @enable_signaling:
+	 *
+	 * Enable software signaling of fence.
+	 *
+	 * For fence implementations that have the capability for hw->hw
+	 * signaling, they can implement this op to enable the necessary
+	 * interrupts, or insert commands into cmdstream, etc, to avoid these
+	 * costly operations for the common case where only hw->hw
+	 * synchronization is required.  This is called in the first
+	 * dma_fence_wait() or dma_fence_add_callback() path to let the fence
+	 * implementation know that there is another driver waiting on the
+	 * signal (ie. hw->sw case).
+	 *
+	 * This function can be called from atomic context, but not
+	 * from irq context, so normal spinlocks can be used.
+	 *
+	 * A return value of false indicates the fence already passed,
+	 * or some failure occurred that made it impossible to enable
+	 * signaling. True indicates successful enabling.
+	 *
+	 * &dma_fence.error may be set in enable_signaling, but only when false
+	 * is returned.
+	 *
+	 * Since many implementations can call dma_fence_signal() even when before
+	 * @enable_signaling has been called there's a race window, where the
+	 * dma_fence_signal() might result in the final fence reference being
+	 * released and its memory freed. To avoid this, implementations of this
+	 * callback should grab their own reference using dma_fence_get(), to be
+	 * released when the fence is signalled (through e.g. the interrupt
+	 * handler).
+	 *
+	 * This callback is mandatory.
+	 */
 	bool (*enable_signaling)(struct dma_fence *fence);
+
+	/**
+	 * @signaled:
+	 *
+	 * Peek whether the fence is signaled, as a fastpath optimization for
+	 * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this
+	 * callback does not need to make any guarantees beyond that a fence
+	 * once indicates as signalled must always return true from this
+	 * callback. This callback may return false even if the fence has
+	 * completed already, in this case information hasn't propogated throug
+	 * the system yet. See also dma_fence_is_signaled().
+	 *
+	 * May set &dma_fence.error if returning true.
+	 *
+	 * This callback is optional.
+	 */
 	bool (*signaled)(struct dma_fence *fence);
+
+	/**
+	 * @wait:
+	 *
+	 * Custom wait implementation, or dma_fence_default_wait.
+	 *
+	 * Must not be NULL, set to dma_fence_default_wait for default implementation.
+	 * the dma_fence_default_wait implementation should work for any fence, as long
+	 * as enable_signaling works correctly.
+	 *
+	 * Must return -ERESTARTSYS if the wait is intr = true and the wait was
+	 * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
+	 * timed out. Can also return other error values on custom implementations,
+	 * which should be treated as if the fence is signaled. For example a hardware
+	 * lockup could be reported like that.
+	 *
+	 * This callback is mandatory.
+	 */
 	signed long (*wait)(struct dma_fence *fence,
 			    bool intr, signed long timeout);
+
+	/**
+	 * @release:
+	 *
+	 * Called on destruction of fence to release additional resources.
+	 * Can be called from irq context.  This callback is optional. If it is
+	 * NULL, then dma_fence_free() is instead called as the default
+	 * implementation.
+	 */
 	void (*release)(struct dma_fence *fence);
 
+	/**
+	 * @fill_driver_data:
+	 *
+	 * Callback to fill in free-form debug info Returns amount of bytes
+	 * filled, or negative error on failure.
+	 *
+	 * This callback is optional.
+	 */
 	int (*fill_driver_data)(struct dma_fence *fence, void *data, int size);
+
+	/**
+	 * @fence_value_str:
+	 *
+	 * Callback to fill in free-form debug info specific to this fence, like
+	 * the sequence number.
+	 *
+	 * This callback is optional.
+	 */
 	void (*fence_value_str)(struct dma_fence *fence, char *str, int size);
+
+	/**
+	 * @timeline_value_str:
+	 *
+	 * Fills in the current value of the timeline as a string, like the
+	 * sequence number. This should match what @fill_driver_data prints for
+	 * the most recently signalled fence (assuming no delayed signalling).
+	 */
 	void (*timeline_value_str)(struct dma_fence *fence,
 				   char *str, int size);
 };
@@ -189,7 +256,7 @@ void dma_fence_free(struct dma_fence *fence);
 
 /**
  * dma_fence_put - decreases refcount of the fence
- * @fence:	[in]	fence to reduce refcount of
+ * @fence: fence to reduce refcount of
  */
 static inline void dma_fence_put(struct dma_fence *fence)
 {
@@ -199,7 +266,7 @@ static inline void dma_fence_put(struct dma_fence *fence)
 
 /**
  * dma_fence_get - increases refcount of the fence
- * @fence:	[in]	fence to increase refcount of
+ * @fence: fence to increase refcount of
  *
  * Returns the same fence, with refcount increased by 1.
  */
@@ -213,7 +280,7 @@ static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
 /**
  * dma_fence_get_rcu - get a fence from a reservation_object_list with
  *                     rcu read lock
- * @fence:	[in]	fence to increase refcount of
+ * @fence: fence to increase refcount of
  *
  * Function returns NULL if no refcount could be obtained, or the fence.
  */
@@ -227,7 +294,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
 
 /**
  * dma_fence_get_rcu_safe  - acquire a reference to an RCU tracked fence
- * @fencep:	[in]	pointer to fence to increase refcount of
+ * @fencep: pointer to fence to increase refcount of
  *
  * Function returns NULL if no refcount could be obtained, or the fence.
  * This function handles acquiring a reference to a fence that may be
@@ -289,14 +356,16 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 /**
  * dma_fence_is_signaled_locked - Return an indication if the fence
  *                                is signaled yet.
- * @fence:	[in]	the fence to check
+ * @fence: the fence to check
  *
  * Returns true if the fence was already signaled, false if not. Since this
  * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback, dma_fence_wait or
- * dma_fence_enable_sw_signaling haven't been called before.
+ * true if dma_fence_add_callback(), dma_fence_wait() or
+ * dma_fence_enable_sw_signaling() haven't been called before.
  *
- * This function requires fence->lock to be held.
+ * This function requires &dma_fence.lock to be held.
+ *
+ * See also dma_fence_is_signaled().
  */
 static inline bool
 dma_fence_is_signaled_locked(struct dma_fence *fence)
@@ -314,17 +383,19 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
 
 /**
  * dma_fence_is_signaled - Return an indication if the fence is signaled yet.
- * @fence:	[in]	the fence to check
+ * @fence: the fence to check
  *
  * Returns true if the fence was already signaled, false if not. Since this
  * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback, dma_fence_wait or
- * dma_fence_enable_sw_signaling haven't been called before.
+ * true if dma_fence_add_callback(), dma_fence_wait() or
+ * dma_fence_enable_sw_signaling() haven't been called before.
  *
  * It's recommended for seqno fences to call dma_fence_signal when the
  * operation is complete, it makes it possible to prevent issues from
  * wraparound between time of issue and time of use by checking the return
  * value of this function before calling hardware-specific wait instructions.
+ *
+ * See also dma_fence_is_signaled_locked().
  */
 static inline bool
 dma_fence_is_signaled(struct dma_fence *fence)
@@ -342,8 +413,8 @@ dma_fence_is_signaled(struct dma_fence *fence)
 
 /**
  * __dma_fence_is_later - return if f1 is chronologically later than f2
- * @f1:	[in]	the first fence's seqno
- * @f2:	[in]	the second fence's seqno from the same context
+ * @f1: the first fence's seqno
+ * @f2: the second fence's seqno from the same context
  *
  * Returns true if f1 is chronologically later than f2. Both fences must be
  * from the same context, since a seqno is not common across contexts.
@@ -355,8 +426,8 @@ static inline bool __dma_fence_is_later(u32 f1, u32 f2)
 
 /**
  * dma_fence_is_later - return if f1 is chronologically later than f2
- * @f1:	[in]	the first fence from the same context
- * @f2:	[in]	the second fence from the same context
+ * @f1: the first fence from the same context
+ * @f2: the second fence from the same context
  *
  * Returns true if f1 is chronologically later than f2. Both fences must be
  * from the same context, since a seqno is not re-used across contexts.
@@ -372,8 +443,8 @@ static inline bool dma_fence_is_later(struct dma_fence *f1,
 
 /**
  * dma_fence_later - return the chronologically later fence
- * @f1:	[in]	the first fence from the same context
- * @f2:	[in]	the second fence from the same context
+ * @f1:	the first fence from the same context
+ * @f2:	the second fence from the same context
  *
  * Returns NULL if both fences are signaled, otherwise the fence that would be
  * signaled last. Both fences must be from the same context, since a seqno is
@@ -398,7 +469,7 @@ static inline struct dma_fence *dma_fence_later(struct dma_fence *f1,
 
 /**
  * dma_fence_get_status_locked - returns the status upon completion
- * @fence: [in]	the dma_fence to query
+ * @fence: the dma_fence to query
  *
  * Drivers can supply an optional error status condition before they signal
  * the fence (to indicate whether the fence was completed due to an error
@@ -422,8 +493,8 @@ int dma_fence_get_status(struct dma_fence *fence);
 
 /**
  * dma_fence_set_error - flag an error condition on the fence
- * @fence: [in]	the dma_fence
- * @error: [in]	the error to store
+ * @fence: the dma_fence
+ * @error: the error to store
  *
  * Drivers can supply an optional error status condition before they signal
  * the fence, to indicate that the fence was completed due to an error
@@ -449,8 +520,8 @@ signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
 
 /**
  * dma_fence_wait - sleep until the fence gets signaled
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
  *
  * This function will return -ERESTARTSYS if interrupted by a signal,
  * or 0 if the fence was signaled. Other error values may be
@@ -459,6 +530,8 @@ signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
  * Performs a synchronous wait on this fence. It is assumed the caller
  * directly or indirectly holds a reference to the fence, otherwise the
  * fence might be freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait_timeout() and dma_fence_wait_any_timeout().
  */
 static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
 {
-- 
2.17.0

[PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Daniel Vetter]

- Switch to inline member docs for dma_fence_ops.
- Mild polish all around.
- hyperlink all the things!

v2: - Remove the various [in] annotations, they seem really uncommon
in kerneldoc and look funny.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Sumit Semwal <sumit.semwal@linaro.org>
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
---
 include/linux/dma-fence.h | 235 +++++++++++++++++++++++++-------------
 1 file changed, 154 insertions(+), 81 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 4c008170fe65..9d6f39bf2111 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -94,11 +94,11 @@ typedef void (*dma_fence_func_t)(struct dma_fence *fence,
 				 struct dma_fence_cb *cb);
 
 /**
- * struct dma_fence_cb - callback for dma_fence_add_callback
- * @node: used by dma_fence_add_callback to append this struct to fence::cb_list
+ * struct dma_fence_cb - callback for dma_fence_add_callback()
+ * @node: used by dma_fence_add_callback() to append this struct to fence::cb_list
  * @func: dma_fence_func_t to call
  *
- * This struct will be initialized by dma_fence_add_callback, additional
+ * This struct will be initialized by dma_fence_add_callback(), additional
  * data can be passed along by embedding dma_fence_cb in another struct.
  */
 struct dma_fence_cb {
@@ -108,75 +108,142 @@ struct dma_fence_cb {
 
 /**
  * struct dma_fence_ops - operations implemented for fence
- * @get_driver_name: returns the driver name.
- * @get_timeline_name: return the name of the context this fence belongs to.
- * @enable_signaling: enable software signaling of fence.
- * @signaled: [optional] peek whether the fence is signaled, can be null.
- * @wait: custom wait implementation, or dma_fence_default_wait.
- * @release: [optional] called on destruction of fence, can be null
- * @fill_driver_data: [optional] callback to fill in free-form debug info
- * Returns amount of bytes filled, or -errno.
- * @fence_value_str: [optional] fills in the value of the fence as a string
- * @timeline_value_str: [optional] fills in the current value of the timeline
- * as a string
  *
- * Notes on enable_signaling:
- * For fence implementations that have the capability for hw->hw
- * signaling, they can implement this op to enable the necessary
- * irqs, or insert commands into cmdstream, etc.  This is called
- * in the first wait() or add_callback() path to let the fence
- * implementation know that there is another driver waiting on
- * the signal (ie. hw->sw case).
- *
- * This function can be called from atomic context, but not
- * from irq context, so normal spinlocks can be used.
- *
- * A return value of false indicates the fence already passed,
- * or some failure occurred that made it impossible to enable
- * signaling. True indicates successful enabling.
- *
- * fence->error may be set in enable_signaling, but only when false is
- * returned.
- *
- * Calling dma_fence_signal before enable_signaling is called allows
- * for a tiny race window in which enable_signaling is called during,
- * before, or after dma_fence_signal. To fight this, it is recommended
- * that before enable_signaling returns true an extra reference is
- * taken on the fence, to be released when the fence is signaled.
- * This will mean dma_fence_signal will still be called twice, but
- * the second time will be a noop since it was already signaled.
- *
- * Notes on signaled:
- * May set fence->error if returning true.
- *
- * Notes on wait:
- * Must not be NULL, set to dma_fence_default_wait for default implementation.
- * the dma_fence_default_wait implementation should work for any fence, as long
- * as enable_signaling works correctly.
- *
- * Must return -ERESTARTSYS if the wait is intr = true and the wait was
- * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
- * timed out. Can also return other error values on custom implementations,
- * which should be treated as if the fence is signaled. For example a hardware
- * lockup could be reported like that.
- *
- * Notes on release:
- * Can be NULL, this function allows additional commands to run on
- * destruction of the fence. Can be called from irq context.
- * If pointer is set to NULL, kfree will get called instead.
  */
-
 struct dma_fence_ops {
+	/**
+	 * @get_driver_name:
+	 *
+	 * Returns the driver name. This is a callback to allow drivers to
+	 * compute the name at runtime, without having it to store permanently
+	 * for each fence, or build a cache of some sort.
+	 *
+	 * This callback is mandatory.
+	 */
 	const char * (*get_driver_name)(struct dma_fence *fence);
+
+	/**
+	 * @get_timeline_name:
+	 *
+	 * Return the name of the context this fence belongs to. This is a
+	 * callback to allow drivers to compute the name at runtime, without
+	 * having it to store permanently for each fence, or build a cache of
+	 * some sort.
+	 *
+	 * This callback is mandatory.
+	 */
 	const char * (*get_timeline_name)(struct dma_fence *fence);
+
+	/**
+	 * @enable_signaling:
+	 *
+	 * Enable software signaling of fence.
+	 *
+	 * For fence implementations that have the capability for hw->hw
+	 * signaling, they can implement this op to enable the necessary
+	 * interrupts, or insert commands into cmdstream, etc, to avoid these
+	 * costly operations for the common case where only hw->hw
+	 * synchronization is required.  This is called in the first
+	 * dma_fence_wait() or dma_fence_add_callback() path to let the fence
+	 * implementation know that there is another driver waiting on the
+	 * signal (ie. hw->sw case).
+	 *
+	 * This function can be called from atomic context, but not
+	 * from irq context, so normal spinlocks can be used.
+	 *
+	 * A return value of false indicates the fence already passed,
+	 * or some failure occurred that made it impossible to enable
+	 * signaling. True indicates successful enabling.
+	 *
+	 * &dma_fence.error may be set in enable_signaling, but only when false
+	 * is returned.
+	 *
+	 * Since many implementations can call dma_fence_signal() even when before
+	 * @enable_signaling has been called there's a race window, where the
+	 * dma_fence_signal() might result in the final fence reference being
+	 * released and its memory freed. To avoid this, implementations of this
+	 * callback should grab their own reference using dma_fence_get(), to be
+	 * released when the fence is signalled (through e.g. the interrupt
+	 * handler).
+	 *
+	 * This callback is mandatory.
+	 */
 	bool (*enable_signaling)(struct dma_fence *fence);
+
+	/**
+	 * @signaled:
+	 *
+	 * Peek whether the fence is signaled, as a fastpath optimization for
+	 * e.g. dma_fence_wait() or dma_fence_add_callback(). Note that this
+	 * callback does not need to make any guarantees beyond that a fence
+	 * once indicates as signalled must always return true from this
+	 * callback. This callback may return false even if the fence has
+	 * completed already, in this case information hasn't propogated throug
+	 * the system yet. See also dma_fence_is_signaled().
+	 *
+	 * May set &dma_fence.error if returning true.
+	 *
+	 * This callback is optional.
+	 */
 	bool (*signaled)(struct dma_fence *fence);
+
+	/**
+	 * @wait:
+	 *
+	 * Custom wait implementation, or dma_fence_default_wait.
+	 *
+	 * Must not be NULL, set to dma_fence_default_wait for default implementation.
+	 * the dma_fence_default_wait implementation should work for any fence, as long
+	 * as enable_signaling works correctly.
+	 *
+	 * Must return -ERESTARTSYS if the wait is intr = true and the wait was
+	 * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
+	 * timed out. Can also return other error values on custom implementations,
+	 * which should be treated as if the fence is signaled. For example a hardware
+	 * lockup could be reported like that.
+	 *
+	 * This callback is mandatory.
+	 */
 	signed long (*wait)(struct dma_fence *fence,
 			    bool intr, signed long timeout);
+
+	/**
+	 * @release:
+	 *
+	 * Called on destruction of fence to release additional resources.
+	 * Can be called from irq context.  This callback is optional. If it is
+	 * NULL, then dma_fence_free() is instead called as the default
+	 * implementation.
+	 */
 	void (*release)(struct dma_fence *fence);
 
+	/**
+	 * @fill_driver_data:
+	 *
+	 * Callback to fill in free-form debug info Returns amount of bytes
+	 * filled, or negative error on failure.
+	 *
+	 * This callback is optional.
+	 */
 	int (*fill_driver_data)(struct dma_fence *fence, void *data, int size);
+
+	/**
+	 * @fence_value_str:
+	 *
+	 * Callback to fill in free-form debug info specific to this fence, like
+	 * the sequence number.
+	 *
+	 * This callback is optional.
+	 */
 	void (*fence_value_str)(struct dma_fence *fence, char *str, int size);
+
+	/**
+	 * @timeline_value_str:
+	 *
+	 * Fills in the current value of the timeline as a string, like the
+	 * sequence number. This should match what @fill_driver_data prints for
+	 * the most recently signalled fence (assuming no delayed signalling).
+	 */
 	void (*timeline_value_str)(struct dma_fence *fence,
 				   char *str, int size);
 };
@@ -189,7 +256,7 @@ void dma_fence_free(struct dma_fence *fence);
 
 /**
  * dma_fence_put - decreases refcount of the fence
- * @fence:	[in]	fence to reduce refcount of
+ * @fence: fence to reduce refcount of
  */
 static inline void dma_fence_put(struct dma_fence *fence)
 {
@@ -199,7 +266,7 @@ static inline void dma_fence_put(struct dma_fence *fence)
 
 /**
  * dma_fence_get - increases refcount of the fence
- * @fence:	[in]	fence to increase refcount of
+ * @fence: fence to increase refcount of
  *
  * Returns the same fence, with refcount increased by 1.
  */
@@ -213,7 +280,7 @@ static inline struct dma_fence *dma_fence_get(struct dma_fence *fence)
 /**
  * dma_fence_get_rcu - get a fence from a reservation_object_list with
  *                     rcu read lock
- * @fence:	[in]	fence to increase refcount of
+ * @fence: fence to increase refcount of
  *
  * Function returns NULL if no refcount could be obtained, or the fence.
  */
@@ -227,7 +294,7 @@ static inline struct dma_fence *dma_fence_get_rcu(struct dma_fence *fence)
 
 /**
  * dma_fence_get_rcu_safe  - acquire a reference to an RCU tracked fence
- * @fencep:	[in]	pointer to fence to increase refcount of
+ * @fencep: pointer to fence to increase refcount of
  *
  * Function returns NULL if no refcount could be obtained, or the fence.
  * This function handles acquiring a reference to a fence that may be
@@ -289,14 +356,16 @@ void dma_fence_enable_sw_signaling(struct dma_fence *fence);
 /**
  * dma_fence_is_signaled_locked - Return an indication if the fence
  *                                is signaled yet.
- * @fence:	[in]	the fence to check
+ * @fence: the fence to check
  *
  * Returns true if the fence was already signaled, false if not. Since this
  * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback, dma_fence_wait or
- * dma_fence_enable_sw_signaling haven't been called before.
+ * true if dma_fence_add_callback(), dma_fence_wait() or
+ * dma_fence_enable_sw_signaling() haven't been called before.
  *
- * This function requires fence->lock to be held.
+ * This function requires &dma_fence.lock to be held.
+ *
+ * See also dma_fence_is_signaled().
  */
 static inline bool
 dma_fence_is_signaled_locked(struct dma_fence *fence)
@@ -314,17 +383,19 @@ dma_fence_is_signaled_locked(struct dma_fence *fence)
 
 /**
  * dma_fence_is_signaled - Return an indication if the fence is signaled yet.
- * @fence:	[in]	the fence to check
+ * @fence: the fence to check
  *
  * Returns true if the fence was already signaled, false if not. Since this
  * function doesn't enable signaling, it is not guaranteed to ever return
- * true if dma_fence_add_callback, dma_fence_wait or
- * dma_fence_enable_sw_signaling haven't been called before.
+ * true if dma_fence_add_callback(), dma_fence_wait() or
+ * dma_fence_enable_sw_signaling() haven't been called before.
  *
  * It's recommended for seqno fences to call dma_fence_signal when the
  * operation is complete, it makes it possible to prevent issues from
  * wraparound between time of issue and time of use by checking the return
  * value of this function before calling hardware-specific wait instructions.
+ *
+ * See also dma_fence_is_signaled_locked().
  */
 static inline bool
 dma_fence_is_signaled(struct dma_fence *fence)
@@ -342,8 +413,8 @@ dma_fence_is_signaled(struct dma_fence *fence)
 
 /**
  * __dma_fence_is_later - return if f1 is chronologically later than f2
- * @f1:	[in]	the first fence's seqno
- * @f2:	[in]	the second fence's seqno from the same context
+ * @f1: the first fence's seqno
+ * @f2: the second fence's seqno from the same context
  *
  * Returns true if f1 is chronologically later than f2. Both fences must be
  * from the same context, since a seqno is not common across contexts.
@@ -355,8 +426,8 @@ static inline bool __dma_fence_is_later(u32 f1, u32 f2)
 
 /**
  * dma_fence_is_later - return if f1 is chronologically later than f2
- * @f1:	[in]	the first fence from the same context
- * @f2:	[in]	the second fence from the same context
+ * @f1: the first fence from the same context
+ * @f2: the second fence from the same context
  *
  * Returns true if f1 is chronologically later than f2. Both fences must be
  * from the same context, since a seqno is not re-used across contexts.
@@ -372,8 +443,8 @@ static inline bool dma_fence_is_later(struct dma_fence *f1,
 
 /**
  * dma_fence_later - return the chronologically later fence
- * @f1:	[in]	the first fence from the same context
- * @f2:	[in]	the second fence from the same context
+ * @f1:	the first fence from the same context
+ * @f2:	the second fence from the same context
  *
  * Returns NULL if both fences are signaled, otherwise the fence that would be
  * signaled last. Both fences must be from the same context, since a seqno is
@@ -398,7 +469,7 @@ static inline struct dma_fence *dma_fence_later(struct dma_fence *f1,
 
 /**
  * dma_fence_get_status_locked - returns the status upon completion
- * @fence: [in]	the dma_fence to query
+ * @fence: the dma_fence to query
  *
  * Drivers can supply an optional error status condition before they signal
  * the fence (to indicate whether the fence was completed due to an error
@@ -422,8 +493,8 @@ int dma_fence_get_status(struct dma_fence *fence);
 
 /**
  * dma_fence_set_error - flag an error condition on the fence
- * @fence: [in]	the dma_fence
- * @error: [in]	the error to store
+ * @fence: the dma_fence
+ * @error: the error to store
  *
  * Drivers can supply an optional error status condition before they signal
  * the fence, to indicate that the fence was completed due to an error
@@ -449,8 +520,8 @@ signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
 
 /**
  * dma_fence_wait - sleep until the fence gets signaled
- * @fence:	[in]	the fence to wait on
- * @intr:	[in]	if true, do an interruptible wait
+ * @fence: the fence to wait on
+ * @intr: if true, do an interruptible wait
  *
  * This function will return -ERESTARTSYS if interrupted by a signal,
  * or 0 if the fence was signaled. Other error values may be
@@ -459,6 +530,8 @@ signed long dma_fence_wait_any_timeout(struct dma_fence **fences,
  * Performs a synchronous wait on this fence. It is assumed the caller
  * directly or indirectly holds a reference to the fence, otherwise the
  * fence might be freed before return, resulting in undefined behavior.
+ *
+ * See also dma_fence_wait_timeout() and dma_fence_wait_any_timeout().
  */
 static inline signed long dma_fence_wait(struct dma_fence *fence, bool intr)
 {
-- 
2.17.0

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

Re: [Intel-gfx] [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Eric Anholt]

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> +	/**
> +	 * @fill_driver_data:
> +	 *
> +	 * Callback to fill in free-form debug info Returns amount of bytes
> +	 * filled, or negative error on failure.

Maybe this "Returns" should be on a new line?  Or at least a '.' in
between.

Other than that,

Reviewed-by: Eric Anholt <eric@anholt.net>

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 354 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> +	/**
> +	 * @fill_driver_data:
> +	 *
> +	 * Callback to fill in free-form debug info Returns amount of bytes
> +	 * filled, or negative error on failure.

Maybe this "Returns" should be on a new line?  Or at least a '.' in
between.

Other than that,

Reviewed-by: Eric Anholt <eric@anholt.net>

Thanks!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [Intel-gfx] [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Daniel Vetter]

On Mon, Apr 30, 2018 at 10:49:00AM -0700, Eric Anholt wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> > +	/**
> > +	 * @fill_driver_data:
> > +	 *
> > +	 * Callback to fill in free-form debug info Returns amount of bytes
> > +	 * filled, or negative error on failure.
> 
> Maybe this "Returns" should be on a new line?  Or at least a '.' in
> between.

Indeed I've missed this, thanks for spotting it. Done both&applied.

Thanks, Daniel

> 
> Other than that,
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> 
> Thanks!



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

Re: [Intel-gfx] [PATCH 01/17] dma-fence: Some kerneldoc polish for dma-fence.h

[Daniel Vetter]

On Mon, Apr 30, 2018 at 10:49:00AM -0700, Eric Anholt wrote:
> Daniel Vetter <daniel.vetter@ffwll.ch> writes:
> > +	/**
> > +	 * @fill_driver_data:
> > +	 *
> > +	 * Callback to fill in free-form debug info Returns amount of bytes
> > +	 * filled, or negative error on failure.
> 
> Maybe this "Returns" should be on a new line?  Or at least a '.' in
> between.

Indeed I've missed this, thanks for spotting it. Done both&applied.

Thanks, Daniel

> 
> Other than that,
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>
> 
> Thanks!



-- 
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

[PATCH 02/17] dma-fence: remove fill_driver_data callback

[Daniel Vetter]

Noticed while I was typing docs. Entirely unused.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/linux/dma-fence.h | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index 9d6f39bf2111..f9a6848f8558 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -217,16 +217,6 @@ struct dma_fence_ops {
 	 */
 	void (*release)(struct dma_fence *fence);
 
-	/**
-	 * @fill_driver_data:
-	 *
-	 * Callback to fill in free-form debug info Returns amount of bytes
-	 * filled, or negative error on failure.
-	 *
-	 * This callback is optional.
-	 */
-	int (*fill_driver_data)(struct dma_fence *fence, void *data, int size);
-
 	/**
 	 * @fence_value_str:
 	 *
-- 
2.17.0

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

Re: [PATCH 02/17] dma-fence: remove fill_driver_data callback

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 937 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Noticed while I was typing docs. Entirely unused.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  include/linux/dma-fence.h | 10 ----------
>  1 file changed, 10 deletions(-)
>
> diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
> index 9d6f39bf2111..f9a6848f8558 100644
> --- a/include/linux/dma-fence.h
> +++ b/include/linux/dma-fence.h
> @@ -217,16 +217,6 @@ struct dma_fence_ops {
>  	 */
>  	void (*release)(struct dma_fence *fence);
>  
> -	/**
> -	 * @fill_driver_data:
> -	 *
> -	 * Callback to fill in free-form debug info Returns amount of bytes
> -	 * filled, or negative error on failure.
> -	 *
> -	 * This callback is optional.
> -	 */
> -	int (*fill_driver_data)(struct dma_fence *fence, void *data, int size);
> -

There's a reference to this one from timeline_value_str() as well.
Could you fix that up?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH] dma-fence: remove fill_driver_data callback

[Daniel Vetter]

Noticed while I was typing docs. Entirely unused.

v2: Remove reference in @timeline_value_str too. While at it clarify
why timeline_value_str has a fence parameter - we don't have an
explicit timeline structure unfortunately.

Cc: Eric Anholt <eric@anholt.net>
Reviewed-by: Christian König <christian.koenig@amd.com> (v1)
Cc: Christian König <christian.koenig@amd.com> (v1)
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 include/linux/dma-fence.h | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index eb9b05aa5aea..111aefe1c956 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -217,17 +217,6 @@ struct dma_fence_ops {
 	 */
 	void (*release)(struct dma_fence *fence);
 
-	/**
-	 * @fill_driver_data:
-	 *
-	 * Callback to fill in free-form debug info.
-	 *
-	 * Returns amount of bytes filled, or negative error on failure.
-	 *
-	 * This callback is optional.
-	 */
-	int (*fill_driver_data)(struct dma_fence *fence, void *data, int size);
-
 	/**
 	 * @fence_value_str:
 	 *
@@ -242,8 +231,9 @@ struct dma_fence_ops {
 	 * @timeline_value_str:
 	 *
 	 * Fills in the current value of the timeline as a string, like the
-	 * sequence number. This should match what @fill_driver_data prints for
-	 * the most recently signalled fence (assuming no delayed signalling).
+	 * sequence number. Note that the specific fence passed to this function
+	 * should not matter, drivers should only use it to look up the
+	 * corresponding timeline structures.
 	 */
 	void (*timeline_value_str)(struct dma_fence *fence,
 				   char *str, int size);
-- 
2.17.0

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

Re: [PATCH] dma-fence: remove fill_driver_data callback

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 366 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> Noticed while I was typing docs. Entirely unused.
>
> v2: Remove reference in @timeline_value_str too. While at it clarify
> why timeline_value_str has a fence parameter - we don't have an
> explicit timeline structure unfortunately.
>
> Cc: Eric Anholt <eric@anholt.net>

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH 03/17] dma-fence: Make ->enable_signaling optional

[Daniel Vetter]

Many drivers have a trivial implementation for ->enable_signaling.
Let's make it optional by assuming that signalling is already
available when the callback isn't present.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
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
---
 drivers/dma-buf/dma-fence.c | 13 ++++++++++++-
 include/linux/dma-fence.h   |  3 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 4edb9fd3cf47..7b5b40d6b70e 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -181,6 +181,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);
@@ -560,7 +567,7 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	       spinlock_t *lock, u64 context, unsigned seqno)
 {
 	BUG_ON(!lock);
-	BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
+	BUG_ON(!ops || !ops->wait ||
 	       !ops->get_driver_name || !ops->get_timeline_name);
 
 	kref_init(&fence->refcount);
@@ -572,6 +579,10 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	fence->flags = 0UL;
 	fence->error = 0;
 
+	if (!ops->enable_signaling)
+		set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+			&fence->flags);
+
 	trace_dma_fence_init(fence);
 }
 EXPORT_SYMBOL(dma_fence_init);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index f9a6848f8558..c730f569621a 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -166,7 +166,8 @@ struct dma_fence_ops {
 	 * released when the fence is signalled (through e.g. the interrupt
 	 * handler).
 	 *
-	 * This callback is mandatory.
+	 * This callback is optional. If this callback is not present, then the
+	 * driver must always have signaling enabled.
 	 */
 	bool (*enable_signaling)(struct dma_fence *fence);
 
-- 
2.17.0

[PATCH 03/17] dma-fence: Make ->enable_signaling optional

[Daniel Vetter]

Many drivers have a trivial implementation for ->enable_signaling.
Let's make it optional by assuming that signalling is already
available when the callback isn't present.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
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
---
 drivers/dma-buf/dma-fence.c | 13 ++++++++++++-
 include/linux/dma-fence.h   |  3 ++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 4edb9fd3cf47..7b5b40d6b70e 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -181,6 +181,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);
@@ -560,7 +567,7 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	       spinlock_t *lock, u64 context, unsigned seqno)
 {
 	BUG_ON(!lock);
-	BUG_ON(!ops || !ops->wait || !ops->enable_signaling ||
+	BUG_ON(!ops || !ops->wait ||
 	       !ops->get_driver_name || !ops->get_timeline_name);
 
 	kref_init(&fence->refcount);
@@ -572,6 +579,10 @@ dma_fence_init(struct dma_fence *fence, const struct dma_fence_ops *ops,
 	fence->flags = 0UL;
 	fence->error = 0;
 
+	if (!ops->enable_signaling)
+		set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+			&fence->flags);
+
 	trace_dma_fence_init(fence);
 }
 EXPORT_SYMBOL(dma_fence_init);
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index f9a6848f8558..c730f569621a 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -166,7 +166,8 @@ struct dma_fence_ops {
 	 * released when the fence is signalled (through e.g. the interrupt
 	 * handler).
 	 *
-	 * This callback is mandatory.
+	 * This callback is optional. If this callback is not present, then the
+	 * driver must always have signaling enabled.
 	 */
 	bool (*enable_signaling)(struct dma_fence *fence);
 
-- 
2.17.0

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

[PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Daniel Vetter]

When this was introduced in

commit a519435a96597d8cd96123246fea4ae5a6c90b02
Author: Christian König <christian.koenig@amd.com>
Date:   Tue Oct 20 16:34:16 2015 +0200

    dma-buf/fence: add fence_wait_any_timeout function v2

there was a restriction added that this only works if the dma-fence
uses the dma_fence_default_wait hook. Which works for amdgpu, which is
the only caller. Well, until you share some buffers with e.g. i915,
then you get an -EINVAL.

But there's really no reason for this, because all drivers must
support callbacks. The special ->wait hook is only as an optimization;
if the driver needs to create a worker thread for an active callback,
then it can avoid to do that if it knows that there's a process
context available already. So ->wait is just an optimization, just
using the logic in dma_fence_default_wait() should work for all
drivers.

Let's remove this restriction.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
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
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/dma-fence.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7b5b40d6b70e..59049375bd19 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
 	for (i = 0; i < count; ++i) {
 		struct dma_fence *fence = fences[i];
 
-		if (fence->ops->wait != dma_fence_default_wait) {
-			ret = -EINVAL;
-			goto fence_rm_cb;
-		}
-
 		cb[i].task = current;
 		if (dma_fence_add_callback(fence, &cb[i].base,
 					   dma_fence_default_wait_cb)) {
-- 
2.17.0

[PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Daniel Vetter]

When this was introduced in

commit a519435a96597d8cd96123246fea4ae5a6c90b02
Author: Christian König <christian.koenig@amd.com>
Date:   Tue Oct 20 16:34:16 2015 +0200

    dma-buf/fence: add fence_wait_any_timeout function v2

there was a restriction added that this only works if the dma-fence
uses the dma_fence_default_wait hook. Which works for amdgpu, which is
the only caller. Well, until you share some buffers with e.g. i915,
then you get an -EINVAL.

But there's really no reason for this, because all drivers must
support callbacks. The special ->wait hook is only as an optimization;
if the driver needs to create a worker thread for an active callback,
then it can avoid to do that if it knows that there's a process
context available already. So ->wait is just an optimization, just
using the logic in dma_fence_default_wait() should work for all
drivers.

Let's remove this restriction.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
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
Cc: Christian König <christian.koenig@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
---
 drivers/dma-buf/dma-fence.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 7b5b40d6b70e..59049375bd19 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
 	for (i = 0; i < count; ++i) {
 		struct dma_fence *fence = fences[i];
 
-		if (fence->ops->wait != dma_fence_default_wait) {
-			ret = -EINVAL;
-			goto fence_rm_cb;
-		}
-
 		cb[i].task = current;
 		if (dma_fence_add_callback(fence, &cb[i].base,
 					   dma_fence_default_wait_cb)) {
-- 
2.17.0

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

Re: [PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Christian König]

Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> When this was introduced in
>
> commit a519435a96597d8cd96123246fea4ae5a6c90b02
> Author: Christian König <christian.koenig@amd.com>
> Date:   Tue Oct 20 16:34:16 2015 +0200
>
>      dma-buf/fence: add fence_wait_any_timeout function v2
>
> there was a restriction added that this only works if the dma-fence
> uses the dma_fence_default_wait hook. Which works for amdgpu, which is
> the only caller. Well, until you share some buffers with e.g. i915,
> then you get an -EINVAL.
>
> But there's really no reason for this, because all drivers must
> support callbacks. The special ->wait hook is only as an optimization;
> if the driver needs to create a worker thread for an active callback,
> then it can avoid to do that if it knows that there's a process
> context available already. So ->wait is just an optimization, just
> using the logic in dma_fence_default_wait() should work for all
> drivers.
>
> Let's remove this restriction.

Mhm, that was intentional introduced because for radeon that is not only 
an optimization, but mandatory for correct operation.

On the other hand radeon isn't using this function, so it should be fine 
as long as the Intel driver can live with it.

Christian.

>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 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
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/dma-buf/dma-fence.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 7b5b40d6b70e..59049375bd19 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
>   	for (i = 0; i < count; ++i) {
>   		struct dma_fence *fence = fences[i];
>   
> -		if (fence->ops->wait != dma_fence_default_wait) {
> -			ret = -EINVAL;
> -			goto fence_rm_cb;
> -		}
> -
>   		cb[i].task = current;
>   		if (dma_fence_add_callback(fence, &cb[i].base,
>   					   dma_fence_default_wait_cb)) {

Re: [PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Christian König]

Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> When this was introduced in
>
> commit a519435a96597d8cd96123246fea4ae5a6c90b02
> Author: Christian König <christian.koenig@amd.com>
> Date:   Tue Oct 20 16:34:16 2015 +0200
>
>      dma-buf/fence: add fence_wait_any_timeout function v2
>
> there was a restriction added that this only works if the dma-fence
> uses the dma_fence_default_wait hook. Which works for amdgpu, which is
> the only caller. Well, until you share some buffers with e.g. i915,
> then you get an -EINVAL.
>
> But there's really no reason for this, because all drivers must
> support callbacks. The special ->wait hook is only as an optimization;
> if the driver needs to create a worker thread for an active callback,
> then it can avoid to do that if it knows that there's a process
> context available already. So ->wait is just an optimization, just
> using the logic in dma_fence_default_wait() should work for all
> drivers.
>
> Let's remove this restriction.

Mhm, that was intentional introduced because for radeon that is not only 
an optimization, but mandatory for correct operation.

On the other hand radeon isn't using this function, so it should be fine 
as long as the Intel driver can live with it.

Christian.

>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> 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
> Cc: Christian König <christian.koenig@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/dma-buf/dma-fence.c | 5 -----
>   1 file changed, 5 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> index 7b5b40d6b70e..59049375bd19 100644
> --- a/drivers/dma-buf/dma-fence.c
> +++ b/drivers/dma-buf/dma-fence.c
> @@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
>   	for (i = 0; i < count; ++i) {
>   		struct dma_fence *fence = fences[i];
>   
> -		if (fence->ops->wait != dma_fence_default_wait) {
> -			ret = -EINVAL;
> -			goto fence_rm_cb;
> -		}
> -
>   		cb[i].task = current;
>   		if (dma_fence_add_callback(fence, &cb[i].base,
>   					   dma_fence_default_wait_cb)) {

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

Re: [PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Daniel Vetter]

On Sun, Apr 29, 2018 at 09:11:31AM +0200, Christian König wrote:
> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> > When this was introduced in
> > 
> > commit a519435a96597d8cd96123246fea4ae5a6c90b02
> > Author: Christian König <christian.koenig@amd.com>
> > Date:   Tue Oct 20 16:34:16 2015 +0200
> > 
> >      dma-buf/fence: add fence_wait_any_timeout function v2
> > 
> > there was a restriction added that this only works if the dma-fence
> > uses the dma_fence_default_wait hook. Which works for amdgpu, which is
> > the only caller. Well, until you share some buffers with e.g. i915,
> > then you get an -EINVAL.
> > 
> > But there's really no reason for this, because all drivers must
> > support callbacks. The special ->wait hook is only as an optimization;
> > if the driver needs to create a worker thread for an active callback,
> > then it can avoid to do that if it knows that there's a process
> > context available already. So ->wait is just an optimization, just
> > using the logic in dma_fence_default_wait() should work for all
> > drivers.
> > 
> > Let's remove this restriction.
> 
> Mhm, that was intentional introduced because for radeon that is not only an
> optimization, but mandatory for correct operation.
> 
> On the other hand radeon isn't using this function, so it should be fine as
> long as the Intel driver can live with it.

Well dma-buf already requires that dma_fence_add_callback works correctly.
And so do various users of it as soon as you engage in a bit of buffer
sharing. I guess whomever cares about buffer sharing with radeon gets to
fix this (you need to spawn a kthread or whatever in ->enable_signaling
which does the same work as your optimized ->wait callback).

But yeah, I'm definitely not making things work with this series, just a
bit more obvious that there's a problem already.
-Daniel

> 
> Christian.
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > 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
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/dma-buf/dma-fence.c | 5 -----
> >   1 file changed, 5 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 7b5b40d6b70e..59049375bd19 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
> >   	for (i = 0; i < count; ++i) {
> >   		struct dma_fence *fence = fences[i];
> > -		if (fence->ops->wait != dma_fence_default_wait) {
> > -			ret = -EINVAL;
> > -			goto fence_rm_cb;
> > -		}
> > -
> >   		cb[i].task = current;
> >   		if (dma_fence_add_callback(fence, &cb[i].base,
> >   					   dma_fence_default_wait_cb)) {
> 

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

Re: [PATCH 04/17] dma-fence: Allow wait_any_timeout for all fences

[Daniel Vetter]

On Sun, Apr 29, 2018 at 09:11:31AM +0200, Christian König wrote:
> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> > When this was introduced in
> > 
> > commit a519435a96597d8cd96123246fea4ae5a6c90b02
> > Author: Christian König <christian.koenig@amd.com>
> > Date:   Tue Oct 20 16:34:16 2015 +0200
> > 
> >      dma-buf/fence: add fence_wait_any_timeout function v2
> > 
> > there was a restriction added that this only works if the dma-fence
> > uses the dma_fence_default_wait hook. Which works for amdgpu, which is
> > the only caller. Well, until you share some buffers with e.g. i915,
> > then you get an -EINVAL.
> > 
> > But there's really no reason for this, because all drivers must
> > support callbacks. The special ->wait hook is only as an optimization;
> > if the driver needs to create a worker thread for an active callback,
> > then it can avoid to do that if it knows that there's a process
> > context available already. So ->wait is just an optimization, just
> > using the logic in dma_fence_default_wait() should work for all
> > drivers.
> > 
> > Let's remove this restriction.
> 
> Mhm, that was intentional introduced because for radeon that is not only an
> optimization, but mandatory for correct operation.
> 
> On the other hand radeon isn't using this function, so it should be fine as
> long as the Intel driver can live with it.

Well dma-buf already requires that dma_fence_add_callback works correctly.
And so do various users of it as soon as you engage in a bit of buffer
sharing. I guess whomever cares about buffer sharing with radeon gets to
fix this (you need to spawn a kthread or whatever in ->enable_signaling
which does the same work as your optimized ->wait callback).

But yeah, I'm definitely not making things work with this series, just a
bit more obvious that there's a problem already.
-Daniel

> 
> Christian.
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > 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
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > ---
> >   drivers/dma-buf/dma-fence.c | 5 -----
> >   1 file changed, 5 deletions(-)
> > 
> > diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
> > index 7b5b40d6b70e..59049375bd19 100644
> > --- a/drivers/dma-buf/dma-fence.c
> > +++ b/drivers/dma-buf/dma-fence.c
> > @@ -503,11 +503,6 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count,
> >   	for (i = 0; i < count; ++i) {
> >   		struct dma_fence *fence = fences[i];
> > -		if (fence->ops->wait != dma_fence_default_wait) {
> > -			ret = -EINVAL;
> > -			goto fence_rm_cb;
> > -		}
> > -
> >   		cb[i].task = current;
> >   		if (dma_fence_add_callback(fence, &cb[i].base,
> >   					   dma_fence_default_wait_cb)) {
> 

-- 
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

[PATCH 05/17] dma-fence: Make ->wait callback optional

[Daniel Vetter]

Almost everyone uses dma_fence_default_wait.

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
---
 drivers/dma-buf/dma-fence-array.c |  1 -
 drivers/dma-buf/dma-fence.c       |  5 ++++-
 drivers/dma-buf/sw_sync.c         |  1 -
 include/linux/dma-fence.h         | 13 ++++++++-----
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c
index dd1edfb27b61..a8c254497251 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,6 @@ const struct dma_fence_ops dma_fence_array_ops = {
 	.get_timeline_name = dma_fence_array_get_timeline_name,
 	.enable_signaling = dma_fence_array_enable_signaling,
 	.signaled = dma_fence_array_signaled,
-	.wait = dma_fence_default_wait,
 	.release = dma_fence_array_release,
 };
 EXPORT_SYMBOL(dma_fence_array_ops);
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 59049375bd19..30fcbe415ff4 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -158,7 +158,10 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
 		return -EINVAL;
 
 	trace_dma_fence_wait_start(fence);
-	ret = fence->ops->wait(fence, intr, timeout);
+	if (fence->ops->wait)
+		ret = fence->ops->wait(fence, intr, timeout);
+	else
+		ret = dma_fence_default_wait(fence, intr, timeout);
 	trace_dma_fence_wait_end(fence);
 	return ret;
 }
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 3d78ca89a605..53c1d6d36a64 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -188,7 +188,6 @@ static const struct dma_fence_ops timeline_fence_ops = {
 	.get_timeline_name = timeline_fence_get_timeline_name,
 	.enable_signaling = timeline_fence_enable_signaling,
 	.signaled = timeline_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = timeline_fence_release,
 	.fence_value_str = timeline_fence_value_str,
 	.timeline_value_str = timeline_fence_timeline_value_str,
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index c730f569621a..d05496ff0d10 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -191,11 +191,14 @@ struct dma_fence_ops {
 	/**
 	 * @wait:
 	 *
-	 * Custom wait implementation, or dma_fence_default_wait.
+	 * Custom wait implementation, defaults to dma_fence_default_wait() if
+	 * not set.
 	 *
-	 * Must not be NULL, set to dma_fence_default_wait for default implementation.
-	 * the dma_fence_default_wait implementation should work for any fence, as long
-	 * as enable_signaling works correctly.
+	 * The dma_fence_default_wait implementation should work for any fence, as long
+	 * as @enable_signaling works correctly. This hook allows drivers to
+	 * have an optimized version for the case where a process context is
+	 * already available, e.g. if @enable_signaling for the general case
+	 * needs to set up a worker thread.
 	 *
 	 * Must return -ERESTARTSYS if the wait is intr = true and the wait was
 	 * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
@@ -203,7 +206,7 @@ struct dma_fence_ops {
 	 * which should be treated as if the fence is signaled. For example a hardware
 	 * lockup could be reported like that.
 	 *
-	 * This callback is mandatory.
+	 * This callback is optional.
 	 */
 	signed long (*wait)(struct dma_fence *fence,
 			    bool intr, signed long timeout);
-- 
2.17.0

[PATCH 05/17] dma-fence: Make ->wait callback optional

[Daniel Vetter]

Almost everyone uses dma_fence_default_wait.

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
---
 drivers/dma-buf/dma-fence-array.c |  1 -
 drivers/dma-buf/dma-fence.c       |  5 ++++-
 drivers/dma-buf/sw_sync.c         |  1 -
 include/linux/dma-fence.h         | 13 ++++++++-----
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c
index dd1edfb27b61..a8c254497251 100644
--- a/drivers/dma-buf/dma-fence-array.c
+++ b/drivers/dma-buf/dma-fence-array.c
@@ -104,7 +104,6 @@ const struct dma_fence_ops dma_fence_array_ops = {
 	.get_timeline_name = dma_fence_array_get_timeline_name,
 	.enable_signaling = dma_fence_array_enable_signaling,
 	.signaled = dma_fence_array_signaled,
-	.wait = dma_fence_default_wait,
 	.release = dma_fence_array_release,
 };
 EXPORT_SYMBOL(dma_fence_array_ops);
diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 59049375bd19..30fcbe415ff4 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -158,7 +158,10 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout)
 		return -EINVAL;
 
 	trace_dma_fence_wait_start(fence);
-	ret = fence->ops->wait(fence, intr, timeout);
+	if (fence->ops->wait)
+		ret = fence->ops->wait(fence, intr, timeout);
+	else
+		ret = dma_fence_default_wait(fence, intr, timeout);
 	trace_dma_fence_wait_end(fence);
 	return ret;
 }
diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 3d78ca89a605..53c1d6d36a64 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -188,7 +188,6 @@ static const struct dma_fence_ops timeline_fence_ops = {
 	.get_timeline_name = timeline_fence_get_timeline_name,
 	.enable_signaling = timeline_fence_enable_signaling,
 	.signaled = timeline_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = timeline_fence_release,
 	.fence_value_str = timeline_fence_value_str,
 	.timeline_value_str = timeline_fence_timeline_value_str,
diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h
index c730f569621a..d05496ff0d10 100644
--- a/include/linux/dma-fence.h
+++ b/include/linux/dma-fence.h
@@ -191,11 +191,14 @@ struct dma_fence_ops {
 	/**
 	 * @wait:
 	 *
-	 * Custom wait implementation, or dma_fence_default_wait.
+	 * Custom wait implementation, defaults to dma_fence_default_wait() if
+	 * not set.
 	 *
-	 * Must not be NULL, set to dma_fence_default_wait for default implementation.
-	 * the dma_fence_default_wait implementation should work for any fence, as long
-	 * as enable_signaling works correctly.
+	 * The dma_fence_default_wait implementation should work for any fence, as long
+	 * as @enable_signaling works correctly. This hook allows drivers to
+	 * have an optimized version for the case where a process context is
+	 * already available, e.g. if @enable_signaling for the general case
+	 * needs to set up a worker thread.
 	 *
 	 * Must return -ERESTARTSYS if the wait is intr = true and the wait was
 	 * interrupted, and remaining jiffies if fence has signaled, or 0 if wait
@@ -203,7 +206,7 @@ struct dma_fence_ops {
 	 * which should be treated as if the fence is signaled. For example a hardware
 	 * lockup could be reported like that.
 	 *
-	 * This callback is mandatory.
+	 * This callback is optional.
 	 */
 	signed long (*wait)(struct dma_fence *fence,
 			    bool intr, signed long timeout);
-- 
2.17.0

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

[PATCH 06/17] drm/amdgpu: Remove unecessary dma_fence_ops

[Daniel Vetter]

dma_fence_default_wait is the default now.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Monk Liu <Monk.Liu@amd.com>
Cc: pding <Pixel.Ding@amd.com>
Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Cc: Evan Quan <evan.quan@amd.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Kees Cook <keescook@chromium.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c | 2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c        | 1 -
 2 files changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
index 2c14025e5e76..574c1181ae9a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
@@ -173,7 +173,5 @@ static const struct dma_fence_ops amdkfd_fence_ops = {
 	.get_driver_name = amdkfd_fence_get_driver_name,
 	.get_timeline_name = amdkfd_fence_get_timeline_name,
 	.enable_signaling = amdkfd_fence_enable_signaling,
-	.signaled = NULL,
-	.wait = dma_fence_default_wait,
 	.release = amdkfd_fence_release,
 };
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 97449e06a242..9dbbd69dd2b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -645,7 +645,6 @@ static const struct dma_fence_ops amdgpu_fence_ops = {
 	.get_driver_name = amdgpu_fence_get_driver_name,
 	.get_timeline_name = amdgpu_fence_get_timeline_name,
 	.enable_signaling = amdgpu_fence_enable_signaling,
-	.wait = dma_fence_default_wait,
 	.release = amdgpu_fence_release,
 };
 
-- 
2.17.0

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

Re: [PATCH 06/17] drm/amdgpu: Remove unecessary dma_fence_ops

[Christian König]

Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> dma_fence_default_wait is the default now.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Monk Liu <Monk.Liu@amd.com>
> Cc: pding <Pixel.Ding@amd.com>
> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Cc: Evan Quan <evan.quan@amd.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Kees Cook <keescook@chromium.org>

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

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c | 2 --
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c        | 1 -
>   2 files changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
> index 2c14025e5e76..574c1181ae9a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c
> @@ -173,7 +173,5 @@ static const struct dma_fence_ops amdkfd_fence_ops = {
>   	.get_driver_name = amdkfd_fence_get_driver_name,
>   	.get_timeline_name = amdkfd_fence_get_timeline_name,
>   	.enable_signaling = amdkfd_fence_enable_signaling,
> -	.signaled = NULL,
> -	.wait = dma_fence_default_wait,
>   	.release = amdkfd_fence_release,
>   };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> index 97449e06a242..9dbbd69dd2b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
> @@ -645,7 +645,6 @@ static const struct dma_fence_ops amdgpu_fence_ops = {
>   	.get_driver_name = amdgpu_fence_get_driver_name,
>   	.get_timeline_name = amdgpu_fence_get_timeline_name,
>   	.enable_signaling = amdgpu_fence_enable_signaling,
> -	.wait = dma_fence_default_wait,
>   	.release = amdgpu_fence_release,
>   };
>   

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

[PATCH 07/17] drm: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Gustavo Padovan <gustavo@padovan.org>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Sean Paul <seanpaul@chromium.org>
Cc: David Airlie <airlied@linux.ie>
---
 drivers/gpu/drm/drm_crtc.c              |  7 -------
 drivers/gpu/drm/drm_syncobj.c           |  1 -
 drivers/gpu/drm/scheduler/sched_fence.c | 11 -----------
 3 files changed, 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a231dd5dce16..e4d3285f4191 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -225,16 +225,9 @@ static const char *drm_crtc_fence_get_timeline_name(struct dma_fence *fence)
 	return crtc->timeline_name;
 }
 
-static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static const struct dma_fence_ops drm_crtc_fence_ops = {
 	.get_driver_name = drm_crtc_fence_get_driver_name,
 	.get_timeline_name = drm_crtc_fence_get_timeline_name,
-	.enable_signaling = drm_crtc_fence_enable_signaling,
-	.wait = dma_fence_default_wait,
 };
 
 struct dma_fence *drm_crtc_create_fence(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index d4f4ce484529..adb3cb27d31e 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -207,7 +207,6 @@ static const struct dma_fence_ops drm_syncobj_null_fence_ops = {
 	.get_driver_name = drm_syncobj_null_fence_get_name,
 	.get_timeline_name = drm_syncobj_null_fence_get_name,
 	.enable_signaling = drm_syncobj_null_fence_enable_signaling,
-	.wait = dma_fence_default_wait,
 	.release = NULL,
 };
 
diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
index 69aab086b913..4843289cc8f0 100644
--- a/drivers/gpu/drm/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/scheduler/sched_fence.c
@@ -81,11 +81,6 @@ static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
 	return (const char *)fence->sched->name;
 }
 
-static bool drm_sched_fence_enable_signaling(struct dma_fence *f)
-{
-	return true;
-}
-
 /**
  * amd_sched_fence_free - free up the fence memory
  *
@@ -134,18 +129,12 @@ static void drm_sched_fence_release_finished(struct dma_fence *f)
 const struct dma_fence_ops drm_sched_fence_ops_scheduled = {
 	.get_driver_name = drm_sched_fence_get_driver_name,
 	.get_timeline_name = drm_sched_fence_get_timeline_name,
-	.enable_signaling = drm_sched_fence_enable_signaling,
-	.signaled = NULL,
-	.wait = dma_fence_default_wait,
 	.release = drm_sched_fence_release_scheduled,
 };
 
 const struct dma_fence_ops drm_sched_fence_ops_finished = {
 	.get_driver_name = drm_sched_fence_get_driver_name,
 	.get_timeline_name = drm_sched_fence_get_timeline_name,
-	.enable_signaling = drm_sched_fence_enable_signaling,
-	.signaled = NULL,
-	.wait = dma_fence_default_wait,
 	.release = drm_sched_fence_release_finished,
 };
 
-- 
2.17.0

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

Re: [PATCH 07/17] drm: Remove unecessary dma_fence_ops

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 251 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH 08/17] drm/etnaviv: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Russell King <linux+etnaviv@armlinux.org.uk>
Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
Cc: etnaviv@lists.freedesktop.org
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 8a88799bf79b..b78d9f49aa23 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1038,11 +1038,6 @@ static const char *etnaviv_fence_get_timeline_name(struct dma_fence *fence)
 	return dev_name(f->gpu->dev);
 }
 
-static bool etnaviv_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static bool etnaviv_fence_signaled(struct dma_fence *fence)
 {
 	struct etnaviv_fence *f = to_etnaviv_fence(fence);
@@ -1060,9 +1055,7 @@ static void etnaviv_fence_release(struct dma_fence *fence)
 static const struct dma_fence_ops etnaviv_fence_ops = {
 	.get_driver_name = etnaviv_fence_get_driver_name,
 	.get_timeline_name = etnaviv_fence_get_timeline_name,
-	.enable_signaling = etnaviv_fence_enable_signaling,
 	.signaled = etnaviv_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = etnaviv_fence_release,
 };
 
-- 
2.17.0

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

Re: [PATCH 08/17] drm/etnaviv: Remove unecessary dma_fence_ops

[Lucas Stach]

Am Freitag, den 27.04.2018, 08:17 +0200 schrieb Daniel Vetter:
> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
> 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Cc: Russell King <linux+etnaviv@armlinux.org.uk>
> > Cc: Christian Gmeiner <christian.gmeiner@gmail.com>
> Cc: etnaviv@lists.freedesktop.org

I guess you are going to take the whole series through drm-misc, right?
If so:

Acked-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 8a88799bf79b..b78d9f49aa23 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1038,11 +1038,6 @@ static const char *etnaviv_fence_get_timeline_name(struct dma_fence *fence)
> >  	return dev_name(f->gpu->dev);
>  }
>  
> -static bool etnaviv_fence_enable_signaling(struct dma_fence *fence)
> -{
> > -	return true;
> -}
> -
>  static bool etnaviv_fence_signaled(struct dma_fence *fence)
>  {
> >  	struct etnaviv_fence *f = to_etnaviv_fence(fence);
> @@ -1060,9 +1055,7 @@ static void etnaviv_fence_release(struct dma_fence *fence)
>  static const struct dma_fence_ops etnaviv_fence_ops = {
> >  	.get_driver_name = etnaviv_fence_get_driver_name,
> >  	.get_timeline_name = etnaviv_fence_get_timeline_name,
> > -	.enable_signaling = etnaviv_fence_enable_signaling,
> >  	.signaled = etnaviv_fence_signaled,
> > -	.wait = dma_fence_default_wait,
> >  	.release = etnaviv_fence_release,
>  };
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/17] drm/i915: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

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 | 7 -------
 2 files changed, 14 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..0c02446f613d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
+++ b/drivers/gpu/drm/i915/selftests/i915_sw_fence.c
@@ -611,16 +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,
 };
 
-- 
2.17.0

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

[PATCH 10/17] drm/msm: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

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 | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_fence.c b/drivers/gpu/drm/msm/msm_fence.c
index 349c12f670eb..6076f55a603c 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,9 +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,
 };
 
-- 
2.17.0

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

[PATCH 11/17] drm/nouveau: Remove unecessary dma_fence_ops

[Daniel Vetter]

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 503fa94dc06d..444bbde65344 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fence.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fence.c
@@ -527,6 +527,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.17.0

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

[PATCH 12/17] drm/qxl: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/qxl/qxl_release.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 7cb214577275..e37f0097f744 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -50,12 +50,6 @@ static const char *qxl_get_timeline_name(struct dma_fence *fence)
 	return "release";
 }
 
-static bool qxl_nop_signaling(struct dma_fence *fence)
-{
-	/* fences are always automatically signaled, so just pretend we did this.. */
-	return true;
-}
-
 static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 			   signed long timeout)
 {
@@ -119,7 +113,6 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 static const struct dma_fence_ops qxl_fence_ops = {
 	.get_driver_name = qxl_get_driver_name,
 	.get_timeline_name = qxl_get_timeline_name,
-	.enable_signaling = qxl_nop_signaling,
 	.wait = qxl_fence_wait,
 };
 
-- 
2.17.0

Re: [PATCH 12/17] drm/qxl: Remove unecessary dma_fence_ops

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 289 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

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

Drop the mention of dma_fence_default_wait, since this one doesn't use
that?  Other than that,

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH] drm/qxl: Remove unecessary dma_fence_ops

[Daniel Vetter]

The trivial enable_signaling implementation matches the default code.

v2: Fix up commit message to match patch better (Eric).

Cc: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/qxl/qxl_release.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 7cb214577275..e37f0097f744 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -50,12 +50,6 @@ static const char *qxl_get_timeline_name(struct dma_fence *fence)
 	return "release";
 }
 
-static bool qxl_nop_signaling(struct dma_fence *fence)
-{
-	/* fences are always automatically signaled, so just pretend we did this.. */
-	return true;
-}
-
 static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 			   signed long timeout)
 {
@@ -119,7 +113,6 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 static const struct dma_fence_ops qxl_fence_ops = {
 	.get_driver_name = qxl_get_driver_name,
 	.get_timeline_name = qxl_get_timeline_name,
-	.enable_signaling = qxl_nop_signaling,
 	.wait = qxl_fence_wait,
 };
 
-- 
2.17.0

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

[PATCH] drm/qxl: Remove unecessary dma_fence_ops

[Daniel Vetter]

The trivial enable_signaling implementation matches the default code.

v2: Fix up commit message to match patch better (Eric).

Cc: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/qxl/qxl_release.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 7cb214577275..e37f0097f744 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -50,12 +50,6 @@ static const char *qxl_get_timeline_name(struct dma_fence *fence)
 	return "release";
 }
 
-static bool qxl_nop_signaling(struct dma_fence *fence)
-{
-	/* fences are always automatically signaled, so just pretend we did this.. */
-	return true;
-}
-
 static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 			   signed long timeout)
 {
@@ -119,7 +113,6 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 static const struct dma_fence_ops qxl_fence_ops = {
 	.get_driver_name = qxl_get_driver_name,
 	.get_timeline_name = qxl_get_timeline_name,
-	.enable_signaling = qxl_nop_signaling,
 	.wait = qxl_fence_wait,
 };
 
-- 
2.17.0

[PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

[Daniel Vetter]

It's a copy of dma_fence_default_wait, written slightly differently.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_fence.c | 63 ---------------------------
 1 file changed, 63 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
index e86f2bd38410..32690a525bfc 100644
--- a/drivers/gpu/drm/radeon/radeon_fence.c
+++ b/drivers/gpu/drm/radeon/radeon_fence.c
@@ -1051,72 +1051,9 @@ static const char *radeon_fence_get_timeline_name(struct dma_fence *f)
 	}
 }
 
-static inline bool radeon_test_signaled(struct radeon_fence *fence)
-{
-	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);
-}
-
-struct radeon_wait_cb {
-	struct dma_fence_cb base;
-	struct task_struct *task;
-};
-
-static void
-radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
-{
-	struct radeon_wait_cb *wait =
-		container_of(cb, struct radeon_wait_cb, base);
-
-	wake_up_process(wait->task);
-}
-
-static signed long radeon_fence_default_wait(struct dma_fence *f, bool intr,
-					     signed long t)
-{
-	struct radeon_fence *fence = to_radeon_fence(f);
-	struct radeon_device *rdev = fence->rdev;
-	struct radeon_wait_cb cb;
-
-	cb.task = current;
-
-	if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))
-		return t;
-
-	while (t > 0) {
-		if (intr)
-			set_current_state(TASK_INTERRUPTIBLE);
-		else
-			set_current_state(TASK_UNINTERRUPTIBLE);
-
-		/*
-		 * radeon_test_signaled must be called after
-		 * set_current_state to prevent a race with wake_up_process
-		 */
-		if (radeon_test_signaled(fence))
-			break;
-
-		if (rdev->needs_reset) {
-			t = -EDEADLK;
-			break;
-		}
-
-		t = schedule_timeout(t);
-
-		if (t > 0 && intr && signal_pending(current))
-			t = -ERESTARTSYS;
-	}
-
-	__set_current_state(TASK_RUNNING);
-	dma_fence_remove_callback(f, &cb.base);
-
-	return t;
-}
-
 const struct dma_fence_ops radeon_fence_ops = {
 	.get_driver_name = radeon_fence_get_driver_name,
 	.get_timeline_name = radeon_fence_get_timeline_name,
 	.enable_signaling = radeon_fence_enable_signaling,
 	.signaled = radeon_fence_is_signaled,
-	.wait = radeon_fence_default_wait,
-	.release = NULL,
 };
-- 
2.17.0

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

Re: [PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

[Christian König]

NAK, there is a subtitle but major difference:

> -		if (rdev->needs_reset) {
> -			t = -EDEADLK;
> -			break;
> -		}

Without that the whole radeon GPU reset code breaks.

Regards,
Christian.


Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> It's a copy of dma_fence_default_wait, written slightly differently.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> ---
>   drivers/gpu/drm/radeon/radeon_fence.c | 63 ---------------------------
>   1 file changed, 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> index e86f2bd38410..32690a525bfc 100644
> --- a/drivers/gpu/drm/radeon/radeon_fence.c
> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> @@ -1051,72 +1051,9 @@ static const char *radeon_fence_get_timeline_name(struct dma_fence *f)
>   	}
>   }
>   
> -static inline bool radeon_test_signaled(struct radeon_fence *fence)
> -{
> -	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);
> -}
> -
> -struct radeon_wait_cb {
> -	struct dma_fence_cb base;
> -	struct task_struct *task;
> -};
> -
> -static void
> -radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> -{
> -	struct radeon_wait_cb *wait =
> -		container_of(cb, struct radeon_wait_cb, base);
> -
> -	wake_up_process(wait->task);
> -}
> -
> -static signed long radeon_fence_default_wait(struct dma_fence *f, bool intr,
> -					     signed long t)
> -{
> -	struct radeon_fence *fence = to_radeon_fence(f);
> -	struct radeon_device *rdev = fence->rdev;
> -	struct radeon_wait_cb cb;
> -
> -	cb.task = current;
> -
> -	if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))
> -		return t;
> -
> -	while (t > 0) {
> -		if (intr)
> -			set_current_state(TASK_INTERRUPTIBLE);
> -		else
> -			set_current_state(TASK_UNINTERRUPTIBLE);
> -
> -		/*
> -		 * radeon_test_signaled must be called after
> -		 * set_current_state to prevent a race with wake_up_process
> -		 */
> -		if (radeon_test_signaled(fence))
> -			break;
> -
> -		if (rdev->needs_reset) {
> -			t = -EDEADLK;
> -			break;
> -		}
> -
> -		t = schedule_timeout(t);
> -
> -		if (t > 0 && intr && signal_pending(current))
> -			t = -ERESTARTSYS;
> -	}
> -
> -	__set_current_state(TASK_RUNNING);
> -	dma_fence_remove_callback(f, &cb.base);
> -
> -	return t;
> -}
> -
>   const struct dma_fence_ops radeon_fence_ops = {
>   	.get_driver_name = radeon_fence_get_driver_name,
>   	.get_timeline_name = radeon_fence_get_timeline_name,
>   	.enable_signaling = radeon_fence_enable_signaling,
>   	.signaled = radeon_fence_is_signaled,
> -	.wait = radeon_fence_default_wait,
> -	.release = NULL,
>   };

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

Re: [PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

[Daniel Vetter]

On Sun, Apr 29, 2018 at 09:08:31AM +0200, Christian König wrote:
> NAK, there is a subtitle but major difference:
> 
> > -		if (rdev->needs_reset) {
> > -			t = -EDEADLK;
> > -			break;
> > -		}
> 
> Without that the whole radeon GPU reset code breaks.

Oops I've missed that. How does this work when you register a callback
using ->enable_signaling and then block on it? Everything just dies?

We have lots of users of that for buffer/fence sharing. A really ugly, but
probably working fix for this would be a kthread worker that just looks
for ->needs_reset and force-completes all fences with
dma_fence_set_error(-EIO), which is kinda what's supposed to happen here
anyway.
-Daniel

> 
> Regards,
> Christian.
> 
> 
> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> > It's a copy of dma_fence_default_wait, written slightly differently.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >   drivers/gpu/drm/radeon/radeon_fence.c | 63 ---------------------------
> >   1 file changed, 63 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
> > index e86f2bd38410..32690a525bfc 100644
> > --- a/drivers/gpu/drm/radeon/radeon_fence.c
> > +++ b/drivers/gpu/drm/radeon/radeon_fence.c
> > @@ -1051,72 +1051,9 @@ static const char *radeon_fence_get_timeline_name(struct dma_fence *f)
> >   	}
> >   }
> > -static inline bool radeon_test_signaled(struct radeon_fence *fence)
> > -{
> > -	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);
> > -}
> > -
> > -struct radeon_wait_cb {
> > -	struct dma_fence_cb base;
> > -	struct task_struct *task;
> > -};
> > -
> > -static void
> > -radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
> > -{
> > -	struct radeon_wait_cb *wait =
> > -		container_of(cb, struct radeon_wait_cb, base);
> > -
> > -	wake_up_process(wait->task);
> > -}
> > -
> > -static signed long radeon_fence_default_wait(struct dma_fence *f, bool intr,
> > -					     signed long t)
> > -{
> > -	struct radeon_fence *fence = to_radeon_fence(f);
> > -	struct radeon_device *rdev = fence->rdev;
> > -	struct radeon_wait_cb cb;
> > -
> > -	cb.task = current;
> > -
> > -	if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))
> > -		return t;
> > -
> > -	while (t > 0) {
> > -		if (intr)
> > -			set_current_state(TASK_INTERRUPTIBLE);
> > -		else
> > -			set_current_state(TASK_UNINTERRUPTIBLE);
> > -
> > -		/*
> > -		 * radeon_test_signaled must be called after
> > -		 * set_current_state to prevent a race with wake_up_process
> > -		 */
> > -		if (radeon_test_signaled(fence))
> > -			break;
> > -
> > -		if (rdev->needs_reset) {
> > -			t = -EDEADLK;
> > -			break;
> > -		}
> > -
> > -		t = schedule_timeout(t);
> > -
> > -		if (t > 0 && intr && signal_pending(current))
> > -			t = -ERESTARTSYS;
> > -	}
> > -
> > -	__set_current_state(TASK_RUNNING);
> > -	dma_fence_remove_callback(f, &cb.base);
> > -
> > -	return t;
> > -}
> > -
> >   const struct dma_fence_ops radeon_fence_ops = {
> >   	.get_driver_name = radeon_fence_get_driver_name,
> >   	.get_timeline_name = radeon_fence_get_timeline_name,
> >   	.enable_signaling = radeon_fence_enable_signaling,
> >   	.signaled = radeon_fence_is_signaled,
> > -	.wait = radeon_fence_default_wait,
> > -	.release = NULL,
> >   };
> 

-- 
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

Re: [PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

[Christian König]

Am 30.04.2018 um 17:38 schrieb Daniel Vetter:
> On Sun, Apr 29, 2018 at 09:08:31AM +0200, Christian König wrote:
>> NAK, there is a subtitle but major difference:
>>
>>> -		if (rdev->needs_reset) {
>>> -			t = -EDEADLK;
>>> -			break;
>>> -		}
>> Without that the whole radeon GPU reset code breaks.
> Oops I've missed that. How does this work when you register a callback
> using ->enable_signaling and then block on it? Everything just dies?

The short answer is we simply avoid using enable_signaling() from inside 
driver IOCTLs.

> We have lots of users of that for buffer/fence sharing. A really ugly, but
> probably working fix for this would be a kthread worker that just looks
> for ->needs_reset and force-completes all fences with
> dma_fence_set_error(-EIO), which is kinda what's supposed to happen here
> anyway.

That actually won't help. Radeon does this dance to return an error from 
dma_fence_wait() when the GPU needs a reset.

This way all IOCTLs should return to userspace with -EAGAIN and when 
they are restarted we block for the running GPU reset to finish.

I was against this approach, but it works as long as radeon only has to 
deal with it's own fences.

Christian.

> -Daniel
>
>> Regards,
>> Christian.
>>
>>
>> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
>>> It's a copy of dma_fence_default_wait, written slightly differently.
>>>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>> Cc: "Christian König" <christian.koenig@amd.com>
>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
>>> Cc: amd-gfx@lists.freedesktop.org
>>> ---
>>>    drivers/gpu/drm/radeon/radeon_fence.c | 63 ---------------------------
>>>    1 file changed, 63 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c b/drivers/gpu/drm/radeon/radeon_fence.c
>>> index e86f2bd38410..32690a525bfc 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_fence.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
>>> @@ -1051,72 +1051,9 @@ static const char *radeon_fence_get_timeline_name(struct dma_fence *f)
>>>    	}
>>>    }
>>> -static inline bool radeon_test_signaled(struct radeon_fence *fence)
>>> -{
>>> -	return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->base.flags);
>>> -}
>>> -
>>> -struct radeon_wait_cb {
>>> -	struct dma_fence_cb base;
>>> -	struct task_struct *task;
>>> -};
>>> -
>>> -static void
>>> -radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
>>> -{
>>> -	struct radeon_wait_cb *wait =
>>> -		container_of(cb, struct radeon_wait_cb, base);
>>> -
>>> -	wake_up_process(wait->task);
>>> -}
>>> -
>>> -static signed long radeon_fence_default_wait(struct dma_fence *f, bool intr,
>>> -					     signed long t)
>>> -{
>>> -	struct radeon_fence *fence = to_radeon_fence(f);
>>> -	struct radeon_device *rdev = fence->rdev;
>>> -	struct radeon_wait_cb cb;
>>> -
>>> -	cb.task = current;
>>> -
>>> -	if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))
>>> -		return t;
>>> -
>>> -	while (t > 0) {
>>> -		if (intr)
>>> -			set_current_state(TASK_INTERRUPTIBLE);
>>> -		else
>>> -			set_current_state(TASK_UNINTERRUPTIBLE);
>>> -
>>> -		/*
>>> -		 * radeon_test_signaled must be called after
>>> -		 * set_current_state to prevent a race with wake_up_process
>>> -		 */
>>> -		if (radeon_test_signaled(fence))
>>> -			break;
>>> -
>>> -		if (rdev->needs_reset) {
>>> -			t = -EDEADLK;
>>> -			break;
>>> -		}
>>> -
>>> -		t = schedule_timeout(t);
>>> -
>>> -		if (t > 0 && intr && signal_pending(current))
>>> -			t = -ERESTARTSYS;
>>> -	}
>>> -
>>> -	__set_current_state(TASK_RUNNING);
>>> -	dma_fence_remove_callback(f, &cb.base);
>>> -
>>> -	return t;
>>> -}
>>> -
>>>    const struct dma_fence_ops radeon_fence_ops = {
>>>    	.get_driver_name = radeon_fence_get_driver_name,
>>>    	.get_timeline_name = radeon_fence_get_timeline_name,
>>>    	.enable_signaling = radeon_fence_enable_signaling,
>>>    	.signaled = radeon_fence_is_signaled,
>>> -	.wait = radeon_fence_default_wait,
>>> -	.release = NULL,
>>>    };

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

Re: [PATCH 13/17] drm/radeon: Remove custom dma_fence_ops->wait implementation

[Daniel Vetter]

On Mon, Apr 30, 2018 at 8:26 PM, Christian König
<ckoenig.leichtzumerken@gmail.com> wrote:
> Am 30.04.2018 um 17:38 schrieb Daniel Vetter:
>>
>> On Sun, Apr 29, 2018 at 09:08:31AM +0200, Christian König wrote:
>>>
>>> NAK, there is a subtitle but major difference:
>>>
>>>> -               if (rdev->needs_reset) {
>>>> -                       t = -EDEADLK;
>>>> -                       break;
>>>> -               }
>>>
>>> Without that the whole radeon GPU reset code breaks.
>>
>> Oops I've missed that. How does this work when you register a callback
>> using ->enable_signaling and then block on it? Everything just dies?
>
>
> The short answer is we simply avoid using enable_signaling() from inside
> driver IOCTLs.
>
>> We have lots of users of that for buffer/fence sharing. A really ugly, but
>> probably working fix for this would be a kthread worker that just looks
>> for ->needs_reset and force-completes all fences with
>> dma_fence_set_error(-EIO), which is kinda what's supposed to happen here
>> anyway.
>
>
> That actually won't help. Radeon does this dance to return an error from
> dma_fence_wait() when the GPU needs a reset.
>
> This way all IOCTLs should return to userspace with -EAGAIN and when they
> are restarted we block for the running GPU reset to finish.
>
> I was against this approach, but it works as long as radeon only has to deal
> with it's own fences.

Yeah, as soon as you mix in a 2nd driver it goes boom, since you can
easily construct loops (even if they go through ->enable_signaling and
maybe just an irq handler to fire off something somewhere else).
Currently we're really bad a detecting these loops (aka there's
nothing), but I hope that the cross-release lockdep stuff gets enabled
soon. Then we could annotate dma_fences and lockdep should complain
about a lot of these issues.

Alas, problem exists already, but I'm not going to attempt to fix it.

Anyway, I'll drop this patch here.
-Daniel

>
> Christian.
>
>
>> -Daniel
>>
>>> Regards,
>>> Christian.
>>>
>>>
>>> Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
>>>>
>>>> It's a copy of dma_fence_default_wait, written slightly differently.
>>>>
>>>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Cc: "Christian König" <christian.koenig@amd.com>
>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
>>>> Cc: amd-gfx@lists.freedesktop.org
>>>> ---
>>>>    drivers/gpu/drm/radeon/radeon_fence.c | 63
>>>> ---------------------------
>>>>    1 file changed, 63 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/radeon/radeon_fence.c
>>>> b/drivers/gpu/drm/radeon/radeon_fence.c
>>>> index e86f2bd38410..32690a525bfc 100644
>>>> --- a/drivers/gpu/drm/radeon/radeon_fence.c
>>>> +++ b/drivers/gpu/drm/radeon/radeon_fence.c
>>>> @@ -1051,72 +1051,9 @@ static const char
>>>> *radeon_fence_get_timeline_name(struct dma_fence *f)
>>>>         }
>>>>    }
>>>> -static inline bool radeon_test_signaled(struct radeon_fence *fence)
>>>> -{
>>>> -       return test_bit(DMA_FENCE_FLAG_SIGNALED_BIT,
>>>> &fence->base.flags);
>>>> -}
>>>> -
>>>> -struct radeon_wait_cb {
>>>> -       struct dma_fence_cb base;
>>>> -       struct task_struct *task;
>>>> -};
>>>> -
>>>> -static void
>>>> -radeon_fence_wait_cb(struct dma_fence *fence, struct dma_fence_cb *cb)
>>>> -{
>>>> -       struct radeon_wait_cb *wait =
>>>> -               container_of(cb, struct radeon_wait_cb, base);
>>>> -
>>>> -       wake_up_process(wait->task);
>>>> -}
>>>> -
>>>> -static signed long radeon_fence_default_wait(struct dma_fence *f, bool
>>>> intr,
>>>> -                                            signed long t)
>>>> -{
>>>> -       struct radeon_fence *fence = to_radeon_fence(f);
>>>> -       struct radeon_device *rdev = fence->rdev;
>>>> -       struct radeon_wait_cb cb;
>>>> -
>>>> -       cb.task = current;
>>>> -
>>>> -       if (dma_fence_add_callback(f, &cb.base, radeon_fence_wait_cb))
>>>> -               return t;
>>>> -
>>>> -       while (t > 0) {
>>>> -               if (intr)
>>>> -                       set_current_state(TASK_INTERRUPTIBLE);
>>>> -               else
>>>> -                       set_current_state(TASK_UNINTERRUPTIBLE);
>>>> -
>>>> -               /*
>>>> -                * radeon_test_signaled must be called after
>>>> -                * set_current_state to prevent a race with
>>>> wake_up_process
>>>> -                */
>>>> -               if (radeon_test_signaled(fence))
>>>> -                       break;
>>>> -
>>>> -               if (rdev->needs_reset) {
>>>> -                       t = -EDEADLK;
>>>> -                       break;
>>>> -               }
>>>> -
>>>> -               t = schedule_timeout(t);
>>>> -
>>>> -               if (t > 0 && intr && signal_pending(current))
>>>> -                       t = -ERESTARTSYS;
>>>> -       }
>>>> -
>>>> -       __set_current_state(TASK_RUNNING);
>>>> -       dma_fence_remove_callback(f, &cb.base);
>>>> -
>>>> -       return t;
>>>> -}
>>>> -
>>>>    const struct dma_fence_ops radeon_fence_ops = {
>>>>         .get_driver_name = radeon_fence_get_driver_name,
>>>>         .get_timeline_name = radeon_fence_get_timeline_name,
>>>>         .enable_signaling = radeon_fence_enable_signaling,
>>>>         .signaled = radeon_fence_is_signaled,
>>>> -       .wait = radeon_fence_default_wait,
>>>> -       .release = NULL,
>>>>    };
>
>



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 14/17] drm/vc4: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_fence.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_fence.c b/drivers/gpu/drm/vc4/vc4_fence.c
index dbf5a5a5d5f5..9f40c0ddadc4 100644
--- a/drivers/gpu/drm/vc4/vc4_fence.c
+++ b/drivers/gpu/drm/vc4/vc4_fence.c
@@ -33,11 +33,6 @@ static const char *vc4_fence_get_timeline_name(struct dma_fence *fence)
 	return "vc4-v3d";
 }
 
-static bool vc4_fence_enable_signaling(struct dma_fence *fence)
-{
-	return true;
-}
-
 static bool vc4_fence_signaled(struct dma_fence *fence)
 {
 	struct vc4_fence *f = to_vc4_fence(fence);
@@ -49,8 +44,6 @@ static bool vc4_fence_signaled(struct dma_fence *fence)
 const struct dma_fence_ops vc4_fence_ops = {
 	.get_driver_name = vc4_fence_get_driver_name,
 	.get_timeline_name = vc4_fence_get_timeline_name,
-	.enable_signaling = vc4_fence_enable_signaling,
 	.signaled = vc4_fence_signaled,
-	.wait = dma_fence_default_wait,
 	.release = dma_fence_free,
 };
-- 
2.17.0

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

Re: [PATCH 14/17] drm/vc4: Remove unecessary dma_fence_ops

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 251 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

> dma_fence_default_wait is the default now, same for the trivial
> enable_signaling implementation.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH 15/17] drm/vgem: Remove unecessary dma_fence_ops

[Daniel Vetter]

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.17.0

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

[PATCH 16/17] drm/virtio: Remove unecessary dma_fence_ops

[Daniel Vetter]

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

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: David Airlie <airlied@linux.ie>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/virtio/virtgpu_fence.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c
index 23353521f903..00c742a441bf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_fence.c
+++ b/drivers/gpu/drm/virtio/virtgpu_fence.c
@@ -36,11 +36,6 @@ static const char *virtio_get_timeline_name(struct dma_fence *f)
 	return "controlq";
 }
 
-static bool virtio_enable_signaling(struct dma_fence *f)
-{
-	return true;
-}
-
 static bool virtio_signaled(struct dma_fence *f)
 {
 	struct virtio_gpu_fence *fence = to_virtio_fence(f);
@@ -67,9 +62,7 @@ static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size)
 static const struct dma_fence_ops virtio_fence_ops = {
 	.get_driver_name     = virtio_get_driver_name,
 	.get_timeline_name   = virtio_get_timeline_name,
-	.enable_signaling    = virtio_enable_signaling,
 	.signaled            = virtio_signaled,
-	.wait                = dma_fence_default_wait,
 	.fence_value_str     = virtio_fence_value_str,
 	.timeline_value_str  = virtio_timeline_value_str,
 };
-- 
2.17.0

Re: [PATCH 16/17] drm/virtio: Remove unecessary dma_fence_ops

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 193 bytes --]

Daniel Vetter <daniel.vetter@ffwll.ch> writes:

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

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

[PATCH 17/17] dma-fence: Polish kernel-doc for dma-fence.c

[Daniel Vetter]

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

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          | 140 ++++++++++++++++++---------
 2 files changed, 102 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 30fcbe415ff4..4e931e1de198 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 fence->lock held.
+ * Unlike dma_fence_signal(), this function must be called with &dma_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 =
@@ -199,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)
 {
@@ -226,24 +274,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.
@@ -292,7 +340,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
@@ -317,8 +365,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
@@ -329,6 +377,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)
@@ -365,9 +416,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
@@ -460,12 +511,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
@@ -474,6 +525,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,
@@ -546,19 +599,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.17.0

[PATCH 17/17] dma-fence: Polish kernel-doc for dma-fence.c

[Daniel Vetter]

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

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          | 140 ++++++++++++++++++---------
 2 files changed, 102 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 30fcbe415ff4..4e931e1de198 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 fence->lock held.
+ * Unlike dma_fence_signal(), this function must be called with &dma_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 =
@@ -199,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)
 {
@@ -226,24 +274,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.
@@ -292,7 +340,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
@@ -317,8 +365,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
@@ -329,6 +377,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)
@@ -365,9 +416,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
@@ -460,12 +511,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
@@ -474,6 +525,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,
@@ -546,19 +599,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.17.0

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

✗ Fi.CI.CHECKPATCH: warning for dma-fence doc polish and small cleanup

[Patchwork]

== Series Details ==

Series: dma-fence doc polish and small cleanup
URL   : https://patchwork.freedesktop.org/series/42373/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
d608bb886c37 dma-fence: Some kerneldoc polish for dma-fence.h
d3e894db59f2 dma-fence: remove fill_driver_data callback
7a0436313af4 dma-fence: Make ->enable_signaling optional
-:39: WARNING:AVOID_BUG: Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()
#39: FILE: drivers/dma-buf/dma-fence.c:570:
+	BUG_ON(!ops || !ops->wait ||

total: 0 errors, 1 warnings, 0 checks, 40 lines checked
c992546a4299 dma-fence: Allow wait_any_timeout for all fences
-:11: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit a519435a9659 ("dma-buf/fence: add fence_wait_any_timeout function v2")'
#11: 
commit a519435a96597d8cd96123246fea4ae5a6c90b02

total: 1 errors, 0 warnings, 0 checks, 11 lines checked
5bb2c4489fe2 dma-fence: Make ->wait callback optional
82a1077c8bc4 drm/amdgpu: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/amdgpu: Remove unecessary dma_fence_ops

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

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

total: 0 errors, 1 warnings, 0 checks, 20 lines checked
882e1829f9ea 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, 35 lines checked
d96eb2b842cb 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, 20 lines checked
ef2128ee2e42 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
b56969ece9ef drm/qxl: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/qxl: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 19 lines checked
de3955faf8ba drm/radeon: Remove custom dma_fence_ops->wait implementation
9ee21af0f11e drm/vc4: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/vc4: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 19 lines checked
1d34900aa346 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
dab6a77e297e drm/virtio: Remove unecessary dma_fence_ops
-:4: WARNING:TYPO_SPELLING: 'unecessary' may be misspelled - perhaps 'unnecessary'?
#4: 
Subject: [PATCH] drm/virtio: Remove unecessary dma_fence_ops

total: 0 errors, 1 warnings, 0 checks, 20 lines checked
78ce7209ca31 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

✗ Fi.CI.BAT: failure for dma-fence doc polish and small cleanup

[Patchwork]

== Series Details ==

Series: dma-fence doc polish and small cleanup
URL   : https://patchwork.freedesktop.org/series/42373/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4107 -> Patchwork_8820 =

== Summary - FAILURE ==

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

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

== Possible new issues ==

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

  === IGT changes ===

    ==== Possible regressions ====

    igt@core_auth@basic-auth:
      fi-kbl-r:           PASS -> INCOMPLETE
      fi-bwr-2160:        PASS -> INCOMPLETE
      fi-kbl-7560u:       PASS -> INCOMPLETE

    igt@debugfs_test@read_all_entries:
      fi-cfl-u:           PASS -> INCOMPLETE
      fi-ivb-3770:        PASS -> INCOMPLETE
      fi-cfl-s3:          PASS -> INCOMPLETE
      fi-skl-6260u:       PASS -> INCOMPLETE
      fi-snb-2600:        PASS -> INCOMPLETE
      fi-hsw-4770r:       PASS -> INCOMPLETE
      fi-kbl-7500u:       PASS -> INCOMPLETE
      fi-hsw-4200u:       PASS -> INCOMPLETE
      fi-bdw-5557u:       PASS -> INCOMPLETE
      fi-skl-guc:         PASS -> INCOMPLETE
      fi-kbl-7567u:       PASS -> INCOMPLETE
      fi-skl-6600u:       PASS -> INCOMPLETE
      fi-cfl-8700k:       PASS -> INCOMPLETE
      fi-ivb-3520m:       PASS -> INCOMPLETE
      fi-skl-6700k2:      PASS -> INCOMPLETE
      fi-hsw-4770:        PASS -> INCOMPLETE
      fi-skl-6770hq:      PASS -> INCOMPLETE

    igt@gem_close_race@basic-process:
      fi-ilk-650:         PASS -> INCOMPLETE
      fi-blb-e6850:       PASS -> INCOMPLETE
      fi-gdg-551:         PASS -> INCOMPLETE
      fi-pnv-d510:        PASS -> INCOMPLETE
      fi-bsw-n3050:       PASS -> INCOMPLETE

    
== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@core_auth@basic-auth:
      fi-glk-j4005:       PASS -> INCOMPLETE (k.org#198133, fdo#103359)
      fi-bdw-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)
      fi-cnl-psr:         PASS -> INCOMPLETE (fdo#105086)
      fi-cnl-y3:          PASS -> INCOMPLETE (fdo#105086)
      fi-skl-gvtdvm:      PASS -> INCOMPLETE (fdo#105600)

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@gem_close_race@basic-process:
      fi-elk-e7500:       PASS -> INCOMPLETE (fdo#103989)
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)
      fi-byt-j1900:       PASS -> INCOMPLETE (fdo#102657)
      fi-bxt-j4205:       PASS -> INCOMPLETE (fdo#103927)
      fi-byt-n2820:       PASS -> INCOMPLETE (fdo#102657)

    
  fdo#102657 https://bugs.freedesktop.org/show_bug.cgi?id=102657
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
  fdo#105086 https://bugs.freedesktop.org/show_bug.cgi?id=105086
  fdo#105600 https://bugs.freedesktop.org/show_bug.cgi?id=105600
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (39 -> 36) ==

  Missing    (3): fi-ctg-p8600 fi-ilk-m540 fi-skl-6700hq 


== Build changes ==

    * Linux: CI_DRM_4107 -> Patchwork_8820

  CI_DRM_4107: f711c0b36f2382983c964bd69d6c477482e604f1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4450: 0350f0e7f6a0e07281445fc3082aa70419f4aac7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_8820: 78ce7209ca318f37b53fe082a8bfafb4d74ac79a @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4450: b57600ba58ae0cdbad86826fd653aa0191212f27 @ git://anongit.freedesktop.org/piglit


== Linux commits ==

78ce7209ca31 dma-fence: Polish kernel-doc for dma-fence.c
dab6a77e297e drm/virtio: Remove unecessary dma_fence_ops
1d34900aa346 drm/vgem: Remove unecessary dma_fence_ops
9ee21af0f11e drm/vc4: Remove unecessary dma_fence_ops
de3955faf8ba drm/radeon: Remove custom dma_fence_ops->wait implementation
b56969ece9ef drm/qxl: Remove unecessary dma_fence_ops
ef2128ee2e42 drm/nouveau: Remove unecessary dma_fence_ops
d96eb2b842cb drm/msm: Remove unecessary dma_fence_ops
882e1829f9ea drm/i915: Remove unecessary dma_fence_ops
cd72740696a4 drm/etnaviv: Remove unecessary dma_fence_ops
0a9e1f9ea902 drm: Remove unecessary dma_fence_ops
82a1077c8bc4 drm/amdgpu: Remove unecessary dma_fence_ops
5bb2c4489fe2 dma-fence: Make ->wait callback optional
c992546a4299 dma-fence: Allow wait_any_timeout for all fences
7a0436313af4 dma-fence: Make ->enable_signaling optional
d3e894db59f2 dma-fence: remove fill_driver_data callback
d608bb886c37 dma-fence: Some kerneldoc polish for dma-fence.h

== Logs ==

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

Re: [PATCH 00/17] dma-fence doc polish and small cleanup

[Christian König]

Patches #1-#5 in this series are Reviewed-by: Christian König 
<christian.koenig@amd.com>

Regards,
Christian.

Am 27.04.2018 um 08:17 schrieb Daniel Vetter:
> Hi all,
>
> Somewhat motivated by me looking at the v3d patch I went and dug around
> in the dma-fence code a bit. Result was a bit of doc polish and making a
> bunch of callbacks optional for cases where most implementations wanted
> the same default behaviour.
>
> Comments, testing, feedback very much welcome.
>
> Thanks, Daniel
>
> Daniel Vetter (17):
>    dma-fence: Some kerneldoc polish for dma-fence.h
>    dma-fence: remove fill_driver_data callback
>    dma-fence: Make ->enable_signaling optional
>    dma-fence: Allow wait_any_timeout for all fences
>    dma-fence: Make ->wait callback optional
>    drm/amdgpu: Remove unecessary dma_fence_ops
>    drm: Remove unecessary dma_fence_ops
>    drm/etnaviv: Remove unecessary dma_fence_ops
>    drm/i915: Remove unecessary dma_fence_ops
>    drm/msm: Remove unecessary dma_fence_ops
>    drm/nouveau: Remove unecessary dma_fence_ops
>    drm/qxl: Remove unecessary dma_fence_ops
>    drm/radeon: Remove custom dma_fence_ops->wait implementation
>    drm/vc4: Remove unecessary dma_fence_ops
>    drm/vgem: Remove unecessary dma_fence_ops
>    drm/virtio: 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-array.c             |   1 -
>   drivers/dma-buf/dma-fence.c                   | 163 ++++++++----
>   drivers/dma-buf/sw_sync.c                     |   1 -
>   .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_fence.c  |   2 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c     |   1 -
>   drivers/gpu/drm/drm_crtc.c                    |   7 -
>   drivers/gpu/drm/drm_syncobj.c                 |   1 -
>   drivers/gpu/drm/etnaviv/etnaviv_gpu.c         |   7 -
>   drivers/gpu/drm/i915/i915_gem_clflush.c       |   7 -
>   .../gpu/drm/i915/selftests/i915_sw_fence.c    |   7 -
>   drivers/gpu/drm/msm/msm_fence.c               |   7 -
>   drivers/gpu/drm/nouveau/nouveau_fence.c       |   1 -
>   drivers/gpu/drm/qxl/qxl_release.c             |   7 -
>   drivers/gpu/drm/radeon/radeon_fence.c         |  63 -----
>   drivers/gpu/drm/scheduler/sched_fence.c       |  11 -
>   drivers/gpu/drm/vc4/vc4_fence.c               |   7 -
>   drivers/gpu/drm/vgem/vgem_fence.c             |  14 --
>   drivers/gpu/drm/virtio/virtgpu_fence.c        |   7 -
>   include/linux/dma-fence.h                     | 231 +++++++++++-------
>   20 files changed, 267 insertions(+), 284 deletions(-)
>

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

✗ Fi.CI.BAT: failure for dma-fence doc polish and small cleanup (rev3)

[Patchwork]

== Series Details ==

Series: dma-fence doc polish and small cleanup (rev3)
URL   : https://patchwork.freedesktop.org/series/42373/
State : failure

== Summary ==

Applying: dma-fence: Some kerneldoc polish for dma-fence.h
error: Failed to merge in the changes.
Using index info to reconstruct a base tree...
M	include/linux/dma-fence.h
Falling back to patching base and 3-way merge...
Auto-merging include/linux/dma-fence.h
CONFLICT (content): Merge conflict in include/linux/dma-fence.h
Patch failed at 0001 dma-fence: Some kerneldoc polish for dma-fence.h
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

[PATCH] drm/qxl: Remove unecessary dma_fence_ops

[Daniel Vetter]

The trivial enable_signaling implementation matches the default code.

v2: Fix up commit message to match patch better (Eric).

Cc: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/qxl/qxl_release.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 7cb214577275..e37f0097f744 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -50,12 +50,6 @@ static const char *qxl_get_timeline_name(struct dma_fence *fence)
 	return "release";
 }
 
-static bool qxl_nop_signaling(struct dma_fence *fence)
-{
-	/* fences are always automatically signaled, so just pretend we did this.. */
-	return true;
-}
-
 static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 			   signed long timeout)
 {
@@ -119,7 +113,6 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 static const struct dma_fence_ops qxl_fence_ops = {
 	.get_driver_name = qxl_get_driver_name,
 	.get_timeline_name = qxl_get_timeline_name,
-	.enable_signaling = qxl_nop_signaling,
 	.wait = qxl_fence_wait,
 };
 
-- 
2.17.0

[PATCH] drm/qxl: Remove unecessary dma_fence_ops

[Daniel Vetter]

The trivial enable_signaling implementation matches the default code.

v2: Fix up commit message to match patch better (Eric).

Cc: Eric Anholt <eric@anholt.net>
Reviewed-by: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
---
 drivers/gpu/drm/qxl/qxl_release.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c
index 7cb214577275..e37f0097f744 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -50,12 +50,6 @@ static const char *qxl_get_timeline_name(struct dma_fence *fence)
 	return "release";
 }
 
-static bool qxl_nop_signaling(struct dma_fence *fence)
-{
-	/* fences are always automatically signaled, so just pretend we did this.. */
-	return true;
-}
-
 static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 			   signed long timeout)
 {
@@ -119,7 +113,6 @@ static long qxl_fence_wait(struct dma_fence *fence, bool intr,
 static const struct dma_fence_ops qxl_fence_ops = {
 	.get_driver_name = qxl_get_driver_name,
 	.get_timeline_name = qxl_get_timeline_name,
-	.enable_signaling = qxl_nop_signaling,
 	.wait = qxl_fence_wait,
 };
 
-- 
2.17.0

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