Re: [PATCH] drm/i915: Use hash tables for the command parser

[Damien Lespiau <damien.lespiau@intel.com>]

On Thu, May 08, 2014 at 08:53:38AM -0700, Volkin, Bradley D wrote:
> On Thu, May 08, 2014 at 06:15:44AM -0700, Lespiau, Damien wrote:
> > On Thu, May 08, 2014 at 02:05:07PM +0100, Damien Lespiau wrote:
> > > On Mon, Apr 28, 2014 at 08:22:08AM -0700, bradley.d.volkin@intel.com wrote:
> > > > From: Brad Volkin <bradley.d.volkin@intel.com>
> > > > +/*
> > > > + * 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
> > > 
> > > This is not very convicing (but could well be correct).
> > > 
> > > #define STD_MI_OPCODE_MASK  0xFF800000
> > > #define STD_3D_OPCODE_MASK  0xFFFF0000
> > > 
> > > So it only works if the 3D opcodes have the top 9 bits all distinct?
> > 
> > To expand on that, with the attached program:
> > 
> > $ ./minimal-hash-hsw-render  | wc -l
> > 44
> > 
> > $ ./minimal-hash-hsw-render  | sort -u | wc -l
> > 37
> 
> Yes, as it's currently written, some commands may hash to the same
> bucket. The per-bucket search will use the full mask from the cmd
> descriptor to get an exact match.
> 
> The issue is that, for example, MI commands in a batch may use bits
> 22:16 for something other than the opcode (e.g. GGTT vs PPGTT). If we
> mask a command from a batch with 0xFFFF0000 then an MI command may hash
> to the wrong bucket. If we want a perfect hash then I suppose we should
> look at bits 31:29 and mask with the exact STD_xx_OPCODE_MASK for the
> client. The existing INSTR_CLIENT_MASK/SHIFT defines could be reused
> for that.

Ah that works then, not super convinced that's the best way we can do
it, but it seems to be an improvement, so well... can't argue with that.

-- 
Damien