Re: [PATCH] drm/i915: Add more registers to the whitelist for mesa

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Apr 09, 2014 at 09:22:19AM -0700, Kenneth Graunke wrote:
> On 04/08/2014 02:18 PM, bradley.d.volkin@intel.com wrote:
> > From: Brad Volkin <bradley.d.volkin@intel.com>
> > 
> > These are additional registers needed for performance monitoring and
> > ARB_draw_indirect extensions in mesa.
> 
> Whoops...I totally missed this.  Thanks!
> 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76719
> > Cc: Kenneth Graunke <kenneth@whitecape.org>
> > Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_cmd_parser.c | 9 +++++++++
> >  drivers/gpu/drm/i915/i915_reg.h        | 8 ++++++++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > index 29184d6..3486ef7 100644
> > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > @@ -408,10 +408,19 @@ static const u32 gen7_render_regs[] = {
> >  	REG64(PS_INVOCATION_COUNT),
> >  	REG64(PS_DEPTH_COUNT),
> >  	OACONTROL, /* Only allowed for LRI and SRM. See below. */
> 
> It would be great to add:
> 
>       GEN7_3DPRIM_END_OFFSET,
> 
> which is the other ARB_draw_indirect register.  I have no idea why we
> don't use it - sure seems like we should...
> 
> > +	GEN7_3DPRIM_START_VERTEX,
> > +	GEN7_3DPRIM_VERTEX_COUNT,
> > +	GEN7_3DPRIM_INSTANCE_COUNT,
> > +	GEN7_3DPRIM_START_INSTANCE,
> > +	GEN7_3DPRIM_BASE_VERTEX,
> >  	REG64(GEN7_SO_NUM_PRIMS_WRITTEN(0)),
> >  	REG64(GEN7_SO_NUM_PRIMS_WRITTEN(1)),
> >  	REG64(GEN7_SO_NUM_PRIMS_WRITTEN(2)),
> >  	REG64(GEN7_SO_NUM_PRIMS_WRITTEN(3)),
> > +	REG64(GEN7_SO_PRIM_STORAGE_NEEDED(0)),
> > +	REG64(GEN7_SO_PRIM_STORAGE_NEEDED(1)),
> > +	REG64(GEN7_SO_PRIM_STORAGE_NEEDED(2)),
> > +	REG64(GEN7_SO_PRIM_STORAGE_NEEDED(3)),
> 
> FWIW, I don't think we actually need to write these...we just read them.
>  Though, there's not much harm in it.
> 
> >  	GEN7_SO_WRITE_OFFSET(0),
> >  	GEN7_SO_WRITE_OFFSET(1),
> >  	GEN7_SO_WRITE_OFFSET(2),
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 8e60737..533ec0a 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -427,6 +427,14 @@
> >  /* There are the 4 64-bit counter registers, one for each stream output */
> >  #define GEN7_SO_NUM_PRIMS_WRITTEN(n) (0x5200 + (n) * 8)
> >  
> > +#define GEN7_SO_PRIM_STORAGE_NEEDED(n)  (0x5240 + (n) * 8)
> > +
> 
> #define GEN7_3DPRIM_END_OFFSET          0x2420
> 
> > +#define GEN7_3DPRIM_START_VERTEX        0x2430
> > +#define GEN7_3DPRIM_VERTEX_COUNT        0x2434
> > +#define GEN7_3DPRIM_INSTANCE_COUNT      0x2438
> > +#define GEN7_3DPRIM_START_INSTANCE      0x243C
> > +#define GEN7_3DPRIM_BASE_VERTEX         0x2440
> > +
> >  #define OACONTROL 0x2360
> >  
> >  #define _GEN7_PIPEA_DE_LOAD_SL	0x70068
> 
> With END_OFFSET added, this would get a:
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

Brad I've already pulled this in for my rebasing tree to unblock mesa
testing. Feel free to either supply a fixup for me to squash in or a
replacement patch, I can handle either.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch