On Thu, Sep 29, 2016 at 08:53:31AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> Several things we can clean up in this function:
>
> * Request and file passed in cannot be NULL. There is
> a single caller which makes it impossible so change
> that condition to a GEM_BUG_ON instead of a WARN
> with a return code.
>
> * Same is true for req->file_priv which is always
> zeroed before this function is called.
>
> * With the above we can remove the error return
> from the function making it void.
>
> * dev_private local variable was unused.
>
> * Call site in i915_gem_do_execbuffer can be
> simplified since there is no error handling any
> longer.
>
> v2:
> * Move next to the only caller. (Chris Wilson)
> * Remove useless asserts. (Joonas Lahtinen)
>
> v3:
> * Shorten name, remove last standing assert and
> fix flushing after failed submit. (Chris Wilson)
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> (v1)
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 ++++++++++++----
> drivers/gpu/drm/i915/i915_gem_request.c | 25 -------------------------
> drivers/gpu/drm/i915/i915_gem_request.h | 2 --
> 3 files changed, 12 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 33c85227643d..f7b96391ff66 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1612,6 +1612,17 @@ eb_select_engine(struct drm_i915_private *dev_priv,
> return engine;
> }
>
> +static void
> +add_to_client(struct drm_i915_gem_request *req, struct drm_file *file)
> +{
> + struct drm_i915_file_private *file_priv = file->driver_priv;
> +
> + spin_lock(&file_priv->mm.lock);
> + req->file_priv = file_priv;
> + list_add_tail(&req->client_list, &file_priv->mm.request_list);
> + spin_unlock(&file_priv->mm.lock);
> +}
> +
> static int
> i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> struct drm_file *file,
> @@ -1824,9 +1835,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
> */
> params->request->batch = params->batch;
>
> - ret = i915_gem_request_add_to_client(params->request, file);
> - if (ret)
> - goto err_request;
> + add_to_client(params->request, file);
So we add to the client list even if we fail to submit the batch? For
that reason I put the call to add_to_client() in execbuf_submit.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
