On 08/03/2018 14:07, Chris Wilson wrote:
There is some redundancy between dma_fence->ops->enable_signaling (via
i915_fence_enable_signaling) and our backend,
intel_engine_enable_signaling() in that both levels recheck the fence
status multiple times. If we convert intel_engine_enable_signaling() to
return the information desired by dma_fence->ops->enable_signaling, we
can reduce i915_fence_enable_signaling to a simple stub and avoid
trying to reinterpret the same information.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_request.c | 6 +-----
drivers/gpu/drm/i915/intel_breadcrumbs.c | 21 +++++++++++++--------
drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +-
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index d437beac3969..2a841800d4cf 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -59,11 +59,7 @@ static bool i915_fence_signaled(struct dma_fence *fence)
static bool i915_fence_enable_signaling(struct dma_fence *fence)
{
- if (i915_fence_signaled(fence))
- return false;
This was based on hws seqno check...
-
- intel_engine_enable_signaling(to_request(fence), true);
- return !i915_fence_signaled(fence);
+ return intel_engine_enable_signaling(to_request(fence), true);
}
static signed long i915_fence_wait(struct dma_fence *fence,
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index 1f79e7a47433..671a6d61e29d 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -730,10 +730,11 @@ static void insert_signal(struct intel_breadcrumbs *b,
list_add(&request->signaling.link, &iter->signaling.link);
}
-void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
+bool intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
{
struct intel_engine_cs *engine = request->engine;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
+ struct intel_wait *wait = &request->signaling.wait;
u32 seqno;
/*
@@ -750,12 +751,12 @@ void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
seqno = i915_request_global_seqno(request);
if (!seqno) /* will be enabled later upon execution */
- return;
+ return true;
- GEM_BUG_ON(request->signaling.wait.seqno);
- request->signaling.wait.tsk = b->signaler;
- request->signaling.wait.request = request;
- request->signaling.wait.seqno = seqno;
+ GEM_BUG_ON(wait->seqno);
+ wait->tsk = b->signaler;
+ wait->request = request;
+ wait->seqno = seqno;
/*
* Add ourselves into the list of waiters, but registering our
@@ -768,11 +769,15 @@ void intel_engine_enable_signaling(struct i915_request *request, bool wakeup)
*/
spin_lock(&b->rb_lock);
insert_signal(b, request, seqno);
- wakeup &= __intel_engine_add_wait(engine, &request->signaling.wait);
+ wakeup &= __intel_engine_add_wait(engine, wait);
spin_unlock(&b->rb_lock);
- if (wakeup)
+ if (wakeup) {
wake_up_process(b->signaler);
+ return !intel_wait_complete(wait);
... and would now be based on breadcrumb waiter waking up and removing
itself from the tree.
So some potential latency where it wasn't before, well, enable-disable
signalling cycles actually.
If I got it right.. breadcrumbs code confuses me massively these days.
Regards,
Tvrtko
+ }
+
+ return true;
}
void intel_engine_cancel_signaling(struct i915_request *request)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 0320c2c4cfba..aa34b2ff04bc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -939,7 +939,7 @@ bool intel_engine_add_wait(struct intel_engine_cs *engine,
struct intel_wait *wait);
void intel_engine_remove_wait(struct intel_engine_cs *engine,
struct intel_wait *wait);
-void intel_engine_enable_signaling(struct i915_request *request, bool wakeup);
+bool intel_engine_enable_signaling(struct i915_request *request, bool wakeup);
void intel_engine_cancel_signaling(struct i915_request *request);
static inline bool intel_engine_has_waiter(const struct intel_engine_cs *engine)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
