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

Commit Message

[e-mail] June 13, 2014, 3:37 p.m. UTC

From: Oscar Mateo <[e-mail]>

Very similar to the legacy add_request, only modified to account for
logical ringbuffer.

Signed-off-by: Oscar Mateo <[e-mail]>
---
 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(+)

Comments

[e-mail] June 20, 2014, 9:18 p.m. UTC | #1

On Fri, Jun 13, 2014 at 08:37:45AM -0700, [e-mail] wrote:
> From: Oscar Mateo <[e-mail]>
> 
> Very similar to the legacy add_request, only modified to account for
> logical ringbuffer.
> 
> Signed-off-by: Oscar Mateo <[e-mail]>
> ---
>  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
> [e-mail]
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[e-mail] June 23, 2014, 3:48 p.m. UTC | #2

> -----Original Message-----
> From: Volkin, Bradley D
> Sent: Friday, June 20, 2014 10:18 PM
> To: Mateo Lozano, Oscar
> Cc: [e-mail]
> Subject: Re: [Intel-gfx] [PATCH 27/53] drm/i915/bdw: GEN-specific logical
> ring emit request
> 
> On Fri, Jun 13, 2014 at 08:37:45AM -0700, [e-mail] wrote:
> > From: Oscar Mateo <[e-mail]>
> >
> > Very similar to the legacy add_request, only modified to account for
> > logical ringbuffer.
> >
> > Signed-off-by: Oscar Mateo <[e-mail]>
> > ---
> >  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+.

Hmmmmm... this is inherited from back when we only had the simulator, and its true meaning has been lost in the multiple rewrites:

drm/i915/bdw: Use MI_FLUSH_DW for requests
    
    The primary reason for doing this is MI_STORE_DWORD_IDX doesn't work in
    simulation. The simulator doesn't complain, it's just the seqno never
    gets pushed to memory. The theory is (and AFAICT this may be broken on
    existing platforms) we must issue an MI_FLUSH_DW after we emit the
    seqno, if we want to be able to read it back coherently.

I´ll rewrite it to use the same MI_STORE_DATA_IMM for every ring and then test it on read hardware.
Thanks for the heads up!

> > +	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.

Will do, thanks.

-- Oscar

Patch

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;
+	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 */
+
+	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