[PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[Abdiel Janulgue]

Adds support for executing the resource streamer on BDW

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..238bb25 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -356,6 +356,7 @@
 #define MI_BATCH_BUFFER_START	MI_INSTR(0x31, 0)
 #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
 #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
+#define   MI_BATCH_RESOURCE_STREAMER (1<<10)
 
 #define MI_PREDICATE_SRC0	(0x2400)
 #define MI_PREDICATE_SRC1	(0x2408)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 441e250..9045144 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2385,7 +2385,8 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs *ring,
 		return ret;
 
 	/* FIXME(BDW): Address space and security selectors. */
-	intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8));
+	intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8) |
+			(dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0));
 	intel_ring_emit(ring, lower_32_bits(offset));
 	intel_ring_emit(ring, upper_32_bits(offset));
 	intel_ring_emit(ring, MI_NOOP);
@@ -2408,7 +2409,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs *ring,
 	intel_ring_emit(ring,
 			MI_BATCH_BUFFER_START |
 			(dispatch_flags & I915_DISPATCH_SECURE ?
-			 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW));
+			 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) |
+			(dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0));
 	/* bit0-7 is the length on GEN6+ */
 	intel_ring_emit(ring, offset);
 	intel_ring_advance(ring);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

drm/i915/hsw/bdw: Enable resource streamer v2

[Abdiel Janulgue]

Changes since initial posting:

- Don't split the conversion from args->flags to ring from its
  subsequent EINVAL check (Chris Wilson <chris@chris-wilson.co.uk>)
- Execlists support (Minu Mathai <minu.mathai@intel.com>)

--
Abdiel Janulgue (2):
      drm/i915/hsw/bdw: Expose I915_EXEC_RESOURCE_STREAMER flag
      drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h            |  1 +
 drivers/gpu/drm/i915/intel_lrc.c           |  3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  6 ++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  1 +
 include/uapi/drm/i915_drm.h                |  7 ++++++-
 6 files changed, 29 insertions(+), 4 deletions(-)

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 1/2] drm/i915/hsw/bdw: Expose I915_EXEC_RESOURCE_STREAMER flag

[Abdiel Janulgue]

Ensures that the batch buffer is executed by the resource streamer

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  1 +
 include/uapi/drm/i915_drm.h                |  7 ++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a3190e79..f937ad0 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1485,6 +1485,21 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
+	if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
+		if (!IS_HASWELL(dev) && INTEL_INFO(dev)->gen < 8) {
+			DRM_DEBUG("RS is only allowed for Haswell, Gen8 "
+				  "and above\n");
+			return -EINVAL;
+		}
+		if (ring->id != RCS) {
+			DRM_DEBUG("RS is not available on %s)\n",
+				 ring->name);
+			return -EINVAL;
+		}
+
+		dispatch_flags |= I915_DISPATCH_RS;
+	}
+
 	intel_runtime_pm_get(dev_priv);
 
 	ret = i915_mutex_lock_interruptible(dev);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index c761fe0..3521bc0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -167,6 +167,7 @@ struct  intel_engine_cs {
 					       unsigned dispatch_flags);
 #define I915_DISPATCH_SECURE 0x1
 #define I915_DISPATCH_PINNED 0x2
+#define I915_DISPATCH_RS     0x4
 	void		(*cleanup)(struct intel_engine_cs *ring);
 
 	/* GEN8 signal/wait table - never trust comments!
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 551b673..a4c1a5c 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -760,7 +760,12 @@ struct drm_i915_gem_execbuffer2 {
 #define I915_EXEC_BSD_RING1		(1<<13)
 #define I915_EXEC_BSD_RING2		(2<<13)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
+/** Tell the kernel that the batchbuffer is processed by
+ *  the resource streamer.
+ */
+#define I915_EXEC_RESOURCE_STREAMER     (1<<16)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER <<1)
 
 #define I915_EXEC_CONTEXT_ID_MASK	(0xffffffff)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[Abdiel Janulgue]

Adds support for executing the resource streamer on BDW and HSW

v2: Add support for Execlists (Minu Mathai <minu.mathai@intel.com>)

Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h         | 1 +
 drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b522eb6..238bb25 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -356,6 +356,7 @@
 #define MI_BATCH_BUFFER_START	MI_INSTR(0x31, 0)
 #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
 #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
+#define   MI_BATCH_RESOURCE_STREAMER (1<<10)
 
 #define MI_PREDICATE_SRC0	(0x2400)
 #define MI_PREDICATE_SRC1	(0x2408)
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fcb074b..d523494 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1172,7 +1172,8 @@ static int gen8_emit_bb_start(struct intel_ringbuffer *ringbuf,
 		return ret;
 
 	/* FIXME(BDW): Address space and security selectors. */
-	intel_logical_ring_emit(ringbuf, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8));
+	intel_logical_ring_emit(ringbuf, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8) |
+				(I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0));
 	intel_logical_ring_emit(ringbuf, lower_32_bits(offset));
 	intel_logical_ring_emit(ringbuf, upper_32_bits(offset));
 	intel_logical_ring_emit(ringbuf, MI_NOOP);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 441e250..9045144 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2385,7 +2385,8 @@ gen8_ring_dispatch_execbuffer(struct intel_engine_cs *ring,
 		return ret;
 
 	/* FIXME(BDW): Address space and security selectors. */
-	intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8));
+	intel_ring_emit(ring, MI_BATCH_BUFFER_START_GEN8 | (ppgtt<<8) |
+			(dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0));
 	intel_ring_emit(ring, lower_32_bits(offset));
 	intel_ring_emit(ring, upper_32_bits(offset));
 	intel_ring_emit(ring, MI_NOOP);
@@ -2408,7 +2409,8 @@ hsw_ring_dispatch_execbuffer(struct intel_engine_cs *ring,
 	intel_ring_emit(ring,
 			MI_BATCH_BUFFER_START |
 			(dispatch_flags & I915_DISPATCH_SECURE ?
-			 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW));
+			 0 : MI_BATCH_PPGTT_HSW | MI_BATCH_NON_SECURE_HSW) |
+			(dispatch_flags & I915_DISPATCH_RS ? MI_BATCH_RESOURCE_STREAMER : 0));
 	/* bit0-7 is the length on GEN6+ */
 	intel_ring_emit(ring, offset);
 	intel_ring_advance(ring);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[Chris Wilson]

On Wed, May 13, 2015 at 11:13:24AM +0300, Abdiel Janulgue wrote:
> Adds support for executing the resource streamer on BDW and HSW
> 
> v2: Add support for Execlists (Minu Mathai <minu.mathai@intel.com>)
> 
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

I would have liked to have seen the comments for HSW_CTX_TOTAL_SIZE
updated to include the resource streamer requirements.

Also

/* These flags are for resource streamer on HSW+ */
if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
	flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);

implies to me that we should be setting something for hsw to
save/restore RS that we do not currently. So either the comment needs
fixing, or we have a lack of code.

Note that intel_lrc.c has duplicate functions for gen8. :|
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[Abdiel Janulgue]

Hi,

On 05/13/2015 01:22 PM, Chris Wilson wrote:
> On Wed, May 13, 2015 at 11:13:24AM +0300, Abdiel Janulgue wrote:
>> Adds support for executing the resource streamer on BDW and HSW
>>
>> v2: Add support for Execlists (Minu Mathai <minu.mathai@intel.com>)
>>
>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> 
> I would have liked to have seen the comments for HSW_CTX_TOTAL_SIZE
> updated to include the resource streamer requirements.

Comment block mention HSW_CTX_TOTAL_SIZE as 70720 bytes which already
implicitly include RS dwords.
I can add "70720 bytes include resource streamer dwords" to the comments
or do you want me to break this down further still?

> 
> Also
> 
> /* These flags are for resource streamer on HSW+ */
> if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
> 	flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> 
> implies to me that we should be setting something for hsw to
> save/restore RS that we do not currently. So either the comment needs
> fixing, or we have a lack of code.

I checked, the current code does disable context save and restore for RS
in HSW (commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2)

But I've now modified this to:

if (IS_HASWELL(ring->dev))
	flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);


so that MI_SET_CONTEXT includes RS context on HSW.

Anyway, this seems to work perfectly fine in HSW as well. I'll include
that in the next version.

> 
> Note that intel_lrc.c has duplicate functions for gen8. :|
> -Chris
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/hsw/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START

[Chris Wilson]

On Fri, May 15, 2015 at 12:16:06PM +0300, Abdiel Janulgue wrote:
> Hi,
> 
> On 05/13/2015 01:22 PM, Chris Wilson wrote:
> > On Wed, May 13, 2015 at 11:13:24AM +0300, Abdiel Janulgue wrote:
> >> Adds support for executing the resource streamer on BDW and HSW
> >>
> >> v2: Add support for Execlists (Minu Mathai <minu.mathai@intel.com>)
> >>
> >> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> > 
> > I would have liked to have seen the comments for HSW_CTX_TOTAL_SIZE
> > updated to include the resource streamer requirements.
> 
> Comment block mention HSW_CTX_TOTAL_SIZE as 70720 bytes which already
> implicitly include RS dwords.
> I can add "70720 bytes include resource streamer dwords" to the comments
> or do you want me to break this down further still?

I am just unnerved by the "full size is 70720, but we use 66944 bytes".
So perhaps changing the wording there to illustrate the boundaries would
be useful. However, I would be happy with just changing the wording to 
"...so the final size, including the extra state required for the
Resource Streamer, is 66944 bytes"

The important part here (for me at least) is that we do have something
in the git history to double check later on (i.e. we know that when we
did the RS enabling, that we did check the context size). Having clear
comments is a boon.

> > Also
> > 
> > /* These flags are for resource streamer on HSW+ */
> > if (!IS_HASWELL(ring->dev) && INTEL_INFO(ring->dev)->gen < 8)
> > 	flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> > 
> > implies to me that we should be setting something for hsw to
> > save/restore RS that we do not currently. So either the comment needs
> > fixing, or we have a lack of code.
> 
> I checked, the current code does disable context save and restore for RS
> in HSW (commit e80f14b6d36e3e07111cf2ab084ef8dd5d015ce2)
> 
> But I've now modified this to:
> 
> if (IS_HASWELL(ring->dev))
> 	flags |= (MI_SAVE_EXT_STATE_EN | MI_RESTORE_EXT_STATE_EN);
> 
> 
> so that MI_SET_CONTEXT includes RS context on HSW.

Now that makes more sense, I'd been puzzled by that comment and line of
code for ages :)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx