From patchwork Fri May 6 19:31:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 9035931 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 8C5CABF29F for ; Fri, 6 May 2016 19:31:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7DDF6203A0 for ; Fri, 6 May 2016 19:31:57 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 6F24C2039D for ; Fri, 6 May 2016 19:31:56 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E353A6E2C4; Fri, 6 May 2016 19:31:55 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by gabe.freedesktop.org (Postfix) with ESMTP id 374886E2C3 for ; Fri, 6 May 2016 19:31:55 +0000 (UTC) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by fmsmga104.fm.intel.com with ESMTP; 06 May 2016 12:31:47 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,587,1455004800"; d="scan'208";a="800624593" Received: from dsgordon-linux2.isw.intel.com ([10.102.226.88]) by orsmga003.jf.intel.com with ESMTP; 06 May 2016 12:31:40 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Fri, 6 May 2016 20:31:32 +0100 Message-Id: <1462563095-13233-3-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1462563095-13233-1-git-send-email-david.s.gordon@intel.com> References: <1462563095-13233-1-git-send-email-david.s.gordon@intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH v3 3/6] drm/i915/guc: pass request (not client) to i915_guc_{wq_check_space, submit}() X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP The knowledge of how to derive the relevant client from the request should be localised within i915_guc_submission.c; the LRC code shouldn't have to know about the internal details of the GuC submission process. And all the information the GuC code needs should be encapsulated in (or reachable from) the request. v3: GEM_BUG_ON() for bad GuC client (Tvrtko Ursulin). Add/update kerneldoc explaining check_space/submit protocol Signed-off-by: Dave Gordon Reviewed-by: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_guc_submission.c | 46 ++++++++++++++++++++++++------ drivers/gpu/drm/i915/intel_guc.h | 5 ++-- drivers/gpu/drm/i915/intel_lrc.c | 9 ++---- 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index e6af672..9e30762 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -451,14 +451,30 @@ static void guc_fini_ctx_desc(struct intel_guc *guc, sizeof(desc) * client->ctx_index); } -int i915_guc_wq_check_space(struct i915_guc_client *gc) +/** + * i915_guc_wq_check_space() - check that the GuC can accept a request + * @request: request associated with the commands + * + * Return: 0 if space is available + * -EAGAIN if space is not currently available + * + * This function must be called (and must return 0) before a request + * is submitted to the GuC via i915_guc_submit() below. Once a result + * of 0 has been returned, it remains valid until (but only until) + * the next call to submit(). + * + * This precheck allows the caller to determine in advance that space + * will be available for the next submission before committing resources + * to it, and helps avoid late failures with complicated recovery paths. + */ +int i915_guc_wq_check_space(struct drm_i915_gem_request *request) { + const size_t size = sizeof(struct guc_wq_item); + struct i915_guc_client *gc = request->i915->guc.execbuf_client; struct guc_process_desc *desc; - u32 size = sizeof(struct guc_wq_item); int ret = -ETIMEDOUT, timeout_counter = 200; - if (!gc) - return 0; + GEM_BUG_ON(gc == NULL); desc = gc->client_base + gc->proc_desc_offset; @@ -532,16 +548,28 @@ static int guc_add_workqueue_item(struct i915_guc_client *gc, /** * i915_guc_submit() - Submit commands through GuC - * @client: the guc client where commands will go through * @rq: request associated with the commands * - * Return: 0 if succeed + * Return: 0 on success, otherwise an errno. + * (Note: nonzero really shouldn't happen!) + * + * The caller must have already called i915_guc_wq_check_space() above + * with a result of 0 (success) since the last request submission. This + * guarantees that there is space in the work queue for the new request, + * so enqueuing the item cannot fail. + * + * Bad Things Will Happen if the caller violates this protocol e.g. calls + * submit() when check() says there's no space, or calls submit() multiple + * times with no intervening check(). + * + * The only error here arises if the doorbell hardware isn't functioning + * as expected, which really shouln't happen. */ -int i915_guc_submit(struct i915_guc_client *client, - struct drm_i915_gem_request *rq) +int i915_guc_submit(struct drm_i915_gem_request *rq) { - struct intel_guc *guc = client->guc; unsigned int engine_id = rq->engine->guc_id; + struct intel_guc *guc = &rq->i915->guc; + struct i915_guc_client *client = guc->execbuf_client; int q_ret, b_ret; q_ret = guc_add_workqueue_item(client, rq); diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h index 9d79c4c..b37c731 100644 --- a/drivers/gpu/drm/i915/intel_guc.h +++ b/drivers/gpu/drm/i915/intel_guc.h @@ -148,10 +148,9 @@ extern int intel_guc_resume(struct drm_device *dev); /* i915_guc_submission.c */ int i915_guc_submission_init(struct drm_device *dev); int i915_guc_submission_enable(struct drm_device *dev); -int i915_guc_submit(struct i915_guc_client *client, - struct drm_i915_gem_request *rq); +int i915_guc_wq_check_space(struct drm_i915_gem_request *rq); +int i915_guc_submit(struct drm_i915_gem_request *rq); void i915_guc_submission_disable(struct drm_device *dev); void i915_guc_submission_fini(struct drm_device *dev); -int i915_guc_wq_check_space(struct i915_guc_client *client); #endif diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index d876352..79302e1 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -698,9 +698,7 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request * going any further, as the i915_add_request() call * later on mustn't fail ... */ - struct intel_guc *guc = &request->i915->guc; - - ret = i915_guc_wq_check_space(guc->execbuf_client); + ret = i915_guc_wq_check_space(request); if (ret) return ret; } @@ -749,7 +747,6 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request intel_logical_ring_advance_and_submit(struct drm_i915_gem_request *request) { struct intel_ringbuffer *ringbuf = request->ringbuf; - struct drm_i915_private *dev_priv = request->i915; struct intel_engine_cs *engine = request->engine; intel_logical_ring_advance(ringbuf); @@ -777,8 +774,8 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request request->previous_context = engine->last_context; engine->last_context = request->ctx; - if (dev_priv->guc.execbuf_client) - i915_guc_submit(dev_priv->guc.execbuf_client, request); + if (i915.enable_guc_submission) + i915_guc_submit(request); else execlists_context_queue(request);