On 25/06/16 11:13, Chris Wilson wrote:
Under the assumption that enabling signaling will be a frequent
operation, lets preallocate our attachments for signaling inside the
(rather large) request struct (and so benefiting from the slab cache).
v2: Convert from void * to more meaningful names and types.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_breadcrumbs.c | 76 ++++++++++++++++----------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 10 ++++-
3 files changed, 46 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7089cedb80c..440f6267ec27 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2381,6 +2381,7 @@ struct drm_i915_gem_request {
struct drm_i915_private *i915;
struct intel_engine_cs *engine;
unsigned reset_counter;
+ struct intel_signal_node signaling;
/** GEM sequence number associated with the previous request,
* when the HWS breadcrumb is equal to this the GPU is processing
diff --git a/drivers/gpu/drm/i915/intel_breadcrumbs.c b/drivers/gpu/drm/i915/intel_breadcrumbs.c
index bc0e9e8b7871..661d6b415ac3 100644
--- a/drivers/gpu/drm/i915/intel_breadcrumbs.c
+++ b/drivers/gpu/drm/i915/intel_breadcrumbs.c
@@ -351,35 +351,29 @@ out_unlock:
spin_unlock(&b->lock);
}
-struct signal {
- struct rb_node node;
- struct intel_wait wait;
- struct drm_i915_gem_request *request;
-};
-
-static bool signal_complete(struct signal *signal)
+static bool signal_complete(struct drm_i915_gem_request *request)
{
- if (signal == NULL)
+ if (request == NULL)
return false;
/* If another process served as the bottom-half it may have already
* signalled that this wait is already completed.
*/
- if (intel_wait_complete(&signal->wait))
+ if (intel_wait_complete(&request->signaling.wait))
return true;
/* Carefully check if the request is complete, giving time for the
* seqno to be visible or if the GPU hung.
*/
- if (__i915_request_irq_complete(signal->request))
+ if (__i915_request_irq_complete(request))
return true;
return false;
}
-static struct signal *to_signal(struct rb_node *rb)
+static struct drm_i915_gem_request *to_signaler(struct rb_node *rb)
{
- return container_of(rb, struct signal, node);
+ return container_of(rb, struct drm_i915_gem_request, signaling.node);
}
static void signaler_set_rtpriority(void)
@@ -392,7 +386,7 @@ static int intel_breadcrumbs_signaler(void *arg)
{
struct intel_engine_cs *engine = arg;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
- struct signal *signal;
+ struct drm_i915_gem_request *request;
/* Install ourselves with high priority to reduce signalling latency */
signaler_set_rtpriority();
@@ -408,14 +402,13 @@ static int intel_breadcrumbs_signaler(void *arg)
* need to wait for a new interrupt from the GPU or for
* a new client.
*/
- signal = READ_ONCE(b->first_signal);
- if (signal_complete(signal)) {
+ request = READ_ONCE(b->first_signal);
+ if (signal_complete(request)) {
/* Wake up all other completed waiters and select the
* next bottom-half for the next user interrupt.
*/
- intel_engine_remove_wait(engine, &signal->wait);
-
- i915_gem_request_unreference(signal->request);
+ intel_engine_remove_wait(engine,
+ &request->signaling.wait);
/* Find the next oldest signal. Note that as we have
* not been holding the lock, another client may
@@ -424,12 +417,15 @@ static int intel_breadcrumbs_signaler(void *arg)
* the oldest before picking the next one.
*/
spin_lock(&b->lock);
- if (signal == b->first_signal)
- b->first_signal = rb_next(&signal->node);
- rb_erase(&signal->node, &b->signals);
+ if (request == b->first_signal) {
+ struct rb_node *rb =
+ rb_next(&request->signaling.node);
+ b->first_signal = rb ? to_signaler(rb) : NULL;
+ }
+ rb_erase(&request->signaling.node, &b->signals);
spin_unlock(&b->lock);
- kfree(signal);
+ i915_gem_request_unreference(request);
} else {
if (kthread_should_stop())
break;
@@ -442,22 +438,25 @@ static int intel_breadcrumbs_signaler(void *arg)
return 0;
}
-int intel_engine_enable_signaling(struct drm_i915_gem_request *request)
+void intel_engine_enable_signaling(struct drm_i915_gem_request *request)
{
struct intel_engine_cs *engine = request->engine;
struct intel_breadcrumbs *b = &engine->breadcrumbs;
struct rb_node *parent, **p;
- struct signal *signal;
bool first, wakeup;
- signal = kmalloc(sizeof(*signal), GFP_ATOMIC);
- if (unlikely(!signal))
- return -ENOMEM;
+ if (unlikely(READ_ONCE(request->signaling.wait.tsk)))
+ return;
- signal->wait.tsk = b->signaler;
- signal->wait.seqno = request->seqno;
+ spin_lock(&b->lock);
+ if (unlikely(request->signaling.wait.tsk)) {
+ wakeup = false;
+ goto unlock;
+ }
- signal->request = i915_gem_request_reference(request);
+ request->signaling.wait.tsk = b->signaler;
+ request->signaling.wait.seqno = request->seqno;
+ i915_gem_request_reference(request);
/* First add ourselves into the list of waiters, but register our
* bottom-half as the signaller thread. As per usual, only the oldest
@@ -467,35 +466,34 @@ int intel_engine_enable_signaling(struct drm_i915_gem_request *request)
* If we are the oldest waiter, enable the irq (after which we
* must double check that the seqno did not complete).
*/
- wakeup = intel_engine_add_wait(engine, &signal->wait);
+ wakeup = __intel_engine_add_wait(engine, &request->signaling.wait);
/* Now insert ourselves into the retirement ordered list of signals
* on this engine. We track the oldest seqno as that will be the
* first signal to complete.
*/
- spin_lock(&b->lock);
parent = NULL;
first = true;
p = &b->signals.rb_node;
while (*p) {
parent = *p;
- if (i915_seqno_passed(signal->wait.seqno,
- to_signal(parent)->wait.seqno)) {
+ if (i915_seqno_passed(request->seqno,
+ to_signaler(parent)->seqno)) {
p = &parent->rb_right;
first = false;
} else
p = &parent->rb_left;
}
- rb_link_node(&signal->node, parent, p);
- rb_insert_color(&signal->node, &b->signals);
+ rb_link_node(&request->signaling.node, parent, p);
+ rb_insert_color(&request->signaling.node, &b->signals);
if (first)
- smp_store_mb(b->first_signal, signal);
+ smp_store_mb(b->first_signal, request);
+
+unlock:
spin_unlock(&b->lock);
if (wakeup)
wake_up_process(b->signaler);
-
- return 0;
}
int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c05b45727f7d..b55b9961d9f6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -185,7 +185,7 @@ struct intel_engine_cs {
struct intel_wait *first_wait; /* oldest waiter by retirement */
struct task_struct *tasklet; /* bh for user interrupts */
struct task_struct *signaler; /* used for fence signalling */
- void *first_signal;
+ struct drm_i915_gem_request *first_signal;
struct timer_list fake_irq; /* used after a missed interrupt */
bool irq_enabled;
bool rpm_wakelock;
@@ -530,6 +530,12 @@ struct intel_wait {
struct task_struct *tsk;
u32 seqno;
};
+
+struct intel_signal_node {
+ struct rb_node node;
+ struct intel_wait wait;
+};
+
int intel_engine_init_breadcrumbs(struct intel_engine_cs *engine);
static inline void intel_wait_init(struct intel_wait *wait, u32 seqno)
{
@@ -544,7 +550,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);
-int intel_engine_enable_signaling(struct drm_i915_gem_request *request);
+void intel_engine_enable_signaling(struct drm_i915_gem_request *request);
static inline bool intel_engine_has_waiter(struct intel_engine_cs *engine)
{
return READ_ONCE(engine->breadcrumbs.tasklet);
Looks OK.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
