[Intel-gfx] [PATCH 05/53] drm/i915: Add return code check to i915_gem_execbuffer_retire_commands()

[Daniel Vetter]

On Thu, Mar 05, 2015 at 02:45:11PM +0000, Tomas Elf wrote:
> On 19/02/2015 17:17, John.C.Harrison at Intel.com wrote:
> >From: John Harrison <John.C.Harrison at Intel.com>
> >
> >For some reason, the i915_add_request() call in
> >i915_gem_execbuffer_retire_commands() was explicitly having its return code
> >ignored. The _retire_commands() function itself was 'void'. Given that
> >_add_request() can fail without dispatching the batch buffer, this seems odd.
> >
> >Also shrunk the parameter list to a single structure as everything it requires
> >is available in the execbuff_params object.
> >
> >For: VIZ-5115
> >Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> >---
> >  drivers/gpu/drm/i915/i915_drv.h            |    5 +----
> >  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   16 +++++++---------
> >  drivers/gpu/drm/i915/intel_lrc.c           |    3 +--
> >  3 files changed, 9 insertions(+), 15 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> >index fdd63ab..b350910 100644
> >--- a/drivers/gpu/drm/i915/i915_drv.h
> >+++ b/drivers/gpu/drm/i915/i915_drv.h
> >@@ -2640,10 +2640,7 @@ int i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data,
> >  			     struct drm_file *file_priv);
> >  void i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> >  					struct intel_engine_cs *ring);
> >-void i915_gem_execbuffer_retire_commands(struct drm_device *dev,
> >-					 struct drm_file *file,
> >-					 struct intel_engine_cs *ring,
> >-					 struct drm_i915_gem_object *obj);
> >+int i915_gem_execbuffer_retire_commands(struct i915_execbuffer_params *params);
> >  int i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
> >  				   struct drm_i915_gem_execbuffer2 *args,
> >  				   struct list_head *vmas);
> >diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >index 93b0ef0..ca85803 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >@@ -989,17 +989,15 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas,
> >  	}
> >  }
> >
> >-void
> >-i915_gem_execbuffer_retire_commands(struct drm_device *dev,
> >-				    struct drm_file *file,
> >-				    struct intel_engine_cs *ring,
> >-				    struct drm_i915_gem_object *obj)
> >+int
> >+i915_gem_execbuffer_retire_commands(struct i915_execbuffer_params *params)
> >  {
> >  	/* Unconditionally force add_request to emit a full flush. */
> >-	ring->gpu_caches_dirty = true;
> >+	params->ring->gpu_caches_dirty = true;
> >
> >  	/* Add a breadcrumb for the completion of the batch buffer */
> >-	(void)__i915_add_request(ring, file, obj);
> >+	return __i915_add_request(params->ring, params->file,
> >+				  params->batch_obj);
> >  }
> >
> >  static int
> >@@ -1282,8 +1280,8 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params,
> >  	trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), params->dispatch_flags);
> >
> >  	i915_gem_execbuffer_move_to_active(vmas, ring);
> >-	i915_gem_execbuffer_retire_commands(params->dev, params->file, ring,
> >-					    params->batch_obj);
> >+
> >+	ret = i915_gem_execbuffer_retire_commands(params);
> >
> >  error:
> >  	kfree(cliprects);
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index f6a24e8a2..dc474b4 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -715,9 +715,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
> >  	trace_i915_gem_ring_dispatch(intel_ring_get_request(ring), params->dispatch_flags);
> >
> >  	i915_gem_execbuffer_move_to_active(vmas, ring);
> >-	i915_gem_execbuffer_retire_commands(params->dev, params->file, ring, params->batch_obj);
> >
> >-	return 0;
> >+	return i915_gem_execbuffer_retire_commands(params);
> >  }
> >
> >  void intel_execlists_retire_requests(struct intel_engine_cs *ring)
> >
> 
> Reviewed-by: Tomas Elf <tomas.elf at intel.com>

Patch discussion unfortunately split since I've made all my comments on
the earlier iteration. Anyway there's a big discussion there, you might
want to look into that and reconsder your r-b ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch