From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 32/43] drm/i915/bdw: Avoid non-lite-restore preemptions Date: Thu, 14 Aug 2014 22:31:12 +0200 Message-ID: <20140814203112.GJ10500@phenom.ffwll.local> References: <1406217891-8912-1-git-send-email-thomas.daniel@intel.com> <1406217891-8912-33-git-send-email-thomas.daniel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by gabe.freedesktop.org (Postfix) with ESMTP id 961C56E74E for ; Thu, 14 Aug 2014 13:31:01 -0700 (PDT) Received: by mail-we0-f182.google.com with SMTP id k48so1601589wev.41 for ; Thu, 14 Aug 2014 13:31:00 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1406217891-8912-33-git-send-email-thomas.daniel@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Thomas Daniel Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Jul 24, 2014 at 05:04:40PM +0100, Thomas Daniel wrote: > From: Oscar Mateo > = > In the current Execlists feeding mechanism, full preemption is not > supported yet: only lite-restores are allowed (this is: the GPU > simply samples a new tail pointer for the context currently in > execution). > = > But we have identified an scenario in which a full preemption occurs: > 1) We submit two contexts for execution (A & B). > 2) The GPU finishes with the first one (A), switches to the second one > (B) and informs us. > 3) We submit B again (hoping to cause a lite restore) together with C, > but in the time we spend writing to the ELSP, the GPU finishes B. > 4) The GPU start executing B again (since we told it so). > 5) We receive a B finished interrupt and, mistakenly, we submit C (again) > and D, causing a full preemption of B. > = > The race is avoided by keeping track of how many times a context has been > submitted to the hardware and by better discriminating the received conte= xt > switch interrupts: in the example, when we have submitted B twice, we won= =B4t > submit C and D as soon as we receive the notification that B is completed > because we were expecting to get a LITE_RESTORE and we didn=B4t, so we kn= ow a > second completion will be received shortly. > = > Without this explicit checking, somehow, the batch buffer execution order > gets messed with. This can be verified with the IGT test I sent together = with > the series. I don=B4t know the exact mechanism by which the pre-emption m= esses > with the execution order but, since other people is working on the Schedu= ler > + Preemption on Execlists, I didn=B4t try to fix it. In these series, onl= y Lite > Restores are supported (other kind of preemptions WARN). Where's this igt patch? The kernel patch here is at least missing the Testcase: igt/foo tag. Please supply. -Daniel > = > v2: elsp_submitted belongs in the new intel_ctx_submit_request. Several > rebase changes. > = > v3: Clarify how the race is avoided, as requested by Daniel. > = > Signed-off-by: Oscar Mateo > --- > drivers/gpu/drm/i915/intel_lrc.c | 28 ++++++++++++++++++++++++---- > drivers/gpu/drm/i915/intel_lrc.h | 2 ++ > 2 files changed, 26 insertions(+), 4 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/inte= l_lrc.c > index 65f4f26..895dbfc 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -264,6 +264,7 @@ static void execlists_context_unqueue(struct intel_en= gine_cs *ring) > else if (req0->ctx =3D=3D cursor->ctx) { > /* Same ctx: ignore first request, as second request > * will update tail past first request's workload */ > + cursor->elsp_submitted =3D req0->elsp_submitted; > list_del(&req0->execlist_link); > queue_work(dev_priv->wq, &req0->work); > req0 =3D cursor; > @@ -273,8 +274,14 @@ static void execlists_context_unqueue(struct intel_e= ngine_cs *ring) > } > } > = > + WARN_ON(req1 && req1->elsp_submitted); > + > BUG_ON(execlists_submit_context(ring, req0->ctx, req0->tail, > req1? req1->ctx : NULL, req1? req1->tail : 0)); > + > + req0->elsp_submitted++; > + if (req1) > + req1->elsp_submitted++; > } > = > static bool execlists_check_remove_request(struct intel_engine_cs *ring, > @@ -291,9 +298,13 @@ static bool execlists_check_remove_request(struct in= tel_engine_cs *ring, > struct drm_i915_gem_object *ctx_obj =3D > head_req->ctx->engine[ring->id].state; > if (intel_execlists_ctx_id(ctx_obj) =3D=3D request_id) { > - list_del(&head_req->execlist_link); > - queue_work(dev_priv->wq, &head_req->work); > - return true; > + WARN(head_req->elsp_submitted =3D=3D 0, > + "Never submitted head request\n"); > + if (--head_req->elsp_submitted <=3D 0) { > + list_del(&head_req->execlist_link); > + queue_work(dev_priv->wq, &head_req->work); > + return true; > + } > } > } > = > @@ -326,7 +337,16 @@ void intel_execlists_handle_ctx_events(struct intel_= engine_cs *ring) > status_id =3D I915_READ(RING_CONTEXT_STATUS_BUF(ring) + > (read_pointer % 6) * 8 + 4); > = > - if (status & GEN8_CTX_STATUS_COMPLETE) { > + if (status & GEN8_CTX_STATUS_PREEMPTED) { > + if (status & GEN8_CTX_STATUS_LITE_RESTORE) { > + if (execlists_check_remove_request(ring, status_id)) > + WARN(1, "Lite Restored request removed from queue\n"); > + } else > + WARN(1, "Preemption without Lite Restore\n"); > + } > + > + if ((status & GEN8_CTX_STATUS_ACTIVE_IDLE) || > + (status & GEN8_CTX_STATUS_ELEMENT_SWITCH)) { > if (execlists_check_remove_request(ring, status_id)) > submit_contexts++; > } > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/inte= l_lrc.h > index 2e8929f..074b44f 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -67,6 +67,8 @@ struct intel_ctx_submit_request { > = > struct list_head execlist_link; > struct work_struct work; > + > + int elsp_submitted; > }; > = > void intel_execlists_handle_ctx_events(struct intel_engine_cs *ring); > -- = > 1.7.9.5 > = > _______________________________________________ > 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