From patchwork Mon Apr 28 15:22:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bradley.d.volkin@intel.com X-Patchwork-Id: 4079211 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.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2076FBFF02 for ; Mon, 28 Apr 2014 15:22:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 00495201F9 for ; Mon, 28 Apr 2014 15:22:13 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id 9DEF3201EF for ; Mon, 28 Apr 2014 15:22:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id BAEA36E88A; Mon, 28 Apr 2014 08:22:10 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 25E356E88A for ; Mon, 28 Apr 2014 08:22:09 -0700 (PDT) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga102.jf.intel.com with ESMTP; 28 Apr 2014 08:16:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.97,944,1389772800"; d="scan'208";a="502339794" Received: from bdvolkin-cube.ra.intel.com (HELO bdvolkin-ubuntu-desktop.ra.intel.com) ([10.10.34.187]) by orsmga001.jf.intel.com with ESMTP; 28 Apr 2014 08:21:51 -0700 From: bradley.d.volkin@intel.com To: intel-gfx@lists.freedesktop.org Date: Mon, 28 Apr 2014 08:22:08 -0700 Message-Id: <1398698528-25256-1-git-send-email-bradley.d.volkin@intel.com> X-Mailer: git-send-email 1.8.3.2 Subject: [Intel-gfx] [PATCH] drm/i915: Use hash tables for the command parser X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 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.8 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 From: Brad Volkin For clients that submit large batch buffers the command parser has a substantial impact on performance. On my HSW ULT system performance drops as much as ~20% on some tests. Most of the time is spent in the command lookup code. Converting that from the current naive search to a hash table lookup reduces the performance impact by as much as ~10%. The choice of value for I915_CMD_HASH_ORDER allows all commands currently used in the parser tables to hash to their own bucket (except for one collision on the render ring). The tradeoff is that it wastes memory. Because the opcodes for the commands in the tables are not particularly well distributed, reducing the order still leaves many buckets empty. The increased collisions don't seem to have a huge impact on the performance gain, but for now anyhow, the parser trades memory for performance. Signed-off-by: Brad Volkin --- drivers/gpu/drm/i915/i915_cmd_parser.c | 136 ++++++++++++++++++++++++-------- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 2 + drivers/gpu/drm/i915/intel_ringbuffer.h | 11 ++- 4 files changed, 116 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c index 9bac097..9dca899 100644 --- a/drivers/gpu/drm/i915/i915_cmd_parser.c +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -498,16 +498,18 @@ static u32 gen7_blt_get_cmd_length_mask(u32 cmd_header) return 0; } -static bool validate_cmds_sorted(struct intel_ring_buffer *ring) +static bool validate_cmds_sorted(struct intel_ring_buffer *ring, + const struct drm_i915_cmd_table *cmd_tables, + int cmd_table_count) { int i; bool ret = true; - if (!ring->cmd_tables || ring->cmd_table_count == 0) + if (!cmd_tables || cmd_table_count == 0) return true; - for (i = 0; i < ring->cmd_table_count; i++) { - const struct drm_i915_cmd_table *table = &ring->cmd_tables[i]; + for (i = 0; i < cmd_table_count; i++) { + const struct drm_i915_cmd_table *table = &cmd_tables[i]; u32 previous = 0; int j; @@ -557,6 +559,60 @@ static bool validate_regs_sorted(struct intel_ring_buffer *ring) ring->master_reg_count); } +struct cmd_node { + const struct drm_i915_cmd_descriptor *desc; + struct hlist_node node; +}; + +/* + * Different command ranges have different numbers of bits for the opcode. + * In order to use the opcode bits, and only the opcode bits, for the hash key + * we should use the MI_* command opcode mask (since those commands use the + * fewest bits for the opcode.) + */ +#define CMD_HASH_MASK STD_MI_OPCODE_MASK + +static int init_hash_table(struct intel_ring_buffer *ring, + const struct drm_i915_cmd_table *cmd_tables, + int cmd_table_count) +{ + int i, j; + + hash_init(ring->cmd_hash); + + for (i = 0; i < cmd_table_count; i++) { + const struct drm_i915_cmd_table *table = &cmd_tables[i]; + + for (j = 0; j < table->count; j++) { + const struct drm_i915_cmd_descriptor *desc = + &table->table[j]; + struct cmd_node *desc_node = + kmalloc(sizeof(*desc_node), GFP_KERNEL); + + if (!desc_node) + return -ENOMEM; + + desc_node->desc = desc; + hash_add(ring->cmd_hash, &desc_node->node, + desc->cmd.value & CMD_HASH_MASK); + } + } + + return 0; +} + +static void fini_hash_table(struct intel_ring_buffer *ring) +{ + struct hlist_node *tmp; + struct cmd_node *desc_node; + int i; + + hash_for_each_safe(ring->cmd_hash, i, tmp, desc_node, node) { + hash_del(&desc_node->node); + kfree(desc_node); + } +} + /** * i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer * @ring: the ringbuffer to initialize @@ -567,18 +623,21 @@ static bool validate_regs_sorted(struct intel_ring_buffer *ring) */ void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) { + const struct drm_i915_cmd_table *cmd_tables; + int cmd_table_count; + if (!IS_GEN7(ring->dev)) return; switch (ring->id) { case RCS: if (IS_HASWELL(ring->dev)) { - ring->cmd_tables = hsw_render_ring_cmds; - ring->cmd_table_count = + cmd_tables = hsw_render_ring_cmds; + cmd_table_count = ARRAY_SIZE(hsw_render_ring_cmds); } else { - ring->cmd_tables = gen7_render_cmds; - ring->cmd_table_count = ARRAY_SIZE(gen7_render_cmds); + cmd_tables = gen7_render_cmds; + cmd_table_count = ARRAY_SIZE(gen7_render_cmds); } ring->reg_table = gen7_render_regs; @@ -595,17 +654,17 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) ring->get_cmd_length_mask = gen7_render_get_cmd_length_mask; break; case VCS: - ring->cmd_tables = gen7_video_cmds; - ring->cmd_table_count = ARRAY_SIZE(gen7_video_cmds); + cmd_tables = gen7_video_cmds; + cmd_table_count = ARRAY_SIZE(gen7_video_cmds); ring->get_cmd_length_mask = gen7_bsd_get_cmd_length_mask; break; case BCS: if (IS_HASWELL(ring->dev)) { - ring->cmd_tables = hsw_blt_ring_cmds; - ring->cmd_table_count = ARRAY_SIZE(hsw_blt_ring_cmds); + cmd_tables = hsw_blt_ring_cmds; + cmd_table_count = ARRAY_SIZE(hsw_blt_ring_cmds); } else { - ring->cmd_tables = gen7_blt_cmds; - ring->cmd_table_count = ARRAY_SIZE(gen7_blt_cmds); + cmd_tables = gen7_blt_cmds; + cmd_table_count = ARRAY_SIZE(gen7_blt_cmds); } ring->reg_table = gen7_blt_regs; @@ -622,8 +681,8 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) ring->get_cmd_length_mask = gen7_blt_get_cmd_length_mask; break; case VECS: - ring->cmd_tables = hsw_vebox_cmds; - ring->cmd_table_count = ARRAY_SIZE(hsw_vebox_cmds); + cmd_tables = hsw_vebox_cmds; + cmd_table_count = ARRAY_SIZE(hsw_vebox_cmds); /* VECS can use the same length_mask function as VCS */ ring->get_cmd_length_mask = gen7_bsd_get_cmd_length_mask; break; @@ -633,18 +692,38 @@ void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) BUG(); } - BUG_ON(!validate_cmds_sorted(ring)); + BUG_ON(!validate_cmds_sorted(ring, cmd_tables, cmd_table_count)); BUG_ON(!validate_regs_sorted(ring)); + + BUG_ON(init_hash_table(ring, cmd_tables, cmd_table_count)); + + ring->needs_cmd_parser = true; +} + +/** + * i915_cmd_parser_fini_ring() - clean up cmd parser related fields + * @ring: the ringbuffer to clean up + * + * Releases any resources related to command parsing that may have been + * initialized for the specified ring. + */ +void i915_cmd_parser_fini_ring(struct intel_ring_buffer *ring) +{ + if (!ring->needs_cmd_parser) + return; + + fini_hash_table(ring); } static const struct drm_i915_cmd_descriptor* -find_cmd_in_table(const struct drm_i915_cmd_table *table, +find_cmd_in_table(struct intel_ring_buffer *ring, u32 cmd_header) { - int i; + struct cmd_node *desc_node; - for (i = 0; i < table->count; i++) { - const struct drm_i915_cmd_descriptor *desc = &table->table[i]; + hash_for_each_possible(ring->cmd_hash, desc_node, node, + cmd_header & CMD_HASH_MASK) { + const struct drm_i915_cmd_descriptor *desc = desc_node->desc; u32 masked_cmd = desc->cmd.mask & cmd_header; u32 masked_value = desc->cmd.value & desc->cmd.mask; @@ -668,16 +747,12 @@ find_cmd(struct intel_ring_buffer *ring, u32 cmd_header, struct drm_i915_cmd_descriptor *default_desc) { + const struct drm_i915_cmd_descriptor *desc; u32 mask; - int i; - - for (i = 0; i < ring->cmd_table_count; i++) { - const struct drm_i915_cmd_descriptor *desc; - desc = find_cmd_in_table(&ring->cmd_tables[i], cmd_header); - if (desc) - return desc; - } + desc = find_cmd_in_table(ring, cmd_header); + if (desc) + return desc; mask = ring->get_cmd_length_mask(cmd_header); if (!mask) @@ -748,8 +823,7 @@ bool i915_needs_cmd_parser(struct intel_ring_buffer *ring) { struct drm_i915_private *dev_priv = ring->dev->dev_private; - /* No command tables indicates a platform without parsing */ - if (!ring->cmd_tables) + if (!ring->needs_cmd_parser) return false; /* diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 7d6acb4..8c8388f 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2414,6 +2414,7 @@ const char *i915_cache_level_str(int type); /* i915_cmd_parser.c */ int i915_cmd_parser_get_version(void); void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring); +void i915_cmd_parser_fini_ring(struct intel_ring_buffer *ring); bool i915_needs_cmd_parser(struct intel_ring_buffer *ring); int i915_parse_cmds(struct intel_ring_buffer *ring, struct drm_i915_gem_object *batch_obj, diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index eb3dd26..06fa9a3 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1469,6 +1469,8 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring) ring->cleanup(ring); cleanup_status_page(ring); + + i915_cmd_parser_fini_ring(ring); } static int intel_ring_wait_request(struct intel_ring_buffer *ring, int n) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 413cdc7..48daa91 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -1,6 +1,10 @@ #ifndef _INTEL_RINGBUFFER_H_ #define _INTEL_RINGBUFFER_H_ +#include + +#define I915_CMD_HASH_ORDER 9 + /* * Gen2 BSpec "1. Programming Environment" / 1.4.4.6 "Ring Buffer Use" * Gen3 BSpec "vol1c Memory Interface Functions" / 2.3.4.5 "Ring Buffer Use" @@ -164,12 +168,13 @@ struct intel_ring_buffer { volatile u32 *cpu_page; } scratch; + bool needs_cmd_parser; + /* - * Tables of commands the command parser needs to know about + * Table of commands the command parser needs to know about * for this ring. */ - const struct drm_i915_cmd_table *cmd_tables; - int cmd_table_count; + DECLARE_HASHTABLE(cmd_hash, I915_CMD_HASH_ORDER); /* * Table of registers allowed in commands that read/write registers.