On 28/09/2016 18:09, Chris Wilson wrote:
On Wed, Sep 28, 2016 at 05:48:23PM +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)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> (v1)
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 +++++++++++++++-----
drivers/gpu/drm/i915/i915_gem_request.c | 25 -------------------------
drivers/gpu/drm/i915/i915_gem_request.h | 2 --
3 files changed, 15 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 33c85227643d..20dc7d90cecf 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1612,6 +1612,19 @@ eb_select_engine(struct drm_i915_private *dev_priv,
return engine;
}
+static void i915_gem_request_add_to_client(struct drm_i915_gem_request *req,
+ struct drm_file *file)
Just add_to_client.
Ok.
+{
+ struct drm_i915_file_private *file_priv = file->driver_priv;
+
+ GEM_BUG_ON(req->file_priv);
The error isn't associated with execbuf. The explosion will happen
elsewhere and will be from a path that doesn't go near execbuf.
I did not even spot it while looking. Will remove.
+ 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 +1837,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;
+ i915_gem_request_add_to_client(params->request, file);
/*
* Save assorted stuff away to pass through to *_submission().
@@ -1841,8 +1852,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
params->ctx = ctx;
ret = execbuf_submit(params, args, &eb->vmas);
-err_request:
- __i915_add_request(params->request, ret == 0);
+ __i915_add_request(params->request, true);
We don't want to force the flush if submit fails either.
Embarrassing. :I
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
