On Fri, May 09, 2014 at 01:09:19PM +0100, oscar.mateo@xxxxxxxxx wrote:
> From: Oscar Mateo <oscar.mateo@xxxxxxxxx>
>
> If we receive a storm of requests for the same context (see gem_storedw_loop_*)
> we might end up iterating over too many elements in interrupt time, looking for
> contexts to squash together. Instead, share the burden by giving more
> intelligence to the queue function. At most, the interrupt will iterate over
> three elements.
>
> Signed-off-by: Oscar Mateo <oscar.mateo@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_lrc.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index d9edd10..0aad721 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -410,9 +410,11 @@ int gen8_switch_context_queue(struct intel_engine *ring,
> struct i915_hw_context *to,
> u32 tail)
> {
> + struct drm_i915_private *dev_priv = ring->dev->dev_private;
> struct drm_i915_gem_request *req = NULL;
> unsigned long flags;
> - bool was_empty;
> + struct drm_i915_gem_request *cursor;
> + int num_elements = 0;
>
> req = kzalloc(sizeof(*req), GFP_KERNEL);
> if (req == NULL)
> @@ -425,9 +427,24 @@ int gen8_switch_context_queue(struct intel_engine *ring,
>
> spin_lock_irqsave(&ring->execlist_lock, flags);
>
> - was_empty = list_empty(&ring->execlist_queue);
> + list_for_each_entry(cursor, &ring->execlist_queue, execlist_link)
> + if (++num_elements > 2)
> + break;
> +
> + if (num_elements > 2) {
> + struct drm_i915_gem_request *tail_req =
> + list_last_entry(&ring->execlist_queue,
> + struct drm_i915_gem_request, execlist_link);
> + if (to == tail_req->ctx) {
> + WARN(tail_req->elsp_submitted != 0,
> + "More than 2 already-submitted reqs queued\n");
> + list_del(&tail_req->execlist_link);
> + queue_work(dev_priv->wq, &tail_req->work);
> + }
> + }
Completely forgotten to mention this: Chris&I discussed this on irc and I
guess this issue will disappear if we track contexts instead of requests
in the scheduler. I guess this is an artifact of the gen7 scheduler you've
based this on, but even for that I think scheduling contexts (with preempt
point after each batch) is the right approach. But I haven't dug out the
scheduler patches again so might be wrong with that.
-Daniel
> +
> list_add_tail(&req->execlist_link, &ring->execlist_queue);
> - if (was_empty)
> + if (num_elements == 0)
> gen8_switch_context_unqueue(ring);
>
> spin_unlock_irqrestore(&ring->execlist_lock, flags);
> --
> 1.9.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
