From patchwork Tue Mar 22 11:48:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dave Gordon X-Patchwork-Id: 8641431 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 60277C0553 for ; Tue, 22 Mar 2016 11:48:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E0E34202B8 for ; Tue, 22 Mar 2016 11:48:28 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 604D320254 for ; Tue, 22 Mar 2016 11:48:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id E84436E2A9; Tue, 22 Mar 2016 11:48:25 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 002DA6E2B7 for ; Tue, 22 Mar 2016 11:48:23 +0000 (UTC) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga102.fm.intel.com with ESMTP; 22 Mar 2016 04:48:24 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,376,1455004800"; d="scan'208";a="939089568" Received: from dsgordon-linux2.isw.intel.com ([10.102.226.88]) by orsmga002.jf.intel.com with ESMTP; 22 Mar 2016 04:48:22 -0700 From: Dave Gordon To: intel-gfx@lists.freedesktop.org Date: Tue, 22 Mar 2016 11:48:19 +0000 Message-Id: <1458647300-30914-1-git-send-email-david.s.gordon@intel.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <56F0120D.201@intel.com> References: <56F0120D.201@intel.com> In-reply-to: <56F0120D.201@intel.com> Organization: Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ Subject: [Intel-gfx] [PATCH v3 1/2] drm/i915: name the anonymous per-engine context struct 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=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_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 Introduce a name for the previously anonymous per-engine structure embedded inside the intel_context, and use local pointers to the relevant element of this array to simplify various execlist functions. This may improve the compiler's ability to avoid redundant dereference- and index operations. This version is derived from an original by Chris Wilson, detached from the original set of patches in which it was posted and rebased to current nightly. Then it was updated by Tvrtko Ursulin , who noted: This anonymous struct was causing a good amount of overly verbose code. Also, if we name it and cache the pointer locally when there are multiple accesses to it, not only the code is more readable, but the compiler manages to generate smaller binary. Along the way I also shortened access to dev_priv and eliminated some unused variables and cached some where I spotted the opportunity. This version uses the name "struct intel_engine_ctx" in accordance with the English (and German) convention that the concrete noun goes at the end of a noun-sequence, with earlier ones acting as adjectives i.e. an "engine-context" is the context of an engine, whereas a "context-engine" would be some sort of engine that processes contexts. Since object and structure names are noun-like, it's more consistent to name them this way. Originally-by: Chris Wilson Signed-off-by: Tvrtko Ursulin Signed-off-by: Dave Gordon Cc: Chris Wilson --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_lrc.c | 122 +++++++++++++++++++-------------------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 00c41a4..be3709f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -840,7 +840,7 @@ struct intel_context { } legacy_hw_ctx; /* Execlists */ - struct { + struct intel_engine_ctx { struct drm_i915_gem_object *state; struct intel_ringbuffer *ringbuf; int pin_count; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index f727822..a054a32 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -316,16 +316,16 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists intel_lr_context_descriptor_update(struct intel_context *ctx, struct intel_engine_cs *engine) { + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; uint64_t lrca, desc; - lrca = ctx->engine[engine->id].lrc_vma->node.start + - LRC_PPHWSP_PN * PAGE_SIZE; + lrca = ectx->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE; - desc = engine->ctx_desc_template; /* bits 0-11 */ + desc = engine->ctx_desc_template; /* bits 0-11 */ desc |= lrca; /* bits 12-31 */ desc |= (lrca >> PAGE_SHIFT) << GEN8_CTX_ID_SHIFT; /* bits 32-51 */ - ctx->engine[engine->id].lrc_desc = desc; + ectx->lrc_desc = desc; } uint64_t intel_lr_context_descriptor(struct intel_context *ctx, @@ -361,8 +361,7 @@ static void execlists_elsp_write(struct drm_i915_gem_request *rq0, { struct intel_engine_cs *engine = rq0->engine; - struct drm_device *dev = engine->dev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = rq0->i915; uint64_t desc[2]; if (rq1) { @@ -468,11 +467,8 @@ static void execlists_context_unqueue__locked(struct intel_engine_cs *engine) * resubmit the request. See gen8_emit_request() for where we * prepare the padding after the end of the request. */ - struct intel_ringbuffer *ringbuf; - - ringbuf = req0->ctx->engine[engine->id].ringbuf; req0->tail += 8; - req0->tail &= ringbuf->size - 1; + req0->tail &= req0->ringbuf->size - 1; } execlists_submit_requests(req0, req1); @@ -669,7 +665,8 @@ static int logical_ring_invalidate_all_caches(struct drm_i915_gem_request *req) static int execlists_move_to_gpu(struct drm_i915_gem_request *req, struct list_head *vmas) { - const unsigned other_rings = ~intel_engine_flag(req->engine); + struct intel_engine_cs *engine = req->engine; + const unsigned other_rings = ~intel_engine_flag(engine); struct i915_vma *vma; uint32_t flush_domains = 0; bool flush_chipset = false; @@ -679,7 +676,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req, struct drm_i915_gem_object *obj = vma->obj; if (obj->active & other_rings) { - ret = i915_gem_object_sync(obj, req->engine, &req); + ret = i915_gem_object_sync(obj, engine, &req); if (ret) return ret; } @@ -701,9 +698,11 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req, int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request) { + struct intel_engine_cs *engine = request->engine; + struct intel_context *ctx = request->ctx; int ret = 0; - request->ringbuf = request->ctx->engine[request->engine->id].ringbuf; + request->ringbuf = ctx->engine[engine->id].ringbuf; if (i915.enable_guc_submission) { /* @@ -718,8 +717,8 @@ int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request return ret; } - if (request->ctx != request->i915->kernel_context) - ret = intel_lr_context_pin(request->ctx, request->engine); + if (ctx != request->i915->kernel_context) + ret = intel_lr_context_pin(ctx, engine); return ret; } @@ -781,6 +780,7 @@ static int logical_ring_wait_for_space(struct drm_i915_gem_request *req, struct intel_ringbuffer *ringbuf = request->ringbuf; struct drm_i915_private *dev_priv = request->i915; struct intel_engine_cs *engine = request->engine; + struct intel_context *ctx = request->ctx; intel_logical_ring_advance(ringbuf); request->tail = ringbuf->tail; @@ -798,12 +798,12 @@ static int logical_ring_wait_for_space(struct drm_i915_gem_request *req, if (intel_engine_stopped(engine)) return 0; - if (engine->last_context != request->ctx) { + if (engine->last_context != ctx) { if (engine->last_context) intel_lr_context_unpin(engine->last_context, engine); - if (request->ctx != request->i915->kernel_context) { - intel_lr_context_pin(request->ctx, engine); - engine->last_context = request->ctx; + if (ctx != dev_priv->kernel_context) { + intel_lr_context_pin(ctx, engine); + engine->last_context = ctx; } else { engine->last_context = NULL; } @@ -948,9 +948,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, struct drm_i915_gem_execbuffer2 *args, struct list_head *vmas) { - struct drm_device *dev = params->dev; struct intel_engine_cs *engine = params->engine; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(params->dev); struct intel_ringbuffer *ringbuf = params->ctx->engine[engine->id].ringbuf; u64 exec_start; int instp_mode; @@ -1024,6 +1023,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, void intel_execlists_retire_requests(struct intel_engine_cs *engine) { + struct intel_context *dctx = to_i915(engine->dev)->kernel_context; struct drm_i915_gem_request *req, *tmp; struct list_head retired_list; @@ -1038,10 +1038,8 @@ void intel_execlists_retire_requests(struct intel_engine_cs *engine) list_for_each_entry_safe(req, tmp, &retired_list, execlist_link) { struct intel_context *ctx = req->ctx; - struct drm_i915_gem_object *ctx_obj = - ctx->engine[engine->id].state; - if (ctx_obj && (ctx != req->i915->kernel_context)) + if (ctx != dctx && ctx->engine[engine->id].state) intel_lr_context_unpin(ctx, engine); list_del(&req->execlist_link); @@ -1091,17 +1089,18 @@ static int intel_lr_context_do_pin(struct intel_context *ctx, struct intel_engine_cs *engine) { struct drm_device *dev = engine->dev; - struct drm_i915_private *dev_priv = dev->dev_private; - struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state; - struct intel_ringbuffer *ringbuf = ctx->engine[engine->id].ringbuf; + struct drm_i915_private *dev_priv = ctx->i915; + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; + struct drm_i915_gem_object *ctx_obj = ectx->state; + struct intel_ringbuffer *ringbuf = ectx->ringbuf; struct page *lrc_state_page; uint32_t *lrc_reg_state; int ret; - WARN_ON(!mutex_is_locked(&engine->dev->struct_mutex)); + WARN_ON(!mutex_is_locked(&dev->struct_mutex)); ret = i915_gem_obj_ggtt_pin(ctx_obj, GEN8_LR_CONTEXT_ALIGN, - PIN_OFFSET_BIAS | GUC_WOPCM_TOP); + PIN_OFFSET_BIAS | GUC_WOPCM_TOP); if (ret) return ret; @@ -1111,15 +1110,15 @@ static int intel_lr_context_do_pin(struct intel_context *ctx, goto unpin_ctx_obj; } - ret = intel_pin_and_map_ringbuffer_obj(engine->dev, ringbuf); + ret = intel_pin_and_map_ringbuffer_obj(dev, ringbuf); if (ret) goto unpin_ctx_obj; - ctx->engine[engine->id].lrc_vma = i915_gem_obj_to_ggtt(ctx_obj); + ectx->lrc_vma = i915_gem_obj_to_ggtt(ctx_obj); intel_lr_context_descriptor_update(ctx, engine); lrc_reg_state = kmap(lrc_state_page); lrc_reg_state[CTX_RING_BUFFER_START+1] = ringbuf->vma->node.start; - ctx->engine[engine->id].lrc_reg_state = lrc_reg_state; + ectx->lrc_reg_state = lrc_reg_state; ctx_obj->dirty = true; /* Invalidate GuC TLB. */ @@ -1137,9 +1136,10 @@ static int intel_lr_context_do_pin(struct intel_context *ctx, static int intel_lr_context_pin(struct intel_context *ctx, struct intel_engine_cs *engine) { + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; int ret = 0; - if (ctx->engine[engine->id].pin_count++ == 0) { + if (ectx->pin_count++ == 0) { ret = intel_lr_context_do_pin(ctx, engine); if (ret) goto reset_pin_count; @@ -1149,23 +1149,24 @@ static int intel_lr_context_pin(struct intel_context *ctx, return ret; reset_pin_count: - ctx->engine[engine->id].pin_count = 0; + ectx->pin_count = 0; return ret; } void intel_lr_context_unpin(struct intel_context *ctx, struct intel_engine_cs *engine) { - struct drm_i915_gem_object *ctx_obj = ctx->engine[engine->id].state; + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; WARN_ON(!mutex_is_locked(&ctx->i915->dev->struct_mutex)); - if (--ctx->engine[engine->id].pin_count == 0) { - kunmap(kmap_to_page(ctx->engine[engine->id].lrc_reg_state)); - intel_unpin_ringbuffer_obj(ctx->engine[engine->id].ringbuf); - i915_gem_object_ggtt_unpin(ctx_obj); - ctx->engine[engine->id].lrc_vma = NULL; - ctx->engine[engine->id].lrc_desc = 0; - ctx->engine[engine->id].lrc_reg_state = NULL; + + if (--ectx->pin_count == 0) { + intel_unpin_ringbuffer_obj(ectx->ringbuf); + i915_gem_object_ggtt_unpin(ectx->state); + kunmap(kmap_to_page(ectx->lrc_reg_state)); + ectx->lrc_reg_state = NULL; + ectx->lrc_vma = NULL; + ectx->lrc_desc = 0; i915_gem_context_unreference(ctx); } @@ -1176,8 +1177,7 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req) int ret, i; struct intel_engine_cs *engine = req->engine; struct intel_ringbuffer *ringbuf = req->ringbuf; - struct drm_device *dev = engine->dev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = req->i915; struct i915_workarounds *w = &dev_priv->workarounds; if (w->count == 0) @@ -1563,12 +1563,11 @@ static int intel_init_workaround_bb(struct intel_engine_cs *engine) static int gen8_init_common_ring(struct intel_engine_cs *engine) { - struct drm_device *dev = engine->dev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = engine->dev->dev_private; + struct intel_engine_ctx *ectx = &dev_priv->kernel_context->engine[engine->id]; unsigned int next_context_status_buffer_hw; - lrc_setup_hardware_status_page(engine, - dev_priv->kernel_context->engine[engine->id].state); + lrc_setup_hardware_status_page(engine, ectx->state); I915_WRITE_IMR(engine, ~(engine->irq_enable_mask | engine->irq_keep_mask)); @@ -2512,8 +2511,9 @@ void intel_lr_context_free(struct intel_context *ctx) int i; for (i = I915_NUM_ENGINES; --i >= 0; ) { - struct intel_ringbuffer *ringbuf = ctx->engine[i].ringbuf; - struct drm_i915_gem_object *ctx_obj = ctx->engine[i].state; + struct intel_engine_ctx *ectx = &ctx->engine[i]; + struct intel_ringbuffer *ringbuf = ectx->ringbuf; + struct drm_i915_gem_object *ctx_obj = ectx->state; if (!ctx_obj) continue; @@ -2523,7 +2523,7 @@ void intel_lr_context_free(struct intel_context *ctx) i915_gem_object_ggtt_unpin(ctx_obj); } - WARN_ON(ctx->engine[i].pin_count); + WARN_ON(ectx->pin_count); intel_ringbuffer_free(ringbuf); drm_gem_object_unreference(&ctx_obj->base); } @@ -2603,13 +2603,14 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx, struct intel_engine_cs *engine) { struct drm_device *dev = engine->dev; + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; struct drm_i915_gem_object *ctx_obj; uint32_t context_size; struct intel_ringbuffer *ringbuf; int ret; WARN_ON(ctx->legacy_hw_ctx.rcs_state != NULL); - WARN_ON(ctx->engine[engine->id].state); + WARN_ON(ectx->state); context_size = round_up(intel_lr_context_size(engine), 4096); @@ -2634,8 +2635,8 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx, goto error_ringbuf; } - ctx->engine[engine->id].ringbuf = ringbuf; - ctx->engine[engine->id].state = ctx_obj; + ectx->ringbuf = ringbuf; + ectx->state = ctx_obj; if (ctx != ctx->i915->kernel_context && engine->init_context) { struct drm_i915_gem_request *req; @@ -2662,23 +2663,22 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx, intel_ringbuffer_free(ringbuf); error_deref_obj: drm_gem_object_unreference(&ctx_obj->base); - ctx->engine[engine->id].ringbuf = NULL; - ctx->engine[engine->id].state = NULL; + ectx->ringbuf = NULL; + ectx->state = NULL; return ret; } void intel_lr_context_reset(struct drm_device *dev, - struct intel_context *ctx) + struct intel_context *ctx) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_engine_cs *engine; int i; for_each_engine(engine, dev_priv, i) { - struct drm_i915_gem_object *ctx_obj = - ctx->engine[engine->id].state; - struct intel_ringbuffer *ringbuf = - ctx->engine[engine->id].ringbuf; + struct intel_engine_ctx *ectx = &ctx->engine[engine->id]; + struct drm_i915_gem_object *ctx_obj = ectx->state; + struct intel_ringbuffer *ringbuf = ectx->ringbuf; uint32_t *reg_state; struct page *page;