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