diff mbox series

drm/i915: Improve execbuf debug

Message ID 20191209122314.16289-1-tvrtko.ursulin@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Improve execbuf debug | expand

Commit Message

Tvrtko Ursulin Dec. 9, 2019, 12:23 p.m. UTC
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Convert i915_gem_check_execbuffer to return the error code instead of
a boolean so our neat EINVAL debugging trick works within this function.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 .../gpu/drm/i915/gem/i915_gem_execbuffer.c    | 22 ++++++++++---------
 1 file changed, 12 insertions(+), 10 deletions(-)

Comments

Chris Wilson Dec. 9, 2019, 12:26 p.m. UTC | #1
Quoting Tvrtko Ursulin (2019-12-09 12:23:14)
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Convert i915_gem_check_execbuffer to return the error code instead of
> a boolean so our neat EINVAL debugging trick works within this function.

Who would need it? :)
 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Tvrtko Ursulin Dec. 9, 2019, 1:17 p.m. UTC | #2
On 09/12/2019 12:26, Chris Wilson wrote:
> Quoting Tvrtko Ursulin (2019-12-09 12:23:14)
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Convert i915_gem_check_execbuffer to return the error code instead of
>> a boolean so our neat EINVAL debugging trick works within this function.
> 
> Who would need it? :)

Hey I found it helpful! :)

>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Thanks, but if you don't like it just say.

Regards,

Tvrtko
Chris Wilson Dec. 9, 2019, 1:20 p.m. UTC | #3
Quoting Tvrtko Ursulin (2019-12-09 13:17:38)
> 
> On 09/12/2019 12:26, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-12-09 12:23:14)
> >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >> Convert i915_gem_check_execbuffer to return the error code instead of
> >> a boolean so our neat EINVAL debugging trick works within this function.
> > 
> > Who would need it? :)
> 
> Hey I found it helpful! :)
> 
> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Thanks, but if you don't like it just say.

I was jesting, more precise cost-free debug is great! :)
-Chris
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 34044c6203a5..5003e616a1ad 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1915,15 +1915,15 @@  static int eb_move_to_gpu(struct i915_execbuffer *eb)
 	return err;
 }
 
-static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
+static int i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
 {
 	if (exec->flags & __I915_EXEC_ILLEGAL_FLAGS)
-		return false;
+		return -EINVAL;
 
 	/* Kernel clipping was a DRI1 misfeature */
 	if (!(exec->flags & I915_EXEC_FENCE_ARRAY)) {
 		if (exec->num_cliprects || exec->cliprects_ptr)
-			return false;
+			return -EINVAL;
 	}
 
 	if (exec->DR4 == 0xffffffff) {
@@ -1931,12 +1931,12 @@  static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec)
 		exec->DR4 = 0;
 	}
 	if (exec->DR1 || exec->DR4)
-		return false;
+		return -EINVAL;
 
 	if ((exec->batch_start_offset | exec->batch_len) & 0x7)
-		return false;
+		return -EINVAL;
 
-	return true;
+	return 0;
 }
 
 static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
@@ -2768,8 +2768,9 @@  i915_gem_execbuffer_ioctl(struct drm_device *dev, void *data,
 	exec2.flags = I915_EXEC_RENDER;
 	i915_execbuffer2_set_context_id(exec2, 0);
 
-	if (!i915_gem_check_execbuffer(&exec2))
-		return -EINVAL;
+	err = i915_gem_check_execbuffer(&exec2);
+	if (err)
+		return err;
 
 	/* Copy in the exec list from userland */
 	exec_list = kvmalloc_array(count, sizeof(*exec_list),
@@ -2846,8 +2847,9 @@  i915_gem_execbuffer2_ioctl(struct drm_device *dev, void *data,
 		return -EINVAL;
 	}
 
-	if (!i915_gem_check_execbuffer(args))
-		return -EINVAL;
+	err = i915_gem_check_execbuffer(args);
+	if (err)
+		return err;
 
 	/* Allocate an extra slot for use by the command parser */
 	exec2_list = kvmalloc_array(count + 1, eb_element_size(),