drm/i915/bdw: Enable resource streamer on Broadwell

drm/i915/bdw: Enable resource streamer on Broadwell

[Abdiel Janulgue]

From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

This is a re-spin of my resource streamer patchset from October
adapted to enable the feature on Broadwell instead.

The resource streamer is a hw-feature that helps in reducing commands
being submitted by the CPU. Haswell initially has this feature.
Unfortunately, HSW seems to have problems switching between non-RS
and RS-enabled commands[1]. On BDW however it works seamlessly
as expected.

The i-g-t test comes next on a separate patch-series.

--
[1] http://lists.freedesktop.org/archives/intel-gfx/2014-May/044577.html 

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

 drivers/gpu/drm/i915/i915_gem_execbuffer.c |    8 ++++++++
 drivers/gpu/drm/i915/i915_reg.h            |    1 +
 drivers/gpu/drm/i915/intel_ringbuffer.c    |    3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
 include/uapi/drm/i915_drm.h                |    7 ++++++-
 5 files changed, 18 insertions(+), 2 deletions(-)

[PATCH 1/2] drm/i915/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 |    3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |    1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 0eff337..2d5d0ab 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -286,6 +286,7 @@
 #define   MI_BATCH_NON_SECURE_HSW	(1<<13)
 #define MI_BATCH_BUFFER_START	MI_INSTR(0x31, 0)
 #define   MI_BATCH_GTT		    (2<<6) /* aliased with (1<<7) on gen4 */
+#define   MI_BATCH_RESOURCE_STREAMER (1<<10)
 #define MI_BATCH_BUFFER_START_GEN8	MI_INSTR(0x31, 1)
 
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 40a7aa4..cc0110c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1810,7 +1810,8 @@ gen8_ring_dispatch_execbuffer(struct intel_ring_buffer *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) |
+			(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);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 72c3c15..cf9e259 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -116,6 +116,7 @@ struct  intel_ring_buffer {
 					       unsigned flags);
 #define I915_DISPATCH_SECURE 0x1
 #define I915_DISPATCH_PINNED 0x2
+#define I915_DISPATCH_RS     0x4
 	void		(*cleanup)(struct intel_ring_buffer *ring);
 
 	struct {
-- 
1.7.9.5

[PATCH 2/2] drm/i915/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 |    8 ++++++++
 include/uapi/drm/i915_drm.h                |    7 ++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 47fe8ec..68aaa9b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1074,6 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (args->flags & I915_EXEC_IS_PINNED)
 		flags |= I915_DISPATCH_PINNED;
 
+	if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
+		if ((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_RENDER ||
+		    !IS_BROADWELL(dev))
+			return -EINVAL;
+
+		flags |= I915_DISPATCH_RS;
+	}
+
 	if ((args->flags & I915_EXEC_RING_MASK) > LAST_USER_RING) {
 		DRM_DEBUG("execbuf with unknown ring: %d\n",
 			  (int)(args->flags & I915_EXEC_RING_MASK));
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 8a3e4ef00..adfc45d 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -734,7 +734,12 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_HANDLE_LUT		(1<<12)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1)
+/** Tell the kernel that the batchbuffer is processed by
+ *  the resource streamer.
+ */
+#define I915_EXEC_RESOURCE_STREAMER     (1<<13)
+
+#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.7.9.5

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

[Chris Wilson]

On Tue, May 06, 2014 at 10:25:06PM +0300, Abdiel Janulgue wrote:
> 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 |    8 ++++++++
>  include/uapi/drm/i915_drm.h                |    7 ++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 47fe8ec..68aaa9b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1074,6 +1074,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	if (args->flags & I915_EXEC_IS_PINNED)
>  		flags |= I915_DISPATCH_PINNED;
>  
> +	if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
> +		if ((args->flags & I915_EXEC_RING_MASK) != I915_EXEC_RENDER ||
> +		    !IS_BROADWELL(dev))
> +			return -EINVAL;

This misses I915_EXEC_DEFAULT. If you reorder this testing to after we
decide upon which ring is being run, this should look simpler.

 if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
	if (!IS_BROADWELL(dev)) {
		DRM_DEBUG("resource streamer is only enabled for Broadwell\n");
		return -EINVAL;
	}
	if (ring->id != RCS) {
		DRM_DEBUG("resource streamer is only available on the render ring (not %s)\n", ring->name);
		return -EINVAL;
	}
	flags |= I915_DISPATCH_RS;
 }
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: drm/i915/bdw: Enable resource streamer on Broadwell

[Jesse Barnes]

On Tue,  6 May 2014 22:25:04 +0300
Abdiel Janulgue <abdiel.janulgue@linux.intel.com> wrote:

> From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> 
> This is a re-spin of my resource streamer patchset from October
> adapted to enable the feature on Broadwell instead.
> 
> The resource streamer is a hw-feature that helps in reducing commands
> being submitted by the CPU. Haswell initially has this feature.
> Unfortunately, HSW seems to have problems switching between non-RS
> and RS-enabled commands[1]. On BDW however it works seamlessly
> as expected.
> 
> The i-g-t test comes next on a separate patch-series.
> 
> --
> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-May/044577.html 
> 
> Abdiel Janulgue (2):
>       drm/i915/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START
>       drm/i915/bdw: Expose I915_EXEC_RESOURCE_STREAMER flag
> 
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |    8 ++++++++
>  drivers/gpu/drm/i915/i915_reg.h            |    1 +
>  drivers/gpu/drm/i915/intel_ringbuffer.c    |    3 ++-
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
>  include/uapi/drm/i915_drm.h                |    7 ++++++-
>  5 files changed, 18 insertions(+), 2 deletions(-)

These seem trivial enough... have you seen cases where you'd like to
enable it in Mesa?  If so, it probably makes sense to merge this patch
so you can do your tuning and enabling on the Mesa side...

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: drm/i915/bdw: Enable resource streamer on Broadwell

[Abdiel Janulgue]

On 11.06.2014 11:10, Jesse Barnes wrote:
> On Tue,  6 May 2014 22:25:04 +0300
> Abdiel Janulgue <abdiel.janulgue@linux.intel.com> wrote:
>
>> From: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>>
>> This is a re-spin of my resource streamer patchset from October
>> adapted to enable the feature on Broadwell instead.
>>
>> The resource streamer is a hw-feature that helps in reducing commands
>> being submitted by the CPU. Haswell initially has this feature.
>> Unfortunately, HSW seems to have problems switching between non-RS
>> and RS-enabled commands[1]. On BDW however it works seamlessly
>> as expected.
>>
>> The i-g-t test comes next on a separate patch-series.
>>
>> --
>> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-May/044577.html
>>
>> Abdiel Janulgue (2):
>>        drm/i915/bdw: Enable resource streamer bits on MI_BATCH_BUFFER_START
>>        drm/i915/bdw: Expose I915_EXEC_RESOURCE_STREAMER flag
>>
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |    8 ++++++++
>>   drivers/gpu/drm/i915/i915_reg.h            |    1 +
>>   drivers/gpu/drm/i915/intel_ringbuffer.c    |    3 ++-
>>   drivers/gpu/drm/i915/intel_ringbuffer.h    |    1 +
>>   include/uapi/drm/i915_drm.h                |    7 ++++++-
>>   5 files changed, 18 insertions(+), 2 deletions(-)
> These seem trivial enough... have you seen cases where you'd like to
> enable it in Mesa?  If so, it probably makes sense to merge this patch
> so you can do your tuning and enabling on the Mesa side...

I've had some Mesa patches since last year. Unfortunately, the changes 
didn't
give any dramatic performance improvements. On the other hand, I admit: 
1. I've
only run it against GLBenchmark and Unigine benchmarks. Discussion with 
Portland
folks seem to suggest that we need to have a comprehensive run with more
benchmarks. 2. I only did the benchmarks in Haswell. Would be nice to see
how it does on Broadwell.

I agree though that further tuning of the performance needs to be done on
the Mesa side.