On 22/04/16 11:57, Matthew Auld wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
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@xxxxxxxxx>
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
