On 10/11/2011 11:31 AM, Ben Widawsky wrote:
> This will strongly order synchronization commands with respect to 3d
> state and 3d primitive commands. AFAIK, this shouldn't impact anything
> as these sync commands are all for privileged (or ppgtt) batches only,
> so user space should not be relying on this, and the kernel wouldn't be
> relying on 3d state or primitive commands.
>
> This will help when we enable PPGTT, and perhaps this synchronization is
> currently useful and I just don't realize it.
>
> This was found through doc inspection by Ken and applies to Gen6+;
>
> Reported-by: Kenneth Graunke <kenn...@whitecape.org>
> Signed-off-by: Ben Widawsky <b...@bwidawsk.net>
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++++--
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++
> 3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 9572e52..1d55842 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -976,6 +976,7 @@ i915_set_constant_offset(struct intel_ring_buffer *ring,
> int mode)
> {
> struct drm_device *dev = ring->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> + uint32_t mask = I915_EXEC_CONSTANTS_MASK;
> int ret;
>
> switch (mode) {
> @@ -991,6 +992,10 @@ i915_set_constant_offset(struct intel_ring_buffer *ring,
> int mode)
> mode == I915_EXEC_CONSTANTS_REL_SURFACE)
> return -EINVAL;
>
> + /* Never clear this bit because of execbuffer */
> + if (INTEL_INFO(dev)->gen >= 6)
> + mask &= ~(INSTPM_FORCE_ORDERING);
> +
I might do:
/* This bit doesn't exist on Gen6+. */
if (INTEL_INFO(dev)->gen >= 6)
mask &= ~I915_EXEC_CONSTANTS_REL_SURFACE;
...but, I don't mind either way.
> ret = intel_ring_begin(ring, 4);
> if (ret)
> return ret;
> @@ -998,8 +1003,7 @@ i915_set_constant_offset(struct intel_ring_buffer *ring,
> int mode)
> intel_ring_emit(ring, MI_NOOP);
> intel_ring_emit(ring, MI_LOAD_REGISTER_IMM(1));
> intel_ring_emit(ring, INSTPM);
> - intel_ring_emit(ring,
> - I915_EXEC_CONSTANTS_MASK << 16 | mode);
> + intel_ring_emit(ring, mask << 16 | mode);
> intel_ring_advance(ring);
>
> dev_priv->relative_constants_mode = mode;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 138eae1..51569f2 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -436,6 +436,7 @@
> #define INSTPM_AGPBUSY_DIS (1<<11) /* gen3: when disabled, pending
> interrupts
> will not assert AGPBUSY# and will only
> be delivered when out of C3. */
> +#define INSTPM_FORCE_ORDERING (1<<7) /* GEN6+
> */
> #define ACTHD 0x020c8
> #define FW_BLC 0x020d8
> #define FW_BLC2 0x020dc
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
> b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 0e99589..b1d312f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -297,6 +297,8 @@ static int init_render_ring(struct intel_ring_buffer
> *ring)
> }
>
> if (INTEL_INFO(dev)->gen >= 6) {
> + I915_WRITE(INSTPM, INSTPM_FORCE_ORDERING << 16 |
> + INSTPM_FORCE_ORDERING);
> } else if (IS_GEN5(dev)) {
> ret = init_pipe_control(ring);
> if (ret)
I might only enable this on Gen7 for now, unless it actually fixes
something on Sandybridge. It's not listed as required for Gen6.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
