On Thu, Mar 05, 2015 at 02:45:11PM +0000, Tomas Elf wrote:
> On 19/02/2015 17:17, John.C.Harrison@xxxxxxxxx wrote:
> >From: John Harrison <John.C.Harrison@xxxxxxxxx>
> >
> >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@xxxxxxxxx>
> >---
> > 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@xxxxxxxxx>
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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
