Re: [PATCH] drm/i915: tidy up request alloc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 30/06/16 11:22, Chris Wilson wrote:
On Thu, Jun 30, 2016 at 09:50:20AM +0100, Tvrtko Ursulin wrote:

On 30/06/16 02:35, Hong Liu wrote:
Return the allocated request pointer directly to remove
the double pointer parameter.

Signed-off-by: Hong Liu <hong.liu@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c | 25 +++++++------------------
  1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1d98782..9881455 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2988,32 +2988,26 @@ void i915_gem_request_free(struct kref *req_ref)
  	kmem_cache_free(req->i915->requests, req);
  }

-static inline int
+static inline struct drm_i915_gem_request *
  __i915_gem_request_alloc(struct intel_engine_cs *engine,
-			 struct i915_gem_context *ctx,
-			 struct drm_i915_gem_request **req_out)
+			 struct i915_gem_context *ctx)
  {
  	struct drm_i915_private *dev_priv = engine->i915;
  	unsigned reset_counter = i915_reset_counter(&dev_priv->gpu_error);
  	struct drm_i915_gem_request *req;
  	int ret;

-	if (!req_out)
-		return -EINVAL;
-
-	*req_out = NULL;
-
  	/* ABI: Before userspace accesses the GPU (e.g. execbuffer), report
  	 * EIO if the GPU is already wedged, or EAGAIN to drop the struct_mutex
  	 * and restart.
  	 */
  	ret = i915_gem_check_wedge(reset_counter, dev_priv->mm.interruptible);
  	if (ret)
-		return ret;
+		return ERR_PTR(ret);

  	req = kmem_cache_zalloc(dev_priv->requests, GFP_KERNEL);
  	if (req == NULL)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);

  	ret = i915_gem_get_seqno(engine->i915, &req->seqno);
  	if (ret)
@@ -3041,14 +3035,13 @@ __i915_gem_request_alloc(struct intel_engine_cs *engine,
  	if (ret)
  		goto err_ctx;

-	*req_out = req;
-	return 0;
+	return req;

  err_ctx:
  	i915_gem_context_unreference(ctx);
  err:
  	kmem_cache_free(dev_priv->requests, req);
-	return ret;
+	return ERR_PTR(ret);
  }

  /**
@@ -3067,13 +3060,9 @@ struct drm_i915_gem_request *
  i915_gem_request_alloc(struct intel_engine_cs *engine,
  		       struct i915_gem_context *ctx)
  {
-	struct drm_i915_gem_request *req;
-	int err;
-
  	if (ctx == NULL)
  		ctx = engine->i915->kernel_context;
-	err = __i915_gem_request_alloc(engine, ctx, &req);
-	return err ? ERR_PTR(err) : req;
+	return __i915_gem_request_alloc(engine, ctx);
  }

  struct drm_i915_gem_request *


Looks good to me. And have this feeling I've seen this somewhere before.

Several times. This is not the full tidy, nor does it realise the
ramifactions of request alloc through the stack.

Hm I can't spot that it is doing anything wrong or making anything worse. You don't want to let the small cleanup in?

Regards,

Tvrtko



_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux