public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Propagate error from drm_gem_object_init()
  2015-09-23 11:32 [PATCH 5/5] drm/i915: Propagating correct error codes to the userspace Chris Wilson
@ 2015-09-23 13:14 ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2015-09-23 13:14 UTC (permalink / raw)
  To: intel-gfx

Propagate the real error from drm_gem_object_init(). Note this also
fixes some confusion in the error return from i915_gem_alloc_object...

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem.c         | 12 +++++++-----
 drivers/gpu/drm/i915/i915_gem_context.c |  4 ++--
 drivers/gpu/drm/i915/intel_display.c    |  4 ++--
 drivers/gpu/drm/i915/intel_fbdev.c      |  2 +-
 drivers/gpu/drm/i915/intel_overlay.c    |  2 +-
 5 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1455bb0ab668..6eabab5c96c4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -379,8 +379,8 @@ i915_gem_create(struct drm_file *file,
 
 	/* Allocate the new object */
 	obj = i915_gem_alloc_object(dev, size);
-	if (obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, &handle);
 	/* drop reference from allocate - handle holds it now */
@@ -4303,14 +4303,16 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 	struct address_space *mapping;
 	gfp_t mask;
+	int ret;
 
 	obj = i915_gem_object_alloc(dev);
 	if (obj == NULL)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
-	if (drm_gem_object_init(dev, &obj->base, size) != 0) {
+	ret = drm_gem_object_init(dev, &obj->base, size);
+	if (ret) {
 		i915_gem_object_free(obj);
-		return NULL;
+		return ERR_PTR(ret);
 	}
 
 	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b01137a445ff..e3aae52752c5 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -235,8 +235,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
 	int ret;
 
 	obj = i915_gem_alloc_object(dev, size);
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return obj;
 
 	/*
 	 * Try to make the context utilize L3 as well as LLC.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3ad3c10a8a03..0a8e83eb91a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10121,8 +10121,8 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
 
 	obj = i915_gem_alloc_object(dev,
 				    intel_framebuffer_size_for_mode(mode, bpp));
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
 
 	mode_cmd.width = mode->hdisplay;
 	mode_cmd.height = mode->vdisplay;
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index d3e2201e0734..e3892f71467d 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -143,7 +143,7 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
 	obj = i915_gem_object_create_stolen(dev, size);
 	if (obj == NULL)
 		obj = i915_gem_alloc_object(dev, size);
-	if (!obj) {
+	if (IS_ERR_OR_NULL(obj)) {
 		DRM_ERROR("failed to allocate framebuffer\n");
 		ret = -ENOMEM;
 		goto out;
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 3b7ea47f963b..b443a0bd889a 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1397,7 +1397,7 @@ void intel_setup_overlay(struct drm_device *dev)
 		reg_bo = i915_gem_object_create_stolen(dev, PAGE_SIZE);
 	if (reg_bo == NULL)
 		reg_bo = i915_gem_alloc_object(dev, PAGE_SIZE);
-	if (reg_bo == NULL)
+	if (IS_ERR_OR_NULL(reg_bo))
 		goto out_free;
 	overlay->reg_bo = reg_bo;
 
-- 
2.5.3

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH] drm/i915: Propagate error from drm_gem_object_init()
@ 2016-04-22 10:57 Matthew Auld
  2016-04-22 11:59 ` Dave Gordon
  2016-04-24  6:57 ` ✗ Fi.CI.BAT: failure for " Patchwork
  0 siblings, 2 replies; 6+ messages in thread
From: Matthew Auld @ 2016-04-22 10:57 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

Propagate the real error from drm_gem_object_init(). Note this also
fixes some confusion in the error return from i915_gem_alloc_object...

v2:
(Matthew Auld)
  - updated new users of gem_alloc_object from latest drm-nightly
  - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
v3:
(Joonas Lahtinen)
  - fix double "From:" in commit message
  - add goto teardown path

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c              | 21 +++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_batch_pool.c   |  4 ++--
 drivers/gpu/drm/i915/i915_gem_context.c      |  4 ++--
 drivers/gpu/drm/i915/i915_gem_render_state.c |  7 +++++--
 drivers/gpu/drm/i915/i915_guc_submission.c   |  2 +-
 drivers/gpu/drm/i915/intel_display.c         |  4 ++--
 drivers/gpu/drm/i915/intel_fbdev.c           |  4 ++--
 drivers/gpu/drm/i915/intel_lrc.c             | 10 ++++++----
 drivers/gpu/drm/i915/intel_overlay.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c      | 19 ++++++++++---------
 10 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 261a3ef..c6c17dd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -382,8 +382,8 @@ i915_gem_create(struct drm_file *file,
 
 	/* Allocate the new object */
 	obj = i915_gem_alloc_object(dev, size);
-	if (obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, &handle);
 	/* drop reference from allocate - handle holds it now */
@@ -4498,15 +4498,15 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 	struct address_space *mapping;
 	gfp_t mask;
+	int ret;
 
 	obj = i915_gem_object_alloc(dev);
 	if (obj == NULL)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
-	if (drm_gem_object_init(dev, &obj->base, size) != 0) {
-		i915_gem_object_free(obj);
-		return NULL;
-	}
+	ret = drm_gem_object_init(dev, &obj->base, size);
+	if (ret)
+		goto fail;
 
 	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
 	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
@@ -4543,6 +4543,11 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
 	trace_i915_gem_object_create(obj);
 
 	return obj;
+
+fail:
+	i915_gem_object_free(obj);
+
+	return ERR_PTR(ret);
 }
 
 static bool discard_backing_storage(struct drm_i915_gem_object *obj)
@@ -5362,7 +5367,7 @@ i915_gem_object_create_from_data(struct drm_device *dev,
 	int ret;
 
 	obj = i915_gem_alloc_object(dev, round_up(size, PAGE_SIZE));
-	if (IS_ERR_OR_NULL(obj))
+	if (IS_ERR(obj))
 		return obj;
 
 	ret = i915_gem_object_set_to_cpu_domain(obj, true);
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
index 7bf2f3f..d79caa2 100644
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
@@ -135,8 +135,8 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool,
 		int ret;
 
 		obj = i915_gem_alloc_object(pool->dev, size);
-		if (obj == NULL)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(obj))
+			return obj;
 
 		ret = i915_gem_object_get_pages(obj);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index e5acc39..34f4930 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -179,8 +179,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
 	int ret;
 
 	obj = i915_gem_alloc_object(dev, size);
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return obj;
 
 	/*
 	 * Try to make the context utilize L3 as well as LLC.
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 71611bf..071c11e 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -58,8 +58,11 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
 		return -EINVAL;
 
 	so->obj = i915_gem_alloc_object(dev, 4096);
-	if (so->obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(so->obj)) {
+		ret = PTR_ERR(so->obj);
+		so->obj = NULL;
+		return ret;
+	}
 
 	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index d40c13f..85122d9 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -588,7 +588,7 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 
 	obj = i915_gem_alloc_object(dev, size);
-	if (!obj)
+	if (IS_ERR(obj))
 		return NULL;
 
 	if (i915_gem_object_get_pages(obj)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ff60241..5aff4ca 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10320,8 +10320,8 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
 
 	obj = i915_gem_alloc_object(dev,
 				    intel_framebuffer_size_for_mode(mode, bpp));
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
 
 	mode_cmd.width = mode->hdisplay;
 	mode_cmd.height = mode->vdisplay;
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index 79ac202..7047b38 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -151,9 +151,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
 		obj = i915_gem_object_create_stolen(dev, size);
 	if (obj == NULL)
 		obj = i915_gem_alloc_object(dev, size);
-	if (!obj) {
+	if (IS_ERR(obj)) {
 		DRM_ERROR("failed to allocate framebuffer\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(obj);
 		goto out;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6179b59..1983198 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1474,9 +1474,11 @@ static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *engine, u32 size)
 
 	engine->wa_ctx.obj = i915_gem_alloc_object(engine->dev,
 						   PAGE_ALIGN(size));
-	if (!engine->wa_ctx.obj) {
+	if (IS_ERR(engine->wa_ctx.obj)) {
 		DRM_DEBUG_DRIVER("alloc LRC WA ctx backing obj failed.\n");
-		return -ENOMEM;
+		ret = PTR_ERR(engine->wa_ctx.obj);
+		engine->wa_ctx.obj = NULL;
+		return ret;
 	}
 
 	ret = i915_gem_obj_ggtt_pin(engine->wa_ctx.obj, PAGE_SIZE, 0);
@@ -2666,9 +2668,9 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 	context_size += PAGE_SIZE * LRC_PPHWSP_PN;
 
 	ctx_obj = i915_gem_alloc_object(dev, context_size);
-	if (!ctx_obj) {
+	if (IS_ERR(ctx_obj)) {
 		DRM_DEBUG_DRIVER("Alloc LRC backing obj failed.\n");
-		return -ENOMEM;
+		return PTR_ERR(ctx_obj);
 	}
 
 	ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index bcc3b6a..0d4bcbe 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1397,7 +1397,7 @@ void intel_setup_overlay(struct drm_device *dev)
 		reg_bo = i915_gem_object_create_stolen(dev, PAGE_SIZE);
 	if (reg_bo == NULL)
 		reg_bo = i915_gem_alloc_object(dev, PAGE_SIZE);
-	if (reg_bo == NULL)
+	if (IS_ERR(reg_bo))
 		goto out_free;
 	overlay->reg_bo = reg_bo;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 245386e..29094ef 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -672,9 +672,10 @@ intel_init_pipe_control(struct intel_engine_cs *engine)
 	WARN_ON(engine->scratch.obj);
 
 	engine->scratch.obj = i915_gem_alloc_object(engine->dev, 4096);
-	if (engine->scratch.obj == NULL) {
+	if (IS_ERR(engine->scratch.obj)) {
 		DRM_ERROR("Failed to allocate seqno page\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(engine->scratch.obj);
+		engine->scratch.obj = NULL;
 		goto err;
 	}
 
@@ -2023,9 +2024,9 @@ static int init_status_page(struct intel_engine_cs *engine)
 		int ret;
 
 		obj = i915_gem_alloc_object(engine->dev, 4096);
-		if (obj == NULL) {
+		if (IS_ERR(obj)) {
 			DRM_ERROR("Failed to allocate status page\n");
-			return -ENOMEM;
+			return PTR_ERR(obj);
 		}
 
 		ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
@@ -2164,8 +2165,8 @@ static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
 		obj = i915_gem_object_create_stolen(dev, ringbuf->size);
 	if (obj == NULL)
 		obj = i915_gem_alloc_object(dev, ringbuf->size);
-	if (obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
 
 	/* mark ring buffers as read-only from GPU side by default */
 	obj->gt_ro = 1;
@@ -2777,7 +2778,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen >= 8) {
 		if (i915_semaphore_is_enabled(dev)) {
 			obj = i915_gem_alloc_object(dev, 4096);
-			if (obj == NULL) {
+			if (IS_ERR(obj)) {
 				DRM_ERROR("Failed to allocate semaphore bo. Disabling semaphores\n");
 				i915.semaphores = 0;
 			} else {
@@ -2886,9 +2887,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	/* Workaround batchbuffer to combat CS tlb bug. */
 	if (HAS_BROKEN_CS_TLB(dev)) {
 		obj = i915_gem_alloc_object(dev, I830_WA_SIZE);
-		if (obj == NULL) {
+		if (IS_ERR(obj)) {
 			DRM_ERROR("Failed to allocate batch bo\n");
-			return -ENOMEM;
+			return PTR_ERR(obj);
 		}
 
 		ret = i915_gem_obj_ggtt_pin(obj, 0, 0);
-- 
2.5.5

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Propagate error from drm_gem_object_init()
  2016-04-22 10:57 [PATCH] drm/i915: Propagate error from drm_gem_object_init() Matthew Auld
@ 2016-04-22 11:59 ` Dave Gordon
  2016-04-22 13:44   ` Joonas Lahtinen
  2016-04-24  6:57 ` ✗ Fi.CI.BAT: failure for " Patchwork
  1 sibling, 1 reply; 6+ messages in thread
From: Dave Gordon @ 2016-04-22 11:59 UTC (permalink / raw)
  To: intel-gfx, Chris Wilson

On 22/04/16 11:57, Matthew Auld wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> Propagate the real error from drm_gem_object_init(). Note this also
> fixes some confusion in the error return from i915_gem_alloc_object...
>
> v2:
> (Matthew Auld)
>    - updated new users of gem_alloc_object from latest drm-nightly
>    - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
> v3:
> (Joonas Lahtinen)
>    - fix double "From:" in commit message
>    - add goto teardown path
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem.c              | 21 +++++++++++++--------
>   drivers/gpu/drm/i915/i915_gem_batch_pool.c   |  4 ++--
>   drivers/gpu/drm/i915/i915_gem_context.c      |  4 ++--
>   drivers/gpu/drm/i915/i915_gem_render_state.c |  7 +++++--
>   drivers/gpu/drm/i915/i915_guc_submission.c   |  2 +-
>   drivers/gpu/drm/i915/intel_display.c         |  4 ++--
>   drivers/gpu/drm/i915/intel_fbdev.c           |  4 ++--
>   drivers/gpu/drm/i915/intel_lrc.c             | 10 ++++++----
>   drivers/gpu/drm/i915/intel_overlay.c         |  2 +-
>   drivers/gpu/drm/i915/intel_ringbuffer.c      | 19 ++++++++++---------
>   10 files changed, 44 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 261a3ef..c6c17dd 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -382,8 +382,8 @@ i915_gem_create(struct drm_file *file,
>
>   	/* Allocate the new object */
>   	obj = i915_gem_alloc_object(dev, size);
> -	if (obj == NULL)
> -		return -ENOMEM;
> +	if (IS_ERR(obj))
> +		return PTR_ERR(obj);
>
>   	ret = drm_gem_handle_create(file, &obj->base, &handle);
>   	/* drop reference from allocate - handle holds it now */
> @@ -4498,15 +4498,15 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
>   	struct drm_i915_gem_object *obj;
>   	struct address_space *mapping;
>   	gfp_t mask;
> +	int ret;
>
>   	obj = i915_gem_object_alloc(dev);
>   	if (obj == NULL)
> -		return NULL;
> +		return ERR_PTR(-ENOMEM);

The two changes above looked really really confusing, where one tests 
the returned pointer and returns it if it's an ERR_PTR, and the other 
tests for NULL and returns ERR_PTR(-ENOMEM).

Then I realised one was i915_gem_alloc_object() and the other was 
i915_gem_object_alloc()!

Can we please get rid of one or the other? Since we generally use 
subsystem_class_action naming, I'd suggest keeping (the low-level 
memory-allocator) i915_gem_object_alloc(), and renaming the high-level 
i915_gem_alloc_object() to i915_gem_object_create() or similar.

> -	if (drm_gem_object_init(dev, &obj->base, size) != 0) {
> -		i915_gem_object_free(obj);
> -		return NULL;
> -	}
> +	ret = drm_gem_object_init(dev, &obj->base, size);
> +	if (ret)
> +		goto fail;
>
>   	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
>   	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> @@ -4543,6 +4543,11 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
>   	trace_i915_gem_object_create(obj);

Oh and BTW i915_gem_alloc_object() already calls itself 
i915_gem_object_create() in trace messages!

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Propagate error from drm_gem_object_init()
  2016-04-22 11:59 ` Dave Gordon
@ 2016-04-22 13:44   ` Joonas Lahtinen
  0 siblings, 0 replies; 6+ messages in thread
From: Joonas Lahtinen @ 2016-04-22 13:44 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx, Chris Wilson, Matthew Auld

On pe, 2016-04-22 at 12:59 +0100, Dave Gordon wrote:
> On 22/04/16 11:57, Matthew Auld wrote:
> > 
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Propagate the real error from drm_gem_object_init(). Note this also
> > fixes some confusion in the error return from i915_gem_alloc_object...
> > 
> > v2:
> > (Matthew Auld)
> >    - updated new users of gem_alloc_object from latest drm-nightly
> >    - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
> > v3:
> > (Joonas Lahtinen)
> >    - fix double "From:" in commit message
> >    - add goto teardown path
> > 
> > Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/i915_gem.c              | 21 +++++++++++++--------
> >   drivers/gpu/drm/i915/i915_gem_batch_pool.c   |  4 ++--
> >   drivers/gpu/drm/i915/i915_gem_context.c      |  4 ++--
> >   drivers/gpu/drm/i915/i915_gem_render_state.c |  7 +++++--
> >   drivers/gpu/drm/i915/i915_guc_submission.c   |  2 +-
> >   drivers/gpu/drm/i915/intel_display.c         |  4 ++--
> >   drivers/gpu/drm/i915/intel_fbdev.c           |  4 ++--
> >   drivers/gpu/drm/i915/intel_lrc.c             | 10 ++++++----
> >   drivers/gpu/drm/i915/intel_overlay.c         |  2 +-
> >   drivers/gpu/drm/i915/intel_ringbuffer.c      | 19 ++++++++++---------
> >   10 files changed, 44 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index 261a3ef..c6c17dd 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -382,8 +382,8 @@ i915_gem_create(struct drm_file *file,
> > 
> >   	/* Allocate the new object */
> >   	obj = i915_gem_alloc_object(dev, size);
> > -	if (obj == NULL)
> > -		return -ENOMEM;
> > +	if (IS_ERR(obj))
> > +		return PTR_ERR(obj);
> > 
> >   	ret = drm_gem_handle_create(file, &obj->base, &handle);
> >   	/* drop reference from allocate - handle holds it now */
> > @@ -4498,15 +4498,15 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> >   	struct drm_i915_gem_object *obj;
> >   	struct address_space *mapping;
> >   	gfp_t mask;
> > +	int ret;
> > 
> >   	obj = i915_gem_object_alloc(dev);
> >   	if (obj == NULL)
> > -		return NULL;
> > +		return ERR_PTR(-ENOMEM);
> The two changes above looked really really confusing, where one tests 
> the returned pointer and returns it if it's an ERR_PTR, and the other 
> tests for NULL and returns ERR_PTR(-ENOMEM).
> 
> Then I realised one was i915_gem_alloc_object() and the other was 
> i915_gem_object_alloc()!

Yep, noticed that a few days ago too, +1 on correcting it.

Regards, Joonas

> 
> Can we please get rid of one or the other? Since we generally use 
> subsystem_class_action naming, I'd suggest keeping (the low-level 
> memory-allocator) i915_gem_object_alloc(), and renaming the high-level 
> i915_gem_alloc_object() to i915_gem_object_create() or similar.
> 
> > 
> > -	if (drm_gem_object_init(dev, &obj->base, size) != 0) {
> > -		i915_gem_object_free(obj);
> > -		return NULL;
> > -	}
> > +	ret = drm_gem_object_init(dev, &obj->base, size);
> > +	if (ret)
> > +		goto fail;
> > 
> >   	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
> >   	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
> > @@ -4543,6 +4543,11 @@ struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
> >   	trace_i915_gem_object_create(obj);
> Oh and BTW i915_gem_alloc_object() already calls itself 
> i915_gem_object_create() in trace messages!
> 
> .Dave.
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: Propagate error from drm_gem_object_init()
  2016-04-22 10:57 [PATCH] drm/i915: Propagate error from drm_gem_object_init() Matthew Auld
  2016-04-22 11:59 ` Dave Gordon
@ 2016-04-24  6:57 ` Patchwork
  1 sibling, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-04-24  6:57 UTC (permalink / raw)
  To: Matthew Auld; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Propagate error from drm_gem_object_init()
URL   : https://patchwork.freedesktop.org/series/6149/
State : failure

== Summary ==

Series 6149v1 drm/i915: Propagate error from drm_gem_object_init()
http://patchwork.freedesktop.org/api/1.0/series/6149/revisions/1/mbox/

Test drv_module_reload_basic:
                pass       -> INCOMPLETE (skl-nuci5)

bdw-ultra        total:193  pass:170  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:192  pass:153  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:192  pass:154  dwarn:0   dfail:0   fail:0   skip:38 
ilk-hp8440p      total:193  pass:136  dwarn:0   dfail:0   fail:0   skip:57 
ivb-t430s        total:193  pass:165  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:193  pass:168  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:111  pass:103  dwarn:0   dfail:0   fail:0   skip:7  
snb-dellxps      total:193  pass:155  dwarn:0   dfail:0   fail:0   skip:38 

Results at /archive/results/CI_IGT_test/Patchwork_2029/

340c485ad98d0ec0369a3b18d4a09938f3f5537d drm-intel-nightly: 2016y-04m-22d-17h-32m-25s UTC integration manifest
0836141 drm/i915: Propagate error from drm_gem_object_init()

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH] drm/i915: Propagate error from drm_gem_object_init()
@ 2016-04-25 12:32 Matthew Auld
  0 siblings, 0 replies; 6+ messages in thread
From: Matthew Auld @ 2016-04-25 12:32 UTC (permalink / raw)
  To: intel-gfx

From: Chris Wilson <chris@chris-wilson.co.uk>

Propagate the real error from drm_gem_object_init(). Note this also
fixes some confusion in the error return from i915_gem_alloc_object...

v2:
(Matthew Auld)
  - updated new users of gem_alloc_object from latest drm-nightly
  - replaced occurrences of IS_ERR_OR_NULL() with IS_ERR()
v3:
(Joonas Lahtinen)
  - fix double "From:" in commit message
  - add goto teardown path
v4:
(Matthew Auld)
  - rebase with i915_gem_alloc_object name change

Signed-off-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c              | 21 +++++++++++++--------
 drivers/gpu/drm/i915/i915_gem_batch_pool.c   |  4 ++--
 drivers/gpu/drm/i915/i915_gem_context.c      |  4 ++--
 drivers/gpu/drm/i915/i915_gem_render_state.c |  7 +++++--
 drivers/gpu/drm/i915/i915_guc_submission.c   |  2 +-
 drivers/gpu/drm/i915/intel_display.c         |  4 ++--
 drivers/gpu/drm/i915/intel_fbdev.c           |  4 ++--
 drivers/gpu/drm/i915/intel_lrc.c             | 10 ++++++----
 drivers/gpu/drm/i915/intel_overlay.c         |  2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c      | 19 ++++++++++---------
 10 files changed, 44 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index d493e79..f749065 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -382,8 +382,8 @@ i915_gem_create(struct drm_file *file,
 
 	/* Allocate the new object */
 	obj = i915_gem_object_create(dev, size);
-	if (obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, &handle);
 	/* drop reference from allocate - handle holds it now */
@@ -4498,15 +4498,15 @@ struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 	struct address_space *mapping;
 	gfp_t mask;
+	int ret;
 
 	obj = i915_gem_object_alloc(dev);
 	if (obj == NULL)
-		return NULL;
+		return ERR_PTR(-ENOMEM);
 
-	if (drm_gem_object_init(dev, &obj->base, size) != 0) {
-		i915_gem_object_free(obj);
-		return NULL;
-	}
+	ret = drm_gem_object_init(dev, &obj->base, size);
+	if (ret)
+		goto fail;
 
 	mask = GFP_HIGHUSER | __GFP_RECLAIMABLE;
 	if (IS_CRESTLINE(dev) || IS_BROADWATER(dev)) {
@@ -4543,6 +4543,11 @@ struct drm_i915_gem_object *i915_gem_object_create(struct drm_device *dev,
 	trace_i915_gem_object_create(obj);
 
 	return obj;
+
+fail:
+	i915_gem_object_free(obj);
+
+	return ERR_PTR(ret);
 }
 
 static bool discard_backing_storage(struct drm_i915_gem_object *obj)
@@ -5348,7 +5353,7 @@ i915_gem_object_create_from_data(struct drm_device *dev,
 	int ret;
 
 	obj = i915_gem_object_create(dev, round_up(size, PAGE_SIZE));
-	if (IS_ERR_OR_NULL(obj))
+	if (IS_ERR(obj))
 		return obj;
 
 	ret = i915_gem_object_set_to_cpu_domain(obj, true);
diff --git a/drivers/gpu/drm/i915/i915_gem_batch_pool.c b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
index 1bc46ba..3752d5d 100644
--- a/drivers/gpu/drm/i915/i915_gem_batch_pool.c
+++ b/drivers/gpu/drm/i915/i915_gem_batch_pool.c
@@ -135,8 +135,8 @@ i915_gem_batch_pool_get(struct i915_gem_batch_pool *pool,
 		int ret;
 
 		obj = i915_gem_object_create(pool->dev, size);
-		if (obj == NULL)
-			return ERR_PTR(-ENOMEM);
+		if (IS_ERR(obj))
+			return obj;
 
 		ret = i915_gem_object_get_pages(obj);
 		if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 4e12bae..e78e2f6 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -179,8 +179,8 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
 	int ret;
 
 	obj = i915_gem_object_create(dev, size);
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return obj;
 
 	/*
 	 * Try to make the context utilize L3 as well as LLC.
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 65c9dad..841081e 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -58,8 +58,11 @@ static int render_state_init(struct render_state *so, struct drm_device *dev)
 		return -EINVAL;
 
 	so->obj = i915_gem_object_create(dev, 4096);
-	if (so->obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(so->obj)) {
+		ret = PTR_ERR(so->obj);
+		so->obj = NULL;
+		return ret;
+	}
 
 	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
 	if (ret)
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 72d6665..a304b0e 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -588,7 +588,7 @@ static struct drm_i915_gem_object *gem_allocate_guc_obj(struct drm_device *dev,
 	struct drm_i915_gem_object *obj;
 
 	obj = i915_gem_object_create(dev, size);
-	if (!obj)
+	if (IS_ERR(obj))
 		return NULL;
 
 	if (i915_gem_object_get_pages(obj)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b7cb632..739c12f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10320,8 +10320,8 @@ intel_framebuffer_create_for_mode(struct drm_device *dev,
 
 	obj = i915_gem_object_create(dev,
 				    intel_framebuffer_size_for_mode(mode, bpp));
-	if (obj == NULL)
-		return ERR_PTR(-ENOMEM);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
 
 	mode_cmd.width = mode->hdisplay;
 	mode_cmd.height = mode->vdisplay;
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index af56154..37fc68e 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -151,9 +151,9 @@ static int intelfb_alloc(struct drm_fb_helper *helper,
 		obj = i915_gem_object_create_stolen(dev, size);
 	if (obj == NULL)
 		obj = i915_gem_object_create(dev, size);
-	if (!obj) {
+	if (IS_ERR(obj)) {
 		DRM_ERROR("failed to allocate framebuffer\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(obj);
 		goto out;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2b7e6bb..1b065e7 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1474,9 +1474,11 @@ static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *engine, u32 size)
 
 	engine->wa_ctx.obj = i915_gem_object_create(engine->dev,
 						   PAGE_ALIGN(size));
-	if (!engine->wa_ctx.obj) {
+	if (IS_ERR(engine->wa_ctx.obj)) {
 		DRM_DEBUG_DRIVER("alloc LRC WA ctx backing obj failed.\n");
-		return -ENOMEM;
+		ret = PTR_ERR(engine->wa_ctx.obj);
+		engine->wa_ctx.obj = NULL;
+		return ret;
 	}
 
 	ret = i915_gem_obj_ggtt_pin(engine->wa_ctx.obj, PAGE_SIZE, 0);
@@ -2666,9 +2668,9 @@ int intel_lr_context_deferred_alloc(struct intel_context *ctx,
 	context_size += PAGE_SIZE * LRC_PPHWSP_PN;
 
 	ctx_obj = i915_gem_object_create(dev, context_size);
-	if (!ctx_obj) {
+	if (IS_ERR(ctx_obj)) {
 		DRM_DEBUG_DRIVER("Alloc LRC backing obj failed.\n");
-		return -ENOMEM;
+		return PTR_ERR(ctx_obj);
 	}
 
 	ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c
index 597fbcd..6b016c9 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -1397,7 +1397,7 @@ void intel_setup_overlay(struct drm_device *dev)
 		reg_bo = i915_gem_object_create_stolen(dev, PAGE_SIZE);
 	if (reg_bo == NULL)
 		reg_bo = i915_gem_object_create(dev, PAGE_SIZE);
-	if (reg_bo == NULL)
+	if (IS_ERR(reg_bo))
 		goto out_free;
 	overlay->reg_bo = reg_bo;
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 66f69cd..aff26be 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -672,9 +672,10 @@ intel_init_pipe_control(struct intel_engine_cs *engine)
 	WARN_ON(engine->scratch.obj);
 
 	engine->scratch.obj = i915_gem_object_create(engine->dev, 4096);
-	if (engine->scratch.obj == NULL) {
+	if (IS_ERR(engine->scratch.obj)) {
 		DRM_ERROR("Failed to allocate seqno page\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(engine->scratch.obj);
+		engine->scratch.obj = NULL;
 		goto err;
 	}
 
@@ -2027,9 +2028,9 @@ static int init_status_page(struct intel_engine_cs *engine)
 		int ret;
 
 		obj = i915_gem_object_create(engine->dev, 4096);
-		if (obj == NULL) {
+		if (IS_ERR(obj)) {
 			DRM_ERROR("Failed to allocate status page\n");
-			return -ENOMEM;
+			return PTR_ERR(obj);
 		}
 
 		ret = i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
@@ -2168,8 +2169,8 @@ static int intel_alloc_ringbuffer_obj(struct drm_device *dev,
 		obj = i915_gem_object_create_stolen(dev, ringbuf->size);
 	if (obj == NULL)
 		obj = i915_gem_object_create(dev, ringbuf->size);
-	if (obj == NULL)
-		return -ENOMEM;
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
 
 	/* mark ring buffers as read-only from GPU side by default */
 	obj->gt_ro = 1;
@@ -2781,7 +2782,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	if (INTEL_INFO(dev)->gen >= 8) {
 		if (i915_semaphore_is_enabled(dev)) {
 			obj = i915_gem_object_create(dev, 4096);
-			if (obj == NULL) {
+			if (IS_ERR(obj)) {
 				DRM_ERROR("Failed to allocate semaphore bo. Disabling semaphores\n");
 				i915.semaphores = 0;
 			} else {
@@ -2890,9 +2891,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
 	/* Workaround batchbuffer to combat CS tlb bug. */
 	if (HAS_BROKEN_CS_TLB(dev)) {
 		obj = i915_gem_object_create(dev, I830_WA_SIZE);
-		if (obj == NULL) {
+		if (IS_ERR(obj)) {
 			DRM_ERROR("Failed to allocate batch bo\n");
-			return -ENOMEM;
+			return PTR_ERR(obj);
 		}
 
 		ret = i915_gem_obj_ggtt_pin(obj, 0, 0);
-- 
2.4.11

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-25 12:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-22 10:57 [PATCH] drm/i915: Propagate error from drm_gem_object_init() Matthew Auld
2016-04-22 11:59 ` Dave Gordon
2016-04-22 13:44   ` Joonas Lahtinen
2016-04-24  6:57 ` ✗ Fi.CI.BAT: failure for " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2016-04-25 12:32 [PATCH] " Matthew Auld
2015-09-23 11:32 [PATCH 5/5] drm/i915: Propagating correct error codes to the userspace Chris Wilson
2015-09-23 13:14 ` [PATCH] drm/i915: Propagate error from drm_gem_object_init() Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox