drm/i915/cmdparser: Improve hash function

The existing code's hashfunction is very suboptimal (most 3D commands
use the same bucket degrading the hash to a long list). The code even
acknowledge that the issue was known and the fix simple:

/*
 * If we attempt to generate a perfect hash, we should be able to look at bits
 * 31:29 of a command from a batch buffer and use the full mask for that
 * client. The existing INSTR_CLIENT_MASK/SHIFT defines can be used for this.
 */

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20160818161718.27187-35-chris@chris-wilson.co.uk

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index d1858f80d64c6f6d854f54b646659a42bae2169d..54aeab3f961469af66ff5e712bc05b0f01452e90 100644 (file)
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -86,24 +86,24 @@
  * general bitmasking mechanism.
  */
 
-#define STD_MI_OPCODE_MASK  0xFF800000
-#define STD_3D_OPCODE_MASK  0xFFFF0000
-#define STD_2D_OPCODE_MASK  0xFFC00000
-#define STD_MFX_OPCODE_MASK 0xFFFF0000
+#define STD_MI_OPCODE_SHIFT  (32 - 9)
+#define STD_3D_OPCODE_SHIFT  (32 - 16)
+#define STD_2D_OPCODE_SHIFT  (32 - 10)
+#define STD_MFX_OPCODE_SHIFT (32 - 16)
 
 #define CMD(op, opm, f, lm, fl, ...)                           \
        {                                                       \
                .flags = (fl) | ((f) ? CMD_DESC_FIXED : 0),     \
-               .cmd = { (op), (opm) },                         \
+               .cmd = { (op), ~0u << (opm) },                  \
                .length = { (lm) },                             \
                __VA_ARGS__                                     \
        }
 
 /* Convenience macros to compress the tables */
-#define SMI STD_MI_OPCODE_MASK
-#define S3D STD_3D_OPCODE_MASK
-#define S2D STD_2D_OPCODE_MASK
-#define SMFX STD_MFX_OPCODE_MASK
+#define SMI STD_MI_OPCODE_SHIFT
+#define S3D STD_3D_OPCODE_SHIFT
+#define S2D STD_2D_OPCODE_SHIFT
+#define SMFX STD_MFX_OPCODE_SHIFT
 #define F true
 #define S CMD_DESC_SKIP
 #define R CMD_DESC_REJECT
@@ -696,12 +696,26 @@ struct cmd_node {
  * non-opcode bits being set. But if we don't include those bits, some 3D
  * commands may hash to the same bucket due to not including opcode bits that
  * make the command unique. For now, we will risk hashing to the same bucket.
- *
- * If we attempt to generate a perfect hash, we should be able to look at bits
- * 31:29 of a command from a batch buffer and use the full mask for that
- * client. The existing INSTR_CLIENT_MASK/SHIFT defines can be used for this.
  */
-#define CMD_HASH_MASK STD_MI_OPCODE_MASK
+static inline u32 cmd_header_key(u32 x)
+{
+       u32 shift;
+
+       switch (x >> INSTR_CLIENT_SHIFT) {
+       default:
+       case INSTR_MI_CLIENT:
+               shift = STD_MI_OPCODE_SHIFT;
+               break;
+       case INSTR_RC_CLIENT:
+               shift = STD_3D_OPCODE_SHIFT;
+               break;
+       case INSTR_BC_CLIENT:
+               shift = STD_2D_OPCODE_SHIFT;
+               break;
+       }
+
+       return x >> shift;
+}
 
 static int init_hash_table(struct intel_engine_cs *engine,
                           const struct drm_i915_cmd_table *cmd_tables,
@@ -725,7 +739,7 @@ static int init_hash_table(struct intel_engine_cs *engine,
 
                        desc_node->desc = desc;
                        hash_add(engine->cmd_hash, &desc_node->node,
-                                desc->cmd.value & CMD_HASH_MASK);
+                                cmd_header_key(desc->cmd.value));
                }
        }
 
@@ -859,12 +873,9 @@ find_cmd_in_table(struct intel_engine_cs *engine,
        struct cmd_node *desc_node;
 
        hash_for_each_possible(engine->cmd_hash, desc_node, node,
-                              cmd_header & CMD_HASH_MASK) {
+                              cmd_header_key(cmd_header)) {
                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;
-
-               if (masked_cmd == masked_value)
+               if (((cmd_header ^ desc->cmd.value) & desc->cmd.mask) == 0)
                        return desc;
        }