From patchwork Fri Nov 15 15:52:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 11246633 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 9C30413BD for ; Fri, 15 Nov 2019 15:53:08 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 84D102073A for ; Fri, 15 Nov 2019 15:53:08 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 84D102073A Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=chris-wilson.co.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=intel-gfx-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id EB75B8920C; Fri, 15 Nov 2019 15:53:07 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id 372378920C for ; Fri, 15 Nov 2019 15:53:06 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 19215430-1500050 for multiple; Fri, 15 Nov 2019 15:52:30 +0000 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Fri, 15 Nov 2019 15:52:28 +0000 Message-Id: <20191115155228.883523-1-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.24.0 In-Reply-To: <20191115121712.809037-1-chris@chris-wilson.co.uk> References: <20191115121712.809037-1-chris@chris-wilson.co.uk> MIME-Version: 1.0 Subject: [Intel-gfx] [PATCH v2] drm/i915/gem: Excise the per-batch whitelist from the context X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" One does not lightly add a new hidden struct_mutex dependency deep within the execbuf bowels! The immediate suspicion in seeing the whitelist cached on the context, is that it is intended to be preserved between batches, as the kernel is quite adept at caching small allocations itself. But no, it's sole purpose is to serialise command submission in order to save a kmalloc on a slow, slow path! Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Joonas Lahtinen --- Don't set_bit(NULL) on gen7! --- drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 -- .../gpu/drm/i915/gem/i915_gem_context_types.h | 7 --- drivers/gpu/drm/i915/i915_cmd_parser.c | 60 +++++++------------ 3 files changed, 21 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c index 1284f47303fa..17f395672e5e 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c @@ -274,8 +274,6 @@ static void i915_gem_context_free(struct i915_gem_context *ctx) free_engines(rcu_access_pointer(ctx->engines)); mutex_destroy(&ctx->engines_mutex); - kfree(ctx->jump_whitelist); - if (ctx->timeline) intel_timeline_put(ctx->timeline); @@ -583,9 +581,6 @@ __create_context(struct drm_i915_private *i915) for (i = 0; i < ARRAY_SIZE(ctx->hang_timestamp); i++) ctx->hang_timestamp[i] = jiffies - CONTEXT_FAST_HANG_JIFFIES; - ctx->jump_whitelist = NULL; - ctx->jump_whitelist_cmds = 0; - spin_lock(&i915->gem.contexts.lock); list_add_tail(&ctx->link, &i915->gem.contexts.list); spin_unlock(&i915->gem.contexts.lock); diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h index c060bc428f49..69df5459c350 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_context_types.h +++ b/drivers/gpu/drm/i915/gem/i915_gem_context_types.h @@ -168,13 +168,6 @@ struct i915_gem_context { */ struct radix_tree_root handles_vma; - /** jump_whitelist: Bit array for tracking cmds during cmdparsing - * Guarded by struct_mutex - */ - unsigned long *jump_whitelist; - /** jump_whitelist_cmds: No of cmd slots available */ - u32 jump_whitelist_cmds; - /** * @name: arbitrary name, used for user debug * diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index f24096e27bef..ded2c1ed0f7e 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -1310,7 +1310,8 @@ static int check_bbstart(const struct i915_gem_context *ctx, u32 *cmd, u32 offset, u32 length, u32 batch_len, u64 batch_start, - u64 shadow_batch_start) + u64 shadow_batch_start, + unsigned long *jump_whitelist) { u64 jump_offset, jump_target; u32 target_cmd_offset, target_cmd_index; @@ -1352,10 +1353,7 @@ static int check_bbstart(const struct i915_gem_context *ctx, if (target_cmd_index == offset) return 0; - if (ctx->jump_whitelist_cmds <= target_cmd_index) { - DRM_DEBUG("CMD: Rejecting BB_START - truncated whitelist array\n"); - return -EINVAL; - } else if (!test_bit(target_cmd_index, ctx->jump_whitelist)) { + if (!test_bit(target_cmd_index, jump_whitelist)) { DRM_DEBUG("CMD: BB_START to 0x%llx not a previously executed cmd\n", jump_target); return -EINVAL; @@ -1364,40 +1362,19 @@ static int check_bbstart(const struct i915_gem_context *ctx, return 0; } -static void init_whitelist(struct i915_gem_context *ctx, u32 batch_len) +static unsigned long * +alloc_whitelist(struct drm_i915_private *i915, u32 batch_len) { - const u32 batch_cmds = DIV_ROUND_UP(batch_len, sizeof(u32)); - const u32 exact_size = BITS_TO_LONGS(batch_cmds); - u32 next_size = BITS_TO_LONGS(roundup_pow_of_two(batch_cmds)); - unsigned long *next_whitelist; - - if (CMDPARSER_USES_GGTT(ctx->i915)) - return; - - if (batch_cmds <= ctx->jump_whitelist_cmds) { - bitmap_zero(ctx->jump_whitelist, batch_cmds); - return; - } + unsigned long *jmp; -again: - next_whitelist = kcalloc(next_size, sizeof(long), GFP_KERNEL); - if (next_whitelist) { - kfree(ctx->jump_whitelist); - ctx->jump_whitelist = next_whitelist; - ctx->jump_whitelist_cmds = - next_size * BITS_PER_BYTE * sizeof(long); - return; - } - - if (next_size > exact_size) { - next_size = exact_size; - goto again; - } + if (CMDPARSER_USES_GGTT(i915)) + return NULL; - DRM_DEBUG("CMD: Failed to extend whitelist. BB_START may be disallowed\n"); - bitmap_zero(ctx->jump_whitelist, ctx->jump_whitelist_cmds); + jmp = bitmap_zalloc(DIV_ROUND_UP(batch_len, sizeof(u32)), GFP_KERNEL); + if (!jmp) + return ERR_PTR(-ENOMEM); - return; + return jmp; } #define LENGTH_BIAS 2 @@ -1433,6 +1410,7 @@ int intel_engine_cmd_parser(struct i915_gem_context *ctx, struct drm_i915_cmd_descriptor default_desc = noop_desc; const struct drm_i915_cmd_descriptor *desc = &default_desc; bool needs_clflush_after = false; + unsigned long *jump_whitelist; int ret = 0; cmd = copy_batch(shadow_batch_obj, batch_obj, @@ -1443,7 +1421,9 @@ int intel_engine_cmd_parser(struct i915_gem_context *ctx, return PTR_ERR(cmd); } - init_whitelist(ctx, batch_len); + jump_whitelist = alloc_whitelist(ctx->i915, batch_len); + if (IS_ERR(jump_whitelist)) + return PTR_ERR(jump_whitelist); /* * We use the batch length as size because the shadow object is as @@ -1487,15 +1467,16 @@ int intel_engine_cmd_parser(struct i915_gem_context *ctx, if (desc->cmd.value == MI_BATCH_BUFFER_START) { ret = check_bbstart(ctx, cmd, offset, length, batch_len, batch_start, - shadow_batch_start); + shadow_batch_start, + jump_whitelist); if (ret) goto err; break; } - if (ctx->jump_whitelist_cmds > offset) - set_bit(offset, ctx->jump_whitelist); + if (jump_whitelist) + set_bit(offset, jump_whitelist); cmd += length; offset += length; @@ -1513,6 +1494,7 @@ int intel_engine_cmd_parser(struct i915_gem_context *ctx, } err: + kfree(jump_whitelist); i915_gem_object_unpin_map(shadow_batch_obj); return ret; }