[Intel-gfx] [PATCH 10/13] drm/i915: Enable PPGTT command parser checks

[Volkin, Bradley D]

[snip]

On Wed, Feb 05, 2014 at 07:37:51AM -0800, Jani Nikula wrote:
> On Wed, 29 Jan 2014, bradley.d.volkin at intel.com wrote:
> >  int i915_needs_cmd_parser(struct intel_ring_buffer *ring)
> >  {
> > +	drm_i915_private_t *dev_priv =
> > +		(drm_i915_private_t *)ring->dev->dev_private;
> > +
> >  	/* No command tables indicates a platform without parsing */
> >  	if (!ring->cmd_tables)
> >  		return 0;
> >  
> > +	/*
> > +	 * XXX: VLV is Gen7 and therefore has cmd_tables, but has PPGTT
> > +	 * disabled. That will cause all of the parser's PPGTT checks to
> > +	 * fail. For now, disable parsing when PPGTT is off.
> > +	 */
> > +	if(!dev_priv->mm.aliasing_ppgtt)
>    	  ^ missing space.

Oops

> 
> > +		return 0;
> > +
> 
> Hmm, shouldn't this belong to some other patch, much earlier in the
> series? Like patch 2 or 3?

Not necessarily. It's only because we've added the PPGTT checks without
somehow making them conditional on aliasing_ppgtt==true that we have a problem,
and that only happens with this patch. The parser works, though is less useful,
on !aliasing_ppgtt platforms up to this point.

Chris suggested that we just fix it up so that the PPGTT checks are conditional
on PPGTT actually enabled, so I'm going to look at that.
- Brad

> 
> >  	return i915.enable_cmd_parser;
> >  }
> >  
> > @@ -675,6 +782,16 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,
> >  				u32 dword = cmd[desc->bits[i].offset] &
> >  					desc->bits[i].mask;
> >  
> > +				if (desc->bits[i].condition_mask != 0) {
> > +					u32 offset =
> > +						desc->bits[i].condition_offset;
> > +					u32 condition = cmd[offset] &
> > +						desc->bits[i].condition_mask;
> > +
> > +					if (condition == 0)
> > +						continue;
> > +				}
> > +
> >  				if (dword != desc->bits[i].expected) {
> >  					DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (ring=%d)\n",
> >  							 *cmd,
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 8aed80f..2d1d2ef 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1829,11 +1829,17 @@ struct drm_i915_cmd_descriptor {
> >  	 * compared against an expected value. If the command does not match
> >  	 * the expected value, the parser rejects it. Only valid if flags has
> >  	 * the CMD_DESC_BITMASK bit set.
> > +	 *
> > +	 * If the check specifies a non-zero condition_mask then the parser
> > +	 * only performs the check when the bits specified by condition_mask
> > +	 * are non-zero.
> >  	 */
> >  	struct {
> >  		u32 offset;
> >  		u32 mask;
> >  		u32 expected;
> > +		u32 condition_offset;
> > +		u32 condition_mask;
> >  	} bits[MAX_CMD_DESC_BITMASKS];
> >  	/** Number of valid entries in the bits array */
> >  	int bits_count;
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index c2e4898..ff263f4 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -179,6 +179,8 @@
> >   * Memory interface instructions used by the kernel
> >   */
> >  #define MI_INSTR(opcode, flags) (((opcode) << 23) | (flags))
> > +/* Many MI commands use bit 22 of the header dword for GGTT vs PPGTT */
> > +#define  MI_GLOBAL_GTT    (1<<22)
> >  
> >  #define MI_NOOP			MI_INSTR(0, 0)
> >  #define MI_USER_INTERRUPT	MI_INSTR(0x02, 0)
> > @@ -258,6 +260,7 @@
> >  #define   MI_FLUSH_DW_STORE_INDEX	(1<<21)
> >  #define   MI_INVALIDATE_TLB		(1<<18)
> >  #define   MI_FLUSH_DW_OP_STOREDW	(1<<14)
> > +#define   MI_FLUSH_DW_OP_MASK		(3<<14)
> >  #define   MI_FLUSH_DW_NOTIFY		(1<<8)
> >  #define   MI_INVALIDATE_BSD		(1<<7)
> >  #define   MI_FLUSH_DW_USE_GTT		(1<<2)
> > @@ -324,6 +327,7 @@
> >  #define   PIPE_CONTROL_CS_STALL				(1<<20)
> >  #define   PIPE_CONTROL_TLB_INVALIDATE			(1<<18)
> >  #define   PIPE_CONTROL_QW_WRITE				(1<<14)
> > +#define   PIPE_CONTROL_POST_SYNC_OP_MASK                (3<<14)
> >  #define   PIPE_CONTROL_DEPTH_STALL			(1<<13)
> >  #define   PIPE_CONTROL_WRITE_FLUSH			(1<<12)
> >  #define   PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH	(1<<12) /* gen6+ */
> > @@ -352,6 +356,8 @@
> >  #define MI_URB_CLEAR            MI_INSTR(0x19, 0)
> >  #define MI_UPDATE_GTT           MI_INSTR(0x23, 0)
> >  #define MI_CLFLUSH              MI_INSTR(0x27, 0)
> > +#define MI_REPORT_PERF_COUNT    MI_INSTR(0x28, 0)
> > +#define   MI_REPORT_PERF_COUNT_GGTT (1<<0)
> >  #define MI_LOAD_REGISTER_MEM    MI_INSTR(0x29, 0)
> >  #define MI_LOAD_REGISTER_REG    MI_INSTR(0x2A, 0)
> >  #define MI_RS_STORE_DATA_IMM    MI_INSTR(0x2B, 0)
> > -- 
> > 1.8.5.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center