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