From 51f170a544bdb06d93316d8ff0814a52daa24a6c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Thu, 3 May 2018 12:31:38 +0200 Subject: [PATCH] Revert 190c462d5be19ba622a82f5fd0625087c870a1e6..bf3012ada1b2222e770de5c35c1bb16f73b3a01d" I shouldn't have pushed this, CI was right - I failed to remove the BUG_ON(!ops->wait); Reported-by: Chris Wilson Acked-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/dma-buf/dma-fence-array.c | 1 + drivers/dma-buf/dma-fence.c | 23 ++++++------------ drivers/dma-buf/sw_sync.c | 1 + drivers/gpu/drm/drm_crtc.c | 7 ++++++ drivers/gpu/drm/drm_syncobj.c | 1 + drivers/gpu/drm/qxl/qxl_release.c | 7 ++++++ drivers/gpu/drm/scheduler/sched_fence.c | 11 +++++++++ include/linux/dma-fence.h | 32 +++++++++++++++---------- 8 files changed, 54 insertions(+), 29 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index a8c254497251..dd1edfb27b61 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -104,6 +104,7 @@ 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 30fcbe415ff4..4edb9fd3cf47 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -158,10 +158,7 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) return -EINVAL; trace_dma_fence_wait_start(fence); - if (fence->ops->wait) - ret = fence->ops->wait(fence, intr, timeout); - else - ret = dma_fence_default_wait(fence, intr, timeout); + ret = fence->ops->wait(fence, intr, timeout); trace_dma_fence_wait_end(fence); return ret; } @@ -184,13 +181,6 @@ 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); @@ -506,6 +496,11 @@ 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)) { @@ -565,7 +560,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 || + BUG_ON(!ops || !ops->wait || !ops->enable_signaling || !ops->get_driver_name || !ops->get_timeline_name); kref_init(&fence->refcount); @@ -577,10 +572,6 @@ 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/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c index 53c1d6d36a64..3d78ca89a605 100644 --- a/drivers/dma-buf/sw_sync.c +++ b/drivers/dma-buf/sw_sync.c @@ -188,6 +188,7 @@ 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/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e4d3285f4191..a231dd5dce16 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -225,9 +225,16 @@ 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 adb3cb27d31e..d4f4ce484529 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -207,6 +207,7 @@ 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/qxl/qxl_release.c b/drivers/gpu/drm/qxl/qxl_release.c index 04f3605ac42a..5d84a66fed36 100644 --- a/drivers/gpu/drm/qxl/qxl_release.c +++ b/drivers/gpu/drm/qxl/qxl_release.c @@ -50,6 +50,12 @@ 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) { @@ -113,6 +119,7 @@ signaled: 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, }; diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c index 4843289cc8f0..69aab086b913 100644 --- a/drivers/gpu/drm/scheduler/sched_fence.c +++ b/drivers/gpu/drm/scheduler/sched_fence.c @@ -81,6 +81,11 @@ 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 * @@ -129,12 +134,18 @@ 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, }; diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 02dba8cd033d..eb9b05aa5aea 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -166,8 +166,7 @@ struct dma_fence_ops { * released when the fence is signalled (through e.g. the interrupt * handler). * - * This callback is optional. If this callback is not present, then the - * driver must always have signaling enabled. + * This callback is mandatory. */ bool (*enable_signaling)(struct dma_fence *fence); @@ -191,14 +190,11 @@ struct dma_fence_ops { /** * @wait: * - * Custom wait implementation, defaults to dma_fence_default_wait() if - * not set. + * Custom wait implementation, or dma_fence_default_wait. * - * 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 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 @@ -206,7 +202,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 optional. + * This callback is mandatory. */ signed long (*wait)(struct dma_fence *fence, bool intr, signed long timeout); @@ -221,6 +217,17 @@ 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: * @@ -235,9 +242,8 @@ struct dma_fence_ops { * @timeline_value_str: * * Fills in the current value of the timeline as a string, like the - * 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. + * 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); -- 2.20.1