Re: [PATCH 27/53] drm/i915/bdw: GEN-specific logical ring emit request

["Volkin, Bradley D" <bradley.d.volkin@intel.com>]

On Fri, Jun 13, 2014 at 08:37:45AM -0700, oscar.mateo@intel.com wrote:
> From: Oscar Mateo <oscar.mateo@intel.com>
> 
> Very similar to the legacy add_request, only modified to account for
> logical ringbuffer.
> 
> Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h         |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c        | 61 +++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.h |  2 ++
>  3 files changed, 64 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9c8692a..63ec3ea 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -267,6 +267,7 @@
>  #define   MI_FORCE_RESTORE		(1<<1)
>  #define   MI_RESTORE_INHIBIT		(1<<0)
>  #define MI_STORE_DWORD_IMM	MI_INSTR(0x20, 1)
> +#define MI_STORE_DWORD_IMM_GEN8	MI_INSTR(0x20, 2)
>  #define   MI_MEM_VIRTUAL	(1 << 22) /* 965+ only */
>  #define MI_STORE_DWORD_INDEX	MI_INSTR(0x21, 1)
>  #define   MI_STORE_DWORD_INDEX_SHIFT 2
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 89aed7a..3debe8b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -359,6 +359,62 @@ static void gen8_submit_ctx(struct intel_engine_cs *ring,
>  	DRM_ERROR("Execlists still not ready!\n");
>  }
>  
> +static int gen8_emit_request(struct intel_engine_cs *ring,
> +			     struct intel_context *ctx)
> +{
> +	struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx);
> +	u32 cmd;
> +	int ret;
> +
> +	ret = intel_logical_ring_begin(ring, ctx, 6);
> +	if (ret)
> +		return ret;
> +
> +	cmd = MI_FLUSH_DW + 1;
> +	cmd |= MI_INVALIDATE_TLB;

Is the TLB invalidation truely required here? Otherwise it seems
like we could use the same function for all rings, like on gen6+.

> +	cmd |= MI_FLUSH_DW_OP_STOREDW;
> +
> +	intel_logical_ring_emit(ringbuf, cmd);
> +	intel_logical_ring_emit(ringbuf,
> +				(ring->status_page.gfx_addr +
> +				(I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT)) |
> +				MI_FLUSH_DW_USE_GTT);
> +	intel_logical_ring_emit(ringbuf, 0);
> +	intel_logical_ring_emit(ringbuf, ring->outstanding_lazy_seqno);
> +	intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
> +	intel_logical_ring_emit(ringbuf, MI_NOOP);
> +	intel_logical_ring_advance_and_submit(ring, ctx);
> +
> +	return 0;
> +}
> +
> +static int gen8_emit_request_render(struct intel_engine_cs *ring,
> +				    struct intel_context *ctx)
> +{
> +	struct intel_ringbuffer *ringbuf = logical_ringbuf_get(ring, ctx);
> +	u32 cmd;
> +	int ret;
> +
> +	ret = intel_logical_ring_begin(ring, ctx, 6);
> +	if (ret)
> +		return ret;
> +
> +	cmd = MI_STORE_DWORD_IMM_GEN8;
> +	cmd |= (1 << 22); /* use global GTT */

We could use MI_MEM_VIRTUAL or MI_GLOBAL_GTT instead.

Brad

> +
> +	intel_logical_ring_emit(ringbuf, cmd);
> +	intel_logical_ring_emit(ringbuf,
> +				(ring->status_page.gfx_addr +
> +				(I915_GEM_HWS_INDEX << MI_STORE_DWORD_INDEX_SHIFT)));
> +	intel_logical_ring_emit(ringbuf, 0);
> +	intel_logical_ring_emit(ringbuf, ring->outstanding_lazy_seqno);
> +	intel_logical_ring_emit(ringbuf, MI_USER_INTERRUPT);
> +	intel_logical_ring_emit(ringbuf, MI_NOOP);
> +	intel_logical_ring_advance_and_submit(ring, ctx);
> +
> +	return 0;
> +}
> +
>  void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
>  {
>  	if (!intel_ring_initialized(ring))
> @@ -434,6 +490,7 @@ static int logical_render_ring_init(struct drm_device *dev)
>  	ring->get_seqno = gen8_get_seqno;
>  	ring->set_seqno = gen8_set_seqno;
>  	ring->submit_ctx = gen8_submit_ctx;
> +	ring->emit_request = gen8_emit_request_render;
>  
>  	return logical_ring_init(dev, ring);
>  }
> @@ -453,6 +510,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)
>  	ring->get_seqno = gen8_get_seqno;
>  	ring->set_seqno = gen8_set_seqno;
>  	ring->submit_ctx = gen8_submit_ctx;
> +	ring->emit_request = gen8_emit_request;
>  
>  	return logical_ring_init(dev, ring);
>  }
> @@ -472,6 +530,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)
>  	ring->get_seqno = gen8_get_seqno;
>  	ring->set_seqno = gen8_set_seqno;
>  	ring->submit_ctx = gen8_submit_ctx;
> +	ring->emit_request = gen8_emit_request;
>  
>  	return logical_ring_init(dev, ring);
>  }
> @@ -491,6 +550,7 @@ static int logical_blt_ring_init(struct drm_device *dev)
>  	ring->get_seqno = gen8_get_seqno;
>  	ring->set_seqno = gen8_set_seqno;
>  	ring->submit_ctx = gen8_submit_ctx;
> +	ring->emit_request = gen8_emit_request;
>  
>  	return logical_ring_init(dev, ring);
>  }
> @@ -510,6 +570,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)
>  	ring->get_seqno = gen8_get_seqno;
>  	ring->set_seqno = gen8_set_seqno;
>  	ring->submit_ctx = gen8_submit_ctx;
> +	ring->emit_request = gen8_emit_request;
>  
>  	return logical_ring_init(dev, ring);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 1a6df42..d8ded14 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -151,6 +151,8 @@ struct  intel_engine_cs {
>  	/* Execlists */
>  	void		(*submit_ctx)(struct intel_engine_cs *ring,
>  				      struct intel_context *ctx, u32 value);
> +	int		(*emit_request)(struct intel_engine_cs *ring,
> +					struct intel_context *ctx);
>  
>  	/**
>  	 * List of objects currently involved in rendering from the
> -- 
> 1.9.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx