[1/3] drm/i915: Enable resource streamer bits on MI_BATCH_BUFFER_START

Submitted by Abdiel Janulgue on June 8, 2015, 10:04 a.m.

Details

Message ID 1433757849-3139-2-git-send-email-abdiel.janulgue@linux.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Abdiel Janulgue June 8, 2015, 10:04 a.m.
Adds support for executing the resource streamer on BDW and HSW

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

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
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        | 4 +++-
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
 4 files changed, 11 insertions(+), 3 deletions(-)

Patch hide | download patch | download mbox

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..3b168f6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1172,7 +1172,9 @@  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..715cb2a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2385,7 +2385,9 @@  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 +2410,9 @@  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);
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!

Comments

On Mon, Jun 08, 2015 at 01:04:07PM +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>)
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 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        | 4 +++-
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++--
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
>  4 files changed, 11 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..3b168f6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1172,7 +1172,9 @@ 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 ?

That doesn't look right.

> +					      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..715cb2a 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -2385,7 +2385,9 @@ 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 +2410,9 @@ 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);
> 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!
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 06/08/2015 07:10 PM, Ville Syrjälä wrote:
> On Mon, Jun 08, 2015 at 01:04:07PM +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>)
>>
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>> 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        | 4 +++-
>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++--
>>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
>>  4 files changed, 11 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..3b168f6 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1172,7 +1172,9 @@ 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 ?
> 
> That doesn't look right.

Yay.. Didn't catch these since this path never gets executed under GEN8
anyway which uses execlist not legacy batch buffer execution. Better
remove this then.
> 
>> +					      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..715cb2a 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -2385,7 +2385,9 @@ 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 +2410,9 @@ 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);
>> 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!
>> -- 
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
On Mon, Jun 08, 2015 at 08:42:20PM +0300, Abdiel Janulgue wrote:
> 
> 
> On 06/08/2015 07:10 PM, Ville Syrjälä wrote:
> > On Mon, Jun 08, 2015 at 01:04:07PM +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>)
> >>
> >> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> 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        | 4 +++-
> >>  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++--
> >>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
> >>  4 files changed, 11 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..3b168f6 100644
> >> --- a/drivers/gpu/drm/i915/intel_lrc.c
> >> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> >> @@ -1172,7 +1172,9 @@ 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 ?
> > 
> > That doesn't look right.
> 
> Yay.. Didn't catch these since this path never gets executed under GEN8
> anyway which uses execlist not legacy batch buffer execution. Better
> remove this then.

Which makes gen8 support conditional and so needs reflecting in the
interface.

Just test the flag correctly.
-Chris
On 08/06/15 18:42, Abdiel Janulgue wrote:
> 
> On 06/08/2015 07:10 PM, Ville Syrjälä wrote:
>> On Mon, Jun 08, 2015 at 01:04:07PM +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>)
>>>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> 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        | 4 +++-
>>>  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++--
>>>  drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
>>>  4 files changed, 11 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..3b168f6 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1172,7 +1172,9 @@ 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 ?
>>
>> That doesn't look right.
> 
> Yay.. Didn't catch these since this path never gets executed under GEN8
> anyway which uses execlist not legacy batch buffer execution. Better
> remove this then.

But GEN8 HW can run in legacy ringbuffer mode, and the driver continues
to support it, at least for now, so AFAIK there's nothing preventing you
using the Resource Streamer in ringbuffer mode; indeed I note that the
MI_RS_CONTEXT instruction can ONLY be used in ringbuffer mode.

Please don't conflate changes that are (or ought to be) orthogonal, such
as 32- vs 48-bit addressing and ringbuffer vs. execlists, just because
they were introduced in the same h/w generation ...

.Dave.