[v3,5/5] drm/i915: Expose I915_EXEC_RESOURCE_STREAMER flag
diff mbox

Message ID 1435734743-14562-1-git-send-email-abdiel.janulgue@linux.intel.com
State New
Headers show

Commit Message

Abdiel Janulgue July 1, 2015, 7:12 a.m. UTC
Ensures that the batch buffer is executed by the resource streamer

v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)

Testcase: igt/gem_exec_params
Cc: Jani Nikula <jani.nikula@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_gem_execbuffer.c | 14 ++++++++++++++
 include/uapi/drm/i915_drm.h                |  7 ++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

Comments

Chris Wilson July 2, 2015, 10:15 a.m. UTC | #1
On Wed, Jul 01, 2015 at 10:12:23AM +0300, Abdiel Janulgue wrote:
> Ensures that the batch buffer is executed by the resource streamer
> 
> v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
> v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)
> 
> Testcase: igt/gem_exec_params
> Cc: Jani Nikula <jani.nikula@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>

This no longer applies (unrecognised base). Honestly, I would prefer 4&5
squashed together, or 4 after 5 so that we do not declare
HAS_RESOURCE_STREAMER before we accept the RS execbuf.

Minor bit of patch reordering, but the code in 4 looks ok, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # for 4/5
-Chris
Daniel Vetter July 6, 2015, 8:28 a.m. UTC | #2
On Thu, Jul 02, 2015 at 11:15:40AM +0100, Chris Wilson wrote:
> On Wed, Jul 01, 2015 at 10:12:23AM +0300, Abdiel Janulgue wrote:
> > Ensures that the batch buffer is executed by the resource streamer
> > 
> > v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
> > v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)
> > 
> > Testcase: igt/gem_exec_params
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
> 
> This no longer applies (unrecognised base). Honestly, I would prefer 4&5
> squashed together, or 4 after 5 so that we do not declare
> HAS_RESOURCE_STREAMER before we accept the RS execbuf.
> 
> Minor bit of patch reordering, but the code in 4 looks ok, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # for 4/5

Yeah I just reordered 5 to go before 4. Series merged to dinq, thanks.
Abdiel can you pls push the corresponding igts (or ask Thomas to do it for
you)?
-Daniel
Abdiel Janulgue July 6, 2015, 8:46 a.m. UTC | #3
On 07/06/2015 11:28 AM, Daniel Vetter wrote:
> On Thu, Jul 02, 2015 at 11:15:40AM +0100, Chris Wilson wrote:
>> On Wed, Jul 01, 2015 at 10:12:23AM +0300, Abdiel Janulgue wrote:
>>> Ensures that the batch buffer is executed by the resource streamer
>>>
>>> v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
>>> v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)
>>>
>>> Testcase: igt/gem_exec_params
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>>
>> This no longer applies (unrecognised base). Honestly, I would prefer 4&5
>> squashed together, or 4 after 5 so that we do not declare
>> HAS_RESOURCE_STREAMER before we accept the RS execbuf.
>>
>> Minor bit of patch reordering, but the code in 4 looks ok, so
>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # for 4/5
> 
> Yeah I just reordered 5 to go before 4. Series merged to dinq, thanks.
> Abdiel can you pls push the corresponding igts (or ask Thomas to do it for
> you)?

Thank you! I don't have commit rights in igt, but here is the patch:
http://lists.freedesktop.org/archives/intel-gfx/2015-June/068799.html

-Abdiel
Thomas Wood July 6, 2015, 9:21 a.m. UTC | #4
On 6 July 2015 at 09:46, Abdiel Janulgue
<abdiel.janulgue@linux.intel.com> wrote:
>
>
> On 07/06/2015 11:28 AM, Daniel Vetter wrote:
>> On Thu, Jul 02, 2015 at 11:15:40AM +0100, Chris Wilson wrote:
>>> On Wed, Jul 01, 2015 at 10:12:23AM +0300, Abdiel Janulgue wrote:
>>>> Ensures that the batch buffer is executed by the resource streamer
>>>>
>>>> v2: Don't skip 1<<15 for the exec flags (Jani Nikula)
>>>> v3: Use HAS_RESOURCE_STREAMER macro for execbuf validation (Chris Wilson)
>>>>
>>>> Testcase: igt/gem_exec_params
>>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>>>> Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com>
>>>
>>> This no longer applies (unrecognised base). Honestly, I would prefer 4&5
>>> squashed together, or 4 after 5 so that we do not declare
>>> HAS_RESOURCE_STREAMER before we accept the RS execbuf.
>>>
>>> Minor bit of patch reordering, but the code in 4 looks ok, so
>>> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> # for 4/5
>>
>> Yeah I just reordered 5 to go before 4. Series merged to dinq, thanks.
>> Abdiel can you pls push the corresponding igts (or ask Thomas to do it for
>> you)?
>
> Thank you! I don't have commit rights in igt, but here is the patch:
> http://lists.freedesktop.org/archives/intel-gfx/2015-June/068799.html

Pushed, thanks.

>
> -Abdiel

Patch
diff mbox

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 600db74..83577c6 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1490,6 +1490,20 @@  i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
+	if (args->flags & I915_EXEC_RESOURCE_STREAMER) {
+		if (!HAS_RESOURCE_STREAMER(dev)) {
+			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/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 51137bd..e7c29f1 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -766,7 +766,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<<15)
+
+#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) \