From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 49/50] drm/i915/bdw: Help out the ctx switch interrupt handler Date: Wed, 11 Jun 2014 13:50:11 +0200 Message-ID: <20140611115011.GJ5821@phenom.ffwll.local> References: <1399637360-4277-1-git-send-email-oscar.mateo@intel.com> <1399637360-4277-50-git-send-email-oscar.mateo@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by gabe.freedesktop.org (Postfix) with ESMTP id B22386E84B for ; Wed, 11 Jun 2014 04:50:21 -0700 (PDT) Received: by mail-wi0-f171.google.com with SMTP id n15so4802331wiw.4 for ; Wed, 11 Jun 2014 04:50:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1399637360-4277-50-git-send-email-oscar.mateo@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: oscar.mateo@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, May 09, 2014 at 01:09:19PM +0100, oscar.mateo@intel.com wrote: > From: Oscar Mateo > > 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 > --- > 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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch