drm/i915: Embed signaling node into the GEM request
authorChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Jul 2016 16:23:26 +0000 (17:23 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Fri, 1 Jul 2016 20:04:14 +0000 (21:04 +0100)
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@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1467390209-3576-17-git-send-email-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_breadcrumbs.c
drivers/gpu/drm/i915/intel_ringbuffer.h

index ed4116f..0dcc43d 100644 (file)
@@ -2380,6 +2380,7 @@ struct drm_i915_gem_request {
        /** On Which ring this request was generated */
        struct drm_i915_private *i915;
        struct intel_engine_cs *engine;
+       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
index fa1e957..f20ccac 100644 (file)
@@ -356,35 +356,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)
+       if (!request)
                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)
@@ -398,7 +392,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();
@@ -414,14 +408,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
@@ -430,12 +423,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;
@@ -448,22 +444,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
@@ -473,36 +472,35 @@ 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)
index 1d4a236..cac8383 100644 (file)
@@ -173,7 +173,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;
@@ -519,6 +519,11 @@ struct intel_wait {
        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)
@@ -536,7 +541,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)
 {