On ti, 2016-04-12 at 16:57 +0100, Matthew Auld wrote: > From: Chris Wilson <[email protected]> > > From: Chris Wilson <[email protected]>
Double "From: ", one should be enough. > > 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() > > Signed-off-by: Matthew Auld <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++------ > 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, 39 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index b37ffea..6a6998c 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -387,8 +387,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 */ > @@ -4527,14 +4527,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); While at it, I'd make this have a goto teardown path. > } > > mask = GFP_HIGHUSER | __GFP_RECLAIMABLE; > @@ -5390,7 +5392,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 fe580cb..a5adc66 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; > + } The teardown path in this function leaves so->obj != NULL in later failure path. Also i915_gem_alloc_object has only drm_gem_object_unreference as a counterpart so it'll leak too. Should be fixed in a follow-up patch. > > 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 da86bdb..004f8f7 100644 > --- a/drivers/gpu/drm/i915/i915_guc_submission.c > +++ b/drivers/gpu/drm/i915/i915_guc_submission.c > @@ -617,7 +617,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 551541b303..2247bdb 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10283,8 +10283,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 0d6dc5e..c0543bb 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1483,9 +1483,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); > @@ -2638,9 +2640,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 6694e92..77ee12f 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 41b604e..b20cf91 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; > } > > @@ -2021,9 +2022,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); > @@ -2156,8 +2157,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; > @@ -2780,7 +2781,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 { > @@ -2889,9 +2890,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); Some comments above. Reviewed-by: Joonas Lahtinen <[email protected]> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
