Re: [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.

From: Daniel Vetter <daniel@ffwll.ch>
To: Kenneth Graunke <kenneth@whitecape.org>,
	Brad Volkin <bradley.d.volkin@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.
Date: Wed, 26 Mar 2014 07:21:23 +0100	[thread overview]
Message-ID: <20140326062123.GO26878@phenom.ffwll.local> (raw)
In-Reply-To: <1395813123-2027-1-git-send-email-kenneth@whitecape.org>

On Tue, Mar 25, 2014 at 10:52:03PM -0700, Kenneth Graunke wrote:
> Mesa needs to be able to write OACONTROL in order to expose the
> Observability Architecture's performance counters via OpenGL.
> 
> Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>

Thanks a lot for quickly tracking this down. Now when we've talked about
OA a little while ago we concluded that mesa should clear OACONTROL again
before the batch ends to make sure that userspace can't unduly observe
other processes. So I think it'd be worth to keep track of this with a
flag (set when OACONTROL is != 0 and reset when the batch loads 0). Also
we need to make sure that userspace sets the right OACONTROL modes (not
the one which streams into a global gtt buffer essentially). So some
additional work required.

I've added a FIXME comment to the code.

Brad, can you please look into this, including the corresponding i-g-t
which tries to enable OA but doesn't disable it and one which tries to
enable it in the continuous mode (which writes to gtt)? Ken's previous
mails has the cmds mesa emits.

Thanks, Daniel
> ---
>  drivers/gpu/drm/i915/i915_cmd_parser.c | 1 +
>  drivers/gpu/drm/i915/i915_reg.h        | 2 ++
>  2 files changed, 3 insertions(+)
> 
> This patch needs to go before
> 
>    commit 6d42f94084b8c69813d7ecd0466c33fe561bf127
>    Author: Brad Volkin <bradley.d.volkin@intel.com>
>    Date:   Tue Feb 18 10:15:57 2014 -0800
> 
>        drm/i915: Enable command parsing by default
> 
> in whatever branch gets submitted to Dave Airlie.  Or, that commit needs
> to be reverted.  Otherwise, every OpenGL program will abort.  Examples
> of programs that abort include GNOME, KDE, Firefox, and glxgears.
> 
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index bae7c2f..d4a50b9 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -415,6 +415,7 @@ static const u32 gen7_render_regs[] = {
>  	GEN7_SO_WRITE_OFFSET(1),
>  	GEN7_SO_WRITE_OFFSET(2),
>  	GEN7_SO_WRITE_OFFSET(3),
> +	OACONTROL,
>  };
>  
>  static const u32 gen7_blt_regs[] = {
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9f9e2b7..0ebc20d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -427,6 +427,8 @@
>  /* There are the 4 64-bit counter registers, one for each stream output */
>  #define GEN7_SO_NUM_PRIMS_WRITTEN(n) (0x5200 + (n) * 8)
>  
> +#define OACONTROL 0x2360
> +
>  #define _GEN7_PIPEA_DE_LOAD_SL	0x70068
>  #define _GEN7_PIPEB_DE_LOAD_SL	0x71068
>  #define GEN7_PIPE_DE_LOAD_SL(pipe) _PIPE(pipe, \
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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